[PATCH 0/2] MR8414: Fix EV Nova game text truncation.
Fix the rectangle from DrawText(..., DT_CALCRECT) with a selected font might not be enough to actually draw text with the font for the EV Nova game. This ensures that text extent calculated with a font selected in a device context is enough when drawing texts using the same font and device context. The root cause is that FreeType might report different font metrics for different GGO formats. GetTextExtentExPoint() gets text extent with GGO_METRICS. And GGO_METRICS gets font metrics from glyphs loaded with FT_LOAD_TARGET_NORMAL, so when drawing bitmap fonts, which is of GGO_BITMAP format loaded with FT_LOAD_TARGET_MONO, with the calculated text extent, the result text might get truncated because of the font metrics difference. On Windows, anti-aliased font and bitmap font of the same face font reports the same font metrics so such a problem doesn't happen. The ultimate fix is to let FreeType hint the bitmap fonts with the same font metrics as anti-aliased fonts but there are no settings that can achieve it unless we write a new FreeType hinter. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57309 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8414
From: Zhiyi Zhang <zzhang(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57309 --- dlls/win32u/font.c | 52 +++++++++++++++++++++---------------- dlls/win32u/ntgdi_private.h | 4 +-- include/wingdi.h | 11 ++++---- 3 files changed, 38 insertions(+), 29 deletions(-) diff --git a/dlls/win32u/font.c b/dlls/win32u/font.c index 3d70710e059..02f4045f6d9 100644 --- a/dlls/win32u/font.c +++ b/dlls/win32u/font.c @@ -2413,7 +2413,7 @@ static struct gdi_font *alloc_gdi_font( const WCHAR *file, void *data_ptr, SIZE_ static void free_gdi_font( struct gdi_font *font ) { - DWORD i; + DWORD i, j; struct gdi_font *child, *child_next; if (font->private) font_funcs->destroy_font( font ); @@ -2423,12 +2423,16 @@ static void free_gdi_font( struct gdi_font *font ) list_remove( &child->entry ); free_gdi_font( child ); } - for (i = 0; i < font->gm_size; i++) free( font->gm[i] ); + for (i = 0; i < ARRAY_SIZE(font->gm_size); i++) + { + for (j = 0; j < font->gm_size[i]; i++) + free( font->gm[i][j] ); + free( font->gm[i] ); + } free( font->otm.otmpFamilyName ); free( font->otm.otmpStyleName ); free( font->otm.otmpFaceName ); free( font->otm.otmpFullName ); - free( font->gm ); free( font->kern_pairs ); free( font->gsub_table ); free( font ); @@ -2469,16 +2473,17 @@ struct glyph_metrics #define GM_BLOCK_SIZE 128 -/* TODO: GGO format support */ -static BOOL get_gdi_font_glyph_metrics( struct gdi_font *font, UINT index, GLYPHMETRICS *gm, ABC *abc ) +static BOOL get_gdi_font_glyph_metrics( struct gdi_font *font, UINT format, UINT index, + GLYPHMETRICS *gm, ABC *abc ) { UINT block = index / GM_BLOCK_SIZE; UINT entry = index % GM_BLOCK_SIZE; - if (block < font->gm_size && font->gm[block] && font->gm[block][entry].init) + if (format < WINE_GGO_FORMAT_COUNT && block < font->gm_size[format] && font->gm[format][block] + && font->gm[format][block][entry].init) { - *gm = font->gm[block][entry].gm; - *abc = font->gm[block][entry].abc; + *gm = font->gm[format][block][entry].gm; + *abc = font->gm[format][block][entry].abc; TRACE( "cached gm: %u, %u, %s, %d, %d abc: %d, %u, %d\n", gm->gmBlackBoxX, gm->gmBlackBoxY, wine_dbgstr_point( &gm->gmptGlyphOrigin ), @@ -2489,29 +2494,32 @@ static BOOL get_gdi_font_glyph_metrics( struct gdi_font *font, UINT index, GLYPH return FALSE; } -static void set_gdi_font_glyph_metrics( struct gdi_font *font, UINT index, +static void set_gdi_font_glyph_metrics( struct gdi_font *font, UINT format, UINT index, const GLYPHMETRICS *gm, const ABC *abc ) { UINT block = index / GM_BLOCK_SIZE; UINT entry = index % GM_BLOCK_SIZE; - if (block >= font->gm_size) + if (format >= WINE_GGO_FORMAT_COUNT) + return; + + if (block >= font->gm_size[format]) { struct glyph_metrics **ptr; - if (!(ptr = realloc( font->gm, (block + 1) * sizeof(*ptr) ))) return; - memset( ptr + font->gm_size, 0, (block + 1 - font->gm_size) * sizeof(*ptr) ); - font->gm_size = block + 1; - font->gm = ptr; + if (!(ptr = realloc( font->gm[format], (block + 1) * sizeof(*ptr) ))) return; + memset( ptr + font->gm_size[format], 0, (block + 1 - font->gm_size[format]) * sizeof(*ptr) ); + font->gm_size[format] = block + 1; + font->gm[format] = ptr; } - if (!font->gm[block]) + if (!font->gm[format][block]) { - font->gm[block] = calloc( sizeof(**font->gm), GM_BLOCK_SIZE ); - if (!font->gm[block]) return; + font->gm[format][block] = calloc( sizeof(***font->gm), GM_BLOCK_SIZE ); + if (!font->gm[format][block]) return; } - font->gm[block][entry].gm = *gm; - font->gm[block][entry].abc = *abc; - font->gm[block][entry].init = TRUE; + font->gm[format][block][entry].gm = *gm; + font->gm[format][block][entry].abc = *abc; + font->gm[format][block][entry].init = TRUE; } @@ -3771,14 +3779,14 @@ static DWORD get_glyph_outline( struct gdi_font *font, UINT glyph, UINT format, if (mat && !memcmp( mat, &identity, sizeof(*mat) )) mat = NULL; - if (format == GGO_METRICS && !mat && get_gdi_font_glyph_metrics( font, index, &gm, &abc )) + if (format == GGO_METRICS && !mat && get_gdi_font_glyph_metrics( font, format, index, &gm, &abc )) goto done; ret = font_funcs->get_glyph_outline( font, index, format, &gm, &abc, buflen, buf, mat, tategaki ); if (ret == GDI_ERROR) return ret; if (format == GGO_METRICS && !mat) - set_gdi_font_glyph_metrics( font, index, &gm, &abc ); + set_gdi_font_glyph_metrics( font, format, index, &gm, &abc ); done: if (gm_ret) *gm_ret = gm; diff --git a/dlls/win32u/ntgdi_private.h b/dlls/win32u/ntgdi_private.h index 78b3ad89796..58eea19e6c1 100644 --- a/dlls/win32u/ntgdi_private.h +++ b/dlls/win32u/ntgdi_private.h @@ -255,8 +255,8 @@ struct gdi_font struct list entry; struct list unused_entry; DWORD refcount; - DWORD gm_size; - struct glyph_metrics **gm; + DWORD gm_size[WINE_GGO_FORMAT_COUNT]; + struct glyph_metrics **gm[WINE_GGO_FORMAT_COUNT]; OUTLINETEXTMETRICW otm; KERNINGPAIR *kern_pairs; int kern_count; diff --git a/include/wingdi.h b/include/wingdi.h index ed0e59462e9..31ef01b075c 100644 --- a/include/wingdi.h +++ b/include/wingdi.h @@ -1342,11 +1342,12 @@ typedef struct #define GGO_UNHINTED 0x100 #ifdef __WINESRC__ -#define WINE_GGO_GRAY16_BITMAP 0x10 -#define WINE_GGO_HRGB_BITMAP 0x11 -#define WINE_GGO_HBGR_BITMAP 0x12 -#define WINE_GGO_VRGB_BITMAP 0x13 -#define WINE_GGO_VBGR_BITMAP 0x14 +#define WINE_GGO_GRAY16_BITMAP 7 +#define WINE_GGO_HRGB_BITMAP 8 +#define WINE_GGO_HBGR_BITMAP 9 +#define WINE_GGO_VRGB_BITMAP 10 +#define WINE_GGO_VBGR_BITMAP 11 +#define WINE_GGO_FORMAT_COUNT (WINE_GGO_VBGR_BITMAP + 1) #endif typedef struct -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8414
From: Zhiyi Zhang <zzhang(a)codeweavers.com> Fix the rectangle from DrawText(..., DT_CALCRECT) with a selected font might not be enough to actually draw text with the font for the EV Nova game. This ensures that text extent calculated with a font selected in a device context is enough when drawing texts using the same font and device context. The root cause is that FreeType might report different font metrics for different GGO formats. GetTextExtentExPoint() gets text extent with GGO_METRICS. And GGO_METRICS gets font metrics from glyphs loaded with FT_LOAD_TARGET_NORMAL, so when drawing bitmap fonts, which is of GGO_BITMAP format loaded with FT_LOAD_TARGET_MONO, with the calculated text extent, the result text might get truncated because of the font metrics difference. On Windows, anti-aliased font and bitmap font of the same face font reports the same font metrics so such a problem doesn't happen. The ultimate fix is to let FreeType hint the bitmap fonts with the same font metrics as anti-aliased fonts but there are no settings that can achieve it unless we write a new FreeType hinter. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57309 --- dlls/win32u/font.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/dlls/win32u/font.c b/dlls/win32u/font.c index 02f4045f6d9..be71f80ebd3 100644 --- a/dlls/win32u/font.c +++ b/dlls/win32u/font.c @@ -3746,7 +3746,7 @@ static UINT get_glyph_index_linked( struct gdi_font **font, UINT glyph ) return 0; } -static DWORD get_glyph_outline( struct gdi_font *font, UINT glyph, UINT format, +static DWORD get_glyph_outline( struct gdi_font *font, UINT glyph, UINT format, UINT dc_format, GLYPHMETRICS *gm_ret, ABC *abc_ret, DWORD buflen, void *buf, const MAT2 *mat ) { @@ -3755,6 +3755,7 @@ static DWORD get_glyph_outline( struct gdi_font *font, UINT glyph, UINT format, DWORD ret = 1; UINT index = glyph; BOOL tategaki = (*get_gdi_font_name( font ) == '@'); + UINT actual_format; if (format & GGO_GLYPH_INDEX) { @@ -3779,14 +3780,21 @@ static DWORD get_glyph_outline( struct gdi_font *font, UINT glyph, UINT format, if (mat && !memcmp( mat, &identity, sizeof(*mat) )) mat = NULL; - if (format == GGO_METRICS && !mat && get_gdi_font_glyph_metrics( font, format, index, &gm, &abc )) + /* FreeType might report different font metrics for different GGO formats. So to get the correct + * font metrics when GGO_METRICS is specified, use the actual format in dc->aa_flags. Otherwise, + * GGO_METRICS always gets the font metrics from glyphs loaded with FT_LOAD_TARGET_NORMAL, but + * the font metrics might not be sufficient to draw bitmap glyphs loaded with FT_LOAD_TARGET_MONO. + * This way, when format is GGO_METRICS, the returned font metrics should always be enough to + * draw the corresponding font glyphs with specified quality */ + actual_format = format == GGO_METRICS ? dc_format : format; + if (format == GGO_METRICS && !mat && get_gdi_font_glyph_metrics( font, actual_format, index, &gm, &abc )) goto done; - ret = font_funcs->get_glyph_outline( font, index, format, &gm, &abc, buflen, buf, mat, tategaki ); + ret = font_funcs->get_glyph_outline( font, index, actual_format, &gm, &abc, buflen, buf, mat, tategaki ); if (ret == GDI_ERROR) return ret; - if (format == GGO_METRICS && !mat) - set_gdi_font_glyph_metrics( font, format, index, &gm, &abc ); + if (!mat) + set_gdi_font_glyph_metrics( font, actual_format, index, &gm, &abc ); done: if (gm_ret) *gm_ret = gm; @@ -3817,6 +3825,7 @@ static BOOL font_FontIsLinked( PHYSDEV dev ) static BOOL font_GetCharABCWidths( PHYSDEV dev, UINT first, UINT count, WCHAR *chars, ABC *buffer ) { struct font_physdev *physdev = get_font_dev( dev ); + DC *dc = get_physdev_dc( dev ); UINT c, i; if (!physdev->font) @@ -3831,7 +3840,7 @@ static BOOL font_GetCharABCWidths( PHYSDEV dev, UINT first, UINT count, WCHAR *c for (i = 0; i < count; i++) { c = chars ? chars[i] : first + i; - get_glyph_outline( physdev->font, c, GGO_METRICS, NULL, &buffer[i], 0, NULL, NULL ); + get_glyph_outline( physdev->font, c, GGO_METRICS, dc->aa_flags, NULL, &buffer[i], 0, NULL, NULL ); } pthread_mutex_unlock( &font_lock ); return TRUE; @@ -3844,6 +3853,7 @@ static BOOL font_GetCharABCWidths( PHYSDEV dev, UINT first, UINT count, WCHAR *c static BOOL font_GetCharABCWidthsI( PHYSDEV dev, UINT first, UINT count, WORD *gi, ABC *buffer ) { struct font_physdev *physdev = get_font_dev( dev ); + DC *dc = get_physdev_dc( dev ); UINT c; if (!physdev->font) @@ -3857,7 +3867,7 @@ static BOOL font_GetCharABCWidthsI( PHYSDEV dev, UINT first, UINT count, WORD *g pthread_mutex_lock( &font_lock ); for (c = 0; c < count; c++, buffer++) get_glyph_outline( physdev->font, gi ? gi[c] : first + c, GGO_METRICS | GGO_GLYPH_INDEX, - NULL, buffer, 0, NULL, NULL ); + dc->aa_flags, NULL, buffer, 0, NULL, NULL ); pthread_mutex_unlock( &font_lock ); return TRUE; } @@ -3869,6 +3879,7 @@ static BOOL font_GetCharABCWidthsI( PHYSDEV dev, UINT first, UINT count, WORD *g static BOOL font_GetCharWidth( PHYSDEV dev, UINT first, UINT count, const WCHAR *chars, INT *buffer ) { struct font_physdev *physdev = get_font_dev( dev ); + DC *dc = get_physdev_dc( dev ); UINT c, i; ABC abc; @@ -3884,7 +3895,7 @@ static BOOL font_GetCharWidth( PHYSDEV dev, UINT first, UINT count, const WCHAR for (i = 0; i < count; i++) { c = chars ? chars[i] : i + first; - if (get_glyph_outline( physdev->font, c, GGO_METRICS, NULL, &abc, 0, NULL, NULL ) == GDI_ERROR) + if (get_glyph_outline( physdev->font, c, GGO_METRICS, dc->aa_flags, NULL, &abc, 0, NULL, NULL ) == GDI_ERROR) buffer[i] = 0; else buffer[i] = abc.abcA + abc.abcB + abc.abcC; @@ -4055,6 +4066,7 @@ static DWORD font_GetGlyphOutline( PHYSDEV dev, UINT glyph, UINT format, GLYPHMETRICS *gm, DWORD buflen, void *buf, const MAT2 *mat ) { struct font_physdev *physdev = get_font_dev( dev ); + DC *dc = get_physdev_dc( dev ); DWORD ret; if (!physdev->font) @@ -4063,7 +4075,7 @@ static DWORD font_GetGlyphOutline( PHYSDEV dev, UINT glyph, UINT format, return dev->funcs->pGetGlyphOutline( dev, glyph, format, gm, buflen, buf, mat ); } pthread_mutex_lock( &font_lock ); - ret = get_glyph_outline( physdev->font, glyph, format, gm, NULL, buflen, buf, mat ); + ret = get_glyph_outline( physdev->font, glyph, format, dc->aa_flags, gm, NULL, buflen, buf, mat ); pthread_mutex_unlock( &font_lock ); return ret; } @@ -4229,6 +4241,7 @@ static UINT font_GetTextCharsetInfo( PHYSDEV dev, FONTSIGNATURE *fs, DWORD flags static BOOL font_GetTextExtentExPoint( PHYSDEV dev, const WCHAR *str, INT count, INT *dxs ) { struct font_physdev *physdev = get_font_dev( dev ); + DC *dc = get_physdev_dc( dev ); INT i, pos; ABC abc; @@ -4243,7 +4256,7 @@ static BOOL font_GetTextExtentExPoint( PHYSDEV dev, const WCHAR *str, INT count, pthread_mutex_lock( &font_lock ); for (i = pos = 0; i < count; i++) { - get_glyph_outline( physdev->font, str[i], GGO_METRICS, NULL, &abc, 0, NULL, NULL ); + get_glyph_outline( physdev->font, str[i], GGO_METRICS, dc->aa_flags, NULL, &abc, 0, NULL, NULL ); pos += abc.abcA + abc.abcB + abc.abcC; dxs[i] = pos; } @@ -4258,6 +4271,7 @@ static BOOL font_GetTextExtentExPoint( PHYSDEV dev, const WCHAR *str, INT count, static BOOL font_GetTextExtentExPointI( PHYSDEV dev, const WORD *indices, INT count, INT *dxs ) { struct font_physdev *physdev = get_font_dev( dev ); + DC *dc = get_physdev_dc( dev ); INT i, pos; ABC abc; @@ -4272,7 +4286,7 @@ static BOOL font_GetTextExtentExPointI( PHYSDEV dev, const WORD *indices, INT co pthread_mutex_lock( &font_lock ); for (i = pos = 0; i < count; i++) { - get_glyph_outline( physdev->font, indices[i], GGO_METRICS | GGO_GLYPH_INDEX, + get_glyph_outline( physdev->font, indices[i], GGO_METRICS | GGO_GLYPH_INDEX, dc->aa_flags, NULL, &abc, 0, NULL, NULL ); pos += abc.abcA + abc.abcB + abc.abcC; dxs[i] = pos; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8414
It doesn't seem easy to add an automatic test to demonstrate the issue. Attached is a sample program to reproduce the bug visually. [test.zip](/uploads/614d1a4b43a26fcb08e22192554ea51f/test.zip) Also, attaching a picture comparing the results on Windows and Wine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8414#note_107763
I think the current mismatch between requested text extents and actual font rendering affects much more than EV Nova game, starting from, e. g., misplaced cursor positions in file dialogs with true type fonts used. FWIW I was attempting to address similar issue here: https://gitlab.winehq.org/wine/wine/-/merge_requests/4406, although that MR didn't get much attention. That was also working in Proton for a longer time. While looking at your MR I can probably see some things missed in mine, I wonder if plainly passing different format (actual_format instead of requested format) to font_funcs->get_glyph_outline can work right? That affects the format of the returned data, I think we should strictly return the data in the requested format? While GGO_METRIX should return the sizes which correspond to the font de-facto anti aliasing determined when the font is selected to DC. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8414#note_107804
I wonder if plainly passing different format (actual_format instead of requested format) to font_funcs-\>get_glyph_outline can work right?
nevermind, that only differs for GGO_METRICS. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8414#note_107806
... and the test failures in d3dx9_36/core.c are probably the same as discussed and fixed in the linked MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8414#note_107807
FWIW I was attempting to address similar issue here: https://gitlab.winehq.org/wine/wine/-/merge_requests/4406, although that MR didn't get much attention.
Yeah, looks like it's the same issue. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8414#note_108466
@huw Hi, could you give a review? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8414#note_111514
Is there anything I can do to help progress this? (I'm the one who raised the bug) I don't know much about text rendering, so forgive me if this is a dumb question, but would it not be better to ensure bitmapped and anti-aliased have the same spacing like they do in windows? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8414#note_138140
participants (4)
-
andrews05 (＠andrews05) -
Paul Gofman (＠gofman) -
Zhiyi Zhang
-
Zhiyi Zhang (＠zhiyi)