[PATCH v6 0/5] MR5068: win32u: Fix rendering of the rotated and scaled text.
These patches make a test case attached to the bug https://bugs.winehq.org/show_bug.cgi?id=33190 work. -- v6: win32u: NtGdiExtTextOutW() should translate x,y from logical to device units at the last step. win32u: Fix device<->world width/height converters. win32u: Use slightly more readable names for DP/LP converters. win32u: Use correct helper for converting width to device units. gdi32/tests: Add some tests for rotated font metrics. https://gitlab.winehq.org/wine/wine/-/merge_requests/5068
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/gdi32/tests/font.c | 117 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c index 933e061ce31..72bd5eb9220 100644 --- a/dlls/gdi32/tests/font.c +++ b/dlls/gdi32/tests/font.c @@ -22,6 +22,7 @@ #include <stdarg.h> #include <stdio.h> #include <assert.h> +#include <math.h> #include "windef.h" #include "winbase.h" @@ -7892,6 +7893,121 @@ static void test_font_weight(void) ok(bret, "got error %ld\n", GetLastError()); } +static void test_rotated_metrics(void) +{ + static const WCHAR str[] = L"Hello World!"; + static const struct + { + float angle_g; + POINT pt; + LONG tmHeight, tmAscent, tmDescent; + SIZE ext; + int flip_x, flip_y; + } test[] = + { + { 0.0f, {50,50}, 87,72,15, {392,87}, 0, 0 }, + { 0.0f, {-50,50}, 87,72,15, {392,87}, 1, 0 }, + { 0.0f, {50,-50}, 87,72,15, {392,87}, 0, 1 }, + + { 45.0f, {94,24}, 107,76,31, {391,107}, 0, 0 }, + { 45.0f, {-283,212}, 107,76,31, {391,107}, 1, 0 }, + { 45.0f, {0,-71}, 107,76,31, {391,107}, 0, 1 }, + + { 90.0f, {100,-50}, 90,74,16, {391,90}, 0, 0 }, + { 90.0f, {100,-50}, 90,74,16, {391,90}, 1, 0 }, + { 90.0f, {100,-50}, 90,74,16, {391,90}, 0, 1 }, + + { 135.0f, {0,-71}, 108,76,32, {391,108}, 0, 0 }, + { 135.0f, {0,-71}, 108,76,32, {391,108}, 1, 0 }, + { 135.0f, {-283,212}, 108,76,32, {391,108}, 0, 1 }, + + { 180.0f, {-50,-50}, 87,72,15, {392,87}, 0, 0 }, + { 180.0f, {50,-50}, 87,72,15, {392,87}, 1, 0 }, + { 180.0f, {-50,50}, 87,72,15, {392,87}, 0, 1 } + }; + float angle_r; + HDC hdc; + LOGFONTW lf; + HFONT hfont; + XFORM xform; + TEXTMETRICW tm; + POINT pt; + SIZE sz; + int i; + + hdc = CreateCompatibleDC(0); + + memset(&lf, 0, sizeof(lf)); + wcscpy(lf.lfFaceName, L"Tahoma"); + lf.lfHeight = -72; + hfont = CreateFontIndirectW(&lf); + + SetGraphicsMode(hdc, GM_ADVANCED); + SetMapMode(hdc, MM_TEXT); + + hfont = SelectObject(hdc, hfont); + + pt.x = 100; + pt.y = 100; + DPtoLP(hdc, &pt, 1); + ok(pt.x == 100, "got %ld\n", pt.x); + ok(pt.y == 100, "got %ld\n", pt.y); + + GetTextMetricsW(hdc, &tm); + ok(match_off_by_n(tm.tmHeight, 87, 5), "got %ld\n", tm.tmHeight); + ok(match_off_by_n(tm.tmAscent, 72, 5), "got %ld\n", tm.tmAscent); + ok(match_off_by_n(tm.tmDescent, 15, 5), "got %ld\n", tm.tmDescent); + + GetTextExtentPoint32W(hdc, str, wcslen(str), &sz); + ok(match_off_by_n(sz.cx, 391, 5), "got %ld\n", sz.cx); + ok(match_off_by_n(sz.cy, 87, 5), "got %ld\n", sz.cy); + + for (i = 0; i < ARRAY_SIZE(test); i++) + { + winetest_push_context("%d", i); + + angle_r = test[i].angle_g * 3.141592f / 180.0f; + + xform.eM11 = cosf(angle_r) * 2.0f; + xform.eM12 = sinf(angle_r); + xform.eM21 = -xform.eM12 * 2.0f; + xform.eM22 = xform.eM11; + if (test[i].flip_x) xform.eM11 = -xform.eM11; + if (test[i].flip_y) xform.eM22 = -xform.eM22; + xform.eDx = 0.0f; + xform.eDy = 0.0f; + SetWorldTransform(hdc, &xform); + + pt.x = 100; + pt.y = 100; + DPtoLP(hdc, &pt, 1); + ok(match_off_by_n(pt.x, test[i].pt.x, 1), "got pt.x %ld\n", pt.x); + ok(match_off_by_n(pt.y, test[i].pt.y, 1), "got pt.y %ld\n", pt.y); + + GetTextMetricsW(hdc, &tm); + /* Windows produces noticeable diff for angles 45.0 and 135.0 */ + todo_wine_if(i == 3 || i == 4 || i == 5 || i == 6 || i == 7 || i == 8 || i == 9 || i == 10 || i == 11) + ok(match_off_by_n(tm.tmHeight, test[i].tmHeight, 25), "got %ld\n", tm.tmHeight); + todo_wine_if(i == 3 || i == 4 || i == 5 || i == 6 || i == 7 || i == 8 || i == 9 || i == 10 || i == 11) + ok(match_off_by_n(tm.tmAscent, test[i].tmAscent, 10), "got %ld\n", tm.tmAscent); + todo_wine_if(i == 3 || i == 9) + ok(match_off_by_n(tm.tmDescent, test[i].tmDescent, 17), "got %ld\n", tm.tmDescent); + + GetTextExtentPoint32W(hdc, str, wcslen(str), &sz); + todo_wine_if(i == 3 || i == 4 || i == 5 || i == 6 || i == 7 || i == 8 || i == 9 || i == 10 || i == 11) + ok(match_off_by_n(sz.cx, test[i].ext.cx, 10), "got %ld\n", sz.cx); + todo_wine_if(i == 3 || i == 4 || i == 5 || i == 6 || i == 7 || i == 8 || i == 9 || i == 10 || i == 11) + ok(match_off_by_n(sz.cy, test[i].ext.cy, 21), "got %ld\n", sz.cy); + + winetest_pop_context(); + } + + hfont = SelectObject(hdc, hfont); + DeleteObject(hfont); + + DeleteDC(hdc); +} + START_TEST(font) { static const char *test_names[] = @@ -7913,6 +8029,7 @@ START_TEST(font) return; } + test_rotated_metrics(); test_stock_fonts(); test_logfont(); test_bitmap_font(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5068
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/win32u/font.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/dlls/win32u/font.c b/dlls/win32u/font.c index 3d70710e059..1ace77fc310 100644 --- a/dlls/win32u/font.c +++ b/dlls/win32u/font.c @@ -262,6 +262,16 @@ static inline INT height_to_LP( DC *dc, INT height ) return GDI_ROUND( (double)height * fabs( dc->xformVport2World.eM22 )); } +static inline INT INTERNAL_XWSTODS(DC *dc, INT width) +{ + POINT pt[2]; + pt[0].x = pt[0].y = 0; + pt[1].x = width; + pt[1].y = 0; + lp_to_dp(dc, pt, 2); + return pt[1].x - pt[0].x; +} + static inline INT INTERNAL_YWSTODS(DC *dc, INT height) { POINT pt[2]; @@ -5741,7 +5751,7 @@ BOOL nulldrv_ExtTextOut( PHYSDEV dev, INT x, INT y, UINT flags, const RECT *rect */ static inline int get_line_width( DC *dc, int metric_size ) { - int width = abs( INTERNAL_YWSTODS( dc, metric_size )); + int width = abs( INTERNAL_XWSTODS( dc, metric_size )); if (width == 0) width = 1; if (metric_size < 0) width = -width; return width; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5068
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/win32u/font.c | 56 +++++++++++++--------------------------------- 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/dlls/win32u/font.c b/dlls/win32u/font.c index 1ace77fc310..e2c652c3afe 100644 --- a/dlls/win32u/font.c +++ b/dlls/win32u/font.c @@ -224,32 +224,6 @@ static inline int facename_compare( const WCHAR *str1, const WCHAR *str2, SIZE_T /* Device -> World size conversion */ -/* Performs a device to world transformation on the specified width (which - * is in integer format). - */ -static inline INT INTERNAL_XDSTOWS(DC *dc, INT width) -{ - double floatWidth; - - /* Perform operation with floating point */ - floatWidth = (double)width * dc->xformVport2World.eM11; - /* Round to integers */ - return GDI_ROUND(floatWidth); -} - -/* Performs a device to world transformation on the specified size (which - * is in integer format). - */ -static inline INT INTERNAL_YDSTOWS(DC *dc, INT height) -{ - double floatHeight; - - /* Perform operation with floating point */ - floatHeight = (double)height * dc->xformVport2World.eM22; - /* Round to integers */ - return GDI_ROUND(floatHeight); -} - /* scale width and height but don't mirror them */ static inline INT width_to_LP( DC *dc, INT width ) @@ -262,7 +236,7 @@ static inline INT height_to_LP( DC *dc, INT height ) return GDI_ROUND( (double)height * fabs( dc->xformVport2World.eM22 )); } -static inline INT INTERNAL_XWSTODS(DC *dc, INT width) +static inline INT width_to_DP(DC *dc, INT width) { POINT pt[2]; pt[0].x = pt[0].y = 0; @@ -272,7 +246,7 @@ static inline INT INTERNAL_XWSTODS(DC *dc, INT width) return pt[1].x - pt[0].x; } -static inline INT INTERNAL_YWSTODS(DC *dc, INT height) +static inline INT height_to_DP(DC *dc, INT height) { POINT pt[2]; pt[0].x = pt[0].y = 0; @@ -5258,7 +5232,7 @@ BOOL WINAPI NtGdiGetTextExtentExW( HDC hdc, const WCHAR *str, INT count, INT max { for (i = 0; i < count; i++) { - unsigned int dx = abs( INTERNAL_XDSTOWS( dc, pos[i] )) + + unsigned int dx = abs( width_to_LP( dc, pos[i] )) + (i + 1) * dc->attr->char_extra; if (nfit && dx > (unsigned int)max_ext) break; if (dxs) dxs[i] = dx; @@ -5266,8 +5240,8 @@ BOOL WINAPI NtGdiGetTextExtentExW( HDC hdc, const WCHAR *str, INT count, INT max if (nfit) *nfit = i; } - size->cx = abs( INTERNAL_XDSTOWS( dc, size->cx )) + count * dc->attr->char_extra; - size->cy = abs( INTERNAL_YDSTOWS( dc, size->cy )); + size->cx = abs( width_to_LP( dc, size->cx )) + count * dc->attr->char_extra; + size->cy = abs( height_to_LP( dc, size->cy )); } if (pos != buffer && pos != dxs) free( pos ); @@ -5371,16 +5345,16 @@ UINT WINAPI NtGdiGetOutlineTextMetricsInternalW( HDC hdc, UINT cbData, output->otmTextMetrics.tmOverhang = width_to_LP( dc, output->otmTextMetrics.tmOverhang ); output->otmAscent = height_to_LP( dc, output->otmAscent); output->otmDescent = height_to_LP( dc, output->otmDescent); - output->otmLineGap = INTERNAL_YDSTOWS(dc, output->otmLineGap); - output->otmsCapEmHeight = INTERNAL_YDSTOWS(dc, output->otmsCapEmHeight); - output->otmsXHeight = INTERNAL_YDSTOWS(dc, output->otmsXHeight); + output->otmLineGap = height_to_LP(dc, output->otmLineGap); + output->otmsCapEmHeight = height_to_LP(dc, output->otmsCapEmHeight); + output->otmsXHeight = height_to_LP(dc, output->otmsXHeight); output->otmrcFontBox.top = height_to_LP( dc, output->otmrcFontBox.top); output->otmrcFontBox.bottom = height_to_LP( dc, output->otmrcFontBox.bottom); output->otmrcFontBox.left = width_to_LP( dc, output->otmrcFontBox.left); output->otmrcFontBox.right = width_to_LP( dc, output->otmrcFontBox.right); output->otmMacAscent = height_to_LP( dc, output->otmMacAscent); output->otmMacDescent = height_to_LP( dc, output->otmMacDescent); - output->otmMacLineGap = INTERNAL_YDSTOWS(dc, output->otmMacLineGap); + output->otmMacLineGap = height_to_LP(dc, output->otmMacLineGap); output->otmptSubscriptSize.x = width_to_LP( dc, output->otmptSubscriptSize.x); output->otmptSubscriptSize.y = height_to_LP( dc, output->otmptSubscriptSize.y); output->otmptSubscriptOffset.x = width_to_LP( dc, output->otmptSubscriptOffset.x); @@ -5389,7 +5363,7 @@ UINT WINAPI NtGdiGetOutlineTextMetricsInternalW( HDC hdc, UINT cbData, output->otmptSuperscriptSize.y = height_to_LP( dc, output->otmptSuperscriptSize.y); output->otmptSuperscriptOffset.x = width_to_LP( dc, output->otmptSuperscriptOffset.x); output->otmptSuperscriptOffset.y = height_to_LP( dc, output->otmptSuperscriptOffset.y); - output->otmsStrikeoutSize = INTERNAL_YDSTOWS(dc, output->otmsStrikeoutSize); + output->otmsStrikeoutSize = height_to_LP(dc, output->otmsStrikeoutSize); output->otmsStrikeoutPosition = height_to_LP( dc, output->otmsStrikeoutPosition); output->otmsUnderscoreSize = height_to_LP( dc, output->otmsUnderscoreSize); output->otmsUnderscorePosition = height_to_LP( dc, output->otmsUnderscorePosition); @@ -5751,7 +5725,7 @@ BOOL nulldrv_ExtTextOut( PHYSDEV dev, INT x, INT y, UINT flags, const RECT *rect */ static inline int get_line_width( DC *dc, int metric_size ) { - int width = abs( INTERNAL_XWSTODS( dc, metric_size )); + int width = abs( width_to_DP( dc, metric_size )); if (width == 0) width = 1; if (metric_size < 0) width = -width; return width; @@ -5982,8 +5956,8 @@ BOOL WINAPI NtGdiExtTextOutW( HDC hdc, INT x, INT y, UINT flags, const RECT *lpr width = desired[1]; } - tm.tmAscent = abs(INTERNAL_YWSTODS(dc, tm.tmAscent)); - tm.tmDescent = abs(INTERNAL_YWSTODS(dc, tm.tmDescent)); + tm.tmAscent = abs(height_to_DP(dc, tm.tmAscent)); + tm.tmDescent = abs(height_to_DP(dc, tm.tmDescent)); switch( align & (TA_LEFT | TA_RIGHT | TA_CENTER) ) { case TA_LEFT: @@ -6080,10 +6054,10 @@ done: { otm = malloc( size ); NtGdiGetOutlineTextMetricsInternalW( hdc, size, otm, 0 ); - underlinePos = abs( INTERNAL_YWSTODS( dc, otm->otmsUnderscorePosition )); + underlinePos = abs( height_to_DP( dc, otm->otmsUnderscorePosition )); if (otm->otmsUnderscorePosition < 0) underlinePos = -underlinePos; underlineWidth = get_line_width( dc, otm->otmsUnderscoreSize ); - strikeoutPos = abs( INTERNAL_YWSTODS( dc, otm->otmsStrikeoutPosition )); + strikeoutPos = abs( height_to_DP( dc, otm->otmsStrikeoutPosition )); if (otm->otmsStrikeoutPosition < 0) strikeoutPos = -strikeoutPos; strikeoutWidth = get_line_width( dc, otm->otmsStrikeoutSize ); free( otm ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5068
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/gdi32/tests/font.c | 5 ----- dlls/win32u/font.c | 38 ++++++++++++++++++++------------------ 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c index 72bd5eb9220..518448a2928 100644 --- a/dlls/gdi32/tests/font.c +++ b/dlls/gdi32/tests/font.c @@ -7986,17 +7986,12 @@ static void test_rotated_metrics(void) GetTextMetricsW(hdc, &tm); /* Windows produces noticeable diff for angles 45.0 and 135.0 */ - todo_wine_if(i == 3 || i == 4 || i == 5 || i == 6 || i == 7 || i == 8 || i == 9 || i == 10 || i == 11) ok(match_off_by_n(tm.tmHeight, test[i].tmHeight, 25), "got %ld\n", tm.tmHeight); - todo_wine_if(i == 3 || i == 4 || i == 5 || i == 6 || i == 7 || i == 8 || i == 9 || i == 10 || i == 11) ok(match_off_by_n(tm.tmAscent, test[i].tmAscent, 10), "got %ld\n", tm.tmAscent); - todo_wine_if(i == 3 || i == 9) ok(match_off_by_n(tm.tmDescent, test[i].tmDescent, 17), "got %ld\n", tm.tmDescent); GetTextExtentPoint32W(hdc, str, wcslen(str), &sz); - todo_wine_if(i == 3 || i == 4 || i == 5 || i == 6 || i == 7 || i == 8 || i == 9 || i == 10 || i == 11) ok(match_off_by_n(sz.cx, test[i].ext.cx, 10), "got %ld\n", sz.cx); - todo_wine_if(i == 3 || i == 4 || i == 5 || i == 6 || i == 7 || i == 8 || i == 9 || i == 10 || i == 11) ok(match_off_by_n(sz.cy, test[i].ext.cy, 21), "got %ld\n", sz.cy); winetest_pop_context(); diff --git a/dlls/win32u/font.c b/dlls/win32u/font.c index e2c652c3afe..cdbf7064b46 100644 --- a/dlls/win32u/font.c +++ b/dlls/win32u/font.c @@ -228,32 +228,34 @@ static inline int facename_compare( const WCHAR *str1, const WCHAR *str2, SIZE_T static inline INT width_to_LP( DC *dc, INT width ) { - return GDI_ROUND( (double)width * fabs( dc->xformVport2World.eM11 )); + float scale_x; + + scale_x = hypotf( dc->xformWorld2Vport.eM11, dc->xformWorld2Vport.eM12 ); + return GDI_ROUND( (float)width / scale_x ); } static inline INT height_to_LP( DC *dc, INT height ) { - return GDI_ROUND( (double)height * fabs( dc->xformVport2World.eM22 )); + float scale_y; + + scale_y = hypotf( dc->xformWorld2Vport.eM21, dc->xformWorld2Vport.eM22 ); + return GDI_ROUND( (float)height / scale_y ); } static inline INT width_to_DP(DC *dc, INT width) { - POINT pt[2]; - pt[0].x = pt[0].y = 0; - pt[1].x = width; - pt[1].y = 0; - lp_to_dp(dc, pt, 2); - return pt[1].x - pt[0].x; + float scale_x; + + scale_x = hypotf( dc->xformWorld2Vport.eM11, dc->xformWorld2Vport.eM12 ); + return GDI_ROUND( (float)width * scale_x ); } static inline INT height_to_DP(DC *dc, INT height) { - POINT pt[2]; - pt[0].x = pt[0].y = 0; - pt[1].x = 0; - pt[1].y = height; - lp_to_dp(dc, pt, 2); - return pt[1].y - pt[0].y; + float scale_y; + + scale_y = hypotf( dc->xformWorld2Vport.eM21, dc->xformWorld2Vport.eM22 ); + return GDI_ROUND( (float)height * scale_y ); } static INT FONT_GetObjectW( HGDIOBJ handle, INT count, LPVOID buffer ); @@ -4087,8 +4089,8 @@ static void scale_outline_font_metrics( const struct gdi_font *font, OUTLINETEXT else scale_x = font->scale_y; - scale_x *= fabs(font->matrix.eM11); - scale_y = font->scale_y * fabs(font->matrix.eM22); + scale_x *= hypotf(font->matrix.eM11, font->matrix.eM12); + scale_y = font->scale_y * hypotf(font->matrix.eM21, font->matrix.eM22); /* Windows scales these values as signed integers even if they are unsigned */ #define SCALE_X(x) (x) = GDI_ROUND((int)(x) * (scale_x)) @@ -4302,8 +4304,8 @@ static void scale_font_metrics( struct gdi_font *font, TEXTMETRICW *tm ) else scale_x = font->scale_y; - scale_x *= fabs(font->matrix.eM11); - scale_y = font->scale_y * fabs(font->matrix.eM22); + scale_x *= hypotf(font->matrix.eM11, font->matrix.eM12); + scale_y = font->scale_y * hypotf(font->matrix.eM21, font->matrix.eM22); #define SCALE_X(x) (x) = GDI_ROUND((x) * scale_x) #define SCALE_Y(y) (y) = GDI_ROUND((y) * scale_y) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5068
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Bug: https://bugs.winehq.org/show_bug.cgi?id=33190 Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/win32u/font.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/dlls/win32u/font.c b/dlls/win32u/font.c index cdbf7064b46..226c6a6ce5a 100644 --- a/dlls/win32u/font.c +++ b/dlls/win32u/font.c @@ -5856,12 +5856,6 @@ BOOL WINAPI NtGdiExtTextOutW( HDC hdc, INT x, INT y, UINT flags, const RECT *lpr goto done; } - pt.x = x; - pt.y = y; - lp_to_dp(dc, &pt, 1); - x = pt.x; - y = pt.y; - char_extra = dc->attr->char_extra; if (char_extra && lpDx && NtGdiGetDeviceCaps( hdc, TECHNOLOGY ) == DT_RASPRINTER) char_extra = 0; /* Printer drivers don't add char_extra if lpDx is supplied */ @@ -5958,8 +5952,6 @@ BOOL WINAPI NtGdiExtTextOutW( HDC hdc, INT x, INT y, UINT flags, const RECT *lpr width = desired[1]; } - tm.tmAscent = abs(height_to_DP(dc, tm.tmAscent)); - tm.tmDescent = abs(height_to_DP(dc, tm.tmDescent)); switch( align & (TA_LEFT | TA_RIGHT | TA_CENTER) ) { case TA_LEFT: @@ -6021,12 +6013,19 @@ BOOL WINAPI NtGdiExtTextOutW( HDC hdc, INT x, INT y, UINT flags, const RECT *lpr text_box.bottom = y + tm.tmDescent; if (flags & ETO_CLIPPED) intersect_rect( &text_box, &text_box, &rc ); + lp_to_dp(dc, (POINT *)&text_box, 2); if (!IsRectEmpty( &text_box )) physdev->funcs->pExtTextOut( physdev, 0, 0, ETO_OPAQUE, &text_box, NULL, 0, NULL ); } } } + pt.x = x; + pt.y = y; + lp_to_dp(dc, &pt, 1); + x = pt.x; + y = pt.y; + ret = physdev->funcs->pExtTextOut( physdev, x, y, (flags & ~ETO_OPAQUE), &rc, str, count, (INT*)deltas ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5068
On Mon Apr 28 12:50:03 2025 +0000, Huw Davies wrote:
Do your tests in commit 3 show that the changes in commit 2 are necessary? What I'd suggest is to swap the order of commits 2 and 3, associated with commit 3 would be the removal of some `todo_wine`s. I went ahead and made the tests very first commit, also I restored original logic in the tests where eM21 = -eM12 \* 2.0f, that shows more failures in the current code. Is this what you had in mind?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5068#note_102119
On Tue Apr 29 07:50:01 2025 +0000, Dmitry Timoshkov wrote:
I went ahead and made the tests very first commit, also I restored original logic in the tests where eM21 = -eM12 \* 2.0f, that shows more failures in the current code. Is this what you had in mind? I was hoping to see some tests fixed by the change in what's now commit three.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5068#note_102122
On Tue Apr 29 08:14:48 2025 +0000, Huw Davies wrote:
I was hoping to see some tests fixed by the change in what's now commit three. Aren't missing failures indicate that the propsed changes are correct? Otherwise what kind of test failres did you expect to see? I'm a little bit lost...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5068#note_102124
commit 08f171e764e241aa54d88534af2c409226c547ee is making a functional change so I'd expect to see an existing test (like those added in the first commit) change behaviour. That happens after the fourth commit, but not after the third. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5068#note_102126
On Tue Apr 29 08:30:38 2025 +0000, Huw Davies wrote:
commit 08f171e764e241aa54d88534af2c409226c547ee is making a functional change so I'd expect to see an existing test (like those added in the first commit) change behaviour. That happens after the fourth commit, but not after the third. I see. What kind of test would you suggest to add to see that the change is correct? I can't come up with a particulal test besides the already added mirroring transform that didn't reveal new test failures.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5068#note_102127
On Tue Apr 29 08:20:44 2025 +0000, Dmitry Timoshkov wrote:
Aren't missing failures indicate that the propsed changes are correct? Otherwise what kind of test failres did you expect to see? I'm a little bit lost... Missing failures could also mean that the tests are not correctly checking the relevant functionality. Adding the test with `todo_wine` first and then removing the `todo_wine` in the following commits would clearly show that there is an issue and that it's fixed by the proposed changes.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5068#note_102201
On Wed Apr 30 03:32:17 2025 +0000, Anton Baskanov wrote:
Missing failures could also mean that the tests are not correctly checking the relevant functionality. Adding the test with `todo_wine` first and then removing the `todo_wine` in the following commits would clearly show that there is an issue and that it's fixed by the proposed changes. Thank you for the insight. The question is what kind of the test could be added besides the reflection transform.
As I understand it although INTERNAL_XDSTOWS() may return negative value abs() makes this invisible, and changing INTERNAL_XDSTOWS() to width_to_LP() makes abs() redundant. Probably abs() should be removed with this change? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5068#note_102220
I spent a bit of time looking at the tests in the first commit. The error term passed to some of the calls to `match_off_by_n()` is very large, which means they're hiding the fact that the proposed fixes aren't correct. We should be able to get this to match to within a pixel or two. I don't have a suggestion on what the correct fix would be, but it's clear that this isn't right as it stands. It might be easier (at least for development) to start with simple rotations and once those are understood, add back the shear. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5068#note_108208
On Fri Jun 27 15:32:51 2025 +0000, Dmitry Timoshkov wrote:
Thanks for looking at it. As explained in the comment in the tests Windows produces noticeable diff for angles 45.0 and 135.0, so very large margin is to make tests pass under Windows. Should I remove the tests for angles 45 and 135, or do something else? And we should take into account that font metrics for Wine and Windows versions of Tahoma couldn't match precisely, so a match within 1-2 pixels is not really possible.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5068#note_108253
On Fri Jun 27 15:32:51 2025 +0000, Huw Davies wrote:
I spent a bit of time looking at the tests in the first commit. The error term passed to some of the calls to `match_off_by_n()` is very large, which means they're hiding the fact that the proposed fixes aren't correct. We should be able to get this to match to within a pixel or two. I don't have a suggestion on what the correct fix would be, but it's clear that this isn't right as it stands. It might be easier (at least for development) to start with simple rotations and once those are understood, add back the shear. Thanks for looking at it. As explained in the comment in the tests Windows produces noticeable diff for angles 45.0 and 135.0, so very large margin is to make tests pass under Windows. Should I remove the tests for angles 45 and 135, or do something else?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5068#note_108251
participants (4)
-
Anton Baskanov (＠baskanov) -
Dmitry Timoshkov
-
Dmitry Timoshkov (＠dmitry)
-
Huw Davies (＠huw)