From fd205d9bb99fabe6a2113dac578a211197b856be Mon Sep 17 00:00:00 2001
From: Hans Goudey <hans@blender.org>
Date: Thu, 20 Jun 2024 12:50:52 -0400
Subject: [PATCH] Sculpt: Add function to push multiple undo nodes at once

Pushing multiple nodes at the same time helps to reduce the amount
of time spent waiting for threads to unlock while they manipulate the
nodes map, and equalizes the amount of work per thread, since we
can iterate over just the nodes that need data stored. I observed a
2.6% speedup in the benchmark file from #118145 (0.59s to 0.57s).
---
 source/blender/editors/sculpt_paint/sculpt.cc | 32 +++++++++++--------
 .../editors/sculpt_paint/sculpt_intern.hh     |  1 +
 .../editors/sculpt_paint/sculpt_undo.cc       | 30 +++++++++++++++++
 3 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/source/blender/editors/sculpt_paint/sculpt.cc b/source/blender/editors/sculpt_paint/sculpt.cc
index 2329b3d673c..2f6c8e1e2d2 100644
--- a/source/blender/editors/sculpt_paint/sculpt.cc
+++ b/source/blender/editors/sculpt_paint/sculpt.cc
@@ -3544,38 +3544,46 @@ static void sculpt_topology_update(const Scene & /*scene*/,
   mul_m4_v3(ob.object_to_world().ptr(), location);
 }
 
-static void push_undo_nodes(Object &ob, const Brush &brush, PBVHNode *node)
+static void push_undo_nodes(Object &ob, const Brush &brush, const Span<PBVHNode *> nodes)
 {
   SculptSession &ss = *ob.sculpt;
 
   bool need_coords = ss.cache->supports_gravity;
 
   if (brush.sculpt_tool == SCULPT_TOOL_DRAW_FACE_SETS) {
-    BKE_pbvh_node_mark_update_face_sets(node);
+    for (PBVHNode *node : nodes) {
+      BKE_pbvh_node_mark_update_face_sets(node);
+    }
 
     /* Draw face sets in smooth mode moves the vertices. */
     if (ss.cache->alt_smooth) {
       need_coords = true;
     }
     else {
-      undo::push_node(ob, node, undo::Type::FaceSet);
+      undo::push_nodes(ob, nodes, undo::Type::FaceSet);
     }
   }
   else if (brush.sculpt_tool == SCULPT_TOOL_MASK) {
-    undo::push_node(ob, node, undo::Type::Mask);
-    BKE_pbvh_node_mark_update_mask(node);
+    undo::push_nodes(ob, nodes, undo::Type::Mask);
+    for (PBVHNode *node : nodes) {
+      BKE_pbvh_node_mark_update_mask(node);
+    }
   }
   else if (SCULPT_tool_is_paint(brush.sculpt_tool)) {
-    undo::push_node(ob, node, undo::Type::Color);
-    BKE_pbvh_node_mark_update_color(node);
+    undo::push_nodes(ob, nodes, undo::Type::Color);
+    for (PBVHNode *node : nodes) {
+      BKE_pbvh_node_mark_update_color(node);
+    }
   }
   else {
     need_coords = true;
   }
 
   if (need_coords) {
-    undo::push_node(ob, node, undo::Type::Position);
-    BKE_pbvh_node_mark_positions_update(node);
+    undo::push_nodes(ob, nodes, undo::Type::Position);
+    for (PBVHNode *node : nodes) {
+      BKE_pbvh_node_mark_positions_update(node);
+    }
   }
 }
 
@@ -3685,11 +3693,7 @@ static void do_brush_action(const Scene &scene,
   float location[3];
 
   if (!use_pixels) {
-    threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) {
-      for (const int i : range) {
-        push_undo_nodes(ob, brush, nodes[i]);
-      }
-    });
+    push_undo_nodes(ob, brush, nodes);
   }
 
   if (sculpt_brush_needs_normal(ss, sd, brush)) {
diff --git a/source/blender/editors/sculpt_paint/sculpt_intern.hh b/source/blender/editors/sculpt_paint/sculpt_intern.hh
index eae526c16b9..72075b7411e 100644
--- a/source/blender/editors/sculpt_paint/sculpt_intern.hh
+++ b/source/blender/editors/sculpt_paint/sculpt_intern.hh
@@ -1653,6 +1653,7 @@ namespace blender::ed::sculpt_paint::undo {
  * This is only possible when building an undo step, in between #push_begin and #push_end.
  */
 void push_node(const Object &object, const PBVHNode *node, undo::Type type);
+void push_nodes(Object &object, Span<const PBVHNode *> nodes, undo::Type type);
 
 /**
  * Retrieve the undo data of a given type for the active undo step. For example, this is used to
diff --git a/source/blender/editors/sculpt_paint/sculpt_undo.cc b/source/blender/editors/sculpt_paint/sculpt_undo.cc
index 661f53f729c..f93ae7f5750 100644
--- a/source/blender/editors/sculpt_paint/sculpt_undo.cc
+++ b/source/blender/editors/sculpt_paint/sculpt_undo.cc
@@ -1587,6 +1587,36 @@ void push_node(const Object &object, const PBVHNode *node, Type type)
   fill_node_data(object, node, type, *unode);
 }
 
+void push_nodes(Object &object, const Span<const PBVHNode *> nodes, const Type type)
+{
+  SculptSession &ss = *object.sculpt;
+  if (ss.bm || ELEM(type, Type::DyntopoBegin, Type::DyntopoEnd)) {
+    for (const PBVHNode *node : nodes) {
+      bmesh_push(object, node, type);
+    }
+    return;
+  }
+
+  StepData *step_data = get_step_data();
+
+  Vector<std::pair<const PBVHNode *, Node *>, 32> nodes_to_fill;
+  for (const PBVHNode *node : nodes) {
+    bool newly_added;
+    Node *unode = ensure_node(*step_data, *node, type, newly_added);
+    if (newly_added) {
+      nodes_to_fill.append({node, unode});
+    }
+  }
+
+  ss.needs_flush_to_id = 1;
+
+  threading::parallel_for(nodes_to_fill.index_range(), 1, [&](const IndexRange range) {
+    for (const auto &[node, unode] : nodes_to_fill.as_span().slice(range)) {
+      fill_node_data(object, node, type, *unode);
+    }
+  });
+}
+
 static void save_active_attribute(Object &object, SculptAttrRef *attr)
 {
   Mesh *mesh = BKE_object_get_original_mesh(&object);