blender
/
blender
136
418
Fork 600
Code Issues 6.3k Pull Requests 778 Projects 19 Wiki Activity

Mesh: Store BMEditMesh in shared pointer #120276

Merged
Hans Goudey merged 5 commits from HooglyBoogly/blender:mesh-edit-mesh-shared-ptr into main 2024-04-18 13:52:32 +02:00
Conversation 7 Commits 5 Files Changed 46 +14 -15
5 changed files with 14 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 7206f7955c - Show all commits

4
source/blender/blenkernel/BKE_mesh_wrapper.hh
View File

@ -7,6 +7,8 @@
* \ingroup bke
*/
#include <memory>
#include "BLI_math_vector_types.hh"
#include "BLI_span.hh"
@ -14,7 +16,7 @@ struct BMEditMesh;
struct CustomData_MeshMasks;
struct Mesh;
Mesh *BKE_mesh_wrapper_from_editmesh(BMEditMesh *em,
Mesh *BKE_mesh_wrapper_from_editmesh(std::shared_ptr<BMEditMesh> em,
const CustomData_MeshMasks *cd_mask_extra,
const Mesh *me_settings);
void BKE_mesh_wrapper_ensure_mdata(Mesh *mesh);

16
source/blender/blenkernel/intern/DerivedMesh.cc
View File

@ -1009,7 +1009,6 @@ static MutableSpan<float3> mesh_wrapper_vert_coords_ensure_for_write(Mesh *mesh)
static void editbmesh_calc_modifiers(Depsgraph *depsgraph,
const Scene *scene,
Object *ob,
BMEditMesh *em_input,
const CustomData_MeshMasks *dataMask,
/* return args */
Mesh **r_cage,
@ -1017,6 +1016,8 @@ static void editbmesh_calc_modifiers(Depsgraph *depsgraph,
GeometrySet **r_geometry_set)
{
Mesh *mesh_input = (Mesh *)ob->data;
BMEditMesh *em_input = mesh_input->runtime->edit_mesh.get();
Mesh *mesh_cage = nullptr;
/* This geometry set contains the non-mesh data that might be generated by modifiers. */
GeometrySet geometry_set_final;
@ -1049,7 +1050,8 @@ static void editbmesh_calc_modifiers(Depsgraph *depsgraph,
CDMaskLink *md_datamask = datamasks;
CustomData_MeshMasks append_mask = CD_MASK_BAREMESH;
Mesh *mesh_final = BKE_mesh_wrapper_from_editmesh(em_input, &final_datamask, mesh_input);
Mesh *mesh_final = BKE_mesh_wrapper_from_editmesh(
mesh_input->runtime->edit_mesh, &final_datamask, mesh_input);
int cageIndex = BKE_modifiers_get_cage_index(scene, ob, nullptr, true);
if (r_cage && cageIndex == -1) {
@ -1293,14 +1295,8 @@ static void editbmesh_build_data(Depsgraph *depsgraph,
Mesh *me_final;
GeometrySet *non_mesh_components;
editbmesh_calc_modifiers(depsgraph,
scene,
obedit,
mesh->runtime->edit_mesh.get(),
dataMask,
&me_cage,
&me_final,
&non_mesh_components);
editbmesh_calc_modifiers(
depsgraph, scene, obedit, dataMask, &me_cage, &me_final, &non_mesh_components);
/* The modifier stack result is expected to share edit mesh pointer with the input.
* This is similar `mesh_calc_finalize()`. */

3
source/blender/blenkernel/intern/crazyspace.cc
View File

@ -273,7 +273,8 @@ int BKE_crazyspace_get_first_deform_matrices_editbmesh(
cd_mask_extra = datamasks->mask;
BLI_linklist_free((LinkNode *)datamasks, nullptr);
mesh = BKE_mesh_wrapper_from_editmesh(em, &cd_mask_extra, me_input);
mesh = BKE_mesh_wrapper_from_editmesh(
std::make_shared<BMEditMesh>(*em), &cd_mask_extra, me_input);
deformcos.reinitialize(verts_num);
BKE_mesh_wrapper_vert_coords_copy(mesh, deformcos);
deformmats.reinitialize(verts_num);

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

@ -52,7 +52,7 @@
using blender::float3;
using blender::Span;
Mesh *BKE_mesh_wrapper_from_editmesh(BMEditMesh *em,
Mesh *BKE_mesh_wrapper_from_editmesh(std::shared_ptr<BMEditMesh> em,
const CustomData_MeshMasks *cd_mask_extra,
const Mesh *me_settings)
{
@ -68,7 +68,7 @@ Mesh *BKE_mesh_wrapper_from_editmesh(BMEditMesh *em,
/* Use edit-mesh directly where possible. */
mesh->runtime->is_original_bmesh = true;
mesh->runtime->edit_mesh = std::make_shared<BMEditMesh>(*em);
mesh->runtime->edit_mesh = std::move(em);
HooglyBoogly marked this conversation as resolved Outdated
Sergey Sharybin commented 2024-04-15 17:12:22 +02:00
Outdated
Review
Copy Link

This seems to be a wrong usage of usage of shared pointers. There are now code-paths which will get bare pointer from the mesh->runtime->edit_mesh.get(), pass it to the BKE_mesh_wrapper_from_editmesh, and here a new shared pointer is created.
From my understanding it defeats the original intent of an evaluated state sharing the same em pointer as the input.

This seems to be a wrong usage of usage of shared pointers. There are now code-paths which will get bare pointer from the `mesh->runtime->edit_mesh.get()`, pass it to the `BKE_mesh_wrapper_from_editmesh`, and here a new shared pointer is created. From my understanding it defeats the original intent of an evaluated state sharing the same `em` pointer as the input.
Hans Goudey commented 2024-04-15 18:12:37 +02:00
Outdated
Review
Copy Link

Indeed, thanks. It isn't harmful to correctness in this PR, but it might have messed with my future plans here (comparing edit mesh pointer equality to decide whether to use mapped GPU data extraction). I'll change this function to take a shared pointer argument. That should reduce memory usage too.

Indeed, thanks. It isn't harmful to correctness in this PR, but it might have messed with my future plans here (comparing edit mesh pointer equality to decide whether to use mapped GPU data extraction). I'll change this function to take a shared pointer argument. That should reduce memory usage too.
/* Make sure we crash if these are ever used. */
#ifndef NDEBUG

2
source/blender/editors/geometry/node_group_operator.cc
View File

@ -182,7 +182,7 @@ static bke::GeometrySet get_original_geometry_eval_copy(Object &object)
}
case OB_MESH: {
const Mesh *mesh = static_cast<const Mesh *>(object.data);
if (BMEditMesh *em = mesh->runtime->edit_mesh.get()) {
if (std::shared_ptr<BMEditMesh> &em = mesh->runtime->edit_mesh) {
Mesh *mesh_copy = BKE_mesh_wrapper_from_editmesh(em, nullptr, mesh);
BKE_mesh_wrapper_ensure_mdata(mesh_copy);
Mesh *final_copy = BKE_mesh_copy_for_eval(mesh_copy);