From b8d4a2aff8069dd7d6fb91ad0d9427eed489b68f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 8 Sep 2020 10:04:41 +0200 Subject: 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. --- source/blender/editors/object/object_relations.c | 316 ++++++++++++----------- 1 file changed, 165 insertions(+), 151 deletions(-) (limited to 'source/blender/editors/object') 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_GROUPS_NAME, false); } - else if (pararm && (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_GROUPS_NAME, false); - } - else if (partype == PAR_ARMATURE_ENVELOPE) { - ED_object_vgroup_calc_from_armature( - reports, depsgraph, scene, ob, par, ARM_GROUPS_ENVELOPE, xmirror); - } - else if (partype == PAR_ARMATURE_AUTO) { - WM_cursor_wait(1); - ED_object_vgroup_calc_from_armature( - reports, depsgraph, scene, ob, par, ARM_GROUPS_AUTO, xmirror); - WM_cursor_wait(0); - } - /* get corrected inverse */ - ob->partype = PAROBJECT; - BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob); - - invert_m4_m4(ob->parentinv, workob.obmat); + else if (partype == PAR_ARMATURE_ENVELOPE) { + ED_object_vgroup_calc_from_armature( + reports, depsgraph, scene, ob, par, ARM_GROUPS_ENVELOPE, xmirror); } - else if (pararm && (ob->type == OB_GPENCIL) && (par->type == OB_ARMATURE)) { - if (partype == PAR_ARMATURE) { - ED_gpencil_add_armature(C, reports, ob, par); - } - else if (partype == PAR_ARMATURE_NAME) { - ED_gpencil_add_armature_weights(C, reports, ob, par, GP_PAR_ARMATURE_NAME); - } - else if ((partype == PAR_ARMATURE_AUTO) || (partype == PAR_ARMATURE_ENVELOPE)) { - WM_cursor_wait(1); - ED_gpencil_add_armature_weights(C, reports, ob, par, GP_PAR_ARMATURE_AUTO); - WM_cursor_wait(0); - } - /* get corrected inverse */ - ob->partype = PAROBJECT; - BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob); - - invert_m4_m4(ob->parentinv, workob.obmat); + else if (partype == PAR_ARMATURE_AUTO) { + WM_cursor_wait(1); + ED_object_vgroup_calc_from_armature( + reports, depsgraph, scene, ob, par, ARM_GROUPS_AUTO, xmirror); + WM_cursor_wait(0); } - else if ((ob->type == OB_GPENCIL) && (par->type == OB_LATTICE)) { - /* Add Lattice modifier */ - if (partype == PAR_LATTICE) { - ED_gpencil_add_lattice_modifier(C, reports, ob, par); - } - /* get corrected inverse */ - ob->partype = PAROBJECT; - BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob); + /* get corrected inverse */ + ob->partype = PAROBJECT; + BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob); - invert_m4_m4(ob->parentinv, workob.obmat); + invert_m4_m4(ob->parentinv, workob.obmat); + } + else if (is_armature_parent && (ob->type == OB_GPENCIL) && (par->type == OB_ARMATURE)) { + if (partype == PAR_ARMATURE) { + ED_gpencil_add_armature(C, reports, ob, par); } - else { - /* calculate inverse parent matrix */ - BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob); - invert_m4_m4(ob->parentinv, workob.obmat); + else if (partype == PAR_ARMATURE_NAME) { + ED_gpencil_add_armature_weights(C, reports, ob, par, GP_PAR_ARMATURE_NAME); + } + else if ((partype == PAR_ARMATURE_AUTO) || (partype == PAR_ARMATURE_ENVELOPE)) { + WM_cursor_wait(1); + ED_gpencil_add_armature_weights(C, reports, ob, par, GP_PAR_ARMATURE_AUTO); + WM_cursor_wait(0); } + /* get corrected inverse */ + ob->partype = PAROBJECT; + BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob); - DEG_id_tag_update(&ob->id, ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY); + invert_m4_m4(ob->parentinv, workob.obmat); + } + else if ((ob->type == OB_GPENCIL) && (par->type == OB_LATTICE)) { + /* Add Lattice modifier */ + if (partype == PAR_LATTICE) { + ED_gpencil_add_lattice_modifier(C, reports, ob, par); + } + /* get corrected inverse */ + ob->partype = PAROBJECT; + BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob); + + invert_m4_m4(ob->parentinv, workob.obmat); + } + else { + /* calculate inverse parent matrix */ + BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob); + invert_m4_m4(ob->parentinv, workob.obmat); } + DEG_id_tag_update(&ob->id, ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY); return true; } -- cgit v1.2.3