Revert "[Impeller] Reland: remove most temporary allocation during polyline generation. (flutter#52180)"

This reverts commit 46ff024.

Author: jonahwilliams

impeller/core/range.h

@@ -7,6 +7,8 @@
 
 #include <cstddef>
 
+#include "flutter/fml/macros.h"
+
 namespace impeller {
 
 struct Range {

impeller/entity/geometry/fill_path_geometry.cc

@@ -36,8 +36,15 @@ GeometryResult FillPathGeometry::GetPositionBuffer(
     };
   }
 
-  VertexBuffer vertex_buffer = renderer.GetTessellator()->TessellateConvex(
-      path_, host_buffer, entity.GetTransform().GetMaxBasisLength());
+  VertexBuffer vertex_buffer;
+
+  auto points = renderer.GetTessellator()->TessellateConvex(
+      path_, entity.GetTransform().GetMaxBasisLength());
+
+  vertex_buffer.vertex_buffer = host_buffer.Emplace(
+      points.data(), points.size() * sizeof(Point), alignof(Point));
+  vertex_buffer.index_buffer = {}, vertex_buffer.vertex_count = points.size();
+  vertex_buffer.index_type = IndexType::kNone;
 
   return GeometryResult{
       .type = PrimitiveType::kTriangleStrip,
@@ -54,6 +61,8 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer(
     const ContentContext& renderer,
     const Entity& entity,
     RenderPass& pass) const {
+  using VS = TextureFillVertexShader;
+
   const auto& bounding_box = path_.GetBoundingBox();
   if (bounding_box.has_value() && bounding_box->IsEmpty()) {
     return GeometryResult{
@@ -71,13 +80,22 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer(
   auto uv_transform =
       texture_coverage.GetNormalizingTransform() * effect_transform;
 
-  VertexBuffer vertex_buffer = renderer.GetTessellator()->TessellateConvex(
-      path_, renderer.GetTransientsBuffer(),
-      entity.GetTransform().GetMaxBasisLength(), uv_transform);
+  auto points = renderer.GetTessellator()->TessellateConvex(
+      path_, entity.GetTransform().GetMaxBasisLength());
+
+  VertexBufferBuilder<VS::PerVertexData> vertex_builder;
+  vertex_builder.Reserve(points.size());
+  for (auto i = 0u; i < points.size(); i++) {
+    VS::PerVertexData data;
+    data.position = points[i];
+    data.texture_coords = uv_transform * points[i];
+    vertex_builder.AppendVertex(data);
+  }
 
   return GeometryResult{
       .type = PrimitiveType::kTriangleStrip,
-      .vertex_buffer = vertex_buffer,
+      .vertex_buffer =
+          vertex_builder.CreateVertexBuffer(renderer.GetTransientsBuffer()),
       .transform = entity.GetShaderTransform(pass),
       .mode = GetResultMode(),
   };

impeller/geometry/geometry_benchmarks.cc

@@ -59,22 +59,39 @@ template <class... Args>
 static void BM_Polyline(benchmark::State& state, Args&&... args) {
   auto args_tuple = std::make_tuple(std::move(args)...);
   auto path = std::get<Path>(args_tuple);
+  bool tessellate = std::get<bool>(args_tuple);
 
   size_t point_count = 0u;
   size_t single_point_count = 0u;
   auto points = std::make_unique<std::vector<Point>>();
   points->reserve(2048);
   while (state.KeepRunning()) {
-    auto polyline = path.CreatePolyline(
-        // Clang-tidy doesn't know that the points get moved back before
-        // getting moved again in this loop.
-        // NOLINTNEXTLINE(clang-analyzer-cplusplus.Move)
-        1.0f, std::move(points),
-        [&points](Path::Polyline::PointBufferPtr reclaimed) {
-          points = std::move(reclaimed);
-        });
-    single_point_count = polyline.points->size();
-    point_count += single_point_count;
+    if (tessellate) {
+      tess.Tessellate(path, 1.0f,
+                      [&point_count, &single_point_count](
+                          const float* vertices, size_t vertices_count,
+                          const uint16_t* indices, size_t indices_count) {
+                        if (indices_count > 0) {
+                          single_point_count = indices_count;
+                          point_count += indices_count;
+                        } else {
+                          single_point_count = vertices_count;
+                          point_count += vertices_count;
+                        }
+                        return true;
+                      });
+    } else {
+      auto polyline = path.CreatePolyline(
+          // Clang-tidy doesn't know that the points get moved back before
+          // getting moved again in this loop.
+          // NOLINTNEXTLINE(clang-analyzer-cplusplus.Move)
+          1.0f, std::move(points),
+          [&points](Path::Polyline::PointBufferPtr reclaimed) {
+            points = std::move(reclaimed);
+          });
+      single_point_count = polyline.points->size();
+      point_count += single_point_count;
+    }
   }
   state.counters["SinglePointCount"] = single_point_count;
   state.counters["TotalPointCount"] = point_count;
@@ -138,13 +155,11 @@ static void BM_Convex(benchmark::State& state, Args&&... args) {
   size_t point_count = 0u;
   size_t single_point_count = 0u;
   auto points = std::make_unique<std::vector<Point>>();
-  auto indices = std::make_unique<std::vector<uint16_t>>();
   points->reserve(2048);
-  indices->reserve(2048);
   while (state.KeepRunning()) {
-    tess.TessellateConvexInternal(path, *points, *indices, 1.0f);
-    single_point_count = indices->size();
-    point_count += indices->size();
+    auto points = tess.TessellateConvex(path, 1.0f);
+    single_point_count = points.size();
+    point_count += points.size();
   }
   state.counters["SinglePointCount"] = single_point_count;
   state.counters["TotalPointCount"] = point_count;
@@ -167,17 +182,27 @@ static void BM_Convex(benchmark::State& state, Args&&... args) {
   MAKE_STROKE_BENCHMARK_CAPTURE_CAPS_JOINS(path, _uvNoTx, UVMode::kUVRect)
 
 BENCHMARK_CAPTURE(BM_Polyline, cubic_polyline, CreateCubic(true), false);
+BENCHMARK_CAPTURE(BM_Polyline, cubic_polyline_tess, CreateCubic(true), true);
 BENCHMARK_CAPTURE(BM_Polyline,
                   unclosed_cubic_polyline,
                   CreateCubic(false),
                   false);
+BENCHMARK_CAPTURE(BM_Polyline,
+                  unclosed_cubic_polyline_tess,
+                  CreateCubic(false),
+                  true);
 MAKE_STROKE_BENCHMARK_CAPTURE_UVS(Cubic);
 
 BENCHMARK_CAPTURE(BM_Polyline, quad_polyline, CreateQuadratic(true), false);
+BENCHMARK_CAPTURE(BM_Polyline, quad_polyline_tess, CreateQuadratic(true), true);
 BENCHMARK_CAPTURE(BM_Polyline,
                   unclosed_quad_polyline,
                   CreateQuadratic(false),
                   false);
+BENCHMARK_CAPTURE(BM_Polyline,
+                  unclosed_quad_polyline_tess,
+                  CreateQuadratic(false),
+                  true);
 MAKE_STROKE_BENCHMARK_CAPTURE_UVS(Quadratic);
 
 BENCHMARK_CAPTURE(BM_Convex, rrect_convex, CreateRRect(), true);

impeller/geometry/path.cc

@@ -349,54 +349,4 @@ std::optional<Rect> Path::GetTransformedBoundingBox(
   return bounds->TransformBounds(transform);
 }
 
-void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
-  auto& path_components = data_->components;
-  auto& path_points = data_->points;
-
-  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: {
-        const LinearPathComponent* linear =
-            reinterpret_cast<const LinearPathComponent*>(
-                &path_points[path_component.index]);
-        writer.Write(linear->p2);
-        break;
-      }
-      case ComponentType::kQuadratic: {
-        const QuadraticPathComponent* quad =
-            reinterpret_cast<const QuadraticPathComponent*>(
-                &path_points[path_component.index]);
-        quad->ToLinearPathComponents(scale, writer);
-        break;
-      }
-      case ComponentType::kCubic: {
-        const CubicPathComponent* cubic =
-            reinterpret_cast<const CubicPathComponent*>(
-                &path_points[path_component.index]);
-        cubic->ToLinearPathComponents(scale, writer);
-        break;
-      }
-      case 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.
-          continue;
-        }
-        writer.EndContour();
-
-        // Insert contour start.
-        const auto& next_component = path_components[component_i + 1];
-        // It doesn't matter what type this is as all structs are laid out
-        // with p1 as the first member.
-        const LinearPathComponent* linear =
-            reinterpret_cast<const LinearPathComponent*>(
-                &path_points[next_component.index]);
-        writer.Write(linear->p1);
-        break;
-    }
-  }
-}
-
 }  // namespace impeller

impeller/geometry/path.h

@@ -11,7 +11,6 @@
 #include <vector>
 
 #include "impeller/geometry/path_component.h"
-#include "impeller/geometry/rect.h"
 
 namespace impeller {
 
@@ -169,13 +168,6 @@ class Path {
           std::make_unique<std::vector<Point>>(),
       Polyline::ReclaimPointBufferCallback reclaim = nullptr) const;
 
-  /// Generate a polyline into the temporary storage held by the [writer].
-  ///
-  /// It is suitable to use the max basis length of the matrix used to transform
-  /// the path. If the provided scale is 0, curves will revert to straight
-  /// lines.
-  void WritePolyline(Scalar scale, VertexWriter& writer) const;
-
   std::optional<Rect> GetBoundingBox() const;
 
   std::optional<Rect> GetTransformedBoundingBox(const Matrix& transform) const;

impeller/geometry/path_component.cc

@@ -10,67 +10,6 @@
 
 namespace impeller {
 
-VertexWriter::VertexWriter(std::vector<Point>& points,
-                           std::vector<uint16_t>& indices,
-                           std::optional<Matrix> uv_transform)
-    : points_(points), indices_(indices), uv_transform_(uv_transform) {}
-
-void VertexWriter::EndContour() {
-  if (points_.size() == 0u || contour_start_ == points_.size() - 1) {
-    // Empty or first contour.
-    return;
-  }
-
-  auto start = contour_start_;
-  auto end = points_.size() - 1;
-  // All filled paths are drawn as if they are closed, but if
-  // there is an explicit close then a lineTo to the origin
-  // is inserted. This point isn't strictly necesary to
-  // correctly render the shape and can be dropped.
-  if (points_[end] == points_[start]) {
-    end--;
-  }
-
-  if (contour_start_ > 0) {
-    // Triangle strip break.
-    indices_.emplace_back(indices_.back());
-    indices_.emplace_back(start);
-    indices_.emplace_back(start);
-
-    // If the contour has an odd number of points, insert an extra point when
-    // bridging to the next contour to preserve the correct triangle winding
-    // order.
-    if (previous_contour_odd_points_) {
-      indices_.emplace_back(start);
-    }
-  } else {
-    indices_.emplace_back(start);
-  }
-
-  size_t a = start + 1;
-  size_t b = end;
-  while (a < b) {
-    indices_.emplace_back(a);
-    indices_.emplace_back(b);
-    a++;
-    b--;
-  }
-  if (a == b) {
-    indices_.emplace_back(a);
-    previous_contour_odd_points_ = false;
-  } else {
-    previous_contour_odd_points_ = true;
-  }
-  contour_start_ = points_.size();
-}
-
-void VertexWriter::Write(Point point) {
-  points_.emplace_back(point);
-  if (uv_transform_.has_value()) {
-    points_.emplace_back(*uv_transform_ * point);
-  }
-}
-
 /*
  *  Based on: https://en.wikipedia.org/wiki/B%C3%A9zier_curve#Specific_cases
  */
@@ -180,16 +119,6 @@ void QuadraticPathComponent::ToLinearPathComponents(
   proc(p2);
 }
 
-void QuadraticPathComponent::ToLinearPathComponents(
-    Scalar scale,
-    VertexWriter& writer) const {
-  Scalar line_count = std::ceilf(ComputeQuadradicSubdivisions(scale, *this));
-  for (size_t i = 1; i < line_count; i += 1) {
-    writer.Write(Solve(i / line_count));
-  }
-  writer.Write(p2);
-}
-
 std::vector<Point> QuadraticPathComponent::Extrema() const {
   CubicPathComponent elevated(*this);
   return elevated.Extrema();
@@ -260,15 +189,6 @@ void CubicPathComponent::ToLinearPathComponents(Scalar scale,
   proc(p2);
 }
 
-void CubicPathComponent::ToLinearPathComponents(Scalar scale,
-                                                VertexWriter& writer) const {
-  Scalar line_count = std::ceilf(ComputeCubicSubdivisions(scale, *this));
-  for (size_t i = 1; i < line_count; i++) {
-    writer.Write(Solve(i / line_count));
-  }
-  writer.Write(p2);
-}
-
 static inline bool NearEqual(Scalar a, Scalar b, Scalar epsilon) {
   return (a > (b - epsilon)) && (a < (b + epsilon));
 }

impeller/geometry/path_component.h

@@ -10,34 +10,12 @@
 #include <variant>
 #include <vector>
 
-#include "impeller/geometry/matrix.h"
 #include "impeller/geometry/point.h"
+#include "impeller/geometry/rect.h"
 #include "impeller/geometry/scalar.h"
 
 namespace impeller {
 
-/// @brief An interface for generating a multi contour polyline as a triangle
-///        strip.
-class VertexWriter {
- public:
-  explicit VertexWriter(std::vector<Point>& points,
-                        std::vector<uint16_t>& indices,
-                        std::optional<Matrix> uv_transform);
-
-  ~VertexWriter() = default;
-
-  void EndContour();
-
-  void Write(Point point);
-
- private:
-  bool previous_contour_odd_points_ = false;
-  size_t contour_start_ = 0u;
-  std::vector<Point>& points_;
-  std::vector<uint16_t>& indices_;
-  std::optional<Matrix> uv_transform_;
-};
-
 struct LinearPathComponent {
   Point p1;
   Point p2;
@@ -86,8 +64,6 @@ struct QuadraticPathComponent {
 
   void ToLinearPathComponents(Scalar scale_factor, const PointProc& proc) const;
 
-  void ToLinearPathComponents(Scalar scale, VertexWriter& writer) const;
-
   std::vector<Point> Extrema() const;
 
   bool operator==(const QuadraticPathComponent& other) const {
@@ -133,8 +109,6 @@ struct CubicPathComponent {
 
   void ToLinearPathComponents(Scalar scale, const PointProc& proc) const;
 
-  void ToLinearPathComponents(Scalar scale, VertexWriter& writer) const;
-
   CubicPathComponent Subsegment(Scalar t0, Scalar t1) const;
 
   bool operator==(const CubicPathComponent& other) const {