[PATCH 0/1] MR5619: win32u: Use FT_LOAD_PEDANTIC on first load try in freetype_get_glyph_outline().
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/win32u/freetype.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/win32u/freetype.c b/dlls/win32u/freetype.c index 3e640115ab7..9016f670bba 100644 --- a/dlls/win32u/freetype.c +++ b/dlls/win32u/freetype.c @@ -3469,7 +3469,7 @@ static UINT freetype_get_glyph_outline( struct gdi_font *font, UINT glyph, UINT if (matrices || format != GGO_BITMAP) load_flags |= FT_LOAD_NO_BITMAP; if (vertical_metrics) load_flags |= FT_LOAD_VERTICAL_LAYOUT; - err = pFT_Load_Glyph(ft_face, glyph, load_flags); + err = pFT_Load_Glyph(ft_face, glyph, load_flags & FT_LOAD_NO_HINTING ? load_flags : load_flags | FT_LOAD_PEDANTIC); if (err && !(load_flags & FT_LOAD_NO_HINTING)) { WARN("Failed to load glyph %#x, retrying without hinting. Error %#x.\n", glyph, err); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5619
The practical problem is observed in Cosmoteer game which fails to display some Chineese characters (when Chineese language is selected for the game UI). That happens with the specific font shipped with the game (play_msyh.ttf, play_msyh_bold.ttf), replacing it with Wine font avoids the problem. The rendering issue is present in the glyph image we get from freetype library. Opening any of these fonts with ftview program from FreeType demo programs immediately shows the glitches for some Chineese characters. The font is correctly displayed on Windows (as well as rendering the problematic glyphs goes without visual glitches). I am not attaching those fonts here because apparently those are copyrighted materials. The problem is only there when hinting is enabled, disabling it in ftview with 'h' or in Wine by passing FT_LOAD_NO_HINTING (or FT_LOAD_FORCE_AUTOHINT) avoids the problem. Internally FreeType library is getting errors when interpreting hinting bytecode for those glyphs (various errors, e. g., Invalid_Reference or Stack_Overflow). While those errors may be either FreeType bugs or font bugs, such errors are possible in TTF fonts even without FreeType bugs and both FreeType and Wine already mind such a possibility and trying to handle that, so I guess we are interested to do it correctly (regardless of whether specific problem with the font is a font bug or freetype bug). freetype_get_glyph_outline() already minds possible bugs related to hinting, if FT_Load_Glyph failed it tries to call that with FT_LOAD_NO_HINTING flag. The problem is that freetype library doesn't return any error (at least now, maybe that changed since the time this Wine code was first introduced) from interpreting hinting bytecode if FT_LOAD_PEDANTIC flag is not specified. While the glyph may be affected by partially executed bytecode which is then aborted by the error. This doesn't look like an accidental bug, it is a documented behaviour [1]: "... In particular, errors from the TrueType bytecode engine are not passed to the application if this flag is not set; this might result in partially hinted or distorted glyphs in case a glyph's bytecode is buggy." So I guess setting the flag on the first attempt and then falling back to no hinting / no pedantic checking is the right thing to do (and it fixes the issue). 1. https://freetype.org/freetype2/docs/reference/ft2-glyph_retrieval.html#ft_lo... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5619#note_70157
Regarding LOAD_PEDANTIC, I don't think I ever used that. Any idea what would be potential performance impact? Does it enable some validation incrementally, or is everything validated at every Load_Glyph() call.
I grepped freetype code for the usage of that and as far as I can tell the flag is only used to propagate or ignore errors, with all the same checks performed regardless. I also don't see caches used on a way of Load_Glyph. There is a separate cache subsystem in freetype (https://freetype.org/freetype2/docs/reference/ft2-cache_subsystem.html) which I think is supposed to be used explicitly by an application. So apart from specific cases when we weren't falling back to no hinting (an extra Load_Glyph call) and will be falling back now (presumably avoiding rendering glitches which were present before) I don't see it affecting performance. I think if we care to speed that up we should introduce some glyph caching ourselves, possibly not in freetype driver but rather in a generic win32u part? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5619#note_70161
I think there are two concerns here. If this works correctly on Windows, which could mean different things, freetype would be interested I think in fixing this as well, depending on how this is broken exactly. I think it's worth reporting. Regarding LOAD_PEDANTIC, I don't think I ever used that. Any idea what would be potential performance impact? Does it enable some validation incrementally, or is everything validated at every Load_Glyph() call. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5619#note_70160
On Fri May 10 20:13:12 2024 +0000, Paul Gofman wrote:
Regarding LOAD_PEDANTIC, I don't think I ever used that. Any idea what would be potential performance impact? Does it enable some validation incrementally, or is everything validated at every Load_Glyph() call. I grepped freetype code for the usage of that and as far as I can tell the flag is only used to propagate or ignore errors, with all the same checks performed regardless. I also don't see caches used on a way of Load_Glyph. There is a separate cache subsystem in freetype (https://freetype.org/freetype2/docs/reference/ft2-cache_subsystem.html) which I think is supposed to be used explicitly by an application. So apart from specific cases when we weren't falling back to no hinting (an extra Load_Glyph call) and will be falling back now (presumably avoiding rendering glitches which were present before) I don't see it affecting performance. I think if we care to speed that up we should introduce some glyph caching ourselves, possibly not in freetype driver but rather in a generic win32u part? Opened freetype issue: https://gitlab.freedesktop.org/freetype/freetype/-/issues/1274
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5619#note_70162
On Fri May 10 20:29:37 2024 +0000, Paul Gofman wrote:
Opened freetype issue: https://gitlab.freedesktop.org/freetype/freetype/-/issues/1274 Forgot to report here, the font was looked at by FreeType developers and they concluded that the font is buggy and there is nothing to fix in FreeType, the only way for such cases is to disable hinting.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5619#note_75206
On Fri Jul 5 15:28:02 2024 +0000, Paul Gofman wrote:
Forgot to report here, the font was looked at by FreeType developers and they concluded that the font is buggy and there is nothing to fix in FreeType, the only way for such cases is to disable hinting. I'd say let's try it and see if it causes problems.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5619#note_75207
This merge request was approved by Nikolay Sivov. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5619
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5619
participants (4)
-
Huw Davies (＠huw) -
Nikolay Sivov (＠nsivov)
-
Paul Gofman
-
Paul Gofman (＠gofman)