[PATCH v5 0/2] MR9775: gdiplus: Move X position calculation to callbacks.
An application I'm working on does GdipMeasureCharacterRanges(graphics, "1", 1, ..., 2, regions); GdipGetRegionBounds(regions[0], graphics, &bounds); GdipSetClipRegion(graphics, regions[0], CombineModeReplace); GdipDrawString(graphics, graphics, "1", 1, ..); which leads to an almost completely clipped text output. Attached test application graphically shows the results of bounding boxes calculated by GdipMeasureCharacterRanges(), GdipMeasureString() and GdipDrawString(). This patchset makes the resulting image look closer to what is observed under Windows. Gdip[test_GdipMeasureCharacterRanges.tar.xz](/uploads/53c8bd3f9b7b005d5b0b675296f0631c/test_GdipMeasureCharacterRanges.tar.xz) -- v5: gdiplus: Copy logic for calculating y-offset from GdipDrawString() to GdipMeasureCharacterRanges(). gdiplus: Copy logic for trimming bounds width from GdipMeasureString() to GdipMeasureCharacterRanges(). https://gitlab.winehq.org/wine/wine/-/merge_requests/9775
From: Dmitry Timoshkov <dmitry@baikal.ru> Signed-off-by: Dmitry Timoshkov <dmitry@baikal.ru> --- dlls/gdiplus/graphics.c | 7 ++++++- dlls/gdiplus/tests/graphics.c | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 1d046e7d147..898a2e90d28 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -6012,7 +6012,6 @@ GpStatus WINGDIPAPI GdipMeasureCharacterRanges(GpGraphics* graphics, if (stringFormat->attr) TRACE("may be ignoring some format flags: attr %x\n", stringFormat->attr); - margin_x = stringFormat->generic_typographic ? 0.0 : font->emSize / 6.0; margin_x *= units_scale(font->unit, graphics->unit, graphics->xres, graphics->printer_display); transform_properties(graphics, NULL, TRUE, &args.rel_width, &args.rel_height, NULL); @@ -6020,6 +6019,12 @@ GpStatus WINGDIPAPI GdipMeasureCharacterRanges(GpGraphics* graphics, scaled_rect.Y = layoutRect->Y * args.rel_height; scaled_rect.Width = layoutRect->Width * args.rel_width; scaled_rect.Height = layoutRect->Height * args.rel_height; + if (scaled_rect.Width >= 0.5f) + { + scaled_rect.Width -= margin_x * 2.0f * args.rel_width; + if (scaled_rect.Width < 0.5f) /* doesn't fit */ + scaled_rect.Width = 0.5f; + } if (scaled_rect.Width >= 1 << 23) scaled_rect.Width = 1 << 23; if (scaled_rect.Height >= 1 << 23) scaled_rect.Height = 1 << 23; diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 6ac85fe303c..337e1955021 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -4749,8 +4749,11 @@ static void test_measure_string(void) set_rect_empty(&bounds); status = GdipGetRegionBounds(region, graphics, &bounds); expect(Ok, status); + todo_wine expectf_(5.0 + margin_x, bounds.X, 1.0); + todo_wine expectf(5.0, bounds.Y); + todo_wine expectf_(width_1, bounds.Width, 1.0); todo_wine expectf_(height - margin_y, bounds.Height, 1.0); @@ -4780,9 +4783,13 @@ static void test_measure_string(void) set_rect_empty(&bounds); status = GdipGetRegionBounds(region, graphics, &bounds); expect(Ok, status); + todo_wine expectf_(5.0 + margin_x, bounds.X, 1.0); + todo_wine expectf(5.0, bounds.Y); + todo_wine expectf_(width_1, bounds.Width, 1.0); + todo_wine expectf(height_rgn, bounds.Height); set_rect_empty(&rect); @@ -4948,7 +4955,6 @@ static void test_measure_string(void) set_rect_empty(&bounds); status = GdipGetRegionBounds(region, graphics, &bounds); expect(Ok, status); - todo_wine expectf_(5.0 + width_rgn/2.0, bounds.X, 1.0); todo_wine expectf_(5.0 + height_rgn/2.0, bounds.Y, 1.0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9775
From: Dmitry Timoshkov <dmitry@baikal.ru> Signed-off-by: Dmitry Timoshkov <dmitry@baikal.ru> --- dlls/gdiplus/graphics.c | 19 +++++++++++++++++-- dlls/gdiplus/tests/graphics.c | 4 ---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 898a2e90d28..0faac1325d2 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -5986,7 +5986,7 @@ GpStatus WINGDIPAPI GdipMeasureCharacterRanges(GpGraphics* graphics, struct measure_ranges_args args; HDC hdc, temp_hdc=NULL; RectF scaled_rect; - REAL margin_x; + REAL margin_x, offsety = 0.0f; TRACE("(%p %s %d %p %s %p %d %p)\n", graphics, debugstr_wn(string, length), length, font, debugstr_rectf(layoutRect), stringFormat, regionCount, regions); @@ -6012,11 +6012,26 @@ GpStatus WINGDIPAPI GdipMeasureCharacterRanges(GpGraphics* graphics, if (stringFormat->attr) TRACE("may be ignoring some format flags: attr %x\n", stringFormat->attr); + if (stringFormat->line_align != StringAlignmentNear) + { + RectF bounds, in_rect = *layoutRect; + in_rect.Height = 0.0f; /* avoid height clipping */ + GdipMeasureString(graphics, string, length, font, &in_rect, stringFormat, &bounds, NULL, NULL); + + TRACE("bounds %s\n", debugstr_rectf(&bounds)); + + if (stringFormat->line_align == StringAlignmentCenter) + offsety = (layoutRect->Height - bounds.Height) / 2.0f; + else if (stringFormat->line_align == StringAlignmentFar) + offsety = layoutRect->Height - bounds.Height; + } + TRACE("line align %d, offsety %f\n", stringFormat->line_align, offsety); + margin_x = stringFormat->generic_typographic ? 0.0 : font->emSize / 6.0; margin_x *= units_scale(font->unit, graphics->unit, graphics->xres, graphics->printer_display); transform_properties(graphics, NULL, TRUE, &args.rel_width, &args.rel_height, NULL); scaled_rect.X = (layoutRect->X + margin_x) * args.rel_width; - scaled_rect.Y = layoutRect->Y * args.rel_height; + scaled_rect.Y = (layoutRect->Y + offsety) * args.rel_height; scaled_rect.Width = layoutRect->Width * args.rel_width; scaled_rect.Height = layoutRect->Height * args.rel_height; if (scaled_rect.Width >= 0.5f) diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 337e1955021..8703081070f 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -4956,7 +4956,6 @@ static void test_measure_string(void) status = GdipGetRegionBounds(region, graphics, &bounds); expect(Ok, status); expectf_(5.0 + width_rgn/2.0, bounds.X, 1.0); - todo_wine expectf_(5.0 + height_rgn/2.0, bounds.Y, 1.0); expectf_(width_rgn, bounds.Width, 1.0); expectf_(height_rgn, bounds.Height, 1.0); @@ -4972,7 +4971,6 @@ static void test_measure_string(void) expect(Ok, status); todo_wine expectf_(5.0 - width_rgn/2.0, bounds.X, 1.0); - todo_wine expectf_(5.0 - height_rgn/2.0, bounds.Y, 1.0); expectf_(width_rgn, bounds.Width, 1.0); expectf_(height_rgn, bounds.Height, 1.0); @@ -5020,7 +5018,6 @@ static void test_measure_string(void) expect(Ok, status); todo_wine expectf_(5.0 + width_rgn, bounds.X, 2.0); - todo_wine expectf_(5.0 + height_rgn, bounds.Y, 1.0); expectf_(width_rgn, bounds.Width, 1.0); expectf_(height_rgn, bounds.Height, 1.0); @@ -5036,7 +5033,6 @@ static void test_measure_string(void) expect(Ok, status); todo_wine expectf_(5.0 - width_rgn, bounds.X, 2.0); - todo_wine expectf_(5.0 - height_rgn, bounds.Y, 1.0); expectf_(width_rgn, bounds.Width, 1.0); expectf_(height_rgn, bounds.Height, 1.0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9775
On Fri Apr 24 11:39:25 2026 +0000, Esme Povirk wrote:
I think I'm OK with moving horizontal alignment into the callbacks, on the basis that they're sort of half handling it already, and this seems to simplify it. I've updated the patches taking into account current git changes, and these patches make my test application attached to this MR work, and your test app still works as before.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_137596
Bartosz Kosiorek (@gang65) commented about dlls/gdiplus/graphics.c:
if (stringFormat->attr) TRACE("may be ignoring some format flags: attr %x\n", stringFormat->attr);
+ if (stringFormat->line_align != StringAlignmentNear) + { + RectF bounds, in_rect = *layoutRect; + in_rect.Height = 0.0f; /* avoid height clipping */ + GdipMeasureString(graphics, string, length, font, &in_rect, stringFormat, &bounds, NULL, NULL); + + TRACE("bounds %s\n", debugstr_rectf(&bounds)); + + if (stringFormat->line_align == StringAlignmentCenter) + offsety = (layoutRect->Height - bounds.Height) / 2.0f; + else if (stringFormat->line_align == StringAlignmentFar) + offsety = layoutRect->Height - bounds.Height; + }
@dmitry Can you create separate Merge Request only with this change (`line_align` support). Is it introducing regression in tests? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_137682
Bartosz Kosiorek (@gang65) commented about dlls/gdiplus/tests/graphics.c:
set_rect_empty(&bounds); status = GdipGetRegionBounds(region, graphics, &bounds); expect(Ok, status); + todo_wine expectf_(5.0 + margin_x, bounds.X, 1.0); + todo_wine expectf(5.0, bounds.Y); + todo_wine expectf_(width_1, bounds.Width, 1.0); + todo_wine
I am worried that with this MR, new regressions was introduced. I would prefer to introduce fixes, which doesn't broke existing tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_137683
On Sun Apr 26 14:06:21 2026 +0000, Bartosz Kosiorek wrote:
I am worried that with this MR, new regressions was introduced. I would prefer to introduce fixes, which doesn't broke existing tests. The tests that I marked as todo_wine were added by me long time ago for completeness, they don't replicate something that I observed in real applications. It's pretty unlikely that an application would provide a rectangle which is intentionally very small for the provided text, and depend on how the text could be clipped in that rectangle. Quoting https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136717: I had to mark some of the tests that rely on strange GdipMeasureCharacterRanges() logic for cases when the string doesn't fit onto the provided bounds as todo_wine in last commit since I couldn't figure out how it's supposed to work, that could be investigated separately.
Do you have a reason to worry about these particular tests being marked as todo? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_137684
On Sun Apr 26 14:32:21 2026 +0000, Dmitry Timoshkov wrote:
The tests that I marked as todo_wine were added by me long time ago for completeness, they don't replicate something that I observed in real applications. It's pretty unlikely that an application would provide a rectangle which is intentionally very small for the provided text, and depend on how the text could be clipped in that rectangle. Quoting https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136717: I had to mark some of the tests that rely on strange GdipMeasureCharacterRanges() logic for cases when the string doesn't fit onto the provided bounds as todo_wine in last commit since I couldn't figure out how it's supposed to work, that could be investigated separately. Do you have a reason to worry about these particular tests being marked as todo? Generally I would like to reach full compatibility with native libraries. I know that sometimes it is not possible, but it is only my vision of Wine libraries.
If this test case has low priority, then it is ok for me to introduce `todo_wine`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_137694
This merge request was approved by Esme Povirk. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775
participants (4)
-
Bartosz Kosiorek (＠gang65) -
Dmitry Timoshkov
-
Dmitry Timoshkov (＠dmitry)
-
Esme Povirk (＠madewokherd)