[PATCH v4 0/4] 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) -- v4: gdiplus: Copy logic for trimming bounds width from GdipMeasureString() to GdipMeasureCharacterRanges(). gdiplus: Move vertical alignment calculation from GdipDrawString() to common helper. gdiplus: Take into account line alignment in GdipMeasureString(). gdiplus: gdip_format_string() always passes valid gdip_format_string_info->format to callbacks. 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 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 2419eea94db..a9aa56bf063 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -5960,16 +5960,17 @@ static GpStatus measure_string_callback(struct gdip_format_string_info *info) if (args->linesfilled) (*args->linesfilled)++; - switch (info->format ? info->format->align : StringAlignmentNear) + switch (info->format->align) { + case StringAlignmentNear: + default: + break; case StringAlignmentCenter: bounds->X = bounds->X + (info->rect->Width/2) - (bounds->Width/2); break; case StringAlignmentFar: bounds->X = bounds->X + info->rect->Width - bounds->Width; break; - default: - break; } return Ok; -- 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 | 17 +++++++++++++++-- dlls/gdiplus/tests/graphics.c | 4 ---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index a9aa56bf063..f282ab447a4 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -5966,10 +5966,23 @@ static GpStatus measure_string_callback(struct gdip_format_string_info *info) default: break; case StringAlignmentCenter: - bounds->X = bounds->X + (info->rect->Width/2) - (bounds->Width/2); + bounds->X += (info->rect->Width - bounds->Width) / 2; break; case StringAlignmentFar: - bounds->X = bounds->X + info->rect->Width - bounds->Width; + bounds->X += info->rect->Width - bounds->Width; + break; + } + + switch (info->format->line_align) + { + case StringAlignmentNear: + default: + break; + case StringAlignmentCenter: + bounds->Y += (info->rect->Height - bounds->Height) / 2; + break; + case StringAlignmentFar: + bounds->Y += info->rect->Height - bounds->Height; break; } diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 96ef52ce935..e1c99e98080 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -4921,7 +4921,6 @@ static void test_measure_string(void) expect(3, glyphs); expect(1, lines); expectf_(5.0 + width/2.0, bounds.X, 0.01); - todo_wine expectf(5.0 + height/2.0, bounds.Y); expectf_(width, bounds.Width, 0.01); expectf(height, bounds.Height); @@ -4937,7 +4936,6 @@ static void test_measure_string(void) expect(1, lines); todo_wine expectf_(5.0 - width/2.0, bounds.X, 0.01); - todo_wine expectf(5.0 - height/2.0, bounds.Y); expectf_(width, bounds.Width, 0.01); expectf(height, bounds.Height); @@ -4988,7 +4986,6 @@ static void test_measure_string(void) expect(3, glyphs); expect(1, lines); expectf_(5.0 + width, bounds.X, 0.01); - todo_wine expectf(5.0 + height, bounds.Y); expectf_(width, bounds.Width, 0.01); expectf(height, bounds.Height); @@ -5004,7 +5001,6 @@ static void test_measure_string(void) expect(1, lines); todo_wine expectf_(5.0 - width, bounds.X, 0.01); - todo_wine expectf(5.0 - height, bounds.Y); expectf_(width, bounds.Width, 0.01); expectf(height, bounds.Height); -- 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 | 47 +++++++++++++++-------------------- dlls/gdiplus/tests/graphics.c | 6 +---- 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index f282ab447a4..d04af8c17e8 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -5576,7 +5576,6 @@ GpStatus gdip_format_string(GpGraphics *graphics, HDC hdc, int sum = 0, height = 0, fit, fitcpy, i, j, lret, nwidth, nheight, lineend, lineno = 0; RectF bounds; - StringAlignment halign; GpStatus stat = Ok; SIZE size; HotkeyPrefix hkprefix; @@ -5657,8 +5656,6 @@ GpStatus gdip_format_string(GpGraphics *graphics, HDC hdc, length = j; - halign = format->align; - generate_font_link_info(&info, length, font); while(sum < length){ @@ -5723,19 +5720,33 @@ GpStatus gdip_format_string(GpGraphics *graphics, HDC hdc, else bounds.Height = size.cy; + bounds.X = rect->X; + + switch (format->align) + { + case StringAlignmentNear: + default: + break; + case StringAlignmentCenter: + bounds.X += (rect->Width - bounds.Width) / 2; + break; + case StringAlignmentFar: + bounds.X += rect->Width - bounds.Width; + break; + } + bounds.Y = rect->Y + height; - switch (halign) + switch (format->line_align) { case StringAlignmentNear: default: - bounds.X = rect->X; break; case StringAlignmentCenter: - bounds.X = rect->X + (rect->Width/2) - (bounds.Width/2); + bounds.Y += (rect->Height - bounds.Height) / 2; break; case StringAlignmentFar: - bounds.X = rect->X + rect->Width - bounds.Width; + bounds.Y += rect->Height - bounds.Height; break; } @@ -5946,7 +5957,7 @@ static GpStatus measure_string_callback(struct gdip_format_string_info *info) REAL new_width, new_height; new_width = info->bounds->Width / args->rel_width; - new_height = (info->bounds->Height + info->bounds->Y) / args->rel_height - bounds->Y; + new_height = info->bounds->Height / args->rel_height; if (new_width > bounds->Width) bounds->Width = new_width; @@ -6158,7 +6169,6 @@ GpStatus WINGDIPAPI GdipDrawString(GpGraphics *graphics, GDIPCONST WCHAR *string POINT corners[4]; REAL rel_width, rel_height, margin_x; INT save_state, format_flags = 0; - REAL offsety = 0.0; struct draw_string_args args; RectF scaled_rect; HDC hdc, temp_hdc=NULL; @@ -6186,24 +6196,7 @@ GpStatus WINGDIPAPI GdipDrawString(GpGraphics *graphics, GDIPCONST WCHAR *string if(format){ TRACE("may be ignoring some format flags: attr %x\n", format->attr); - format_flags = format->attr; - - /* Should be no need to explicitly test for StringAlignmentNear as - * that is default behavior if no alignment is passed. */ - if(format->line_align != StringAlignmentNear){ - RectF bounds, in_rect = *rect; - in_rect.Height = 0.0; /* avoid height clipping */ - GdipMeasureString(graphics, string, length, font, &in_rect, format, &bounds, 0, 0); - - TRACE("bounds %s\n", debugstr_rectf(&bounds)); - - if(format->line_align == StringAlignmentCenter) - offsety = (rect->Height - bounds.Height) / 2; - else if(format->line_align == StringAlignmentFar) - offsety = (rect->Height - bounds.Height); - } - TRACE("line align %d, offsety %f\n", format->line_align, offsety); } save_state = SaveDC(hdc); @@ -6248,7 +6241,7 @@ GpStatus WINGDIPAPI GdipDrawString(GpGraphics *graphics, GDIPCONST WCHAR *string args.brush = brush; args.x = rect->X; - args.y = rect->Y + offsety; + args.y = rect->Y; args.rel_width = rel_width; args.rel_height = rel_height; diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index e1c99e98080..ebeb8c03a37 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -3387,7 +3387,7 @@ static void test_string_functions(void) expectf(0.0, bounds.Y); ok(bounds.Width > char_bounds.Width + char_width * 2, "got %0.2f, expected at least %0.2f\n", bounds.Width, char_bounds.Width + char_width * 2); - ok(bounds.Height > char_bounds.Height, "got %0.2f, expected at least %0.2f\n", bounds.Height, char_bounds.Height); + ok(bounds.Height >= char_bounds.Height, "got %0.2f, expected at least %0.2f\n", bounds.Height, char_bounds.Height); expect(6, codepointsfitted); expect(2, linesfilled); char_height = bounds.Height - char_bounds.Height; @@ -4951,7 +4951,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); @@ -4967,7 +4966,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); @@ -5016,7 +5014,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); @@ -5032,7 +5029,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
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 d04af8c17e8..9cfde41f4aa 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -5895,7 +5895,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); @@ -5903,6 +5902,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.5) + { + scaled_rect.Width -= margin_x * 2.0 * args.rel_width; + if (scaled_rect.Width < 0.5) /* doesn't fit */ + scaled_rect.Width = 0.5; + } 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 ebeb8c03a37..5b17010c4b4 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); @@ -4949,7 +4956,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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9775
On Fri Apr 17 11:37:58 2026 +0000, Esme Povirk wrote:
The character range results look very wrong with my own test program: [measuretext.c](/uploads/66d1d1ff10f3728fc6f0e83c4bf587a4/measuretext.c) Character ranges are always ending up on the first line in the case of wrapped text, and apparently always start at X=0. It's also strange to me that any of the callbacks have to care about Y alignment, and unclear to me why we should redesign them to handle X alignment. Does new version of the patches resemble something you had in mind? 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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136717
Esme Povirk (@madewokherd) commented about dlls/gdiplus/graphics.c:
+ bounds->X += info->rect->Width - bounds->Width; + break; + } + + switch (info->format->line_align) + { + case StringAlignmentNear: + default: + break; + case StringAlignmentCenter: + bounds->Y += (info->rect->Height - bounds->Height) / 2; + break; + case StringAlignmentFar: + bounds->Y += info->rect->Height - bounds->Height; break; } I don't think it's possible to handle this correctly in the callback. We need to know the full height before we know how to offset the results.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136812
Esme Povirk (@madewokherd) commented about dlls/gdiplus/graphics.c:
- switch (halign) + switch (format->line_align) { case StringAlignmentNear: default: - bounds.X = rect->X; break; case StringAlignmentCenter: - bounds.X = rect->X + (rect->Width/2) - (bounds.Width/2); + bounds.Y += (rect->Height - bounds.Height) / 2; break; case StringAlignmentFar: - bounds.X = rect->X + rect->Width - bounds.Width; + bounds.Y += rect->Height - bounds.Height; break; } I don't think it's possible to adjust Y properly within this loop. We need to know the total height of all lines, after wrapping and clipping, first.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136815
On Fri Apr 17 11:37:58 2026 +0000, Dmitry Timoshkov wrote:
Does new version of the patches resemble something you had in mind? 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. My understanding is that native gdiplus does its calculations in the font's native size, and linearly scales them. Since rendered text (at least the way we do it) doesn't actually linearly scale, there is a possibility that they will have to render text in too small a space. The padding exists to make this less likely. If the text size does not match the space it's supposed to render in, native will squeeze characters together or spread them apart.
This results in complex code that renders ugly text, so I don't think we should do that. I'm guessing that GdipMeasureCharacterRanges reflects the actual rendering, while GdipMeasureString and GdipDrawString do not. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136816
On Fri Apr 17 18:47:13 2026 +0000, Esme Povirk wrote:
My understanding is that native gdiplus does its calculations in the font's native size, and linearly scales them. Since rendered text (at least the way we do it) doesn't actually linearly scale, there is a possibility that they will have to render text in too small a space. The padding exists to make this less likely. If the text size does not match the space it's supposed to render in, native will squeeze characters together or spread them apart. This results in complex code that renders ugly text, so I don't think we should do that. I'm guessing that GdipMeasureCharacterRanges reflects the actual rendering, while GdipMeasureString and GdipDrawString do not. This page goes into detail about how GDI+ adjusts text to scale linearly: https://web.archive.org/web/20080119000747/http://support.microsoft.com/kb/3...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136825
On Fri Apr 17 19:06:54 2026 +0000, Esme Povirk wrote:
This page goes into detail about how GDI+ adjusts text to scale linearly: https://web.archive.org/web/20080119000747/http://support.microsoft.com/kb/3... Do I understand it correctly that you are suggesting a full rewrite of existing code? My current approach was to add incremental changes on top existing logic, and that worked pretty well and allowed make my test (and the application it tries to follow) work and keep working your sample application. Is it possible to accept this MR since it a visible improvement, and start working on a rewrite later?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136827
On Fri Apr 17 19:26:27 2026 +0000, Dmitry Timoshkov wrote:
Do I understand it correctly that you are suggesting a full rewrite of existing code? My current approach was to add incremental changes on top existing logic, and that worked pretty well and allowed make my test (and the application it tries to follow) work and keep working your sample application. Is it possible to accept this MR since it a visible improvement, and start working on a rewrite later? In my previous two comments, I was just trying to explain native's behavior. I'm not sure what we should do about it. I don't think we should try to scale text the way native does.
I'm OK with declaring that some tests aren't going to be fixed in Wine because there are details we don't want to copy. It sounds like we may have to do this with some MeasureCharacterRanges tests. I'll approve the code if it makes sense to me and looks good in my test case. Right now, I don't really understand why horizontal alignment is being moved into the callback, but I can be convinced. I don't think vertical alignment can be moved to the callback at all without breaking multi-line text. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136842
On Fri Apr 17 20:05:49 2026 +0000, Esme Povirk wrote:
In my previous two comments, I was just trying to explain native's behavior. I'm not sure what we should do about it. I don't think we should try to scale text the way native does. I'm OK with declaring that some tests aren't going to be fixed in Wine because there are details we don't want to copy. It sounds like we may have to do this with some MeasureCharacterRanges tests. I'll approve the code if it makes sense to me and looks good in my test case. Right now, I don't really understand why horizontal alignment is being moved into the callback, but I can be convinced. I don't think vertical alignment can be moved to the callback at all without breaking multi-line text. Thanks. I suppose that I'll wait for your further test results and final judgement.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136862
On Fri Apr 17 22:56:35 2026 +0000, Dmitry Timoshkov wrote:
Thanks. I suppose that I'll wait for your further test results and final judgement. I'm waiting for the other two threads to be resolved.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_136881
On Fri Apr 17 18:37:46 2026 +0000, Esme Povirk wrote:
I don't think it's possible to adjust Y properly within this loop. We need to know the total height of all lines, after wrapping and clipping, first. What you would suggest? Duplicate the logic for calculating y-offset from GdipDrawString() to GdipMeasureString()/GdipMeasureCharacterRanges()?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_137073
On Fri Apr 17 18:31:48 2026 +0000, Esme Povirk wrote:
I don't think it's possible to handle this correctly in the callback. We need to know the full height before we know how to offset the results. I guess it's the same question about duplicating logic from GdipDrawString().
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_137074
On Mon Apr 20 08:08:57 2026 +0000, Dmitry Timoshkov wrote:
What you would suggest? Duplicate the logic for calculating y-offset from GdipDrawString() to GdipMeasureString()/GdipMeasureCharacterRanges()? Move that logic into gdip_format_string. Either that does require a design change, where the lines are processed all at once before being sent to the callback, or a recursive call using StringAlignmentNear when vertical alignment is something else.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_137111
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9775#note_137113
participants (3)
-
Dmitry Timoshkov -
Dmitry Timoshkov (＠dmitry)
-
Esme Povirk (＠madewokherd)