[PATCH 0/1] MR10736: gdiplus: Don't add X margin on Center/Fat for GdipMeasureCharacterRanges
We should shift X position only for StringAlignmentNear. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736
From: Bartosz Kosiorek <gang65@poczta.onet.pl> We should shift X position only for StringAlignmentNear. --- dlls/gdiplus/graphics.c | 5 ++++- dlls/gdiplus/tests/graphics.c | 4 ---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 1d046e7d147..1d553e4b3e9 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -6016,7 +6016,10 @@ 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; + if (stringFormat->align == StringAlignmentNear) + scaled_rect.X = (layoutRect->X + margin_x) * args.rel_width; + else + scaled_rect.X = layoutRect->X * args.rel_width; scaled_rect.Y = layoutRect->Y * args.rel_height; scaled_rect.Width = layoutRect->Width * args.rel_width; scaled_rect.Height = layoutRect->Height * args.rel_height; diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 6ac85fe303c..c42a8087538 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -4948,7 +4948,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); @@ -4964,7 +4963,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); @@ -5012,7 +5010,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); todo_wine expectf_(5.0 + height_rgn, bounds.Y, 1.0); @@ -5028,7 +5025,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); todo_wine expectf_(5.0 - height_rgn, bounds.Y, 1.0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10736
According to my testing a correct fix would be something like https://gitlab.winehq.org/wine/wine/-/merge_requests/9775/diffs?commit_id=2b... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_137564
On Sat Apr 25 18:40:31 2026 +0000, Dmitry Timoshkov wrote:
According to my testing a correct fix would be something like https://gitlab.winehq.org/wine/wine/-/merge_requests/9775/diffs?commit_id=2b... My idea is to introduce fixes, based on test cases and logic behind that. With this patch, I am not introducing any regression in current test cases, besides fixing four `todo_wine`. I think it is step in correct direction, as with small step, we are closer to ideal solution.
It would be extremally useful if you could reproduce issue from https://gitlab.winehq.org/wine/wine/-/merge_requests/9775/diffs?commit_id=2b... in new test case. With such test case, I would be able to find proper solution which will work in all cases. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_137673
On Sat Apr 25 18:48:49 2026 +0000, Bartosz Kosiorek wrote:
My idea is to introduce fixes, based on test cases and logic behind that. With this patch, I didn't introduce any regression in current test cases, besides fixing four `todo_wine`. I think it is step in correct direction, as with small step, we are closer to ideal solution. It would be extremally useful if you could reproduce issue from https://gitlab.winehq.org/wine/wine/-/merge_requests/9775/diffs?commit_id=2b... in new test case. With such test case, I would be able to find proper solution which will work in all cases. A test is attached to https://gitlab.winehq.org/wine/wine/-/merge_requests/9775: https://gitlab.winehq.org/-/project/5/uploads/53c8bd3f9b7b005d5b0b675296f063...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_137674
On Sat Apr 25 20:46:24 2026 +0000, Dmitry Timoshkov wrote:
A test is attached to https://gitlab.winehq.org/wine/wine/-/merge_requests/9775: https://gitlab.winehq.org/-/project/5/uploads/53c8bd3f9b7b005d5b0b675296f063... If you notice that some use case is not covered by test cases, please create Merge Request with corresponding test case into: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/gdiplus/tests/graphic...
With such approach it will secure Wine from regression. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_137680
Bartosz Kosiorek (@gang65) commented about dlls/gdiplus/graphics.c:
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; + if (stringFormat->align == StringAlignmentNear) + scaled_rect.X = (layoutRect->X + margin_x) * args.rel_width; + else + scaled_rect.X = layoutRect->X * args.rel_width;
Still missing correct `line_align` support. It was implemented in https://gitlab.winehq.org/wine/wine/-/merge_requests/9775/diffs -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_137681
On Sun Apr 26 13:27:13 2026 +0000, Bartosz Kosiorek wrote:
If you notice that some use case is not covered by test cases, please create Merge Request with corresponding test case into: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/gdiplus/tests/graphic... With such approach it will secure Wine from regression. FWIW, I don't think trying to reproduce native's precise behavior, down to exact measurements and particularly adjustments in case of limited space, is a good idea for text rendering specifically.
We know from [an MS-published article](https://web.archive.org/web/20080119000747/http://support.microsoft.com/kb/3...) that native treats text as linearly scalable. It accounts for deviations in scaled glyph size by either adding/removing spacing (between words and even between individual characters), or rendering the whole thing in vectors. Both of these approaches would add a lot of complexity, with the end result being ugly text. To my knowledge, real applications do not depend on linear text scaling, but we cannot match native's precise measurements without providing it. Therefore, I think we should ideally have this in mind and not depend on it when writing tests. I'm also willing to accept test regressions that are made necessary by this approach. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_137786
With GdipMeasureCharacterRanges in particular, I think it's more important that the behavior match that of GdipDrawString than that it match measurements from native. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_137788
GdipDrawString and GdipMeasureCharacterRanges should probably be changed together, unless we're correcting a discrepancy between them. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10736#note_137793
participants (4)
-
Bartosz Kosiorek -
Bartosz Kosiorek (＠gang65)
-
Dmitry Timoshkov (＠dmitry)
-
Esme Povirk (＠madewokherd)