[Bf-blender-cvs] [b0aaf6ff4a9] master: Fix T96294: Crash and error with shape key normal calculation

Hans Goudey noreply at git.blender.org
Tue Mar 22 15:43:10 CET 2022


Commit: b0aaf6ff4a9731e9142d8cf32ebcc1a01a4f5cc8
Author: Hans Goudey
Date:   Tue Mar 22 09:43:02 2022 -0500
Branches: master
https://developer.blender.org/rBb0aaf6ff4a9731e9142d8cf32ebcc1a01a4f5cc8

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

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

M	source/blender/blenkernel/BKE_key.h
M	source/blender/blenkernel/BKE_mesh.h
M	source/blender/blenkernel/intern/key.c
M	source/blender/blenkernel/intern/mesh_convert.cc
M	source/blender/blenkernel/intern/mesh_normals.cc
M	source/blender/blenkernel/intern/object.cc
M	source/blender/io/collada/GeometryExporter.cpp

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

diff --git a/source/blender/blenkernel/BKE_key.h b/source/blender/blenkernel/BKE_key.h
index da436b7d33f..bf9e7651e36 100644
--- a/source/blender/blenkernel/BKE_key.h
+++ b/source/blender/blenkernel/BKE_key.h
@@ -13,6 +13,7 @@ struct Lattice;
 struct ListBase;
 struct Main;
 struct Mesh;
+struct MVert;
 struct Object;
 
 /* Kernel prototypes */
@@ -119,7 +120,8 @@ void BKE_keyblock_convert_to_curve(struct KeyBlock *kb, struct Curve *cu, struct
 
 void BKE_keyblock_update_from_mesh(struct Mesh *me, struct KeyBlock *kb);
 void BKE_keyblock_convert_from_mesh(struct Mesh *me, struct Key *key, struct KeyBlock *kb);
-void BKE_keyblock_convert_to_mesh(struct KeyBlock *kb, struct Mesh *me);
+void BKE_keyblock_convert_to_mesh(struct KeyBlock *kb, struct MVert *mvert, int totvert);
+
 /**
  * Computes normals (vertices, polygons and/or loops ones) of given mesh for given shape key.
  *
diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h
index 2373bb289cd..0ba9713b96d 100644
--- a/source/blender/blenkernel/BKE_mesh.h
+++ b/source/blender/blenkernel/BKE_mesh.h
@@ -471,6 +471,21 @@ void BKE_mesh_calc_normals_poly(const struct MVert *mvert,
                                 int mpoly_len,
                                 float (*r_poly_normals)[3]);
 
+/**
+ * Calculate face and vertex normals directly into result arrays.
+ *
+ * \note Usually #BKE_mesh_vertex_normals_ensure is the preferred way to access vertex normals,
+ * since they may already be calculated and cached on the mesh.
+ */
+void BKE_mesh_calc_normals_poly_and_vertex(const struct MVert *mvert,
+                                           int mvert_len,
+                                           const struct MLoop *mloop,
+                                           int mloop_len,
+                                           const struct MPoly *mpoly,
+                                           int mpoly_len,
+                                           float (*r_poly_normals)[3],
+                                           float (*r_vert_normals)[3]);
+
 /**
  * Calculate vertex and face normals, storing the result in custom data layers on the mesh.
  *
diff --git a/source/blender/blenkernel/intern/key.c b/source/blender/blenkernel/intern/key.c
index 11d6e6ef973..5247e9f358b 100644
--- a/source/blender/blenkernel/intern/key.c
+++ b/source/blender/blenkernel/intern/key.c
@@ -2171,16 +2171,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);
@@ -2193,68 +2191,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 1542f7119d1..40c6fbcf67e 100644
--- a/source/blender/blenkernel/intern/mesh_convert.cc
+++ b/source/blender/blenkernel/intern/mesh_convert.cc
@@ -1308,7 +1308,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 1c2a903d8c3..7633a3155ba 100644
--- a/source/blender/blenkernel/intern/mesh_normals.cc
+++ b/source/blender/blenkernel/intern/mesh_normals.cc
@@ -306,14 +306,14 @@ static void mesh_calc_normals_poly_and_vertex_finalize_fn(
   }
 }
 
-static void mesh_calc_normals_poly_and_vertex(const 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(const 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);
@@ -372,14 +372,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_mut

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list