[Bf-blender-cvs] [e73fd4f0c03] master: Fix (unreported) important memory leak in Boolean modifier using a Collection operand and Fast mode.

Bastien Montagne noreply at git.blender.org
Fri Jun 3 16:08:38 CEST 2022


Commit: e73fd4f0c03f9584eb3ac4aa3d1c07c83ddec759
Author: Bastien Montagne
Date:   Fri Jun 3 15:35:49 2022 +0200
Branches: master
https://developer.blender.org/rBe73fd4f0c03f9584eb3ac4aa3d1c07c83ddec759

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.

===================================================================

M	source/blender/modifiers/intern/MOD_boolean.cc

===================================================================

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;



More information about the Bf-blender-cvs mailing list