[PATCH v5 0/1] MR10736: gdiplus: Don't add X margin to Center/Far for GdipMeasureCharacterRanges
We should shift X position only for StringAlignmentNear. -- v5: gdiplus: Don't add X margin on Center/Far for GdipMeasureCharacterRanges and GdipDrawString https://gitlab.winehq.org/wine/wine/-/merge_requests/10736
From: Bartosz Kosiorek <gang65@poczta.onet.pl> We should shift X position only for StringAlignmentNear. For Center and Far alignment, is is already correctly set by gdip_format_string. --- dlls/gdiplus/graphics.c | 31 +++++++++++++++++++++---------- dlls/gdiplus/tests/graphics.c | 3 --- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 0faac1325d2..e4d00e329f7 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -6030,16 +6030,22 @@ GpStatus WINGDIPAPI GdipMeasureCharacterRanges(GpGraphics* graphics, 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 + 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) + if (stringFormat->align == StringAlignmentNear) { - scaled_rect.Width -= margin_x * 2.0f * args.rel_width; - if (scaled_rect.Width < 0.5f) /* doesn't fit */ - scaled_rect.Width = 0.5f; + scaled_rect.X = (layoutRect->X + margin_x) * args.rel_width; + 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; + } } + else + scaled_rect.X = layoutRect->X * args.rel_width; if (scaled_rect.Width >= 1 << 23) scaled_rect.Width = 1 << 23; if (scaled_rect.Height >= 1 << 23) scaled_rect.Height = 1 << 23; @@ -6368,16 +6374,21 @@ GpStatus WINGDIPAPI GdipDrawString(GpGraphics *graphics, GDIPCONST WCHAR *string margin_x = (format && format->generic_typographic) ? 0.0 : font->emSize / 6.0; margin_x *= units_scale(font->unit, graphics->unit, graphics->xres, graphics->printer_display); - scaled_rect.X = margin_x * rel_width; scaled_rect.Y = 0.0; scaled_rect.Width = rel_width * rect->Width; scaled_rect.Height = rel_height * rect->Height; - if (scaled_rect.Width >= 0.5) + if (!format || format->align == StringAlignmentNear) { - scaled_rect.Width -= margin_x * 2.0 * rel_width; - if (scaled_rect.Width < 0.5) /* doesn't fit */ - goto end; + scaled_rect.X = margin_x * rel_width; + if (scaled_rect.Width >= 0.5f) + { + scaled_rect.Width -= margin_x * 2.0f * rel_width; + if (scaled_rect.Width < 0.5f) /* doesn't fit */ + goto end; + } } + else + scaled_rect.X = 0.0f; 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 8703081070f..5433f814a0f 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -4969,7 +4969,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); expectf_(5.0 - height_rgn/2.0, bounds.Y, 1.0); expectf_(width_rgn, bounds.Width, 1.0); @@ -5016,7 +5015,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, bounds.X, 2.0); expectf_(5.0 + height_rgn, bounds.Y, 1.0); expectf_(width_rgn, bounds.Width, 1.0); @@ -5031,7 +5029,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, bounds.X, 2.0); expectf_(5.0 - height_rgn, bounds.Y, 1.0); expectf_(width_rgn, bounds.Width, 1.0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10736
On Tue Apr 28 18:30:48 2026 +0000, Esme Povirk wrote:
I'd want to hear if @dmitry has an opinion on that, since he added them originally. I'm ambivalent about having these at all. Given that the effective margins are going to vary with details of the font and how hinting affects the glyph sizes, I'm not sure we should assume they're stable even on Windows. The margins were added for an application that generates recognizable printing forms with PDF417 barcodes, so it was pretty important to make the forms look similar to the results obtained under Windows. Besides, the margins is a documented gdiplus feature. So, removing margins doesn't sound like an improvement to me, and needs very convincing arguments.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_138039
On Tue Apr 28 19:00:04 2026 +0000, Dmitry Timoshkov wrote:
The margins were added for an application that generates recognizable printing forms with PDF417 barcodes, so it was pretty important to make the forms look similar to the results obtained under Windows. Besides, the margins is a documented gdiplus feature. So, removing margins doesn't sound like an improvement to me, and needs very convincing arguments. Sure. From my observation, it seems these functions are returning integer values (floating numbers are rounded).
I wondering if we should also implement such feature in Wine (rounding floating numbers). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_138042
On Tue Apr 28 19:04:28 2026 +0000, Bartosz Kosiorek wrote:
Sure. From my observation, it seems these functions are returning integer values (floating numbers are rounded). I wondering if we should also implement such feature in Wine (rounding floating numbers). My guess is that the margins are still added in the other alignment cases, just in other places.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_138058
On Tue Apr 28 20:08:27 2026 +0000, Esme Povirk wrote:
My guess is that the margins are still added in the other alignment cases, just in other places. Oh, I think my comment earlier was unclear. We do need margins for compatibility. I am ambivalent about testing them precisely.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_138060
On Tue Apr 28 20:09:58 2026 +0000, Esme Povirk wrote:
Oh, I think my comment earlier was unclear. We do need margins for compatibility. I am ambivalent about testing them precisely. I'd prefer to not interfere with accepting this MR, especially since it removes quite a bit of todo_wine statements.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_138061
On Tue Apr 28 20:15:20 2026 +0000, Dmitry Timoshkov wrote:
I'd prefer to not interfere with accepting this MR, especially since it removes quite a bit of todo_wine statements. After further testing this patch with my test application from https://gitlab.winehq.org/wine/wine/-/merge_requests/9775 I'd like to reject this MR since it breaks GdipMeasureCharacterRanges() in StringAlignmentFar case.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_138070
On Tue Apr 28 20:31:50 2026 +0000, Dmitry Timoshkov wrote:
After further testing this patch with my test application from https://gitlab.winehq.org/wine/wine/-/merge_requests/9775 I'd like to reject this MR since it breaks GdipMeasureCharacterRanges() in StringAlignmentFar case. Thanks @dmitry. Do you think, I should add test case for that into Wine for StringAlignmentFar case?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_138071
This merge request was closed by Bartosz Kosiorek. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736
On Tue Apr 28 20:52:11 2026 +0000, Bartosz Kosiorek wrote:
Thanks @dmitry. Do you think, I should add test case for that into Wine for StringAlignmentFar case? Adding more tests is always welcome.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_138128
participants (4)
-
Bartosz Kosiorek -
Bartosz Kosiorek (＠gang65)
-
Dmitry Timoshkov (＠dmitry)
-
Esme Povirk (＠madewokherd)