[PATCH v2 0/8] Fix infinite loop for END_OPEN figures with coincident last/first vertex
This is an attempt to fix https://bugs.winehq.org/show_bug.cgi?id=51139 The first two patches just increase the coverage for stroking and StrokeContains, and proved helpful during development of the actual fix. The third patch fixes a small issue in the segment comparision helper. The fourth patch adds the test case triggering bug 51139. As the test causes a hang, the critical part is commented out instead of using the regular wine_todo. The fifth patch is preparative refactoring, so the bugfix itself is more self-contained. The sixth patch is the actual fix, and patch 7 reenables the critical test case parts. The last patch is just some minor, final cleanup. This patch series obsoletes the previous attempt: "d2d: Suppress last empty segment also for D2D1_FIGURE_END_OPEN", which avoided the infinite loop, but also caused several regressions. --- v2: Patch 2: Update reference to Windows CI output, add some slack for minor rendering differences between Wine and Windows Patch 4: Remove leftover debug trace, add some tolerance for GetBounds on bezier segment Patch 6: Remove a spurious whitespace change Stefan Brüns (8): d2d1/tests: Test StrokeContains when last segment is bezier and closes the path d2d1/tests: Add test for single quadratic bezier segment d2d1/tests: Avoid out-of-bounds access when comparing segments d2d1/tests: Test coincident last/first vertex for both OPEN and CLOSED d2d1: Refactor outline generation as preparation for new TYPE_END vertex d2d1: Use special vertex TYPE_END when last/first coincede d2d1/tests: Reenable tests formerly causing infinite loop d2d1: remove extraneous checks for last vertex dlls/d2d1/geometry.c | 215 +++++++++++++++-------------------------- dlls/d2d1/tests/d2d1.c | 170 ++++++++++++++++++++++++++++++-- 2 files changed, 242 insertions(+), 143 deletions(-) -- 2.34.1
Also check StrokeContains result on the last segment when the last vertex of a bezier curve coincedes with the first vertex of the figure and the figure is explicitly closed. Signed-off-by: Stefan Brüns <stefan.bruens(a)rwth-aachen.de> --- dlls/d2d1/tests/d2d1.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 70c47795b4e..2d9bcfc892e 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -10253,8 +10253,14 @@ static void test_stroke_contains_point(BOOL d3d11) {{{{1.0f, 0.0f, 0.0f, 1.0f, 10.0f, 10.0f}}}, { 90.5f, 485.3f}, 0.1f, 1.0f, TRUE, TRUE}, {{{{1.0f, 0.0f, 0.0f, 1.0f, 10.0f, 10.0f}}}, { 91.5f, 485.3f}, 0.1f, 1.0f, TRUE, FALSE}, {{{{1.0f, 0.0f, 0.0f, 1.0f, 10.0f, 10.0f}}}, { 89.0f, 485.3f}, 0.1f, 1.0f, TRUE, FALSE}, - - /* 20. A curve where the control points project beyond the endpoints + {{{{1.0f, 0.0f, 0.0f, 1.0f, 10.0f, 10.0f}}}, {167.0f, 729.8f}, 0.0f, 1.0f, TRUE, TRUE}, + {{{{1.0f, 0.0f, 0.0f, 1.0f, 10.0f, 10.0f}}}, {167.0f, 728.6f}, 0.0f, 1.0f, TRUE, FALSE}, + {{{{1.0f, 0.0f, 0.0f, 1.0f, 10.0f, 10.0f}}}, {167.0f, 731.0f}, 0.0f, 1.0f, TRUE, FALSE}, + {{{{1.0f, 0.0f, 0.0f, 1.0f, 10.0f, 10.0f}}}, {173.0f, 729.8f}, 0.0f, 1.0f, TRUE, TRUE}, + {{{{1.0f, 0.0f, 0.0f, 1.0f, 10.0f, 10.0f}}}, {173.0f, 728.6f}, 0.0f, 1.0f, TRUE, FALSE}, + {{{{1.0f, 0.0f, 0.0f, 1.0f, 10.0f, 10.0f}}}, {173.0f, 731.0f}, 0.0f, 1.0f, TRUE, FALSE}, + + /* 26. A curve where the control points project beyond the endpoints * onto the line through the endpoints. */ {{{{0.0f}}}, {306.97f, 791.67f}, 0.0f, 1.0f, FALSE, FALSE}, {{{{0.0f}}}, {307.27f, 791.67f}, 0.0f, 1.0f, FALSE, TRUE}, @@ -10271,7 +10277,7 @@ static void test_stroke_contains_point(BOOL d3d11) {{{{0.0f}}}, {392.73f, 791.67f}, 0.0f, 1.0f, FALSE, TRUE}, {{{{0.0f}}}, {393.03f, 791.67f}, 0.0f, 1.0f, FALSE, FALSE}, - /* 32. A curve where the endpoints coincide. */ + /* 38. A curve where the endpoints coincide. */ {{{{0.0f}}}, {570.23f, 799.77f}, 0.0f, 1.0f, FALSE, FALSE}, {{{{0.0f}}}, {570.53f, 799.77f}, 0.0f, 1.0f, FALSE, TRUE}, {{{{0.0f}}}, {571.73f, 799.77f}, 0.0f, 1.0f, FALSE, TRUE}, @@ -10287,7 +10293,7 @@ static void test_stroke_contains_point(BOOL d3d11) {{{{0.0f}}}, {629.47f, 799.77f}, 0.0f, 1.0f, FALSE, TRUE}, {{{{0.0f}}}, {629.77f, 799.77f}, 0.0f, 1.0f, FALSE, FALSE}, - /* 44. A curve with coinciding endpoints, as well as coinciding + /* 50. A curve with coinciding endpoints, as well as coinciding * control points. */ {{{{0.0f}}}, {825.00f, 800.00f}, 0.0f, 1.0f, FALSE, TRUE}, {{{{0.0f}}}, {861.00f, 824.00f}, 0.0f, 1.0f, FALSE, TRUE}, @@ -10296,7 +10302,7 @@ static void test_stroke_contains_point(BOOL d3d11) {{{{0.0f}}}, {864.00f, 824.00f}, 0.0f, 1.0f, FALSE, FALSE}, {{{{0.0f}}}, {864.00f, 826.00f}, 0.0f, 1.0f, FALSE, FALSE}, - /* 50. Shear transforms. */ + /* 56. Shear transforms. */ {{{{1.0f, 0.0f, 1.0f, 1.0f}}}, { 837.2f, 600.0f}, 0.1f, 5.0f, TRUE, FALSE}, {{{{1.0f, 0.0f, 1.0f, 1.0f}}}, { 837.5f, 600.0f}, 0.1f, 5.0f, TRUE, TRUE}, {{{{1.0f, 0.0f, 1.0f, 1.0f}}}, {1186.3f, 791.7f}, 0.1f, 5.0f, TRUE, TRUE}, -- 2.34.1
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=105142 Your paranoid android. === build (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply
Extra test for outline generation code, path with only 2 vertices. Signed-off-by: Stefan Brüns <stefan.bruens(a)rwth-aachen.de> --- dlls/d2d1/tests/d2d1.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 2d9bcfc892e..efa4bf488e1 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -6879,6 +6879,45 @@ static void test_draw_geometry(BOOL d3d11) "AiWaAiScAiKdAiGeAh+hAhyjAhmuAg3GxgEA"); ok(match, "Figure does not match.\n"); + hr = ID2D1Factory_CreatePathGeometry(factory, &geometry); + ok(SUCCEEDED(hr), "Failed to create path geometry, hr %#x.\n", hr); + hr = ID2D1PathGeometry_Open(geometry, &sink); + ok(SUCCEEDED(hr), "Failed to open geometry sink, hr %#x.\n", hr); + + set_point(&point, 20.0f, 80.0f); + ID2D1GeometrySink_BeginFigure(sink, point, D2D1_FIGURE_BEGIN_HOLLOW); + quadratic_to(sink, 20.0f, 160.0f, 60.0f, 160.0f); + ID2D1GeometrySink_EndFigure(sink, D2D1_FIGURE_END_CLOSED); + + set_point(&point, 100.0f, 80.0f); + ID2D1GeometrySink_BeginFigure(sink, point, D2D1_FIGURE_BEGIN_HOLLOW); + quadratic_to(sink, 100.0f, 160.0f, 140.0f, 160.0f); + ID2D1GeometrySink_EndFigure(sink, D2D1_FIGURE_END_OPEN); + + hr = ID2D1GeometrySink_Close(sink); + ok(SUCCEEDED(hr), "Failed to close geometry sink, hr %#x.\n", hr); + ID2D1GeometrySink_Release(sink); + + ID2D1RenderTarget_BeginDraw(rt); + ID2D1RenderTarget_Clear(rt, &color); + ID2D1RenderTarget_DrawGeometry(rt, (ID2D1Geometry *)geometry, (ID2D1Brush *)brush, 10.0f, NULL); + hr = ID2D1RenderTarget_EndDraw(rt, NULL, NULL); + ok(SUCCEEDED(hr), "Failed to end draw, hr %#x.\n", hr); + ID2D1PathGeometry_Release(geometry); + + match = compare_figure(&ctx, 0, 0, 160, 160, 0xff652e89, 17, + "3iUCngEEnAEGmgEImAEKlgEMlAEOkgEQkAESjgEUjAEWigEYiAEahgEchAEeggEggQEhfyN9JXsn" + "eih4KnYUAhZ1FAMWcxQFFnIUBhZwFQcWbxQJFm4UChZsFQsWaxUMFmoVDRZpFQ4WaBUPFmcVEBZm" + "FREWZhURFmUVEhZkFhIWZBYSFmQXERZkFxEWZBgQFmQaDhZlGwwWZh0JFmchBBZpOWw2bzN0Lnsn" + "2mEA"); + ok(match, "Figure does not match.\n"); + + match = compare_figure(&ctx, 160, 0, 160, 160, 0xff652e89, 24, + "njIUjAEUjAEUjAEUjAEUjAEUjQEUjAEUjAEUjAEUjQEUjAEUjAEUjQEUjAEUjQEUjAEVjAEUjQEU" + "jAEVjAEVjAEVjAEVjAEVjAEVjAEVjQEVjAEVjAEWjAEWjAEXiwEXiwEYigEaiQEbiAEdhgEhgwEz" + "ci53KX4ihwEZ6GEA"); + ok(match, "Figure does not match.\n"); + ID2D1SolidColorBrush_Release(brush); ID2D1Factory_Release(factory); release_test_context(&ctx); -- 2.34.1
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=105143 Your paranoid android. === build (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply
In case real and expected segment count differ, don't compare the segment type/position for extra segments. Signed-off-by: Stefan Brüns <stefan.bruens(a)rwth-aachen.de> --- dlls/d2d1/tests/d2d1.c | 44 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index efa4bf488e1..3b7c45357ae 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -1248,6 +1248,7 @@ static void geometry_sink_check_(unsigned int line, const struct geometry_sink * const struct expected_geometry_figure *expected_figure; const struct geometry_figure *figure; unsigned int i, j; + unsigned int segment_count; BOOL match; ok_(__FILE__, line)(sink->fill_mode == fill_mode, @@ -1276,7 +1277,10 @@ static void geometry_sink_check_(unsigned int line, const struct geometry_sink * "Got unexpected figure %u segment count %u, expected %u.\n", i, figure->segment_count, expected_figure->segment_count); - for (j = 0; j < figure->segment_count; ++j) + segment_count = expected_figure->segment_count < figure->segment_count ? + expected_figure->segment_count : figure->segment_count; + + for (j = 0; j < segment_count; ++j) { expected_segment = &expected_figure->segments[j]; segment = &figure->segments[j]; @@ -1315,6 +1319,44 @@ static void geometry_sink_check_(unsigned int line, const struct geometry_sink * break; } } + + for (j = segment_count; j < expected_figure->segment_count; ++j) + { + segment = &expected_figure->segments[j]; + switch (segment->type) + { + case SEGMENT_LINE: + ok_(__FILE__, line)(FALSE, "Missing figure %u segment %u {%.8e, %.8e}.\n", + i, j, segment->u.line.x, segment->u.line.y); + break; + case SEGMENT_BEZIER: + ok_(__FILE__, line)(FALSE, "Missing figure %u segment %u " + "{%.8e, %.8e, %.8e, %.8e, %.8e, %.8e}\n", + i, j, segment->u.bezier.point1.x, segment->u.bezier.point1.y, + segment->u.bezier.point2.x, segment->u.bezier.point2.y, + segment->u.bezier.point3.x, segment->u.bezier.point3.y); + break; + } + } + + for (j = segment_count; j < figure->segment_count; ++j) + { + segment = &figure->segments[j]; + switch (segment->type) + { + case SEGMENT_LINE: + ok_(__FILE__, line)(FALSE, "Got unexpected figure %u segment %u {%.8e, %.8e}.\n", + i, j, segment->u.line.x, segment->u.line.y); + break; + case SEGMENT_BEZIER: + ok_(__FILE__, line)(FALSE, "Got unexpected figure %u segment %u " + "{%.8e, %.8e, %.8e, %.8e, %.8e, %.8e}\n", + i, j, segment->u.bezier.point1.x, segment->u.bezier.point1.y, + segment->u.bezier.point2.x, segment->u.bezier.point2.y, + segment->u.bezier.point3.x, segment->u.bezier.point3.y); + break; + } + } } } -- 2.34.1
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=105144 Your paranoid android. === build (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Provide two orientatations for both cases (END_OPEN/END_CLOSED) each. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51139 Signed-off-by: Stefan Brüns <stefan.bruens(a)rwth-aachen.de> --- dlls/d2d1/tests/d2d1.c | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 3b7c45357ae..9f2c432c55e 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -3083,6 +3083,23 @@ static void test_path_geometry(BOOL d3d11) /* Figure 28. */ {SEGMENT_LINE, {{{ 75.0f, 300.0f}}}}, {SEGMENT_LINE, {{{ 5.0f, 300.0f}}}}, + /* Figure 29. */ + {SEGMENT_LINE, {{{ 40.0f, 100.0f}}}}, + {SEGMENT_BEZIER, {{{4.00000000e+01f, 7.66666666e+01f}, + {3.33333333e+01f, 7.00000000e+01f}, + {2.00000000e+01f, 8.00000000e+01f}}}}, + {SEGMENT_LINE, {{{ 60.0f, 40.0f}}}}, + {SEGMENT_BEZIER, {{{8.33333333e+01f, 4.00000000e+01f}, + {9.00000000e+01f, 3.33333333e+01f}, + {8.00000000e+01f, 2.00000000e+01f}}}}, + {SEGMENT_LINE, {{{140.0f, 160.0f}}}}, + {SEGMENT_BEZIER, {{{1.16666666e+02f, 1.60000000e+02f}, + {1.10000000e+02f, 1.66666666e+02f}, + {1.20000000e+02f, 1.80000000e+02f}}}}, + {SEGMENT_LINE, {{{170.0f, 110.0f}}}}, + {SEGMENT_BEZIER, {{{1.70000000e+02f, 1.26666666e+02f}, + {1.73333333e+02f, 1.30000000e+02f}, + {1.80000000e+02f, 1.20000000e+02f}}}}, }; static const struct expected_geometry_figure expected_figures[] = { @@ -3126,6 +3143,11 @@ static void test_path_geometry(BOOL d3d11) {D2D1_FIGURE_BEGIN_FILLED, D2D1_FIGURE_END_CLOSED, { 20.0f, 612.0f}, 8, &expected_segments[164]}, /* 28 */ {D2D1_FIGURE_BEGIN_HOLLOW, D2D1_FIGURE_END_OPEN, { 40.0f, 20.0f}, 2, &expected_segments[172]}, + /* 29 */ + {D2D1_FIGURE_BEGIN_FILLED, D2D1_FIGURE_END_OPEN, { 20.0f, 80.0f}, 2, &expected_segments[174]}, + {D2D1_FIGURE_BEGIN_FILLED, D2D1_FIGURE_END_OPEN, { 80.0f, 20.0f}, 2, &expected_segments[176]}, + {D2D1_FIGURE_BEGIN_FILLED, D2D1_FIGURE_END_CLOSED, { 120.0f, 180.0f}, 2, &expected_segments[178]}, + {D2D1_FIGURE_BEGIN_FILLED, D2D1_FIGURE_END_CLOSED, { 180.0f, 120.0f}, 2, &expected_segments[180]}, }; if (!init_test_context(&ctx, d3d11)) @@ -3871,6 +3893,65 @@ static void test_path_geometry(BOOL d3d11) ID2D1PathGeometry_Release(geometry); + hr = ID2D1Factory_CreatePathGeometry(factory, &geometry); + ok(SUCCEEDED(hr), "Failed to create path geometry, hr %#x.\n", hr); + hr = ID2D1PathGeometry_Open(geometry, &sink); + ok(SUCCEEDED(hr), "Failed to open geometry sink, hr %#x.\n", hr); + +#if 0 + set_point(&point, 20.0f, 80.0f); + ID2D1GeometrySink_BeginFigure(sink, point, D2D1_FIGURE_BEGIN_FILLED); + line_to(sink, 40.0f, 100.0f); + quadratic_to(sink, 40.0f, 65.0f, 20.0f, 80.0f); + ID2D1GeometrySink_EndFigure(sink, D2D1_FIGURE_END_OPEN); + + set_point(&point, 80.0f, 20.0f); + ID2D1GeometrySink_BeginFigure(sink, point, D2D1_FIGURE_BEGIN_FILLED); + line_to(sink, 60.0f, 40.0f); + quadratic_to(sink, 95.0f, 40.0f, 80.0f, 20.0f); + ID2D1GeometrySink_EndFigure(sink, D2D1_FIGURE_END_OPEN); +#endif + + set_point(&point, 120.0f, 180.0f); + ID2D1GeometrySink_BeginFigure(sink, point, D2D1_FIGURE_BEGIN_FILLED); + line_to(sink, 140.0f, 160.0f); + quadratic_to(sink, 105.0f, 160.0f, 120.0f, 180.0f); + ID2D1GeometrySink_EndFigure(sink, D2D1_FIGURE_END_CLOSED); + + set_point(&point, 180.0f, 120.0f); + ID2D1GeometrySink_BeginFigure(sink, point, D2D1_FIGURE_BEGIN_FILLED); + line_to(sink, 170.0f, 110.0f); + quadratic_to(sink, 170.0f, 135.0f, 180.0f, 120.0f); + ID2D1GeometrySink_EndFigure(sink, D2D1_FIGURE_END_CLOSED); + + hr = ID2D1GeometrySink_Close(sink); + ok(SUCCEEDED(hr), "Failed to close geometry sink, hr %#x.\n", hr); + ID2D1GeometrySink_Release(sink); + + set_rect(&rect, 0.0f, 0.0f, 0.0f, 0.0f); + hr = ID2D1PathGeometry_GetBounds(geometry, NULL, &rect); + ok(SUCCEEDED(hr), "Failed to get geometry bounds, hr %#x.\n", hr); +#if 0 + match = compare_rect(&rect, 20.0f, 20.0f, 180.0f, 180.0f, 0); +#else + match = compare_rect(&rect, 115.5f, 110.0f, 180.0f, 180.0f, 1); +#endif + ok(match, "Got unexpected rectangle {%.8e, %.8e, %.8e, %.8e}.\n", + rect.left, rect.top, rect.right, rect.bottom); + + geometry_sink_init(&simplify_sink); + hr = ID2D1PathGeometry_Simplify(geometry, D2D1_GEOMETRY_SIMPLIFICATION_OPTION_CUBICS_AND_LINES, + NULL, 0.0f, &simplify_sink.ID2D1SimplifiedGeometrySink_iface); + ok(SUCCEEDED(hr), "Failed to simplify geometry, hr %#x.\n", hr); +#if 0 + geometry_sink_check(&simplify_sink, D2D1_FILL_MODE_ALTERNATE, 4, &expected_figures[29], 1); +#else + geometry_sink_check(&simplify_sink, D2D1_FILL_MODE_ALTERNATE, 2, &expected_figures[31], 1); +#endif + geometry_sink_cleanup(&simplify_sink); + + ID2D1PathGeometry_Release(geometry); + ID2D1SolidColorBrush_Release(brush); ID2D1Factory_Release(factory); release_test_context(&ctx); -- 2.34.1
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=105145 Your paranoid android. === build (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply
This is a preparative patch for the next change, behavior is essentially unchanged, though it may be slightly faster. Rearrange code for outline segment and join generation so it will work for END_OPEN/END_CLOSED path, with coincident and disparate last/first vertex. Each vertex is now fetched once, and pivoted on the next iteration. Also move invariants in front of the loop, Path segments are drawn starting with the first segment, up to vertex_count - 2 (index of start vertex). Only in case of a END_CLOSED figure with non-coincident last/first vertex, also the last line segment is drawn. Joins are added between all drawn segments, and only for END_CLOSED also the join at the first vertex is added. Signed-off-by: Stefan Brüns <stefan.bruens(a)rwth-aachen.de> --- dlls/d2d1/geometry.c | 97 ++++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 45 deletions(-) diff --git a/dlls/d2d1/geometry.c b/dlls/d2d1/geometry.c index a7074899fda..41987771ed4 100644 --- a/dlls/d2d1/geometry.c +++ b/dlls/d2d1/geometry.c @@ -2597,43 +2597,69 @@ static BOOL d2d_geometry_outline_add_arc_quadrant(struct d2d_geometry *geometry, static BOOL d2d_geometry_add_figure_outline(struct d2d_geometry *geometry, struct d2d_figure *figure, D2D1_FIGURE_END figure_end) { - const D2D1_POINT_2F *prev, *p0, *next; - enum d2d_vertex_type prev_type, type; - size_t bezier_idx, i; + const D2D1_POINT_2F *prev, *p0, *p1, *next, *next_prev; + enum d2d_vertex_type type; + size_t bezier_idx, i, vertex_count; + + vertex_count = figure->vertex_count; + /* An OPEN path needs at least 2 vertices, a CLOSED one + * needs at least 1 vertex */ + if (vertex_count < 2 + && !(figure_end == D2D1_FIGURE_END_CLOSED && vertex_count >= 1)) + return TRUE; + + if (figure_end == D2D1_FIGURE_END_OPEN) + vertex_count--; + + prev = p0 = &figure->vertices[0]; + if (figure_end == D2D1_FIGURE_END_CLOSED) + { + /* In case of a CLOSED path, a join between first and last vertex is required */ + if (d2d_vertex_type_is_bezier(figure->vertex_types[vertex_count - 1])) + prev = &figure->bezier_controls[figure->bezier_control_count - 1]; + else + prev = &figure->vertices[vertex_count - 1]; + } - for (i = 0, bezier_idx = 0; i < figure->vertex_count; ++i) + for (i = 0, bezier_idx = 0; i < vertex_count; ++i) { type = figure->vertex_types[i]; if (type == D2D_VERTEX_TYPE_NONE) + { + prev = next_prev = &figure->vertices[i]; continue; + } - p0 = &figure->vertices[i]; - - if (!i) + /* next: tangent along next segment, at p0 + * p1: next vertex */ + if (d2d_vertex_type_is_bezier(type)) { - prev_type = figure->vertex_types[figure->vertex_count - 1]; - if (d2d_vertex_type_is_bezier(prev_type)) - prev = &figure->bezier_controls[figure->bezier_control_count - 1]; - else - prev = &figure->vertices[figure->vertex_count - 1]; + next_prev = next = &figure->bezier_controls[bezier_idx++]; + /* type BEZIER implies i + 1 < figure->vertex_count */ + p1 = &figure->vertices[i + 1]; + + if (!d2d_geometry_outline_add_bezier_segment(geometry, p0, next, p1)) + { + ERR("Failed to add bezier segment.\n"); + return FALSE; + } } else { - prev_type = figure->vertex_types[i - 1]; - if (d2d_vertex_type_is_bezier(prev_type)) - prev = &figure->bezier_controls[bezier_idx - 1]; + if (i + 1 == figure->vertex_count) + next = p1 = &figure->vertices[0]; else - prev = &figure->vertices[i - 1]; - } + next = p1 = &figure->vertices[i + 1]; + next_prev = p0; - if (d2d_vertex_type_is_bezier(type)) - next = &figure->bezier_controls[bezier_idx++]; - else if (i == figure->vertex_count - 1) - next = &figure->vertices[0]; - else - next = &figure->vertices[i + 1]; + if (!d2d_geometry_outline_add_line_segment(geometry, p0, p1)) + { + ERR("Failed to add line segment.\n"); + return FALSE; + } + } - if (figure_end == D2D1_FIGURE_END_CLOSED || (i && i < figure->vertex_count - 1)) + if (i || figure_end == D2D1_FIGURE_END_CLOSED) { D2D1_POINT_2F q_next, q_prev; @@ -2650,27 +2676,8 @@ static BOOL d2d_geometry_add_figure_outline(struct d2d_geometry *geometry, } } - if (type == D2D_VERTEX_TYPE_LINE && (figure_end == D2D1_FIGURE_END_CLOSED || i < figure->vertex_count - 1) - && !d2d_geometry_outline_add_line_segment(geometry, p0, next)) - { - ERR("Failed to add line segment.\n"); - return FALSE; - } - else if (d2d_vertex_type_is_bezier(type)) - { - const D2D1_POINT_2F *p2; - - if (i == figure->vertex_count - 1) - p2 = &figure->vertices[0]; - else - p2 = &figure->vertices[i + 1]; - - if (!d2d_geometry_outline_add_bezier_segment(geometry, p0, next, p2)) - { - ERR("Failed to add bezier segment.\n"); - return FALSE; - } - } + p0 = p1; + prev = next_prev; } return TRUE; -- 2.34.1
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=105146 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply
When the last vertex is coincident with the first vertex, the last segment should be suppressed for both END_OPEN and END_CLOSED. Only when last and first vertex are not coincident the additional line segment may be added - always for intersection tests and similar, and for stroking opereations when the figure is CLOSED. Trying to use an zero-length segment in d2d_geometry_intersect_self will create invalid segments, causing infinite loops later. Instead of reducing the vertex_count for cooincident first/last vertices add a dedicated type. This is required as some operations need the last segment, others do not. This also allows to remove some replicated code in StrokeContains/GetBounds/Simplify, as a last Bezier segment is always processed in the regular loop. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51139 Signed-off-by: Stefan Brüns <stefan.bruens(a)rwth-aachen.de> --- dlls/d2d1/geometry.c | 97 ++++++++++++-------------------------------- 1 file changed, 25 insertions(+), 72 deletions(-) diff --git a/dlls/d2d1/geometry.c b/dlls/d2d1/geometry.c index 41987771ed4..d60ea4e4afd 100644 --- a/dlls/d2d1/geometry.c +++ b/dlls/d2d1/geometry.c @@ -50,6 +50,7 @@ enum d2d_vertex_type D2D_VERTEX_TYPE_LINE, D2D_VERTEX_TYPE_BEZIER, D2D_VERTEX_TYPE_SPLIT_BEZIER, + D2D_VERTEX_TYPE_END, }; struct d2d_segment_idx @@ -2220,6 +2221,9 @@ static BOOL d2d_geometry_intersect_self(struct d2d_geometry *geometry) for (idx_p.vertex_idx = 0; idx_p.vertex_idx < figure_p->vertex_count; ++idx_p.vertex_idx) { type_p = figure_p->vertex_types[idx_p.vertex_idx]; + if (type_p == D2D_VERTEX_TYPE_END) + continue; + for (idx_q.figure_idx = 0; idx_q.figure_idx <= idx_p.figure_idx; ++idx_q.figure_idx) { figure_q = &geometry->u.path.figures[idx_q.figure_idx]; @@ -2228,6 +2232,8 @@ static BOOL d2d_geometry_intersect_self(struct d2d_geometry *geometry) if (!d2d_rect_check_overlap(&figure_p->bounds, &figure_q->bounds)) continue; max_q = figure_q->vertex_count; + if (max_q && figure_q->vertex_types[max_q - 1] == D2D_VERTEX_TYPE_END) + max_q--; } else { @@ -2608,7 +2614,8 @@ static BOOL d2d_geometry_add_figure_outline(struct d2d_geometry *geometry, && !(figure_end == D2D1_FIGURE_END_CLOSED && vertex_count >= 1)) return TRUE; - if (figure_end == D2D1_FIGURE_END_OPEN) + if (figure->vertex_types[vertex_count - 1] == D2D_VERTEX_TYPE_END + || figure_end == D2D1_FIGURE_END_OPEN) vertex_count--; prev = p0 = &figure->vertices[0]; @@ -2919,13 +2926,15 @@ static void STDMETHODCALLTYPE d2d_geometry_sink_EndFigure(ID2D1GeometrySink *ifa } figure = &geometry->u.path.figures[geometry->u.path.figure_count - 1]; - figure->vertex_types[figure->vertex_count - 1] = D2D_VERTEX_TYPE_LINE; if (figure_end == D2D1_FIGURE_END_CLOSED) { ++geometry->u.path.segment_count; figure->flags |= D2D_FIGURE_FLAG_CLOSED; - if (!memcmp(&figure->vertices[0], &figure->vertices[figure->vertex_count - 1], sizeof(*figure->vertices))) - --figure->vertex_count; + } + figure->vertex_types[figure->vertex_count - 1] = D2D_VERTEX_TYPE_END; + if (memcmp(&figure->vertices[0], &figure->vertices[figure->vertex_count - 1], sizeof(*figure->vertices))) + { + figure->vertex_types[figure->vertex_count - 1] = D2D_VERTEX_TYPE_LINE; } if (!d2d_geometry_add_figure_outline(geometry, figure, figure_end)) @@ -3474,14 +3483,6 @@ static HRESULT STDMETHODCALLTYPE d2d_path_geometry_GetBounds(ID2D1PathGeometry * if (figure->flags & D2D_FIGURE_FLAG_HOLLOW) continue; - /* Single vertex figures are reduced by CloseFigure(). */ - if (figure->vertex_count == 0) - { - d2d_point_transform(&p, transform, figure->bounds.left, figure->bounds.top); - d2d_rect_expand(bounds, &p); - continue; - } - for (j = 0; j < figure->vertex_count; ++j) { if (figure->vertex_types[j] == D2D_VERTEX_TYPE_NONE) @@ -3496,8 +3497,8 @@ static HRESULT STDMETHODCALLTYPE d2d_path_geometry_GetBounds(ID2D1PathGeometry * for (bezier_idx = 0, ++j; j < figure->vertex_count; ++j) { - if (figure->vertex_types[j] == D2D_VERTEX_TYPE_NONE - || d2d_vertex_type_is_split_bezier(figure->vertex_types[j])) + enum d2d_vertex_type next_type = figure->vertex_types[j]; + if (next_type == D2D_VERTEX_TYPE_NONE || d2d_vertex_type_is_split_bezier(next_type)) continue; switch (type) @@ -3535,26 +3536,7 @@ static HRESULT STDMETHODCALLTYPE d2d_path_geometry_GetBounds(ID2D1PathGeometry * break; } - type = figure->vertex_types[j]; - } - - if (d2d_vertex_type_is_bezier(type)) - { - /* FIXME: This attempts to approximate a cubic Bézier with - * a quadratic one. */ - p1 = figure->original_bezier_controls[bezier_idx++]; - d2d_point_transform(&p1, transform, p1.x, p1.y); - p2 = figure->original_bezier_controls[bezier_idx++]; - d2d_point_transform(&p2, transform, p2.x, p2.y); - p1.x = (p1.x + p2.x) * 0.75f; - p1.y = (p1.y + p2.y) * 0.75f; - p2 = figure->vertices[0]; - d2d_point_transform(&p2, transform, p2.x, p2.y); - p1.x -= (p.x + p2.x) * 0.25f; - p1.y -= (p.y + p2.y) * 0.25f; - - d2d_rect_get_bezier_bounds(&bezier_bounds, &p, &p1, &p2); - d2d_rect_union(bounds, &bezier_bounds); + type = next_type; } } @@ -3617,8 +3599,8 @@ static HRESULT STDMETHODCALLTYPE d2d_path_geometry_StrokeContainsPoint(ID2D1Path for (bezier_idx = 0, ++j; j < figure->vertex_count; ++j) { - if (figure->vertex_types[j] == D2D_VERTEX_TYPE_NONE - || d2d_vertex_type_is_split_bezier(figure->vertex_types[j])) + enum d2d_vertex_type next_type = figure->vertex_types[j]; + if (next_type == D2D_VERTEX_TYPE_NONE || d2d_vertex_type_is_split_bezier(next_type)) continue; switch (type) @@ -3644,25 +3626,14 @@ static HRESULT STDMETHODCALLTYPE d2d_path_geometry_StrokeContainsPoint(ID2D1Path } if (*contains) return S_OK; - type = figure->vertex_types[j]; + type = next_type; } - if (figure->flags & D2D_FIGURE_FLAG_CLOSED && (!*contains)) + if (type == D2D_VERTEX_TYPE_LINE) { - if (type == D2D_VERTEX_TYPE_LINE) - { - p1 = figure->vertices[0]; + p1 = figure->vertices[0]; + if (figure->flags & D2D_FIGURE_FLAG_CLOSED) *contains = d2d_point_on_line_segment(&point, &p, &p1, transform, stroke_width * 0.5f, tolerance); - p = p1; - } - else if (d2d_vertex_type_is_bezier(type)) - { - b.point1 = figure->original_bezier_controls[bezier_idx++]; - b.point2 = figure->original_bezier_controls[bezier_idx++]; - b.point3 = figure->vertices[0]; - *contains = d2d_point_on_bezier_segment(&point, &p, &b, transform, stroke_width, tolerance); - p = b.point3; - } } if (*contains) @@ -3785,8 +3756,8 @@ static HRESULT STDMETHODCALLTYPE d2d_path_geometry_Simplify(ID2D1PathGeometry *i for (bezier_idx = 0, ++j; j < figure->vertex_count; ++j) { - if (figure->vertex_types[j] == D2D_VERTEX_TYPE_NONE - || d2d_vertex_type_is_split_bezier(figure->vertex_types[j])) + enum d2d_vertex_type next_type = figure->vertex_types[j]; + if (next_type == D2D_VERTEX_TYPE_NONE || d2d_vertex_type_is_split_bezier(next_type)) continue; switch (type) @@ -3825,25 +3796,7 @@ static HRESULT STDMETHODCALLTYPE d2d_path_geometry_Simplify(ID2D1PathGeometry *i break; } - type = figure->vertex_types[j]; - } - - if (d2d_vertex_type_is_bezier(type)) - { - b.point1 = figure->original_bezier_controls[bezier_idx++]; - b.point2 = figure->original_bezier_controls[bezier_idx++]; - b.point3 = figure->vertices[0]; - if (transform) - { - d2d_point_transform(&b.point1, transform, b.point1.x, b.point1.y); - d2d_point_transform(&b.point2, transform, b.point2.x, b.point2.y); - d2d_point_transform(&b.point3, transform, b.point3.x, b.point3.y); - } - - if (option == D2D1_GEOMETRY_SIMPLIFICATION_OPTION_LINES) - d2d_geometry_flatten_cubic(sink, &p, &b, tolerance); - else - ID2D1SimplifiedGeometrySink_AddBeziers(sink, &b, 1); + type = next_type; } end = figure->flags & D2D_FIGURE_FLAG_CLOSED ? D2D1_FIGURE_END_CLOSED : D2D1_FIGURE_END_OPEN; -- 2.34.1
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=105147 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51139 Signed-off-by: Stefan Brüns <stefan.bruens(a)rwth-aachen.de> --- dlls/d2d1/tests/d2d1.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 9f2c432c55e..5efa87bd89c 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -3898,7 +3898,6 @@ static void test_path_geometry(BOOL d3d11) hr = ID2D1PathGeometry_Open(geometry, &sink); ok(SUCCEEDED(hr), "Failed to open geometry sink, hr %#x.\n", hr); -#if 0 set_point(&point, 20.0f, 80.0f); ID2D1GeometrySink_BeginFigure(sink, point, D2D1_FIGURE_BEGIN_FILLED); line_to(sink, 40.0f, 100.0f); @@ -3910,7 +3909,6 @@ static void test_path_geometry(BOOL d3d11) line_to(sink, 60.0f, 40.0f); quadratic_to(sink, 95.0f, 40.0f, 80.0f, 20.0f); ID2D1GeometrySink_EndFigure(sink, D2D1_FIGURE_END_OPEN); -#endif set_point(&point, 120.0f, 180.0f); ID2D1GeometrySink_BeginFigure(sink, point, D2D1_FIGURE_BEGIN_FILLED); @@ -3931,11 +3929,7 @@ static void test_path_geometry(BOOL d3d11) set_rect(&rect, 0.0f, 0.0f, 0.0f, 0.0f); hr = ID2D1PathGeometry_GetBounds(geometry, NULL, &rect); ok(SUCCEEDED(hr), "Failed to get geometry bounds, hr %#x.\n", hr); -#if 0 match = compare_rect(&rect, 20.0f, 20.0f, 180.0f, 180.0f, 0); -#else - match = compare_rect(&rect, 115.5f, 110.0f, 180.0f, 180.0f, 1); -#endif ok(match, "Got unexpected rectangle {%.8e, %.8e, %.8e, %.8e}.\n", rect.left, rect.top, rect.right, rect.bottom); @@ -3943,11 +3937,7 @@ static void test_path_geometry(BOOL d3d11) hr = ID2D1PathGeometry_Simplify(geometry, D2D1_GEOMETRY_SIMPLIFICATION_OPTION_CUBICS_AND_LINES, NULL, 0.0f, &simplify_sink.ID2D1SimplifiedGeometrySink_iface); ok(SUCCEEDED(hr), "Failed to simplify geometry, hr %#x.\n", hr); -#if 0 geometry_sink_check(&simplify_sink, D2D1_FILL_MODE_ALTERNATE, 4, &expected_figures[29], 1); -#else - geometry_sink_check(&simplify_sink, D2D1_FILL_MODE_ALTERNATE, 2, &expected_figures[31], 1); -#endif geometry_sink_cleanup(&simplify_sink); ID2D1PathGeometry_Release(geometry); -- 2.34.1
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=105148 Your paranoid android. === build (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply
The very last vertex of a figure can only be a of type LINE (non-coincident last/first vertices) or type END (otherwise). In case the current vertex starts a bezier path, there is always at least one more vertex, i.e. (vertex_idx + 1) < vertex_count is always true. Signed-off-by: Stefan Brüns <stefan.bruens(a)rwth-aachen.de> --- dlls/d2d1/geometry.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/dlls/d2d1/geometry.c b/dlls/d2d1/geometry.c index d60ea4e4afd..61952fdf782 100644 --- a/dlls/d2d1/geometry.c +++ b/dlls/d2d1/geometry.c @@ -2007,8 +2007,6 @@ static BOOL d2d_geometry_intersect_bezier_line(struct d2d_geometry *geometry, p[0] = &figure->vertices[idx_p->vertex_idx]; p[1] = &figure->bezier_controls[idx_p->control_idx]; next = idx_p->vertex_idx + 1; - if (next == figure->vertex_count) - next = 0; p[2] = &figure->vertices[next]; figure = &geometry->u.path.figures[idx_q->figure_idx]; @@ -2085,16 +2083,12 @@ static BOOL d2d_geometry_intersect_bezier_bezier(struct d2d_geometry *geometry, p[0] = &figure->vertices[idx_p->vertex_idx]; p[1] = &figure->bezier_controls[idx_p->control_idx]; next = idx_p->vertex_idx + 1; - if (next == figure->vertex_count) - next = 0; p[2] = &figure->vertices[next]; figure = &geometry->u.path.figures[idx_q->figure_idx]; q[0] = &figure->vertices[idx_q->vertex_idx]; q[1] = &figure->bezier_controls[idx_q->control_idx]; next = idx_q->vertex_idx + 1; - if (next == figure->vertex_count) - next = 0; q[2] = &figure->vertices[next]; d2d_rect_get_bezier_segment_bounds(&p_bounds, p[0], p[1], p[2], start_p, end_p); @@ -2178,8 +2172,6 @@ static BOOL d2d_geometry_apply_intersections(struct d2d_geometry *geometry, p[0] = &figure->vertices[inter->vertex_idx + vertex_offset]; p[1] = &figure->bezier_controls[inter->control_idx + control_offset]; next = inter->vertex_idx + vertex_offset + 1; - if (next == figure->vertex_count) - next = 0; p[2] = &figure->vertices[next]; d2d_point_lerp(&q[0], p[0], p[1], t); @@ -3014,18 +3006,12 @@ static BOOL d2d_geometry_check_bezier_overlap(struct d2d_geometry *geometry, figure = &geometry->u.path.figures[idx_p->figure_idx]; a[0] = &figure->vertices[idx_p->vertex_idx]; a[1] = &figure->bezier_controls[idx_p->control_idx]; - if (idx_p->vertex_idx == figure->vertex_count - 1) - a[2] = &figure->vertices[0]; - else - a[2] = &figure->vertices[idx_p->vertex_idx + 1]; + a[2] = &figure->vertices[idx_p->vertex_idx + 1]; figure = &geometry->u.path.figures[idx_q->figure_idx]; b[0] = &figure->vertices[idx_q->vertex_idx]; b[1] = &figure->bezier_controls[idx_q->control_idx]; - if (idx_q->vertex_idx == figure->vertex_count - 1) - b[2] = &figure->vertices[0]; - else - b[2] = &figure->vertices[idx_q->vertex_idx + 1]; + b[2] = &figure->vertices[idx_q->vertex_idx + 1]; if (d2d_point_ccw(a[0], a[1], a[2]) == 0.0f || d2d_point_ccw(b[0], b[1], b[2]) == 0.0f) return FALSE; @@ -3095,9 +3081,6 @@ static float d2d_geometry_bezier_ccw(struct d2d_geometry *geometry, const struct const struct d2d_figure *figure = &geometry->u.path.figures[idx->figure_idx]; size_t next = idx->vertex_idx + 1; - if (next == figure->vertex_count) - next = 0; - return d2d_point_ccw(&figure->vertices[idx->vertex_idx], &figure->bezier_controls[idx->control_idx], &figure->vertices[next]); } @@ -3113,8 +3096,6 @@ static BOOL d2d_geometry_split_bezier(struct d2d_geometry *geometry, const struc p[0] = &figure->vertices[idx->vertex_idx]; p[1] = &figure->bezier_controls[idx->control_idx]; next = idx->vertex_idx + 1; - if (next == figure->vertex_count) - next = 0; p[2] = &figure->vertices[next]; d2d_point_lerp(&q[0], p[0], p[1], 0.5f); -- 2.34.1
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=105149 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply === debian11 (build log) === error: patch failed: dlls/d2d1/tests/d2d1.c:10253 Task: Patch failed to apply
On Tue, 11 Jan 2022 at 08:55, Stefan Brüns <stefan.bruens(a)rwth-aachen.de> wrote:
This is an attempt to fix https://bugs.winehq.org/show_bug.cgi?id=51139
Thanks! I've sent a slightly modified version of these patches.
participants (4)
-
Henri Verbeet -
Henri Verbeet -
Marvin -
Stefan Brüns