[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 <bsjeon at hanmail.net>
> ---
> 
> 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_LOAD_XXX
> ---
> .. 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.