From 4bdf5bf2e289039b869ab91f1d25539e00a66b04 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Wed, 3 Apr 2024 14:15:31 +0200 Subject: [PATCH] Mesh: Remove unnecessary wrapper_type_finalize logic After some discussions and investigation over the last couple months, it's not clear what the "wrapper type finalize" logic is necessary for. For edit meshes and regular meshes, normals are calculated lazily when building the draw cache. Apart from the unnecessary complication for mesh GPU draw data extraction, this code also causes normals to always be calculated when turning an edit mesh wrapper into a regular mesh. However, those normals are immediately discarded since the edit deform cache is deleted in the next line. Beyond the obvious simplification, the motivation for this change is to avoid requesting write access on the evaluated mesh and cage mesh. This works better with implicit sharing, allowing other improvements. Pull Request: https://projects.blender.org/blender/blender/pulls/120066 --- source/blender/blenkernel/BKE_mesh.h | 3 -- source/blender/blenkernel/BKE_mesh_types.hh | 5 -- .../blender/blenkernel/intern/DerivedMesh.cc | 48 ------------------- source/blender/blenkernel/intern/mesh.cc | 1 - .../blender/blenkernel/intern/mesh_wrapper.cc | 4 -- 5 files changed, 61 deletions(-) diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h index ba4f08993e9..f2e66c87dc4 100644 --- a/source/blender/blenkernel/BKE_mesh.h +++ b/source/blender/blenkernel/BKE_mesh.h @@ -494,9 +494,6 @@ bool BKE_mesh_validate_all_customdata(CustomData *vert_data, void BKE_mesh_strip_loose_faces(Mesh *mesh); -/* In DerivedMesh.cc */ -void BKE_mesh_wrapper_deferred_finalize_mdata(Mesh *mesh_eval); - /* **** Depsgraph evaluation **** */ void BKE_mesh_eval_geometry(Depsgraph *depsgraph, Mesh *mesh); diff --git a/source/blender/blenkernel/BKE_mesh_types.hh b/source/blender/blenkernel/BKE_mesh_types.hh index b69ccce25dd..580456d4abd 100644 --- a/source/blender/blenkernel/BKE_mesh_types.hh +++ b/source/blender/blenkernel/BKE_mesh_types.hh @@ -163,11 +163,6 @@ struct MeshRuntime { /** #eMeshWrapperType and others. */ eMeshWrapperType wrapper_type = ME_WRAPPER_TYPE_MDATA; - /** - * A type mask from wrapper_type, - * in case there are differences in finalizing logic between types. - */ - eMeshWrapperType wrapper_type_finalize = ME_WRAPPER_TYPE_MDATA; /** * Settings for lazily evaluating the subdivision on the CPU if needed. These are diff --git a/source/blender/blenkernel/intern/DerivedMesh.cc b/source/blender/blenkernel/intern/DerivedMesh.cc index 4e193d679bc..13c44f50004 100644 --- a/source/blender/blenkernel/intern/DerivedMesh.cc +++ b/source/blender/blenkernel/intern/DerivedMesh.cc @@ -89,8 +89,6 @@ using blender::bke::MeshComponent; #endif static void mesh_init_origspace(Mesh *mesh); -static void editbmesh_calc_modifier_final_normals(Mesh *mesh_final); -static void editbmesh_calc_modifier_final_normals_or_defer(Mesh *mesh_final); /* -------------------------------------------------------------------- */ @@ -465,16 +463,6 @@ static void mesh_calc_finalize(const Mesh *mesh_input, Mesh *mesh_eval) mesh_eval->runtime->edit_mesh = mesh_input->runtime->edit_mesh; } -void BKE_mesh_wrapper_deferred_finalize_mdata(Mesh *mesh_eval) -{ - if (mesh_eval->runtime->wrapper_type_finalize & (1 << ME_WRAPPER_TYPE_BMESH)) { - editbmesh_calc_modifier_final_normals(mesh_eval); - mesh_eval->runtime->wrapper_type_finalize = eMeshWrapperType( - mesh_eval->runtime->wrapper_type_finalize & ~(1 << ME_WRAPPER_TYPE_BMESH)); - } - BLI_assert(mesh_eval->runtime->wrapper_type_finalize == 0); -} - /** * Modifies the given mesh and geometry set. The mesh is not passed as part of the mesh component * in the \a geometry_set input, it is only passed in \a input_mesh and returned in the return @@ -1001,36 +989,6 @@ bool editbmesh_modifier_is_enabled(const Scene *scene, return true; } -static void editbmesh_calc_modifier_final_normals(Mesh *mesh_final) -{ - switch (mesh_final->runtime->wrapper_type) { - case ME_WRAPPER_TYPE_SUBD: - case ME_WRAPPER_TYPE_MDATA: - break; - case ME_WRAPPER_TYPE_BMESH: { - BMEditMesh &em = *mesh_final->runtime->edit_mesh; - blender::bke::EditMeshData &emd = *mesh_final->runtime->edit_data; - if (!emd.vert_positions.is_empty()) { - BKE_editmesh_cache_ensure_vert_normals(em, emd); - BKE_editmesh_cache_ensure_face_normals(em, emd); - } - return; - } - } -} - -static void editbmesh_calc_modifier_final_normals_or_defer(Mesh *mesh_final) -{ - if (mesh_final->runtime->wrapper_type != ME_WRAPPER_TYPE_MDATA) { - /* Generated at draw time. */ - mesh_final->runtime->wrapper_type_finalize = eMeshWrapperType( - 1 << mesh_final->runtime->wrapper_type); - return; - } - - editbmesh_calc_modifier_final_normals(mesh_final); -} - static MutableSpan mesh_wrapper_vert_coords_ensure_for_write(Mesh *mesh) { switch (mesh->runtime->wrapper_type) { @@ -1244,12 +1202,6 @@ static void editbmesh_calc_modifiers(Depsgraph *depsgraph, BKE_id_free(nullptr, mesh_orco); } - /* Compute normals. */ - editbmesh_calc_modifier_final_normals_or_defer(mesh_final); - if (mesh_cage && (mesh_cage != mesh_final)) { - editbmesh_calc_modifier_final_normals_or_defer(mesh_cage); - } - /* Return final mesh. */ *r_final = mesh_final; if (r_cage) { diff --git a/source/blender/blenkernel/intern/mesh.cc b/source/blender/blenkernel/intern/mesh.cc index 84c8705db13..048b3a9837c 100644 --- a/source/blender/blenkernel/intern/mesh.cc +++ b/source/blender/blenkernel/intern/mesh.cc @@ -113,7 +113,6 @@ 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; - mesh_dst->runtime->wrapper_type_finalize = mesh_src->runtime->wrapper_type_finalize; 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 diff --git a/source/blender/blenkernel/intern/mesh_wrapper.cc b/source/blender/blenkernel/intern/mesh_wrapper.cc index ad287d4225b..8ccd2d2051f 100644 --- a/source/blender/blenkernel/intern/mesh_wrapper.cc +++ b/source/blender/blenkernel/intern/mesh_wrapper.cc @@ -129,10 +129,6 @@ void BKE_mesh_wrapper_ensure_mdata(Mesh *mesh) mesh->runtime->is_original_bmesh = false; } - if (mesh->runtime->wrapper_type_finalize) { - BKE_mesh_wrapper_deferred_finalize_mdata(mesh); - } - mesh->runtime->edit_data.reset(); break; }