[PATCH v3 0/4] MR3517: Fixes for GDI/GDI+ newline measurements.
-- v3: gdiplus: Include the newline in the measurement of each line. gdiplus/tests: Add test for bounds of newline. win32u: Detect and handle characters that are considered to have no width for GetTextExtentExPoint. gdi32/tests: Add test for width of carriage return and line feed. https://gitlab.winehq.org/wine/wine/-/merge_requests/3517
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/3517
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/3517
From: Torge Matthies <tmatthies(a)codeweavers.com> Signed-off-by: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/gdiplus/tests/graphics.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 309a1b1e8ed..96a940d5fb7 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -3360,6 +3360,20 @@ static void test_string_functions(void) expect(6, codepointsfitted); expect(4, linesfilled); + rc.Width = 0; + rc.Height = 0; + + status = GdipMeasureString(graphics, L"\n", -1, font, &rc, NULL, &bounds, &codepointsfitted, &linesfilled); + expect(Ok, status); + expectf(0.0, bounds.X); + expectf(0.0, bounds.Y); + expectf_(3.33, bounds.Width, 0.01); + todo_wine + expectf_(char_bounds.Height, bounds.Height, 0.05); + todo_wine + expect(1, codepointsfitted); + expect(1, linesfilled); + for (i = 0; i < 4; i++) regions[i] = (GpRegion *)0xdeadbeef; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3517
From: Torge Matthies <tmatthies(a)codeweavers.com> Signed-off-by: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/gdiplus/graphics.c | 38 +++++++++++++++++------------------ dlls/gdiplus/graphicspath.c | 7 +++++++ dlls/gdiplus/tests/graphics.c | 4 +--- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 65b33cdc960..fdb1b7d8b36 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -5171,7 +5171,7 @@ GpStatus gdip_format_string(HDC hdc, INT *hotkeyprefix_offsets=NULL; INT hotkeyprefix_count=0; INT hotkeyprefix_pos=0, hotkeyprefix_end_pos=0; - BOOL seen_prefix = FALSE, unixstyle_newline = TRUE; + BOOL seen_prefix = FALSE, found_newline; if(length == -1) length = lstrlenW(string); @@ -5244,24 +5244,27 @@ GpStatus gdip_format_string(HDC hdc, if(fit == 0) break; + found_newline = FALSE; for(lret = 0; lret < fit; lret++) { if(*(stringdup + sum + lret) == '\n') { - unixstyle_newline = TRUE; + lret += 1; + found_newline = TRUE; break; } if(*(stringdup + sum + lret) == '\r' && lret + 1 < fit && *(stringdup + sum + lret + 1) == '\n') { - unixstyle_newline = FALSE; + lret += 2; + found_newline = TRUE; break; } } /* Line break code (may look strange, but it imitates windows). */ - if(lret < fit) - lineend = fit = lret; /* this is not an off-by-one error */ + if(found_newline) + lineend = fit = lret; else if(fit < (length - sum)){ if(*(stringdup + sum + fit) == ' ') while(*(stringdup + sum + fit) == ' ') @@ -5328,19 +5331,9 @@ GpStatus gdip_format_string(HDC hdc, if (stat != Ok) break; - - if (unixstyle_newline) - { - height += size.cy; - lineno++; - sum += fit + (lret < fitcpy ? 1 : 0); - } - else - { - height += size.cy; - lineno++; - sum += fit + (lret < fitcpy ? 2 : 0); - } + height += size.cy; + lineno++; + sum += fit; hotkeyprefix_pos = hotkeyprefix_end_pos; @@ -5348,7 +5341,7 @@ GpStatus gdip_format_string(HDC hdc, break; /* Stop if this was a linewrap (but not if it was a linebreak). */ - if ((lret == fitcpy) && (format->attr & StringFormatFlagsNoWrap)) + if (!found_newline && (format->attr & StringFormatFlagsNoWrap)) break; } @@ -5645,6 +5638,13 @@ static GpStatus draw_string_callback(HDC hdc, position.X = args->x + bounds->X / args->rel_width; position.Y = args->y + bounds->Y / args->rel_height + args->ascent; + if (length > 0 && string[index + length - 1] == '\n') + { + length--; + if (length > 0 && string[index + length - 1] == '\r') + length--; + } + stat = draw_driver_string(args->graphics, &string[index], length, font, format, args->brush, &position, DriverStringOptionsCmapLookup|DriverStringOptionsRealizedAdvance, NULL); diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c index fd2622daf0e..67d8a980ee5 100644 --- a/dlls/gdiplus/graphicspath.c +++ b/dlls/gdiplus/graphicspath.c @@ -964,6 +964,13 @@ static GpStatus format_string_callback(HDC dc, float y = rect->Y + (bounds->Y - rect->Y) * args->scale; int i; + if (length > 0 && string[index + length - 1] == '\n') + { + length--; + if (length > 0 && string[index + length - 1] == '\r') + length--; + } + if (underlined_index_count) FIXME("hotkey underlines not drawn yet\n"); diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 96a940d5fb7..392c828a442 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -3321,7 +3321,7 @@ static void test_string_functions(void) expect(Ok, status); expectf(0.0, bounds.X); expectf(0.0, bounds.Y); - todo_wine expect(5, codepointsfitted); + expect(5, codepointsfitted); todo_wine expect(1, linesfilled); /* Cut off everything after the first space. */ @@ -3368,9 +3368,7 @@ static void test_string_functions(void) expectf(0.0, bounds.X); expectf(0.0, bounds.Y); expectf_(3.33, bounds.Width, 0.01); - todo_wine expectf_(char_bounds.Height, bounds.Height, 0.05); - todo_wine expect(1, codepointsfitted); expect(1, linesfilled); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3517
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=137343 Your paranoid android. === debian11b (64 bit WoW report) === d3dx9_36: core.c:830: Test succeeded inside todo block: Got unexpected height 12.
On Wed Sep 13 14:55:09 2023 +0000, Torge Matthies wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/3517/diffs?diff_id=68800&start_sha=e5e16d4184a4086502e7a9a5225ee8bb8820ee13#aaf93e29527c1cbc47ec54679da5828ebd679128_5265_5266) Sorry, I didn't find the part you referred to where I decrement `length`, which line do you mean? Or did I already change/remove it?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3517#note_45231
On Wed Sep 13 14:56:56 2023 +0000, Torge Matthies wrote:
Sorry, I didn't find the part you referred to where I decrement `length`, which line do you mean? Or did I already change/remove it? A couple of gdip_format_string_callback's were modified to check for newline characters at the end. I think the callback signature should be modified to include the number of newline characters instead (and possibly that callback has reached the point where passing a pointer to a struct would make more sense).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3517#note_45604
Seems like the gdiplus/tests commit has some gdi32 changes that shouldn't be there? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3517#note_45605
This merge request was closed by Torge Matthies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3517
Closed since the gdi32 part now lives in https://gitlab.winehq.org/wine/wine/-/merge_requests/3876 and gdiplus will be a separate MR -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3517#note_45706
participants (4)
-
Esme Povirk (＠madewokherd) -
Marvin -
Torge Matthies
-
Torge Matthies (＠tmatthies)