griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Refactor: Speed up and fix some issues in mesh wrapper

Browse Source 
* Fix mesh_copy_data setting the subd wrapper type without copying the
  associated tessellated mesh. It's unclear if this currently causes any
  issues, but with future changes it would crash.
* Use double checked lock to avoid mutex locks on every call to
  BKE_mesh_wrapper_ensure_mdata and BKE_mesh_wrapper_ensure_subdivision.
* Set wrapper type to subd also if subdivision was skipped due to bad
  topology or level set to zero. Previous it would do mutex locking and
  TBB task isolation on every function call.
* Add assert when BKE_mesh_wrapper_ensure_mdata changes the wrapper type
  from subd to mdata. This doesn't really make any sense, it just loses
  the subdivision detail. If any code relies on this we need to understand
  why and maybe change it.

Pull Request: https://projects.blender.org/blender/blender/pulls/135895
This commit is contained in:
Brecht Van Lommel
2025-03-13 01:18:40 +01:00
parent 9526fe9993
commit 321ec72c74
2 changed files with 50 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
source/blender/blenkernel/intern/mesh.cc
View File

@@ -109,7 +109,10 @@ static void mesh_copy_data(Main *bmain,
mesh_dst->runtime = new blender::bke::MeshRuntime();
mesh_dst->runtime->deformed_only = mesh_src->runtime->deformed_only;
mesh_dst->runtime->wrapper_type = mesh_src->runtime->wrapper_type;
/* Subd runtime.mesh_eval is not copied, will need to be reevaluated. */
mesh_dst->runtime->wrapper_type = (mesh_src->runtime->wrapper_type == ME_WRAPPER_TYPE_SUBD) ?
ME_WRAPPER_TYPE_MDATA :
mesh_src->runtime->wrapper_type;
mesh_dst->runtime->subsurf_runtime_data = mesh_src->runtime->subsurf_runtime_data;
mesh_dst->runtime->cd_mask_extra = mesh_src->runtime->cd_mask_extra;
/* Copy face dot tags and edge tags, since meshes may be duplicated after a subsurf modifier or

86
source/blender/blenkernel/intern/mesh_wrapper.cc
View File

@@ -78,6 +78,10 @@ Mesh *BKE_mesh_wrapper_from_editmesh(std::shared_ptr<BMEditMesh> em,
void BKE_mesh_wrapper_ensure_mdata(Mesh *mesh)
{
if (mesh->runtime->wrapper_type == ME_WRAPPER_TYPE_MDATA) {
return;
}
std::lock_guard lock{mesh->runtime->eval_mutex};
if (mesh->runtime->wrapper_type == ME_WRAPPER_TYPE_MDATA) {
return;
@@ -85,42 +89,38 @@ void BKE_mesh_wrapper_ensure_mdata(Mesh *mesh)
/* Must isolate multithreaded tasks while holding a mutex lock. */
blender::threading::isolate_task([&]() {
switch (mesh->runtime->wrapper_type) {
case ME_WRAPPER_TYPE_MDATA:
case ME_WRAPPER_TYPE_SUBD: {
break; /* Quiet warning. */
if (mesh->runtime->wrapper_type == ME_WRAPPER_TYPE_BMESH) {
mesh->verts_num = 0;
mesh->edges_num = 0;
mesh->faces_num = 0;
mesh->corners_num = 0;
BLI_assert(mesh->runtime->edit_mesh != nullptr);
BLI_assert(mesh->runtime->edit_data != nullptr);
BMEditMesh *em = mesh->runtime->edit_mesh.get();
BM_mesh_bm_to_me_for_eval(*em->bm, *mesh, &mesh->runtime->cd_mask_extra);
/* Adding original index layers here assumes that all BMesh Mesh wrappers are created from
* original edit mode meshes (the only case where adding original indices makes sense).
* If that assumption is broken, the layers might be incorrect because they might not
* actually be "original".
*
* There is also a performance aspect, where this also assumes that original indices are
* always needed when converting a BMesh to a mesh with the mesh wrapper system. That might
* be wrong, but it's not harmful. */
BKE_mesh_ensure_default_orig_index_customdata_no_check(mesh);
blender::bke::EditMeshData &edit_data = *mesh->runtime->edit_data;
if (!edit_data.vert_positions.is_empty()) {
mesh->vert_positions_for_write().copy_from(edit_data.vert_positions);
mesh->runtime->is_original_bmesh = false;
}
case ME_WRAPPER_TYPE_BMESH: {
mesh->verts_num = 0;
mesh->edges_num = 0;
mesh->faces_num = 0;
mesh->corners_num = 0;
BLI_assert(mesh->runtime->edit_mesh != nullptr);
BLI_assert(mesh->runtime->edit_data != nullptr);
BMEditMesh *em = mesh->runtime->edit_mesh.get();
BM_mesh_bm_to_me_for_eval(*em->bm, *mesh, &mesh->runtime->cd_mask_extra);
/* Adding original index layers here assumes that all BMesh Mesh wrappers are created from
* original edit mode meshes (the only case where adding original indices makes sense).
* If that assumption is broken, the layers might be incorrect because they might not
* actually be "original".
*
* There is also a performance aspect, where this also assumes that original indices are
* always needed when converting a BMesh to a mesh with the mesh wrapper system. That might
* be wrong, but it's not harmful. */
BKE_mesh_ensure_default_orig_index_customdata_no_check(mesh);
blender::bke::EditMeshData &edit_data = *mesh->runtime->edit_data;
if (!edit_data.vert_positions.is_empty()) {
mesh->vert_positions_for_write().copy_from(edit_data.vert_positions);
mesh->runtime->is_original_bmesh = false;
}
mesh->runtime->edit_data.reset();
break;
}
mesh->runtime->edit_data.reset();
}
else if (mesh->runtime->wrapper_type == ME_WRAPPER_TYPE_SUBD) {
BLI_assert(!"Should not be converting subd wrapper to mdata wrapper");
}
/* Keep type assignment last, so that read-only access only uses the mdata code paths after all
@@ -311,7 +311,8 @@ static Mesh *mesh_wrapper_ensure_subdivision(Mesh *mesh)
{
using namespace blender::bke;
SubsurfRuntimeData *runtime_data = mesh->runtime->subsurf_runtime_data;
if (runtime_data == nullptr || runtime_data->settings.level == 0) {
if (runtime_data->settings.level == 0) {
mesh->runtime->wrapper_type = ME_WRAPPER_TYPE_SUBD;
return mesh;
}
@@ -323,6 +324,7 @@ static Mesh *mesh_wrapper_ensure_subdivision(Mesh *mesh)
mesh_settings.use_optimal_display = runtime_data->use_optimal_display;
if (mesh_settings.resolution < 3) {
mesh->runtime->wrapper_type = ME_WRAPPER_TYPE_SUBD;
return mesh;
}
@@ -330,6 +332,7 @@ static Mesh *mesh_wrapper_ensure_subdivision(Mesh *mesh)
runtime_data, mesh, false);
if (subdiv == nullptr) {
/* Happens on bad topology, but also on empty input mesh. */
mesh->runtime->wrapper_type = ME_WRAPPER_TYPE_SUBD;
return mesh;
}
const bool use_clnors = runtime_data->use_loop_normals;
@@ -362,6 +365,7 @@ static Mesh *mesh_wrapper_ensure_subdivision(Mesh *mesh)
}
mesh->runtime->mesh_eval = subdiv_mesh;
mesh->runtime->wrapper_type = ME_WRAPPER_TYPE_SUBD;
BLI_assert(mesh->runtime->mesh_eval != nullptr);
}
return mesh->runtime->mesh_eval;
@@ -369,17 +373,19 @@ static Mesh *mesh_wrapper_ensure_subdivision(Mesh *mesh)
Mesh *BKE_mesh_wrapper_ensure_subdivision(Mesh *mesh)
{
std::lock_guard lock{mesh->runtime->eval_mutex};
if (mesh->runtime->wrapper_type == ME_WRAPPER_TYPE_SUBD) {
return mesh->runtime->mesh_eval;
/* Subdiv evaluation might have been skipped, in which case the original mesh is ok. */
return (mesh->runtime->mesh_eval) ? mesh->runtime->mesh_eval : mesh;
}
if (mesh->runtime->subsurf_runtime_data == nullptr) {
return mesh;
}
Mesh *result;
std::lock_guard lock{mesh->runtime->eval_mutex};
/* Must isolate multithreaded tasks while holding a mutex lock. */
Mesh *result;
blender::threading::isolate_task([&]() { result = mesh_wrapper_ensure_subdivision(mesh); });
return result;
}