[PATCH 0/2] MR10724: gdiplus: Elide empty spans.
We assume a span is never empty and therefore can never begin at the right bound. An empty span there can break the requirement in combine_regions_to_spans() that current span Y coordinates are never less than the loop Y coordinate. Empty spans also break the in_left and in_right calculation. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59678 The added test will assert without the fix. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10724
From: Conor McCarthy <cmccarthy@codeweavers.com> We assume a span is never empty and therefore can never begin at the right bound. An empty span there can break the requirement in combine_regions_to_spans() that current span Y coordinates are never less than the loop Y coordinate. Empty spans also break the in_left and in_right calculation. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59678 --- dlls/gdiplus/region.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/gdiplus/region.c b/dlls/gdiplus/region.c index e430308685b..0969aa6aacb 100644 --- a/dlls/gdiplus/region.c +++ b/dlls/gdiplus/region.c @@ -2261,6 +2261,10 @@ static GpStatus edge_list_to_spans_alternate(struct edge_list *edges, struct spa assert(edge[0].y == edge[1].y); + /* edges can be collapsed to the left or right bound, making an empty span */ + if (edge[0].x == edge[1].x) + continue; + span = &spans->spans[spans->length++]; span->x[0] = edge[0].x; span->x[1] = edge[1].x; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10724
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/gdiplus/tests/region.c | 49 +++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/dlls/gdiplus/tests/region.c b/dlls/gdiplus/tests/region.c index 00837096092..2f9d447264f 100644 --- a/dlls/gdiplus/tests/region.c +++ b/dlls/gdiplus/tests/region.c @@ -2727,6 +2727,54 @@ static void test_rounding(void) GdipDeleteMatrix(matrix); } +static void test_empty_spans(void) +{ + GpStatus status; + GpGraphics *graphics; + GpBitmap *bitmap; + GpPath *path, *path2; + const GpPointF inside_path_points[] = { { 20, 20 }, { 80, 20 }, { 80, 80 } }; + const GpPointF outside_path_points[] = { { 100, 20 }, { 101, 20 }, { 101, 80 } }; + GpRegion *region; + GpBrush *brush; + + status = GdipCreateBitmapFromScan0(100, 100, 400, PixelFormat32bppARGB, NULL, &bitmap); + expect(Ok, status); + + status = GdipGetImageGraphicsContext((GpImage*)bitmap, &graphics); + expect(Ok, status); + + GdipCreatePath(FillModeAlternate, &path); + status = GdipAddPathPolygon(path, inside_path_points, ARRAY_SIZE(inside_path_points)); + expect(Ok, status); + + GdipCreatePath(FillModeAlternate, &path2); + /* add a path inside the device bounds and one outside, + * resulting in an empty span at the end of each scanline */ + status = GdipAddPathPolygon(path2, inside_path_points, ARRAY_SIZE(inside_path_points)); + expect(Ok, status); + status = GdipAddPathPolygon(path2, outside_path_points, ARRAY_SIZE(outside_path_points)); + expect(Ok, status); + + status = GdipCreateRegionPath(path, ®ion); + expect(Ok, status); + status = GdipCombineRegionPath(region, path2, CombineModeIntersect); + expect(Ok, status); + + status = GdipCreateSolidFill(0xffffffff, (GpSolidFill**)&brush); + expect(Ok, status); + + status = GdipFillRegion(graphics, brush, region); + expect(Ok, status); + + GdipDeleteBrush(brush); + GdipDeleteRegion(region); + GdipDeletePath(path2); + GdipDeletePath(path); + GdipDisposeImage((GpImage*)bitmap); + GdipDeleteGraphics(graphics); +} + START_TEST(region) { struct GdiplusStartupInput gdiplusStartupInput; @@ -2763,6 +2811,7 @@ START_TEST(region) test_GdipCreateRegionRgnData(); test_incombinedregion(); test_rounding(); + test_empty_spans(); GdiplusShutdown(gdiplusToken); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10724
Nice fix! Please swap order of commits. First should be commit with tests and `todo_wine`. As a result it will be visible what kind of failures you are fixing. The second commit with fix, and removal of `todo_wine`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10724#note_137405
On Thu Apr 23 05:10:39 2026 +0000, Bartosz Kosiorek wrote:
Nice fix! Please swap order of commits. First should be commit with tests and `todo_wine`. As a result it will be visible what kind of failures you are fixing. The second commit with fix, and removal of `todo_wine`. We do not want tests to assert ;)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10724#note_137406
Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/region.c:
+ GpStatus status; + GpGraphics *graphics; + GpBitmap *bitmap; + GpPath *path, *path2; + const GpPointF inside_path_points[] = { { 20, 20 }, { 80, 20 }, { 80, 80 } }; + const GpPointF outside_path_points[] = { { 100, 20 }, { 101, 20 }, { 101, 80 } }; + GpRegion *region; + GpBrush *brush; + + status = GdipCreateBitmapFromScan0(100, 100, 400, PixelFormat32bppARGB, NULL, &bitmap); + expect(Ok, status); + + status = GdipGetImageGraphicsContext((GpImage*)bitmap, &graphics); + expect(Ok, status); + + GdipCreatePath(FillModeAlternate, &path); Please test status for the GdipCreatePath calls. We don't need to handle the failure, just need to know if it fails.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10724#note_137778
participants (4)
-
Bartosz Kosiorek (@gang65) -
Conor McCarthy -
Conor McCarthy (@cmccarthy) -
Esme Povirk (@madewokherd)