Refactor: Sculpt: Separate geometry and per-node undo step code paths

Sometimes undo data is stored per PBVH node, and sometimes the entire
geometry is stored. Currently these go through the same `undo::push_node`
function, which is confusing and limits improvements we can make to the
pushing of per-node undo data.

source/blender/editors/sculpt_paint/paint_hide.cc

@@ -1277,11 +1277,12 @@ static void partialvis_gesture_update_bmesh(gesture::GestureData &gesture_data)
       *gesture_data.vc.obact, gesture_data.nodes, operation->action, selection_test_fn);
 }
 
-static void hide_show_begin(bContext &C, gesture::GestureData & /*gesture_data*/)
+static void hide_show_begin(bContext &C, wmOperator &op, gesture::GestureData & /*gesture_data*/)
 {
   Object *ob = CTX_data_active_object(&C);
   Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(&C);
 
+  undo::push_begin(*ob, &op);
   BKE_sculpt_object_pbvh_ensure(depsgraph, ob);
 }
 
@@ -1305,6 +1306,7 @@ static void hide_show_end(bContext &C, gesture::GestureData &gesture_data)
 {
   SCULPT_topology_islands_invalidate(*gesture_data.vc.obact->sculpt);
   tag_update_visibility(C);
+  undo::push_end(*gesture_data.vc.obact);
 }
 
 static void hide_show_init_properties(bContext & /*C*/,

source/blender/editors/sculpt_paint/paint_mask.cc

@@ -557,10 +557,11 @@ struct MaskOperation {
   float value;
 };
 
-static void gesture_begin(bContext &C, gesture::GestureData &gesture_data)
+static void gesture_begin(bContext &C, wmOperator &op, gesture::GestureData &gesture_data)
 {
   Depsgraph *depsgraph = CTX_data_depsgraph_pointer(&C);
   BKE_sculpt_update_object_for_edit(depsgraph, gesture_data.vc.obact, false);
+  undo::push_begin(*gesture_data.vc.obact, &op);
 }
 
 static float mask_gesture_get_new_value(const float elem, FloodFillMode mode, float value)
@@ -648,6 +649,7 @@ static void gesture_end(bContext &C, gesture::GestureData &gesture_data)
     multires_mark_as_modified(depsgraph, gesture_data.vc.obact, MULTIRES_COORDS_MODIFIED);
   }
   blender::bke::pbvh::update_mask(*gesture_data.ss->pbvh);
+  undo::push_end(*gesture_data.vc.obact);
 }
 
 static void init_operation(bContext &C, gesture::GestureData &gesture_data, wmOperator &op)

source/blender/editors/sculpt_paint/sculpt_face_set.cc

@@ -1740,10 +1740,11 @@ struct FaceSetOperation {
   int new_face_set_id;
 };
 
-static void gesture_begin(bContext &C, gesture::GestureData &gesture_data)
+static void gesture_begin(bContext &C, wmOperator &op, gesture::GestureData &gesture_data)
 {
   Depsgraph *depsgraph = CTX_data_depsgraph_pointer(&C);
   BKE_sculpt_update_object_for_edit(depsgraph, gesture_data.vc.obact, false);
+  undo::push_begin(*gesture_data.vc.obact, &op);
 }
 
 static void gesture_apply_mesh(gesture::GestureData &gesture_data, const Span<PBVHNode *> nodes)
@@ -1845,7 +1846,10 @@ static void gesture_apply_for_symmetry_pass(bContext & /*C*/, gesture::GestureDa
   }
 }
 
-static void gesture_end(bContext & /*C*/, gesture::GestureData & /*gesture_data*/) {}
+static void gesture_end(bContext & /*C*/, gesture::GestureData &gesture_data)
+{
+  undo::push_end(*gesture_data.vc.obact);
+}
 
 static void init_operation(gesture::GestureData &gesture_data, wmOperator & /*op*/)
 {

source/blender/editors/sculpt_paint/sculpt_gesture.cc

@@ -444,9 +444,8 @@ bool is_affected(GestureData &gesture_data, const float3 &co, const float3 &vert
 void apply(bContext &C, GestureData &gesture_data, wmOperator &op)
 {
   Operation *operation = gesture_data.operation;
-  undo::push_begin(*CTX_data_active_object(&C), &op);
 
-  operation->begin(C, gesture_data);
+  operation->begin(C, op, gesture_data);
 
   for (int symmpass = 0; symmpass <= gesture_data.symm; symmpass++) {
     if (SCULPT_is_symmetry_iteration_valid(symmpass, gesture_data.symm)) {
@@ -459,8 +458,6 @@ void apply(bContext &C, GestureData &gesture_data, wmOperator &op)
 
   operation->end(C, gesture_data);
 
-  undo::push_end(*CTX_data_active_object(&C));
-
   SCULPT_tag_update_overlays(&C);
 }
 }  // namespace blender::ed::sculpt_paint::gesture

source/blender/editors/sculpt_paint/sculpt_intern.hh

@@ -1806,7 +1806,7 @@ struct GestureData {
 /* Common abstraction structure for gesture operations. */
 struct Operation {
   /* Initial setup (data updates, special undo push...). */
-  void (*begin)(bContext &, GestureData &);
+  void (*begin)(bContext &, wmOperator &, GestureData &);
 
   /* Apply the gesture action for each symmetry pass. */
   void (*apply_for_symmetry_pass)(bContext &, GestureData &);

source/blender/editors/sculpt_paint/sculpt_project.cc

@@ -24,10 +24,11 @@ struct ProjectOperation {
   gesture::Operation operation;
 };
 
-static void gesture_begin(bContext &C, gesture::GestureData &gesture_data)
+static void gesture_begin(bContext &C, wmOperator &op, gesture::GestureData &gesture_data)
 {
   Depsgraph *depsgraph = CTX_data_depsgraph_pointer(&C);
   BKE_sculpt_update_object_for_edit(depsgraph, gesture_data.vc.obact, false);
+  undo::push_begin(*gesture_data.vc.obact, &op);
 }
 
 static void apply_projection(gesture::GestureData &gesture_data, PBVHNode *node)
@@ -92,6 +93,7 @@ static void gesture_end(bContext &C, gesture::GestureData &gesture_data)
 
   flush_update_step(&C, UpdateType::Position);
   flush_update_done(&C, *gesture_data.vc.obact, UpdateType::Position);
+  undo::push_end(*gesture_data.vc.obact);
 }
 
 static void init_operation(gesture::GestureData &gesture_data, wmOperator & /*op*/)

source/blender/editors/sculpt_paint/sculpt_trim.cc

@@ -505,7 +505,7 @@ static void generate_geometry(gesture::GestureData &gesture_data)
   update_normals(gesture_data);
 }
 
-static void gesture_begin(bContext &C, gesture::GestureData &gesture_data)
+static void gesture_begin(bContext &C, wmOperator &op, gesture::GestureData &gesture_data)
 {
   Object *object = gesture_data.vc.obact;
   SculptSession &ss = *object->sculpt;
@@ -522,7 +522,7 @@ static void gesture_begin(bContext &C, gesture::GestureData &gesture_data)
   generate_geometry(gesture_data);
   SCULPT_topology_islands_invalidate(ss);
   BKE_sculpt_update_object_for_edit(depsgraph, gesture_data.vc.obact, false);
-  undo::push_node(*gesture_data.vc.obact, nullptr, undo::Type::Geometry);
+  undo::geometry_begin(*gesture_data.vc.obact, &op);
 }
 
 static int bm_face_isect_pair(BMFace *f, void * /*user_data*/)
@@ -657,7 +657,7 @@ static void gesture_end(bContext & /*C*/, gesture::GestureData &gesture_data)
 
   free_geometry(gesture_data);
 
-  undo::push_node(*gesture_data.vc.obact, nullptr, undo::Type::Geometry);
+  undo::geometry_end(*object);
   BKE_mesh_batch_cache_dirty_tag(mesh, BKE_MESH_BATCH_DIRTY_ALL);
   DEG_id_tag_update(&gesture_data.vc.obact->id, ID_RECALC_GEOMETRY);
 }

source/blender/editors/sculpt_paint/sculpt_undo.cc

@@ -1304,9 +1304,12 @@ static Node *alloc_node(const Object &object, const PBVHNode *node, const Type t
     case Type::DyntopoBegin:
     case Type::DyntopoEnd:
     case Type::DyntopoSymmetrize:
-      BLI_assert_msg(0, "Dynamic topology should've already been handled");
+      /* Dyntopo should be handled elsewhere. */
+      BLI_assert_unreachable();
       break;
     case Type::Geometry:
+      /* See #geometry_push. */
+      BLI_assert_unreachable();
       break;
     case Type::FaceSet: {
       unode->face_sets.reinitialize(unode->face_indices.size());
@@ -1590,11 +1593,6 @@ Node *push_node(const Object &object, const PBVHNode *node, Type type)
       // return unode;
       return;
     }
-    if (type == Type::Geometry) {
-      unode = geometry_push(object);
-      // return unode;
-      return;
-    }
     if ((unode = get_node(node, type))) {
       // return unode;
       return;
@@ -1624,8 +1622,12 @@ Node *push_node(const Object &object, const PBVHNode *node, Type type)
       case Type::DyntopoBegin:
       case Type::DyntopoEnd:
       case Type::DyntopoSymmetrize:
-        BLI_assert_msg(0, "Dynamic topology should've already been handled");
+        /* Dyntopo should be handled above. */
+        BLI_assert_unreachable();
+        break;
       case Type::Geometry:
+        /* See #geometry_push. */
+        BLI_assert_unreachable();
         break;
       case Type::FaceSet:
         store_face_sets(*static_cast<const Mesh *>(object.data), *unode);
@@ -1958,18 +1960,18 @@ static void step_free(UndoStep *us_p)
 void geometry_begin(Object &ob, const wmOperator *op)
 {
   push_begin(ob, op);
-  push_node(ob, nullptr, Type::Geometry);
+  geometry_push(ob);
 }
 
 void geometry_begin_ex(Object &ob, const char *name)
 {
   push_begin_ex(ob, name);
-  push_node(ob, nullptr, Type::Geometry);
+  geometry_push(ob);
 }
 
 void geometry_end(Object &ob)
 {
-  push_node(ob, nullptr, Type::Geometry);
+  geometry_push(ob);
   push_end(ob);
 }
 
@@ -2055,7 +2057,7 @@ void push_multires_mesh_begin(bContext *C, const char *str)
 
   push_begin_ex(*object, str);
 
-  Node *geometry_unode = push_node(*object, nullptr, Type::Geometry);
+  Node *geometry_unode = geometry_push(*object);
   geometry_unode->geometry_clear_pbvh = false;
 
   push_all_grids(object);
@@ -2070,7 +2072,7 @@ void push_multires_mesh_end(bContext *C, const char *str)
 
   Object *object = CTX_data_active_object(C);
 
-  Node *geometry_unode = push_node(*object, nullptr, Type::Geometry);
+  Node *geometry_unode = geometry_push(*object);
   geometry_unode->geometry_clear_pbvh = false;
 
   push_end(*object);