[PATCH v8 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) -- v8: gdiplus: Improve performance of DrawImagePointsRect by avoid TransformMatrixPoints gdiplus: Improve performance of GdipDrawImagePointsRect gdiplus: use iterator instead calculate pointer position every time gdiplus: limit calculating transformation points to resampling https://gitlab.winehq.org/wine/wine/-/merge_requests/2864
From: Bartosz Kosiorek <gang65(a)poczta.onet.pl> Calculating transformation points and not using it it is a waste of CPU time. With this patch the code was moved where it is used. It is improving performance of GdipDrawImagePointsRect when resampling is not needed. 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> With previous implementation, every iteration pointer value was calculated by taking row and column of the image. It needs multiply calculation. With current implementation, pointer value calculation, is replaced with iterator, which takes next pixel data. It 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 Mon May 22 17:43:51 2023 +0000, Esme Povirk wrote:
I agree with @jrueger, the commit messages need to say what the commits do. Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2864#note_33478
participants (2)
-
Bartosz Kosiorek -
Bartosz Kosiorek (＠gang65)