Bug 183210 - [FreeType] Remove FontPlatformData fallbacks
Summary: [FreeType] Remove FontPlatformData fallbacks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-28 05:44 PST by Carlos Garcia Campos
Modified: 2018-03-01 22:42 PST (History)
5 users (show)

See Also:


Attachments
Patch (11.49 KB, patch)
2018-03-01 03:37 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (434.47 KB, patch)
2018-03-01 03:38 PST, Carlos Garcia Campos
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2018-02-28 05:44:37 PST
They are only used by FontCache::systemFallbackForCharacters() where a direct FcFontMatch provides the same results in a more efficient way. This produces better results in 3 tests:

fast/text/international/bidi-LDB-2-CSS.html
fast/text/international/bidi-LDB-2-HTML.html
fast/text/international/bidi-LDB-2-formatting-characters.html

For some reason when using the fallbacks to get the system fallback font the non-bold font is selected for H1, while using FcFontMatch returns the right bold font. I can only reproduce this inside the testing environment, so maybe it's not actually a bug, but in the best case removing the fallbacks doesn't regress.
Comment 1 Carlos Garcia Campos 2018-02-28 09:14:49 PST
Ok, I know why. When using the fallbacks we use FcFontSort() to ge the list of patterns for all fonts in the system, but sorted by better matching the given pattern. But we are passing True for the trim parameter, which for some reason makes dejavu sans bold to not be included in the list. That's why the best match is not bold. Passing False to FcFontSort also makes the test use the bold font, but then, again, it's effectively the same than using FcFontMatch directly. Another interesting thing is that the pattern we use has the default font settings, so it's always using dejavu sans, I would expect in the tests to end up using liberation. And Liberation is probably the first one in the sorted set, but since out pattern has dejavu it ends up matching dejavu.
Comment 2 Carlos Garcia Campos 2018-03-01 03:37:27 PST
Created attachment 334804 [details]
Patch
Comment 3 Carlos Garcia Campos 2018-03-01 03:38:52 PST
Created attachment 334805 [details]
Patch

Include binary diffs
Comment 4 Carlos Garcia Campos 2018-03-01 22:41:09 PST
Committed r229164: <https://trac.webkit.org/changeset/229164>
Comment 5 Radar WebKit Bug Importer 2018-03-01 22:42:35 PST
<rdar://problem/38056646>