[PATCH v2 0/1] MR5360: gdiplus: Check for empty height in bonding box calculation
This resolves the issue in StudioTax where the bounding box dimensions is reported as 0. Because the height is less than 0, the check for height in GdipAddPathRectangle fails, and the X and Y coordinates of the points is never set. I am fairly ignorant of this code, this seems like a good approach, however I am happy for guidance from others more familiar in how gdiplus works. -- v2: gdiplus: Check for empty height in bonding box calculation https://gitlab.winehq.org/wine/wine/-/merge_requests/5360
From: James McDonnell <topgamer7(a)gmail.com> This resolves the issue in StudioTax where the bounding box dimensions is reported as 0. Because the height is less than 0, the check for height in GdipAddPathRectangle fails, and the X and Y coordinates of the points is never set. --- dlls/gdiplus/graphics.c | 6 ++++-- dlls/gdiplus/tests/graphics.c | 2 -- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 0727b63ed4f..9de0c302c3b 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -5448,11 +5448,11 @@ GpStatus gdip_format_string(GpGraphics *graphics, HDC hdc, bounds.Width = size.cx; - if(height + size.cy > nheight) + if(height + size.cy > nheight && (format->attr & StringFormatFlagsNoClip) == 0) { if (format->attr & StringFormatFlagsLineLimit) break; - bounds.Height = nheight - (height + size.cy); + bounds.Height = nheight - height; } else bounds.Height = size.cy; @@ -5516,6 +5516,8 @@ GpStatus gdip_format_string(GpGraphics *graphics, HDC hdc, free(stringdup); free(hotkeyprefix_offsets); + TRACE("%s bounds %s \n", debugstr_wn(string, length), debugstr_rectf(&bounds)); + return stat; } diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 4af7f179ec2..651c3b32e51 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -4667,7 +4667,6 @@ static void test_measure_string(void) expectf(0.0, bounds.X); expectf(0.0, bounds.Y); expectf(width, bounds.Width); - todo_wine expectf(height / 2.0, bounds.Height); range.First = 0; @@ -4786,7 +4785,6 @@ static void test_measure_string(void) expectf(0.0, bounds.X); expectf(0.0, bounds.Y); expectf_(width, bounds.Width, 0.01); - todo_wine expectf(height, bounds.Height); set_rect_empty(&rect); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5360
Alright, try number 2. It now appears to fix 2 todos. Which I've included in the same commit as the change. Wasn't sure if I should split it out. Wiki only talks about fixing a lot of conformance tests. Could you run the tests against mono again? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5360#note_66347
Hm, this makes sense to me but should probably be in a separate commit. The commit message also needs updated, as there's no longer a check for empty height. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5360#note_66495
I'll check the Mono tests, but in case you want to run them yourself, you can download them from https://dl.winehq.org/wine/wine-mono/9.0.0/wine-mono-9.0.0-tests.zip and run `wine run-tests.exe System.Drawing`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5360#note_66496
On Fri Mar 29 18:24:07 2024 +0000, Esme Povirk wrote:
Hm, this makes sense to me but should probably be in a separate commit. The commit message also needs updated, as there's no longer a check for empty height. The NoClip bit vs the bounds.Height bit? Or the tests vs the change in general. I didn't mean to include the trace statement. So I'll undo that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5360#note_66497
On Fri Mar 29 18:24:07 2024 +0000, James McDonnell wrote:
The NoClip bit vs the bounds.Height bit? Or the tests vs the change in general. I didn't mean to include the trace statement. So I'll undo that. The NoClip and bounds.Height bits should be in separate commits. The todo removals should be in the commit that fixes them.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5360#note_66498
participants (3)
-
Esme Povirk (＠madewokherd) -
James McDonnell
-
James McDonnell (＠ElementalWarrior)