[PATCH v6 0/2] MR1618: gdiplus: improve performance of GdipScaleMatrix
-- v6: gdiplus: improve performance of matrix multiplication by unrolling loop. https://gitlab.winehq.org/wine/wine/-/merge_requests/1618
From: Bartosz Kosiorek <gang65(a)poczta.onet.pl> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53947 --- dlls/gdiplus/matrix.c | 25 +++++++++++++---------- dlls/gdiplus/tests/matrix.c | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/dlls/gdiplus/matrix.c b/dlls/gdiplus/matrix.c index 40abbc93e21..13e7aa05e11 100644 --- a/dlls/gdiplus/matrix.c +++ b/dlls/gdiplus/matrix.c @@ -287,24 +287,27 @@ GpStatus WINGDIPAPI GdipRotateMatrix(GpMatrix *matrix, REAL angle, GpStatus WINGDIPAPI GdipScaleMatrix(GpMatrix *matrix, REAL scaleX, REAL scaleY, GpMatrixOrder order) { - REAL scale[6]; - TRACE("(%p, %.2f, %.2f, %d)\n", matrix, scaleX, scaleY, order); if(!matrix) return InvalidParameter; - scale[0] = scaleX; - scale[1] = 0.0; - scale[2] = 0.0; - scale[3] = scaleY; - scale[4] = 0.0; - scale[5] = 0.0; - if(order == MatrixOrderAppend) - matrix_multiply(matrix->matrix, scale, matrix->matrix); + { + matrix->matrix[0] *= scaleX; + matrix->matrix[1] *= scaleY; + matrix->matrix[2] *= scaleX; + matrix->matrix[3] *= scaleY; + matrix->matrix[4] *= scaleX; + matrix->matrix[5] *= scaleY; + } else if (order == MatrixOrderPrepend) - matrix_multiply(scale, matrix->matrix, matrix->matrix); + { + matrix->matrix[0] *= scaleX; + matrix->matrix[1] *= scaleX; + matrix->matrix[2] *= scaleY; + matrix->matrix[3] *= scaleY; + } else return InvalidParameter; diff --git a/dlls/gdiplus/tests/matrix.c b/dlls/gdiplus/tests/matrix.c index 5ca0209f6a9..a2f6916fd66 100644 --- a/dlls/gdiplus/tests/matrix.c +++ b/dlls/gdiplus/tests/matrix.c @@ -110,6 +110,45 @@ static void test_transform(void) GdipDeleteMatrix(matrix); } +static void test_scale(void) +{ + GpStatus status; + GpMatrix *matrix = NULL; + REAL elems[6]; + + static const REAL expected_elem[] = {3.0, -4.0, 90.0, 80.0, -1500.0, 1200.0}; + static const REAL expected_elem2[] = {3.0, -6.0, 60.0, 80.0, -500.0, 600.0}; + + GdipCreateMatrix2(1.0, -2.0, 30.0, 40.0, -500.0, 600.0, &matrix); + + status = GdipScaleMatrix(NULL, 3, 2, MatrixOrderAppend); + expect(InvalidParameter, status); + status = GdipScaleMatrix(matrix, 3, 2, MatrixOrderAppend); + expect(Ok, status); + + status = GdipGetMatrixElements(matrix, elems); + expect(Ok, status); + GdipDeleteMatrix(matrix); + + for(INT i = 0; i < 6; i++) + ok(expected_elem[i] == elems[i], "Expected #%d to be (%.1f) but got (%.1f)\n", i, + expected_elem[i], elems[i]); + + GdipCreateMatrix2(1.0, -2.0, 30.0, 40.0, -500.0, 600.0, &matrix); + + status = GdipScaleMatrix(matrix, 3, 2, MatrixOrderPrepend); + expect(Ok, status); + + status = GdipGetMatrixElements(matrix, elems); + expect(Ok, status); + GdipDeleteMatrix(matrix); + + for(INT i = 0; i < 6; i++) + ok(expected_elem2[i] == elems[i], "Expected #%d to be (%.1f) but got (%.1f)\n", i, + expected_elem2[i], elems[i]); + +} + static void test_isinvertible(void) { GpStatus status; @@ -390,6 +429,7 @@ START_TEST(matrix) GdiplusStartup(&gdiplusToken, &gdiplusStartupInput, NULL); test_constructor_destructor(); + test_scale(); test_transform(); test_isinvertible(); test_invert(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1618
From: Bartosz Kosiorek <gang65(a)poczta.onet.pl> --- dlls/gdiplus/matrix.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/dlls/gdiplus/matrix.c b/dlls/gdiplus/matrix.c index 13e7aa05e11..d790daab2af 100644 --- a/dlls/gdiplus/matrix.c +++ b/dlls/gdiplus/matrix.c @@ -39,17 +39,15 @@ WINE_DEFAULT_DEBUG_CHANNEL(gdiplus); * * and puts the output in out. * */ -static void matrix_multiply(GDIPCONST REAL * left, GDIPCONST REAL * right, REAL * out) +static inline void matrix_multiply(GDIPCONST REAL * left, GDIPCONST REAL * right, REAL * out) { REAL temp[6]; - int i, odd; - - for(i = 0; i < 6; i++){ - odd = i % 2; - temp[i] = left[i - odd] * right[odd] + left[i - odd + 1] * right[odd + 2] + - (i >= 4 ? right[odd + 4] : 0.0); - } - + temp[0] = left[0] * right[0] + left[1] * right[2]; + temp[1] = left[0] * right[1] + left[1] * right[3]; + temp[2] = left[2] * right[0] + left[3] * right[2]; + temp[3] = left[2] * right[1] + left[3] * right[3]; + temp[4] = left[4] * right[0] + left[5] * right[2] + right[4]; + temp[5] = left[4] * right[1] + left[5] * right[3] + right[5]; memcpy(out, temp, 6 * sizeof(REAL)); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1618
On Fri Dec 2 16:13:04 2022 +0000, Bartosz Kosiorek wrote:
I will check that. I think inlining should be applied anyway. Thanks for suggesting. Still with enrolling I don't need to calculate the modulo every loop and array indexes are hardcoded (no need to calculate it). Unfortunately I am not expert with checking compiler output. Could you please give some advice regarding that? Generally the best results I have with inlining, loop enrolling and scale optimization.
Lowest values of running Times on Linux 64 bit (I run about 10 times and get lowest value): Wine gdiplus.dll **without** optimizations: * 500 Matrix Scaling time (seconds): 0.31s * 700 Matrix Multipling time (seconds): 0.20s Wine gdiplus.dll **without** optimizations with **inlining** `matrix_multiply` (Scaling is also using `matrix_multiply`): * 500 Matrix Scaling time (seconds): 0.21s * 700 Matrix Multipling time (seconds): 0.16s Wine gdiplus.dll **with** optimizations and **inlining** (current Merge Request): * 500 Matrix Scaling time (seconds): 0.07s * 700 Matrix Multipling time (seconds): 0.11s -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1618#note_18465
Alex Henrie (@alexhenrie) commented about dlls/gdiplus/matrix.c:
* * and puts the output in out. * */ -static void matrix_multiply(GDIPCONST REAL * left, GDIPCONST REAL * right, REAL * out) +static inline void matrix_multiply(GDIPCONST REAL * left, GDIPCONST REAL * right, REAL * out) You want `FORCEINLINE`, not `inline`. `inline` doesn't mean "inline" in C, it just means "Deduplicate multiple function definitions and don't warn if the function is unused." `FORCEINLINE` actually inlines the function.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1618#note_18466
participants (3)
-
Alex Henrie (＠alexhenrie) -
Bartosz Kosiorek
-
Bartosz Kosiorek (＠gang65)