Re: [PATCH v3 1/3] gdi32: Specify the hinting algorithms to apply explicitly.

[Huw Davies]

On Tue, Nov 06, 2018 at 04:08:06PM +0900, Byeongsik Jeon wrote:
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=41639
> Signed-off-by: Byeongsik Jeon
>
> As a result of analyzing the native Tahoma font in the ftview, I realized 
> that the bug #41639 is a result of not specifying the appropriate hinting 
> algorithm.
> https://www.freetype.org/freetype2/docs/reference/ft2-base_interface.html#FT...
>
> .. values to select a specific hinting algorithm for the hinter.
> ..
> You can use a hinting algorithm that doesn't correspond to the same 
> rendering mode. As an example, it is possible to use the ‘light’ 
> hinting algorithm and have the results rendered in horizontal LCD pixel 
> mode, with code like
>
> FT_Load_Glyph( face, glyph_index, load_flags | FT_LOAD_TARGET_LIGHT );
> FT_Render_Glyph( face->glyph, FT_RENDER_MODE_LCD );
>
> FT_LOAD_TARGET_NORMAL is 0.
> So, we have been applying FT_LOAD_TARGET_NORMAL to all load_flags.
>
> FT_LOAD_TARGET_NORMAL :
> Default render mode; it corresponds to 8-bit anti-aliased bitmaps.
> FT_LOAD_TARGET_MONO :
> Strong hinting algorithm that should only be used for monochrome output.
>
> We usually do not see this problem because we use the Wine's Tahoma with 
> the embeded bitmap.
>
> MacOS XQuartz 2.7.11, Freetype 2.7:
> - subpixel rendering disabled Freetype.
> - MacOS builtined Tahoma.
>
> This bug appears because of the combination of two situations.
>
> This is about the Freetype interpreter version 40.
> Version 40 is the default.
>
> dlls/gdi32/freetype.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
> index 07de6f2f6c..56d1e99873 100644
> --- a/dlls/gdi32/freetype.c
> +++ b/dlls/gdi32/freetype.c
> @@ -7001,6 +7001,30 @@ static DWORD get_glyph_outline(GdiFont *incoming_font, UINT glyph, UINT format,
>      if (needsTransform || format != GGO_BITMAP) load_flags |= FT_LOAD_NO_BITMAP;
>      if (vertical_metrics) load_flags |= FT_LOAD_VERTICAL_LAYOUT;
> - if (!(load_flags & FT_LOAD_NO_HINTING))
> - {
> -
>    switch (format)
> -
>    {
> -
>    case GGO_BITMAP:
> -
>        load_flags |= FT_LOAD_TARGET_MONO;
> -
>        break;
> -
>    case GGO_GRAY2_BITMAP:
> -
>    case GGO_GRAY4_BITMAP:
> -
>    case GGO_GRAY8_BITMAP:
> -
>    case WINE_GGO_GRAY16_BITMAP:
> -
>        load_flags |= FT_LOAD_TARGET_NORMAL;
> -
>        break;
> -
>    case WINE_GGO_HRGB_BITMAP:
> -
>    case WINE_GGO_HBGR_BITMAP:
> -
>        load_flags |= FT_LOAD_TARGET_LCD;
> -
>        break;
> -
>    case WINE_GGO_VRGB_BITMAP:
> -
>    case WINE_GGO_VBGR_BITMAP:
> -
>        load_flags |= FT_LOAD_TARGET_LCD_V;
> -
>        break;
> -
>    }
> - }
> -

This seems worthwhile, however could you make this a helper function
(e.g. get_load_flags() and call it earlier on, right before where
GGO_UNHINTED is handled?  The helper would also return
FT_LOAD_IGNORE_GLOBAL_ADVANCE_WIDTH.

Huw.