[PATCH v4 0/1] MR2865: gdiplus: Handle Windows style newline.
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 <k.kahurani(a)gmail.com> -- v4: gdiplus: Handle Windows style newline. https://gitlab.winehq.org/wine/wine/-/merge_requests/2865
From: David Kahurani <k.kahurani(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54962 Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2865
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2865#note_34032
The way it works can be changed if needed, but I'd need to see tests showing that it's needed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2865#note_34035
This merge request was approved by Esme Povirk. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2865
participants (4)
-
David Kahurani -
David Kahurani (@kahurani) -
Esme Povirk (@madewokherd) -
Nikolay Sivov (@nsivov)