[Bf-blender-cvs] [aa1e3419eec] temp-fix-normals-custom-data: Fix T95839: Data race when lazily creating mesh normal layers
Hans Goudey
noreply at git.blender.org
Mon Feb 21 19:54:11 CET 2022
Commit: aa1e3419eec4f4980544a27f26e6c83b00217912
Author: Hans Goudey
Date: Mon Feb 21 12:18:17 2022 -0500
Branches: temp-fix-normals-custom-data
https://developer.blender.org/rBaa1e3419eec4f4980544a27f26e6c83b00217912
Fix T95839: Data race when lazily creating mesh normal layers
===================================================================
M source/blender/blenkernel/BKE_mesh.h
M source/blender/blenkernel/intern/customdata.cc
M source/blender/blenkernel/intern/mesh.cc
M source/blender/blenkernel/intern/mesh_convert.cc
M source/blender/blenkernel/intern/mesh_normals.cc
M source/blender/blenkernel/intern/mesh_runtime.c
M source/blender/bmesh/intern/bmesh_mesh_convert.cc
M source/blender/editors/mesh/mesh_data.c
M source/blender/makesdna/DNA_customdata_types.h
M source/blender/makesdna/DNA_mesh_types.h
M source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
===================================================================
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
@@ -427,6 +427,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.
*/
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..6c51965aa4a 100644
--- a/source/blender/blenkernel/intern/mesh.cc
+++ b/source/blender/blenkernel/intern/mesh.cc
@@ -1111,16 +1111,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);
@@ -1986,7 +1976,6 @@ struct SplitFaceNewVert {
struct SplitFaceNewVert *next;
int new_index;
int orig_index;
- float *vnor;
};
struct SplitFaceNewEdge {
@@ -2065,7 +2054,6 @@ static int split_faces_prepare_new_verts(Mesh *mesh,
sizeof(*new_vert));
new_vert->orig_index = vert_idx;
new_vert->new_index = new_vert_idx;
- new_vert->vnor = (*lnor_space)->vec_lnor; /* See note above. */
new_vert->next = *new_verts;
*new_verts = new_vert;
}
@@ -2148,7 +2136,6 @@ static void split_faces_split_new_verts(Mesh *mesh,
{
const int verts_len = mesh->totvert - num_new_verts;
MVert *mvert = mesh->mvert;
- float(*vert_normals)[3] = BKE_mesh_vertex_normals_for_write(mesh);
/* Remember new_verts is a single linklist, so its items are in reversed order... */
MVert *new_mv = &mvert[mesh->totvert - 1];
@@ -2156,11 +2143,7 @@ static void split_faces_split_new_verts(Mesh *mesh,
BLI_assert(new_verts->new_index == i);
BLI_assert(new_verts->new_index != new_verts->orig_index);
CustomData_copy_data(&mesh->vdata, &mesh->vdata, new_verts->orig_index, i, 1);
- if (new_verts->vnor) {
- copy_v3_v3(vert_normals[i], new_verts->vnor);
- }
}
- BKE_mesh_vertex_normals_clear_dirty(mesh);
}
/* Perform actual split of edges. */
@@ -2248,7 +2231,8 @@ void BKE_mesh_split_faces(Mesh *mesh, bool free_loop_normals)
/* Also frees new_verts/edges temp data, since we used its memarena to allocate them. */
BKE_lnor_spacearr_free(&lnors_spacearr);
- BKE_mesh_assert_normals_dirty_or_calculated(mesh);
+ BKE_mesh_clear_derived_normals(mesh);
+
#ifdef VALIDATE_MESH
BKE_mesh_validate(mesh, true, true);
#endif
diff --git a/source/blender/blenkernel/intern/mesh_convert.cc b/source/blender/blenkernel/intern/mesh_convert.cc
index 3562f6c6b17..d57f06666f7 100644
--- a/source/blender/blenkernel/intern/mesh_convert.cc
+++ b/source/blender/blenkernel/intern/mesh_convert.cc
@@ -1495,15 +1495,7 @@ 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);
- }
+ 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..56020db8efd 100644
--- a/source/blender/blenkernel/intern/mesh_normals.cc
+++ b/source/blender/blenkernel/intern/mesh_normals.cc
@@ -110,53 +110,68 @@ 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__);
+ }
+ 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__);
+ }
+ 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)
+{
+ /* This function doesn't change the logical state of a mesh, so there could
+ * be a version that takes a const mesh and locks the mesh normals mutex. */
+ 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 +373,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)
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list