Fix T95839: Data race when lazily creating mesh normal layers

Currently, when normals are calculated for a const mesh, a custom data
layer might be added if it doesn't already exist. Adding a custom data
layer to a mesh is not thread-safe, so this can be a problem in some
situations.

This commit moves derived mesh normals for polygons and
vertices out of `CustomData` to `Mesh_Runtime`. Most of the
hard work for this was already done by rBcfa53e0fbeed7178.
Some changes to logic elsewhere are necessary/helpful:
- No need to call both `BKE_mesh_runtime_clear_cache` and
  `BKE_mesh_normals_tag_dirty`, since the former also does the latter.
- Cleanup/simplify mesh conversion and copying since normals are
  handled with other runtime data.

Storing these normals like other runtime data clarifies their status
as derived data, meaning custom data moves more towards storing
original/editable data. This means normals won't automatically benefit
from the planned copy-on-write refactor (T95845), so it will have to be
added manually like for the other runtime data.

Differential Revision: https://developer.blender.org/D14154

10 files changed, 95 insertions, 56 deletions

diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h
index 2b32c6a5420..72a1303fc6b 100644
--- a/source/blender/blenkernel/BKE_mesh.h
+++ b/source/blender/blenkernel/BKE_mesh.h
@@ -428,6 +428,17 @@ 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.
  */
 void BKE_mesh_vertex_normals_clear_dirty(struct Mesh *mesh);
diff --git a/source/blender/blenkernel/intern/customdata.cc b/source/blender/blenkernel/intern/customdata.cc
index e4c18325d76..0935a902c3a 100644
--- a/source/blender/blenkernel/intern/customdata.cc
+++ b/source/blender/blenkernel/intern/customdata.cc
@@ -2000,10 +2000,10 @@ const CustomData_MeshMasks CD_MASK_FACECORNERS = {
      CD_MASK_NORMAL | CD_MASK_MLOOPTANGENT),
 };
 const CustomData_MeshMasks CD_MASK_EVERYTHING = {
-    /* vmask */ (CD_MASK_MVERT | CD_MASK_BM_ELEM_PYPTR | CD_MASK_ORIGINDEX | CD_MASK_NORMAL |
-                 CD_MASK_MDEFORMVERT | CD_MASK_BWEIGHT | CD_MASK_MVERT_SKIN | CD_MASK_ORCO |
-                 CD_MASK_CLOTH_ORCO | CD_MASK_SHAPEKEY | CD_MASK_SHAPE_KEYINDEX |
-                 CD_MASK_PAINT_MASK | CD_MASK_PROP_ALL | CD_MASK_PROP_COLOR | CD_MASK_CREASE),
+    /* vmask */ (CD_MASK_MVERT | CD_MASK_BM_ELEM_PYPTR | CD_MASK_ORIGINDEX | CD_MASK_MDEFORMVERT |
+                 CD_MASK_BWEIGHT | CD_MASK_MVERT_SKIN | CD_MASK_ORCO | CD_MASK_CLOTH_ORCO |
+                 CD_MASK_SHAPEKEY | CD_MASK_SHAPE_KEYINDEX | CD_MASK_PAINT_MASK |
+                 CD_MASK_PROP_ALL | CD_MASK_PROP_COLOR | CD_MASK_CREASE),
     /* emask */
     (CD_MASK_MEDGE | CD_MASK_BM_ELEM_PYPTR | CD_MASK_ORIGINDEX | CD_MASK_BWEIGHT | CD_MASK_CREASE |
      CD_MASK_FREESTYLE_EDGE | CD_MASK_PROP_ALL),
@@ -2012,7 +2012,7 @@ const CustomData_MeshMasks CD_MASK_EVERYTHING = {
      CD_MASK_ORIGSPACE | CD_MASK_TANGENT | CD_MASK_TESSLOOPNORMAL | CD_MASK_PREVIEW_MCOL |
      CD_MASK_PROP_ALL),
     /* pmask */
-    (CD_MASK_MPOLY | CD_MASK_BM_ELEM_PYPTR | CD_MASK_ORIGINDEX | CD_MASK_NORMAL | CD_MASK_FACEMAP |
+    (CD_MASK_MPOLY | CD_MASK_BM_ELEM_PYPTR | CD_MASK_ORIGINDEX | CD_MASK_FACEMAP |
      CD_MASK_FREESTYLE_FACE | CD_MASK_PROP_ALL | CD_MASK_SCULPT_FACE_SETS),
     /* lmask */
     (CD_MASK_MLOOP | CD_MASK_BM_ELEM_PYPTR | CD_MASK_MDISPS | CD_MASK_NORMAL | CD_MASK_MLOOPUV |
diff --git a/source/blender/blenkernel/intern/mesh.cc b/source/blender/blenkernel/intern/mesh.cc
index 351535a6f78..09d9b19330a 100644
--- a/source/blender/blenkernel/intern/mesh.cc
+++ b/source/blender/blenkernel/intern/mesh.cc
@@ -75,6 +75,8 @@
 
 #include "BLO_read_write.h"
 
+using blender::float3;
+
 static void mesh_clear_geometry(Mesh *mesh);
 static void mesh_tessface_clear_intern(Mesh *mesh, int free_customdata);
 
@@ -1111,16 +1113,6 @@ Mesh *BKE_mesh_new_nomain_from_template_ex(const Mesh *me_src,
     mesh_tessface_clear_intern(me_dst, false);
   }
 
-  me_dst->runtime.cd_dirty_poly = me_src->runtime.cd_dirty_poly;
-  me_dst->runtime.cd_dirty_vert = me_src->runtime.cd_dirty_vert;
-
-  /* Ensure that when no normal layers exist, they are marked dirty, because
-   * normals might not have been included in the mask of copied layers. */
-  if (!CustomData_has_layer(&me_dst->vdata, CD_NORMAL) ||
-      !CustomData_has_layer(&me_dst->pdata, CD_NORMAL)) {
-    BKE_mesh_normals_tag_dirty(me_dst);
-  }
-
   /* The destination mesh should at least have valid primary CD layers,
    * even in cases where the source mesh does not. */
   mesh_ensure_cdlayers_primary(me_dst, do_tessface);
@@ -2150,6 +2142,10 @@ static void split_faces_split_new_verts(Mesh *mesh,
   MVert *mvert = mesh->mvert;
   float(*vert_normals)[3] = BKE_mesh_vertex_normals_for_write(mesh);
 
+  /* Normals were already calculated at the beginning of this operation, we rely on that to update
+   * them partially here. */
+  BLI_assert(!BKE_mesh_vertex_normals_are_dirty(mesh));
+
   /* Remember new_verts is a single linklist, so its items are in reversed order... */
   MVert *new_mv = &mvert[mesh->totvert - 1];
   for (int i = mesh->totvert - 1; i >= verts_len; i--, new_mv--, new_verts = new_verts->next) {
@@ -2160,7 +2156,6 @@ static void split_faces_split_new_verts(Mesh *mesh,
       copy_v3_v3(vert_normals[i], new_verts->vnor);
     }
   }
-  BKE_mesh_vertex_normals_clear_dirty(mesh);
 }
 
 /* Perform actual split of edges. */
@@ -2231,6 +2226,10 @@ void BKE_mesh_split_faces(Mesh *mesh, bool free_loop_normals)
     /* Update pointers to a newly allocated memory. */
     BKE_mesh_update_customdata_pointers(mesh, false);
 
+    /* Update normals manually to avoid recalculation after this operation. */
+    mesh->runtime.vert_normals = (float(*)[3])MEM_reallocN(mesh->runtime.vert_normals,
+                                                           sizeof(float[3]) * mesh->totvert);
+
     /* Perform actual split of vertices and edges. */
     split_faces_split_new_verts(mesh, new_verts, num_new_verts);
     if (do_edges) {
diff --git a/source/blender/blenkernel/intern/mesh_convert.cc b/source/blender/blenkernel/intern/mesh_convert.cc
index 3562f6c6b17..1db17c950b3 100644
--- a/source/blender/blenkernel/intern/mesh_convert.cc
+++ b/source/blender/blenkernel/intern/mesh_convert.cc
@@ -1495,15 +1495,8 @@ void BKE_mesh_nomain_to_mesh(Mesh *mesh_src,
   tmp.cd_flag = mesh_src->cd_flag;
   tmp.runtime.deformed_only = mesh_src->runtime.deformed_only;
 
-  tmp.runtime.cd_dirty_poly = mesh_src->runtime.cd_dirty_poly;
-  tmp.runtime.cd_dirty_vert = mesh_src->runtime.cd_dirty_vert;
-
-  /* Ensure that when no normal layers exist, they are marked dirty, because
-   * normals might not have been included in the mask of copied layers. */
-  if (!CustomData_has_layer(&tmp.vdata, CD_NORMAL) ||
-      !CustomData_has_layer(&tmp.pdata, CD_NORMAL)) {
-    BKE_mesh_normals_tag_dirty(&tmp);
-  }
+  /* Clear the normals completely, since the new vertex / polygon count might be different. */
+  BKE_mesh_clear_derived_normals(&tmp);
 
   if (CustomData_has_layer(&mesh_src->vdata, CD_SHAPEKEY)) {
     KeyBlock *kb;
diff --git a/source/blender/blenkernel/intern/mesh_normals.cc b/source/blender/blenkernel/intern/mesh_normals.cc
index 1b3c7e01be8..b690d7f1e32 100644
--- a/source/blender/blenkernel/intern/mesh_normals.cc
+++ b/source/blender/blenkernel/intern/mesh_normals.cc
@@ -110,53 +110,72 @@ static void add_v3_v3_atomic(float r[3], const float a[3])
 
 void BKE_mesh_normals_tag_dirty(Mesh *mesh)
 {
-  mesh->runtime.cd_dirty_vert |= CD_MASK_NORMAL;
-  mesh->runtime.cd_dirty_poly |= CD_MASK_NORMAL;
+  mesh->runtime.vert_normals_dirty = true;
+  mesh->runtime.poly_normals_dirty = true;
 }
 
 float (*BKE_mesh_vertex_normals_for_write(Mesh *mesh))[3]
 {
-  CustomData_duplicate_referenced_layer(&mesh->vdata, CD_NORMAL, mesh->totvert);
-  return (float(*)[3])CustomData_add_layer(
-      &mesh->vdata, CD_NORMAL, CD_CALLOC, nullptr, mesh->totvert);
+  if (mesh->runtime.vert_normals == nullptr) {
+    mesh->runtime.vert_normals = (float(*)[3])MEM_malloc_arrayN(
+        mesh->totvert, sizeof(float[3]), __func__);
+  }
+
+  BLI_assert(MEM_allocN_len(mesh->runtime.vert_normals) >= sizeof(float[3]) * mesh->totvert);
+
+  return mesh->runtime.vert_normals;
 }
 
 float (*BKE_mesh_poly_normals_for_write(Mesh *mesh))[3]
 {
-  CustomData_duplicate_referenced_layer(&mesh->pdata, CD_NORMAL, mesh->totpoly);
-  return (float(*)[3])CustomData_add_layer(
-      &mesh->pdata, CD_NORMAL, CD_CALLOC, nullptr, mesh->totpoly);
+  if (mesh->runtime.poly_normals == nullptr) {
+    mesh->runtime.poly_normals = (float(*)[3])MEM_malloc_arrayN(
+        mesh->totpoly, sizeof(float[3]), __func__);
+  }
+
+  BLI_assert(MEM_allocN_len(mesh->runtime.poly_normals) >= sizeof(float[3]) * mesh->totpoly);
+
+  return mesh->runtime.poly_normals;
 }
 
 void BKE_mesh_vertex_normals_clear_dirty(Mesh *mesh)
 {
-  mesh->runtime.cd_dirty_vert &= ~CD_MASK_NORMAL;
+  mesh->runtime.vert_normals_dirty = false;
   BKE_mesh_assert_normals_dirty_or_calculated(mesh);
 }
 
 void BKE_mesh_poly_normals_clear_dirty(Mesh *mesh)
 {
-  mesh->runtime.cd_dirty_poly &= ~CD_MASK_NORMAL;
+  mesh->runtime.poly_normals_dirty = false;
   BKE_mesh_assert_normals_dirty_or_calculated(mesh);
 }
 
 bool BKE_mesh_vertex_normals_are_dirty(const Mesh *mesh)
 {
-  return mesh->runtime.cd_dirty_vert & CD_MASK_NORMAL;
+  return mesh->runtime.vert_normals_dirty;
 }
 
 bool BKE_mesh_poly_normals_are_dirty(const Mesh *mesh)
 {
-  return mesh->runtime.cd_dirty_poly & CD_MASK_NORMAL;
+  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.cd_dirty_vert & CD_MASK_NORMAL)) {
-    BLI_assert(CustomData_has_layer(&mesh->vdata, CD_NORMAL) || mesh->totvert == 0);
+  if (!mesh->runtime.vert_normals_dirty) {
+    BLI_assert(mesh->runtime.vert_normals || mesh->totvert == 0);
   }
-  if (!(mesh->runtime.cd_dirty_poly & CD_MASK_NORMAL)) {
-    BLI_assert(CustomData_has_layer(&mesh->pdata, CD_NORMAL) || mesh->totpoly == 0);
+  if (!mesh->runtime.poly_normals_dirty) {
+    BLI_assert(mesh->runtime.poly_normals || mesh->totpoly == 0);
   }
 }
 
@@ -358,8 +377,8 @@ static void mesh_calc_normals_poly_and_vertex(MVert *mvert,
 const float (*BKE_mesh_vertex_normals_ensure(const Mesh *mesh))[3]
 {
   if (!(BKE_mesh_vertex_normals_are_dirty(mesh) || BKE_mesh_poly_normals_are_dirty(mesh))) {
-    BLI_assert(CustomData_has_layer(&mesh->vdata, CD_NORMAL) || mesh->totvert == 0);
-    return (const float(*)[3])CustomData_get_layer(&mesh->vdata, CD_NORMAL);
+    BLI_assert(mesh->runtime.vert_normals != nullptr || mesh->totvert == 0);
+    return mesh->runtime.vert_normals;
   }
 
   if (mesh->totvert == 0) {
@@ -369,9 +388,9 @@ const float (*BKE_mesh_vertex_normals_ensure(const Mesh *mesh))[3]
   ThreadMutex *normals_mutex = (ThreadMutex *)mesh->runtime.normals_mutex;
   BLI_mutex_lock(normals_mutex);
   if (!(BKE_mesh_vertex_normals_are_dirty(mesh) || BKE_mesh_poly_normals_are_dirty(mesh))) {
-    BLI_assert(CustomData_has_layer(&mesh->vdata, CD_NORMAL));
+    BLI_assert(mesh->runtime.vert_normals != nullptr);
     BLI_mutex_unlock(normals_mutex);
-    return (const float(*)[3])CustomData_get_layer(&mesh->vdata, CD_NORMAL);
+    return mesh->runtime.vert_normals;
   }
 
   float(*vert_normals)[3];
@@ -404,8 +423,8 @@ const float (*BKE_mesh_vertex_normals_ensure(const Mesh *mesh))[3]
 const float (*BKE_mesh_poly_normals_ensure(const Mesh *mesh))[3]
 {
   if (!BKE_mesh_poly_normals_are_dirty(mesh)) {
-    BLI_assert(CustomData_has_layer(&mesh->pdata, CD_NORMAL) || mesh->totpoly == 0);
-    return (const float(*)[3])CustomData_get_layer(&mesh->pdata, CD_NORMAL);
+    BLI_assert(mesh->runtime.poly_normals != nullptr || mesh->totpoly == 0);
+    return mesh->runtime.poly_normals;
   }
 
   if (mesh->totpoly == 0) {
@@ -415,9 +434,9 @@ const float (*BKE_mesh_poly_normals_ensure(const Mesh *mesh))[3]
   ThreadMutex *normals_mutex = (ThreadMutex *)mesh->runtime.normals_mutex;
   BLI_mutex_lock(normals_mutex);
   if (!BKE_mesh_poly_normals_are_dirty(mesh)) {
-    BLI_assert(CustomData_has_layer(&mesh->pdata, CD_NORMAL));
+    BLI_assert(mesh->runtime.poly_normals != nullptr);
     BLI_mutex_unlock(normals_mutex);
-    return (const float(*)[3])CustomData_get_layer(&mesh->pdata, CD_NORMAL);
+    return mesh->runtime.poly_normals;
   }
 
   float(*poly_normals)[3];
diff --git a/source/blender/blenkernel/intern/mesh_runtime.c b/source/blender/blenkernel/intern/mesh_runtime.c
index e7e5064df7c..92bc94c2c96 100644
--- a/source/blender/blenkernel/intern/mesh_runtime.c
+++ b/source/blender/blenkernel/intern/mesh_runtime.c
@@ -104,6 +104,11 @@ void BKE_mesh_runtime_reset_on_copy(Mesh *mesh, const int UNUSED(flag))
   runtime->bvh_cache = NULL;
   runtime->shrinkwrap_data = NULL;
 
+  runtime->vert_normals_dirty = true;
+  runtime->poly_normals_dirty = true;
+  runtime->vert_normals = NULL;
+  runtime->poly_normals = NULL;
+
   mesh_runtime_init_mutexes(mesh);
 }
 
@@ -117,6 +122,7 @@ void BKE_mesh_runtime_clear_cache(Mesh *mesh)
   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);
 }
 
 /**
diff --git a/source/blender/bmesh/intern/bmesh_mesh_convert.cc b/source/blender/bmesh/intern/bmesh_mesh_convert.cc
index 41ca96b109e..4635da29d34 100644
--- a/source/blender/bmesh/intern/bmesh_mesh_convert.cc
+++ b/source/blender/bmesh/intern/bmesh_mesh_convert.cc
@@ -1205,7 +1205,9 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks *
   const int cd_edge_bweight_offset = CustomData_get_offset(&bm->edata, CD_BWEIGHT);
   const int cd_edge_crease_offset = CustomData_get_offset(&bm->edata, CD_CREASE);
 
-  BKE_mesh_normals_tag_dirty(me);
+  /* 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;
 
diff --git a/source/blender/makesdna/DNA_customdata_types.h b/source/blender/makesdna/DNA_customdata_types.h
index 629a5e88de7..3d65c62b09d 100644
--- a/source/blender/makesdna/DNA_customdata_types.h
+++ b/source/blender/makesdna/DNA_customdata_types.h
@@ -114,6 +114,10 @@ typedef enum CustomDataType {
   CD_MTFACE = 5,
   CD_MCOL = 6,
   CD_ORIGINDEX = 7,
+  /**
+   * Used for derived face corner normals on mesh `ldata`, since currently they are not computed
+   * lazily. Derived vertex and polygon normals are stored in #Mesh_Runtime.
+   */
   CD_NORMAL = 8,
   CD_FACEMAP = 9, /* exclusive face group, each face can only be part of one */
   CD_PROP_FLOAT = 10,
diff --git a/source/blender/makesdna/DNA_mesh_types.h b/source/blender/makesdna/DNA_mesh_types.h
index bf50100b302..24f3d1c95ed 100644
--- a/source/blender/makesdna/DNA_mesh_types.h
+++ b/source/blender/makesdna/DNA_mesh_types.h
@@ -132,14 +132,23 @@ typedef struct Mesh_Runtime {
    */
   char wrapper_type_finalize;
 
+  int subsurf_resolution;
   /**
    * Settings for lazily evaluating the subdivision on the CPU if needed. These are
    * set in the modifier when GPU subdivision can be performed.
    */
   char subsurf_apply_render;
   char subsurf_use_optimal_display;
-  char _pad[2];
-  int subsurf_resolution;
+
+  /**
+   * Caches for lazily computed vertex and polygon normals. These are stored here rather than in
+   * #CustomData because they can be calculated on a const mesh, and adding custom data layers on a
+   * const mesh is not thread-safe.
+   */
+  char vert_normals_dirty;
+  char poly_normals_dirty;
+  float (*vert_normals)[3];
+  float (*poly_normals)[3];
 
   void *_pad2;
 
diff --git a/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc b/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
index 1d1c5bd2285..c39c092f30a 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
@@ -316,7 +316,6 @@ static void extrude_mesh_vertices(MeshComponent &component,
   }
 
   BKE_mesh_runtime_clear_cache(&mesh);
-  BKE_mesh_normals_tag_dirty(&mesh);
 }
 
 static Array<Vector<int, 2>> mesh_calculate_polys_of_edge(const Mesh &mesh)
@@ -640,7 +639,6 @@ static void extrude_mesh_edges(MeshComponent &component,
   }
 
   BKE_mesh_runtime_clear_cache(&mesh);
-  BKE_mesh_normals_tag_dirty(&mesh);
 }
 
 /**
@@ -1009,7 +1007,6 @@ static void extrude_mesh_face_regions(MeshComponent &component,
   }
 
   BKE_mesh_runtime_clear_cache(&mesh);
-  BKE_mesh_normals_tag_dirty(&mesh);
 }
 
 /* Get the range into an array of extruded corners, edges, or vertices for a particular polygon. */
@@ -1277,7 +1274,6 @@ static void extrude_individual_mesh_faces(MeshComponent &component,
   }
 
   BKE_mesh_runtime_clear_cache(&mesh);
-  BKE_mesh_normals_tag_dirty(&mesh);
 }
 
 static void node_geo_exec(GeoNodeExecParams params)