diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 0499459fde694..abd0eeb72ae33 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3145,6 +3145,29 @@ TEST_P(AiksTest, DrawAtlasPlusWideGamut) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +// https://github.com/flutter/flutter/issues/146648 +TEST_P(AiksTest, StrokedPathWithMoveToThenCloseDrawnCorrectly) { + Path path = PathBuilder{} + .MoveTo({0, 400}) + .LineTo({0, 0}) + .LineTo({400, 0}) + // MoveTo implicitly adds a contour, ensure that close doesn't + // add another nearly-empty contour. + .MoveTo({0, 400}) + .Close() + .TakePath(); + + Canvas canvas; + canvas.Translate({50, 50, 0}); + canvas.DrawPath(path, { + .color = Color::Blue(), + .stroke_width = 10, + .stroke_cap = Cap::kRound, + .style = Paint::Style::kStroke, + }); + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + } // namespace testing } // namespace impeller diff --git a/impeller/geometry/path_builder.cc b/impeller/geometry/path_builder.cc index 5cc9e28127cb3..be591d92884c7 100644 --- a/impeller/geometry/path_builder.cc +++ b/impeller/geometry/path_builder.cc @@ -38,7 +38,12 @@ PathBuilder& PathBuilder::MoveTo(Point point, bool relative) { } PathBuilder& PathBuilder::Close() { - LineTo(subpath_start_); + // If the subpath start is the same as the current position, this + // is an empty contour and inserting a line segment will just + // confuse the tessellator. + if (subpath_start_ != current_) { + LineTo(subpath_start_); + } SetContourClosed(true); AddContourComponent(current_); return *this; diff --git a/impeller/geometry/path_unittests.cc b/impeller/geometry/path_unittests.cc index 6ffd69a1de56a..6015bb264a176 100644 --- a/impeller/geometry/path_unittests.cc +++ b/impeller/geometry/path_unittests.cc @@ -47,8 +47,8 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) { Path path = PathBuilder{}.AddCircle({100, 100}, 50).TakePath(); ContourComponent contour; path.GetContourComponentAtIndex(0, contour); - ASSERT_POINT_NEAR(contour.destination, Point(100, 50)); - ASSERT_TRUE(contour.is_closed); + EXPECT_POINT_NEAR(contour.destination, Point(100, 50)); + EXPECT_TRUE(contour.is_closed); } { @@ -56,8 +56,8 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) { PathBuilder{}.AddOval(Rect::MakeXYWH(100, 100, 100, 100)).TakePath(); ContourComponent contour; path.GetContourComponentAtIndex(0, contour); - ASSERT_POINT_NEAR(contour.destination, Point(150, 100)); - ASSERT_TRUE(contour.is_closed); + EXPECT_POINT_NEAR(contour.destination, Point(150, 100)); + EXPECT_TRUE(contour.is_closed); } { @@ -65,8 +65,8 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) { PathBuilder{}.AddRect(Rect::MakeXYWH(100, 100, 100, 100)).TakePath(); ContourComponent contour; path.GetContourComponentAtIndex(0, contour); - ASSERT_POINT_NEAR(contour.destination, Point(100, 100)); - ASSERT_TRUE(contour.is_closed); + EXPECT_POINT_NEAR(contour.destination, Point(100, 100)); + EXPECT_TRUE(contour.is_closed); } { @@ -75,8 +75,8 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) { .TakePath(); ContourComponent contour; path.GetContourComponentAtIndex(0, contour); - ASSERT_POINT_NEAR(contour.destination, Point(110, 100)); - ASSERT_TRUE(contour.is_closed); + EXPECT_POINT_NEAR(contour.destination, Point(110, 100)); + EXPECT_TRUE(contour.is_closed); } { @@ -86,8 +86,8 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) { .TakePath(); ContourComponent contour; path.GetContourComponentAtIndex(0, contour); - ASSERT_POINT_NEAR(contour.destination, Point(110, 100)); - ASSERT_TRUE(contour.is_closed); + EXPECT_POINT_NEAR(contour.destination, Point(110, 100)); + EXPECT_TRUE(contour.is_closed); } // Open shapes. @@ -454,6 +454,34 @@ TEST(PathTest, SimplePath) { }); } +TEST(PathTest, RepeatCloseDoesNotAddNewLines) { + PathBuilder builder; + auto path = builder.LineTo({0, 10}) + .LineTo({10, 10}) + .Close() // Returns to (0, 0) + .Close() // No Op + .Close() // Still No op + .TakePath(); + + EXPECT_EQ(path.GetComponentCount(), 5u); + EXPECT_EQ(path.GetComponentCount(Path::ComponentType::kLinear), 3u); + EXPECT_EQ(path.GetComponentCount(Path::ComponentType::kContour), 2u); +} + +TEST(PathTest, CloseAfterMoveDoesNotAddNewLines) { + PathBuilder builder; + auto path = builder.LineTo({0, 10}) + .LineTo({10, 10}) + .MoveTo({30, 30}) // Moves to (30, 30) + .Close() // No Op + .Close() // Still No op + .TakePath(); + + EXPECT_EQ(path.GetComponentCount(), 4u); + EXPECT_EQ(path.GetComponentCount(Path::ComponentType::kLinear), 2u); + EXPECT_EQ(path.GetComponentCount(Path::ComponentType::kContour), 2u); +} + TEST(PathTest, CanBeCloned) { PathBuilder builder; builder.MoveTo({10, 10}); diff --git a/testing/impeller_golden_tests_output.txt b/testing/impeller_golden_tests_output.txt index f18c299b3ce94..491482dabf944 100644 --- a/testing/impeller_golden_tests_output.txt +++ b/testing/impeller_golden_tests_output.txt @@ -718,6 +718,9 @@ impeller_Play_AiksTest_SrgbToLinearFilterSubpassCollapseOptimization_Vulkan.png impeller_Play_AiksTest_StrokedCirclesRenderCorrectly_Metal.png impeller_Play_AiksTest_StrokedCirclesRenderCorrectly_OpenGLES.png impeller_Play_AiksTest_StrokedCirclesRenderCorrectly_Vulkan.png +impeller_Play_AiksTest_StrokedPathWithMoveToThenCloseDrawnCorrectly_Metal.png +impeller_Play_AiksTest_StrokedPathWithMoveToThenCloseDrawnCorrectly_OpenGLES.png +impeller_Play_AiksTest_StrokedPathWithMoveToThenCloseDrawnCorrectly_Vulkan.png impeller_Play_AiksTest_SubpassWithClearColorOptimization_Metal.png impeller_Play_AiksTest_SubpassWithClearColorOptimization_OpenGLES.png impeller_Play_AiksTest_SubpassWithClearColorOptimization_Vulkan.png