Hello Wine community, this is my first Wine patch.
This series fixes some drawing of GDI+ DrawImage() where resampling was applied. I tried to write some unit tests, but it turned out that there is no simple and effective way how to catch all differences between native gdiplus library and Wine lib (without putting significant part of DrawImage() into tests file). At least I wrote relative complex standalone tests for my own testing where I was comparing every pixel drawn by buildin and native lib, and I am pretty sure this series fixes mentioned issues without breaking anything else.
There are much more drawing issues I noticed so far, but for some I do not know to fix them without breaking other stuff, for some issues I know what caused them, but I do not know how to fix them, and for some for I have no idea why are they appearing. But I will keep testing.
Jan Havran (2): gdiplus: Fix rounding dst_area borders for GdipDrawImagePointsRect(). gdiplus: Fix resample offset for nearest neighbor interpolation.
dlls/gdiplus/graphics.c | 46 ++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 8 deletions(-)
Signed-off-by: Jan Havran havran.jan@email.cz --- dlls/gdiplus/graphics.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 3216a3ea43..82f0cc79f5 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -302,6 +302,16 @@ static void round_points(POINT *pti, GpPointF *ptf, INT count) } }
+static void ceil_points(POINT *pti, GpPointF *ptf, INT count) +{ + int i; + + for(i = 0; i < count; i++){ + pti[i].x = ceilr(ptf[i].X); + pti[i].y = ceilr(ptf[i].Y); + } +} + static void gdi_alpha_blend(GpGraphics *graphics, INT dst_x, INT dst_y, INT dst_width, INT dst_height, HDC hdc, INT src_x, INT src_y, INT src_width, INT src_height) { @@ -3013,7 +3023,20 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image 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); + + switch (graphics->pixeloffset) + { + default: + case PixelOffsetModeNone: + case PixelOffsetModeHighSpeed: + ceil_points(pti, ptf, 4); + break; + + case PixelOffsetModeHalf: + case PixelOffsetModeHighQuality: + round_points(pti, ptf, 4); + break; + }
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]));
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=49848
Your paranoid android.
=== debian9 (32 bit report) ===
gdiplus: image.c:4461: Test failed: 0: data should match image.c:4461: Test failed: 7: data should match
=== debian9 (32 bit Chinese:China report) ===
gdiplus: image.c:4461: Test failed: 0: data should match image.c:4461: Test failed: 7: data should match
=== debian9b (32 bit WoW report) ===
gdiplus: image.c:4461: Test failed: 0: data should match image.c:4461: Test failed: 7: data should match
=== debian9b (64 bit WoW report) ===
gdiplus: image.c:4461: Test failed: 0: data should match image.c:4461: Test failed: 7: data should match
While PixelOffsetModeHighSpeed starts resampling from top left corner, the PixelOffsetModeHighQuality uses half of step as an offset.
Signed-off-by: Jan Havran havran.jan@email.cz --- dlls/gdiplus/graphics.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 82f0cc79f5..ec4cb6c69b 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -959,7 +959,7 @@ static ARGB sample_bitmap_pixel(GDIPCONST GpRect *src_rect, LPBYTE bits, UINT wi
static ARGB resample_bitmap_pixel(GDIPCONST GpRect *src_rect, LPBYTE bits, UINT width, UINT height, GpPointF *point, GDIPCONST GpImageAttributes *attributes, - InterpolationMode interpolation, PixelOffsetMode offset_mode) + InterpolationMode interpolation, PixelOffsetMode offset_mode, REAL x_dx_half, REAL y_dy_half) { static int fixme;
@@ -1005,22 +1005,24 @@ static ARGB resample_bitmap_pixel(GDIPCONST GpRect *src_rect, LPBYTE bits, UINT } case InterpolationModeNearestNeighbor: { - FLOAT pixel_offset; + FLOAT pixel_offset_x, pixel_offset_y; switch (offset_mode) { default: case PixelOffsetModeNone: case PixelOffsetModeHighSpeed: - pixel_offset = 0.5; + pixel_offset_x = 0.5; + pixel_offset_y = 0.5; break;
case PixelOffsetModeHalf: case PixelOffsetModeHighQuality: - pixel_offset = 0.0; + pixel_offset_x = x_dx_half; + pixel_offset_y = y_dy_half; break; } return sample_bitmap_pixel(src_rect, bits, width, height, - floorf(point->X + pixel_offset), floorf(point->Y + pixel_offset), attributes); + floorf(point->X + pixel_offset_x), floorf(point->Y + pixel_offset_y), attributes); }
} @@ -1311,6 +1313,8 @@ static GpStatus brush_fill_pixels(GpGraphics *graphics, GpBrush *brush, REAL x_dy = draw_points[1].Y - draw_points[0].Y; REAL y_dx = draw_points[2].X - draw_points[0].X; REAL y_dy = draw_points[2].Y - draw_points[0].Y; + REAL x_dx_half = x_dx / 2.0; + REAL y_dy_half = y_dx / 2.0;
for (y=0; y<fill_area->Height; y++) { @@ -1323,7 +1327,7 @@ static GpStatus brush_fill_pixels(GpGraphics *graphics, GpBrush *brush, argb_pixels[x + y*cdwStride] = resample_bitmap_pixel( &src_area, fill->bitmap_bits, bitmap->width, bitmap->height, &point, fill->imageattributes, graphics->interpolation, - graphics->pixeloffset); + graphics->pixeloffset, x_dx_half, y_dy_half); } } } @@ -3083,6 +3087,7 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image 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; + REAL x_dx_half, y_dy_half; static const GpImageAttributes defaultImageAttributes = {WrapModeClamp, 0, FALSE};
if (!imageAttributes) @@ -3187,6 +3192,8 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image 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_half = x_dx / 2.0; + y_dy_half = y_dy / 2.0;
for (x=dst_area.left; x<dst_area.right; x++) { @@ -3202,7 +3209,7 @@ GpStatus WINGDIPAPI GdipDrawImagePointsRect(GpGraphics *graphics, GpImage *image
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); + imageAttributes, interpolation, offset_mode, x_dx_half, y_dy_half); else *dst_color = 0; }
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=49849
Your paranoid android.
=== debian9 (32 bit report) ===
gdiplus: image.c:4461: Test failed: 0: data should match image.c:4461: Test failed: 7: data should match
=== debian9 (32 bit Chinese:China report) ===
gdiplus: image.c:4461: Test failed: 0: data should match image.c:4461: Test failed: 7: data should match
=== debian9b (32 bit WoW report) ===
gdiplus: image.c:4461: Test failed: 0: data should match image.c:4461: Test failed: 7: data should match
=== debian9b (64 bit WoW report) ===
gdiplus: image.c:4461: Test failed: 0: data should match image.c:4461: Test failed: 7: data should match
On Sun, Mar 24, 2019 at 5:49 PM Jan Havran havran.jan@email.cz wrote:
Hello Wine community, this is my first Wine patch.
This series fixes some drawing of GDI+ DrawImage() where resampling was applied. I tried to write some unit tests, but it turned out that there is no simple and effective way how to catch all differences between native gdiplus library and Wine lib (without putting significant part of DrawImage() into tests file). At least I wrote relative complex standalone tests for my own testing where I was comparing every pixel drawn by buildin and native lib, and I am pretty sure this series fixes mentioned issues without breaking anything else.
There are much more drawing issues I noticed so far, but for some I do not know to fix them without breaking other stuff, for some issues I know what caused them, but I do not know how to fix them, and for some for I have no idea why are they appearing. But I will keep testing.
Jan Havran (2): gdiplus: Fix rounding dst_area borders for GdipDrawImagePointsRect(). gdiplus: Fix resample offset for nearest neighbor interpolation.
dlls/gdiplus/graphics.c | 46 ++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 8 deletions(-)
Hi, Jan.
Could you open a bug report for that, and attach before/after images there that show artifacts?
-- 2.21.0
On 24. 03. 19 16:54, Nikolay Sivov wrote:
On Sun, Mar 24, 2019 at 5:49 PM Jan Havran havran.jan@email.cz wrote:
Hello Wine community, this is my first Wine patch.
This series fixes some drawing of GDI+ DrawImage() where resampling was applied. I tried to write some unit tests, but it turned out that there is no simple and effective way how to catch all differences between native gdiplus library and Wine lib (without putting significant part of DrawImage() into tests file). At least I wrote relative complex standalone tests for my own testing where I was comparing every pixel drawn by buildin and native lib, and I am pretty sure this series fixes mentioned issues without breaking anything else.
There are much more drawing issues I noticed so far, but for some I do not know to fix them without breaking other stuff, for some issues I know what caused them, but I do not know how to fix them, and for some for I have no idea why are they appearing. But I will keep testing.
Jan Havran (2): gdiplus: Fix rounding dst_area borders for GdipDrawImagePointsRect(). gdiplus: Fix resample offset for nearest neighbor interpolation.
dlls/gdiplus/graphics.c | 46 ++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 8 deletions(-)
Hi, Jan.
Could you open a bug report for that, and attach before/after images there that show artifacts?
Hi Nikolay,
sure I can.
It seems like (according to the bot results) I somehow missed related tests in image.c - my fault, but honestly I expected DrawImage tests in graphics.c, where is other DrawImage stuff (my initial understanding was that each file in tests folder is focused on testing implementation of related file in gdiplus folder... anyway I should run all tests next time).
I need to do more investigation. My scale tests were based on calling GdipDrawImagePointsRect with different values in points array (doing the scale), while test in image.c is doing scale by changing world matrix. Interesting thing is that these ways give same results in buildin gdiplus, while it differs in native lib (I have to verify it).
Tests in image.c showed me the way how to write tests for DrawImage, so I will try to cover my patches by these kind of tests next time.
I was pretty sure that I isolated the issues and fixed them without breaking other stuff, but it seems to be more complex than I thought. At least the second patch seems to be ok...
-
Jan Havran -
Marvin -
Nikolay Sivov