From 678bbc27da1644e664ec55cbc8406621661a92de Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 5 Mar 2024 16:24:58 -0800 Subject: [PATCH] [Impeller] Fix path winding when bridging from contours with an odd number of points. --- impeller/aiks/aiks_path_unittests.cc | 48 +++++++++++++------ impeller/tessellator/tessellator.cc | 8 ++-- impeller/tessellator/tessellator_unittests.cc | 8 ++-- 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/impeller/aiks/aiks_path_unittests.cc b/impeller/aiks/aiks_path_unittests.cc index a1f85dee47484..85d617856c533 100644 --- a/impeller/aiks/aiks_path_unittests.cc +++ b/impeller/aiks/aiks_path_unittests.cc @@ -412,20 +412,40 @@ TEST_P(AiksTest, CanRenderOverlappingMultiContourPath) { const Scalar kTriangleHeight = 100; canvas.Translate(Vector2(200, 200)); - // Form a path similar to the Material drop slider value indicator. - auto path = - PathBuilder{} - .MoveTo({0, kTriangleHeight}) - .LineTo({-kTriangleHeight / 2.0f, 0}) - .LineTo({kTriangleHeight / 2.0f, 0}) - .Close() - .AddRoundedRect( - Rect::MakeXYWH(-kTriangleHeight / 2.0f, -kTriangleHeight / 2.0f, - kTriangleHeight, kTriangleHeight), - radii) - .TakePath(); - - canvas.DrawPath(path, paint); + // Form a path similar to the Material drop slider value indicator. Both + // shapes should render identically side-by-side. + { + auto path = + PathBuilder{} + .MoveTo({0, kTriangleHeight}) + .LineTo({-kTriangleHeight / 2.0f, 0}) + .LineTo({kTriangleHeight / 2.0f, 0}) + .Close() + .AddRoundedRect( + Rect::MakeXYWH(-kTriangleHeight / 2.0f, -kTriangleHeight / 2.0f, + kTriangleHeight, kTriangleHeight), + radii) + .TakePath(); + + canvas.DrawPath(path, paint); + } + canvas.Translate(Vector2(100, 0)); + { + auto path = + PathBuilder{} + .MoveTo({0, kTriangleHeight}) + .LineTo({-kTriangleHeight / 2.0f, 0}) + .LineTo({0, -10}) + .LineTo({kTriangleHeight / 2.0f, 0}) + .Close() + .AddRoundedRect( + Rect::MakeXYWH(-kTriangleHeight / 2.0f, -kTriangleHeight / 2.0f, + kTriangleHeight, kTriangleHeight), + radii) + .TakePath(); + + canvas.DrawPath(path, paint); + } ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index e771c6dff9e84..40a1a542ab761 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -212,15 +212,15 @@ std::vector Tessellator::TessellateConvex(const Path& path, size_t a = start + 1; size_t b = end - 1; - while (a < b) { + while (a <= b) { + // If the contour has an odd number of points, two identical points will + // be appended when a == b. This ensures the triangle winding order will + // remain the same after bridging to the next contour. output.emplace_back(polyline.GetPoint(a)); output.emplace_back(polyline.GetPoint(b)); a++; b--; } - if (a == b) { - output.emplace_back(polyline.GetPoint(a)); - } } return output; } diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc index 7bbe1e314633c..d01a3ba33b2c3 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -112,7 +112,7 @@ TEST(TessellatorTest, TessellateConvex) { PathBuilder{}.AddRect(Rect::MakeLTRB(0, 0, 10, 10)).TakePath(), 1.0); std::vector expected = { - {0, 0}, {10, 0}, {0, 10}, {10, 10}, // + {0, 0}, {10, 0}, {0, 10}, {10, 10}, {10, 10}, // }; EXPECT_EQ(pts, expected); } @@ -125,9 +125,9 @@ TEST(TessellatorTest, TessellateConvex) { .TakePath(), 1.0); - std::vector expected = {{0, 0}, {10, 0}, {0, 10}, {10, 10}, - {10, 10}, {20, 20}, {20, 20}, {20, 20}, - {30, 20}, {20, 30}, {30, 30}}; + std::vector expected = { + {0, 0}, {10, 0}, {0, 10}, {10, 10}, {10, 10}, {10, 10}, {20, 20}, + {20, 20}, {20, 20}, {30, 20}, {20, 30}, {30, 30}, {30, 30}}; EXPECT_EQ(pts, expected); } }