diff options
author | Bastien Montagne <bastien@blender.org> | 2022-06-03 16:35:49 +0300 |
---|---|---|
committer | Bastien Montagne <bastien@blender.org> | 2022-06-03 17:07:05 +0300 |
commit | e73fd4f0c03f9584eb3ac4aa3d1c07c83ddec759 (patch) | |
tree | 1147331cb083a623b396a1823f8338f4a76dbd0e /source | |
parent | 691ab3fc46a3225560c3e4c5d46c7bb2fd413391 (diff) |
Fix (unreported) important memory leak in Boolean modifier using a Collection operand and Fast mode.
Code handling repetitive boolean operations when using several objects
from a Collection would not handle result mesh properly, re-creating for
each object without properly freeing it.
Further more, existing code was effectively converting the BMesh to mesh
twice, including a modification of the initial (input) mesh, which
modifiers should never do!
Removed the extra useless conversion, which also gives a small
improvement in performances:
With as simple of a scene as four objects (three operands in a
collection, and the modified one) totalling 20k vertices/faces, this
commit:
* Avoids 2MB memory leak per evaluation (!).
* Speeds up boolean evaluation by 5-10%.
Found while investigating some production files of the Project Heist
here at the Blender Studio.
Diffstat (limited to 'source')
-rw-r--r-- | source/blender/modifiers/intern/MOD_boolean.cc | 29 |
1 files changed, 17 insertions, 12 deletions
diff --git a/source/blender/modifiers/intern/MOD_boolean.cc b/source/blender/modifiers/intern/MOD_boolean.cc index 7581df19996..07504d91fea 100644 --- a/source/blender/modifiers/intern/MOD_boolean.cc +++ b/source/blender/modifiers/intern/MOD_boolean.cc @@ -518,24 +518,29 @@ static Mesh *modifyMesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh * Mesh *mesh_operand_ob = BKE_modifier_get_evaluated_mesh_from_evaluated_object(operand_ob, false); - if (mesh_operand_ob) { - /* XXX This is utterly non-optimal, we may go from a bmesh to a mesh back to a bmesh! - * But for 2.90 better not try to be smart here. */ - BKE_mesh_wrapper_ensure_mdata(mesh_operand_ob); + if (mesh_operand_ob == nullptr) { + continue; + } - bool is_flip; - BMesh *bm = BMD_mesh_bm_create(mesh, object, mesh_operand_ob, operand_ob, &is_flip); + /* XXX This is utterly non-optimal, we may go from a bmesh to a mesh back to a bmesh! + * But for 2.90 better not try to be smart here. */ + BKE_mesh_wrapper_ensure_mdata(mesh_operand_ob); - BMD_mesh_intersection(bm, md, ctx, mesh_operand_ob, object, operand_ob, is_flip); + bool is_flip; + BMesh *bm = BMD_mesh_bm_create(result, object, mesh_operand_ob, operand_ob, &is_flip); - /* Needed for multiple objects to work. */ - BMeshToMeshParams bmesh_to_mesh_params{}; - bmesh_to_mesh_params.calc_object_remap = false; - BM_mesh_bm_to_me(nullptr, bm, mesh, &bmesh_to_mesh_params); + BMD_mesh_intersection(bm, md, ctx, mesh_operand_ob, object, operand_ob, is_flip); + /* Needed for multiple objects to work. */ + if (result == mesh) { result = BKE_mesh_from_bmesh_for_eval_nomain(bm, nullptr, mesh); - BM_mesh_free(bm); } + else { + BMeshToMeshParams bmesh_to_mesh_params{}; + bmesh_to_mesh_params.calc_object_remap = false; + BM_mesh_bm_to_me(nullptr, bm, result, &bmesh_to_mesh_params); + } + BM_mesh_free(bm); } } FOREACH_COLLECTION_OBJECT_RECURSIVE_END; |