Cleanup: Simplify freeing and clearing mesh runtime data

Separate freeing and clearing mesh runtime data in a more obvious way.
This makes it easier to see what data is meant to be cleared on certain
changes, rather than conflating it with freeing all of the runtime
caches.

Also comment and reduce the surface area of the "mesh runtime" API.
The redundancy in some functions made it confusing which one should
be used, resulting in subtle bugs or unnecessary boilerplate code.

Also, now bke::MeshRuntime is able to free all the data it owns by
itself, which makes this area easier to reason about. That required
changing the interface of a few functions to avoid passing Mesh when
they really just dealt with some runtime struct.

With more RAII semantics in the future, more of this manual freeing
will become unnecessary.

source/blender/blenkernel/BKE_mesh.h

@@ -413,17 +413,6 @@ float (*BKE_mesh_vertex_normals_for_write(struct Mesh *mesh))[3];
  */
 float (*BKE_mesh_poly_normals_for_write(struct Mesh *mesh))[3];
 
-/**
- * Free any cached vertex or poly normals. Face corner (loop) normals are also derived data,
- * but are not handled with the same method yet, so they are not included. It's important that this
- * is called after the mesh changes size, since otherwise cached normal arrays might not be large
- * enough (though it may be called indirectly by other functions).
- *
- * \note Normally it's preferred to call #BKE_mesh_normals_tag_dirty instead,
- * but this can be used in specific situations to reset a mesh or reduce memory usage.
- */
-void BKE_mesh_clear_derived_normals(struct Mesh *mesh);
-
 /**
  * Mark the mesh's vertex normals non-dirty, for when they are calculated or assigned manually.
  */
@@ -987,10 +976,10 @@ void BKE_mesh_eval_geometry(struct Depsgraph *depsgraph, struct Mesh *mesh);
 
 /* Draw Cache */
 void BKE_mesh_batch_cache_dirty_tag(struct Mesh *me, eMeshBatchDirtyMode mode);
-void BKE_mesh_batch_cache_free(struct Mesh *me);
+void BKE_mesh_batch_cache_free(void *batch_cache);
 
 extern void (*BKE_mesh_batch_cache_dirty_tag_cb)(struct Mesh *me, eMeshBatchDirtyMode mode);
-extern void (*BKE_mesh_batch_cache_free_cb)(struct Mesh *me);
+extern void (*BKE_mesh_batch_cache_free_cb)(void *batch_cache);
 
 /* mesh_debug.c */
 

source/blender/blenkernel/BKE_mesh_runtime.h

@@ -8,14 +8,12 @@
  * This file contains access functions for the Mesh.runtime struct.
  */
 
-//#include "BKE_customdata.h"  /* for eCustomDataMask */
 #include "BKE_mesh_types.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-struct CustomData;
 struct CustomData_MeshMasks;
 struct Depsgraph;
 struct KeyBlock;
@@ -26,31 +24,44 @@ struct Mesh;
 struct Object;
 struct Scene;
 
-/**
- * \brief Free all data (and mutexes) inside the runtime of the given mesh.
- */
-void BKE_mesh_runtime_free_data(struct Mesh *mesh);
-
+/** Return the number of derived triangles (looptris). */
 int BKE_mesh_runtime_looptri_len(const struct Mesh *mesh);
-void BKE_mesh_runtime_looptri_recalc(struct Mesh *mesh);
+
 /**
- * \note This function only fills a cache, and therefore the mesh argument can
- * be considered logically const. Concurrent access is protected by a mutex.
- * \note This is a ported copy of dm_getLoopTriArray(dm).
+ * Return mesh triangulation data, calculated lazily when necessary necessary.
+ * See #MLoopTri for further description of mesh triangulation.
+ *
+ * \note Prefer #Mesh::looptris() in C++ code.
  */
 const struct MLoopTri *BKE_mesh_runtime_looptri_ensure(const struct Mesh *mesh);
+
 bool BKE_mesh_runtime_ensure_edit_data(struct Mesh *mesh);
-bool BKE_mesh_runtime_clear_edit_data(struct Mesh *mesh);
-bool BKE_mesh_runtime_reset_edit_data(struct Mesh *mesh);
-void BKE_mesh_runtime_clear_geometry(struct Mesh *mesh);
+void BKE_mesh_runtime_reset_edit_data(struct Mesh *mesh);
+
 /**
- * \brief This function clears runtime cache of the given mesh.
+ * Clear and any derived caches associated with the mesh geometry data. Examples include BVH
+ * caches, normals, triangulation, etc. This should be called when replacing a mesh's geometry
+ * directly or making other large changes to topology. It does not need to be called on new meshes.
  *
- * Call this function to recalculate runtime data when used.
+ * For "smaller" changes to meshes like updating positions, consider calling a more specific update
+ * function like #BKE_mesh_tag_coords_changed.
+ *
+ * Also note that some derived caches like #CD_NORMAL and #CD_TANGENT are stored directly in
+ * #CustomData.
+ */
+void BKE_mesh_runtime_clear_geometry(struct Mesh *mesh);
+
+/**
+ * Similar to #BKE_mesh_runtime_clear_geometry, but subtly different in that it also clears
+ * data-block level features like evaluated data-blocks and edit mode data. They will be
+ * functionally the same in most cases, but prefer this function if unsure, since it clears
+ * more data.
  */
 void BKE_mesh_runtime_clear_cache(struct Mesh *mesh);
 
-/* This is a copy of DM_verttri_from_looptri(). */
+/**
+ * Convert triangles encoded as face corner indices to triangles encoded as vertex indices.
+ */
 void BKE_mesh_runtime_verttri_from_looptri(struct MVertTri *r_verttri,
                                            const struct MLoop *mloop,
                                            const struct MLoopTri *looptri,

source/blender/blenkernel/BKE_mesh_types.h

@@ -161,8 +161,7 @@ struct MeshRuntime {
   uint32_t *subsurf_face_dot_tags = nullptr;
 
   MeshRuntime() = default;
-  /** \warning This does not free all data currently. See #BKE_mesh_runtime_free_data. */
-  ~MeshRuntime() = default;
+  ~MeshRuntime();
 
   MEM_CXX_CLASS_ALLOC_FUNCS("MeshRuntime")
 };

source/blender/blenkernel/BKE_shrinkwrap.h

@@ -63,7 +63,7 @@ typedef struct ShrinkwrapBoundaryData {
 /**
  * Free boundary data for target project.
  */
-void BKE_shrinkwrap_discard_boundary_data(struct Mesh *mesh);
+void BKE_shrinkwrap_boundary_data_free(ShrinkwrapBoundaryData *data);
 void BKE_shrinkwrap_compute_boundary_data(struct Mesh *mesh);
 
 /* Information about a mesh and BVH tree. */

source/blender/blenkernel/intern/mesh.cc

@@ -201,7 +201,6 @@ static void mesh_free_data(ID *id)
 
   BKE_mesh_free_editmesh(mesh);
 
-  BKE_mesh_runtime_free_data(mesh);
   mesh_clear_geometry(mesh);
   MEM_SAFE_FREE(mesh->mat);
 

source/blender/blenkernel/intern/mesh_normals.cc

@@ -147,15 +147,6 @@ bool BKE_mesh_poly_normals_are_dirty(const Mesh *mesh)
   return mesh->runtime->poly_normals_dirty;
 }
 
-void BKE_mesh_clear_derived_normals(Mesh *mesh)
-{
-  MEM_SAFE_FREE(mesh->runtime->vert_normals);
-  MEM_SAFE_FREE(mesh->runtime->poly_normals);
-
-  mesh->runtime->vert_normals_dirty = true;
-  mesh->runtime->poly_normals_dirty = true;
-}
-
 void BKE_mesh_assert_normals_dirty_or_calculated(const Mesh *mesh)
 {
   if (!mesh->runtime->vert_normals_dirty) {

source/blender/blenkernel/intern/mesh_runtime.cc

@@ -31,24 +31,84 @@ using blender::Span;
 /** \name Mesh Runtime Struct Utils
  * \{ */
 
-void BKE_mesh_runtime_free_data(Mesh *mesh)
+namespace blender::bke {
+
+static void edit_data_reset(EditMeshData &edit_data)
 {
-  BKE_mesh_runtime_clear_cache(mesh);
+  MEM_SAFE_FREE(edit_data.polyCos);
+  MEM_SAFE_FREE(edit_data.polyNos);
+  MEM_SAFE_FREE(edit_data.vertexCos);
+  MEM_SAFE_FREE(edit_data.vertexNos);
 }
 
-void BKE_mesh_runtime_clear_cache(Mesh *mesh)
+static void free_edit_data(MeshRuntime &mesh_runtime)
 {
-  if (mesh->runtime->mesh_eval != nullptr) {
-    mesh->runtime->mesh_eval->edit_mesh = nullptr;
-    BKE_id_free(nullptr, mesh->runtime->mesh_eval);
-    mesh->runtime->mesh_eval = nullptr;
+  if (mesh_runtime.edit_data) {
+    edit_data_reset(*mesh_runtime.edit_data);
+    MEM_freeN(mesh_runtime.edit_data);
+    mesh_runtime.edit_data = nullptr;
   }
-  BKE_mesh_runtime_clear_geometry(mesh);
-  BKE_mesh_batch_cache_free(mesh);
-  BKE_mesh_runtime_clear_edit_data(mesh);
-  BKE_mesh_clear_derived_normals(mesh);
 }
 
+static void free_mesh_eval(MeshRuntime &mesh_runtime)
+{
+  if (mesh_runtime.mesh_eval != nullptr) {
+    mesh_runtime.mesh_eval->edit_mesh = nullptr;
+    BKE_id_free(nullptr, mesh_runtime.mesh_eval);
+    mesh_runtime.mesh_eval = nullptr;
+  }
+}
+
+static void free_subdiv_ccg(MeshRuntime &mesh_runtime)
+{
+  /* TODO(sergey): Does this really belong here? */
+  if (mesh_runtime.subdiv_ccg != nullptr) {
+    BKE_subdiv_ccg_destroy(mesh_runtime.subdiv_ccg);
+    mesh_runtime.subdiv_ccg = nullptr;
+  }
+}
+
+static void free_bvh_cache(MeshRuntime &mesh_runtime)
+{
+  if (mesh_runtime.bvh_cache) {
+    bvhcache_free(mesh_runtime.bvh_cache);
+    mesh_runtime.bvh_cache = nullptr;
+  }
+}
+
+static void free_normals(MeshRuntime &mesh_runtime)
+{
+  MEM_SAFE_FREE(mesh_runtime.vert_normals);
+  MEM_SAFE_FREE(mesh_runtime.poly_normals);
+  mesh_runtime.vert_normals_dirty = true;
+  mesh_runtime.poly_normals_dirty = true;
+}
+
+static void free_batch_cache(MeshRuntime &mesh_runtime)
+{
+  if (mesh_runtime.batch_cache) {
+    BKE_mesh_batch_cache_free(mesh_runtime.batch_cache);
+    mesh_runtime.batch_cache = nullptr;
+  }
+}
+
+MeshRuntime::~MeshRuntime()
+{
+  free_mesh_eval(*this);
+  free_subdiv_ccg(*this);
+  free_bvh_cache(*this);
+  free_edit_data(*this);
+  free_batch_cache(*this);
+  free_normals(*this);
+  if (this->shrinkwrap_data) {
+    BKE_shrinkwrap_boundary_data_free(this->shrinkwrap_data);
+  }
+  MEM_SAFE_FREE(this->subsurf_face_dot_tags);
+  MEM_SAFE_FREE(this->looptris.array);
+}
+
+}  // namespace blender::bke
+
 blender::Span<MLoopTri> Mesh::looptris() const
 {
   const MLoopTri *looptris = BKE_mesh_runtime_looptri_ensure(this);
@@ -90,7 +150,7 @@ static void mesh_ensure_looptri_data(Mesh *mesh)
   }
 }
 
-void BKE_mesh_runtime_looptri_recalc(Mesh *mesh)
+static void recalc_loopris(Mesh *mesh)
 {
   mesh_ensure_looptri_data(mesh);
   BLI_assert(mesh->totpoly == 0 || mesh->runtime->looptris.array_wip != nullptr);
@@ -142,8 +202,7 @@ const MLoopTri *BKE_mesh_runtime_looptri_ensure(const Mesh *mesh)
   }
   else {
     /* Must isolate multithreaded tasks while holding a mutex lock. */
-    blender::threading::isolate_task(
-        [&]() { BKE_mesh_runtime_looptri_recalc(const_cast<Mesh *>(mesh)); });
+    blender::threading::isolate_task([&]() { recalc_loopris(const_cast<Mesh *>(mesh)); });
     looptri = mesh->runtime->looptris.array;
   }
 
@@ -172,56 +231,39 @@ bool BKE_mesh_runtime_ensure_edit_data(struct Mesh *mesh)
   return true;
 }
 
-bool BKE_mesh_runtime_reset_edit_data(Mesh *mesh)
+void BKE_mesh_runtime_reset_edit_data(Mesh *mesh)
 {
-  EditMeshData *edit_data = mesh->runtime->edit_data;
-  if (edit_data == nullptr) {
-    return false;
+  using namespace blender::bke;
+  if (EditMeshData *edit_data = mesh->runtime->edit_data) {
+    edit_data_reset(*edit_data);
   }
-
-  MEM_SAFE_FREE(edit_data->polyCos);
-  MEM_SAFE_FREE(edit_data->polyNos);
-  MEM_SAFE_FREE(edit_data->vertexCos);
-  MEM_SAFE_FREE(edit_data->vertexNos);
-
-  return true;
 }
 
-bool BKE_mesh_runtime_clear_edit_data(Mesh *mesh)
+void BKE_mesh_runtime_clear_cache(Mesh *mesh)
 {
-  if (mesh->runtime->edit_data == nullptr) {
-    return false;
-  }
-  BKE_mesh_runtime_reset_edit_data(mesh);
-
-  MEM_freeN(mesh->runtime->edit_data);
-  mesh->runtime->edit_data = nullptr;
-
-  return true;
+  using namespace blender::bke;
+  free_mesh_eval(*mesh->runtime);
+  free_batch_cache(*mesh->runtime);
+  free_edit_data(*mesh->runtime);
+  BKE_mesh_runtime_clear_geometry(mesh);
 }
 
 void BKE_mesh_runtime_clear_geometry(Mesh *mesh)
 {
-  BKE_mesh_tag_coords_changed(mesh);
-
-  /* TODO(sergey): Does this really belong here? */
-  if (mesh->runtime->subdiv_ccg != nullptr) {
-    BKE_subdiv_ccg_destroy(mesh->runtime->subdiv_ccg);
-    mesh->runtime->subdiv_ccg = nullptr;
+  free_bvh_cache(*mesh->runtime);
+  free_normals(*mesh->runtime);
+  free_subdiv_ccg(*mesh->runtime);
+  if (mesh->runtime->shrinkwrap_data) {
+    BKE_shrinkwrap_boundary_data_free(mesh->runtime->shrinkwrap_data);
   }
-  BKE_shrinkwrap_discard_boundary_data(mesh);
-
   MEM_SAFE_FREE(mesh->runtime->subsurf_face_dot_tags);
 }
 
 void BKE_mesh_tag_coords_changed(Mesh *mesh)
 {
   BKE_mesh_normals_tag_dirty(mesh);
+  free_bvh_cache(*mesh->runtime);
   MEM_SAFE_FREE(mesh->runtime->looptris.array);
-  if (mesh->runtime->bvh_cache) {
-    bvhcache_free(mesh->runtime->bvh_cache);
-    mesh->runtime->bvh_cache = nullptr;
-  }
   mesh->runtime->bounds_cache.tag_dirty();
 }
 
@@ -258,7 +300,7 @@ eMeshWrapperType BKE_mesh_wrapper_type(const struct Mesh *mesh)
 
 /* Draw Engine */
 void (*BKE_mesh_batch_cache_dirty_tag_cb)(Mesh *me, eMeshBatchDirtyMode mode) = nullptr;
-void (*BKE_mesh_batch_cache_free_cb)(Mesh *me) = nullptr;
+void (*BKE_mesh_batch_cache_free_cb)(void *batch_cache) = nullptr;
 
 void BKE_mesh_batch_cache_dirty_tag(Mesh *me, eMeshBatchDirtyMode mode)
 {
@@ -266,11 +308,9 @@ void BKE_mesh_batch_cache_dirty_tag(Mesh *me, eMeshBatchDirtyMode mode)
     BKE_mesh_batch_cache_dirty_tag_cb(me, mode);
   }
 }
-void BKE_mesh_batch_cache_free(Mesh *me)
+void BKE_mesh_batch_cache_free(void *batch_cache)
 {
-  if (me->runtime->batch_cache) {
-    BKE_mesh_batch_cache_free_cb(me);
-  }
+  BKE_mesh_batch_cache_free_cb(batch_cache);
 }
 
 /** \} */

source/blender/blenkernel/intern/shrinkwrap.cc

@@ -152,20 +152,14 @@ void BKE_shrinkwrap_free_tree(ShrinkwrapTreeData *data)
   free_bvhtree_from_mesh(&data->treeData);
 }
 
-void BKE_shrinkwrap_discard_boundary_data(Mesh *mesh)
+void BKE_shrinkwrap_boundary_data_free(ShrinkwrapBoundaryData *data)
 {
-  ShrinkwrapBoundaryData *data = mesh->runtime->shrinkwrap_data;
+  MEM_freeN((void *)data->edge_is_boundary);
+  MEM_freeN((void *)data->looptri_has_boundary);
+  MEM_freeN((void *)data->vert_boundary_id);
+  MEM_freeN((void *)data->boundary_verts);
 
-  if (data != nullptr) {
-    MEM_freeN((void *)data->edge_is_boundary);
-    MEM_freeN((void *)data->looptri_has_boundary);
-    MEM_freeN((void *)data->vert_boundary_id);
-    MEM_freeN((void *)data->boundary_verts);
-
-    MEM_freeN(data);
-  }
-
-  mesh->runtime->shrinkwrap_data = nullptr;
+  MEM_freeN(data);
 }
 
 /* Accumulate edge for average boundary edge direction. */
@@ -326,8 +320,9 @@ static ShrinkwrapBoundaryData *shrinkwrap_build_boundary_data(Mesh *mesh)
 
 void BKE_shrinkwrap_compute_boundary_data(Mesh *mesh)
 {
-  BKE_shrinkwrap_discard_boundary_data(mesh);
-
+  if (mesh->runtime->shrinkwrap_data) {
+    BKE_shrinkwrap_boundary_data_free(mesh->runtime->shrinkwrap_data);
+  }
   mesh->runtime->shrinkwrap_data = shrinkwrap_build_boundary_data(mesh);
 }
 

source/blender/bmesh/intern/bmesh_mesh_convert.cc

@@ -958,6 +958,8 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh
   CustomData_free(&me->ldata, me->totloop);
   CustomData_free(&me->pdata, me->totpoly);
 
+  BKE_mesh_runtime_clear_geometry(me);
+
   /* Add new custom data. */
   me->totvert = bm->totvert;
   me->totedge = bm->totedge;
@@ -994,10 +996,6 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh
   bool need_hide_poly = false;
   bool need_material_index = false;
 
-  /* Clear normals on the mesh completely, since the original vertex and polygon count might be
-   * different than the BMesh's. */
-  BKE_mesh_clear_derived_normals(me);
-
   i = 0;
   BM_ITER_MESH (v, &iter, bm, BM_VERTS_OF_MESH) {
     copy_v3_v3(mvert[i].co, v->co);
@@ -1219,6 +1217,8 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks *
   /* Must be an empty mesh. */
   BLI_assert(me->totvert == 0);
   BLI_assert(cd_mask_extra == nullptr || (cd_mask_extra->vmask & CD_MASK_SHAPEKEY) == 0);
+  /* Just in case, clear the derived geometry caches from the input mesh. */
+  BKE_mesh_runtime_clear_geometry(me);
 
   me->totvert = bm->totvert;
   me->totedge = bm->totedge;
@@ -1254,10 +1254,6 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks *
   MLoop *mloop = loops.data();
   uint i, j;
 
-  /* Clear normals on the mesh completely, since the original vertex and polygon count might be
-   * different than the BMesh's. */
-  BKE_mesh_clear_derived_normals(me);
-
   me->runtime->deformed_only = true;
 
   bke::MutableAttributeAccessor mesh_attributes = me->attributes_for_write();

source/blender/draw/intern/draw_cache_impl.h

@@ -42,7 +42,7 @@ void DRW_curve_batch_cache_free(struct Curve *cu);
 
 void DRW_mesh_batch_cache_dirty_tag(struct Mesh *me, eMeshBatchDirtyMode mode);
 void DRW_mesh_batch_cache_validate(struct Object *object, struct Mesh *me);
-void DRW_mesh_batch_cache_free(struct Mesh *me);
+void DRW_mesh_batch_cache_free(void *batch_cache);
 
 void DRW_lattice_batch_cache_dirty_tag(struct Lattice *lt, int mode);
 void DRW_lattice_batch_cache_validate(struct Lattice *lt);

source/blender/draw/intern/draw_cache_impl_mesh.cc

@@ -201,7 +201,7 @@ static constexpr DRWBatchFlag batches_that_use_buffer(const int buffer_index)
 }
 
 static void mesh_batch_cache_discard_surface_batches(MeshBatchCache *cache);
-static void mesh_batch_cache_clear(Mesh *me);
+static void mesh_batch_cache_clear(MeshBatchCache *cache);
 
 static void mesh_batch_cache_discard_batch(MeshBatchCache *cache, const DRWBatchFlag batch_map)
 {
@@ -618,7 +618,9 @@ static void mesh_batch_cache_init(Object *object, Mesh *me)
 void DRW_mesh_batch_cache_validate(Object *object, Mesh *me)
 {
   if (!mesh_batch_cache_valid(object, me)) {
-    mesh_batch_cache_clear(me);
+    if (me->runtime->batch_cache) {
+      mesh_batch_cache_clear(static_cast<MeshBatchCache *>(me->runtime->batch_cache));
+    }
     mesh_batch_cache_init(object, me);
   }
 }
@@ -819,12 +821,8 @@ static void mesh_batch_cache_free_subdiv_cache(MeshBatchCache *cache)
   }
 }
 
-static void mesh_batch_cache_clear(Mesh *me)
+static void mesh_batch_cache_clear(MeshBatchCache *cache)
 {
-  MeshBatchCache *cache = static_cast<MeshBatchCache *>(me->runtime->batch_cache);
-  if (!cache) {
-    return;
-  }
   FOREACH_MESH_BUFFER_CACHE (cache, mbc) {
     mesh_buffer_cache_clear(mbc);
   }
@@ -850,10 +848,12 @@ static void mesh_batch_cache_clear(Mesh *me)
   mesh_batch_cache_free_subdiv_cache(cache);
 }
 
-void DRW_mesh_batch_cache_free(Mesh *me)
+void DRW_mesh_batch_cache_free(void *batch_cache)
 {
-  mesh_batch_cache_clear(me);
-  MEM_SAFE_FREE(me->runtime->batch_cache);
+  if (batch_cache) {
+    mesh_batch_cache_clear(static_cast<MeshBatchCache *>(batch_cache));
+    MEM_freeN(batch_cache);
+  }
 }
 
 /** \} */

source/blender/editors/mesh/meshtools.cc

@@ -421,7 +421,6 @@ int ED_mesh_join_objects_exec(bContext *C, wmOperator *op)
    * Even though this mesh wont typically have run-time data, the Python API can for e.g.
    * create loop-triangle cache here, which is confusing when left in the mesh, see: T90798. */
   BKE_mesh_runtime_clear_geometry(me);
-  BKE_mesh_clear_derived_normals(me);
 
   /* new material indices and material array */
   if (totmat) {