-- v2: 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.
From: Torge Matthies tmatthies@codeweavers.com
Signed-off-by: Torge Matthies tmatthies@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");
From: Torge Matthies tmatthies@codeweavers.com
Signed-off-by: Torge Matthies tmatthies@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..7baeba554f5 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, UINT index ) +{ + 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, index ); + 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;
From: Torge Matthies tmatthies@codeweavers.com
Signed-off-by: Torge Matthies tmatthies@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;
From: Torge Matthies tmatthies@codeweavers.com
Signed-off-by: Torge Matthies tmatthies@codeweavers.com --- dlls/gdiplus/graphics.c | 17 +++++++++++++---- dlls/gdiplus/graphicspath.c | 7 +++++++ dlls/gdiplus/tests/graphics.c | 4 +--- 3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 65b33cdc960..c249235778e 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -5248,6 +5248,7 @@ GpStatus gdip_format_string(HDC hdc, if(*(stringdup + sum + lret) == '\n') { unixstyle_newline = TRUE; + lret += 1; break; }
@@ -5255,13 +5256,14 @@ GpStatus gdip_format_string(HDC hdc, && *(stringdup + sum + lret + 1) == '\n') { unixstyle_newline = FALSE; + lret += 2; 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(lret > 0 && *(stringdup + sum + lret - 1) == '\n') + lineend = fit = lret; else if(fit < (length - sum)){ if(*(stringdup + sum + fit) == ' ') while(*(stringdup + sum + fit) == ' ') @@ -5333,13 +5335,13 @@ GpStatus gdip_format_string(HDC hdc, { height += size.cy; lineno++; - sum += fit + (lret < fitcpy ? 1 : 0); + sum += fit; } else { height += size.cy; lineno++; - sum += fit + (lret < fitcpy ? 2 : 0); + sum += fit; }
hotkeyprefix_pos = hotkeyprefix_end_pos; @@ -5645,6 +5647,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);
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=137218
Your paranoid android.
=== debian11b (64 bit WoW report) ===
d3dx9_36: core.c:830: Test succeeded inside todo block: Got unexpected height 12.
Nikolay Sivov (@nsivov) commented about dlls/win32u/font.c:
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;
+}
There is a helper called get_char_sa() in dwrite, is it too far off? Another, possibly more important, question I have is how does this work for GGO_GLYPH_INDEX? Isn't 'glyph' already an index in that case? ('index' argument is unused by the way)
Esme Povirk (@madewokherd) commented about dlls/gdiplus/graphics.c:
{ height += size.cy; lineno++;
sum += fit + (lret < fitcpy ? 1 : 0);
sum += fit; } else { height += size.cy; lineno++;
sum += fit + (lret < fitcpy ? 2 : 0);
sum += fit; }
Both parts of this if statement are the same now, and it's the only use of the unixstyle_newline variable.
Esme Povirk (@madewokherd) commented about dlls/gdiplus/graphics.c:
&& *(stringdup + sum + lret + 1) == '\n') { unixstyle_newline = FALSE;
lret += 2; 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(lret > 0 && *(stringdup + sum + lret - 1) == '\n')
I don't like this as a condition, and there's a later `lret == fitcpy` check that's missed. I think I'd rather see these lret/fit[cpy] comparisons replaced with a boolean variable for whether a newline was encountered.
On Mon Sep 11 20:30:30 2023 +0000, Esme Povirk wrote:
I don't like this as a condition, and there's a later `lret == fitcpy` check that's missed. I think I'd rather see these lret/fit[cpy] comparisons replaced with a boolean variable for whether a newline was encountered.
Actually, I think an integer for "length of newline encountered" would be better for the part where you decrement `length` below.
On Mon Sep 11 11:58:50 2023 +0000, Nikolay Sivov wrote:
There is a helper called get_char_sa() in dwrite, is it too far off? Another, possibly more important, question I have is how does this work for GGO_GLYPH_INDEX? Isn't 'glyph' already an index in that case? ('index' argument is unused by the way)
The `get_char_sa` function is *really close* to what Windows seems to hardcode (doesn't take from the font's data). It doesn't match 1:1, but it's similar enough that there is definitely strong correlation.
`glyph` here is a bit of a misnomer that I carried over from the calling `get_glyph_outline` function, where it's also called that. `glyph` is the unicode character code (BMP only), `index` is the index in the font that the character was found in.
I wrapped the whole function in an `if (!(format & GGO_GLYPH_INDEX))` for now, but I will need to test how `GGO_GLYPH_INDEX` is handled, and I dropped the `index` parameter.
Note that I will be splitting up this MR into two parts, one for the gdi part and one for the gdiplus part.
On Wed Sep 13 14:08:58 2023 +0000, Torge Matthies wrote:
The `get_char_sa` function is *really close* to what Windows seems to hardcode (doesn't take from the font's data). It doesn't match 1:1, but it's similar enough that there is definitely strong correlation. `glyph` here is a bit of a misnomer that I carried over from the calling `get_glyph_outline` function, where it's also called that. `glyph` is the unicode character code (BMP only), `index` is the index in the font that the character was found in. I wrapped the whole function in an `if (!(format & GGO_GLYPH_INDEX))` for now, but I will need to test how `GGO_GLYPH_INDEX` is handled, and I dropped the `index` parameter. Note that I will be splitting up this MR into two parts, one for the gdi part and one for the gdiplus part.
It does not have to match gdi 1:1. My point was that looking up unicode categories for every character is not really necessary.
On Wed Sep 13 14:11:56 2023 +0000, Nikolay Sivov wrote:
It does not have to match gdi 1:1. My point was that looking up unicode categories for every character is not really necessary.
I was doing it this way for 1. future-proofing for later Unicode versions, 2. just to figure out what even determines whether a character is always considered to have no width. If 1. is not desired (I can imagine that these older functions are not updated anymore on Win?) then of course I can just hardcode the ranges.
On Mon Sep 11 20:28:47 2023 +0000, Esme Povirk wrote:
Both parts of this if statement are the same now, and it's the only use of the unixstyle_newline variable.
Indeed, thank you, I deduplicated the code and removed the `unixstyle_newline` variable.
-
Esme Povirk (＠madewokherd) -
Marvin -
Nikolay Sivov (＠nsivov)
-
Torge Matthies
-
Torge Matthies (＠tmatthies)