[PATCH 0/2] MR3876: Zero-width character handling in GetTextExtentExPoint[AW]
Certain characters are considered by GetTextExtentExPointA/W to have no width, even if they have a non-zero width and a shape in the font. This MR attempts to bring Wine's implementation of these functions closer to native behavior, though I was not able to get it anywhere near perfect. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3876
From: Torge Matthies <tmatthies(a)codeweavers.com> Signed-off-by: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/gdi32/tests/font.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c index 711c9ca4a32..2388b91b5e6 100644 --- a/dlls/gdi32/tests/font.c +++ b/dlls/gdi32/tests/font.c @@ -1399,7 +1399,7 @@ static void test_GetCharABCWidths(void) static void test_text_extents(void) { - static const WCHAR wt[] = L"One\ntwo 3"; + static const WCHAR wt[] = L"One\r\ntwo 3\nfour"; LPINT extents; INT i, len, fit1, fit2, extents2[3]; LOGFONTA lf; @@ -1462,11 +1462,20 @@ static void test_text_extents(void) ok(extents[len-1] == sz1.cx, "GetTextExtentExPointW extents and size don't match\n"); ok(0 <= fit1 && fit1 <= len, "GetTextExtentExPointW generated illegal value %d for fit\n", fit1); ok(0 < fit1, "GetTextExtentExPointW says we can't even fit one letter in 32767 logical units\n"); + todo_wine + ok(extents[2] == extents[3], "Carriage return has width: %d != %d\n", extents[2], extents[3]); + todo_wine + ok(extents[3] == extents[4], "Line feed has width: %d != %d\n", extents[3], extents[4]); + todo_wine + ok(extents[9] == extents[10], "Unix-style newline has width: %d != %d\n", extents[3], extents[4]); GetTextExtentExPointW(hdc, wt, len, extents[2], &fit2, NULL, &sz2); ok(sz1.cx == sz2.cx && sz1.cy == sz2.cy, "GetTextExtentExPointW returned different sizes for the same string\n"); ok(fit2 == 3, "GetTextExtentExPointW extents isn't consistent with fit\n"); GetTextExtentExPointW(hdc, wt, len, extents[2]-1, &fit2, NULL, &sz2); ok(fit2 == 2, "GetTextExtentExPointW extents isn't consistent with fit\n"); + GetTextExtentExPointW(hdc, wt, len, extents[2]+1, &fit2, NULL, &sz2); + todo_wine + ok(fit2 == 5, "GetTextExtentExPointW newline doesn't fit in 1-pixel space\n"); GetTextExtentExPointW(hdc, wt, 2, 0, NULL, extents + 2, &sz2); ok(extents[0] == extents[2] && extents[1] == extents[3], "GetTextExtentExPointW with lpnFit == NULL returns incorrect results\n"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3876
From: Torge Matthies <tmatthies(a)codeweavers.com> Signed-off-by: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/gdi32/tests/font.c | 4 -- dlls/gdiplus/tests/graphics.c | 2 +- dlls/win32u/font.c | 98 +++++++++++++++++++++++++++++++---- 3 files changed, 89 insertions(+), 15 deletions(-) diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c index 2388b91b5e6..65e8064491c 100644 --- a/dlls/gdi32/tests/font.c +++ b/dlls/gdi32/tests/font.c @@ -1462,11 +1462,8 @@ static void test_text_extents(void) ok(extents[len-1] == sz1.cx, "GetTextExtentExPointW extents and size don't match\n"); ok(0 <= fit1 && fit1 <= len, "GetTextExtentExPointW generated illegal value %d for fit\n", fit1); ok(0 < fit1, "GetTextExtentExPointW says we can't even fit one letter in 32767 logical units\n"); - todo_wine ok(extents[2] == extents[3], "Carriage return has width: %d != %d\n", extents[2], extents[3]); - todo_wine ok(extents[3] == extents[4], "Line feed has width: %d != %d\n", extents[3], extents[4]); - todo_wine ok(extents[9] == extents[10], "Unix-style newline has width: %d != %d\n", extents[3], extents[4]); GetTextExtentExPointW(hdc, wt, len, extents[2], &fit2, NULL, &sz2); ok(sz1.cx == sz2.cx && sz1.cy == sz2.cy, "GetTextExtentExPointW returned different sizes for the same string\n"); @@ -1474,7 +1471,6 @@ static void test_text_extents(void) GetTextExtentExPointW(hdc, wt, len, extents[2]-1, &fit2, NULL, &sz2); ok(fit2 == 2, "GetTextExtentExPointW extents isn't consistent with fit\n"); GetTextExtentExPointW(hdc, wt, len, extents[2]+1, &fit2, NULL, &sz2); - todo_wine ok(fit2 == 5, "GetTextExtentExPointW newline doesn't fit in 1-pixel space\n"); GetTextExtentExPointW(hdc, wt, 2, 0, NULL, extents + 2, &sz2); ok(extents[0] == extents[2] && extents[1] == extents[3], diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 89baff64fa4..309a1b1e8ed 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -3358,7 +3358,7 @@ static void test_string_functions(void) expectf_(char_bounds.Width, bounds.Width, 0.01); expectf_(char_bounds.Height + char_height * 3, bounds.Height, 0.05); expect(6, codepointsfitted); - todo_wine expect(4, linesfilled); + expect(4, linesfilled); for (i = 0; i < 4; i++) regions[i] = (GpRegion *)0xdeadbeef; diff --git a/dlls/win32u/font.c b/dlls/win32u/font.c index c21d87c1fbc..d77b44d77e4 100644 --- a/dlls/win32u/font.c +++ b/dlls/win32u/font.c @@ -3830,15 +3830,74 @@ static UINT get_glyph_index_linked( struct gdi_font **font, UINT glyph ) return 0; } +/* Copied together from parts of dlls/kernelbase/locale.c */ +static WORD get_char_type( DWORD type, WCHAR ch ) +{ + static const BYTE *ctype_idx; + static const WORD *ctypes; + + const BYTE *ptr; + + if (!ctypes) + { + const struct + { + UINT sortkeys; + UINT casemaps; + UINT ctypes; + UINT sortids; + } *header; + SIZE_T size; + const WORD *ctype; + + NtGetNlsSectionPtr( 9, 0, NULL, (void **)&header, &size ); + ctype = (WORD *)((char *)header + header->ctypes); + ctypes = ctype + 2; + ctype_idx = (BYTE *)ctype + ctype[1] + 2; + } + + ptr = ctype_idx + ((const WORD *)ctype_idx)[ch >> 8]; + ptr = ctype_idx + ((const WORD *)ptr)[(ch >> 4) & 0x0f] + (ch & 0x0f); + return ctypes[*ptr * 3 + type / 2]; +} + +static inline BOOL is_zero_width( UINT glyph, UINT format ) +{ + WORD ctype1, ctype2, ctype3; + if (!(format & GGO_GLYPH_INDEX)) + { + if (glyph >= 0x80 && glyph <= 0x9F) + return TRUE; + if (glyph >= 0x2B0 && glyph <= 0x2FF) + return FALSE; + if (glyph >= 0x300 && glyph <= 0x36F) + return TRUE; + ctype1 = get_char_type( CT_CTYPE1, glyph ); + ctype2 = get_char_type( CT_CTYPE2, glyph ); + ctype3 = get_char_type( CT_CTYPE3, glyph ); + if (ctype1 == 0x268 && ctype2 == 9 && ctype3 == 0x8) + return TRUE; + if (ctype1 == 0x228 && ctype2 == 8 && ctype3 == 0x8) + return TRUE; + if (ctype1 == 0x220 && ctype2 == 8 && ctype3 == 0x0) + return TRUE; + if (ctype1 == 0x220 && ctype2 == 9 && ctype3 == 0x0) + return TRUE; + /* Incomplete, but this is the best I got. */ + } + return FALSE; +} + static DWORD get_glyph_outline( struct gdi_font *font, UINT glyph, UINT format, GLYPHMETRICS *gm_ret, ABC *abc_ret, DWORD buflen, void *buf, - const MAT2 *mat ) + const MAT2 *mat, BOOL *zero_width ) { GLYPHMETRICS gm; ABC abc; DWORD ret = 1; UINT index = glyph; BOOL tategaki = (*get_gdi_font_name( font ) == '@'); + UINT orig_format = format; if (format & GGO_GLYPH_INDEX) { @@ -3863,6 +3922,9 @@ static DWORD get_glyph_outline( struct gdi_font *font, UINT glyph, UINT format, if (mat && !memcmp( mat, &identity, sizeof(*mat) )) mat = NULL; + if (zero_width) + *zero_width = is_zero_width( glyph, orig_format ); + if (format == GGO_METRICS && !mat && get_gdi_font_glyph_metrics( font, index, &gm, &abc )) goto done; @@ -3873,6 +3935,7 @@ static DWORD get_glyph_outline( struct gdi_font *font, UINT glyph, UINT format, set_gdi_font_glyph_metrics( font, index, &gm, &abc ); done: + if (zero_width && (abc.abcA + abc.abcB + abc.abcC) == 0) *zero_width = TRUE; if (gm_ret) *gm_ret = gm; if (abc_ret) *abc_ret = abc; return ret; @@ -3915,7 +3978,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, NULL, &buffer[i], 0, NULL, NULL, NULL ); } pthread_mutex_unlock( &font_lock ); return TRUE; @@ -3941,7 +4004,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 ); + NULL, buffer, 0, NULL, NULL, NULL ); pthread_mutex_unlock( &font_lock ); return TRUE; } @@ -3968,7 +4031,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, NULL, &abc, 0, NULL, NULL, NULL ) == GDI_ERROR) buffer[i] = 0; else buffer[i] = abc.abcA + abc.abcB + abc.abcC; @@ -4147,7 +4210,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, gm, NULL, buflen, buf, mat, NULL ); pthread_mutex_unlock( &font_lock ); return ret; } @@ -4327,8 +4390,10 @@ 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 ); - pos += abc.abcA + abc.abcB + abc.abcC; + BOOL zero_width = FALSE; + get_glyph_outline( physdev->font, str[i], GGO_METRICS, NULL, &abc, 0, NULL, NULL, &zero_width ); + if (!zero_width) + pos += abc.abcA + abc.abcB + abc.abcC; dxs[i] = pos; } pthread_mutex_unlock( &font_lock ); @@ -4356,9 +4421,11 @@ static BOOL font_GetTextExtentExPointI( PHYSDEV dev, const WORD *indices, INT co pthread_mutex_lock( &font_lock ); for (i = pos = 0; i < count; i++) { + BOOL zero_width = FALSE; get_glyph_outline( physdev->font, indices[i], GGO_METRICS | GGO_GLYPH_INDEX, - NULL, &abc, 0, NULL, NULL ); - pos += abc.abcA + abc.abcB + abc.abcC; + NULL, &abc, 0, NULL, NULL, &zero_width ); + if (!zero_width) + pos += abc.abcA + abc.abcB + abc.abcC; dxs[i] = pos; } pthread_mutex_unlock( &font_lock ); @@ -5347,7 +5414,18 @@ BOOL WINAPI NtGdiGetTextExtentExW( HDC hdc, const WCHAR *str, INT count, INT max { unsigned int dx = abs( INTERNAL_XDSTOWS( dc, pos[i] )) + (i + 1) * dc->attr->char_extra; - if (nfit && dx > (unsigned int)max_ext) break; + if (nfit) + { + unsigned int dx2 = dx; + if (i >= 1) + { + unsigned int prev_dx = abs( INTERNAL_XDSTOWS( dc, pos[i - 1] )) + + i * dc->attr->char_extra; + if (dx == prev_dx) + dx2 += 1; + } + if (dx2 > (unsigned int)max_ext) break; + } if (dxs) dxs[i] = dx; } if (nfit) *nfit = i; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3876
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=137537 Your paranoid android. === debian11b (64 bit WoW report) === d3dx9_36: core.c:830: Test succeeded inside todo block: Got unexpected height 12.
See https://gitlab.winehq.org/wine/wine/-/merge_requests/3876 for previous conversations -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3876#note_45708
I don't think it makes sense for me to review a single todo_wine removed from the gdiplus tests, and I don't really have the background knowledge for the rest of it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3876#note_45802
On Mon Sep 18 21:43:18 2023 +0000, Esme Povirk wrote:
I don't think it makes sense for me to review a single todo_wine removed from the gdiplus tests, and I don't really have the background knowledge for the rest of it. Yeah, you were added as a reviewer automatically due to that single line change in the tests, sorry about that!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3876#note_45847
participants (4)
-
Esme Povirk (＠madewokherd) -
Marvin -
Torge Matthies
-
Torge Matthies (＠tmatthies)