[PATCH v2 0/1] MR3865: gdiplus: implement PixelOffsetMode Half and HighQuality for GdipDrawImagePointsRect
-- v2: gdiplus: implement PixelOffsetMode Half and HighQuality for GdipDrawImagePointsRect https://gitlab.winehq.org/wine/wine/-/merge_requests/3865
From: Bartosz Kosiorek <gang65(a)poczta.onet.pl> --- dlls/gdiplus/graphics.c | 75 ++++++++++++++++++++++++++++---------- dlls/gdiplus/tests/image.c | 35 ++++++++++-------- 2 files changed, 75 insertions(+), 35 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 65b33cdc960..648f264ebec 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -1072,7 +1072,7 @@ static ARGB resample_bitmap_pixel(GDIPCONST GpRect *src_rect, LPBYTE bits, UINT break; } return sample_bitmap_pixel(src_rect, bits, width, height, - floorf(point->X + pixel_offset), floorf(point->Y + pixel_offset), attributes); + (INT)(point->X + pixel_offset), (INT)(point->Y + pixel_offset), attributes); } } @@ -3094,6 +3094,7 @@ 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); @@ -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,45 @@ 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; + 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 > 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 (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 = 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); + 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; + + // Based on value of dst_area, the pixel number to draw is calculated. + if ((offset_mode == PixelOffsetModeDefault) || + (offset_mode == PixelOffsetModeNone) || + (offset_mode == PixelOffsetModeHighSpeed)) + { + // PixelOffsetMode None and HighSpeed is not drawing outside dst_area, + // that's why we are using ceilf and floorf. + dst_area.left = ceilf(dst_area_left); + dst_area.top = ceilf(dst_area_top); + dst_area.right = floorf(dst_area_right); + dst_area.bottom = floorf(dst_area_bottom); + } + else + { + // Based on experiments dst_area with value 0.5 should be rounded to 0.0 + // That't why we are adding 0.4999 instead of 0.5 + dst_area.left = (INT)(dst_area_left + 0.4999); + dst_area.top = (INT)(dst_area_top + 0.4999); + dst_area.right = (INT)(dst_area_right + 0.4999); + dst_area.bottom = (INT)(dst_area_bottom + 0.4999); + } TRACE("dst_area: %s\n", wine_dbgstr_rect(&dst_area)); @@ -3249,10 +3273,24 @@ 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; - src_pointf_row.Y = dst_to_src.matrix[5] + - dst_area.left * x_dy + dst_area.top * y_dy; + if ((offset_mode == PixelOffsetModeDefault) || + (offset_mode == PixelOffsetModeNone) || + (offset_mode == PixelOffsetModeHighSpeed)) + { + 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.matrix[5] + + dst_area.left * x_dy + dst_area.top * y_dy; + } + else + { + // With PixelOffsetMode Half we are checking color in the middle of scaled pixel + // That's why we are adding 0.5. + src_pointf_row.X = dst_to_src.matrix[4] + + (dst_area.left + 0.5) * x_dx + (dst_area.top + 0.5) * y_dx; + src_pointf_row.Y = dst_to_src.matrix[5] + + (dst_area.left + 0.5) * x_dy + (dst_area.top + 0.5) * y_dy; + } for (y = dst_area.top; y < dst_area.bottom; y++, src_pointf_row.X += y_dx, src_pointf_row.Y += y_dy) @@ -3260,10 +3298,9 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image 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) { - 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); + + *dst_color = resample_bitmap_pixel(&src_area, src_data, bitmap->width, bitmap->height, &src_pointf, + imageAttributes, interpolation, offset_mode); dst_color++; } } diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 18fe8fcba37..86903c15593 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); @@ -4830,7 +4835,6 @@ static void test_DrawImage_scale(void) REAL scale_x; PixelOffsetMode pixel_offset_mode; const BYTE *image; - BOOL todo; } td[] = { { 0.8, PixelOffsetModeNone, image_080 }, /* 0 */ @@ -4851,19 +4855,19 @@ 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.8, PixelOffsetModeHalf, image_180_half, TRUE }, - { 2.0, PixelOffsetModeHalf, image_200_half, TRUE }, - { 2.5, PixelOffsetModeHalf, image_250_half, TRUE }, + { 1.2, PixelOffsetModeHalf, image_120_half }, + { 1.5, PixelOffsetModeHalf, image_150_half }, + { 1.8, PixelOffsetModeHalf, image_180_half }, + { 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.8, PixelOffsetModeHighQuality, image_180_half, TRUE }, - { 2.0, PixelOffsetModeHighQuality, image_200_half, TRUE }, - { 2.5, PixelOffsetModeHighQuality, image_250_half, TRUE }, + { 1.2, PixelOffsetModeHighQuality, image_120_half }, + { 1.5, PixelOffsetModeHighQuality, image_150_half }, + { 1.8, PixelOffsetModeHighQuality, image_180_half }, + { 2.0, PixelOffsetModeHighQuality, image_200_half }, + { 2.5, PixelOffsetModeHighQuality, image_250_half }, }; BYTE src_2x1[6] = { 0x80,0x80,0x80, 0xcc,0xcc,0xcc }; BYTE dst_8x1[24]; @@ -4907,8 +4911,7 @@ static void test_DrawImage_scale(void) 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); + ok(match, "%d: data should match\n", i); if (!match) { trace("Expected: %s\n", dbgstr_hexdata(td[i].image, sizeof(dst_8x1))); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3865
On Fri Sep 15 21:16:04 2023 +0000, Bartosz Kosiorek wrote:
Based on experiments I have managed to check how destination size is calculated. If it makes a difference, there should be tests. Those tests should also show that it is not using banker's rounding.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3865#note_45624
On Sat Sep 16 04:12:44 2023 +0000, Jeffrey Smith wrote:
If it makes a difference, there should be tests. Those tests should also show that it is not using banker's rounding. Without that solution following scaling tests will fail for HighQuality: 150% and 250%.
What is banker's rounding? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3865#note_45627
participants (3)
-
Bartosz Kosiorek -
Bartosz Kosiorek (＠gang65)
-
Jeffrey Smith (＠whydoubt)