[Bf-blender-cvs] [b8d4a2aff80] master: Cleanup: Refactor `ED_object_parent_set`

Sybren A. Stüvel noreply at git.blender.org
Tue Sep 8 11:39:38 CEST 2020


Commit: b8d4a2aff8069dd7d6fb91ad0d9427eed489b68f
Author: Sybren A. Stüvel
Date:   Tue Sep 8 10:04:41 2020 +0200
Branches: master
https://developer.blender.org/rBb8d4a2aff8069dd7d6fb91ad0d9427eed489b68f

Cleanup: Refactor `ED_object_parent_set`

Refactor `ED_object_parent_set`:
- Mark parameters `ob` and `par` as `const` so that it's clear the
  function doesn't assign any other value to them.
- Rename `pararm` to `is_armature_parent`; I mis-read it as `param` all
  the time, and it was very confusing.
- Replace repeated `if-else` statements with `switch` statements.
- Reorder preconditions to have some simple checks first.
- Flip condition on a huge `if`-statement to return early and unindent
  the remainder of the function.

This function still requires splitting up into smaller functions, but at
least this is a step forward.

No functional changes.

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

M	source/blender/editors/object/object_relations.c

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

diff --git a/source/blender/editors/object/object_relations.c b/source/blender/editors/object/object_relations.c
index fd4a7ae1d4a..4adf1a8d9f2 100644
--- a/source/blender/editors/object/object_relations.c
+++ b/source/blender/editors/object/object_relations.c
@@ -678,8 +678,8 @@ EnumPropertyItem prop_make_parent_types[] = {
 bool ED_object_parent_set(ReportList *reports,
                           const bContext *C,
                           Scene *scene,
-                          Object *ob,
-                          Object *par,
+                          Object *const ob,
+                          Object *const par,
                           int partype,
                           const bool xmirror,
                           const bool keep_transform,
@@ -689,99 +689,111 @@ bool ED_object_parent_set(ReportList *reports,
   Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(C);
   bPoseChannel *pchan = NULL;
   bPoseChannel *pchan_eval = NULL;
-  const bool pararm = ELEM(
-      partype, PAR_ARMATURE, PAR_ARMATURE_NAME, PAR_ARMATURE_ENVELOPE, PAR_ARMATURE_AUTO);
   Object *parent_eval = DEG_get_evaluated_object(depsgraph, par);
 
   DEG_id_tag_update(&par->id, ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY);
 
-  /* preconditions */
-  if (partype == PAR_FOLLOW || partype == PAR_PATH_CONST) {
-    if (par->type != OB_CURVE) {
-      return 0;
-    }
-    Curve *cu = par->data;
-    Curve *cu_eval = parent_eval->data;
-    if ((cu->flag & CU_PATH) == 0) {
-      cu->flag |= CU_PATH | CU_FOLLOW;
-      cu_eval->flag |= CU_PATH | CU_FOLLOW;
-      /* force creation of path data */
-      BKE_displist_make_curveTypes(depsgraph, scene, par, false, false);
-    }
-    else {
-      cu->flag |= CU_FOLLOW;
-      cu_eval->flag |= CU_FOLLOW;
-    }
+  /* Preconditions. */
+  if (ob == par) {
+    /* Parenting an object to itself is impossible. */
+    return true;
+  }
 
-    /* if follow, add F-Curve for ctime (i.e. "eval_time") so that path-follow works */
-    if (partype == PAR_FOLLOW) {
-      /* get or create F-Curve */
-      bAction *act = ED_id_action_ensure(bmain, &cu->id);
-      FCurve *fcu = ED_action_fcurve_ensure(bmain, act, NULL, NULL, "eval_time", 0);
+  if (BKE_object_parent_loop_check(par, ob)) {
+    BKE_report(reports, RPT_ERROR, "Loop in parents");
+    return false;
+  }
 
-      /* setup dummy 'generator' modifier here to get 1-1 correspondence still working */
-      if (!fcu->bezt && !fcu->fpt && !fcu->modifiers.first) {
-        add_fmodifier(&fcu->modifiers, FMODIFIER_TYPE_GENERATOR, fcu);
+  switch (partype) {
+    case PAR_FOLLOW:
+    case PAR_PATH_CONST: {
+      if (par->type != OB_CURVE) {
+        return false;
+      }
+      Curve *cu = par->data;
+      Curve *cu_eval = parent_eval->data;
+      if ((cu->flag & CU_PATH) == 0) {
+        cu->flag |= CU_PATH | CU_FOLLOW;
+        cu_eval->flag |= CU_PATH | CU_FOLLOW;
+        /* force creation of path data */
+        BKE_displist_make_curveTypes(depsgraph, scene, par, false, false);
+      }
+      else {
+        cu->flag |= CU_FOLLOW;
+        cu_eval->flag |= CU_FOLLOW;
       }
-    }
 
-    /* fall back on regular parenting now (for follow only) */
-    if (partype == PAR_FOLLOW) {
-      partype = PAR_OBJECT;
-    }
-  }
-  else if (ELEM(partype, PAR_BONE, PAR_BONE_RELATIVE)) {
-    pchan = BKE_pose_channel_active(par);
-    pchan_eval = BKE_pose_channel_active(parent_eval);
+      /* if follow, add F-Curve for ctime (i.e. "eval_time") so that path-follow works */
+      if (partype == PAR_FOLLOW) {
+        /* get or create F-Curve */
+        bAction *act = ED_id_action_ensure(bmain, &cu->id);
+        FCurve *fcu = ED_action_fcurve_ensure(bmain, act, NULL, NULL, "eval_time", 0);
 
-    if (pchan == NULL) {
-      BKE_report(reports, RPT_ERROR, "No active bone");
-      return false;
-    }
-  }
+        /* setup dummy 'generator' modifier here to get 1-1 correspondence still working */
+        if (!fcu->bezt && !fcu->fpt && !fcu->modifiers.first) {
+          add_fmodifier(&fcu->modifiers, FMODIFIER_TYPE_GENERATOR, fcu);
+        }
+      }
 
-  if (ob != par) {
-    if (BKE_object_parent_loop_check(par, ob)) {
-      BKE_report(reports, RPT_ERROR, "Loop in parents");
-      return false;
+      /* fall back on regular parenting now (for follow only) */
+      if (partype == PAR_FOLLOW) {
+        partype = PAR_OBJECT;
+      }
+      break;
     }
+    case PAR_BONE:
+    case PAR_BONE_RELATIVE:
+      pchan = BKE_pose_channel_active(par);
+      pchan_eval = BKE_pose_channel_active(parent_eval);
 
-    Object workob;
+      if (pchan == NULL) {
+        BKE_report(reports, RPT_ERROR, "No active bone");
+        return false;
+      }
+  }
 
-    /* apply transformation of previous parenting */
-    if (keep_transform) {
-      /* was removed because of bug [#23577],
-       * but this can be handy in some cases too [#32616], so make optional */
-      BKE_object_apply_mat4(ob, ob->obmat, false, false);
-    }
+  Object workob;
 
-    /* set the parent (except for follow-path constraint option) */
-    if (partype != PAR_PATH_CONST) {
-      ob->parent = par;
-      /* Always clear parentinv matrix for sake of consistency, see T41950. */
-      unit_m4(ob->parentinv);
-      DEG_id_tag_update(&ob->id, ID_RECALC_TRANSFORM);
-    }
+  /* Apply transformation of previous parenting. */
+  if (keep_transform) {
+    /* was removed because of bug [#23577],
+     * but this can be handy in some cases too [#32616], so make optional */
+    BKE_object_apply_mat4(ob, ob->obmat, false, false);
+  }
 
-    /* handle types */
-    if (pchan) {
-      BLI_strncpy(ob->parsubstr, pchan->name, sizeof(ob->parsubstr));
-    }
-    else {
-      ob->parsubstr[0] = 0;
-    }
+  /* Set the parent (except for follow-path constraint option). */
+  if (partype != PAR_PATH_CONST) {
+    ob->parent = par;
+    /* Always clear parentinv matrix for sake of consistency, see T41950. */
+    unit_m4(ob->parentinv);
+    DEG_id_tag_update(&ob->id, ID_RECALC_TRANSFORM);
+  }
 
-    if (partype == PAR_PATH_CONST) {
-      /* don't do anything here, since this is not technically "parenting" */
-    }
-    else if (ELEM(partype, PAR_CURVE, PAR_LATTICE) || (pararm)) {
+  /* Handle types. */
+  if (pchan) {
+    BLI_strncpy(ob->parsubstr, pchan->name, sizeof(ob->parsubstr));
+  }
+  else {
+    ob->parsubstr[0] = 0;
+  }
+
+  switch (partype) {
+    case PAR_PATH_CONST:
+      /* Don't do anything here, since this is not technically "parenting". */
+      break;
+    case PAR_CURVE:
+    case PAR_LATTICE:
+    case PAR_ARMATURE:
+    case PAR_ARMATURE_NAME:
+    case PAR_ARMATURE_ENVELOPE:
+    case PAR_ARMATURE_AUTO:
       /* partype is now set to PAROBJECT so that invisible 'virtual'
        * modifiers don't need to be created.
        * NOTE: the old (2.4x) method was to set ob->partype = PARSKEL,
        * creating the virtual modifiers.
        */
-      ob->partype = PAROBJECT;     /* note, dna define, not operator property */
-      /* ob->partype = PARSKEL; */ /* note, dna define, not operator property */
+      ob->partype = PAROBJECT;     /* Note: DNA define, not operator property. */
+      /* ob->partype = PARSKEL; */ /* Note: DNA define, not operator property. */
 
       /* BUT, to keep the deforms, we need a modifier,
        * and then we need to set the object that it uses
@@ -823,109 +835,111 @@ bool ED_object_parent_set(ReportList *reports,
             break;
         }
       }
-    }
-    else if (partype == PAR_BONE) {
-      ob->partype = PARBONE; /* note, dna define, not operator property */
+      break;
+    case PAR_BONE:
+      ob->partype = PARBONE; /* Note: DNA define, not operator property. */
       if (pchan->bone) {
         pchan->bone->flag &= ~BONE_RELATIVE_PARENTING;
         pchan_eval->bone->flag &= ~BONE_RELATIVE_PARENTING;
       }
-    }
-    else if (partype == PAR_BONE_RELATIVE) {
-      ob->partype = PARBONE; /* note, dna define, not operator property */
+      break;
+    case PAR_BONE_RELATIVE:
+      ob->partype = PARBONE; /* Note: DNA define, not operator property. */
       if (pchan->bone) {
         pchan->bone->flag |= BONE_RELATIVE_PARENTING;
         pchan_eval->bone->flag |= BONE_RELATIVE_PARENTING;
       }
-    }
-    else if (partype == PAR_VERTEX) {
+      break;
+    case PAR_VERTEX:
       ob->partype = PARVERT1;
       ob->par1 = vert_par[0];
-    }
-    else if (partype == PAR_VERTEX_TRI) {
+      break;
+    case PAR_VERTEX_TRI:
       ob->partype = PARVERT3;
       copy_v3_v3_int(&ob->par1, vert_par);
-    }
-    else {
-      ob->partype = PAROBJECT; /* note, dna define, not operator property */
-    }
+      break;
+    case PAR_OBJECT:
+    case PAR_FOLLOW:
+      ob->partype = PAROBJECT; /* Note: DNA define, not operator property. */
+      break;
+  }
 
-    /* constraint */
-    if (partype == PAR_PATH_CONST) {
-      bConstraint *con;
-      bFollowPathConstraint *data;
-      float cmat[4][4], vec[3];
+  /* Constraint and set parent inverse. */
+  const bool is_armature_parent = ELEM(
+      partype, PAR_ARMATURE, PAR_ARMATURE_NAME, PAR_ARMATURE_ENVELOPE, PAR_ARMATURE_AUTO);
+  if (partype == PAR_PATH_CONST) {
+    bConstraint *con;
+    bFollowPathConstraint *data;
+    float cmat[4][4], vec[3];
 
-      con = BKE_constraint_add_for_object(ob, "AutoPath", CONSTRAINT_TYPE_FOLLOWPATH);
+    con = BKE_constraint_add_for_object(ob, "AutoPath", CONSTRAINT_TYPE_FOLLOWPATH);
 
-      data = con->data;
-      data->tar = par;
+    data = con->data;
+    data->tar = par;
 
-      BKE_constraint_target_matrix_get(
-          depsgraph, scene, con, 0, CONSTRAINT_OBTYPE_OBJECT, NULL, cmat, scene->r.cfra);
-      sub_v3_v3v3(vec, ob->obmat[3], cmat[3]);
+    BKE_constraint_target_matrix_get(
+        depsgraph, scene, con, 0, CONSTRAINT_OBTYPE_OBJECT, NULL, cmat, scene->r.cfra);
+    sub_v3_v3v3(vec, ob->obmat[3], cmat[3]);
 
-      copy_v3_v3(ob->loc, vec);
+    copy_v3_v3(ob->loc, vec);
+  }
+  else if (is_armature_parent && (ob->type == OB_MESH) && (par->type == OB_ARMATURE)) {
+    if (partype == PAR_ARMATURE_NAME) {
+      ED_object_vgroup_calc_from_armature(
+          reports, depsgraph, scene, ob, par, ARM_GRO

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list