[PATCH v3 0/2] 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.

--
  v3: gdiplus: Fix text height calculation when exceeding rect
      gdiplus: Check for NoClip in text height calculation

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

[PATCH v3 1/2] gdiplus: Check for NoClip in text height calculation

[James McDonnell]

From: James McDonnell

---
 dlls/gdiplus/graphics.c       | 2 +-
 dlls/gdiplus/tests/graphics.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
index 0727b63ed4f..aaf04d60961 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 + size.cy > nheight && (format->attr & StringFormatFlagsNoClip) == 0)
         {
             if (format->attr & StringFormatFlagsLineLimit)
                 break;
diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c
index 4af7f179ec2..74dff64a944 100644
--- a/dlls/gdiplus/tests/graphics.c
+++ b/dlls/gdiplus/tests/graphics.c
@@ -4786,7 +4786,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);

[PATCH v3 2/2] gdiplus: Fix text height calculation when exceeding rect

[James McDonnell]

From: James McDonnell

This resulted in cases where the calculated height could be negative.

Fixing this resolves the text rendering issues in StudioTax.
The check for height in GdipAddPathRectangle would fail,
and the X and Y coordinates of the bounds would not be set.
---
 dlls/gdiplus/graphics.c       | 2 +-
 dlls/gdiplus/tests/graphics.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
index aaf04d60961..e7f49a7e646 100644
--- a/dlls/gdiplus/graphics.c
+++ b/dlls/gdiplus/graphics.c
@@ -5452,7 +5452,7 @@ GpStatus gdip_format_string(GpGraphics *graphics, HDC hdc,
         {
             if (format->attr & StringFormatFlagsLineLimit)
                 break;
-            bounds.Height = nheight - (height + size.cy);
+            bounds.Height = nheight - height;
         }
         else
             bounds.Height = size.cy;
diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c
index 74dff64a944..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;

[Esme Povirk (＠madewokherd)]

On Fri Mar 29 20:30:26 2024 +0000, Esme Povirk wrote:
> 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.

The Mono test is still broken.

The flags on GenericTypographic format are `StringFormatFlagsNoFitBlackBox|StringFormatFlagsLineLimit|StringFormatFlagsNoClip`. Maybe the LineLimit flag takes precedence when they're set together?

[James McDonnell (＠ElementalWarrior)]

On Fri Mar 29 20:30:26 2024 +0000, Esme Povirk wrote:
> The Mono test is still broken.
> The flags on GenericTypographic format are
> `StringFormatFlagsNoFitBlackBox|StringFormatFlagsLineLimit|StringFormatFlagsNoClip`.
> Maybe the LineLimit flag takes precedence when they're set together?

```
< 2931 tests passed, 374 tests failed
---
> 3101 tests passed, 402 tests failed

```
I presume I need to get this so that no new tests are failing, correct?

I'll read the documentation for all of those flags. Hunt down the test you refer to and experiment a bit.