Created attachment 54046 [details] Serbian and regular cursive forms for some Cyrillic letters Compiling WebKitGtk with the default freetype backend results in "locl" tables in opentype fonts not being used. Compiling it with the pango backend instead makes it work. Example HTML that demonstrates the problem is the following: compare rendering in Firefox and Epiphany (when Serbian locale is active on your system, eg. set LC_ALL=sr_RS.UTF-8 in an env where you start firefox/epiphany from): <p style="font-style:italic; font-family:serif; font-size: 300%;" lang="sr">бгдпт</p> To clearly see what differences are expected, you can test it directly with pango tools: $ pango-view --font="DejaVu Serif Italic 64" --text "бгдпт" --language=sr $ pango-view --font="DejaVu Serif Italic 64" --text "бгдпт" --language=ru (output of these attached for reference) DejaVu Serif is one of the fonts that has such tables for Romanian and Serbian language. On Ubuntu (most likely Debian as well), it's in ttf-dejavu-extras package, and on Fedora I hear it's in dejavu-serif-fonts-2.30. Upstream is http://dejavu-fonts.org Originally filed as https://bugs.launchpad.net/ubuntu/+source/webkit/+bug/560795 thinking we should just change the default font backend to pango, but Xan told me there are a lot of bugs in the pango layout that it'd be better to fix the freetype backend.
Created attachment 71970 [details] Test case I've attached a test case, which I think demonstrates the problem here more directly. To run the test case call LC_ALL="sr_RS.UTF-8" ./WebKitBuild/Release/Programs/GtkLauncher testcase.html . The first string should be rendered via the complex text path (which uses pango) and the second should be rendered via the simple text path (which does not). Chromium Linux seems to fail in both cases, although I don't have the latest.
Just FTR, it seems to work as you expect in Epiphany. I.e. first case is rendered correctly (as I'd expect it), and the second is not. I was not aware of this CSS property (I see that it's non-standard, so basically, thanks for the tip :).
However, since locl is a "smart font" OpenType feature, and Pango's raison d'être is to implement those features (while freetype alone only provides basic TrueType decoding), it's not really surprising that it fails with the freetype backend. You do need pango (or more precisely harfbuzz, which is used by pango) to handle this level of font smartness.
(In reply to comment #0) > Created an attachment (id=54046) [details] > Serbian and regular cursive forms for some Cyrillic letters > > Compiling WebKitGtk with the default freetype backend results in "locl" tables in opentype fonts not being used. Compiling it with the pango backend instead makes it work. > It seems that this is not exactly "locl" table problem. The "locl" table for Cyrillic in DeJavu Serif has only one entry is U+0431(CYRILLIC SMALL LETTER BE) --> U+F6C5
I just figured out that cursive forms are in DejaVuSerif-Italic.ttf, which means to handle this case in simple text we can check locale information and then use the italic font. This really has nothing to do with the 'locl' table.
(In reply to comment #5) > I just figured out that cursive forms are in DejaVuSerif-Italic.ttf, which means to handle this case in simple text we can check locale information and then use the italic font. This really has nothing to do with the 'locl' table. To clarify, Martin's testcase has nothing to do with 'locl', but Danilo's original bug report is 'locl' related
So to solve the original bug report. It seems to me that there are two tasks to be done: 1. when "style="font-family:'DejaVu Serif'; font-style:italic", make sure that DejavVu-Italic.ttf is used 2. lookup DejavVu-Italic.ttf's 'locl' table when necessary
Created attachment 77714 [details] 'locl' works my dirty hack showing Martin's test case I'll find time to clean the code and send patch
Created attachment 77736 [details] roposed patch use 'locl' table, if current locale (and corresponding language and script in the font) has it
Created attachment 77739 [details] proposed patch if there is no FcPattern, that is, it's CustomFont, should return early
Created attachment 77796 [details] proposed patch use FT_Face->family instead of FcPattern, so that we can handle both FontPlatformData and CustomFont now
Comment on attachment 77796 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=77796&action=review > WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp:67 > + PangoFontDescription* pangoDescription = pango_font_description_from_string(face->family_name); > + if (!pangoDescription) > + return false; > + I'm pretty sure this won't work with custom fonts, since IIUC Pango just does a Fontconfig lookup. It makes more sense to bail out for fonts with no FcPattern.
(In reply to comment #12) > (From update of attachment 77796 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=77796&action=review > > > WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp:67 > > + PangoFontDescription* pangoDescription = pango_font_description_from_string(face->family_name); > > + if (!pangoDescription) > > + return false; > > + > > I'm pretty sure this won't work with custom fonts, since IIUC Pango just does a Fontconfig lookup. It makes more sense to bail out for fonts with no FcPattern. As we discussed on IRC, I tried [1] on Ubuntu. It worked like what on my Mac. I read a little bit Pango source code. It seems to me that there is no way to create PangoFont or PangoFcFont from FT_Face without adding new API to Pango. And the reason why my code works is that Pango can use FreeType without using FontConfig. The PangoFcFont is kinda misnomer, I think. PangoFcFont is can be Xft font, FreeType Font, or FontConfig+FreeType font [1] http://freebsd.csie.nctu.edu.tw/~freedom/webkitgtk-test/serb.html
Created attachment 77994 [details] test case to show why we should lang attribute a test case to show why we should lang attribute. See this with FireFox 4.0 beta
Created attachment 77997 [details] screenshot of the lang attribute test case in ff 4.0 beta 8
Created attachment 78101 [details] update the proposed patch add style name to FontDescription so that http://freebsd.csie.nctu.edu.tw/~freedom/webkitgtk-test/serb.html will be displayed with less problems The way we load font: pango_ft2_font_map_new() --> pango_font_map_create_context() ---> pango_font_map_load_font(fontMap, context, description); --> pango_font_description_from_string() doesn't not require FcPattern input, it will create FcPattern inside PangoFcFont
Created attachment 78222 [details] harfbuzz version same as 78101, use harfbuzz, Pango functions are used to convert locale string to font languge string and to find script in font
Created attachment 78501 [details] patch to import harfbuzz 1. use 'Tools/Scripts/update-webkit --gtk' to pull/sync harfbuzz source 2. harfbuzz is located at WebKit/gtk/libs/harfbuzz/ 3. use in $BUILD/WebKit/gtk/libs/harfbuzz/src/libharfbuzz.so for now
This should be addressed after porting to harfbuzz-ng. And even then, just switching to the complex path if 'locl' is desired.
Martin, does this issue still require some work? Is it still relevant?
(In reply to comment #20) > Martin, does this issue still require some work? Is it still relevant? Yes, as Behdad says we need to switch to the complex text path when appropriate. Behdad probably has more information about when the right time to do that is.
(In reply to comment #21) > (In reply to comment #20) > > Martin, does this issue still require some work? Is it still relevant? > > Yes, as Behdad says we need to switch to the complex text path when appropriate. Behdad probably has more information about when the right time to do that is. On Chrome Linux and Android we already switched to harfbuzz-ng. So, first step is to figure out who still uses harfbuzz.old and switch them over. After that, it's just a matter of developing the heuristics. I personally think that we should get rid of the fastpath and always do complex text processing (requires adding new caches). But that's far from where we are right now.
(In reply to comment #22) > On Chrome Linux and Android we already switched to harfbuzz-ng. So, first step is to figure out who still uses harfbuzz.old and switch them over. To my knowledge, all upstream WebKit ports using harfbuzz directly are using harfbuzz-ng.
(In reply to comment #23) > (In reply to comment #22) > > > On Chrome Linux and Android we already switched to harfbuzz-ng. So, first step is to figure out who still uses harfbuzz.old and switch them over. > > To my knowledge, all upstream WebKit ports using harfbuzz directly are using harfbuzz-ng. How about the Qt port? If that is indeed the case we should remove all harfbuzz.old helper code.
Ossy, Simon - how about Qt? Do you still need the harfbuzz.old code or can it go away?
(In reply to comment #25) > Ossy, Simon - how about Qt? Do you still need the harfbuzz.old code or can it go away? It can go away AFAICS. Currently we use Qt for shaping (which uses Harfbuzz, but that's outside of WebKit). Pierre has done some very promising research towards using harfbuzz-ng directly in the Qt port in WebKit and I'd say if things continue the way they do then we're likely to switch to that in the future. But we are not currently using any existing Harfbuzz code path in WebKit, so feel free to change it as you like ;)
(In reply to comment #22) > After that, it's just a matter of developing the heuristics. I personally think that we should get rid of the fastpath and always do complex text processing (requires adding new caches). But that's far from where we are right now. Get rid of the simple path altogether, in WebKit? Can you elaborate a little bit on the heuristics you have in mind? In this case, just something that's based on locales that need lookups in the locl table (like apparently Serbian, as in the original report)? Or something more generic?
IMHO heuristics are doomed to fail badly sooner or later. The simple path is wishful thinking Complex font features are not limited to specific locales such as Serbian, Serbian is just a locale that needs it more than others. Complex features adoption start there and then spreads elsewhere (some people thought unicode was not necessary for western european locale support, and then German speakers standardized upercase ss, oops) It's much safer to always use the complex codepath, instead of betting on some locales never using tech that is available anyway due to other locale needs. Now that the tools to create complex fonts are there, someone will find a way to use them even for the 'simplest' locales.
(In reply to comment #27) > (In reply to comment #22) > > > After that, it's just a matter of developing the heuristics. I personally think that we should get rid of the fastpath and always do complex text processing (requires adding new caches). But that's far from where we are right now. > > Get rid of the simple path altogether, in WebKit? Yes, that's the way it should be IMO. The reason it's not possible right now, is because WebKit does not cache shaping results. As such, every time text is being shown, it's being shaped and shown. That's why supposedly the complex text path is "too slow" for common usage. With effective caching it's possible to turn full shaping on for all scripts. Indeed, that's what Firefox does. > Can you elaborate a little bit on the heuristics you have in mind? In this case, just something that's based on locales that need lookups in the locl table (like apparently Serbian, as in the original report)? Or something more generic? I don't have any heuristics in mind. You can start by adding Serbian, and then add other languages when people nag. You can tell I'm in the same camp as Nicolas though: even for Latin I think we should do full OpenType shaping.
(In reply to comment #29) Thanks, Behdad. > > Get rid of the simple path altogether, in WebKit? > > Yes, that's the way it should be IMO. The reason it's not possible right now, is because WebKit does not cache shaping results. As such, every time text is being shown, it's being shaped and shown. That's why supposedly the complex text path is "too slow" for common usage. With effective caching it's possible to turn full shaping on for all scripts. Indeed, that's what Firefox does. I don't know enough about it yet - but out of curiosity: would you have to cache the shaping coordinate results for arbitrary string length or is it a matter of caching the results for individual pairs of codepoints? > I don't have any heuristics in mind. You can start by adding Serbian, and then add other languages when people nag. We can call it the nag-heuristic :-)
Hi Dominik, Comments below. This is getting offtopic, so feel free to email me for followup discussion. (In reply to comment #30) > (In reply to comment #29) > > Thanks, Behdad. > > > > Get rid of the simple path altogether, in WebKit? > > > > Yes, that's the way it should be IMO. The reason it's not possible right now, is because WebKit does not cache shaping results. As such, every time text is being shown, it's being shaped and shown. That's why supposedly the complex text path is "too slow" for common usage. With effective caching it's possible to turn full shaping on for all scripts. Indeed, that's what Firefox does. > > I don't know enough about it yet - but out of curiosity: would you have to cache the shaping coordinate results for arbitrary string length or is it a matter of caching the results for individual pairs of codepoints? No "pairs of codepoints"... The way I would do is to start by simply caching the glyph list of each text run. When the text is changed, invalidate the cache. Firefox goes one step further: they also cache the shaping result of individual words, so they can put the glyphs for a run together by looking up individual words. That has other issues, but can be made to work. > > I don't have any heuristics in mind. You can start by adding Serbian, and then add other languages when people nag. > > We can call it the nag-heuristic :-) +1!
(In reply to comment #29) > (In reply to comment #27) > I don't have any heuristics in mind. You can start by adding Serbian, and then add other languages when people nag. You can tell I'm in the same camp as Nicolas though: even for Latin I think we should do full OpenType shaping. I put this idea into bug 108074 for tracking.
(In reply to comment #28) > IMHO heuristics are doomed to fail badly sooner or later. The simple path is wishful thinking > > Complex font features are not limited to specific locales such as Serbian, Serbian is just a locale that needs it more than others. Complex features adoption start there and then spreads elsewhere (some people thought unicode was not necessary for western european locale support, and then German speakers standardized upercase ss, oops) > > It's much safer to always use the complex codepath, instead of betting on some locales never using tech that is available anyway due to other locale needs. Now that the tools to create complex fonts are there, someone will find a way to use them even for the 'simplest' locales. FWIW I’ve put together a test page for the locl features in EB Garamond. They are all “simple” locales (serbian, catalan, german and turkish). http://www.georgduffner.at/ebgaramond/tests/locltest.html
(In reply to comment #33) > FWIW I’ve put together a test page for the locl features in EB Garamond. They are all “simple” locales (serbian, catalan, german and turkish). > > http://www.georgduffner.at/ebgaramond/tests/locltest.html Georg, thanks for these. Could you (e.g. on the page itself) explain a bit more what's supposed to happen if correct locl support is activated vs. what the page looks like if that's not the case?
(In reply to comment #34) > (In reply to comment #33) > > > FWIW I’ve put together a test page for the locl features in EB Garamond. They are all “simple” locales (serbian, catalan, german and turkish). > > > > http://www.georgduffner.at/ebgaramond/tests/locltest.html > > Georg, thanks for these. Could you (e.g. on the page itself) explain a bit more what's supposed to happen if correct locl support is activated vs. what the page looks like if that's not the case? I’ve updated the page with an explanation and with a screenshot of the expected result.
(In reply to comment #35) > (In reply to comment #34) > > (In reply to comment #33) > > > > > FWIW I’ve put together a test page for the locl features in EB Garamond. They are all “simple” locales (serbian, catalan, german and turkish). > > > > > > http://www.georgduffner.at/ebgaramond/tests/locltest.html > > > > Georg, thanks for these. Could you (e.g. on the page itself) explain a bit more what's supposed to happen if correct locl support is activated vs. what the page looks like if that's not the case? > > I’ve updated the page with an explanation and with a screenshot of the expected result. On Chromium/Linux one locl is active, namely the one corresponding to your system’s locale. In my case the latter being de-AT, I get correct rendering for german examples but not only on the right but also on the left side (which should be english locl as declared in the header). I tested this by setting my OS to tr which resulted in correct rendering for all the Turkish examples and false for all the others.
(In reply to comment #36) > On Chromium/Linux one locl is active, namely the one corresponding to your system’s locale. That's wrong, the browser should use the locl corresponding to the locale the run of text is marked with (I suspect chromium has no locl awareness and it only "works" because it is unaware the system font stack is locl-compatible)
(In reply to comment #35) > I’ve updated the page with an explanation and with a screenshot of the expected result. Thanks - that's a good starting point for creating some LayoutTests later.
(In reply to comment #38) > (In reply to comment #35) > > > I’ve updated the page with an explanation and with a screenshot of the expected result. > > Thanks - that's a good starting point for creating some LayoutTests later. (In reply to comment #37) > (In reply to comment #36) > > > On Chromium/Linux one locl is active, namely the one corresponding to your system’s locale. > > That's wrong, the browser should use the locl corresponding to the locale the run of text is marked with > > (I suspect chromium has no locl awareness and it only "works" because it is unaware the system font stack is locl-compatible) Thinking deeper about the above said, it looks, like this is not an issue of webkit any longer. The example of Chromium shows that locl is applied, just not the correct one because the browser doesn’t handle the lang-attribute correctly. I think, we have to look into the browsers we use and file bugs at their trackers. I did so for Chromium here: http://code.google.com/p/chromium/issues/detail?id=224170
(In reply to comment #38) > (In reply to comment #35) > > > I’ve updated the page with an explanation and with a screenshot of the expected result. > > Thanks - that's a good starting point for creating some LayoutTests later. Right now HarfBuzzRun doesn't hold language and doesn't set language on the buffer. This *is* a Webkit bug.
(In reply to comment #40) > (In reply to comment #38) > > (In reply to comment #35) > > > > > I’ve updated the page with an explanation and with a screenshot of the expected result. > > > > Thanks - that's a good starting point for creating some LayoutTests later. > > Right now HarfBuzzRun doesn't hold language and doesn't set language on the buffer. > > This *is* a Webkit bug. Ok, then I’m very curious what’s going on in Chromium. Is there any other part of the program that can do something with Opentype features? Because the effects shown in the testpage can’t be replicated but with locl.
(In reply to comment #41) > (In reply to comment #40) > > (In reply to comment #38) > > > (In reply to comment #35) > > > > > > > I’ve updated the page with an explanation and with a screenshot of the expected result. > > > > > > Thanks - that's a good starting point for creating some LayoutTests later. > > > > Right now HarfBuzzRun doesn't hold language and doesn't set language on the buffer. > > > > This *is* a Webkit bug. > > Ok, then I’m very curious what’s going on in Chromium. Is there any other part of the program that can do something with Opentype features? Because the effects shown in the testpage can’t be replicated but with locl. Yes, as you identified yourself, it applies locl, just not the right one. And to make it apply the right one a fix in webkit is needed. I'm not quite sure what you are talking about.
(In reply to comment #42) > (In reply to comment #41) > > (In reply to comment #40) > > > (In reply to comment #38) > > > > (In reply to comment #35) > > > > > > > > > I’ve updated the page with an explanation and with a screenshot of the expected result. > > > > > > > > Thanks - that's a good starting point for creating some LayoutTests later. > > > > > > Right now HarfBuzzRun doesn't hold language and doesn't set language on the buffer. > > > > > > This *is* a Webkit bug. > > > > Ok, then I’m very curious what’s going on in Chromium. Is there any other part of the program that can do something with Opentype features? Because the effects shown in the testpage can’t be replicated but with locl. > > Yes, as you identified yourself, it applies locl, just not the right one. And to make it apply the right one a fix in webkit is needed. I'm not quite sure what you are talking about. I’m sorry, I probably haven’t understood well the part of HarfBuzzRun "doesn't hold language and doesn't set language on the buffer". I understood it in a way that HB isn’t responsible for the locl feature effects we see. I’m confused. But nevermind, I’m sure you’re doing the right thing.
(In reply to comment #43) > (In reply to comment #42) > > (In reply to comment #41) > > > (In reply to comment #40) > > > > (In reply to comment #38) > > > > > (In reply to comment #35) > > > > > > > > > > > I’ve updated the page with an explanation and with a screenshot of the expected result. > > > > > > > > > > Thanks - that's a good starting point for creating some LayoutTests later. > > > > > > > > Right now HarfBuzzRun doesn't hold language and doesn't set language on the buffer. > > > > > > > > This *is* a Webkit bug. > > > > > > Ok, then I’m very curious what’s going on in Chromium. Is there any other part of the program that can do something with Opentype features? Because the effects shown in the testpage can’t be replicated but with locl. > > > > Yes, as you identified yourself, it applies locl, just not the right one. And to make it apply the right one a fix in webkit is needed. I'm not quite sure what you are talking about. > > I’m sorry, I probably haven’t understood well the part of HarfBuzzRun "doesn't hold language and doesn't set language on the buffer". > I understood it in a way that HB isn’t responsible for the locl feature effects we see. HarfBuzz applies 'locl'. It needs to be told the text language. When it's not, it uses default language. HarfBuzzRun is a Webkit abstraction around HarfBuzz shaper. It's part of the glue code connecting Webkit HarfBuzz. As such, it must collect the correct language from other Webkit layers and pass it to HarfBuzz. It's not doing that right now. > I’m confused. But nevermind, I’m sure you’re doing the right thing. Yes, I believe the issue is clear now.
(In reply to comment #44) > (In reply to comment #43) > > (In reply to comment #42) > > > (In reply to comment #41) > > > > (In reply to comment #40) > > > > > (In reply to comment #38) > > > > > > (In reply to comment #35) > > > > > > > > > > > > > I’ve updated the page with an explanation and with a screenshot of the expected result. > > > > > > > > > > > > Thanks - that's a good starting point for creating some LayoutTests later. > > > > > > > > > > Right now HarfBuzzRun doesn't hold language and doesn't set language on the buffer. > > > > > > > > > > This *is* a Webkit bug. > > > > > > > > Ok, then I’m very curious what’s going on in Chromium. Is there any other part of the program that can do something with Opentype features? Because the effects shown in the testpage can’t be replicated but with locl. > > > > > > Yes, as you identified yourself, it applies locl, just not the right one. And to make it apply the right one a fix in webkit is needed. I'm not quite sure what you are talking about. > > > > I’m sorry, I probably haven’t understood well the part of HarfBuzzRun "doesn't hold language and doesn't set language on the buffer". > > I understood it in a way that HB isn’t responsible for the locl feature effects we see. > > HarfBuzz applies 'locl'. It needs to be told the text language. When it's not, it uses default language. > > HarfBuzzRun is a Webkit abstraction around HarfBuzz shaper. It's part of the glue code connecting Webkit HarfBuzz. As such, it must collect the correct language from other Webkit layers and pass it to HarfBuzz. It's not doing that right now. > > > I’m confused. But nevermind, I’m sure you’re doing the right thing. > > Yes, I believe the issue is clear now. And I too understand it now. Thank you!
I'm going to try to take this one on. I think at this point it's a matter of plumbing the language attribute through, and then calling hb_buffer_set_language() with the appropriate value at the appropriate time.
Created attachment 439060 [details] Patch
Created attachment 442519 [details] Patch
Comment on attachment 442519 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=442519&action=review This is great! Looks like the test is still failing on Mac and this should handle a failed call to hb_language_from_string, but this is a good start. > Source/WebCore/platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp:548 > + hb_language_t language = hb_language_from_string(reinterpret_cast<const char*>(m_font.fontDescription().computedLocale().characters8()), -1); I think it would be better to do this here: hb_language_from_string(m_font.fontDescription().computedLocale().string().toUTF8().data()), -1); This would allow you to avoid the reinterpret_cast. It seems like this call should check for and handle (not set the language below) the situation where HB_LANGUAGE_INVALID is returned here.
Comment on attachment 442519 [details] Patch The in-test GTK LayoutTest EWS at the UAT (still behind a firewall) found a new failure with this patch: imported/w3c/web-platform-tests/css/css-fonts/font-language-override-02.html [ ImageOnlyFailure ]