[skrifa] autohint bug fixes to match FT #1145
Conversation
This is a largish collection of small changes that bring the autohinter into a state where we match with FreeType for at least the Google Fonts and Noto collections. Also updates fauntlet to use the new hinting API.
|
cc @drott |
| @@ -13,6 +15,20 @@ pub fn compare_glyphs( | |||
| // Don't run on bitmap fonts (yet) | |||
| return true; | |||
| } | |||
| if let Some(Hinting::Auto(_)) = options.hinting { | |||
| // This font is a pathological case for autohinting due to the | |||
There was a problem hiding this comment.
Perhaps add an issue reference?
There was a problem hiding this comment.
Will do. I want to get some actual metrics from this font so we can potentially add limits to prevent this performance cliff. Beyond a certain level of geometric complexity, hinting is likely to produce useless results anyway.
There was a problem hiding this comment.
Yes, similarly we might want to disable it for COLRv1 fonts, but I can do that on the client side.
There was a problem hiding this comment.
Sgtm.
Filed #1147 and added a link in the comment
| // Note: this is actually critical for matching FreeType in cases where | ||
| // we have more than one edge with the same fpos. When this happens, | ||
| // linear and binary searches can produce different results. | ||
| // See <https://gitlab.freedesktop.org/freetype/freetype/-/blob/57617782464411201ce7bbc93b086c1b4d7d84a5/src/autofit/afhints.c#L1489> |
There was a problem hiding this comment.
Took me a while to find this one :)
| if group != ScriptGroup::Default { | ||
| // Divergent CJK behavior | ||
| // See <https://gitlab.freedesktop.org/freetype/freetype/-/blob/57617782464411201ce7bbc93b086c1b4d7d84a5/src/autofit/afcjk.c#L1544> | ||
| if dist < 54 { |
There was a problem hiding this comment.
I am aware these are coming from the source, but perhaps in a separate, follow-up CL, any chance we could attempt to give this bouquet of magic numbers some names, or add some comments?
There was a problem hiding this comment.
I’ll do the best I can. Updated #1141 to include this.
This is a largish collection of small changes that bring the autohinter into a state where we match with FreeType for at least the Google Fonts and Noto collections.
Also updates fauntlet to use the new hinting API.
Builds on #1144 which should land first.