Cleanup/refactor binding code for MeshDeform modifier.
authorBastien Montagne <montagne29@wanadoo.fr>
Fri, 7 Dec 2018 10:17:25 +0000 (11:17 +0100)
committerBastien Montagne <montagne29@wanadoo.fr>
Fri, 7 Dec 2018 10:22:31 +0000 (11:22 +0100)
We had two different ways of doing it, SurfaceDeform and LaplacianDeform
would do it through a special modifier stack evaluation triggered from
binding operator, while MeshDeform would do it through a regular
depsgraph update/eval (also triggered from its binding op).

This enforces the later to search back for orig modifier data inside
modifier code (to apply binding on that one, and not on useless CoW
one).

Besides the question of safety about modifying orig data from threaded
despgraph (that was *probably* OK, but think it's bad idea in general),
it's much better to have a common way of doing that kind of things.

For now it remains rather dodgy, but at least it's reasonably consistent
and safe now.

This commit also fixes a potential memleak from binding process of
MeshDeform, and does some general cleanup a bit.

source/blender/editors/armature/meshlaplacian.c
source/blender/editors/include/ED_armature.h
source/blender/editors/object/object_modifier.c
source/blender/makesdna/DNA_modifier_types.h
source/blender/modifiers/intern/MOD_meshdeform.c
source/blender/modifiers/intern/MOD_surfacedeform.c

index 001c8ce215feda168f3f3dcea862c5dec8cec811..96799304845a3b8c59885e607c85905e5dc13869 100644 (file)
@@ -1427,7 +1427,7 @@ static void meshdeform_matrix_solve(MeshDeformModifierData *mmd, MeshDeformBind
        EIG_linear_solver_delete(context);
 }
 
-static void harmonic_coordinates_bind(Scene *UNUSED(scene), MeshDeformModifierData *mmd, MeshDeformBind *mdb)
+static void harmonic_coordinates_bind(MeshDeformModifierData *mmd, MeshDeformBind *mdb)
 {
        MDefBindInfluence *inf;
        MDefInfluence *mdinf;
@@ -1578,7 +1578,7 @@ static void harmonic_coordinates_bind(Scene *UNUSED(scene), MeshDeformModifierDa
 }
 
 void ED_mesh_deform_bind_callback(
-        Scene *scene, MeshDeformModifierData *mmd, Mesh *cagemesh,
+        MeshDeformModifierData *mmd, Mesh *cagemesh,
         float *vertexcos, int totvert, float cagemat[4][4])
 {
        MeshDeformBind mdb;
@@ -1606,7 +1606,7 @@ void ED_mesh_deform_bind_callback(
                mul_v3_m4v3(mdb.vertexcos[a], mdb.cagemat, vertexcos + a * 3);
 
        /* solve */
-       harmonic_coordinates_bind(scene, mmd, &mdb);
+       harmonic_coordinates_bind(mmd, &mdb);
 
        /* assign bind variables */
        mmd->bindcagecos = (float *)mdb.cagecos;
index d5698ecadf7208e750c11062ade919a25e826ca1..3131e5221d8bd0926648f4494adacca6093386e7 100644 (file)
@@ -238,7 +238,6 @@ struct Object *ED_pose_object_from_context(struct bContext *C);
 
 /* meshlaplacian.c */
 void ED_mesh_deform_bind_callback(
-        struct Scene *scene,
         struct MeshDeformModifierData *mmd,
         struct Mesh *cagemesh,
         float *vertexcos, int totvert, float cagemat[4][4]);
index 7b4fba235518cfda75fd240649fbe553f4b802b3..200ff1ab0d66653ac88fefee7820c51d99b11224 100644 (file)
@@ -1944,52 +1944,41 @@ static bool meshdeform_poll(bContext *C)
 
 static int meshdeform_bind_exec(bContext *C, wmOperator *op)
 {
-       Main *bmain = CTX_data_main(C);
        Depsgraph *depsgraph = CTX_data_depsgraph(C);
+       Scene *scene = CTX_data_scene(C);
        Object *ob = ED_object_active_context(C);
        MeshDeformModifierData *mmd = (MeshDeformModifierData *)edit_modifier_property_get(op, ob, eModifierType_MeshDeform);
 
-       if (!mmd)
+       if (mmd == NULL) {
                return OPERATOR_CANCELLED;
+       }
 
-       if (mmd->bindcagecos) {
-               MEM_freeN(mmd->bindcagecos);
-               if (mmd->dyngrid) MEM_freeN(mmd->dyngrid);
-               if (mmd->dyninfluences) MEM_freeN(mmd->dyninfluences);
-               if (mmd->bindinfluences) MEM_freeN(mmd->bindinfluences);
-               if (mmd->bindoffsets) MEM_freeN(mmd->bindoffsets);
-               if (mmd->dynverts) MEM_freeN(mmd->dynverts);
-               if (mmd->bindweights) MEM_freeN(mmd->bindweights);  /* deprecated */
-               if (mmd->bindcos) MEM_freeN(mmd->bindcos);  /* deprecated */
-
-               mmd->bindcagecos = NULL;
-               mmd->dyngrid = NULL;
-               mmd->dyninfluences = NULL;
-               mmd->bindinfluences = NULL;
-               mmd->bindoffsets = NULL;
-               mmd->dynverts = NULL;
-               mmd->bindweights = NULL; /* deprecated */
-               mmd->bindcos = NULL; /* deprecated */
+       if (mmd->bindcagecos != NULL) {
+               MEM_SAFE_FREE(mmd->bindcagecos);
+               MEM_SAFE_FREE(mmd->dyngrid);
+               MEM_SAFE_FREE(mmd->dyninfluences);
+               MEM_SAFE_FREE(mmd->bindinfluences);
+               MEM_SAFE_FREE(mmd->bindoffsets);
+               MEM_SAFE_FREE(mmd->dynverts);
+               MEM_SAFE_FREE(mmd->bindweights);  /* Deprecated */
+               MEM_SAFE_FREE(mmd->bindcos);  /* Deprecated */
                mmd->totvert = 0;
                mmd->totcagevert = 0;
                mmd->totinfluence = 0;
        }
        else {
-               int mode = mmd->modifier.mode;
-               mmd->bindfunc = ED_mesh_deform_bind_callback;
+               /* Force modifier to run, it will call binding routine (this has to happen outside of depsgraph evaluation). */
+               const int mode = mmd->modifier.mode;
                mmd->modifier.mode |= eModifierMode_Realtime;
-
-               /* Force depsgraph update, this will do binding. */
-               DEG_id_tag_update(&ob->id, OB_RECALC_DATA);
-               BKE_scene_graph_update_tagged(depsgraph, bmain);
-
-               mmd->bindfunc = NULL;
+               mmd->bindfunc = ED_mesh_deform_bind_callback;
+               object_force_modifier_update_for_bind(depsgraph, scene, ob);
                mmd->modifier.mode = mode;
+               mmd->bindfunc = NULL;
        }
 
-       DEG_id_tag_update(&ob->id, OB_RECALC_DATA);
+       /* We need DEG_TAG_COPY_ON_WRITE to ensure (un)binding is flushed to CoW copies of the object... */
+       DEG_id_tag_update(&ob->id, DEG_TAG_GEOMETRY | DEG_TAG_COPY_ON_WRITE);
        WM_event_add_notifier(C, NC_OBJECT | ND_MODIFIER, ob);
-
        return OPERATOR_FINISHED;
 }
 
index 1d8df99af7a063737cea8f27ed1b82a331e52c0b..345958ce3974c5b391755e812b46692130989d0d 100644 (file)
@@ -770,7 +770,7 @@ typedef struct MeshDeformModifierData {
        float *bindcos;                 /* deprecated storage of cage coords */
 
        /* runtime */
-       void (*bindfunc)(struct Scene *scene, struct MeshDeformModifierData *mmd, struct Mesh *cagemesh,
+       void (*bindfunc)(struct MeshDeformModifierData *mmd, struct Mesh *cagemesh,
                         float *vertexcos, int totvert, float cagemat[4][4]);
 } MeshDeformModifierData;
 
index 9af76916788755910caf711507bc72964eb33254..d3450e61709aed560997ab0d45a32a7ce89d76b3 100644 (file)
@@ -45,6 +45,7 @@
 #include "BKE_library.h"
 #include "BKE_library_query.h"
 #include "BKE_mesh.h"
+#include "BKE_mesh_runtime.h"
 #include "BKE_modifier.h"
 #include "BKE_deform.h"
 #include "BKE_editmesh.h"
@@ -286,12 +287,14 @@ static void meshdeformModifier_do(
        Mesh *cagemesh;
        MDeformVert *dvert = NULL;
        float imat[4][4], cagemat[4][4], iobmat[4][4], icagemat[3][3], cmat[4][4];
-       float co[3], (*dco)[3], (*bindcagecos)[3];
+       float co[3], (*dco)[3] = NULL, (*bindcagecos)[3];
        int a, totvert, totcagevert, defgrp_index;
-       float (*cagecos)[3];
+       float (*cagecos)[3] = NULL;
        MeshdeformUserdata data;
        bool free_cagemesh = false;
 
+       static int recursive_bind_sentinel = 0;
+
        if (!mmd->object || (!mmd->bindcagecos && !mmd->bindfunc))
                return;
 
@@ -306,6 +309,12 @@ static void meshdeformModifier_do(
         * We'll support this case once granular dependency graph is landed.
         */
        cagemesh = BKE_modifier_get_evaluated_mesh_from_evaluated_object(mmd->object, &free_cagemesh);
+       if (cagemesh == NULL && mmd->bindcagecos == NULL && ob == DEG_get_original_object(ob)) {
+               /* Special case, binding happens outside of depsgraph evaluation, so we can build our own
+                * target mesh if needed. */
+               cagemesh = mesh_create_eval_final_view(ctx->depsgraph, DEG_get_input_scene(ctx->depsgraph), mmd->object, 0);
+               free_cagemesh = cagemesh != NULL;
+       }
 
        if (cagemesh == NULL) {
                modifier_setError(md, "Cannot get mesh from cage object");
@@ -321,22 +330,20 @@ static void meshdeformModifier_do(
 
        /* bind weights if needed */
        if (!mmd->bindcagecos) {
-               static int recursive = 0;
-
                /* progress bar redraw can make this recursive .. */
-               if (!recursive) {
-                       /* Write binding data to original modifier. */
-                       Scene *scene = DEG_get_evaluated_scene(ctx->depsgraph);
-                       Object *ob_orig = DEG_get_original_object(ob);
-                       MeshDeformModifierData *mmd_orig = (MeshDeformModifierData *)modifiers_findByName(
-                               ob_orig, mmd->modifier.name);
-
-                       recursive = 1;
-                       mmd->bindfunc(scene, mmd_orig, cagemesh, (float *)vertexCos, numVerts, cagemat);
-                       recursive = 0;
+               if (!recursive_bind_sentinel) {
+                       if (ob != DEG_get_original_object(ob)) {
+                               BLI_assert(!"Trying to bind inside of depsgraph evaluation");
+                               modifier_setError(md, "Trying to bind inside of depsgraph evaluation");
+                               goto finally;
+                       }
+
+                       recursive_bind_sentinel = 1;
+                       mmd->bindfunc(mmd, cagemesh, (float *)vertexCos, numVerts, cagemat);
+                       recursive_bind_sentinel = 0;
                }
 
-               return;
+               goto finally;
        }
 
        /* verify we have compatible weights */
@@ -345,18 +352,15 @@ static void meshdeformModifier_do(
 
        if (mmd->totvert != totvert) {
                modifier_setError(md, "Verts changed from %d to %d", mmd->totvert, totvert);
-               if (free_cagemesh) BKE_id_free(NULL, cagemesh);
-               return;
+               goto finally;
        }
        else if (mmd->totcagevert != totcagevert) {
                modifier_setError(md, "Cage verts changed from %d to %d", mmd->totcagevert, totcagevert);
-               if (free_cagemesh) BKE_id_free(NULL, cagemesh);
-               return;
+               goto finally;
        }
        else if (mmd->bindcagecos == NULL) {
                modifier_setError(md, "Bind data missing");
-               if (free_cagemesh) BKE_id_free(NULL, cagemesh);
-               return;
+               goto finally;
        }
 
        /* setup deformation data */
@@ -377,8 +381,9 @@ static void meshdeformModifier_do(
                        /* compute difference with world space bind coord */
                        sub_v3_v3v3(dco[a], co, bindcagecos[a]);
                }
-               else
+               else {
                        copy_v3_v3(dco[a], co);
+               }
        }
 
        MOD_get_vgroup(ob, mesh, mmd->defgrp_name, &dvert, &defgrp_index);
@@ -401,9 +406,9 @@ static void meshdeformModifier_do(
                                meshdeform_vert_task,
                                &settings);
 
-       /* release cage mesh */
-       MEM_freeN(dco);
-       MEM_freeN(cagecos);
+finally:
+       MEM_SAFE_FREE(dco);
+       MEM_SAFE_FREE(cagecos);
        if (cagemesh != NULL && free_cagemesh) {
                BKE_id_free(NULL, cagemesh);
        }
index 5a2b54824cced721941715b775266d69db8aa774..6c6cbe3a538153d4dc63ad0bfe68513b860f30ac 100644 (file)
@@ -1126,7 +1126,7 @@ static void surfacedeformModifier_do(
        }
 
        target = BKE_modifier_get_evaluated_mesh_from_evaluated_object(smd->target, &free_target);
-       if (!target && smd->verts == NULL && ob == DEG_get_original_object(ob)) {
+       if (target == NULL && smd->verts == NULL && ob == DEG_get_original_object(ob)) {
                /* Special case, binding happens outside of depsgraph evaluation, so we can build our own
                 * target mesh if needed. */
                target = mesh_create_eval_final_view(ctx->depsgraph, DEG_get_input_scene(ctx->depsgraph), smd->target, 0);