From 462fa724a2aaadf95992a5f1e377af6134e5cdb6 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Sat, 7 Sep 2024 09:24:30 -0700
Subject: [PATCH 01/14] [Impeller] use 2 vecs instead of 3.

---
 impeller/geometry/path.cc           | 265 ++++++++++++----------------
 impeller/geometry/path.h            |  39 ++--
 impeller/geometry/path_builder.cc   | 112 +++++++-----
 impeller/geometry/path_builder.h    |   1 +
 impeller/geometry/path_component.cc |  48 -----
 impeller/geometry/path_component.h  |  36 +---
 impeller/geometry/path_unittests.cc |  78 +-------
 7 files changed, 220 insertions(+), 359 deletions(-)

diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index 9db3663c09f89..7f20010ded1ba 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -37,11 +37,11 @@ size_t Path::GetComponentCount(std::optional<ComponentType> type) const {
   }
   auto type_value = type.value();
   if (type_value == ComponentType::kContour) {
-    return data_->contours.size();
+    return data_->components.size();
   }
   size_t count = 0u;
   for (const auto& component : data_->components) {
-    if (component.type == type_value) {
+    if (component == type_value) {
       count++;
     }
   }
@@ -60,63 +60,21 @@ bool Path::IsEmpty() const {
   return data_->points.empty();
 }
 
-void Path::EnumerateComponents(
-    const Applier<LinearPathComponent>& linear_applier,
-    const Applier<QuadraticPathComponent>& quad_applier,
-    const Applier<CubicPathComponent>& cubic_applier,
-    const Applier<ContourComponent>& contour_applier) const {
-  auto& points = data_->points;
-  size_t currentIndex = 0;
-  for (const auto& component : data_->components) {
-    switch (component.type) {
-      case ComponentType::kLinear:
-        if (linear_applier) {
-          linear_applier(currentIndex,
-                         LinearPathComponent(points[component.index],
-                                             points[component.index + 1]));
-        }
-        break;
-      case ComponentType::kQuadratic:
-        if (quad_applier) {
-          quad_applier(currentIndex,
-                       QuadraticPathComponent(points[component.index],
-                                              points[component.index + 1],
-                                              points[component.index + 2]));
-        }
-        break;
-      case ComponentType::kCubic:
-        if (cubic_applier) {
-          cubic_applier(currentIndex,
-                        CubicPathComponent(points[component.index],
-                                           points[component.index + 1],
-                                           points[component.index + 2],
-                                           points[component.index + 3]));
-        }
-        break;
-      case ComponentType::kContour:
-        if (contour_applier) {
-          contour_applier(currentIndex, data_->contours[component.index]);
-        }
-        break;
-    }
-    currentIndex++;
-  }
-}
-
 void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
   auto& path_components = data_->components;
   auto& path_points = data_->points;
   bool started_contour = false;
   bool first_point = true;
 
+  size_t storage_offset = 0u;
   for (size_t component_i = 0; component_i < path_components.size();
        component_i++) {
     const auto& path_component = path_components[component_i];
-    switch (path_component.type) {
+    switch (path_component) {
       case ComponentType::kLinear: {
         const LinearPathComponent* linear =
             reinterpret_cast<const LinearPathComponent*>(
-                &path_points[path_component.index]);
+                &path_points[storage_offset]);
         if (first_point) {
           writer.Write(linear->p1);
           first_point = false;
@@ -127,7 +85,7 @@ void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
       case ComponentType::kQuadratic: {
         const QuadraticPathComponent* quad =
             reinterpret_cast<const QuadraticPathComponent*>(
-                &path_points[path_component.index]);
+                &path_points[storage_offset]);
         if (first_point) {
           writer.Write(quad->p1);
           first_point = false;
@@ -138,7 +96,7 @@ void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
       case ComponentType::kCubic: {
         const CubicPathComponent* cubic =
             reinterpret_cast<const CubicPathComponent*>(
-                &path_points[path_component.index]);
+                &path_points[storage_offset]);
         if (first_point) {
           writer.Write(cubic->p1);
           first_point = false;
@@ -146,7 +104,7 @@ void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
         cubic->ToLinearPathComponents(scale, writer);
         break;
       }
-      case ComponentType::kContour:
+      case Path::ComponentType::kContour:
         if (component_i == path_components.size() - 1) {
           // If the last component is a contour, that means it's an empty
           // contour, so skip it.
@@ -163,6 +121,7 @@ void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
         started_contour = true;
         first_point = true;
     }
+    storage_offset += VerbToOffset(path_component);
   }
   if (started_contour) {
     writer.EndContour();
@@ -172,18 +131,18 @@ void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
 bool Path::GetLinearComponentAtIndex(size_t index,
                                      LinearPathComponent& linear) const {
   auto& components = data_->components;
-
-  if (index >= components.size()) {
+  if (index >= components.size() ||
+      components[index] != ComponentType::kLinear) {
     return false;
   }
 
-  if (components[index].type != ComponentType::kLinear) {
-    return false;
+  size_t storage_offset = 0u;
+  for (auto i = 0u; i < index; i++) {
+    storage_offset += VerbToOffset(components[i]);
   }
-
   auto& points = data_->points;
-  auto point_index = components[index].index;
-  linear = LinearPathComponent(points[point_index], points[point_index + 1]);
+  linear =
+      LinearPathComponent(points[storage_offset], points[storage_offset + 1]);
   return true;
 }
 
@@ -191,54 +150,58 @@ bool Path::GetQuadraticComponentAtIndex(
     size_t index,
     QuadraticPathComponent& quadratic) const {
   auto& components = data_->components;
-
-  if (index >= components.size()) {
+  if (index >= components.size() ||
+      components[index] != ComponentType::kQuadratic) {
     return false;
   }
 
-  if (components[index].type != ComponentType::kQuadratic) {
-    return false;
+  size_t storage_offset = 0u;
+  for (auto i = 0u; i < index; i++) {
+    storage_offset += VerbToOffset(components[i]);
   }
-
   auto& points = data_->points;
-  auto point_index = components[index].index;
-  quadratic = QuadraticPathComponent(
-      points[point_index], points[point_index + 1], points[point_index + 2]);
+
+  quadratic =
+      QuadraticPathComponent(points[storage_offset], points[storage_offset + 1],
+                             points[storage_offset + 2]);
   return true;
 }
 
 bool Path::GetCubicComponentAtIndex(size_t index,
                                     CubicPathComponent& cubic) const {
   auto& components = data_->components;
-
-  if (index >= components.size()) {
+  if (index >= components.size() ||
+      components[index] != ComponentType::kCubic) {
     return false;
   }
 
-  if (components[index].type != ComponentType::kCubic) {
-    return false;
+  size_t storage_offset = 0u;
+  for (auto i = 0u; i < index; i++) {
+    storage_offset += VerbToOffset(components[i]);
   }
-
   auto& points = data_->points;
-  auto point_index = components[index].index;
-  cubic = CubicPathComponent(points[point_index], points[point_index + 1],
-                             points[point_index + 2], points[point_index + 3]);
+
+  cubic = CubicPathComponent(points[storage_offset], points[storage_offset + 1],
+                             points[storage_offset + 2],
+                             points[storage_offset + 3]);
   return true;
 }
 
 bool Path::GetContourComponentAtIndex(size_t index,
                                       ContourComponent& move) const {
   auto& components = data_->components;
-
-  if (index >= components.size()) {
+  if (index >= components.size() ||
+      components[index] != ComponentType::kContour) {
     return false;
   }
 
-  if (components[index].type != ComponentType::kContour) {
-    return false;
+  size_t storage_offset = 0u;
+  for (auto i = 0u; i < index; i++) {
+    storage_offset += VerbToOffset(components[i]);
   }
+  auto& points = data_->points;
 
-  move = data_->contours[components[index].index];
+  move = ContourComponent(points[storage_offset], points[storage_offset]);
   return true;
 }
 
@@ -270,48 +233,12 @@ Path::Polyline Path::CreatePolyline(
   auto& path_components = data_->components;
   auto& path_points = data_->points;
 
-  auto get_path_component = [&path_components, &path_points](
-                                size_t component_i) -> PathComponentVariant {
-    if (component_i >= path_components.size()) {
-      return std::monostate{};
-    }
-    const auto& component = path_components[component_i];
-    switch (component.type) {
-      case ComponentType::kLinear:
-        return reinterpret_cast<const LinearPathComponent*>(
-            &path_points[component.index]);
-      case ComponentType::kQuadratic:
-        return reinterpret_cast<const QuadraticPathComponent*>(
-            &path_points[component.index]);
-      case ComponentType::kCubic:
-        return reinterpret_cast<const CubicPathComponent*>(
-            &path_points[component.index]);
-      case ComponentType::kContour:
-        return std::monostate{};
-    }
-  };
-
-  auto compute_contour_start_direction =
-      [&get_path_component](size_t current_path_component_index) {
-        size_t next_component_index = current_path_component_index + 1;
-        while (!std::holds_alternative<std::monostate>(
-            get_path_component(next_component_index))) {
-          auto next_component = get_path_component(next_component_index);
-          auto maybe_vector =
-              std::visit(PathComponentStartDirectionVisitor(), next_component);
-          if (maybe_vector.has_value()) {
-            return maybe_vector.value();
-          } else {
-            next_component_index++;
-          }
-        }
-        return Vector2(0, -1);
-      };
-
   std::vector<PolylineContour::Component> poly_components;
   std::optional<size_t> previous_path_component_index;
-  auto end_contour = [&polyline, &previous_path_component_index,
-                      &get_path_component, &poly_components]() {
+  std::optional<Vector2> start_direction = std::nullopt;
+  size_t storage_offset = 0u;
+
+  auto end_contour = [&]() {
     // Whenever a contour has ended, extract the exact end direction from
     // the last component.
     if (polyline.contours.empty()) {
@@ -328,57 +255,95 @@ Path::Polyline Path::CreatePolyline(
     poly_components.clear();
 
     size_t previous_index = previous_path_component_index.value();
-    while (!std::holds_alternative<std::monostate>(
-        get_path_component(previous_index))) {
-      auto previous_component = get_path_component(previous_index);
-      auto maybe_vector =
-          std::visit(PathComponentEndDirectionVisitor(), previous_component);
-      if (maybe_vector.has_value()) {
-        contour.end_direction = maybe_vector.value();
-        break;
-      } else {
-        if (previous_index == 0) {
+    size_t local_storage_offset = storage_offset;
+    while (previous_index > 0) {
+      const auto& path_component = path_components[previous_index];
+      switch (path_component) {
+        case ComponentType::kLinear: {
+          auto* linear = reinterpret_cast<const LinearPathComponent*>(
+              &path_points[local_storage_offset]);
+          auto maybe_end = linear->GetEndDirection();
+          if (maybe_end.has_value()) {
+            contour.end_direction = maybe_end.value();
+            return;
+          }
+          break;
+        }
+        case ComponentType::kQuadratic: {
+          auto* quad = reinterpret_cast<const QuadraticPathComponent*>(
+              &path_points[local_storage_offset]);
+          auto maybe_end = quad->GetEndDirection();
+          if (maybe_end.has_value()) {
+            contour.end_direction = maybe_end.value();
+            return;
+          }
           break;
         }
-        previous_index--;
+        case ComponentType::kCubic: {
+          auto* cubic = reinterpret_cast<const CubicPathComponent*>(
+              &path_points[local_storage_offset]);
+          auto maybe_end = cubic->GetEndDirection();
+          if (maybe_end.has_value()) {
+            contour.end_direction = maybe_end.value();
+            return;
+          }
+          break;
+        }
+        case ComponentType::kContour: {
+          // Hit previous contour, return.
+          return;
+        };
       }
+      storage_offset -= VerbToOffset(path_component);
     }
   };
 
   for (size_t component_i = 0; component_i < path_components.size();
        component_i++) {
     const auto& path_component = path_components[component_i];
-    switch (path_component.type) {
-      case ComponentType::kLinear:
+    switch (path_component) {
+      case ComponentType::kLinear: {
         poly_components.push_back({
             .component_start_index = polyline.points->size() - 1,
             .is_curve = false,
         });
-        reinterpret_cast<const LinearPathComponent*>(
-            &path_points[path_component.index])
-            ->AppendPolylinePoints(*polyline.points);
+        auto* linear = reinterpret_cast<const LinearPathComponent*>(
+            &path_points[storage_offset]);
+        linear->AppendPolylinePoints(*polyline.points);
+        if (!start_direction.has_value()) {
+          start_direction = linear->GetStartDirection();
+        }
         previous_path_component_index = component_i;
         break;
-      case ComponentType::kQuadratic:
+      }
+      case ComponentType::kQuadratic: {
         poly_components.push_back({
             .component_start_index = polyline.points->size() - 1,
             .is_curve = true,
         });
-        reinterpret_cast<const QuadraticPathComponent*>(
-            &path_points[path_component.index])
-            ->AppendPolylinePoints(scale, *polyline.points);
+        auto* quad = reinterpret_cast<const QuadraticPathComponent*>(
+            &path_points[storage_offset]);
+        quad->AppendPolylinePoints(scale, *polyline.points);
         previous_path_component_index = component_i;
+        if (!start_direction.has_value()) {
+          start_direction = quad->GetStartDirection();
+        }
         break;
-      case ComponentType::kCubic:
+      }
+      case ComponentType::kCubic: {
         poly_components.push_back({
             .component_start_index = polyline.points->size() - 1,
             .is_curve = true,
         });
-        reinterpret_cast<const CubicPathComponent*>(
-            &path_points[path_component.index])
-            ->AppendPolylinePoints(scale, *polyline.points);
+        auto* cubic = reinterpret_cast<const CubicPathComponent*>(
+            &path_points[storage_offset]);
+        cubic->AppendPolylinePoints(scale, *polyline.points);
         previous_path_component_index = component_i;
+        if (!start_direction.has_value()) {
+          start_direction = cubic->GetStartDirection();
+        }
         break;
+      }
       case ComponentType::kContour:
         if (component_i == path_components.size() - 1) {
           // If the last component is a contour, that means it's an empty
@@ -387,16 +352,18 @@ Path::Polyline Path::CreatePolyline(
         }
         end_contour();
 
-        Vector2 start_direction = compute_contour_start_direction(component_i);
-        const auto& contour = data_->contours[path_component.index];
-        polyline.contours.push_back({.start_index = polyline.points->size(),
-                                     .is_closed = contour.is_closed,
-                                     .start_direction = start_direction,
-                                     .components = poly_components});
+        auto* contour = reinterpret_cast<const ContourComponent*>(
+            &path_points[storage_offset]);
+        polyline.contours.push_back(
+            {.start_index = polyline.points->size(),
+             .is_closed = contour->IsClosed(),
+             .start_direction = start_direction.value_or(Vector2(0, -1)),
+             .components = poly_components});
 
-        polyline.points->push_back(contour.destination);
+        polyline.points->push_back(contour->destination);
         break;
     }
+    storage_offset += VerbToOffset(path_component);
   }
   end_contour();
   return polyline;
diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h
index 0eebfab5cfd1e..f5f5920f1e2dc 100644
--- a/impeller/geometry/path.h
+++ b/impeller/geometry/path.h
@@ -58,6 +58,21 @@ class Path {
     kContour,
   };
 
+  static constexpr size_t VerbToOffset(Path::ComponentType verb) {
+    switch (verb) {
+      case Path::ComponentType::kLinear:
+        return 2u;
+      case Path::ComponentType::kQuadratic:
+        return 3u;
+      case Path::ComponentType::kCubic:
+        return 4u;
+      case Path::ComponentType::kContour:
+        return 2u;
+        break;
+    }
+    FML_UNREACHABLE();
+  }
+
   struct PolylineContour {
     struct Component {
       size_t component_start_index;
@@ -138,14 +153,6 @@ class Path {
 
   bool IsEmpty() const;
 
-  template <class T>
-  using Applier = std::function<void(size_t index, const T& component)>;
-  void EnumerateComponents(
-      const Applier<LinearPathComponent>& linear_applier,
-      const Applier<QuadraticPathComponent>& quad_applier,
-      const Applier<CubicPathComponent>& cubic_applier,
-      const Applier<ContourComponent>& contour_applier) const;
-
   bool GetLinearComponentAtIndex(size_t index,
                                  LinearPathComponent& linear) const;
 
@@ -183,16 +190,6 @@ class Path {
  private:
   friend class PathBuilder;
 
-  struct ComponentIndexPair {
-    ComponentType type = ComponentType::kLinear;
-    size_t index = 0;
-
-    ComponentIndexPair() {}
-
-    ComponentIndexPair(ComponentType a_type, size_t a_index)
-        : type(a_type), index(a_index) {}
-  };
-
   // All of the data for the path is stored in this structure which is
   // held by a shared_ptr. Since they all share the structure, the
   // copy constructor for Path is very cheap and we don't need to deal
@@ -213,11 +210,9 @@ class Path {
 
     FillType fill = FillType::kNonZero;
     Convexity convexity = Convexity::kUnknown;
-    std::vector<ComponentIndexPair> components;
-    std::vector<Point> points;
-    std::vector<ContourComponent> contours;
-
     std::optional<Rect> bounds;
+    std::vector<Point> points;
+    std::vector<ComponentType> components;
   };
 
   explicit Path(Data data);
diff --git a/impeller/geometry/path_builder.cc b/impeller/geometry/path_builder.cc
index be591d92884c7..a96e093cc2315 100644
--- a/impeller/geometry/path_builder.cc
+++ b/impeller/geometry/path_builder.cc
@@ -22,6 +22,7 @@ Path PathBuilder::CopyPath(FillType fill) {
 Path PathBuilder::TakePath(FillType fill) {
   prototype_.fill = fill;
   UpdateBounds();
+  current_contour_location_ = 0u;
   return Path(std::move(prototype_));
 }
 
@@ -264,24 +265,27 @@ PathBuilder& PathBuilder::AddRoundedRectBottomLeft(Rect rect,
 void PathBuilder::AddContourComponent(const Point& destination,
                                       bool is_closed) {
   auto& components = prototype_.components;
-  auto& contours = prototype_.contours;
+  auto& points = prototype_.points;
+  auto closed = is_closed ? Point{0, 0} : Point{1, 1};
   if (components.size() > 0 &&
-      components.back().type == Path::ComponentType::kContour) {
+      components.back() == Path::ComponentType::kContour) {
     // Never insert contiguous contours.
-    contours.back() = ContourComponent(destination, is_closed);
+    points[current_contour_location_] = closed;
+    points[current_contour_location_ + 1] = destination;
   } else {
-    contours.emplace_back(ContourComponent(destination, is_closed));
-    components.emplace_back(Path::ComponentType::kContour, contours.size() - 1);
+    current_contour_location_ = points.size();
+    points.push_back(destination);
+    points.push_back(closed);
+    components.push_back(Path::ComponentType::kContour);
   }
   prototype_.bounds.reset();
 }
 
 void PathBuilder::AddLinearComponent(const Point& p1, const Point& p2) {
   auto& points = prototype_.points;
-  auto index = points.size();
-  points.emplace_back(p1);
-  points.emplace_back(p2);
-  prototype_.components.emplace_back(Path::ComponentType::kLinear, index);
+  points.push_back(p1);
+  points.push_back(p2);
+  prototype_.components.push_back(Path::ComponentType::kLinear);
   prototype_.bounds.reset();
 }
 
@@ -289,11 +293,10 @@ void PathBuilder::AddQuadraticComponent(const Point& p1,
                                         const Point& cp,
                                         const Point& p2) {
   auto& points = prototype_.points;
-  auto index = points.size();
-  points.emplace_back(p1);
-  points.emplace_back(cp);
-  points.emplace_back(p2);
-  prototype_.components.emplace_back(Path::ComponentType::kQuadratic, index);
+  points.push_back(p1);
+  points.push_back(cp);
+  points.push_back(p2);
+  prototype_.components.push_back(Path::ComponentType::kQuadratic);
   prototype_.bounds.reset();
 }
 
@@ -302,17 +305,17 @@ void PathBuilder::AddCubicComponent(const Point& p1,
                                     const Point& cp2,
                                     const Point& p2) {
   auto& points = prototype_.points;
-  auto index = points.size();
-  points.emplace_back(p1);
-  points.emplace_back(cp1);
-  points.emplace_back(cp2);
-  points.emplace_back(p2);
-  prototype_.components.emplace_back(Path::ComponentType::kCubic, index);
+  points.push_back(p1);
+  points.push_back(cp1);
+  points.push_back(cp2);
+  points.push_back(p2);
+  prototype_.components.push_back(Path::ComponentType::kCubic);
   prototype_.bounds.reset();
 }
 
 void PathBuilder::SetContourClosed(bool is_closed) {
-  prototype_.contours.back().is_closed = is_closed;
+  prototype_.points[current_contour_location_ + 1] =
+      is_closed ? Point{0, 0} : Point{1, 1};
 }
 
 PathBuilder& PathBuilder::AddArc(const Rect& oval_bounds,
@@ -428,29 +431,50 @@ PathBuilder& PathBuilder::AddLine(const Point& p1, const Point& p2) {
 }
 
 PathBuilder& PathBuilder::AddPath(const Path& path) {
-  auto linear = [&](size_t index, const LinearPathComponent& l) {
-    AddLinearComponent(l.p1, l.p2);
-  };
-  auto quadratic = [&](size_t index, const QuadraticPathComponent& q) {
-    AddQuadraticComponent(q.p1, q.cp, q.p2);
-  };
-  auto cubic = [&](size_t index, const CubicPathComponent& c) {
-    AddCubicComponent(c.p1, c.cp1, c.cp2, c.p2);
-  };
-  auto move = [&](size_t index, const ContourComponent& m) {
-    AddContourComponent(m.destination);
-  };
-  path.EnumerateComponents(linear, quadratic, cubic, move);
+  auto& points = prototype_.points;
+  auto& components = prototype_.components;
+
+  points.insert(points.end(), path.data_->points.begin(),
+                path.data_->points.end());
+  components.insert(components.end(), path.data_->components.begin(),
+                    path.data_->components.end());
+
   return *this;
 }
 
 PathBuilder& PathBuilder::Shift(Point offset) {
-  for (auto& point : prototype_.points) {
-    point += offset;
-  }
-  for (auto& contour : prototype_.contours) {
-    contour.destination += offset;
+  auto& points = prototype_.points;
+  size_t storage_offset = 0u;
+  for (const auto& component : prototype_.components) {
+    switch (component) {
+      case Path::ComponentType::kLinear: {
+        auto* linear =
+            reinterpret_cast<LinearPathComponent*>(&points[storage_offset]);
+        linear->p1 += offset;
+        linear->p2 += offset;
+        break;
+      }
+      case Path::ComponentType::kQuadratic: {
+        auto* quad =
+            reinterpret_cast<QuadraticPathComponent*>(&points[storage_offset]);
+        quad->p1 += offset;
+        quad->p2 += offset;
+        quad->cp += offset;
+      } break;
+      case Path::ComponentType::kCubic: {
+        auto* cubic =
+            reinterpret_cast<CubicPathComponent*>(&points[storage_offset]);
+        cubic->p1 += offset;
+        cubic->p2 += offset;
+        cubic->cp1 += offset;
+        cubic->cp1 += offset;
+      } break;
+      case Path::ComponentType::kContour:
+        break;
+    }
+    storage_offset += Path::VerbToOffset(component);
   }
+
   prototype_.bounds.reset();
   return *this;
 }
@@ -499,11 +523,12 @@ std::optional<std::pair<Point, Point>> PathBuilder::GetMinMaxCoveragePoints()
     }
   };
 
+  size_t storage_offset = 0u;
   for (const auto& component : prototype_.components) {
-    switch (component.type) {
+    switch (component) {
       case Path::ComponentType::kLinear: {
         auto* linear = reinterpret_cast<const LinearPathComponent*>(
-            &points[component.index]);
+            &points[storage_offset]);
         clamp(linear->p1);
         clamp(linear->p2);
         break;
@@ -511,14 +536,14 @@ std::optional<std::pair<Point, Point>> PathBuilder::GetMinMaxCoveragePoints()
       case Path::ComponentType::kQuadratic:
         for (const auto& extrema :
              reinterpret_cast<const QuadraticPathComponent*>(
-                 &points[component.index])
+                 &points[storage_offset])
                  ->Extrema()) {
           clamp(extrema);
         }
         break;
       case Path::ComponentType::kCubic:
         for (const auto& extrema : reinterpret_cast<const CubicPathComponent*>(
-                                       &points[component.index])
+                                       &points[storage_offset])
                                        ->Extrema()) {
           clamp(extrema);
         }
@@ -526,6 +551,7 @@ std::optional<std::pair<Point, Point>> PathBuilder::GetMinMaxCoveragePoints()
       case Path::ComponentType::kContour:
         break;
     }
+    storage_offset += Path::VerbToOffset(component);
   }
 
   if (!min.has_value() || !max.has_value()) {
diff --git a/impeller/geometry/path_builder.h b/impeller/geometry/path_builder.h
index 0907b7b67f0f4..f4de96d18b704 100644
--- a/impeller/geometry/path_builder.h
+++ b/impeller/geometry/path_builder.h
@@ -156,6 +156,7 @@ class PathBuilder {
  private:
   Point subpath_start_;
   Point current_;
+  size_t current_contour_location_ = 0u;
   Path::Data prototype_;
 
   PathBuilder& AddRoundedRectTopLeft(Rect rect, RoundingRadii radii);
diff --git a/impeller/geometry/path_component.cc b/impeller/geometry/path_component.cc
index 6c3492fd2ade4..d990f37ed7f73 100644
--- a/impeller/geometry/path_component.cc
+++ b/impeller/geometry/path_component.cc
@@ -373,52 +373,4 @@ std::optional<Vector2> CubicPathComponent::GetEndDirection() const {
   return std::nullopt;
 }
 
-std::optional<Vector2> PathComponentStartDirectionVisitor::operator()(
-    const LinearPathComponent* component) {
-  if (!component) {
-    return std::nullopt;
-  }
-  return component->GetStartDirection();
-}
-
-std::optional<Vector2> PathComponentStartDirectionVisitor::operator()(
-    const QuadraticPathComponent* component) {
-  if (!component) {
-    return std::nullopt;
-  }
-  return component->GetStartDirection();
-}
-
-std::optional<Vector2> PathComponentStartDirectionVisitor::operator()(
-    const CubicPathComponent* component) {
-  if (!component) {
-    return std::nullopt;
-  }
-  return component->GetStartDirection();
-}
-
-std::optional<Vector2> PathComponentEndDirectionVisitor::operator()(
-    const LinearPathComponent* component) {
-  if (!component) {
-    return std::nullopt;
-  }
-  return component->GetEndDirection();
-}
-
-std::optional<Vector2> PathComponentEndDirectionVisitor::operator()(
-    const QuadraticPathComponent* component) {
-  if (!component) {
-    return std::nullopt;
-  }
-  return component->GetEndDirection();
-}
-
-std::optional<Vector2> PathComponentEndDirectionVisitor::operator()(
-    const CubicPathComponent* component) {
-  if (!component) {
-    return std::nullopt;
-  }
-  return component->GetEndDirection();
-}
-
 }  // namespace impeller
diff --git a/impeller/geometry/path_component.h b/impeller/geometry/path_component.h
index dca03126bb0dc..b54735dba61f2 100644
--- a/impeller/geometry/path_component.h
+++ b/impeller/geometry/path_component.h
@@ -8,7 +8,6 @@
 #include <functional>
 #include <optional>
 #include <type_traits>
-#include <variant>
 #include <vector>
 
 #include "impeller/geometry/point.h"
@@ -150,38 +149,19 @@ struct CubicPathComponent {
 
 struct ContourComponent {
   Point destination;
-  bool is_closed = false;
 
-  ContourComponent() {}
+  // 0, 0 for closed, anything else for open.
+  Point closed = Point(1, 1);
 
-  explicit ContourComponent(Point p, bool is_closed = false)
-      : destination(p), is_closed(is_closed) {}
+  ContourComponent() {}
 
-  bool operator==(const ContourComponent& other) const {
-    return destination == other.destination && is_closed == other.is_closed;
-  }
-};
+  constexpr bool IsClosed() const { return closed == Point{0, 0}; }
 
-using PathComponentVariant = std::variant<std::monostate,
-                                          const LinearPathComponent*,
-                                          const QuadraticPathComponent*,
-                                          const CubicPathComponent*>;
-
-struct PathComponentStartDirectionVisitor {
-  std::optional<Vector2> operator()(const LinearPathComponent* component);
-  std::optional<Vector2> operator()(const QuadraticPathComponent* component);
-  std::optional<Vector2> operator()(const CubicPathComponent* component);
-  std::optional<Vector2> operator()(std::monostate monostate) {
-    return std::nullopt;
-  }
-};
+  explicit ContourComponent(Point p, Point closed)
+      : destination(p), closed(closed) {}
 
-struct PathComponentEndDirectionVisitor {
-  std::optional<Vector2> operator()(const LinearPathComponent* component);
-  std::optional<Vector2> operator()(const QuadraticPathComponent* component);
-  std::optional<Vector2> operator()(const CubicPathComponent* component);
-  std::optional<Vector2> operator()(std::monostate monostate) {
-    return std::nullopt;
+  bool operator==(const ContourComponent& other) const {
+    return destination == other.destination && IsClosed() == other.IsClosed();
   }
 };
 
diff --git a/impeller/geometry/path_unittests.cc b/impeller/geometry/path_unittests.cc
index 54f6900a0f9a2..cf827645486af 100644
--- a/impeller/geometry/path_unittests.cc
+++ b/impeller/geometry/path_unittests.cc
@@ -48,7 +48,7 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) {
     ContourComponent contour;
     path.GetContourComponentAtIndex(0, contour);
     EXPECT_POINT_NEAR(contour.destination, Point(100, 50));
-    EXPECT_TRUE(contour.is_closed);
+    EXPECT_TRUE(contour.IsClosed());
   }
 
   {
@@ -57,7 +57,7 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) {
     ContourComponent contour;
     path.GetContourComponentAtIndex(0, contour);
     EXPECT_POINT_NEAR(contour.destination, Point(150, 100));
-    EXPECT_TRUE(contour.is_closed);
+    EXPECT_TRUE(contour.IsClosed());
   }
 
   {
@@ -66,7 +66,7 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) {
     ContourComponent contour;
     path.GetContourComponentAtIndex(0, contour);
     EXPECT_POINT_NEAR(contour.destination, Point(100, 100));
-    EXPECT_TRUE(contour.is_closed);
+    EXPECT_TRUE(contour.IsClosed());
   }
 
   {
@@ -76,7 +76,7 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) {
     ContourComponent contour;
     path.GetContourComponentAtIndex(0, contour);
     EXPECT_POINT_NEAR(contour.destination, Point(110, 100));
-    EXPECT_TRUE(contour.is_closed);
+    EXPECT_TRUE(contour.IsClosed());
   }
 
   {
@@ -87,7 +87,7 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) {
     ContourComponent contour;
     path.GetContourComponentAtIndex(0, contour);
     EXPECT_POINT_NEAR(contour.destination, Point(110, 100));
-    EXPECT_TRUE(contour.is_closed);
+    EXPECT_TRUE(contour.IsClosed());
   }
 
   // Open shapes.
@@ -97,7 +97,7 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) {
     ContourComponent contour;
     path.GetContourComponentAtIndex(0, contour);
     ASSERT_POINT_NEAR(contour.destination, p);
-    ASSERT_FALSE(contour.is_closed);
+    ASSERT_FALSE(contour.IsClosed());
   }
 
   {
@@ -108,7 +108,7 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) {
     ContourComponent contour;
     path.GetContourComponentAtIndex(0, contour);
     ASSERT_POINT_NEAR(contour.destination, Point(100, 100));
-    ASSERT_FALSE(contour.is_closed);
+    ASSERT_FALSE(contour.IsClosed());
   }
 
   {
@@ -118,7 +118,7 @@ TEST(PathTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) {
     ContourComponent contour;
     path.GetContourComponentAtIndex(0, contour);
     ASSERT_POINT_NEAR(contour.destination, Point(100, 100));
-    ASSERT_FALSE(contour.is_closed);
+    ASSERT_FALSE(contour.IsClosed());
   }
 }
 
@@ -394,66 +394,6 @@ TEST(PathTest, EmptyPath) {
   ASSERT_TRUE(polyline.contours.empty());
 }
 
-TEST(PathTest, SimplePath) {
-  PathBuilder builder;
-
-  auto path = builder.AddLine({0, 0}, {100, 100})
-                  .AddQuadraticCurve({100, 100}, {200, 200}, {300, 300})
-                  .AddCubicCurve({300, 300}, {400, 400}, {500, 500}, {600, 600})
-                  .TakePath();
-
-  ASSERT_EQ(path.GetComponentCount(), 6u);
-  ASSERT_EQ(path.GetComponentCount(Path::ComponentType::kLinear), 1u);
-  ASSERT_EQ(path.GetComponentCount(Path::ComponentType::kQuadratic), 1u);
-  ASSERT_EQ(path.GetComponentCount(Path::ComponentType::kCubic), 1u);
-  ASSERT_EQ(path.GetComponentCount(Path::ComponentType::kContour), 3u);
-
-  path.EnumerateComponents(
-      [](size_t index, const LinearPathComponent& linear) {
-        Point p1(0, 0);
-        Point p2(100, 100);
-        ASSERT_EQ(index, 1u);
-        ASSERT_EQ(linear.p1, p1);
-        ASSERT_EQ(linear.p2, p2);
-      },
-      [](size_t index, const QuadraticPathComponent& quad) {
-        Point p1(100, 100);
-        Point cp(200, 200);
-        Point p2(300, 300);
-        ASSERT_EQ(index, 3u);
-        ASSERT_EQ(quad.p1, p1);
-        ASSERT_EQ(quad.cp, cp);
-        ASSERT_EQ(quad.p2, p2);
-      },
-      [](size_t index, const CubicPathComponent& cubic) {
-        Point p1(300, 300);
-        Point cp1(400, 400);
-        Point cp2(500, 500);
-        Point p2(600, 600);
-        ASSERT_EQ(index, 5u);
-        ASSERT_EQ(cubic.p1, p1);
-        ASSERT_EQ(cubic.cp1, cp1);
-        ASSERT_EQ(cubic.cp2, cp2);
-        ASSERT_EQ(cubic.p2, p2);
-      },
-      [](size_t index, const ContourComponent& contour) {
-        // There is an initial countour added for each curve.
-        if (index == 0u) {
-          Point p1(0, 0);
-          ASSERT_EQ(contour.destination, p1);
-        } else if (index == 2u) {
-          Point p1(100, 100);
-          ASSERT_EQ(contour.destination, p1);
-        } else if (index == 4u) {
-          Point p1(300, 300);
-          ASSERT_EQ(contour.destination, p1);
-        } else {
-          ASSERT_FALSE(true);
-        }
-        ASSERT_FALSE(contour.is_closed);
-      });
-}
-
 TEST(PathTest, RepeatCloseDoesNotAddNewLines) {
   PathBuilder builder;
   auto path = builder.LineTo({0, 10})
@@ -553,7 +493,7 @@ TEST(PathTest, PathBuilderDoesNotMutateCopiedPaths) {
             ContourComponent contour;
             EXPECT_TRUE(path.GetContourComponentAtIndex(0, contour)) << label;
             EXPECT_EQ(contour.destination, offset + Point(10, 10)) << label;
-            EXPECT_EQ(contour.is_closed, is_closed) << label;
+            EXPECT_EQ(contour.IsClosed(), is_closed) << label;
           }
           {
             LinearPathComponent line;

From 846c2aadac2b149bec6bf15d60ae3dc88d5d37ff Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Sat, 7 Sep 2024 10:04:08 -0700
Subject: [PATCH 02/14] test cleanups.

---
 impeller/geometry/path.cc           | 11 ++--
 impeller/geometry/path_builder.cc   | 11 ++--
 impeller/geometry/path_unittests.cc | 79 +++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 10 deletions(-)

diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index 7f20010ded1ba..7d9bdafaa7633 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -5,7 +5,6 @@
 #include "impeller/geometry/path.h"
 
 #include <optional>
-#include <variant>
 
 #include "flutter/fml/logging.h"
 #include "impeller/geometry/path_component.h"
@@ -36,9 +35,6 @@ size_t Path::GetComponentCount(std::optional<ComponentType> type) const {
     return data_->components.size();
   }
   auto type_value = type.value();
-  if (type_value == ComponentType::kContour) {
-    return data_->components.size();
-  }
   size_t count = 0u;
   for (const auto& component : data_->components) {
     if (component == type_value) {
@@ -201,7 +197,7 @@ bool Path::GetContourComponentAtIndex(size_t index,
   }
   auto& points = data_->points;
 
-  move = ContourComponent(points[storage_offset], points[storage_offset]);
+  move = ContourComponent(points[storage_offset], points[storage_offset + 1]);
   return true;
 }
 
@@ -256,7 +252,7 @@ Path::Polyline Path::CreatePolyline(
 
     size_t previous_index = previous_path_component_index.value();
     size_t local_storage_offset = storage_offset;
-    while (previous_index > 0) {
+    while (previous_index >= 0 && local_storage_offset >= 0) {
       const auto& path_component = path_components[previous_index];
       switch (path_component) {
         case ComponentType::kLinear: {
@@ -294,7 +290,8 @@ Path::Polyline Path::CreatePolyline(
           return;
         };
       }
-      storage_offset -= VerbToOffset(path_component);
+      local_storage_offset -= VerbToOffset(path_component);
+      previous_index--;
     }
   };
 
diff --git a/impeller/geometry/path_builder.cc b/impeller/geometry/path_builder.cc
index a96e093cc2315..5aa14d3093645 100644
--- a/impeller/geometry/path_builder.cc
+++ b/impeller/geometry/path_builder.cc
@@ -6,6 +6,8 @@
 
 #include <cmath>
 
+#include "impeller/geometry/path_component.h"
+
 namespace impeller {
 
 PathBuilder::PathBuilder() {
@@ -270,8 +272,8 @@ void PathBuilder::AddContourComponent(const Point& destination,
   if (components.size() > 0 &&
       components.back() == Path::ComponentType::kContour) {
     // Never insert contiguous contours.
-    points[current_contour_location_] = closed;
-    points[current_contour_location_ + 1] = destination;
+    points[current_contour_location_] = destination;
+    points[current_contour_location_ + 1] = closed;
   } else {
     current_contour_location_ = points.size();
     points.push_back(destination);
@@ -467,9 +469,12 @@ PathBuilder& PathBuilder::Shift(Point offset) {
         cubic->p1 += offset;
         cubic->p2 += offset;
         cubic->cp1 += offset;
-        cubic->cp1 += offset;
+        cubic->cp2 += offset;
       } break;
       case Path::ComponentType::kContour:
+        auto* contour =
+            reinterpret_cast<ContourComponent*>(&points[storage_offset]);
+        contour->destination += offset;
         break;
     }
     storage_offset += Path::VerbToOffset(component);
diff --git a/impeller/geometry/path_unittests.cc b/impeller/geometry/path_unittests.cc
index cf827645486af..8d1aed57892f7 100644
--- a/impeller/geometry/path_unittests.cc
+++ b/impeller/geometry/path_unittests.cc
@@ -8,6 +8,7 @@
 #include "impeller/geometry/geometry_asserts.h"
 #include "impeller/geometry/path.h"
 #include "impeller/geometry/path_builder.h"
+#include "impeller/geometry/path_component.h"
 
 namespace impeller {
 namespace testing {
@@ -394,6 +395,84 @@ TEST(PathTest, EmptyPath) {
   ASSERT_TRUE(polyline.contours.empty());
 }
 
+TEST(PathTest, SimplePath) {
+  PathBuilder builder;
+
+  auto path = builder.AddLine({0, 0}, {100, 100})
+                  .AddQuadraticCurve({100, 100}, {200, 200}, {300, 300})
+                  .AddCubicCurve({300, 300}, {400, 400}, {500, 500}, {600, 600})
+                  .TakePath();
+
+  EXPECT_EQ(path.GetComponentCount(), 6u);
+  EXPECT_EQ(path.GetComponentCount(Path::ComponentType::kLinear), 1u);
+  EXPECT_EQ(path.GetComponentCount(Path::ComponentType::kQuadratic), 1u);
+  EXPECT_EQ(path.GetComponentCount(Path::ComponentType::kCubic), 1u);
+  EXPECT_EQ(path.GetComponentCount(Path::ComponentType::kContour), 3u);
+
+  {
+    LinearPathComponent linear;
+    EXPECT_TRUE(path.GetLinearComponentAtIndex(1, linear));
+
+    Point p1(0, 0);
+    Point p2(100, 100);
+    EXPECT_EQ(linear.p1, p1);
+    EXPECT_EQ(linear.p2, p2);
+  }
+
+  {
+    QuadraticPathComponent quad;
+    EXPECT_TRUE(path.GetQuadraticComponentAtIndex(3, quad));
+
+    Point p1(100, 100);
+    Point cp(200, 200);
+    Point p2(300, 300);
+    EXPECT_EQ(quad.p1, p1);
+    EXPECT_EQ(quad.cp, cp);
+    EXPECT_EQ(quad.p2, p2);
+  }
+
+  {
+    CubicPathComponent cubic;
+    EXPECT_TRUE(path.GetCubicComponentAtIndex(5, cubic));
+
+    Point p1(300, 300);
+    Point cp1(400, 400);
+    Point cp2(500, 500);
+    Point p2(600, 600);
+    EXPECT_EQ(cubic.p1, p1);
+    EXPECT_EQ(cubic.cp1, cp1);
+    EXPECT_EQ(cubic.cp2, cp2);
+    EXPECT_EQ(cubic.p2, p2);
+  }
+
+  {
+    ContourComponent contour;
+    EXPECT_TRUE(path.GetContourComponentAtIndex(0, contour));
+
+    Point p1(0, 0);
+    EXPECT_EQ(contour.destination, p1);
+    EXPECT_FALSE(contour.IsClosed());
+  }
+
+  {
+    ContourComponent contour;
+    EXPECT_TRUE(path.GetContourComponentAtIndex(2, contour));
+
+    Point p1(100, 100);
+    EXPECT_EQ(contour.destination, p1);
+    EXPECT_FALSE(contour.IsClosed());
+  }
+
+  {
+    ContourComponent contour;
+    EXPECT_TRUE(path.GetContourComponentAtIndex(4, contour));
+
+    Point p1(300, 300);
+    EXPECT_EQ(contour.destination, p1);
+    EXPECT_FALSE(contour.IsClosed());
+  }
+}
+
 TEST(PathTest, RepeatCloseDoesNotAddNewLines) {
   PathBuilder builder;
   auto path = builder.LineTo({0, 10})

From 9102785bfff64304953b8b42d64cc45e0fb1dda5 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Sat, 7 Sep 2024 10:32:10 -0700
Subject: [PATCH 03/14] add missing include to scene.

---
 impeller/scene/node.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/impeller/scene/node.cc b/impeller/scene/node.cc
index aad7fc67199e7..acee3c165dcde 100644
--- a/impeller/scene/node.cc
+++ b/impeller/scene/node.cc
@@ -7,6 +7,7 @@
 #include <inttypes.h>
 #include <atomic>
 #include <memory>
+#include <variant>
 #include <vector>
 
 #include "flutter/fml/logging.h"

From ca55dac15e793fb1029e5706ff86d2ae0c43123f Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Sat, 7 Sep 2024 11:14:48 -0700
Subject: [PATCH 04/14] Fix off by N lookup at end of Path::CreatePolyline

---
 impeller/geometry/path.cc | 137 ++++++++++++++++++++------------------
 impeller/geometry/path.h  |   5 ++
 2 files changed, 78 insertions(+), 64 deletions(-)

diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index 7d9bdafaa7633..61f83dace6e82 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -220,6 +220,72 @@ Path::Polyline::~Polyline() {
   }
 }
 
+void Path::EndContour(
+    size_t storage_offset,
+    Polyline& polyline,
+    std::optional<size_t> previous_path_component_index,
+    std::vector<PolylineContour::Component>& poly_components) const {
+  auto& path_components = data_->components;
+  auto& path_points = data_->points;
+  // Whenever a contour has ended, extract the exact end direction from
+  // the last component.
+  if (polyline.contours.empty()) {
+    return;
+  }
+
+  if (!previous_path_component_index.has_value()) {
+    return;
+  }
+
+  auto& contour = polyline.contours.back();
+  contour.end_direction = Vector2(0, 1);
+  contour.components = poly_components;
+  poly_components.clear();
+
+  size_t previous_index = previous_path_component_index.value();
+  while (previous_index >= 0 && storage_offset >= 0) {
+    const auto& path_component = path_components[previous_index];
+    switch (path_component) {
+      case ComponentType::kLinear: {
+        auto* linear = reinterpret_cast<const LinearPathComponent*>(
+            &path_points[storage_offset]);
+        auto maybe_end = linear->GetEndDirection();
+        if (maybe_end.has_value()) {
+          contour.end_direction = maybe_end.value();
+          return;
+        }
+        break;
+      }
+      case ComponentType::kQuadratic: {
+        auto* quad = reinterpret_cast<const QuadraticPathComponent*>(
+            &path_points[storage_offset]);
+        auto maybe_end = quad->GetEndDirection();
+        if (maybe_end.has_value()) {
+          contour.end_direction = maybe_end.value();
+          return;
+        }
+        break;
+      }
+      case ComponentType::kCubic: {
+        auto* cubic = reinterpret_cast<const CubicPathComponent*>(
+            &path_points[storage_offset]);
+        auto maybe_end = cubic->GetEndDirection();
+        if (maybe_end.has_value()) {
+          contour.end_direction = maybe_end.value();
+          return;
+        }
+        break;
+      }
+      case ComponentType::kContour: {
+        // Hit previous contour, return.
+        return;
+      };
+    }
+    storage_offset -= VerbToOffset(path_component);
+    previous_index--;
+  }
+};
+
 Path::Polyline Path::CreatePolyline(
     Scalar scale,
     Path::Polyline::PointBufferPtr point_buffer,
@@ -228,73 +294,11 @@ Path::Polyline Path::CreatePolyline(
 
   auto& path_components = data_->components;
   auto& path_points = data_->points;
-
   std::vector<PolylineContour::Component> poly_components;
   std::optional<size_t> previous_path_component_index;
   std::optional<Vector2> start_direction = std::nullopt;
   size_t storage_offset = 0u;
 
-  auto end_contour = [&]() {
-    // Whenever a contour has ended, extract the exact end direction from
-    // the last component.
-    if (polyline.contours.empty()) {
-      return;
-    }
-
-    if (!previous_path_component_index.has_value()) {
-      return;
-    }
-
-    auto& contour = polyline.contours.back();
-    contour.end_direction = Vector2(0, 1);
-    contour.components = poly_components;
-    poly_components.clear();
-
-    size_t previous_index = previous_path_component_index.value();
-    size_t local_storage_offset = storage_offset;
-    while (previous_index >= 0 && local_storage_offset >= 0) {
-      const auto& path_component = path_components[previous_index];
-      switch (path_component) {
-        case ComponentType::kLinear: {
-          auto* linear = reinterpret_cast<const LinearPathComponent*>(
-              &path_points[local_storage_offset]);
-          auto maybe_end = linear->GetEndDirection();
-          if (maybe_end.has_value()) {
-            contour.end_direction = maybe_end.value();
-            return;
-          }
-          break;
-        }
-        case ComponentType::kQuadratic: {
-          auto* quad = reinterpret_cast<const QuadraticPathComponent*>(
-              &path_points[local_storage_offset]);
-          auto maybe_end = quad->GetEndDirection();
-          if (maybe_end.has_value()) {
-            contour.end_direction = maybe_end.value();
-            return;
-          }
-          break;
-        }
-        case ComponentType::kCubic: {
-          auto* cubic = reinterpret_cast<const CubicPathComponent*>(
-              &path_points[local_storage_offset]);
-          auto maybe_end = cubic->GetEndDirection();
-          if (maybe_end.has_value()) {
-            contour.end_direction = maybe_end.value();
-            return;
-          }
-          break;
-        }
-        case ComponentType::kContour: {
-          // Hit previous contour, return.
-          return;
-        };
-      }
-      local_storage_offset -= VerbToOffset(path_component);
-      previous_index--;
-    }
-  };
-
   for (size_t component_i = 0; component_i < path_components.size();
        component_i++) {
     const auto& path_component = path_components[component_i];
@@ -347,7 +351,8 @@ Path::Polyline Path::CreatePolyline(
           // contour, so skip it.
           continue;
         }
-        end_contour();
+        EndContour(storage_offset, polyline, previous_path_component_index,
+                   poly_components);
 
         auto* contour = reinterpret_cast<const ContourComponent*>(
             &path_points[storage_offset]);
@@ -362,7 +367,11 @@ Path::Polyline Path::CreatePolyline(
     }
     storage_offset += VerbToOffset(path_component);
   }
-  end_contour();
+  // Subtract the last storage offset increment so that the storage lookup is
+  // correct.
+  storage_offset -= VerbToOffset(path_components.back());
+  EndContour(storage_offset, polyline, previous_path_component_index,
+             poly_components);
   return polyline;
 }
 
diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h
index f5f5920f1e2dc..0d9db9d310189 100644
--- a/impeller/geometry/path.h
+++ b/impeller/geometry/path.h
@@ -176,6 +176,11 @@ class Path {
           std::make_unique<std::vector<Point>>(),
       Polyline::ReclaimPointBufferCallback reclaim = nullptr) const;
 
+  void EndContour(size_t storage_offset,
+                  Polyline& polyline,
+                  std::optional<size_t> previous_path_component_index,
+                  std::vector<PolylineContour::Component>& poly_components) const;
+
   std::optional<Rect> GetBoundingBox() const;
 
   std::optional<Rect> GetTransformedBoundingBox(const Matrix& transform) const;

From 184f719932706536d76f0a1ec3a9945aa34893c1 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Sat, 7 Sep 2024 11:28:07 -0700
Subject: [PATCH 05/14] ++

---
 impeller/geometry/path.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h
index 0d9db9d310189..87a7268f71214 100644
--- a/impeller/geometry/path.h
+++ b/impeller/geometry/path.h
@@ -176,10 +176,11 @@ class Path {
           std::make_unique<std::vector<Point>>(),
       Polyline::ReclaimPointBufferCallback reclaim = nullptr) const;
 
-  void EndContour(size_t storage_offset,
-                  Polyline& polyline,
-                  std::optional<size_t> previous_path_component_index,
-                  std::vector<PolylineContour::Component>& poly_components) const;
+  void EndContour(
+      size_t storage_offset,
+      Polyline& polyline,
+      std::optional<size_t> previous_path_component_index,
+      std::vector<PolylineContour::Component>& poly_components) const;
 
   std::optional<Rect> GetBoundingBox() const;
 

From 827523ce44bc4a79306750907f8c557be3d34ce1 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Sat, 7 Sep 2024 13:22:29 -0700
Subject: [PATCH 06/14] move to header

---
 impeller/scene/node.cc | 1 -
 impeller/scene/node.h  | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/impeller/scene/node.cc b/impeller/scene/node.cc
index acee3c165dcde..aad7fc67199e7 100644
--- a/impeller/scene/node.cc
+++ b/impeller/scene/node.cc
@@ -7,7 +7,6 @@
 #include <inttypes.h>
 #include <atomic>
 #include <memory>
-#include <variant>
 #include <vector>
 
 #include "flutter/fml/logging.h"
diff --git a/impeller/scene/node.h b/impeller/scene/node.h
index b9c7de02928f5..81d23553afaaf 100644
--- a/impeller/scene/node.h
+++ b/impeller/scene/node.h
@@ -8,6 +8,7 @@
 #include <memory>
 #include <optional>
 #include <vector>
+#include <variant>
 
 #include "impeller/base/thread.h"
 #include "impeller/base/thread_safety.h"

From 86302249000c6b455f255300268c7f15a5ffc09d Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Sat, 7 Sep 2024 13:27:25 -0700
Subject: [PATCH 07/14] ++

---
 impeller/geometry/path_builder.cc | 7 +++++++
 impeller/scene/node.h             | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/impeller/geometry/path_builder.cc b/impeller/geometry/path_builder.cc
index 5aa14d3093645..e6b8228f148d7 100644
--- a/impeller/geometry/path_builder.cc
+++ b/impeller/geometry/path_builder.cc
@@ -441,6 +441,13 @@ PathBuilder& PathBuilder::AddPath(const Path& path) {
   components.insert(components.end(), path.data_->components.begin(),
                     path.data_->components.end());
 
+  size_t source_offset = points.size();
+  for (auto component : path.data_->components) {
+    if (component == Path::ComponentType::kContour) {
+      current_contour_location_ = source_offset;
+    }
+    source_offset += Path::VerbToOffset(component);
+  }
   return *this;
 }
 
diff --git a/impeller/scene/node.h b/impeller/scene/node.h
index 81d23553afaaf..9340dbef75100 100644
--- a/impeller/scene/node.h
+++ b/impeller/scene/node.h
@@ -7,8 +7,8 @@
 
 #include <memory>
 #include <optional>
-#include <vector>
 #include <variant>
+#include <vector>
 
 #include "impeller/base/thread.h"
 #include "impeller/base/thread_safety.h"

From 85dbffc59d65eaafd011c7fe8975cfeb8d105f7a Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Mon, 9 Sep 2024 21:05:45 -0700
Subject: [PATCH 08/14] fix incorrect origin computation.

---
 impeller/geometry/path.cc | 43 ++++++++++++++++++++++-----------------
 impeller/geometry/path.h  |  2 +-
 2 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index 61f83dace6e82..06500ecaf8c28 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -223,7 +223,7 @@ Path::Polyline::~Polyline() {
 void Path::EndContour(
     size_t storage_offset,
     Polyline& polyline,
-    std::optional<size_t> previous_path_component_index,
+    size_t component_index,
     std::vector<PolylineContour::Component>& poly_components) const {
   auto& path_components = data_->components;
   auto& path_points = data_->points;
@@ -233,7 +233,7 @@ void Path::EndContour(
     return;
   }
 
-  if (!previous_path_component_index.has_value()) {
+  if (component_index <= 0) {
     return;
   }
 
@@ -242,7 +242,7 @@ void Path::EndContour(
   contour.components = poly_components;
   poly_components.clear();
 
-  size_t previous_index = previous_path_component_index.value();
+  size_t previous_index = component_index - 1;
   while (previous_index >= 0 && storage_offset >= 0) {
     const auto& path_component = path_components[previous_index];
     switch (path_component) {
@@ -294,13 +294,12 @@ Path::Polyline Path::CreatePolyline(
 
   auto& path_components = data_->components;
   auto& path_points = data_->points;
+  std::optional<Vector2> start_direction;
   std::vector<PolylineContour::Component> poly_components;
-  std::optional<size_t> previous_path_component_index;
-  std::optional<Vector2> start_direction = std::nullopt;
   size_t storage_offset = 0u;
+  size_t component_i = 0;
 
-  for (size_t component_i = 0; component_i < path_components.size();
-       component_i++) {
+  for (; component_i < path_components.size(); component_i++) {
     const auto& path_component = path_components[component_i];
     switch (path_component) {
       case ComponentType::kLinear: {
@@ -314,7 +313,6 @@ Path::Polyline Path::CreatePolyline(
         if (!start_direction.has_value()) {
           start_direction = linear->GetStartDirection();
         }
-        previous_path_component_index = component_i;
         break;
       }
       case ComponentType::kQuadratic: {
@@ -325,7 +323,6 @@ Path::Polyline Path::CreatePolyline(
         auto* quad = reinterpret_cast<const QuadraticPathComponent*>(
             &path_points[storage_offset]);
         quad->AppendPolylinePoints(scale, *polyline.points);
-        previous_path_component_index = component_i;
         if (!start_direction.has_value()) {
           start_direction = quad->GetStartDirection();
         }
@@ -339,7 +336,6 @@ Path::Polyline Path::CreatePolyline(
         auto* cubic = reinterpret_cast<const CubicPathComponent*>(
             &path_points[storage_offset]);
         cubic->AppendPolylinePoints(scale, *polyline.points);
-        previous_path_component_index = component_i;
         if (!start_direction.has_value()) {
           start_direction = cubic->GetStartDirection();
         }
@@ -351,27 +347,36 @@ Path::Polyline Path::CreatePolyline(
           // contour, so skip it.
           continue;
         }
-        EndContour(storage_offset, polyline, previous_path_component_index,
-                   poly_components);
+        if (!polyline.contours.empty()) {
+          polyline.contours.back().start_direction =
+              start_direction.value_or(Vector2(0, -1));
+          start_direction = std::nullopt;
+        }
+        EndContour(storage_offset, polyline, component_i, poly_components);
 
         auto* contour = reinterpret_cast<const ContourComponent*>(
             &path_points[storage_offset]);
-        polyline.contours.push_back(
-            {.start_index = polyline.points->size(),
-             .is_closed = contour->IsClosed(),
-             .start_direction = start_direction.value_or(Vector2(0, -1)),
-             .components = poly_components});
+        polyline.contours.push_back(PolylineContour{
+            .start_index = polyline.points->size(),  //
+            .is_closed = contour->IsClosed(),        //
+            .start_direction = Vector2(0, -1),       //
+            .components = poly_components            //
+        });
 
         polyline.points->push_back(contour->destination);
         break;
     }
     storage_offset += VerbToOffset(path_component);
   }
+
   // Subtract the last storage offset increment so that the storage lookup is
   // correct.
   storage_offset -= VerbToOffset(path_components.back());
-  EndContour(storage_offset, polyline, previous_path_component_index,
-             poly_components);
+  if (!polyline.contours.empty()) {
+    polyline.contours.back().start_direction =
+        start_direction.value_or(Vector2(0, -1));
+  }
+  EndContour(storage_offset, polyline, component_i, poly_components);
   return polyline;
 }
 
diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h
index 87a7268f71214..27a345ec082ab 100644
--- a/impeller/geometry/path.h
+++ b/impeller/geometry/path.h
@@ -179,7 +179,7 @@ class Path {
   void EndContour(
       size_t storage_offset,
       Polyline& polyline,
-      std::optional<size_t> previous_path_component_index,
+      size_t component_index,
       std::vector<PolylineContour::Component>& poly_components) const;
 
   std::optional<Rect> GetBoundingBox() const;

From dba9b063d3e0c5b50b0ef0d2810eb3b84d3a7837 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 11 Sep 2024 20:40:54 -0700
Subject: [PATCH 09/14] correct empty contour computation.

---
 impeller/geometry/path.cc | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index 06500ecaf8c28..4fd7a5f7bb8e4 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -298,9 +298,11 @@ Path::Polyline Path::CreatePolyline(
   std::vector<PolylineContour::Component> poly_components;
   size_t storage_offset = 0u;
   size_t component_i = 0;
+  bool last_is_empty_contour = false;
 
   for (; component_i < path_components.size(); component_i++) {
-    const auto& path_component = path_components[component_i];
+    auto path_component = path_components[component_i];
+    FML_LOG(ERROR) << static_cast<int>(path_component);
     switch (path_component) {
       case ComponentType::kLinear: {
         poly_components.push_back({
@@ -345,7 +347,8 @@ Path::Polyline Path::CreatePolyline(
         if (component_i == path_components.size() - 1) {
           // If the last component is a contour, that means it's an empty
           // contour, so skip it.
-          continue;
+          last_is_empty_contour = true;
+          break;
         }
         if (!polyline.contours.empty()) {
           polyline.contours.back().start_direction =
@@ -370,8 +373,12 @@ Path::Polyline Path::CreatePolyline(
   }
 
   // Subtract the last storage offset increment so that the storage lookup is
-  // correct.
-  storage_offset -= VerbToOffset(path_components.back());
+  // correct, including potentially an empty contour as well.
+  storage_offset -= VerbToOffset(path_components[component_i]);
+  if (last_is_empty_contour) {
+    component_i--;
+    storage_offset -= VerbToOffset(path_components[component_i]);
+  }
   if (!polyline.contours.empty()) {
     polyline.contours.back().start_direction =
         start_direction.value_or(Vector2(0, -1));

From 027d4d6bf5c3d98af1f39dacbd25bc649ea44e89 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 11 Sep 2024 21:43:06 -0700
Subject: [PATCH 10/14] remove print.

---
 impeller/geometry/path.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index 4fd7a5f7bb8e4..d6f2983ae1295 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -302,7 +302,6 @@ Path::Polyline Path::CreatePolyline(
 
   for (; component_i < path_components.size(); component_i++) {
     auto path_component = path_components[component_i];
-    FML_LOG(ERROR) << static_cast<int>(path_component);
     switch (path_component) {
       case ComponentType::kLinear: {
         poly_components.push_back({

From 833edc5d7ef17b341ac18be375d20bfe42def5bc Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 11 Sep 2024 22:09:08 -0700
Subject: [PATCH 11/14] ++

---
 impeller/geometry/path.cc | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index d6f2983ae1295..e6ca04bdcdcfe 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -373,10 +373,12 @@ Path::Polyline Path::CreatePolyline(
 
   // Subtract the last storage offset increment so that the storage lookup is
   // correct, including potentially an empty contour as well.
-  storage_offset -= VerbToOffset(path_components[component_i]);
-  if (last_is_empty_contour) {
-    component_i--;
+  if (component_i > 0) {
     storage_offset -= VerbToOffset(path_components[component_i]);
+    if (last_is_empty_contour) {
+      component_i--;
+      storage_offset -= VerbToOffset(path_components[component_i]);
+    }
   }
   if (!polyline.contours.empty()) {
     polyline.contours.back().start_direction =

From 6899f86099743d8989cdbaf5800cfbbf34fbcfda Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 12 Sep 2024 10:10:30 -0700
Subject: [PATCH 12/14] ++

---
 impeller/geometry/path.cc | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index e6ca04bdcdcfe..ae7d87fa2266a 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -229,11 +229,7 @@ void Path::EndContour(
   auto& path_points = data_->points;
   // Whenever a contour has ended, extract the exact end direction from
   // the last component.
-  if (polyline.contours.empty()) {
-    return;
-  }
-
-  if (component_index <= 0) {
+  if (polyline.contours.empty() || component_index == 0) {
     return;
   }
 
@@ -298,7 +294,6 @@ Path::Polyline Path::CreatePolyline(
   std::vector<PolylineContour::Component> poly_components;
   size_t storage_offset = 0u;
   size_t component_i = 0;
-  bool last_is_empty_contour = false;
 
   for (; component_i < path_components.size(); component_i++) {
     auto path_component = path_components[component_i];
@@ -346,7 +341,6 @@ Path::Polyline Path::CreatePolyline(
         if (component_i == path_components.size() - 1) {
           // If the last component is a contour, that means it's an empty
           // contour, so skip it.
-          last_is_empty_contour = true;
           break;
         }
         if (!polyline.contours.empty()) {
@@ -373,13 +367,14 @@ Path::Polyline Path::CreatePolyline(
 
   // Subtract the last storage offset increment so that the storage lookup is
   // correct, including potentially an empty contour as well.
+  if (component_i > 0 && path_components.back() == ComponentType::kContour) {
+    storage_offset -= VerbToOffset(ComponentType::kContour);
+    component_i--;
+  }
   if (component_i > 0) {
-    storage_offset -= VerbToOffset(path_components[component_i]);
-    if (last_is_empty_contour) {
-      component_i--;
-      storage_offset -= VerbToOffset(path_components[component_i]);
-    }
+    storage_offset -= VerbToOffset(path_components[component_i - 1]);
   }
+
   if (!polyline.contours.empty()) {
     polyline.contours.back().start_direction =
         start_direction.value_or(Vector2(0, -1));

From 7f4a2c73c7c7546a5673c7c20701e11bdb1c3146 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 12 Sep 2024 22:46:57 -0700
Subject: [PATCH 13/14] ++

---
 impeller/geometry/path.cc | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index ae7d87fa2266a..471f721e6ff86 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -239,6 +239,8 @@ void Path::EndContour(
   poly_components.clear();
 
   size_t previous_index = component_index - 1;
+  storage_offset -= VerbToOffset(path_components[previous_index]);
+
   while (previous_index >= 0 && storage_offset >= 0) {
     const auto& path_component = path_components[previous_index];
     switch (path_component) {
@@ -371,9 +373,6 @@ Path::Polyline Path::CreatePolyline(
     storage_offset -= VerbToOffset(ComponentType::kContour);
     component_i--;
   }
-  if (component_i > 0) {
-    storage_offset -= VerbToOffset(path_components[component_i - 1]);
-  }
 
   if (!polyline.contours.empty()) {
     polyline.contours.back().start_direction =

From 095fe3410243c94eaf6bf58c72ed67f746feb57c Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Tue, 24 Sep 2024 13:06:49 -0700
Subject: [PATCH 14/14] ++

---
 display_list/geometry/dl_path.cc    | 4 ++++
 impeller/geometry/path.cc           | 4 +++-
 impeller/geometry/path_unittests.cc | 7 +++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/display_list/geometry/dl_path.cc b/display_list/geometry/dl_path.cc
index 38fea72130ccf..e0d708999560d 100644
--- a/display_list/geometry/dl_path.cc
+++ b/display_list/geometry/dl_path.cc
@@ -6,6 +6,7 @@
 
 #include "flutter/display_list/geometry/dl_geometry_types.h"
 #include "flutter/impeller/geometry/path_builder.h"
+#include "impeller/geometry/path.h"
 
 namespace flutter {
 
@@ -81,6 +82,9 @@ using FillType = impeller::FillType;
 using Convexity = impeller::Convexity;
 
 Path DlPath::ConvertToImpellerPath(const SkPath& path, const DlPoint& shift) {
+  if (path.isEmpty()) {
+    return impeller::Path{};
+  }
   auto iterator = SkPath::Iter(path, false);
 
   struct PathData {
diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc
index 471f721e6ff86..7979f8399414e 100644
--- a/impeller/geometry/path.cc
+++ b/impeller/geometry/path.cc
@@ -53,7 +53,9 @@ bool Path::IsConvex() const {
 }
 
 bool Path::IsEmpty() const {
-  return data_->points.empty();
+  return data_->points.empty() ||
+         (data_->components.size() == 1 &&
+          data_->components[0] == ComponentType::kContour);
 }
 
 void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
diff --git a/impeller/geometry/path_unittests.cc b/impeller/geometry/path_unittests.cc
index 8d1aed57892f7..f37689f5e8cf3 100644
--- a/impeller/geometry/path_unittests.cc
+++ b/impeller/geometry/path_unittests.cc
@@ -23,6 +23,13 @@ TEST(PathTest, CubicPathComponentPolylineDoesNotIncludePointOne) {
   ASSERT_EQ(polyline.back().y, 40);
 }
 
+TEST(PathTest, EmptyPathWithContour) {
+  PathBuilder builder;
+  auto path = builder.TakePath();
+
+  EXPECT_TRUE(path.IsEmpty());
+}
+
 TEST(PathTest, PathCreatePolyLineDoesNotDuplicatePoints) {
   PathBuilder builder;
   builder.MoveTo({10, 10});