Bug 228927 - [FreeType] Avoid yucky strong alias computations in font fallback code
Summary: [FreeType] Avoid yucky strong alias computations in font fallback code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-08-09 15:35 PDT by Michael Catanzaro
Modified: 2021-08-26 12:30 PDT (History)
9 users (show)

See Also:


Attachments
Patch (11.50 KB, patch)
2021-08-09 15:38 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (5.19 KB, patch)
2021-08-26 06:48 PDT, Michael Catanzaro
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (5.19 KB, patch)
2021-08-26 06:53 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2021-08-09 15:35:47 PDT
In bug #147057 I added a bunch of copypasted code from Skia that performs black magic to sort font patterns in order to guess whether one font is strongly-aliased to another. In https://bugs.freedesktop.org/show_bug.cgi?id=19375 Fontconfig added a new FcPatternGetWithBinding() API to do this directly. It has been four years now, so in theory we can now remove all the yucky code and use FcPatternGetWithBinding() instead. I've attached a patch that does exactly that.

Problem is, my patch doesn't work at all. For some reason, FcPatternGetWithBinding() reports that *every* binding is a strong binding, so now every match performed by Fontconfig is accepted, which is not what we want. I will ask the Fontconfig developers if I'm doing something obviously wrong.
Comment 1 Michael Catanzaro 2021-08-09 15:38:03 PDT
Created attachment 435217 [details]
Patch
Comment 2 Michael Catanzaro 2021-08-10 06:45:25 PDT
FontConfig issue: https://gitlab.freedesktop.org/fontconfig/fontconfig/-/issues/294
Comment 3 Michael Catanzaro 2021-08-26 06:48:48 PDT
Created attachment 436504 [details]
Patch
Comment 4 Michael Catanzaro 2021-08-26 06:53:04 PDT
Created attachment 436505 [details]
Patch
Comment 5 Myles C. Maxfield 2021-08-26 11:56:02 PDT
Comment on attachment 436505 [details]
Patch

Nice!
Comment 6 Myles C. Maxfield 2021-08-26 11:57:38 PDT
Comment on attachment 436505 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436505&action=review

> Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:358
> +#if FC_VERSION < 21395

What happens if the version of FontConfig that the build uses is not the same version used at runtime? (We have a similar problem for using ICU and have to call functions instead of using macros.)
Comment 7 Michael Catanzaro 2021-08-26 12:25:54 PDT
(In reply to Myles C. Maxfield from comment #6)
> What happens if the version of FontConfig that the build uses is not the
> same version used at runtime? (We have a similar problem for using ICU and
> have to call functions instead of using macros.)

If it's built using a newer version of Fontconfig than is used at runtime, then fonts are going to get really messed up, because every font will be considered a valid alias for every other font. To avoid this, I could use FcGetVersion() to check the Fontconfig version at runtime instead of using the build-time check.

That said, with Linux software, it's well-understood that the runtime environment has to be newer than the build environment, or random breakage like this will result. I know this is different from macOS, because I remember I landed a change once that broke Safari nightlies because I used only a build-time check and not a runtime version check, see r189526 and r189990. I was very surprised that your build environment included a newer version of SQLite than your users were running. With software written for Linux, everyone just assumes that never happens. We would say the build environment is broken if it contains anything newer than the oldest-supported version that might be used at runtime.

The benefit of using build-time checks instead of runtime checks is it makes it easier to see where the legacy code is, so it can be removed more easily in the future. Future me might like this....
Comment 8 EWS 2021-08-26 12:30:39 PDT
Committed r281644 (241000@main): <https://commits.webkit.org/241000@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 436505 [details].