Bug 92098 – [GTK] Bring Harfbuzz-ng support to Gtk

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Dominik Röttsches (drott)]

As a successor the pango backend, let's enable harfbuzz for gtk to get better text shaping support for complex fonts.
Work to enable this on EFL is tracked in bug 91853.

[Grzegorz Czajkowski]

This bug removes Pango from WebKit-Gtk spelling implementation (https://bugs.webkit.org/show_bug.cgi?id=94320).

[Martin Robinson]

I've enabled this locally and I notice some differences in pixel output that look a bit like regressions. For instance, the Arabic characters in LayoutTests/platform/efl/fast/text/complex-text-opacity-expected.png are not elided properly.

[Dominik Röttsches (drott)]

(In reply to comment #2)
> I've enabled this locally and I notice some differences in pixel output that look a bit like regressions. For instance, the Arabic characters in LayoutTests/platform/efl/fast/text/complex-text-opacity-expected.png are not elided properly.

Does this change depending on the Harfbuzz version? Looking at their mailing list, there's lots of changes going on.

[Martin Robinson]

(In reply to comment #3)
> (In reply to comment #2)
> > I've enabled this locally and I notice some differences in pixel output that look a bit like regressions. For instance, the Arabic characters in LayoutTests/platform/efl/fast/text/complex-text-opacity-expected.png are not elided properly.
> 
> Does this change depending on the Harfbuzz version? Looking at their mailing list, there's lots of changes going on.

I see this with the latest harfbuzz release (0.9.5).

[Martin Robinson]

(In reply to comment #4)

> I see this with the latest harfbuzz release (0.9.5).

The issue seems to be with the platform harfbuzz code. Instead of splitting complex text into runs and choosing a font appropriately, the behavior I see is that the text uses the default font. For each character of the complex text that is not in the default font we are falling back to FontCache::getFontDataForCharacters. In the end harfbuzz is not combining those glyphs properly, because they are fallback glyphs.

I guess we should be choosing the appropriate fallback font earlier in the process.

bashi, do you think you can give more information about how this works for Chromium with the HarfbuzzNG path?

[Kenichi Ishibashi]

Adding behdad.
(In reply to comment #5)
> (In reply to comment #4)
> 
> > I see this with the latest harfbuzz release (0.9.5).
> 
> The issue seems to be with the platform harfbuzz code. Instead of splitting complex text into runs and choosing a font appropriately, the behavior I see is that the text uses the default font. For each character of the complex text that is not in the default font we are falling back to FontCache::getFontDataForCharacters. In the end harfbuzz is not combining those glyphs properly, because they are fallback glyphs.
> 
> I guess we should be choosing the appropriate fallback font earlier in the process.
> 
> bashi, do you think you can give more information about how this works for Chromium with the HarfbuzzNG path?

(In reply to comment #5)
> (In reply to comment #4)
> 
> > I see this with the latest harfbuzz release (0.9.5).
> 
> The issue seems to be with the platform harfbuzz code. Instead of splitting complex text into runs and choosing a font appropriately, the behavior I see is that the text uses the default font. For each character of the complex text that is not in the default font we are falling back to FontCache::getFontDataForCharacters. In the end harfbuzz is not combining those glyphs properly, because they are fallback glyphs.
> 
> I guess we should be choosing the appropriate fallback font earlier in the process.
> 
> bashi, do you think you can give more information about how this works for Chromium with the HarfbuzzNG path?

Are you trying to use HarfBuzzShaper on GTK? If so, HarfBuzzShaper::collectHarfBuzzRuns() doesn't select fonts properly?

[Martin Robinson]

Mystery solved (I hope). When the Fontconfig FontCache returns a fallback font, it returns a different font each time -- even if the Fontconfig pattern returns the same font. This is quite bad, as it means that we're keeping many duplicate copies of fallback fonts in the cache. I've fixed this locally and the results look a lot better.

The reason this was causing a problem for Harfbuzz was that HarfBuzzShaper saw each fallback font as a different font and created a separate text run for each character.

[Martin Robinson]

Created attachment 171729 [details]
Patch

[Martin Robinson]

I didn't include the new layout test results because the made the patch a bit too big.

[Martin Robinson]

It looks like the bot is missing ICU pkg-config files.

[Philippe Normand]

(In reply to comment #10)
> It looks like the bot is missing ICU pkg-config files.

The bot has libicu-dev 4.8.1.1-9. Seems like the harfbuzz headers are not found instead when building WebCore.

Is a clean build required?

[Dominik Röttsches (drott)]

(In reply to comment #9)
> I didn't include the new layout test results because the made the patch a bit too big.

Glad you found the reason. You may ping our gardener on IRC once you land this, so that we can take care of the EFL rebaselining. http://trac.webkit.org/wiki/EFLWebKitBuildBots

[Martin Robinson]

(In reply to comment #11)
> (In reply to comment #10)
> > It looks like the bot is missing ICU pkg-config files.
> 
> The bot has libicu-dev 4.8.1.1-9. Seems like the harfbuzz headers are not found instead when building WebCore.
> 
> Is a clean build required?

In Debian, up until very recently, the libicu-dev package did not install pkg-config files. See here: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=687339 I'm not sure what the best way to resolve this is. Locally I install libicu manually.

[Dominik Röttsches (drott)]

(In reply to comment #13)
> I'm not sure what the best way to resolve this is.

I'm proposing
http://lists.freedesktop.org/archives/harfbuzz/2012-November/002612.html

Since we need to bump the version in bug 101323 as well.

[Gustavo Noronha (kov)]

Comment on attachment 171729 [details]
Patch

LGTM, and I think the bots have all been updated some time ago, haven't they?

[Martin Robinson]

Committed r137146: <http://trac.webkit.org/changeset/137146>

[Chris Dumez]

Looks life this patch affected the results for EFL port?

About 50 test cases seem to require rebaselining after this patch? Was that expected?

[Chris Dumez]

Ok, I see from https://bugs.webkit.org/show_bug.cgi?id=92098#c12 that the rebaseline was expected. I guess I'll take care of it then.

[Martin Robinson]

*** Bug 26741 has been marked as a duplicate of this bug. ***

[Martin Robinson]

*** Bug 20783 has been marked as a duplicate of this bug. ***