Fix T96294: Crash and error with shape key normal calculation

A mistake in the mesh normal refactor caused the wrong mesh to
be used when calculating normals with a shape key's deformation.

This commit fixes the normal calculation by using the correct mesh,
with just adjusted vertex positions, and calculating the results
directly into the result arrays when possible. This completely avoids
the need to make a local copy of the mesh, which makes sense,
since the only thing that changes is the vertex positions.

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

4 files changed, 81 insertions, 72 deletions

diff --git a/source/blender/blenkernel/intern/key.c b/source/blender/blenkernel/intern/key.c
index 0df493e28c0..21630d0aea7 100644
--- a/source/blender/blenkernel/intern/key.c
+++ b/source/blender/blenkernel/intern/key.c
@@ -2186,16 +2186,14 @@ void BKE_keyblock_convert_from_mesh(Mesh *me, Key *key, KeyBlock *kb)
   BKE_keyblock_update_from_mesh(me, kb);
 }
 
-void BKE_keyblock_convert_to_mesh(KeyBlock *kb, Mesh *me)
+void BKE_keyblock_convert_to_mesh(KeyBlock *kb, struct MVert *mvert, int totvert)
 {
-  MVert *mvert;
   const float(*fp)[3];
   int a, tot;
 
-  mvert = me->mvert;
   fp = kb->data;
 
-  tot = min_ii(kb->totelem, me->totvert);
+  tot = min_ii(kb->totelem, totvert);
 
   for (a = 0; a < tot; a++, fp++, mvert++) {
     copy_v3_v3(mvert->co, *fp);
@@ -2208,68 +2206,77 @@ void BKE_keyblock_mesh_calc_normals(struct KeyBlock *kb,
                                     float (*r_polynors)[3],
                                     float (*r_loopnors)[3])
 {
-  /* We use a temp, shallow copy of mesh to work. */
-  Mesh me;
-  bool free_polynors = false;
-
   if (r_vertnors == NULL && r_polynors == NULL && r_loopnors == NULL) {
     return;
   }
 
-  me = *mesh;
-  me.mvert = MEM_dupallocN(mesh->mvert);
-  CustomData_reset(&me.vdata);
-  CustomData_reset(&me.edata);
-  CustomData_reset(&me.pdata);
-  CustomData_reset(&me.ldata);
-  CustomData_reset(&me.fdata);
-
-  BKE_keyblock_convert_to_mesh(kb, &me);
-
-  if (r_polynors == NULL && r_loopnors != NULL) {
-    r_polynors = MEM_mallocN(sizeof(float[3]) * me.totpoly, __func__);
-    free_polynors = true;
-  }
-
-  const float(*vert_normals)[3] = BKE_mesh_vertex_normals_ensure(mesh);
-  if (r_vertnors) {
-    memcpy(r_vertnors, vert_normals, sizeof(float[3]) * me.totvert);
-  }
-
-  const float(*face_normals)[3] = BKE_mesh_poly_normals_ensure(mesh);
-  memcpy(r_polynors, face_normals, sizeof(float[3]) * me.totpoly);
-
-  if (r_loopnors) {
+  MVert *mvert = MEM_dupallocN(mesh->mvert);
+  BKE_keyblock_convert_to_mesh(kb, mesh->mvert, mesh->totvert);
+
+  const bool loop_normals_needed = r_loopnors != NULL;
+  const bool vert_normals_needed = r_vertnors != NULL || loop_normals_needed;
+  const bool poly_normals_needed = r_polynors != NULL || vert_normals_needed ||
+                                   loop_normals_needed;
+
+  float(*vert_normals)[3] = r_vertnors;
+  float(*poly_normals)[3] = r_polynors;
+  bool free_vert_normals = false;
+  bool free_poly_normals = false;
+  if (vert_normals_needed && r_vertnors == NULL) {
+    vert_normals = MEM_malloc_arrayN(mesh->totvert, sizeof(float[3]), __func__);
+    free_vert_normals = true;
+  }
+  if (poly_normals_needed && r_polynors == NULL) {
+    poly_normals = MEM_malloc_arrayN(mesh->totpoly, sizeof(float[3]), __func__);
+    free_poly_normals = true;
+  }
+
+  if (poly_normals_needed) {
+    BKE_mesh_calc_normals_poly(mvert,
+                               mesh->totvert,
+                               mesh->mloop,
+                               mesh->totloop,
+                               mesh->mpoly,
+                               mesh->totpoly,
+                               poly_normals);
+  }
+  if (vert_normals_needed) {
+    BKE_mesh_calc_normals_poly_and_vertex(mvert,
+                                          mesh->totvert,
+                                          mesh->mloop,
+                                          mesh->totloop,
+                                          mesh->mpoly,
+                                          mesh->totpoly,
+                                          poly_normals,
+                                          vert_normals);
+  }
+  if (loop_normals_needed) {
     short(*clnors)[2] = CustomData_get_layer(&mesh->ldata, CD_CUSTOMLOOPNORMAL); /* May be NULL. */
-
-    BKE_mesh_normals_loop_split(me.mvert,
+    BKE_mesh_normals_loop_split(mesh->mvert,
                                 vert_normals,
-                                me.totvert,
-                                me.medge,
-                                me.totedge,
-                                me.mloop,
+                                mesh->totvert,
+                                mesh->medge,
+                                mesh->totedge,
+                                mesh->mloop,
                                 r_loopnors,
-                                me.totloop,
-                                me.mpoly,
-                                face_normals,
-                                me.totpoly,
-                                (me.flag & ME_AUTOSMOOTH) != 0,
-                                me.smoothresh,
+                                mesh->totloop,
+                                mesh->mpoly,
+                                poly_normals,
+                                mesh->totpoly,
+                                (mesh->flag & ME_AUTOSMOOTH) != 0,
+                                mesh->smoothresh,
                                 NULL,
                                 clnors,
                                 NULL);
   }
 
-  CustomData_free(&me.vdata, me.totvert);
-  CustomData_free(&me.edata, me.totedge);
-  CustomData_free(&me.pdata, me.totpoly);
-  CustomData_free(&me.ldata, me.totloop);
-  CustomData_free(&me.fdata, me.totface);
-  MEM_freeN(me.mvert);
-
-  if (free_polynors) {
-    MEM_freeN(r_polynors);
+  if (free_vert_normals) {
+    MEM_freeN(vert_normals);
+  }
+  if (free_poly_normals) {
+    MEM_freeN(poly_normals);
   }
+  MEM_freeN(mvert);
 }
 
 /************************* raw coords ************************/
diff --git a/source/blender/blenkernel/intern/mesh_convert.cc b/source/blender/blenkernel/intern/mesh_convert.cc
index 1db17c950b3..9bf7d14eb80 100644
--- a/source/blender/blenkernel/intern/mesh_convert.cc
+++ b/source/blender/blenkernel/intern/mesh_convert.cc
@@ -1321,7 +1321,7 @@ Mesh *BKE_mesh_create_derived_for_modifier(struct Depsgraph *depsgraph,
 
   if (build_shapekey_layers && me->key &&
       (kb = (KeyBlock *)BLI_findlink(&me->key->block, ob_eval->shapenr - 1))) {
-    BKE_keyblock_convert_to_mesh(kb, me);
+    BKE_keyblock_convert_to_mesh(kb, me->mvert, me->totvert);
   }
 
   Mesh *mesh_temp = (Mesh *)BKE_id_copy_ex(nullptr, &me->id, nullptr, LIB_ID_COPY_LOCALIZE);
diff --git a/source/blender/blenkernel/intern/mesh_normals.cc b/source/blender/blenkernel/intern/mesh_normals.cc
index b690d7f1e32..276e823126d 100644
--- a/source/blender/blenkernel/intern/mesh_normals.cc
+++ b/source/blender/blenkernel/intern/mesh_normals.cc
@@ -323,14 +323,14 @@ static void mesh_calc_normals_poly_and_vertex_finalize_fn(
   }
 }
 
-static void mesh_calc_normals_poly_and_vertex(MVert *mvert,
-                                              const int mvert_len,
-                                              const MLoop *mloop,
-                                              const int UNUSED(mloop_len),
-                                              const MPoly *mpoly,
-                                              const int mpoly_len,
-                                              float (*r_poly_normals)[3],
-                                              float (*r_vert_normals)[3])
+void BKE_mesh_calc_normals_poly_and_vertex(MVert *mvert,
+                                           const int mvert_len,
+                                           const MLoop *mloop,
+                                           const int UNUSED(mloop_len),
+                                           const MPoly *mpoly,
+                                           const int mpoly_len,
+                                           float (*r_poly_normals)[3],
+                                           float (*r_vert_normals)[3])
 {
   TaskParallelSettings settings;
   BLI_parallel_range_settings_defaults(&settings);
@@ -403,14 +403,14 @@ const float (*BKE_mesh_vertex_normals_ensure(const Mesh *mesh))[3]
     vert_normals = BKE_mesh_vertex_normals_for_write(&mesh_mutable);
     poly_normals = BKE_mesh_poly_normals_for_write(&mesh_mutable);
 
-    mesh_calc_normals_poly_and_vertex(mesh_mutable.mvert,
-                                      mesh_mutable.totvert,
-                                      mesh_mutable.mloop,
-                                      mesh_mutable.totloop,
-                                      mesh_mutable.mpoly,
-                                      mesh_mutable.totpoly,
-                                      poly_normals,
-                                      vert_normals);
+    BKE_mesh_calc_normals_poly_and_vertex(mesh_mutable.mvert,
+                                          mesh_mutable.totvert,
+                                          mesh_mutable.mloop,
+                                          mesh_mutable.totloop,
+                                          mesh_mutable.mpoly,
+                                          mesh_mutable.totpoly,
+                                          poly_normals,
+                                          vert_normals);
 
     BKE_mesh_vertex_normals_clear_dirty(&mesh_mutable);
     BKE_mesh_poly_normals_clear_dirty(&mesh_mutable);
diff --git a/source/blender/blenkernel/intern/object.cc b/source/blender/blenkernel/intern/object.cc
index 6dc6e37de27..ef86647000d 100644
--- a/source/blender/blenkernel/intern/object.cc
+++ b/source/blender/blenkernel/intern/object.cc
@@ -4902,9 +4902,11 @@ bool BKE_object_shapekey_remove(Main *bmain, Object *ob, KeyBlock *kb)
     if (key->refkey) {
       /* apply new basis key on original data */
       switch (ob->type) {
-        case OB_MESH:
-          BKE_keyblock_convert_to_mesh(key->refkey, (Mesh *)ob->data);
+        case OB_MESH: {
+          Mesh *mesh = (Mesh *)ob->data;
+          BKE_keyblock_convert_to_mesh(key->refkey, mesh->mvert, mesh->totvert);
           break;
+        }
         case OB_CURVE:
         case OB_SURF:
           BKE_keyblock_convert_to_curve(