[PATCH v7 0/4] MR2864: gdiplus: Improve performance of GdipDrawImagePointsRect
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53947 Improve performance of GdipDrawImagePointsRect by: 1. avoiding multiplication and use addition where it is possible. 2. avoid calculating `GdipInvertMatrix` if it is not used It gives noticible speed improvement. I divided MR to several commits, to better undestand what is going on. Application for testing (it needs logo.jpg file): [gdiplusdrawimagepoints.exe](/uploads/545ff7d8ab1d60386366f64999346825/gdiplusdrawimagepoints.exe) -- v7: gdiplus: Improve performance of DrawImagePointsRect by avoid TransformMatrixPoints https://gitlab.winehq.org/wine/wine/-/merge_requests/2864
From: Bartosz Kosiorek <gang65(a)poczta.onet.pl> Calculate transformation points only when is it needed (during resampling. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53947 --- dlls/gdiplus/graphics.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 6831cfcb247..11dc06e6c9e 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -3152,14 +3152,10 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image GpRectF graphics_bounds; GpRect src_area; int i, x, y, src_stride, dst_stride; - GpMatrix dst_to_src; - REAL m11, m12, m21, m22, mdx, mdy; LPBYTE src_data, dst_data, dst_dyn_data=NULL; BitmapData lockeddata; InterpolationMode interpolation = graphics->interpolation; PixelOffsetMode offset_mode = graphics->pixeloffset; - GpPointF dst_to_src_points[3] = {{0.0, 0.0}, {1.0, 0.0}, {0.0, 1.0}}; - REAL x_dx, x_dy, y_dx, y_dy; static const GpImageAttributes defaultImageAttributes = {WrapModeClamp, 0, FALSE}; if (!imageAttributes) @@ -3187,18 +3183,6 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image if (IsRectEmpty(&dst_area)) return Ok; - m11 = (ptf[1].X - ptf[0].X) / srcwidth; - m21 = (ptf[2].X - ptf[0].X) / srcheight; - mdx = ptf[0].X - m11 * srcx - m21 * srcy; - m12 = (ptf[1].Y - ptf[0].Y) / srcwidth; - m22 = (ptf[2].Y - ptf[0].Y) / srcheight; - mdy = ptf[0].Y - m12 * srcx - m22 * srcy; - - GdipSetMatrixElements(&dst_to_src, m11, m12, m21, m22, mdx, mdy); - - stat = GdipInvertMatrix(&dst_to_src); - if (stat != Ok) return stat; - if (do_resampling) { get_bitmap_sample_size(interpolation, imageAttributes->wrap, @@ -3248,6 +3232,23 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image if (do_resampling) { + GpMatrix dst_to_src; + REAL m11, m12, m21, m22, mdx, mdy; + GpPointF dst_to_src_points[3] = {{0.0, 0.0}, {1.0, 0.0}, {0.0, 1.0}}; + REAL x_dx, x_dy, y_dx, y_dy; + + m11 = (ptf[1].X - ptf[0].X) / srcwidth; + m21 = (ptf[2].X - ptf[0].X) / srcheight; + m12 = (ptf[1].Y - ptf[0].Y) / srcwidth; + m22 = (ptf[2].Y - ptf[0].Y) / srcheight; + mdx = ptf[0].X - m11 * srcx - m21 * srcy; + mdy = ptf[0].Y - m12 * srcx - m22 * srcy; + + GdipSetMatrixElements(&dst_to_src, m11, m12, m21, m22, mdx, mdy); + + stat = GdipInvertMatrix(&dst_to_src); + if (stat != Ok) return stat; + /* Transform the bits as needed to the destination. */ dst_data = dst_dyn_data = heap_alloc_zero(sizeof(ARGB) * (dst_area.right - dst_area.left) * (dst_area.bottom - dst_area.top)); if (!dst_data) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2864
From: Bartosz Kosiorek <gang65(a)poczta.onet.pl> Improves efficiency by using addition instead of multiplication for iterating through points data. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53947 --- dlls/gdiplus/graphics.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 11dc06e6c9e..9bd967ac67e 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -3266,19 +3266,18 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image y_dx = dst_to_src_points[2].X - dst_to_src_points[0].X; y_dy = dst_to_src_points[2].Y - dst_to_src_points[0].Y; - for (x=dst_area.left; x<dst_area.right; x++) + for (y = dst_area.top; y < dst_area.bottom; y++) { - for (y=dst_area.top; y<dst_area.bottom; y++) + for (x = dst_area.left; x < dst_area.right; x++, dst_dyn_data += sizeof(ARGB)) { GpPointF src_pointf; - ARGB *dst_color; + ARGB *dst_color = (ARGB*)(dst_dyn_data); src_pointf.X = dst_to_src_points[0].X + x * x_dx + y * y_dx; src_pointf.Y = dst_to_src_points[0].Y + x * x_dy + y * y_dy; - dst_color = (ARGB*)(dst_data + dst_stride * (y - dst_area.top) + sizeof(ARGB) * (x - dst_area.left)); - - if (src_pointf.X >= srcx && src_pointf.X < srcx + srcwidth && src_pointf.Y >= srcy && src_pointf.Y < srcy+srcheight) + if (src_pointf.X >= srcx && src_pointf.X < srcx + srcwidth && + src_pointf.Y >= srcy && src_pointf.Y < srcy + srcheight) *dst_color = resample_bitmap_pixel(&src_area, src_data, bitmap->width, bitmap->height, &src_pointf, imageAttributes, interpolation, offset_mode); else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2864
From: Bartosz Kosiorek <gang65(a)poczta.onet.pl> Improves efficiency by using addition instead of float numbers multiplication for destination points. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53947 --- dlls/gdiplus/graphics.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 9bd967ac67e..985a003f685 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -3236,6 +3236,7 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image REAL m11, m12, m21, m22, mdx, mdy; GpPointF dst_to_src_points[3] = {{0.0, 0.0}, {1.0, 0.0}, {0.0, 1.0}}; REAL x_dx, x_dy, y_dx, y_dy; + GpPointF src_pointf_row, src_pointf; m11 = (ptf[1].X - ptf[0].X) / srcwidth; m21 = (ptf[2].X - ptf[0].X) / srcheight; @@ -3266,16 +3267,21 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image y_dx = dst_to_src_points[2].X - dst_to_src_points[0].X; y_dy = dst_to_src_points[2].Y - dst_to_src_points[0].Y; - for (y = dst_area.top; y < dst_area.bottom; y++) + /* Calculate top left point of transformed image. + It would be used as reference point for adding */ + src_pointf_row.X = dst_to_src_points[0].X + + dst_area.left * x_dx + dst_area.top * y_dx; + src_pointf_row.Y = dst_to_src_points[0].Y + + dst_area.left * x_dy + dst_area.top * y_dy; + + for (y = dst_area.top; y < dst_area.bottom; + y++, src_pointf_row.X += y_dx, src_pointf_row.Y += y_dy) { - for (x = dst_area.left; x < dst_area.right; x++, dst_dyn_data += sizeof(ARGB)) + src_pointf = src_pointf_row; + for (x = dst_area.left; x < dst_area.right; + x++, src_pointf.X += x_dx, src_pointf.Y += x_dy, dst_dyn_data += sizeof(ARGB)) { - GpPointF src_pointf; ARGB *dst_color = (ARGB*)(dst_dyn_data); - - src_pointf.X = dst_to_src_points[0].X + x * x_dx + y * y_dx; - src_pointf.Y = dst_to_src_points[0].Y + x * x_dy + y * y_dy; - if (src_pointf.X >= srcx && src_pointf.X < srcx + srcwidth && src_pointf.Y >= srcy && src_pointf.Y < srcy + srcheight) *dst_color = resample_bitmap_pixel(&src_area, src_data, bitmap->width, bitmap->height, &src_pointf, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2864
From: Bartosz Kosiorek <gang65(a)poczta.onet.pl> Using TransformMatrixPoints is not needed and all values could be taken from transformation matrix: - ShearX from m11, m12 - ShearY from m21, m22 - Translation mdx, mdy The result could be calculated by taking destination points values: {{0.0, 0.0}, {1.0, 0.0}, {0.0, 1.0}} and calculating it with GdipTransformMatrixPoints function: dst_to_src_points[0].X = dst_to_src.matrix[4]; dst_to_src_points[0].Y = dst_to_src.matrix[5]; dst_to_src_points[1].X = dst_to_src.matrix[0] + dst_to_src.matrix[4]; dst_to_src_points[1].Y = dst_to_src.matrix[1] + dst_to_src.matrix[5]; dst_to_src_points[2].X = dst_to_src.matrix[2] + dst_to_src.matrix[4]; dst_to_src_points[2].Y = dst_to_src.matrix[3] + dst_to_src.matrix[5]; --- dlls/gdiplus/graphics.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 985a003f685..26faf4fbb4d 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -3234,13 +3234,12 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image { GpMatrix dst_to_src; REAL m11, m12, m21, m22, mdx, mdy; - GpPointF dst_to_src_points[3] = {{0.0, 0.0}, {1.0, 0.0}, {0.0, 1.0}}; REAL x_dx, x_dy, y_dx, y_dy; GpPointF src_pointf_row, src_pointf; m11 = (ptf[1].X - ptf[0].X) / srcwidth; - m21 = (ptf[2].X - ptf[0].X) / srcheight; m12 = (ptf[1].Y - ptf[0].Y) / srcwidth; + m21 = (ptf[2].X - ptf[0].X) / srcheight; m22 = (ptf[2].Y - ptf[0].Y) / srcheight; mdx = ptf[0].X - m11 * srcx - m21 * srcy; mdy = ptf[0].Y - m12 * srcx - m22 * srcy; @@ -3259,19 +3258,16 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image } dst_stride = sizeof(ARGB) * (dst_area.right - dst_area.left); - - GdipTransformMatrixPoints(&dst_to_src, dst_to_src_points, 3); - - x_dx = dst_to_src_points[1].X - dst_to_src_points[0].X; - x_dy = dst_to_src_points[1].Y - dst_to_src_points[0].Y; - y_dx = dst_to_src_points[2].X - dst_to_src_points[0].X; - y_dy = dst_to_src_points[2].Y - dst_to_src_points[0].Y; + x_dx = dst_to_src.matrix[0]; + x_dy = dst_to_src.matrix[1]; + y_dx = dst_to_src.matrix[2]; + y_dy = dst_to_src.matrix[3]; /* Calculate top left point of transformed image. It would be used as reference point for adding */ - src_pointf_row.X = dst_to_src_points[0].X + + src_pointf_row.X = dst_to_src.matrix[4] + dst_area.left * x_dx + dst_area.top * y_dx; - src_pointf_row.Y = dst_to_src_points[0].Y + + src_pointf_row.Y = dst_to_src.matrix[5] + dst_area.left * x_dy + dst_area.top * y_dy; for (y = dst_area.top; y < dst_area.bottom; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2864
On Tue May 23 03:06:46 2023 +0000, Bartosz Kosiorek wrote:
Previously `dst_dyn_data` was unused pointer defined here: ``` dst_data = dst_dyn_data = heap_alloc_zero(sizeof(ARGB) * (dst_area.right - dst_area.left) * (dst_area.bottom - dst_area.top)); ``` With current implementation, the pointer is used for iterate through memory. The increment is done now with `dst_dyn_data += sizeof(ARGB)` with `for` loop: ``` for (x = dst_area.left; x < dst_area.right; x++, dst_dyn_data += sizeof(ARGB)) ``` To be able to not use `dst_stride` I have to switch `x` and `y` `for` loops. I have tested this implementation with application attached in description. You shouldn't change `dst_dyn_data` because it gets freed later.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2864#note_33475
On Tue May 23 03:11:10 2023 +0000, Esme Povirk wrote:
You shouldn't change `dst_dyn_data` because it gets freed later. Switching the for loops is an improvement by itself, though. It's more efficient to write in memory order.
I'd probably pull the `dst_color` declaration out of the loop and use that as the cursor. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2864#note_33476
participants (3)
-
Bartosz Kosiorek -
Bartosz Kosiorek (＠gang65)
-
Esme Povirk (＠madewokherd)