[PATCH 0/1] MR5619: win32u: Use FT_LOAD_PEDANTIC on first load try in freetype_get_glyph_outline().

[PATCH 1/1] win32u: Use FT_LOAD_PEDANTIC on first load try in freetype_get_glyph_outline().

[Paul Gofman]

From: Paul Gofman

---
 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);

[Paul Gofman (＠gofman)]

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...

[Nikolay Sivov (＠nsivov)]

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.

[Paul Gofman (＠gofman)]

> 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?

[Paul Gofman (＠gofman)]

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

[Paul Gofman (＠gofman)]

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.

[Nikolay Sivov (＠nsivov)]

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.

[PATCH 0/1] MR5619: win32u: Use FT_LOAD_PEDANTIC on first load try in freetype_get_glyph_outline(). - approved

[Nikolay Sivov (＠nsivov)]

This merge request was approved by Nikolay Sivov.

[PATCH 0/1] MR5619: win32u: Use FT_LOAD_PEDANTIC on first load try in freetype_get_glyph_outline(). - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.