[PATCH 0/1] MR5360: gdiplus: Check for empty height in bonding box calculation

[James McDonnell (＠ElementalWarrior)]

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.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5360

[PATCH 1/1] gdiplus: Check for empty height in bonding box calculation

[James McDonnell]

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
index 0727b63ed4f..0253e23c6b6 100644
--- a/dlls/gdiplus/graphics.c
+++ b/dlls/gdiplus/graphics.c
@@ -5448,7 +5448,7 @@ GpStatus gdip_format_string(GpGraphics *graphics, HDC hdc,
 
         bounds.Width = size.cx;
 
-        if(height + size.cy > nheight)
+        if(height != 0 && height + size.cy > nheight)
         {
             if (format->attr & StringFormatFlagsLineLimit)
                 break;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/5360

[PATCH 1/1] gdiplus: Check for empty height in bonding box calculation

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=144225

Your paranoid android.


=== debian11 (32 bit report) ===

gdiplus:
graphics.c:4790: Test succeeded inside todo block: Expected 24.000000, got 24.000000

=== debian11b (64 bit WoW report) ===

gdiplus:
graphics.c:4790: Test succeeded inside todo block: Expected 24.000000, got 24.000000

[Esme Povirk (＠madewokherd)]

This needs tests. It should be possible to test this behavior with GdipMeasureString or GdipMeasureCharacterRanges.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5360#note_65925

[Esme Povirk (＠madewokherd)]

Hm, this does fix a todo_wine, but it also breaks a Mono test: https://gitlab.winehq.org/wine-mono/mono/-/blob/main/mcs/class/System.Drawin...

I think that shows that the StringFormatFlagsLineLimit (which is set by GenericTypographic format) needs to always apply, not just for the first line.

I did some manual testing with [this test program](/uploads/ba8e84627b1ef41c23a120fb585d6fe9/measuretext.c), and I wasn't able to identify any difference between the first line and subsequent lines, with or without the StringFormatFlagsLineLimit flag. So I'm not convinced this is the right approach.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5360#note_65926

[James McDonnell (＠ElementalWarrior)]

Thanks for taking a look. And for the  test program. I'll get it compiled and play around with it. 

StudioTax is a C# application, although I couldn't get it to run under mono.

Their implementation of this appears to be a winforms control comprised of multiple labels. And they're using the width from the bounding box as the offset for the next label.

I'll try to better understand `StringFormatFlagsLineLimit` and text rendering in general. And see if I can identify a better appoach.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5360#note_65932