diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 6e4cde04a925e..17e3f1b5a3bfc 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3135,6 +3135,30 @@ 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}) + // .CubicCurveTo({10, 10}, {-10, -10}, {4, 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::Red(), + .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..da193e1a58e87 100644 --- a/impeller/geometry/path_builder.cc +++ b/impeller/geometry/path_builder.cc @@ -20,6 +20,7 @@ Path PathBuilder::CopyPath(FillType fill) { } Path PathBuilder::TakePath(FillType fill) { + last_verb_close_ = false; prototype_.fill = fill; UpdateBounds(); return Path(std::move(prototype_)); @@ -33,11 +34,19 @@ void PathBuilder::Reserve(size_t point_size, size_t verb_size) { PathBuilder& PathBuilder::MoveTo(Point point, bool relative) { current_ = relative ? current_ + point : point; subpath_start_ = current_; + if (last_verb_close_) { + return *this; + } + last_verb_close_ = true; AddContourComponent(current_); return *this; } PathBuilder& PathBuilder::Close() { + if (last_verb_close_) { + return *this; + } + last_verb_close_ = true; LineTo(subpath_start_); SetContourClosed(true); AddContourComponent(current_); @@ -45,6 +54,7 @@ PathBuilder& PathBuilder::Close() { } PathBuilder& PathBuilder::LineTo(Point point, bool relative) { + last_verb_close_ = false; point = relative ? current_ + point : point; AddLinearComponent(current_, point); current_ = point; @@ -52,6 +62,7 @@ PathBuilder& PathBuilder::LineTo(Point point, bool relative) { } PathBuilder& PathBuilder::HorizontalLineTo(Scalar x, bool relative) { + last_verb_close_ = false; Point endpoint = relative ? Point{current_.x + x, current_.y} : Point{x, current_.y}; AddLinearComponent(current_, endpoint); @@ -60,6 +71,7 @@ PathBuilder& PathBuilder::HorizontalLineTo(Scalar x, bool relative) { } PathBuilder& PathBuilder::VerticalLineTo(Scalar y, bool relative) { + last_verb_close_ = false; Point endpoint = relative ? Point{current_.x, current_.y + y} : Point{current_.x, y}; AddLinearComponent(current_, endpoint); @@ -70,6 +82,7 @@ PathBuilder& PathBuilder::VerticalLineTo(Scalar y, bool relative) { PathBuilder& PathBuilder::QuadraticCurveTo(Point controlPoint, Point point, bool relative) { + last_verb_close_ = false; point = relative ? current_ + point : point; controlPoint = relative ? current_ + controlPoint : controlPoint; AddQuadraticComponent(current_, controlPoint, point); @@ -86,6 +99,7 @@ PathBuilder& PathBuilder::CubicCurveTo(Point controlPoint1, Point controlPoint2, Point point, bool relative) { + last_verb_close_ = false; controlPoint1 = relative ? current_ + controlPoint1 : controlPoint1; controlPoint2 = relative ? current_ + controlPoint2 : controlPoint2; point = relative ? current_ + point : point; @@ -97,6 +111,7 @@ PathBuilder& PathBuilder::CubicCurveTo(Point controlPoint1, PathBuilder& PathBuilder::AddQuadraticCurve(Point p1, Point cp, Point p2) { MoveTo(p1); AddQuadraticComponent(p1, cp, p2); + last_verb_close_ = false; return *this; } @@ -106,10 +121,12 @@ PathBuilder& PathBuilder::AddCubicCurve(Point p1, Point p2) { MoveTo(p1); AddCubicComponent(p1, cp1, cp2, p2); + last_verb_close_ = false; return *this; } PathBuilder& PathBuilder::AddRect(Rect rect) { + last_verb_close_ = false; auto origin = rect.GetOrigin(); auto size = rect.GetSize(); @@ -128,21 +145,25 @@ PathBuilder& PathBuilder::AddRect(Rect rect) { } PathBuilder& PathBuilder::AddCircle(const Point& c, Scalar r) { + last_verb_close_ = false; return AddOval(Rect::MakeXYWH(c.x - r, c.y - r, 2.0f * r, 2.0f * r)); } PathBuilder& PathBuilder::AddRoundedRect(Rect rect, Scalar radius) { + last_verb_close_ = false; return radius <= 0.0 ? AddRect(rect) : AddRoundedRect(rect, RoundingRadii(radius)); } PathBuilder& PathBuilder::AddRoundedRect(Rect rect, Size radii) { + last_verb_close_ = false; return radii.width <= 0 || radii.height <= 0 ? AddRect(rect) : AddRoundedRect(rect, RoundingRadii(radii)); } PathBuilder& PathBuilder::AddRoundedRect(Rect rect, RoundingRadii radii) { + last_verb_close_ = false; if (radii.AreAllZero()) { return AddRect(rect); } diff --git a/impeller/geometry/path_builder.h b/impeller/geometry/path_builder.h index 0907b7b67f0f4..53b6ed15cb27e 100644 --- a/impeller/geometry/path_builder.h +++ b/impeller/geometry/path_builder.h @@ -157,6 +157,10 @@ class PathBuilder { Point subpath_start_; Point current_; Path::Data prototype_; + // Because we don't have an explicit close verb we can't distinguish between + // A series of repeated close verbs today. This is a workaround to allow + // us to fix flutter/flutter#146648 until we remove impeller::Path entirely. + bool last_verb_close_ = false; PathBuilder& AddRoundedRectTopLeft(Rect rect, RoundingRadii radii); diff --git a/testing/impeller_golden_tests_output.txt b/testing/impeller_golden_tests_output.txt index 688d0d6b9cc09..4f631de253ba5 100644 --- a/testing/impeller_golden_tests_output.txt +++ b/testing/impeller_golden_tests_output.txt @@ -726,6 +726,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_StrokedClosedPathDrawnCorrectly_Metal.png +impeller_Play_AiksTest_StrokedClosedPathDrawnCorrectly_OpenGLES.png +impeller_Play_AiksTest_StrokedClosedPathDrawnCorrectly_Vulkan.png impeller_Play_AiksTest_SubpassWithClearColorOptimization_Metal.png impeller_Play_AiksTest_SubpassWithClearColorOptimization_OpenGLES.png impeller_Play_AiksTest_SubpassWithClearColorOptimization_Vulkan.png