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. 3. avoid not needed TransformMatrixPoints
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)
-- v10: test: try to fix failing test on 32 bit
From: Bartosz Kosiorek gang65@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)
From: Bartosz Kosiorek gang65@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 | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 11dc06e6c9e..c7a8dcf39f3 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; + ARGB *dst_color;
m11 = (ptf[1].X - ptf[0].X) / srcwidth; m21 = (ptf[2].X - ptf[0].X) / srcheight; @@ -3249,14 +3250,6 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image 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) - { - heap_free(src_data); - return OutOfMemory; - } - dst_stride = sizeof(ARGB) * (dst_area.right - dst_area.left);
GdipTransformMatrixPoints(&dst_to_src, dst_to_src_points, 3); @@ -3266,19 +3259,25 @@ 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++) + /* 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) + { + heap_free(src_data); + return OutOfMemory; + } + + dst_color = (ARGB*)(dst_data); + 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_color += sizeof(ARGB)) { GpPointF src_pointf; - ARGB *dst_color; - 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
From: Bartosz Kosiorek gang65@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 c7a8dcf39f3..d371d78ea8e 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -3237,6 +3237,7 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image 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; ARGB *dst_color; + 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 heap_free(src_data); return OutOfMemory; } - dst_color = (ARGB*)(dst_data); - 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_color += sizeof(ARGB)) + for (x = dst_area.left, src_pointf = src_pointf_row; x < dst_area.right; + x++, src_pointf.X += x_dx, src_pointf.Y += x_dy, dst_color += sizeof(ARGB)) { - GpPointF src_pointf; - 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,
From: Bartosz Kosiorek gang65@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 d371d78ea8e..9d033af7609 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -3234,14 +3234,13 @@ 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; ARGB *dst_color; 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; @@ -3252,13 +3251,10 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image if (stat != Ok) return stat;
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];
/* 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)); @@ -3271,9 +3267,9 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image
/* 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;
From: Bartosz Kosiorek gang65@poczta.onet.pl
The issue occurs when dst_color was pushed outside the loop ARGB *dst_color = (ARGB*)(dst_dyn_data); --- dlls/gdiplus/graphics.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 9d033af7609..5ee734dfe8f 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -3276,7 +3276,7 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image y++, src_pointf_row.X += y_dx, src_pointf_row.Y += y_dy) { for (x = dst_area.left, src_pointf = src_pointf_row; x < dst_area.right; - x++, src_pointf.X += x_dx, src_pointf.Y += x_dy, dst_color += sizeof(ARGB)) + x++, src_pointf.X += x_dx, src_pointf.Y += x_dy) { if (src_pointf.X >= srcx && src_pointf.X < srcx + srcwidth && src_pointf.Y >= srcy && src_pointf.Y < srcy + srcheight) @@ -3284,6 +3284,7 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image imageAttributes, interpolation, offset_mode); else *dst_color = 0; + dst_color++; } } }
-
Bartosz Kosiorek -
Bartosz Kosiorek (＠gang65)