From: Bartosz Kosiorek gang65@poczta.onet.pl
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53947 --- dlls/gdiplus/graphics.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index fca66c8654b..eb208f63fd9 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -6887,13 +6887,15 @@ GpStatus get_graphics_transform(GpGraphics *graphics, GpCoordinateSpace dst_spac
if (dst_space != src_space) { - scale_x = units_to_pixels(1.0, graphics->unit, graphics->xres, graphics->printer_display); - scale_y = units_to_pixels(1.0, graphics->unit, graphics->yres, graphics->printer_display); - if(graphics->unit != UnitDisplay) { - scale_x *= graphics->scale; - scale_y *= graphics->scale; + scale_x = units_to_pixels(graphics->scale, graphics->unit, graphics->xres, graphics->printer_display); + scale_y = units_to_pixels(graphics->scale, graphics->unit, graphics->yres, graphics->printer_display); + } + else + { + scale_x = units_to_pixels(1.0, graphics->unit, graphics->xres, graphics->printer_display); + scale_y = units_to_pixels(1.0, graphics->unit, graphics->yres, graphics->printer_display); }
if (dst_space < src_space) @@ -6903,12 +6905,11 @@ GpStatus get_graphics_transform(GpGraphics *graphics, GpCoordinateSpace dst_spac { case WineCoordinateSpaceGdiDevice: { - GpMatrix gdixform; - gdixform = graphics->gdi_transform; + GpMatrix gdixform = graphics->gdi_transform; stat = GdipInvertMatrix(&gdixform); if (stat != Ok) break; - GdipMultiplyMatrix(matrix, &gdixform, MatrixOrderAppend); + memcpy(matrix->matrix, gdixform.matrix, 6 * sizeof(REAL)); if (dst_space == CoordinateSpaceDevice) break; /* else fall-through */ @@ -6934,7 +6935,7 @@ GpStatus get_graphics_transform(GpGraphics *graphics, GpCoordinateSpace dst_spac switch ((int)src_space) { case CoordinateSpaceWorld: - GdipMultiplyMatrix(matrix, &graphics->worldtrans, MatrixOrderAppend); + memcpy(matrix->matrix, &graphics->worldtrans, 6 * sizeof(REAL)); if (dst_space == CoordinateSpacePage) break; /* else fall-through */ @@ -6954,7 +6955,7 @@ GpStatus get_graphics_transform(GpGraphics *graphics, GpCoordinateSpace dst_spac return stat; }
-GpStatus gdip_transform_points(GpGraphics *graphics, GpCoordinateSpace dst_space, +GpStatus inline gdip_transform_points(GpGraphics *graphics, GpCoordinateSpace dst_space, GpCoordinateSpace src_space, GpPointF *points, INT count) { GpMatrix matrix;
Bartosz Kosiorek (@gang65) commented about dlls/gdiplus/graphics.c:
if (dst_space != src_space) {
scale_x = units_to_pixels(1.0, graphics->unit, graphics->xres, graphics->printer_display);scale_y = units_to_pixels(1.0, graphics->unit, graphics->yres, graphics->printer_display);if(graphics->unit != UnitDisplay) {scale_x *= graphics->scale;scale_y *= graphics->scale;
Don't waste time for multiply floating numbers if it is not needed. Instead invoke `units_to_pixels` functions with `1.0` or `graphics->scale` values
Bartosz Kosiorek (@gang65) commented about dlls/gdiplus/graphics.c:
{ case WineCoordinateSpaceGdiDevice: {
GpMatrix gdixform;gdixform = graphics->gdi_transform;
GpMatrix gdixform = graphics->gdi_transform; stat = GdipInvertMatrix(&gdixform); if (stat != Ok) break;
GdipMultiplyMatrix(matrix, &gdixform, MatrixOrderAppend);
memcpy(matrix->matrix, gdixform.matrix, 6 * sizeof(REAL));
In previous implementation we have multiplied [Identity Matrix](https://en.wikipedia.org/wiki/Identity_matrix) with GDIMatrix, and the result was always the same (GDIMatrix). Instead of expensive Matrix Multiplication, just copy matrix, and boost perfmance.
Bartosz Kosiorek (@gang65) commented about dlls/gdiplus/graphics.c:
switch ((int)src_space) { case CoordinateSpaceWorld:
GdipMultiplyMatrix(matrix, &graphics->worldtrans, MatrixOrderAppend);
memcpy(matrix->matrix, &graphics->worldtrans, 6 * sizeof(REAL));
Instead of expensive Matrix Multiplication by [Identity Matrix](https://en.wikipedia.org/wiki/Identity_matrix), just copy matrix, and boost performance.
On Wed May 17 17:43:45 2023 +0000, Bartosz Kosiorek wrote:
In previous implementation we have multiplied [Identity Matrix](https://en.wikipedia.org/wiki/Identity_matrix) with GDIMatrix, and the result was always the same (GDIMatrix). Instead of expensive Matrix Multiplication, just copy matrix, and boost perfmance.
I'd suggest `sizeof(matrix->matrix)` rather than hard-coding the size calculation.
Esme Povirk (@madewokherd) commented about dlls/gdiplus/graphics.c:
return stat;}
-GpStatus gdip_transform_points(GpGraphics *graphics, GpCoordinateSpace dst_space, +GpStatus inline gdip_transform_points(GpGraphics *graphics, GpCoordinateSpace dst_space,
I'm not sure if there are any implications to marking a non-static function as inline. I couldn't find another example of it in Wine code.
But, the function isn't used in any other file so it's probably fine to make it static inline.
In the future, I'd prefer that you don't use gitlab's comment feature to explain your code. Either I should be able to figure out what the code is doing and why by reading it, or the code needs a comment.
While I'm not too familiar with the GDI+ code in Wine, I don't see any glaring issues with these changes. I agree that `6 * sizeof(REAL)` should be replaced with `sizeof(matrix->matrix)`, but either way this is reviewed-by me.
-
Bartosz Kosiorek -
Bartosz Kosiorek (＠gang65)
-
Esme Povirk (＠madewokherd)
-
Torge Matthies (＠tmatthies)