[Bf-blender-cvs] [e412fe17986] master: Cleanup: Simplify freeing and clearing mesh runtime data

Hans Goudey noreply at git.blender.org
Wed Nov 16 03:27:21 CET 2022


Commit: e412fe17986ad92963bbc901865c46cf4dda5de1
Author: Hans Goudey
Date:   Tue Nov 15 19:15:35 2022 -0600
Branches: master
https://developer.blender.org/rBe412fe17986ad92963bbc901865c46cf4dda5de1

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.

===================================================================

M	source/blender/blenkernel/BKE_mesh.h
M	source/blender/blenkernel/BKE_mesh_runtime.h
M	source/blender/blenkernel/BKE_mesh_types.h
M	source/blender/blenkernel/BKE_shrinkwrap.h
M	source/blender/blenkernel/intern/mesh.cc
M	source/blender/blenkernel/intern/mesh_normals.cc
M	source/blender/blenkernel/intern/mesh_runtime.cc
M	source/blender/blenkernel/intern/shrinkwrap.cc
M	source/blender/bmesh/intern/bmesh_mesh_convert.cc
M	source/blender/draw/intern/draw_cache_impl.h
M	source/blender/draw/intern/draw_cache_impl_mesh.cc
M	source/blender/editors/mesh/meshtools.cc

===================================================================

diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h
index b1488c93ba6..f1bb1c076c1 100644
--- a/source/blender/blenkernel/BKE_mesh.h
+++ b/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 */
 
diff --git a/source/blender/blenkernel/BKE_mesh_runtime.h b/source/blender/blenkernel/BKE_mesh_runtime.h
index 27e04c1a4de..01e382aaead 100644
--- a/source/blender/blenkernel/BKE_mesh_runtime.h
+++ b/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,
diff --git a/source/blender/blenkernel/BKE_mesh_types.h b/source/blender/blenkernel/BKE_mesh_types.h
index 29b4dafcd35..793f1085aaa 100644
--- a/source/blender/blenkernel/BKE_mesh_types.h
+++ b/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")
 };
diff --git a/source/blender/blenkernel/BKE_shrinkwrap.h b/source/blender/blenkernel/BKE_shrinkwrap.h
index 17a7d274415..70065ab5961 100644
--- a/source/blender/blenkernel/BKE_shrinkwrap.h
+++ b/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. */
diff --git a/source/blender/blenkernel/intern/mesh.cc b/source/blender/blenkernel/intern/mesh.cc
index 40b2029a04a..59f946889b0 100644
--- a/source/blender/blenkernel/intern/mesh.cc
+++ b/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);
 
diff --git a/source/blender/blenkernel/intern/mesh_normals.cc b/source/blender/blenkernel/intern/mesh_normals.cc
index 7a767d983f4..0b827b3a847 100644
--- a/source/blender/blenkernel/intern/mesh_normals.cc
+++ b/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) {
diff --git a/source/blender/blenkernel/intern/mesh_runtime.cc b/source/blender/blenkernel/intern/mesh_runtime.cc
index 62c11535a0b..19e5cdd291d 100644
--- a/source/blender/blenkernel/intern/mesh_runtime.cc
+++ b/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;
+  }
+}
+
+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;
   }
-  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);
 }
 
+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_

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list