[PATCH 0/1] MR3568: gdiplus: implement PixelOffsetModeHighQuality for GdipDrawImagePointsRect

[Bartosz Kosiorek (＠gang65)]

After investigation, it looks like the PixelOffsetModeHalf/PixelOffsetModeHighQuality is using floating point numbers, while PixelOffsetModeNone/PixelOffsetModeHighSpeed is using integers to calculate bitmap colours.

After using floating numbers, most tests are passing now.

[PATCH 1/1] gdiplus: implement PixelOffsetModeHighQuality for GdipDrawImagePointsRect

[Bartosz Kosiorek]

From: Bartosz Kosiorek

---
 dlls/gdiplus/graphics.c    | 92 +++++++++++++++++++++++++++-----------
 dlls/gdiplus/tests/image.c | 69 ++++++++++++++--------------
 2 files changed, 103 insertions(+), 58 deletions(-)

diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
index e3b5661fd67..efeaf94c8aa 100644
--- a/dlls/gdiplus/graphics.c
+++ b/dlls/gdiplus/graphics.c
@@ -3042,7 +3042,6 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image
      DrawImageAbort callback, VOID * callbackData)
 {
     GpPointF ptf[4];
-    POINT pti[4];
     GpStatus stat;

TRACE("(%p, %p, %p, %d, %f, %f, %f, %f, %d, %p, %p, %p)\n", graphics, image, points,
@@ -3094,11 +3093,6 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image
     ptf[3].Y = ptf[2].Y + ptf[1].Y - ptf[0].Y;
     if (!srcwidth || !srcheight || (ptf[3].X == ptf[0].X && ptf[3].Y == ptf[0].Y))
         return Ok;
-    gdip_transform_points(graphics, WineCoordinateSpaceGdiDevice, CoordinateSpaceWorld, ptf, 4);
-    round_points(pti, ptf, 4);
-
-    TRACE("%s %s %s %s\n", wine_dbgstr_point(&pti[0]), wine_dbgstr_point(&pti[1]),
-        wine_dbgstr_point(&pti[2]), wine_dbgstr_point(&pti[3]));

srcx = units_to_pixels(srcx, srcUnit, image->xres, graphics->printer_display);
     srcy = units_to_pixels(srcy, srcUnit, image->yres, graphics->printer_display);
@@ -3111,6 +3105,7 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image
         GpBitmap* bitmap = (GpBitmap*)image;
         BOOL do_resampling = FALSE;
         BOOL use_software = FALSE;
+        POINT pti[4];

TRACE("graphics: %.2fx%.2f dpi, fmt %#x, scale %f, image: %.2fx%.2f dpi, fmt %#x, color %08lx\n",
             graphics->xres, graphics->yres,
@@ -3118,6 +3113,12 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image
             graphics->scale, image->xres, image->yres, bitmap->format,
             imageAttributes ? imageAttributes->outside_color : 0);

+        gdip_transform_points(graphics, WineCoordinateSpaceGdiDevice, CoordinateSpaceWorld, ptf, 4);
+
+
+        TRACE("%s %s %s %s\n", wine_dbgstr_point(&pti[0]), wine_dbgstr_point(&pti[1]),
+            wine_dbgstr_point(&pti[2]), wine_dbgstr_point(&pti[3]));
+
         if (ptf[1].Y != ptf[0].Y || ptf[2].X != ptf[0].X ||
             ptf[1].X - ptf[0].X != srcwidth || ptf[2].Y - ptf[0].Y != srcheight ||
             srcx < 0 || srcy < 0 ||
@@ -3131,6 +3132,7 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image
         if (use_software)
         {
             RECT dst_area;
+            REAL dst_area_left, dst_area_right, dst_area_top, dst_area_bottom;
             GpRectF graphics_bounds;
             GpRect src_area;
             int i, x, y, src_stride, dst_stride;
@@ -3143,23 +3145,62 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image
             if (!imageAttributes)
                 imageAttributes = &defaultImageAttributes;

-            dst_area.left = dst_area.right = pti[0].x;
-            dst_area.top = dst_area.bottom = pti[0].y;
-            for (i=1; i<4; i++)
+            stat = get_graphics_device_bounds(graphics, &graphics_bounds);
+            if (stat != Ok) return stat;
+
+            switch (offset_mode)
+            {
+            default:
+            case PixelOffsetModeNone:
+            case PixelOffsetModeHighSpeed:
             {
-                if (dst_area.left > pti[i].x) dst_area.left = pti[i].x;
-                if (dst_area.right < pti[i].x) dst_area.right = pti[i].x;
-                if (dst_area.top > pti[i].y) dst_area.top = pti[i].y;
-                if (dst_area.bottom < pti[i].y) dst_area.bottom = pti[i].y;
+                round_points(pti, ptf, 4);
+                dst_area.left = dst_area.right = pti[0].x;
+                dst_area.top = dst_area.bottom = pti[0].y;
+                for (i=1; i<4; i++)
+                {
+                    if (dst_area.left > pti[i].x) dst_area.left = pti[i].x;
+                    if (dst_area.right < pti[i].x) dst_area.right = pti[i].x;
+                    if (dst_area.top > pti[i].y) dst_area.top = pti[i].y;
+                    if (dst_area.bottom < pti[i].y) dst_area.bottom = pti[i].y;
+                }
+
+                if (graphics_bounds.X > dst_area.left) dst_area.left = floorf(graphics_bounds.X);
+                if (graphics_bounds.Y > dst_area.top) dst_area.top = floorf(graphics_bounds.Y);
+                if (graphics_bounds.X + graphics_bounds.Width < dst_area.right) dst_area.right = ceilf(graphics_bounds.X + graphics_bounds.Width);
+                if (graphics_bounds.Y + graphics_bounds.Height < dst_area.bottom) dst_area.bottom = ceilf(graphics_bounds.Y + graphics_bounds.Height);
+
+                dst_area_left = dst_area.left;
+                dst_area_top = dst_area.top;
+                dst_area_right = dst_area.right;
+                dst_area_bottom = dst_area.bottom;
+                break;
             }
+            case PixelOffsetModeHalf:
+            case PixelOffsetModeHighQuality:
+            {
+                dst_area_left = dst_area_right = ptf[0].X;
+                dst_area_top = dst_area_bottom = ptf[0].Y;
+                for (i=1; i<4; i++)
+                {
+                    if (dst_area_left > ptf[i].X) dst_area_left = ptf[i].X;
+                    if (dst_area_right < ptf[i].X) dst_area_right = ptf[i].X;
+                    if (dst_area_top > ptf[i].Y) dst_area_top = ptf[i].Y;
+                    if (dst_area_bottom < ptf[i].Y) dst_area_bottom = ptf[i].Y;
+                }

-            stat = get_graphics_device_bounds(graphics, &graphics_bounds);
-            if (stat != Ok) return stat;
+                if (graphics_bounds.X > dst_area_left) dst_area_left = graphics_bounds.X;
+                if (graphics_bounds.Y > dst_area_top) dst_area_top = graphics_bounds.Y;
+                if (graphics_bounds.X + graphics_bounds.Width < dst_area_right) dst_area_right = graphics_bounds.X + graphics_bounds.Width;
+                if (graphics_bounds.Y + graphics_bounds.Height < dst_area_bottom) dst_area_bottom = graphics_bounds.Y + graphics_bounds.Height;

-            if (graphics_bounds.X > dst_area.left) dst_area.left = floorf(graphics_bounds.X);
-            if (graphics_bounds.Y > dst_area.top) dst_area.top = floorf(graphics_bounds.Y);
-            if (graphics_bounds.X + graphics_bounds.Width < dst_area.right) dst_area.right = ceilf(graphics_bounds.X + graphics_bounds.Width);
-            if (graphics_bounds.Y + graphics_bounds.Height < dst_area.bottom) dst_area.bottom = ceilf(graphics_bounds.Y + graphics_bounds.Height);
+                dst_area.left = dst_area_left;
+                dst_area.top = dst_area_top;
+                dst_area.right = dst_area_right;
+                dst_area.bottom = dst_area_bottom;
+                break;
+            }
+            }

TRACE("dst_area: %s\n", wine_dbgstr_rect(&dst_area));

@@ -3173,10 +3214,10 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image
             else
             {
                 /* Make sure src_area is equal in size to dst_area. */
-                src_area.X = srcx + dst_area.left - pti[0].x;
-                src_area.Y = srcy + dst_area.top - pti[0].y;
-                src_area.Width = dst_area.right - dst_area.left;
-                src_area.Height = dst_area.bottom - dst_area.top;
+                src_area.X = srcx + dst_area_left - ptf[0].X;
+                src_area.Y = srcy + dst_area_top - ptf[0].Y;
+                src_area.Width = dst_area_right - dst_area_left;
+                src_area.Height = dst_area_bottom - dst_area_top;
             }

TRACE("src_area: %d x %d\n", src_area.Width, src_area.Height);
@@ -3250,9 +3291,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.matrix[4] +
-                                   dst_area.left * x_dx + dst_area.top * y_dx;
+                                   dst_area_left * x_dx + dst_area_top * y_dx;
                 src_pointf_row.Y = dst_to_src.matrix[5] +
-                                   dst_area.left * x_dy + dst_area.top * y_dy;
+                                   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)
@@ -3367,6 +3408,7 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image

gdi_transform_acquire(graphics);

+            round_points(pti, ptf, 4);
             if (bitmap->format & (PixelFormatAlpha|PixelFormatPAlpha))
             {
                 gdi_alpha_blend(graphics, pti[0].x, pti[0].y, pti[1].x - pti[0].x, pti[2].y - pti[0].y,
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c
index 0baf6eae77b..e0630343be6 100644
--- a/dlls/gdiplus/tests/image.c
+++ b/dlls/gdiplus/tests/image.c
@@ -4702,30 +4702,30 @@ static void test_image_format(void)

static void test_DrawImage_scale(void)
 {
-    static const BYTE back_8x1[24] = { 0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,
-                                       0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40 };
-    static const BYTE image_080[24] = { 0x40,0x40,0x40,0x80,0x80,0x80,0x40,0x40,0x40,0x40,0x40,0x40,
-                                        0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40 };
-    static const BYTE image_100[24] = { 0x40,0x40,0x40,0x80,0x80,0x80,0x80,0x80,0x80,0x40,0x40,0x40,
-                                        0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40 };
-    static const BYTE image_120[24] = { 0x40,0x40,0x40,0x40,0x40,0x40,0x80,0x80,0x80,0x40,0x40,0x40,
-                                        0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40 };
-    static const BYTE image_150[24] = { 0x40,0x40,0x40,0x40,0x40,0x40,0x80,0x80,0x80,0x80,0x80,0x80,
-                                        0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40 };
-    static const BYTE image_180[24] = { 0x40,0x40,0x40,0x40,0x40,0x40,0x80,0x80,0x80,0x80,0x80,0x80,
-                                        0x80,0x80,0x80,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40 };
-    static const BYTE image_200[24] = { 0x40,0x40,0x40,0x40,0x40,0x40,0x80,0x80,0x80,0x80,0x80,0x80,
-                                        0x80,0x80,0x80,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40 };
-    static const BYTE image_250[24] = { 0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x80,0x80,0x80,
-                                        0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x40,0x40,0x40 };
-    static const BYTE image_120_half[24] = { 0x40,0x40,0x40,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,
-                                        0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40 };
-    static const BYTE image_150_half[24] = { 0x40,0x40,0x40,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,
-                                        0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40,0x40 };
-    static const BYTE image_200_half[24] = { 0x40,0x40,0x40,0x40,0x40,0x40,0x80,0x80,0x80,0x80,0x80,0x80,
-                                        0x80,0x80,0x80,0x80,0x80,0x80,0x40,0x40,0x40,0x40,0x40,0x40 };
-    static const BYTE image_250_half[24] = { 0x40,0x40,0x40,0x40,0x40,0x40,0x80,0x80,0x80,0x80,0x80,0x80,
-                                        0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x40,0x40,0x40 };
+    static const BYTE back_8x1[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40,
+                                       0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };
+    static const BYTE image_080[24] = { 0x40,0x40,0x40, 0x80,0x80,0x80, 0x40,0x40,0x40, 0x40,0x40,0x40,
+                                        0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };
+    static const BYTE image_100[24] = { 0x40,0x40,0x40, 0x80,0x80,0x80, 0x80,0x80,0x80, 0x40,0x40,0x40,
+                                        0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };
+    static const BYTE image_120[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80, 0x40,0x40,0x40,
+                                        0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };
+    static const BYTE image_150[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80, 0x80,0x80,0x80,
+                                        0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };
+    static const BYTE image_180[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80, 0x80,0x80,0x80,
+                                        0x80,0x80,0x80, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };
+    static const BYTE image_200[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80, 0x80,0x80,0x80,
+                                        0x80,0x80,0x80, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };
+    static const BYTE image_250[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80,
+                                        0x80,0x80,0x80, 0x80,0x80,0x80, 0x80,0x80,0x80, 0x40,0x40,0x40 };
+    static const BYTE image_120_half[24] = { 0x40,0x40,0x40, 0x80,0x80,0x80, 0x80,0x80,0x80, 0x80,0x80,0x80,
+                                             0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };
+    static const BYTE image_150_half[24] = { 0x40,0x40,0x40, 0x80,0x80,0x80, 0x80,0x80,0x80, 0x80,0x80,0x80,
+                                             0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };
+    static const BYTE image_200_half[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80, 0x80,0x80,0x80,
+                                             0x80,0x80,0x80, 0x80,0x80,0x80, 0x40,0x40,0x40, 0x40,0x40,0x40 };
+    static const BYTE image_250_half[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80, 0x80,0x80,0x80,
+                                             0x80,0x80,0x80, 0x80,0x80,0x80, 0x80,0x80,0x80, 0x40,0x40,0x40 };
     static const struct test_data
     {
         REAL scale_x;
@@ -4753,20 +4753,20 @@ static void test_DrawImage_scale(void)
         { 0.8, PixelOffsetModeHalf, image_080 }, /* 14 */
         { 1.0, PixelOffsetModeHalf, image_100 },
         { 1.2, PixelOffsetModeHalf, image_120_half, TRUE },
-        { 1.5, PixelOffsetModeHalf, image_150_half, TRUE },
+        { 1.5, PixelOffsetModeHalf, image_150_half},
         { 1.8, PixelOffsetModeHalf, image_180 },
-        { 2.0, PixelOffsetModeHalf, image_200_half, TRUE },
-        { 2.5, PixelOffsetModeHalf, image_250_half, TRUE },
+        { 2.0, PixelOffsetModeHalf, image_200_half},
+        { 2.5, PixelOffsetModeHalf, image_250_half},

{ 0.8, PixelOffsetModeHighQuality, image_080 }, /* 21 */
         { 1.0, PixelOffsetModeHighQuality, image_100 },
         { 1.2, PixelOffsetModeHighQuality, image_120_half, TRUE },
-        { 1.5, PixelOffsetModeHighQuality, image_150_half, TRUE },
+        { 1.5, PixelOffsetModeHighQuality, image_150_half},
         { 1.8, PixelOffsetModeHighQuality, image_180 },
-        { 2.0, PixelOffsetModeHighQuality, image_200_half, TRUE },
-        { 2.5, PixelOffsetModeHighQuality, image_250_half, TRUE },
+        { 2.0, PixelOffsetModeHighQuality, image_200_half},
+        { 2.5, PixelOffsetModeHighQuality, image_250_half},
     };
-    BYTE src_2x1[6] = { 0x80,0x80,0x80,0x80,0x80,0x80 };
+    BYTE src_2x1[6] = { 0x80,0x80,0x80, 0x80,0x80,0x80 };
     BYTE dst_8x1[24];
     GpStatus status;
     union
@@ -4802,16 +4802,19 @@ static void test_DrawImage_scale(void)
         status = GdipSetWorldTransform(graphics, matrix);
         expect(Ok, status);
         GdipDeleteMatrix(matrix);
-
         memcpy(dst_8x1, back_8x1, sizeof(dst_8x1));
         status = GdipDrawImageI(graphics, u1.image, 1, 0);
         expect(Ok, status);

+
         match = memcmp(dst_8x1, td[i].image, sizeof(dst_8x1)) == 0;
         todo_wine_if (!match && td[i].todo)
             ok(match, "%d: data should match\n", i);
         if (!match)
-            trace("%s\n", dbgstr_hexdata(dst_8x1, sizeof(dst_8x1)));
+        {
+            trace("Expected: %s\n", dbgstr_hexdata(td[i].image, sizeof(dst_8x1)));
+            trace("Got:      %s\n", dbgstr_hexdata(dst_8x1, sizeof(dst_8x1)));
+        }
     }

status = GdipDeleteGraphics(graphics);

[PATCH 1/1] gdiplus: implement PixelOffsetModeHighQuality for GdipDrawImagePointsRect

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=136161

Your paranoid android.

=== debian11b (64 bit WoW report) ===

gdiplus:
image.c:4812: Test failed: 14: data should match
image.c:4812: Test failed: 21: data should match