Allow brush tips to have multiple outlines (e.g. for particle effects)

Rolling forward after rollback in f8502c0 (which incorrectly repeats a truncated version the original change description from ce4c535).

In `StrokeShapeBuilder`, this involves losing the assumption that the number of outlines is exactly equal to the number of brush tips, since particle brushes have more than one outline per tip. Without that assumption, it can also discard empty outlines a bit earlier.

In `BrushTipExtruder` this involves:

* Keeping a vector of outlines instead of a single outline (since the usual case is just one, this uses `absl::InlinedVector<StrokeOutline, 1>`).

* Keep track of the number of extrusion breaks in `Geometry`.

* Add accessors to `Geometry` to get the number of break-points and the index counts at the lastest break-point. Use those in `Restore` to restore the saved state and in `AppendNewIndices` to get the start points in the latest outline.

* Pull calls to `AppendNewIndices` and `Geometry::UpdateMeshDerivatives` into `ExtrudeBreakPoint` because the former computes the latest outline and the latter may update indices, so it needs to be called first.

* Similarly, move forward the "first mutated" indices in `Geometry::AddExtrusionBreak`, since those track the first mutated vertices that haven't been recorded in the outline. These are renamed to `first_mutated_{left,right}_index_offset_in_current_partition_`, since these no longer track the same thing as `first_mutated_left_index_` and `first_mutated_right_index_` (which still considers a whole stroke update).

In `InProgressStroke`, `GetIndexOutlines` is renamed to `GetCoatOutlines` for clarity, since that makes it clear what the index is referring to.

PiperOrigin-RevId: 712619312

Author: sfreilich

ink/geometry/partitioned_mesh.h

@@ -188,7 +188,10 @@ class PartitionedMesh {
   absl::Span<const Mesh> Meshes() const;
 
   // Returns the number of outlines (which may be zero) in render group
-  // `group_index`.
+  // `group_index`. Groups with discontinuous geometry will always have multiple
+  // outlines, but even a group with continuous geometry may be drawn with
+  // multiple overlapping outlines when this improves rendering quality or
+  // performance.
   //
   // This method CHECK-fails if `group_index` >= `RenderGroupCount()`.
   uint32_t OutlineCount(uint32_t group_index) const;

ink/rendering/skia/native/skia_renderer.cc

@@ -134,7 +134,7 @@ absl::StatusOr<SkiaRenderer::Drawable> SkiaRenderer::CreateDrawable(
 
     if (UsePathRendering(context, brush->GetCoats()[coat_index].paint)) {
       drawables.push_back(PathDrawable(
-          stroke.GetMesh(coat_index), stroke.GetIndexOutlines(coat_index),
+          stroke.GetMesh(coat_index), stroke.GetCoatOutlines(coat_index),
           brush->GetColor(), OpacityMultiplierForPath(*brush, coat_index)));
       continue;
     }

ink/strokes/BUILD.bazel

@@ -110,6 +110,7 @@ cc_library(
         "//ink/strokes/input:stroke_input",
         "//ink/strokes/input:stroke_input_batch",
         "//ink/strokes/input/internal:stroke_input_validation_helpers",
+        "//ink/strokes/internal:stroke_outline",
         "//ink/strokes/internal:stroke_shape_builder",
         "//ink/strokes/internal:stroke_shape_update",
         "//ink/strokes/internal:stroke_vertex",

ink/strokes/in_progress_stroke.cc

@@ -272,7 +272,7 @@ Stroke InProgressStroke::CopyToStroke(
 
     mesh_groups.push_back({
         .mesh = &GetMesh(coat_index),
-        .outlines = GetIndexOutlines(coat_index),
+        .outlines = GetCoatOutlines(coat_index),
         .omit_attributes = omit_attributes[coat_index],
         .packing_params = custom_packing_arrays[coat_index].Values(),
     });

ink/strokes/in_progress_stroke.h

@@ -28,6 +28,7 @@
 #include "ink/geometry/envelope.h"
 #include "ink/geometry/mutable_mesh.h"
 #include "ink/strokes/input/stroke_input_batch.h"
+#include "ink/strokes/internal/stroke_outline.h"
 #include "ink/strokes/internal/stroke_shape_builder.h"
 #include "ink/strokes/stroke.h"
 #include "ink/types/duration.h"
@@ -204,19 +205,22 @@ class InProgressStroke {
   // specified coat of paint.
   const Envelope& GetMeshBounds(uint32_t coat_index) const;
 
-  // Returns one or more spans of vertex indices, one for each of the stroke
-  // outlines for the specified coat of paint.
+  // Returns zero or more non-empty spans of vertex indices, one for each of the
+  // stroke outlines for the specified coat of paint. There will be at least one
+  // outline for each brush tip if the stroke is non-empty. Stroke coats with
+  // discontinuous geometry will always have multiple outlines, but even a coat
+  // with continuous geometry may be drawn with multiple overlapping outlines
+  // when this improves rendering quality or performance.
   //
   // Every returned index value can be used to get an outline position from the
   // `MutableMesh` returned by `GetMesh()`.
   //
-  // For each non-empty span of indices, the first and last elements reference
-  // vertices at the end of the stroke. The indices traverse the mesh such that
-  // the outline has a negative winding number when viewed from the positive
-  // z-axis. I.e. the outline positions would be appear in clockwise order if
-  // the y-axis points up, and in counter-clockwise order if the y-axis points
-  // down.
-  absl::Span<const absl::Span<const uint32_t>> GetIndexOutlines(
+  // The first and last elements in each span reference the vertices at the end
+  // of the stroke outline. The indices traverse the mesh such that the outline
+  // has a negative winding number when viewed from the positive z-axis. That
+  // is, the outline positions are in clockwise order if the y-axis points up,
+  // counter-clockwise order if the y-axis points down.
+  absl::Span<const absl::Span<const uint32_t>> GetCoatOutlines(
       uint32_t coat_index) const;
 
   // Returns the bounding rectangle of mesh positions added, modified, or
@@ -305,9 +309,9 @@ inline const Envelope& InProgressStroke::GetMeshBounds(
 }
 
 inline absl::Span<const absl::Span<const uint32_t>>
-InProgressStroke::GetIndexOutlines(uint32_t coat_index) const {
+InProgressStroke::GetCoatOutlines(uint32_t coat_index) const {
   ABSL_CHECK_LT(coat_index, BrushCoatCount());
-  return shape_builders_[coat_index].GetIndexOutlines();
+  return shape_builders_[coat_index].GetOutlines();
 }
 
 inline const Envelope& InProgressStroke::GetUpdatedRegion() const {

ink/strokes/in_progress_stroke_test.cc

@@ -154,7 +154,7 @@ MATCHER_P3(UpdateShapeFailsAndDoesNotModifyStroke, current_elapsed_time,
     meshes_before_update.push_back(arg->GetMesh(i).Clone());
     bounds_before_update.push_back(arg->GetMeshBounds(i));
     std::vector<std::vector<uint32_t>> outlines;
-    for (absl::Span<const uint32_t> outline : arg->GetIndexOutlines(i)) {
+    for (absl::Span<const uint32_t> outline : arg->GetCoatOutlines(i)) {
       outlines.push_back(std::vector<uint32_t>(outline.begin(), outline.end()));
     }
     index_outlines_before_update.push_back(std::move(outlines));
@@ -186,7 +186,7 @@ MATCHER_P3(UpdateShapeFailsAndDoesNotModifyStroke, current_elapsed_time,
         !ExplainMatchResult(EnvelopeEq(bounds_before_update[i]),
                             arg->GetMeshBounds(i), result_listener) ||
         !ExplainMatchResult(ElementsAreArray(index_outlines_before_update[i]),
-                            arg->GetIndexOutlines(i), result_listener)) {
+                            arg->GetCoatOutlines(i), result_listener)) {
       return false;
     }
   }
@@ -263,7 +263,7 @@ TEST(InProgressStrokeTest, StartAfterConstruction) {
   EXPECT_EQ(stroke.GetMesh(0).VertexCount(), 0u);
   EXPECT_EQ(stroke.GetMesh(0).TriangleCount(), 0u);
   EXPECT_TRUE(stroke.GetMeshBounds(0).IsEmpty());
-  EXPECT_THAT(stroke.GetIndexOutlines(0), ElementsAre(IsEmpty()));
+  EXPECT_THAT(stroke.GetCoatOutlines(0), IsEmpty());
   EXPECT_TRUE(stroke.GetUpdatedRegion().IsEmpty());
   EXPECT_FALSE(stroke.InputsAreFinished());
   EXPECT_FALSE(stroke.NeedsUpdate());
@@ -338,7 +338,7 @@ TEST(InProgressStrokeTest, EmptyEnqueueInputsAndUpdateAfterStart) {
   EXPECT_EQ(stroke.GetMesh(0).VertexCount(), 0u);
   EXPECT_EQ(stroke.GetMesh(0).TriangleCount(), 0u);
   EXPECT_TRUE(stroke.GetMeshBounds(0).IsEmpty());
-  EXPECT_THAT(stroke.GetIndexOutlines(0), ElementsAre(IsEmpty()));
+  EXPECT_THAT(stroke.GetCoatOutlines(0), IsEmpty());
   EXPECT_TRUE(stroke.GetUpdatedRegion().IsEmpty());
   EXPECT_FALSE(stroke.NeedsUpdate());
 }
@@ -392,7 +392,7 @@ TEST(InProgressStrokeTest, NonEmptyInputs) {
               Optional(RectNear(*CalculateEnvelope(stroke.GetMesh(0)).AsRect(),
                                 0.0001)));
 
-  EXPECT_THAT(stroke.GetIndexOutlines(0), ElementsAre(Not(IsEmpty())));
+  EXPECT_THAT(stroke.GetCoatOutlines(0), ElementsAre(Not(IsEmpty())));
   EXPECT_THAT(stroke.GetUpdatedRegion().AsRect(),
               Optional(RectNear(
                   Rect::FromTwoPoints({-0.88, 0.13}, {4.90, 5.87}), 0.01)));
@@ -419,7 +419,7 @@ TEST(InProgressStrokeTest, NonEmptyInputs) {
               Optional(RectNear(*CalculateEnvelope(stroke.GetMesh(0)).AsRect(),
                                 0.0001)));
 
-  EXPECT_THAT(stroke.GetIndexOutlines(0), ElementsAre(Not(IsEmpty())));
+  EXPECT_THAT(stroke.GetCoatOutlines(0), ElementsAre(Not(IsEmpty())));
   EXPECT_THAT(stroke.GetUpdatedRegion().AsRect(),
               Optional(RectNear(
                   Rect::FromTwoPoints({-0.88, -2.88}, {5.88, 5.87}), 0.01)));
@@ -447,7 +447,7 @@ TEST(InProgressStrokeTest, ExtendWithEmptyPredictedButNonEmptyReal) {
       EnvelopeNear(CalculateEnvelope(stroke.GetMesh(0)).AsRect().value(),
                    0.0001));
 
-  EXPECT_THAT(stroke.GetIndexOutlines(0), ElementsAre(Not(IsEmpty())));
+  EXPECT_THAT(stroke.GetCoatOutlines(0), ElementsAre(Not(IsEmpty())));
   EXPECT_THAT(stroke.GetUpdatedRegion().AsRect(),
               Optional(RectNear(
                   Rect::FromTwoPoints({-0.88, 0.13}, {4.87, 3.87}), 0.01)));
@@ -470,7 +470,7 @@ TEST(InProgressStrokeTest, ExtendWithEmptyPredictedButNonEmptyReal) {
       EnvelopeNear(CalculateEnvelope(stroke.GetMesh(0)).AsRect().value(),
                    0.0001));
 
-  EXPECT_THAT(stroke.GetIndexOutlines(0), ElementsAre(Not(IsEmpty())));
+  EXPECT_THAT(stroke.GetCoatOutlines(0), ElementsAre(Not(IsEmpty())));
   EXPECT_THAT(stroke.GetUpdatedRegion().AsRect(),
               Optional(RectNear(
                   Rect::FromTwoPoints({-0.88, -1.87}, {4.90, 3.88}), 0.01)));
@@ -726,7 +726,7 @@ TEST(InProgressStrokeTest, StartAfterExtendingStroke) {
   ASSERT_NE(stroke.GetMesh(0).VertexCount(), 0u);
   ASSERT_NE(stroke.GetMesh(0).TriangleCount(), 0u);
   ASSERT_FALSE(stroke.GetMeshBounds(0).IsEmpty());
-  ASSERT_THAT(stroke.GetIndexOutlines(0), ElementsAre(Not(IsEmpty())));
+  ASSERT_THAT(stroke.GetCoatOutlines(0), ElementsAre(Not(IsEmpty())));
   ASSERT_FALSE(stroke.GetUpdatedRegion().IsEmpty());
 
   stroke.Start(replacement_brush);
@@ -737,7 +737,7 @@ TEST(InProgressStrokeTest, StartAfterExtendingStroke) {
   EXPECT_EQ(stroke.GetMesh(0).VertexCount(), 0u);
   EXPECT_EQ(stroke.GetMesh(0).TriangleCount(), 0u);
   EXPECT_TRUE(stroke.GetMeshBounds(0).IsEmpty());
-  EXPECT_THAT(stroke.GetIndexOutlines(0), ElementsAre(IsEmpty()));
+  EXPECT_THAT(stroke.GetCoatOutlines(0), IsEmpty());
   EXPECT_TRUE(stroke.GetUpdatedRegion().IsEmpty());
 }
 

ink/strokes/internal/BUILD.bazel

@@ -363,6 +363,7 @@ cc_library(
         "//ink/strokes/internal/brush_tip_extruder:mutable_mesh_view",
         "//ink/strokes/internal/brush_tip_extruder:side",
         "@com_google_absl//absl/algorithm:container",
+        "@com_google_absl//absl/container:inlined_vector",
         "@com_google_absl//absl/log:absl_check",
         "@com_google_absl//absl/types:span",
     ],
@@ -454,18 +455,17 @@ cc_library(
         ":brush_tip_extruder",
         ":brush_tip_modeler",
         ":stroke_input_modeler",
+        ":stroke_outline",
         ":stroke_shape_update",
         ":stroke_vertex",
         "//ink/brush:brush_coat",
         "//ink/brush:brush_family",
         "//ink/brush:brush_paint",
-        "//ink/brush:brush_tip",
         "//ink/geometry:envelope",
         "//ink/geometry:mutable_mesh",
         "//ink/strokes/input:stroke_input_batch",
         "//ink/types:duration",
         "@com_google_absl//absl/container:inlined_vector",
-        "@com_google_absl//absl/log",
         "@com_google_absl//absl/log:absl_check",
         "@com_google_absl//absl/types:span",
     ],

ink/strokes/internal/brush_tip_extruder.cc

@@ -68,23 +68,12 @@ void BrushTipExtruder::StartStroke(float brush_epsilon,
   deleted_save_point_extrusions_.clear();
   geometry_.Reset(MutableMeshView(mesh));
   bounds_ = {};
-  outline_.TruncateIndices({0, 0});
+  outlines_.resize(1);
+  outlines_[0].TruncateIndices({0, 0});
 }
 
 namespace {
 
-// Appends left and right indices to `outline` that have been added to
-// `geometry` since the last call to `BrushTipExtruder::Restore()`.
-void AppendNewIndicesToOutline(const Geometry& geometry,
-                               StrokeOutline& outline) {
-  StrokeOutline::IndexCounts counts = outline.GetIndexCounts();
-  auto new_left_indices =
-      absl::MakeSpan(geometry.LeftSide().indices).subspan(counts.left);
-  auto new_right_indices =
-      absl::MakeSpan(geometry.RightSide().indices).subspan(counts.right);
-  outline.AppendNewIndices(new_left_indices, new_right_indices);
-}
-
 StrokeShapeUpdate ConstructUpdate(const Geometry& geometry,
                                   uint32_t triangle_count_before_update,
                                   uint32_t vertex_count_before_update) {
@@ -136,9 +125,6 @@ StrokeShapeUpdate BrushTipExtruder::ExtendStroke(
   }
 
   ExtrudeBreakPoint();
-  geometry_.UpdateMeshDerivatives();
-
-  AppendNewIndicesToOutline(geometry_, outline_);
   UpdateCurrentBounds();
   return ConstructUpdate(geometry_, triangle_count_before_update,
                          vertex_count_before_update);
@@ -225,13 +211,35 @@ void BrushTipExtruder::Save() {
   geometry_.SetSavePoint();
 }
 
+void BrushTipExtruder::TruncateOutlines() {
+  ABSL_DCHECK_LE(geometry_.ExtrusionBreakCount(), outlines_.size());
+  // If we're already working on the outline after the last remaining break
+  // point, drop outlines after that (if there are any) and truncate the last
+  // one.
+  uint32_t max_num_outlines = geometry_.ExtrusionBreakCount() + 1;
+  if (outlines_.size() >= max_num_outlines) {
+    outlines_.resize(max_num_outlines);
+    const Geometry::IndexCounts& last_extrusion_break_offset =
+        geometry_.IndexCountsAtLastExtrusionBreak();
+    ABSL_DCHECK_GE(geometry_.FirstMutatedLeftIndexOffsetInCurrentPartition(),
+                   last_extrusion_break_offset.left);
+    ABSL_DCHECK_GE(geometry_.FirstMutatedRightIndexOffsetInCurrentPartition(),
+                   last_extrusion_break_offset.right);
+    outlines_.back().TruncateIndices({
+        .left = geometry_.FirstMutatedLeftIndexOffsetInCurrentPartition() -
+                last_extrusion_break_offset.left,
+        .right = geometry_.FirstMutatedRightIndexOffsetInCurrentPartition() -
+                 last_extrusion_break_offset.right,
+    });
+  }
+}
+
 void BrushTipExtruder::Restore() {
   extrusions_.resize(saved_extrusion_data_count_);
   absl::c_copy(deleted_save_point_extrusions_,
                extrusions_.end() - deleted_save_point_extrusions_.size());
   geometry_.RevertToSavePoint();
-  outline_.TruncateIndices({.left = geometry_.FirstMutatedLeftIndexOffset(),
-                            .right = geometry_.FirstMutatedRightIndexOffset()});
+  TruncateOutlines();
 }
 
 void BrushTipExtruder::ClearSinceLastExtrusionBreak(
@@ -268,8 +276,7 @@ void BrushTipExtruder::ClearSinceLastExtrusionBreak(
 
   extrusions_.erase(first_extrusion_to_erase, extrusions_.end());
   geometry_.ClearSinceLastExtrusionBreak();
-  outline_.TruncateIndices({.left = geometry_.FirstMutatedLeftIndexOffset(),
-                            .right = geometry_.FirstMutatedRightIndexOffset()});
+  TruncateOutlines();
   ClearCachedPartialBounds();
 }
 
@@ -507,8 +514,45 @@ void BrushTipExtruder::ExtrudeBreakPoint() {
   ExtrudeGeometry(current_extrusion_points_, extrusions_.back().GetState(),
                   simplification_threshold_, is_winding_texture_particle_brush_,
                   geometry_);
+
+  // If no new geometry was added after the last breakpoint, we don't need to
+  // do anything.
+  Geometry::IndexCounts counts_at_last_break =
+      geometry_.IndexCountsAtLastExtrusionBreak();
+
+  if (geometry_.LeftSide().indices.size() <= counts_at_last_break.left &&
+      geometry_.RightSide().indices.size() <= counts_at_last_break.right) {
+    return;
+  }
+
+  // Close the outline and update the mesh derivatives. This needs to be done
+  // after recording the counts (because we want the counts back to the previous
+  // break point) but before adding the new outline (because
+  // UpdateMeshDerivatives can modify the outline indices). Possibly we're
+  // adding a new empty outline here, possibly we're just filling in the end
+  // of a last outline we already allocated.
+  if (outlines_.size() == geometry_.ExtrusionBreakCount()) {
+    outlines_.emplace_back();
+  }
   geometry_.AddExtrusionBreak();
   extrusions_.emplace_back(BrushTipExtrusion::BreakPoint{});
+  geometry_.UpdateMeshDerivatives();
+
+  // Complete the outline before the last breakpoint.
+  StrokeOutline& outline = outlines_.back();
+  ABSL_DCHECK_EQ(geometry_.ExtrusionBreakCount(), outlines_.size());
+  StrokeOutline::IndexCounts outline_counts = outline.GetIndexCounts();
+  ABSL_DCHECK_GE(geometry_.LeftSide().indices.size(),
+                 counts_at_last_break.left + outline_counts.left);
+  absl::Span<const uint32_t> new_left_indices =
+      absl::MakeSpan(geometry_.LeftSide().indices)
+          .subspan(counts_at_last_break.left + outline_counts.left);
+  ABSL_DCHECK_GE(geometry_.RightSide().indices.size(),
+                 counts_at_last_break.right + outline_counts.right);
+  absl::Span<const uint32_t> new_right_indices =
+      absl::MakeSpan(geometry_.RightSide().indices)
+          .subspan(counts_at_last_break.right + outline_counts.right);
+  outline.AppendNewIndices(new_left_indices, new_right_indices);
 }
 
 }  // namespace ink::strokes_internal

ink/strokes/internal/brush_tip_extruder.h

@@ -19,6 +19,7 @@
 #include <cstdint>
 #include <vector>
 
+#include "absl/container/inlined_vector.h"
 #include "absl/types/span.h"
 #include "ink/geometry/envelope.h"
 #include "ink/geometry/mutable_mesh.h"
@@ -96,15 +97,10 @@ class BrushTipExtruder {
   // Returns the bounding region of positions extruded into the current mesh.
   const Envelope& GetBounds() const;
 
-  // Returns the current outline indices.
-  //
-  // If the returned span is not empty, its first and last elements reference
-  // vertices at the end of the stroke. The indices traverse the mesh such that
-  // the outline has a negative winding number when viewed from the positive
-  // z-axis. I.e. the outline will be appear in clockwise orientation if the
-  // y-axis points up, and in counter-clockwise orientation if the y-axis points
-  // down.
-  absl::Span<const uint32_t> GetOutlineIndices() const;
+  // Returns the outlines for the current brush tip. This can include empty
+  // outlines. (In particular, we greedily allocate the first outline, so
+  // that is empty if the stroke is empty.)
+  absl::Span<const StrokeOutline> GetOutlines() const;
 
  private:
   // Data used to incrementally update the bounds of geometry extruded into the
@@ -155,6 +151,9 @@ class BrushTipExtruder {
   // Restores the saved state of the extruder.
   void Restore();
 
+  // Truncate outlines to match the current geometry.
+  void TruncateOutlines();
+
   // Clears the geometry, extrusions, and outline indices since the last break
   // in extrusion. This is either since the last explicitly added break-point,
   // or since the implicit break-point at start of the stroke.
@@ -220,7 +219,7 @@ class BrushTipExtruder {
   ExtrusionPoints current_extrusion_points_;
   brush_tip_extruder_internal::Geometry geometry_;
   Bounds bounds_;
-  StrokeOutline outline_;
+  absl::InlinedVector<StrokeOutline, 1> outlines_;
 };
 
 // ---------------------------------------------------------------------------
@@ -230,8 +229,8 @@ inline const Envelope& BrushTipExtruder::GetBounds() const {
   return bounds_.current;
 }
 
-inline absl::Span<const uint32_t> BrushTipExtruder::GetOutlineIndices() const {
-  return outline_.GetIndices();
+inline absl::Span<const StrokeOutline> BrushTipExtruder::GetOutlines() const {
+  return outlines_;
 }
 
 }  // namespace ink::strokes_internal