WebKit freetype2 font backend doesn't support "locl" tables

Bug #560795 reported by Данило Шеган
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Webkit
New
Medium
webkit (Ubuntu)
New
Undecided
Unassigned

Bug Description

Currently, Ubuntu Lucid webkit package is compiled with the default setting for --with-font-backend ./configure option (freetype). However, "locl" lookup tables, as used for Serbian glyphs and present in DejaVu fonts (most notably with letters бгдпт in DejaVu Serif Italic from ttf-dejavu-extra package) are not supported so pages render with forms unfamiliar to Serbian readers (well, familiar, but very hard to read).

If --with-font-backend=pango is used, they work correctly.

To easily see the difference, you can do:
$ pango-view --font="DejaVu Serif Italic 64" --text "бгдпт" --language=sr
$ pango-view --font="DejaVu Serif Italic 64" --text "бгдпт" --language=ru

http://live.gnome.org/WebKitGtk mentions how all features of pango are included in freetype backend, but that's obviously not true. I've tested it with webkit compiled with the above option, and pushed those packages to my PPA:

  https://edge.launchpad.net/~danilo/+archive/webkit-pango

Good test-case is visiting http://danilo.rs/proba.html from both Firefox (looks good) and Epiphany, and noticing the same difference as with the two pango-view commands. You probably need to set your locale to Serbian to get Pango to use Serbian styles.

Bug #248072 is an old bug asking for Pango usage and it seems at least part of this was reverted for Lucid (or earlier).

If there are really no drawbacks to using Pango, I'm sure many complex-script users would appreciate pango backend over the freetype2 one.

Revision history for this message
In , Данило Шеган (danilo) wrote :

Created attachment 54046
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.

Revision history for this message
Данило Шеган (danilo) wrote :

So, it seems Pango font backend is in a very bad shape, so we should not use it by default. One of the bugs I've seen myself is https://bugs.webkit.org/show_bug.cgi?id=18546. So, discussing it with Xan he said that we should either fix it in the freetype backend, if that's simple enough, or switch to using HarfBuzz for text layout.

When https://bugs.webkit.org/show_bug.cgi?id=37984 gets fixed we can just sync the latest release.

Revision history for this message
In , Mrobinson-d (mrobinson-d) wrote :

Created attachment 71970
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.

Revision history for this message
In , Данило Шеган (danilo) wrote :

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 :).

Revision history for this message
In , Nicolas-mailhot-laposte (nicolas-mailhot-laposte) wrote :

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.

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

(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

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

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.

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

(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

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

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

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

Created attachment 77714
'locl' works

my dirty hack showing Martin's test case
I'll find time to clean the code and send patch

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

Created attachment 77736
roposed patch

use 'locl' table, if current locale (and corresponding language and script in the font) has it

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

Created attachment 77739
proposed patch

if there is no FcPattern, that is, it's CustomFont, should return early

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

Created attachment 77796
proposed patch

use FT_Face->family instead of FcPattern, so that we can handle both FontPlatformData and CustomFont now

Revision history for this message
In , Mrobinson-d (mrobinson-d) wrote :

Comment on attachment 77796
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.

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

(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

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

Created attachment 77994
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

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

Created attachment 77997
screenshot of the lang attribute test case in ff 4.0 beta 8

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

Created attachment 78101
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

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

Created attachment 78222
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

Revision history for this message
In , "freedom" Koan-Sin Tan (koansin-tan) wrote :

Created attachment 78501
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

Changed in webkit:
status: Unknown → New
Changed in webkit:
importance: Unknown → Medium
Revision history for this message
In , Behdad-t (behdad-t) wrote :

This should be addressed after porting to harfbuzz-ng. And even then, just switching to the complex path if 'locl' is desired.

Revision history for this message
In , Dominik Röttsches (drott) wrote :

Martin, does this issue still require some work? Is it still relevant?

Revision history for this message
In , Mrobinson-d (mrobinson-d) wrote :

(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.

Revision history for this message
In , Behdad-t (behdad-t) wrote :

(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.

Revision history for this message
In , Mrobinson-d (mrobinson-d) wrote :

(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.

Revision history for this message
In , Behdad-t (behdad-t) wrote :

(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.

Revision history for this message
In , Dominik Röttsches (drott) wrote :

Ossy, Simon - how about Qt? Do you still need the harfbuzz.old code or can it go away?

Revision history for this message
In , Hausmann (hausmann) wrote :

(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 ;)

Revision history for this message
In , Dominik Röttsches (drott) wrote :

(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?

Revision history for this message
In , Nicolas-mailhot-laposte (nicolas-mailhot-laposte) wrote :

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.

Revision history for this message
In , Behdad-t (behdad-t) wrote :

(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.

Revision history for this message
In , Dominik Röttsches (drott) wrote :

(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 :-)

Revision history for this message
In , Behdad-t (behdad-t) wrote :

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!

Revision history for this message
In , Dominik Röttsches (drott) wrote :

(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.

Revision history for this message
In , G-duffner (g-duffner) wrote :

(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

Revision history for this message
In , Dominik Röttsches (drott) wrote :

(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?

Revision history for this message
In , G-duffner (g-duffner) wrote :

(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.

Revision history for this message
In , G-duffner (g-duffner) wrote :

(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.

Revision history for this message
In , Nicolas-mailhot-laposte (nicolas-mailhot-laposte) wrote :

(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)

Revision history for this message
In , Dominik Röttsches (drott) wrote :

(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.

Revision history for this message
In , G-duffner (g-duffner) wrote :

(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

Revision history for this message
In , Behdad-t (behdad-t) wrote :

(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.

Revision history for this message
In , G-duffner (g-duffner) wrote :

(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.

Revision history for this message
In , Behdad-t (behdad-t) wrote :

(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.

Revision history for this message
In , G-duffner (g-duffner) wrote :

(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.

Revision history for this message
In , Behdad-t (behdad-t) wrote :

(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.

Revision history for this message
In , G-duffner (g-duffner) wrote :

(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!

Revision history for this message
In , Agoldmints (agoldmints) wrote :

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.

Revision history for this message
In , Agoldmints (agoldmints) wrote :

Created attachment 439060
Patch

Revision history for this message
In , Agoldmints (agoldmints) wrote :

Created attachment 442519
Patch

Revision history for this message
In , Mrobinson-d (mrobinson-d) wrote :

Comment on attachment 442519
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.

Revision history for this message
In , carloslp (carloslp) wrote :

Comment on attachment 442519
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 ]

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.