[PATCH 0/1] MR10788: Draft: gdiplus: Fix GdiMeasureString line height to use 4/3 emSize
Hi @dmitry @madewokherd I need some advice from experts before I proceed with further work. By default, GDI+ adds a small amount of padding for non typographic. It is top 1/6 padding and 1/6 bottom padding of emSize. Non-Typographic format uses the 4/3 ratio to ensure vertical spacing and avoid clipping overhanging glyphs. Typographic format doesn't include any padding. This patch is causing massive test success: ``` graphics.c:3987: Test succeeded inside todo block: Expected 36.000000, got 35.760002 graphics.c:4004: Test succeeded inside todo block: Expected 36.000000, got 35.760002 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333344 graphics.c:3987: Test succeeded inside todo block: Expected 18.000000, got 17.880001 graphics.c:4004: Test succeeded inside todo block: Expected 18.000000, got 17.879997 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333321 graphics.c:3987: Test succeeded inside todo block: Expected 150.000000, got 149.000000 graphics.c:4004: Test succeeded inside todo block: Expected 150.000000, got 149.000000 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 75.000000, got 74.500000 graphics.c:4004: Test succeeded inside todo block: Expected 75.000000, got 74.500000 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 12.699999, got 12.615334 graphics.c:4004: Test succeeded inside todo block: Expected 12.699999, got 12.615334 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 6.349999, got 6.307667 graphics.c:4004: Test succeeded inside todo block: Expected 6.349999, got 6.307667 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 36.000000, got 35.760002 graphics.c:4004: Test succeeded inside todo block: Expected 36.000000, got 35.760002 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333344 graphics.c:3987: Test succeeded inside todo block: Expected 18.000000, got 17.880001 graphics.c:4004: Test succeeded inside todo block: Expected 18.000000, got 17.879997 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333321 graphics.c:3987: Test succeeded inside todo block: Expected 150.000000, got 149.000000 graphics.c:4004: Test succeeded inside todo block: Expected 150.000000, got 149.000000 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 75.000000, got 74.500000 graphics.c:4004: Test succeeded inside todo block: Expected 75.000000, got 74.500000 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 12.700000, got 12.615334 graphics.c:4004: Test succeeded inside todo block: Expected 12.700000, got 12.615334 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 6.350000, got 6.307667 graphics.c:4004: Test succeeded inside todo block: Expected 6.350000, got 6.307667 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 36.000000, got 35.760002 graphics.c:4004: Test succeeded inside todo block: Expected 36.000000, got 35.760002 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333344 graphics.c:3987: Test succeeded inside todo block: Expected 18.000000, got 17.880001 graphics.c:4004: Test succeeded inside todo block: Expected 18.000000, got 17.879997 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333321 graphics.c:3987: Test succeeded inside todo block: Expected 150.000000, got 149.000000 graphics.c:4004: Test succeeded inside todo block: Expected 150.000000, got 149.000000 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 75.000000, got 74.500000 graphics.c:4004: Test succeeded inside todo block: Expected 75.000000, got 74.500000 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 12.699999, got 12.615334 graphics.c:4004: Test succeeded inside todo block: Expected 12.699999, got 12.615334 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 6.349999, got 6.307667 graphics.c:4004: Test succeeded inside todo block: Expected 6.349999, got 6.307667 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 35.999996, got 35.760002 graphics.c:4004: Test succeeded inside todo block: Expected 35.999996, got 35.760002 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333344 graphics.c:3987: Test succeeded inside todo block: Expected 17.999998, got 17.880001 graphics.c:4004: Test succeeded inside todo block: Expected 17.999998, got 17.879997 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333321 graphics.c:3987: Test succeeded inside todo block: Expected 150.000000, got 149.000000 graphics.c:4004: Test succeeded inside todo block: Expected 150.000000, got 149.000000 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 75.000000, got 74.500000 graphics.c:4004: Test succeeded inside todo block: Expected 75.000000, got 74.500000 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 12.700000, got 12.615334 graphics.c:4004: Test succeeded inside todo block: Expected 12.700000, got 12.615334 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 graphics.c:3987: Test succeeded inside todo block: Expected 6.350000, got 6.307667 graphics.c:4004: Test succeeded inside todo block: Expected 6.350000, got 6.307667 graphics.c:4016: Test succeeded inside todo block: Expected 100.000000, got 99.333336 ``` and one failure: ``` graphics.c:3444: Test failed: Expected 52.000000, got 50.000000 ``` I would need help with testing and reviewing it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10788
From: Bartosz Kosiorek <gang65@poczta.onet.pl> By default, GDI+ adds a small amount of padding. Top 1/6 padding and 1/6 bottom padding of emSize. Non-Typographic format uses the 4/3 ratio to ensure vertical spacing and avoid clipping overhanging glyphs. Typographic format doesn't include any padding. --- dlls/gdiplus/graphics.c | 6 ++++++ dlls/gdiplus/tests/font.c | 17 ++++++----------- dlls/gdiplus/tests/graphics.c | 2 -- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 0faac1325d2..2f80f77e269 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -5707,6 +5707,7 @@ GpStatus gdip_format_string(GpGraphics *graphics, HDC hdc, StringAlignment halign; GpStatus stat = Ok; SIZE size; + TEXTMETRICW tm; HotkeyPrefix hkprefix; INT *hotkeyprefix_offsets=NULL; INT hotkeyprefix_count=0; @@ -5787,6 +5788,8 @@ GpStatus gdip_format_string(GpGraphics *graphics, HDC hdc, halign = format->align; + GetTextMetricsW(hdc, &tm); + generate_font_link_info(&info, length, font); while(sum < length){ @@ -5842,6 +5845,9 @@ GpStatus gdip_format_string(GpGraphics *graphics, HDC hdc, bounds.Width = size.cx; + if (!format->generic_typographic) + size.cy = (size.cy - tm.tmInternalLeading) * 4.0 / 3.0; + if(height + size.cy > nheight) { if (format->attr & StringFormatFlagsLineLimit) diff --git a/dlls/gdiplus/tests/font.c b/dlls/gdiplus/tests/font.c index 1bbb18f4c6d..4c1aeef0649 100644 --- a/dlls/gdiplus/tests/font.c +++ b/dlls/gdiplus/tests/font.c @@ -1104,15 +1104,14 @@ static void test_font_transform(void) expect(Ok, status); expectf(0.0, bounds.X); expectf(0.0, bounds.Y); - todo_wine - expectf(height + margin_y, bounds.Height); + expectf_(height + margin_y, bounds.Height, 0.5); set_rect_empty(&rect); set_rect_empty(&bounds); status = GdipMeasureString(graphics, string, -1, font, &rect, typographic, &bounds, NULL, NULL); expect(Ok, status); expectf(0.0, bounds.X); expectf(0.0, bounds.Y); - expectf_(height, bounds.Height, 1.0); + expectf_(height, bounds.Height, 0.5); set_rect_empty(&bounds); status = GdipMeasureDriverString(graphics, (const UINT16 *)string, -1, font, pos, DriverStringOptionsCmapLookup, NULL, &bounds); @@ -1148,8 +1147,7 @@ static void test_font_transform(void) expect(Ok, status); expectf(0.0, bounds.X); expectf(0.0, bounds.Y); - todo_wine - expectf(height + margin_y, bounds.Height); + expectf_(height + margin_y, bounds.Height, 0.5); set_rect_empty(&rect); set_rect_empty(&bounds); status = GdipMeasureString(graphics, string, -1, font, &rect, typographic, &bounds, NULL, NULL); @@ -1194,8 +1192,7 @@ static void test_font_transform(void) expect(Ok, status); expectf(0.0, bounds.X); expectf(0.0, bounds.Y); - todo_wine - expectf(height + margin_y, bounds.Height); + expectf_(height + margin_y, bounds.Height, 0.5); set_rect_empty(&rect); set_rect_empty(&bounds); status = GdipMeasureString(graphics, string, -1, font, &rect, typographic, &bounds, NULL, NULL); @@ -1242,8 +1239,7 @@ static void test_font_transform(void) expect(Ok, status); expectf(0.0, bounds.X); expectf(0.0, bounds.Y); - todo_wine - expectf(height + margin_y, bounds.Height); + expectf_(height + margin_y, bounds.Height, 0.5); set_rect_empty(&rect); set_rect_empty(&bounds); status = GdipMeasureString(graphics, string, -1, font, &rect, typographic, &bounds, NULL, NULL); @@ -1290,8 +1286,7 @@ static void test_font_transform(void) expect(Ok, status); expectf(0.0, bounds.X); expectf(0.0, bounds.Y); - todo_wine - expectf(height + margin_y, bounds.Height); + expectf_(height + margin_y, bounds.Height, 0.5); set_rect_empty(&rect); set_rect_empty(&bounds); status = GdipMeasureString(graphics, string, -1, font, &rect, typographic, &bounds, NULL, NULL); diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 8703081070f..ee0384cfb3e 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -3896,7 +3896,6 @@ static void test_GdipMeasureString(void) expectf(0.0, bounds.X); expectf(0.0, bounds.Y); - todo_wine expectf_(height, bounds.Height, height / 100.0); expectf_(bounds.Height / base_cy, bounds.Width / base_cx, 0.1); expect(7, chars); @@ -3913,7 +3912,6 @@ static void test_GdipMeasureString(void) expect(Ok, status); expectf(50.0, bounds.X); expectf(50.0, bounds.Y); - todo_wine expectf_(height, bounds.Height, height / 100.0); expectf_(bounds.Height / base_cy, bounds.Width / base_cx, 0.1); expect(7, chars); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10788
My suggestion is to do our measurement tests in the native size of the font. We should be able to precisely match native at that size, assuming the font/glyph metrics are the same. At other sizes, the decision to not try to make linear scaling work means we can be off by an arbitrary amount. I would also want to check on interactions with other functions like GdipGetFontHeight and MeasureCharacterRanges. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10788#note_138426
participants (3)
-
Bartosz Kosiorek -
Bartosz Kosiorek (＠gang65)
-
Esme Povirk (＠madewokherd)