[PATCH v4 0/1] MR2865: gdiplus: Handle Windows style newline.

[David Kahurani (＠kahurani)]

gdip_format_string doesn't write newline characters. It writes line by line making sure to skip the newline characters. This means we can resolve this issue(at least as far gdiplus is concerned) without concerning ourselves with the issue in GetTextExtentExPointW(which should be dealt with as a different bug).

This patch implements a fix but there might be possible improvements in the code so yeah, input is most welcome.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54962

Signed-off-by: David Kahurani

--
  v4: gdiplus: Handle Windows style newline.

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

[PATCH v4 1/1] gdiplus: Handle Windows style newline.

[David Kahurani]

From: David Kahurani

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54962
Signed-off-by: David Kahurani 
---
 dlls/gdiplus/graphics.c       | 33 +++++++++++++++++++++++++++------
 dlls/gdiplus/tests/graphics.c |  7 +++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
index 03542b31ea7..835c2889bd1 100644
--- a/dlls/gdiplus/graphics.c
+++ b/dlls/gdiplus/graphics.c
@@ -5186,7 +5186,7 @@ GpStatus gdip_format_string(HDC hdc,
     INT *hotkeyprefix_offsets=NULL;
     INT hotkeyprefix_count=0;
     INT hotkeyprefix_pos=0, hotkeyprefix_end_pos=0;
-    BOOL seen_prefix = FALSE;
+    BOOL seen_prefix = FALSE, unixstyle_newline = TRUE;

if(length == -1) length = lstrlenW(string);

@@ -5259,9 +5259,20 @@ GpStatus gdip_format_string(HDC hdc,
         if(fit == 0)
             break;

-        for(lret = 0; lret < fit; lret++)
+        for(lret = 0; lret < fit; lret++) {
             if(*(stringdup + sum + lret) == '\n')
-                break;
+            {
+               unixstyle_newline = TRUE;
+               break;
+            }
+
+            if(*(stringdup + sum + lret) == '\r' && lret + 1 < fit
+               && *(stringdup + sum + lret + 1) == '\n')
+            {
+               unixstyle_newline = FALSE;
+               break;
+            }
+        }

/* Line break code (may look strange, but it imitates windows). */
         if(lret < fit)
@@ -5332,9 +5343,19 @@ GpStatus gdip_format_string(HDC hdc,
         if (stat != Ok)
             break;

-        sum += fit + (lret < fitcpy ? 1 : 0);
-        height += size.cy;
-        lineno++;
+
+        if (unixstyle_newline)
+        {
+            height += size.cy;
+            lineno++;
+            sum += fit + (lret < fitcpy ? 1 : 0);
+        }
+        else
+        {
+            height += size.cy;
+            lineno++;
+            sum += fit + (lret < fitcpy ? 2 : 0);
+        }

hotkeyprefix_pos = hotkeyprefix_end_pos;

diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c
index 0ee2b73c413..0456a451970 100644
--- a/dlls/gdiplus/tests/graphics.c
+++ b/dlls/gdiplus/tests/graphics.c
@@ -3197,6 +3197,7 @@ static void test_string_functions(void)
     HDC hdc = GetDC( hwnd );
     const WCHAR teststring[] = L"MM M\nM";
     const WCHAR teststring2[] = L"j";
+    const WCHAR teststring3[] = L"MM M\r\nM\0";
     REAL char_width, char_height;
     INT codepointsfitted, linesfilled;
     GpStringFormat *format;
@@ -3268,6 +3269,12 @@ static void test_string_functions(void)
     status = GdipMeasureString(graphics, teststring, 6, font, &rc, NULL, &bounds, &codepointsfitted, NULL);
     expect(Ok, status);

+    /* new line handling */
+    status = GdipMeasureString(graphics, teststring3, -1, font, &rc, NULL, &bounds, &codepointsfitted, &linesfilled);
+    expect(Ok, status);
+    expect(7, codepointsfitted);
+    expect(2, linesfilled);
+
     status = GdipMeasureString(graphics, teststring, 1, font, &rc, NULL, &char_bounds, &codepointsfitted, &linesfilled);
     expect(Ok, status);
     expectf(0.0, char_bounds.X);

[Nikolay Sivov (＠nsivov)]

It's not a problem of this patch, but existing issue with gdip_format_string(). Should it really be that limited regarding line breaks, and wrapping (wrt to whitespace handling). Measurements interleaved by string character checks is not how this normally works - first it should identify where it can possibly break, taking in account mandatory breaks, and points where it shouldn't break, for example for combining marks. After that itemization, shaping, building lines. I'm mentioning all that because existing code is already hard to follow.

[Esme Povirk (＠madewokherd)]

The way it works can be changed if needed, but I'd need to see tests showing that it's needed.

[PATCH v4 0/1] MR2865: gdiplus: Handle Windows style newline. - approved

[Esme Povirk (＠madewokherd)]

This merge request was approved by Esme Povirk.