Refactor: Sculpt: Consolidate pbvh clearing actions

This commit introduces a number of changes to make PBVH and related
attribute lifecycle more predictable.

A common method, `BKE_sculptsession_free_pbvh` is introduced to manage
freeing related resources that are stored in `SculptSession`

Prior to this commit, the `active_vert_` attribute was only ever
changed when a raycast successfully hit the mesh. This commit changes
the behavior to not store a stale reference in the following cases:

* When dyntopo is enabled or disabled
* When a mesh with the subdivision modifier is subdivided further
* When the sculpt level of a subdiv modifier is changed
* When the subdivision modifier is removed from a mesh

Pull Request: https://projects.blender.org/blender/blender/pulls/126341

source/blender/blenkernel/BKE_paint.hh

@@ -405,9 +405,9 @@ using ActiveVert = std::variant<std::monostate, int, SubdivCCGCoord, BMVert *>;
 struct SculptSession : blender::NonCopyable, blender::NonMovable {
   /* Mesh data (not copied) can come either directly from a Mesh, or from a MultiresDM */
   struct { /* Special handling for multires meshes */
-    bool active;
-    MultiresModifierData *modifier;
-    int level;
+    bool active = false;
+    MultiresModifierData *modifier = nullptr;
+    int level = 0;
   } multires = {};
 
   /* Depsgraph for the Cloth Brush solver to get the colliders. */
@@ -576,6 +576,10 @@ struct SculptSession : blender::NonCopyable, blender::NonMovable {
   std::unique_ptr<SculptTopologyIslandCache> topology_island_cache;
 
  private:
+  /* In general, this value is expected to be valid (non-empty) as long as the cursor is over the
+   * mesh. Changing the underlying mesh type (e.g. enabling dyntopo, changing multires levels)
+   * should invalidate this value.
+   */
   PBVHVertRef active_vert_ = PBVHVertRef{PBVH_REF_NONE};
 
  public:
@@ -608,11 +612,13 @@ struct SculptSession : blender::NonCopyable, blender::NonMovable {
   blender::float3 active_vert_position(const Depsgraph &depsgraph, const Object &object) const;
 
   void set_active_vert(PBVHVertRef vert);
+  void clear_active_vert();
 };
 
 void BKE_sculptsession_free(Object *ob);
 void BKE_sculptsession_free_deformMats(SculptSession *ss);
 void BKE_sculptsession_free_vwpaint_data(SculptSession *ss);
+void BKE_sculptsession_free_pbvh(SculptSession *ss);
 void BKE_sculptsession_bm_to_me(Object *ob, bool reorder);
 void BKE_sculptsession_bm_to_me_for_render(Object *object);
 int BKE_sculptsession_vertex_count(const SculptSession *ss);

source/blender/blenkernel/intern/multires.cc

@@ -454,7 +454,7 @@ void multires_force_sculpt_rebuild(Object *object)
   }
 
   SculptSession &ss = *object->sculpt;
-  bke::pbvh::free(ss.pbvh);
+  BKE_sculptsession_free_pbvh(&ss);
 }
 
 void multires_force_external_reload(Object *object)

source/blender/blenkernel/intern/paint.cc

@@ -1643,15 +1643,13 @@ void BKE_sculptsession_bm_to_me(Object *ob, bool reorder)
   }
 }
 
-static void sculptsession_free_pbvh(Object *object)
+void BKE_sculptsession_free_pbvh(SculptSession *ss)
 {
-  using namespace blender;
-  SculptSession *ss = object->sculpt;
   if (!ss) {
     return;
   }
 
-  bke::pbvh::free(ss->pbvh);
+  blender::bke::pbvh::free(ss->pbvh);
   ss->vert_to_face_map = {};
   ss->edge_to_face_offsets = {};
   ss->edge_to_face_indices = {};
@@ -1664,6 +1662,8 @@ static void sculptsession_free_pbvh(Object *object)
 
   ss->vertex_info.boundary.clear_and_shrink();
   ss->fake_neighbors.fake_neighbor_index = {};
+
+  ss->clear_active_vert();
 }
 
 void BKE_sculptsession_bm_to_me_for_render(Object *object)
@@ -1700,7 +1700,7 @@ void BKE_sculptsession_free(Object *ob)
       BM_mesh_free(ss->bm);
     }
 
-    sculptsession_free_pbvh(ob);
+    BKE_sculptsession_free_pbvh(ss);
 
     MEM_delete(ss);
 
@@ -1806,6 +1806,11 @@ blender::float3 SculptSession::active_vert_position(const Depsgraph &depsgraph,
   return float3(std::numeric_limits<float>::infinity());
 }
 
+void SculptSession::clear_active_vert()
+{
+  active_vert_ = {PBVH_REF_NONE};
+}
+
 void SculptSession::set_active_vert(const PBVHVertRef vert)
 {
   active_vert_ = vert;
@@ -2110,7 +2115,7 @@ void BKE_sculpt_update_object_before_eval(Object *ob_eval)
       /* We free pbvh on changes, except in the middle of drawing a stroke
        * since it can't deal with changing PVBH node organization, we hope
        * topology does not change in the meantime .. weak. */
-      sculptsession_free_pbvh(ob_eval);
+      BKE_sculptsession_free_pbvh(ss);
 
       BKE_sculptsession_free_deformMats(ob_eval->sculpt);
 

source/blender/editors/sculpt_paint/sculpt.cc

@@ -4905,6 +4905,7 @@ bool SCULPT_cursor_geometry_info_update(bContext *C,
     zero_v3(out->location);
     zero_v3(out->normal);
     zero_v3(out->active_vertex_co);
+    ss.clear_active_vert();
     return false;
   }
 

source/blender/editors/sculpt_paint/sculpt_detail.cc

@@ -23,6 +23,7 @@
 #include "BKE_brush.hh"
 #include "BKE_context.hh"
 #include "BKE_layer.hh"
+#include "BKE_object.hh"
 #include "BKE_paint.hh"
 #include "BKE_pbvh_api.hh"
 #include "BKE_screen.hh"
@@ -142,7 +143,9 @@ static int sculpt_detail_flood_fill_exec(bContext *C, wmOperator *op)
   undo::push_end(ob);
 
   /* Force rebuild of bke::pbvh::Tree for better BB placement. */
-  SCULPT_pbvh_clear(ob);
+  BKE_sculptsession_free_pbvh(&ss);
+  DEG_id_tag_update(&ob.id, ID_RECALC_GEOMETRY);
+
   /* Redraw. */
   WM_event_add_notifier(C, NC_OBJECT | ND_DRAW, &ob);
 

source/blender/editors/sculpt_paint/sculpt_dyntopo.cc

@@ -43,19 +43,6 @@
 #include "bmesh.hh"
 #include "bmesh_tools.hh"
 
-void SCULPT_pbvh_clear(Object &ob)
-{
-  using namespace blender;
-  SculptSession &ss = *ob.sculpt;
-  /* Clear out any existing DM and bke::pbvh::Tree. */
-  bke::pbvh::free(ss.pbvh);
-
-  BKE_object_free_derived_caches(&ob);
-
-  /* Tag to rebuild bke::pbvh::Tree in depsgraph. */
-  DEG_id_tag_update(&ob.id, ID_RECALC_GEOMETRY);
-}
-
 namespace blender::ed::sculpt_paint::dyntopo {
 
 void triangulate(BMesh *bm)
@@ -78,7 +65,7 @@ void enable_ex(Main &bmain, Depsgraph &depsgraph, Object &ob)
   Mesh *mesh = static_cast<Mesh *>(ob.data);
   const BMAllocTemplate allocsize = BMALLOC_TEMPLATE_FROM_ME(mesh);
 
-  SCULPT_pbvh_clear(ob);
+  BKE_sculptsession_free_pbvh(&ss);
 
   /* Dynamic topology doesn't ensure selection state is valid, so remove #36280. */
   BKE_mesh_mselect_clear(mesh);
@@ -132,7 +119,7 @@ static void disable(
     BM_data_layer_free_named(bm, &bm->pdata, ".sculpt_dyntopo_node_id_face");
   }
 
-  SCULPT_pbvh_clear(ob);
+  BKE_sculptsession_free_pbvh(&ss);
 
   if (undo_step) {
     undo::restore_from_bmesh_enter_geometry(*undo_step, *mesh);

source/blender/editors/sculpt_paint/sculpt_intern.hh

@@ -428,8 +428,6 @@ void flush_update_done(const bContext *C, Object &ob, UpdateType update_type);
 
 }
 
-void SCULPT_pbvh_clear(Object &ob);
-
 /**
  * Should be used after modifying the mask or Face Sets IDs.
  */

source/blender/editors/sculpt_paint/sculpt_ops.cc

@@ -146,10 +146,13 @@ static void SCULPT_OT_set_persistent_base(wmOperatorType *ot)
 
 static int sculpt_optimize_exec(bContext *C, wmOperator * /*op*/)
 {
-  Object *ob = CTX_data_active_object(C);
+  Object &ob = *CTX_data_active_object(C);
 
-  SCULPT_pbvh_clear(*ob);
-  WM_event_add_notifier(C, NC_OBJECT | ND_DRAW, ob);
+  SculptSession &ss = *ob.sculpt;
+  BKE_sculptsession_free_pbvh(&ss);
+  DEG_id_tag_update(&ob.id, ID_RECALC_GEOMETRY);
+
+  WM_event_add_notifier(C, NC_OBJECT | ND_DRAW, &ob);
 
   return OPERATOR_FINISHED;
 }
@@ -253,7 +256,8 @@ static int sculpt_symmetrize_exec(bContext *C, wmOperator *op)
 
   islands::invalidate(ss);
 
-  SCULPT_pbvh_clear(ob);
+  BKE_sculptsession_free_pbvh(&ss);
+  DEG_id_tag_update(&ob.id, ID_RECALC_GEOMETRY);
   WM_event_add_notifier(C, NC_OBJECT | ND_DRAW, &ob);
 
   return OPERATOR_FINISHED;

source/blender/editors/sculpt_paint/sculpt_undo.cc

@@ -696,7 +696,8 @@ static void bmesh_restore_generic(StepData &step_data, Object &object, SculptSes
     }
   }
   else {
-    SCULPT_pbvh_clear(object);
+    BKE_sculptsession_free_pbvh(&ss);
+    DEG_id_tag_update(&object.id, ID_RECALC_GEOMETRY);
   }
 }
 
@@ -706,7 +707,8 @@ static void bmesh_enable(Object &object, StepData &step_data)
   SculptSession &ss = *object.sculpt;
   Mesh *mesh = static_cast<Mesh *>(object.data);
 
-  SCULPT_pbvh_clear(object);
+  BKE_sculptsession_free_pbvh(&ss);
+  DEG_id_tag_update(&object.id, ID_RECALC_GEOMETRY);
 
   /* Create empty BMesh and enable logging. */
   BMeshCreateParams bmesh_create_params{};
@@ -816,7 +818,9 @@ static void geometry_free_data(NodeGeometry *geometry)
 static void restore_geometry(StepData &step_data, Object &object)
 {
   if (step_data.geometry_clear_pbvh) {
-    SCULPT_pbvh_clear(object);
+    SculptSession &ss = *object.sculpt;
+    BKE_sculptsession_free_pbvh(&ss);
+    DEG_id_tag_update(&object.id, ID_RECALC_GEOMETRY);
   }
 
   Mesh *mesh = static_cast<Mesh *>(object.data);