[PATCH v5 0/2] MR3877: gdiplus: implement PixelOffsetMode Half and HighQuality for GdipDrawImagePointsRect
-- v5: gdiplus/tests: add tests of InterpolationMode Bilinear for GdipDrawImagePointsRect gdiplus: implement PixelOffsetMode Half and HighQuality for GdipDrawImagePointsRect
From: Bartosz Kosiorek gang65@poczta.onet.pl
--- dlls/gdiplus/graphics.c | 16 ++++++++++++++-- dlls/gdiplus/tests/image.c | 26 +++++++++++++++++--------- 2 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 65b33cdc960..ec57199bd96 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -3217,6 +3217,7 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image GpMatrix dst_to_src; REAL m11, m12, m21, m22, mdx, mdy; REAL x_dx, x_dy, y_dx, y_dy; + REAL dst_pixel_offset; ARGB *dst_color; GpPointF src_pointf_row, src_pointf;
@@ -3247,12 +3248,23 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image } dst_color = (ARGB*)(dst_data);
+ if ((offset_mode == PixelOffsetModeHalf) || + (offset_mode == PixelOffsetModeHighQuality)) + { + // We are checking color in the middle of destination pixel, + // The pixel center is at (0.5, 0.5). + dst_pixel_offset = 0.5; + } + else + // The pixel center is at (0.0, 0.0). + dst_pixel_offset = 0.0; + /* 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 + dst_pixel_offset) * x_dx + (dst_area.top + dst_pixel_offset) * y_dx; src_pointf_row.Y = dst_to_src.matrix[5] + - dst_area.left * x_dy + dst_area.top * y_dy; + (dst_area.left + dst_pixel_offset) * x_dy + (dst_area.top + dst_pixel_offset) * y_dy;
for (y = dst_area.top; y < dst_area.bottom; y++, src_pointf_row.X += y_dx, src_pointf_row.Y += y_dy) diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 18fe8fcba37..b9b3466ea4f 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -4618,8 +4618,10 @@ static void test_DrawImage(void) match = memcmp(white_2x2, black_2x2, sizeof(black_2x2)) == 0; ok(match, "data should match\n"); if (!match) - trace("%s\n", dbgstr_hexdata(white_2x2, sizeof(white_2x2))); - + { + trace("Expected: %s\n", dbgstr_hexdata(black_2x2, sizeof(black_2x2))); + trace("Got: %s\n", dbgstr_hexdata(white_2x2, sizeof(white_2x2))); + } status = GdipDeleteGraphics(graphics); expect(Ok, status); status = GdipDisposeImage(u1.image); @@ -4708,7 +4710,10 @@ static void test_GdipDrawImagePointRect(void) match = memcmp(white_2x2, black_2x2, sizeof(black_2x2)) == 0; ok(match, "data should match\n"); if (!match) - trace("%s\n", dbgstr_hexdata(white_2x2, sizeof(white_2x2))); + { + trace("Expected: %s\n", dbgstr_hexdata(black_2x2, sizeof(black_2x2))); + trace("Got: %s\n", dbgstr_hexdata(white_2x2, sizeof(white_2x2))); + }
status = GdipDeleteGraphics(graphics); expect(Ok, status); @@ -4825,6 +4830,8 @@ static void test_DrawImage_scale(void) 0xcc,0xcc,0xcc, 0xcc,0xcc,0xcc, 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, 0xcc,0xcc,0xcc, 0xcc,0xcc,0xcc, 0x40,0x40,0x40 }; + static const BYTE image_251_half[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80, + 0x80,0x80,0x80, 0xcc,0xcc,0xcc, 0xcc,0xcc,0xcc, 0xcc,0xcc,0xcc }; static const struct test_data { REAL scale_x; @@ -4851,19 +4858,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.2, PixelOffsetModeHalf, image_120_half }, { 1.5, PixelOffsetModeHalf, image_150_half, TRUE }, - { 1.8, PixelOffsetModeHalf, image_180_half, TRUE }, - { 2.0, PixelOffsetModeHalf, image_200_half, TRUE }, + { 1.8, PixelOffsetModeHalf, image_180_half }, + { 2.0, PixelOffsetModeHalf, image_200_half }, { 2.5, PixelOffsetModeHalf, image_250_half, TRUE },
{ 0.8, PixelOffsetModeHighQuality, image_080 }, /* 21 */ { 1.0, PixelOffsetModeHighQuality, image_100 }, - { 1.2, PixelOffsetModeHighQuality, image_120_half, TRUE }, + { 1.2, PixelOffsetModeHighQuality, image_120_half }, { 1.5, PixelOffsetModeHighQuality, image_150_half, TRUE }, - { 1.8, PixelOffsetModeHighQuality, image_180_half, TRUE }, - { 2.0, PixelOffsetModeHighQuality, image_200_half, TRUE }, + { 1.8, PixelOffsetModeHighQuality, image_180_half }, + { 2.0, PixelOffsetModeHighQuality, image_200_half }, { 2.5, PixelOffsetModeHighQuality, image_250_half, TRUE }, + { 2.51, PixelOffsetModeHighQuality, image_251_half }, }; BYTE src_2x1[6] = { 0x80,0x80,0x80, 0xcc,0xcc,0xcc }; BYTE dst_8x1[24];
From: Bartosz Kosiorek gang65@poczta.onet.pl
--- dlls/gdiplus/tests/image.c | 111 +++++++++++++++++++++++++------------ 1 file changed, 76 insertions(+), 35 deletions(-)
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index b9b3466ea4f..77b9d3a230b 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -4802,6 +4802,23 @@ static void test_image_format(void) } }
+INT memcmp_precision(const void *ptr1, const void *ptr2, size_t num, INT precision) +{ + if (ptr1 == NULL || ptr2 == NULL) + return ptr1 < ptr2 ? -1 : 1; + + for (size_t i = 0; i < num; i++) + { + unsigned char byte1 = *((unsigned char *)ptr1 + i); + unsigned char byte2 = *((unsigned char *)ptr2 + i); + + if ((byte1 < byte2 - precision) || (byte1 > byte2 + precision)) + return byte1 < byte2 ? -1 : 1; + } + + return 0; +} + static void test_DrawImage_scale(void) { static const BYTE back_8x1[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, @@ -4832,46 +4849,69 @@ static void test_DrawImage_scale(void) 0x80,0x80,0x80, 0xcc,0xcc,0xcc, 0xcc,0xcc,0xcc, 0x40,0x40,0x40 }; static const BYTE image_251_half[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80, 0x80,0x80,0x80, 0xcc,0xcc,0xcc, 0xcc,0xcc,0xcc, 0xcc,0xcc,0xcc }; + + static const BYTE image_bil_080[24] = { 0x40,0x40,0x40, 0x93,0x93,0x93, 0x86,0x86,0x86, 0x40,0x40,0x40, + 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 }; + static const BYTE image_bil_120[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0xb2,0xb2,0xb2, 0x87,0x87,0x87, + 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 }; + static const BYTE image_bil_150[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x99,0x99,0x99, 0xcc,0xcc,0xcc, + 0x6f,0x6f,0x6f, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 }; + static const BYTE image_bil_180[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x88,0x88,0x88, 0xb2,0xb2,0xb2, + 0xad,0xad,0xad, 0x5f,0x5f,0x5f, 0x40,0x40,0x40, 0x40,0x40,0x40 }; + static const BYTE image_bil_200[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80, 0xa6,0xa6,0xa6, + 0xcc,0xcc,0xcc, 0x86,0x86,0x86, 0x40,0x40,0x40, 0x40,0x40,0x40 }; + static const BYTE image_bil_250[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x8f,0x8f,0x8f, + 0xad,0xad,0xad, 0xcc,0xcc,0xcc, 0x95,0x95,0x95, 0x5c,0x5c,0x5c }; static const struct test_data { REAL scale_x; + InterpolationMode interpolation_mode; PixelOffsetMode pixel_offset_mode; const BYTE *image; + INT precision; BOOL todo; } td[] = { - { 0.8, PixelOffsetModeNone, image_080 }, /* 0 */ - { 1.0, PixelOffsetModeNone, image_100 }, - { 1.2, PixelOffsetModeNone, image_120 }, - { 1.5, PixelOffsetModeNone, image_150 }, - { 1.8, PixelOffsetModeNone, image_180 }, - { 2.0, PixelOffsetModeNone, image_200 }, - { 2.5, PixelOffsetModeNone, image_250 }, - - { 0.8, PixelOffsetModeHighSpeed, image_080 }, /* 7 */ - { 1.0, PixelOffsetModeHighSpeed, image_100 }, - { 1.2, PixelOffsetModeHighSpeed, image_120 }, - { 1.5, PixelOffsetModeHighSpeed, image_150 }, - { 1.8, PixelOffsetModeHighSpeed, image_180 }, - { 2.0, PixelOffsetModeHighSpeed, image_200 }, - { 2.5, PixelOffsetModeHighSpeed, image_250 }, - - { 0.8, PixelOffsetModeHalf, image_080 }, /* 14 */ - { 1.0, PixelOffsetModeHalf, image_100 }, - { 1.2, PixelOffsetModeHalf, image_120_half }, - { 1.5, PixelOffsetModeHalf, image_150_half, TRUE }, - { 1.8, PixelOffsetModeHalf, image_180_half }, - { 2.0, PixelOffsetModeHalf, image_200_half }, - { 2.5, PixelOffsetModeHalf, image_250_half, TRUE }, - - { 0.8, PixelOffsetModeHighQuality, image_080 }, /* 21 */ - { 1.0, PixelOffsetModeHighQuality, image_100 }, - { 1.2, PixelOffsetModeHighQuality, image_120_half }, - { 1.5, PixelOffsetModeHighQuality, image_150_half, TRUE }, - { 1.8, PixelOffsetModeHighQuality, image_180_half }, - { 2.0, PixelOffsetModeHighQuality, image_200_half }, - { 2.5, PixelOffsetModeHighQuality, image_250_half, TRUE }, - { 2.51, PixelOffsetModeHighQuality, image_251_half }, + { 0.8, InterpolationModeNearestNeighbor, PixelOffsetModeNone, image_080 }, /* 0 */ + { 1.0, InterpolationModeNearestNeighbor, PixelOffsetModeNone, image_100 }, + { 1.2, InterpolationModeNearestNeighbor, PixelOffsetModeNone, image_120 }, + { 1.5, InterpolationModeNearestNeighbor, PixelOffsetModeNone, image_150 }, + { 1.8, InterpolationModeNearestNeighbor, PixelOffsetModeNone, image_180 }, + { 2.0, InterpolationModeNearestNeighbor, PixelOffsetModeNone, image_200 }, + { 2.5, InterpolationModeNearestNeighbor, PixelOffsetModeNone, image_250 }, + + { 0.8, InterpolationModeNearestNeighbor, PixelOffsetModeHighSpeed, image_080 }, /* 7 */ + { 1.0, InterpolationModeNearestNeighbor, PixelOffsetModeHighSpeed, image_100 }, + { 1.2, InterpolationModeNearestNeighbor, PixelOffsetModeHighSpeed, image_120 }, + { 1.5, InterpolationModeNearestNeighbor, PixelOffsetModeHighSpeed, image_150 }, + { 1.8, InterpolationModeNearestNeighbor, PixelOffsetModeHighSpeed, image_180 }, + { 2.0, InterpolationModeNearestNeighbor, PixelOffsetModeHighSpeed, image_200 }, + { 2.5, InterpolationModeNearestNeighbor, PixelOffsetModeHighSpeed, image_250 }, + + { 0.8, InterpolationModeNearestNeighbor, PixelOffsetModeHalf, image_080 }, /* 14 */ + { 1.0, InterpolationModeNearestNeighbor, PixelOffsetModeHalf, image_100 }, + { 1.2, InterpolationModeNearestNeighbor, PixelOffsetModeHalf, image_120_half }, + { 1.5, InterpolationModeNearestNeighbor, PixelOffsetModeHalf, image_150_half, 0, TRUE }, + { 1.8, InterpolationModeNearestNeighbor, PixelOffsetModeHalf, image_180_half }, + { 2.0, InterpolationModeNearestNeighbor, PixelOffsetModeHalf, image_200_half }, + { 2.5, InterpolationModeNearestNeighbor, PixelOffsetModeHalf, image_250_half, 0, TRUE }, + + { 0.8, InterpolationModeNearestNeighbor, PixelOffsetModeHighQuality, image_080 }, /* 21 */ + { 1.0, InterpolationModeNearestNeighbor, PixelOffsetModeHighQuality, image_100 }, + { 1.2, InterpolationModeNearestNeighbor, PixelOffsetModeHighQuality, image_120_half }, + { 1.5, InterpolationModeNearestNeighbor, PixelOffsetModeHighQuality, image_150_half, 0, TRUE }, + { 1.8, InterpolationModeNearestNeighbor, PixelOffsetModeHighQuality, image_180_half }, + { 2.0, InterpolationModeNearestNeighbor, PixelOffsetModeHighQuality, image_200_half }, + { 2.5, InterpolationModeNearestNeighbor, PixelOffsetModeHighQuality, image_250_half, 0, TRUE }, + { 2.51, InterpolationModeNearestNeighbor, PixelOffsetModeHighQuality, image_251_half }, + + { 0.8, InterpolationModeBilinear, PixelOffsetModeNone, image_bil_080, 2, TRUE }, /* 29 */ + { 1.0, InterpolationModeBilinear, PixelOffsetModeNone, image_100 }, + { 1.2, InterpolationModeBilinear, PixelOffsetModeNone, image_bil_120, 2 }, + { 1.5, InterpolationModeBilinear, PixelOffsetModeNone, image_bil_150, 2 }, + { 1.8, InterpolationModeBilinear, PixelOffsetModeNone, image_bil_180, 2, TRUE }, + { 2.0, InterpolationModeBilinear, PixelOffsetModeNone, image_bil_200, 2 }, + { 2.5, InterpolationModeBilinear, PixelOffsetModeNone, image_bil_250, 2 }, }; BYTE src_2x1[6] = { 0x80,0x80,0x80, 0xcc,0xcc,0xcc }; BYTE dst_8x1[24]; @@ -4896,11 +4936,12 @@ static void test_DrawImage_scale(void) expect(Ok, status); status = GdipGetImageGraphicsContext(u2.image, &graphics); expect(Ok, status); - status = GdipSetInterpolationMode(graphics, InterpolationModeNearestNeighbor); - expect(Ok, status);
for (i = 0; i < ARRAY_SIZE(td); i++) { + status = GdipSetInterpolationMode(graphics, td[i].interpolation_mode); + expect(Ok, status); + status = GdipSetPixelOffsetMode(graphics, td[i].pixel_offset_mode); expect(Ok, status);
@@ -4914,7 +4955,7 @@ static void test_DrawImage_scale(void) status = GdipDrawImageI(graphics, u1.image, 1, 0); expect(Ok, status);
- match = memcmp(dst_8x1, td[i].image, sizeof(dst_8x1)) == 0; + match = memcmp_precision(dst_8x1, td[i].image, sizeof(dst_8x1), td[i].precision) == 0; todo_wine_if (!match && td[i].todo) ok(match, "%d: data should match\n", i); if (!match)
Jeffrey Smith (@whydoubt) commented about dlls/gdiplus/tests/image.c:
}}
+INT memcmp_precision(const void *ptr1, const void *ptr2, size_t num, INT precision)
To me, memcmp implies treating the buffers as opaque blobs. This function is treating them as BYTE arrays, so I think a different name would be appropriate.
Jeffrey Smith (@whydoubt) commented about dlls/gdiplus/tests/image.c:
0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };- static const BYTE image_bil_150[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x99,0x99,0x99, 0xcc,0xcc,0xcc,
0x6f,0x6f,0x6f, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40 };- static const BYTE image_bil_180[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x88,0x88,0x88, 0xb2,0xb2,0xb2,
0xad,0xad,0xad, 0x5f,0x5f,0x5f, 0x40,0x40,0x40, 0x40,0x40,0x40 };- static const BYTE image_bil_200[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x80,0x80,0x80, 0xa6,0xa6,0xa6,
0xcc,0xcc,0xcc, 0x86,0x86,0x86, 0x40,0x40,0x40, 0x40,0x40,0x40 };- static const BYTE image_bil_250[24] = { 0x40,0x40,0x40, 0x40,0x40,0x40, 0x40,0x40,0x40, 0x8f,0x8f,0x8f,
0xad,0xad,0xad, 0xcc,0xcc,0xcc, 0x95,0x95,0x95, 0x5c,0x5c,0x5c };InterpolationMode interpolation_mode; PixelOffsetMode pixel_offset_mode; const BYTE *image;INT precision;
Is this precision necessary because different versions of gdiplus give different results, in anticipation of some future work, or giving the current implementation some slack?
On Mon Sep 18 17:27:26 2023 +0000, Jeffrey Smith wrote:
Is this precision necessary because different versions of gdiplus give different results, in anticipation of some future work, or giving the current implementation some slack?
Unfortunately different gdiplus versions gives different results.
On Mon Sep 18 17:26:55 2023 +0000, Jeffrey Smith wrote:
To me, memcmp implies treating the buffers as opaque blobs. This function is treating them as BYTE arrays, so I think a different name would be appropriate.
Would you propose some name?
-
Bartosz Kosiorek -
Bartosz Kosiorek (＠gang65)
-
Jeffrey Smith (＠whydoubt)