[Impeller] ensure Path.close does not insert lineTo for stroked paths.

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
 

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,25 +34,35 @@ 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_);
   return *this;
 }
 
 PathBuilder& PathBuilder::LineTo(Point point, bool relative) {
+  last_verb_close_ = false;
   point = relative ? current_ + point : point;
   AddLinearComponent(current_, point);
   current_ = point;
   return *this;
 }
 
 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);
   }

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);
 

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