[PATCH v2 0/2] MR4554: gdi32/tests: Ensure ABC width and GGO_BITMAP gmCellIncX match
Also fix the reported width on existing tests -- v2: win32u: Fix reported metrics by using bitmap strikes https://gitlab.winehq.org/wine/wine/-/merge_requests/4554
From: Brendan McGrath <bmcgrath(a)codeweavers.com> Also fix the reported width on existing tests --- dlls/gdi32/tests/font.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c index 76ea0ac7de7..f8c7833fef0 100644 --- a/dlls/gdi32/tests/font.c +++ b/dlls/gdi32/tests/font.c @@ -1313,24 +1313,30 @@ static void test_GetCharABCWidths(void) } for (j = 1; j <= 80; j++) { - GLYPHMETRICS gm; + GLYPHMETRICS gm, gm_bitmap; lf.lfWidth = j; hfont = CreateFontIndirectA(&lf); hfont = SelectObject(hdc, hfont); nb = GetGlyphOutlineA(hdc, code, GGO_METRICS, &gm, 0, NULL, &mat); - ok(nb, "GetGlyphOutlineA should have succeeded at width %d\n", i); + ok(nb, "GetGlyphOutlineA should have succeeded at width %d\n", j); + + nb = GetGlyphOutlineA(hdc, code, GGO_BITMAP, &gm_bitmap, 0, NULL, &mat); + ok(nb, "GetGlyphOutlineA should have succeeded with bitmap at width %d\n", j); ret = GetCharABCWidthsA(hdc, code, code, abc); - ok(ret, "GetCharABCWidthsA should have succeeded at width %d\n", i); + ok(ret, "GetCharABCWidthsA should have succeeded at width %d\n", j); ok(abc[0].abcA == gm.gmptGlyphOrigin.x, "abcA(%d) and gmptGlyphOrigin.x(%ld) values are different at width %d\n", - abc[0].abcA, gm.gmptGlyphOrigin.x, i); + abc[0].abcA, gm.gmptGlyphOrigin.x, j); ok(abc[0].abcB == gm.gmBlackBoxX, "abcB(%d) and gmBlackBoxX(%d) values are different at width %d\n", - abc[0].abcB, gm.gmBlackBoxX, i); + abc[0].abcB, gm.gmBlackBoxX, j); + todo_wine_if(i == 1 && j == 7) ok(abc[0].abcA + abc[0].abcB + abc[0].abcC == gm_bitmap.gmCellIncX, + "sum of abc value (%d, %d, %d) is different to gmCellIncX of bitmap (%d) at width %d\n", + abc[0].abcA, abc[0].abcB, abc[0].abcC, gm_bitmap.gmCellIncX, j); DeleteObject(SelectObject(hdc, hfont)); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4554
From: Brendan McGrath <bmcgrath(a)codeweavers.com> When rendering the bitmap, bitmap strikes are used, but they were not being used when reporting metrics. This resulted in discrepancies between some characters in some font sizes. This fixes words from being truncated unnecessarily from a static control. --- dlls/gdi32/tests/font.c | 2 +- dlls/win32u/freetype.c | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c index f8c7833fef0..0c472cbf75e 100644 --- a/dlls/gdi32/tests/font.c +++ b/dlls/gdi32/tests/font.c @@ -1334,7 +1334,7 @@ static void test_GetCharABCWidths(void) ok(abc[0].abcB == gm.gmBlackBoxX, "abcB(%d) and gmBlackBoxX(%d) values are different at width %d\n", abc[0].abcB, gm.gmBlackBoxX, j); - todo_wine_if(i == 1 && j == 7) ok(abc[0].abcA + abc[0].abcB + abc[0].abcC == gm_bitmap.gmCellIncX, + ok(abc[0].abcA + abc[0].abcB + abc[0].abcC == gm_bitmap.gmCellIncX, "sum of abc value (%d, %d, %d) is different to gmCellIncX of bitmap (%d) at width %d\n", abc[0].abcA, abc[0].abcB, abc[0].abcC, gm_bitmap.gmCellIncX, j); DeleteObject(SelectObject(hdc, hfont)); diff --git a/dlls/win32u/freetype.c b/dlls/win32u/freetype.c index 6043c66bc53..83620174696 100644 --- a/dlls/win32u/freetype.c +++ b/dlls/win32u/freetype.c @@ -3464,7 +3464,11 @@ static UINT freetype_get_glyph_outline( struct gdi_font *font, UINT glyph, UINT if (vertical_metrics && FT_SimpleVersion < FT_VERSION_VALUE(2, 4, 0)) vertical_metrics = FALSE; - if (matrices || format != GGO_BITMAP) load_flags |= FT_LOAD_NO_BITMAP; + if (matrices || format == GGO_NATIVE) + load_flags |= FT_LOAD_NO_BITMAP; + else if ( format == GGO_METRICS ) + load_flags |= FT_LOAD_BITMAP_METRICS_ONLY; + if (vertical_metrics) load_flags |= FT_LOAD_VERTICAL_LAYOUT; err = pFT_Load_Glyph(ft_face, glyph, load_flags); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4554
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=140562 Your paranoid android. === debian11b (64 bit WoW report) === d3dx9_36: core.c:637: Test failed: Got unexpected texture 00007FFFFE8A4370. core.c:637: Test failed: Got unexpected texture 00007FFFFE8A4370. core.c:637: Test failed: Got unexpected texture 00007FFFFE8A4370. core.c:1851: Test failed: Invalid refcount. Expected 0, got 1 core.c:1852: Test failed: Invalid refcount. Expected 0, got 1
Have you seen my MR https://gitlab.winehq.org/wine/wine/-/merge_requests/4406 ? Does it help the original issue as well? I ran your test, and my MR fixes the added todo. Although I get a lot of test failures locally in the existing test (the part of it before your addition) with and without my MR which I suppose are related to some different issues. If you have some explanation why this change might be better or what did I miss in that MR could you please elaborate if possible? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4554#note_54401
On a separate note, the added test succeeds on Wine only with default non-antialiased font quality. Setting lf.lfQuality = CLEARTYPE_QUALITY; before creating font makes it fail at every width here. I think the only way this can succeed is if we had all the same metrics for all the load targets from freetype (which is not the case), while as far as I could see (testing through some limited set of fonts) glyph metrics do not differ for different antialiasing options on Windows. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4554#note_54406
Hey Paul, I hadn't seen your MR - thanks for bringing that to my attention. I'm now thinking my approach is the wrong way to go and your approach is better. I think mine works because the metrics of the bitmap strike being used just happen to be smaller than the space provided by the application I was fixing. But it results in a different render output, when all I really want to do is fix the metrics (as I believe you have done). The test in gdi32 only tests the one character too (the letter 'a') - so I might have passed that test just by luck. Maybe I should update the test to test 'a-z' and 'A-Z'. With your MR, the text is still truncated in the application I'm trying to fix, but that could be due to the truncation algorithm - as with your MR it only misses by a single pixel (with master it is 8). I know Wine's truncation algorithm is more aggressive than Windows, so I'll turn my focus to that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4554#note_54517
This merge request was closed by Brendan McGrath. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4554
I'll close this MR, as I'll use !4406 for the metrics fix, and raise a new MR with a fix to the truncation algorithm. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4554#note_54519
participants (4)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Marvin -
Paul Gofman (＠gofman)