Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] skip lineTo for empty contours. #52290

Merged
auto-submit merged 7 commits into from
Apr 22, 2024
Merged

[Impeller] skip lineTo for empty contours. #52290

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0aca046
[Impeller] skip lineTo for empty contours.
Apr 21, 2024
550c572
still close contour.
Apr 21, 2024
be07b58
update testing script.
Apr 22, 2024
ebddde6
Update aiks_unittests.cc
Apr 22, 2024
70d4d93
Merge branch 'main' into move_contours
Apr 22, 2024
bfdc1f4
add more unittests
Apr 22, 2024
ec2ba73
Merge branch 'move_contours' of github.com:jonahwilliams/engine into …
Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions impeller/aiks/aiks_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 6 additions & 1 deletion impeller/geometry/path_builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 10 additions & 10 deletions impeller/geometry/path_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,26 +47,26 @@ 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);
}

{
Path path =
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);
}

{
Path path =
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);
}

{
Expand All @@ -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);
}

{
Expand All @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions testing/impeller_golden_tests_output.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down