From cbc33871c6f66150342f7fd34ad463566f9784fd Mon Sep 17 00:00:00 2001 From: Sean Kim Date: Wed, 13 Nov 2024 15:19:05 +0100 Subject: [PATCH] Fix #129425: Using Voxel Remesh in sculpt mode can crash When using the Voxel Remesh operator, there are two possible sources of crashes currently: * TBB task stealing in nested parallel loops with thread local data * Null dereference of PBVH data when pushing sculpt undo steps To fix the former issue, this commit guards the `threading::parallel_for` internal function with `threading::isolate_task` to prevent possible task stealing and corruption of the thread local data. Additionally, it has the effect of fixing debug asserts inside `array_utils::gather` due to the this task stealing. To fix the latter issue, this commit adds a call to `BKE_sculpt_update_object_for_edit` to ensure that this data is populated. Original PR: !129704 Pull Request: https://projects.blender.org/blender/blender/pulls/129823 --- .../blenkernel/intern/mesh_remesh_voxel.cc | 78 ++++++++++--------- .../blender/editors/object/object_remesh.cc | 2 + 2 files changed, 43 insertions(+), 37 deletions(-) diff --git a/source/blender/blenkernel/intern/mesh_remesh_voxel.cc b/source/blender/blenkernel/intern/mesh_remesh_voxel.cc index 5ed33ccf45c..483e0fbf75e 100644 --- a/source/blender/blenkernel/intern/mesh_remesh_voxel.cc +++ b/source/blender/blenkernel/intern/mesh_remesh_voxel.cc @@ -347,16 +347,18 @@ static void find_nearest_faces(const Span src_tri_faces, }; threading::EnumerableThreadSpecific all_tls; threading::parallel_for(dst_faces.index_range(), 512, [&](const IndexRange range) { - TLS &tls = all_tls.local(); - Vector &face_centers = tls.face_centers; - face_centers.reinitialize(range.size()); - calc_face_centers(dst_positions, dst_faces.slice(range), dst_corner_verts, face_centers); + threading::isolate_task([&] { + TLS &tls = all_tls.local(); + Vector &face_centers = tls.face_centers; + face_centers.reinitialize(range.size()); + calc_face_centers(dst_positions, dst_faces.slice(range), dst_corner_verts, face_centers); - Vector &tri_indices = tls.tri_indices; - tri_indices.reinitialize(range.size()); - find_nearest_tris(face_centers, bvhtree, tri_indices); + Vector &tri_indices = tls.tri_indices; + tri_indices.reinitialize(range.size()); + find_nearest_tris(face_centers, bvhtree, tri_indices); - array_utils::gather(src_tri_faces, tri_indices.as_span(), nearest_faces.slice(range)); + array_utils::gather(src_tri_faces, tri_indices.as_span(), nearest_faces.slice(range)); + }); }); } @@ -410,41 +412,43 @@ static void find_nearest_edges(const Span src_positions, }; threading::EnumerableThreadSpecific all_tls; threading::parallel_for(nearest_edges.index_range(), 512, [&](const IndexRange range) { - TLS &tls = all_tls.local(); - Vector &edge_centers = tls.edge_centers; - edge_centers.reinitialize(range.size()); - calc_edge_centers(dst_positions, dst_edges.slice(range), edge_centers); + threading::isolate_task([&] { + TLS &tls = all_tls.local(); + Vector &edge_centers = tls.edge_centers; + edge_centers.reinitialize(range.size()); + calc_edge_centers(dst_positions, dst_edges.slice(range), edge_centers); - Vector &tri_indices = tls.tri_indices; - tri_indices.reinitialize(range.size()); - find_nearest_tris_parallel(edge_centers, bvhtree, tri_indices); + Vector &tri_indices = tls.tri_indices; + tri_indices.reinitialize(range.size()); + find_nearest_tris_parallel(edge_centers, bvhtree, tri_indices); - Vector &face_indices = tls.face_indices; - face_indices.reinitialize(range.size()); - array_utils::gather(src_tri_faces, tri_indices.as_span(), face_indices.as_mutable_span()); + Vector &face_indices = tls.face_indices; + face_indices.reinitialize(range.size()); + array_utils::gather(src_tri_faces, tri_indices.as_span(), face_indices.as_mutable_span()); - /* Find the source edge that's closest to the destination edge in the nearest face. Search - * through the whole face instead of just the triangle because the triangle has edges that - * might not be actual mesh edges. */ - Vector distances; - for (const int i : range.index_range()) { - const int dst_edge = range[i]; - const float3 &dst_position = edge_centers[i]; + /* Find the source edge that's closest to the destination edge in the nearest face. Search + * through the whole face instead of just the triangle because the triangle has edges that + * might not be actual mesh edges. */ + Vector distances; + for (const int i : range.index_range()) { + const int dst_edge = range[i]; + const float3 &dst_position = edge_centers[i]; - const int src_face = face_indices[i]; - const Span src_face_edges = src_corner_edges.slice(src_faces[src_face]); + const int src_face = face_indices[i]; + const Span src_face_edges = src_corner_edges.slice(src_faces[src_face]); - distances.reinitialize(src_face_edges.size()); - for (const int i : src_face_edges.index_range()) { - const int2 src_edge = src_edges[src_face_edges[i]]; - const float3 src_center = math::midpoint(src_positions[src_edge[0]], - src_positions[src_edge[1]]); - distances[i] = math::distance_squared(src_center, dst_position); + distances.reinitialize(src_face_edges.size()); + for (const int i : src_face_edges.index_range()) { + const int2 src_edge = src_edges[src_face_edges[i]]; + const float3 src_center = math::midpoint(src_positions[src_edge[0]], + src_positions[src_edge[1]]); + distances[i] = math::distance_squared(src_center, dst_position); + } + + const int min = std::min_element(distances.begin(), distances.end()) - distances.begin(); + nearest_edges[dst_edge] = src_face_edges[min]; } - - const int min = std::min_element(distances.begin(), distances.end()) - distances.begin(); - nearest_edges[dst_edge] = src_face_edges[min]; - } + }); }); } diff --git a/source/blender/editors/object/object_remesh.cc b/source/blender/editors/object/object_remesh.cc index fff1c5c33fc..0b707190095 100644 --- a/source/blender/editors/object/object_remesh.cc +++ b/source/blender/editors/object/object_remesh.cc @@ -132,6 +132,8 @@ static int voxel_remesh_exec(bContext *C, wmOperator *op) } if (ob->mode == OB_MODE_SCULPT) { + Depsgraph *depsgraph = CTX_data_depsgraph_pointer(C); + BKE_sculpt_update_object_for_edit(depsgraph, ob, false); sculpt_paint::undo::geometry_begin(*ob, op); }