Changeset View
Standalone View
source/blender/editors/armature/armature_add.c
| Show All 16 Lines | |||||
| * All rights reserved. | * All rights reserved. | ||||
| * Operators and API's for creating bones | * Operators and API's for creating bones | ||||
| */ | */ | ||||
| /** \file | /** \file | ||||
| * \ingroup edarmature | * \ingroup edarmature | ||||
| */ | */ | ||||
| #include "DNA_anim_types.h" | |||||
| #include "DNA_armature_types.h" | #include "DNA_armature_types.h" | ||||
| #include "DNA_constraint_types.h" | #include "DNA_constraint_types.h" | ||||
| #include "DNA_object_types.h" | #include "DNA_object_types.h" | ||||
| #include "DNA_scene_types.h" | #include "DNA_scene_types.h" | ||||
| #include "MEM_guardedalloc.h" | #include "MEM_guardedalloc.h" | ||||
| #include "BLI_blenlib.h" | #include "BLI_blenlib.h" | ||||
| #include "BLI_math.h" | #include "BLI_math.h" | ||||
| #include "BLI_ghash.h" | #include "BLI_ghash.h" | ||||
| #include "BLI_string_utils.h" | #include "BLI_string_utils.h" | ||||
| #include "BKE_action.h" | #include "BKE_action.h" | ||||
| #include "BKE_constraint.h" | #include "BKE_constraint.h" | ||||
| #include "BKE_context.h" | #include "BKE_context.h" | ||||
| #include "BKE_fcurve.h" | |||||
| #include "BKE_idprop.h" | #include "BKE_idprop.h" | ||||
| #include "BKE_deform.h" | #include "BKE_deform.h" | ||||
| #include "BKE_layer.h" | #include "BKE_layer.h" | ||||
| #include "BKE_library.h" | |||||
| #include "BKE_main.h" | |||||
| #include "DEG_depsgraph.h" | |||||
| #include "RNA_access.h" | #include "RNA_access.h" | ||||
| #include "RNA_define.h" | #include "RNA_define.h" | ||||
| #include "WM_api.h" | #include "WM_api.h" | ||||
| #include "WM_types.h" | #include "WM_types.h" | ||||
| #include "ED_armature.h" | #include "ED_armature.h" | ||||
| ▲ Show 20 Lines • Show All 315 Lines • ▼ Show 20 Lines | if (ebone_dst) { | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| BLI_ghash_free(name_map, NULL, NULL); | BLI_ghash_free(name_map, NULL, NULL); | ||||
| } | } | ||||
| /* | /* | ||||
sybren: There used to be a comment here with a bit of explanation of what parameter is what. I think… | |||||
Done Inline ActionsThe previous comment here was clarify what src_ob and dst_ob was used for. Now this function simply does what the name suggests, it updates the bone duplicate constraint sub-target. /* add 'a' and 'b' together and return the result */ float add(float a, float b) zeddb: The previous comment here was clarify what `src_ob` and `dst_ob` was used for.
Now this… | |||||
| * Note: When duplicating cross objects, editbones here is the list of bones | * Note: When duplicating cross objects, editbones here is the list of bones | ||||
| * from the SOURCE object but ob is the DESTINATION object | * from the SOURCE object but ob is the DESTINATION object | ||||
| * */ | * */ | ||||
| void updateDuplicateSubtargetObjects(EditBone *dupBone, | void updateDuplicateSubtargetObjects(EditBone *dupBone, | ||||
| ListBase *editbones, | ListBase *editbones, | ||||
| Object *src_ob, | Object *src_ob, | ||||
| Object *dst_ob) | Object *dst_ob) | ||||
| { | { | ||||
| ▲ Show 20 Lines • Show All 45 Lines • ▼ Show 20 Lines | void updateDuplicateSubtargetObjects(EditBone *dupBone, | ||||
| } | } | ||||
| } | } | ||||
| void updateDuplicateSubtarget(EditBone *dupBone, ListBase *editbones, Object *ob) | void updateDuplicateSubtarget(EditBone *dupBone, ListBase *editbones, Object *ob) | ||||
| { | { | ||||
| updateDuplicateSubtargetObjects(dupBone, editbones, ob, ob); | updateDuplicateSubtargetObjects(dupBone, editbones, ob, ob); | ||||
| } | } | ||||
| EditBone *duplicateEditBoneObjects( | static void updateDuplicateConstraintSettings(EditBone *dupBone, Object *ob) | ||||
| EditBone *curBone, const char *name, ListBase *editbones, Object *src_ob, Object *dst_ob) | |||||
| { | { | ||||
| EditBone *eBone = MEM_mallocN(sizeof(EditBone), "addup_editbone"); | /* If an edit bone has been duplicated, lets | ||||
| * update it's constraints if the subtarget | |||||
| * they point to has also been duplicated | |||||
| */ | |||||
Done Inline ActionsEnd sentences with a period. The comment could also be put on two lines, I'm guessing. sybren: End sentences with a period. The comment could also be put on two lines, I'm guessing. | |||||
| EditBone *oldtarget; | |||||
| bPoseChannel *pchan; | |||||
| bConstraint *curcon; | |||||
| ListBase *conlist; | |||||
| /* Copy data from old bone to new bone */ | if ((pchan = BKE_pose_channel_verify(ob->pose, dupBone->name))) { | ||||
Done Inline ActionsThis block is indented too much. If pchan == NULL or conlist == NULL you can just return. This saves two indent levels. sybren: This block is indented too much. If `pchan == NULL` or `conlist == NULL` you can just return. | |||||
| memcpy(eBone, curBone, sizeof(EditBone)); | if ((conlist = &pchan->constraints)) { | ||||
| for (curcon = conlist->first; curcon; curcon = curcon->next) { | |||||
| if (curcon->type == CONSTRAINT_TYPE_ACTION) { | |||||
Done Inline ActionsThis should be a switch statement, and not a sequence of if/elseif. sybren: This should be a `switch` statement, and not a sequence of if/elseif.
Every case should be its… | |||||
| curBone->temp.ebone = eBone; | bActionConstraint *act_con = (bActionConstraint *)curcon->data; | ||||
| eBone->temp.ebone = curBone; | bAction *act = (bAction *)act_con->act; | ||||
| if (name != NULL) { | oldtarget = dupBone->temp.ebone; | ||||
Done Inline ActionsThe next code block (i.e. empty-line-delimited bit of code) doesn't mirror anything, so the comment is a bit weird here. sybren: The next code block (i.e. empty-line-delimited bit of code) doesn't mirror anything, so the… | |||||
| BLI_strncpy(eBone->name, name, sizeof(eBone->name)); | if (oldtarget) { | ||||
| /* Mirror the target range */ | |||||
| float mat[4][4]; | |||||
| unit_m4(mat); | |||||
| bPoseChannel *target_pchan = BKE_pose_channel_find_name(ob->pose, act_con->subtarget); | |||||
| BKE_constraint_mat_convertspace( | |||||
| ob, target_pchan, mat, curcon->tarspace, CONSTRAINT_SPACE_LOCAL, false); | |||||
| float max_axis_val = 0; | |||||
Done Inline ActionsWhat does it mean to "represent X"? This code seems to find the matrix row with the largest first value, but it's not clear why this is even relevant. sybren: What does it mean to "represent X"? This code seems to find the matrix row with the largest… | |||||
| int max_axis = 0; | |||||
| /* Which axis represents X now */ | |||||
| for (int i = 0; i < 3; i++) { | |||||
| float cur_val = fabsf(mat[0][i]); | |||||
| if (cur_val > max_axis_val) { | |||||
| max_axis = i; | |||||
| max_axis_val = cur_val; | |||||
| } | |||||
| } | } | ||||
Done Inline ActionsWhere is this mapped? What defines those values? Where do they come from? So many magic numbers. sybren: Where is this mapped? What defines those values? Where do they come from? So many magic numbers. | |||||
Done Inline ActionsThis is copy pasted from other parts of the animation system in blender. If we wish to update this, it should be done in a separate commit. zeddb: This is copy pasted from other parts of the animation system in blender.
If we wish to update… | |||||
| ED_armature_ebone_unique_name(editbones, eBone->name, NULL); | /* data->type is mapped as follows for backwards compatibility: | ||||
| BLI_addtail(editbones, eBone); | * 00,01,02 - rotation (it used to be like this) | ||||
| * 10,11,12 - scaling | |||||
| * 20,21,22 - location | |||||
| */ | |||||
| if (act_con->type != max_axis) { | |||||
| /* Y or Z rotation */ | |||||
| act_con->min = -act_con->min; | |||||
| act_con->max = -act_con->max; | |||||
| } | |||||
| else if (act_con->type == max_axis + 10) { | |||||
| /* X scaling */ | |||||
| // TODO Scaling should not need any conversions, right? | |||||
| } | |||||
| else if (act_con->type == max_axis + 20) { | |||||
| /* X location */ | |||||
| float imat[4][4]; | |||||
| invert_m4_m4(imat, mat); | |||||
| float min_vec[3], max_vec[3]; | |||||
| zero_v3(min_vec); | |||||
| zero_v3(max_vec); | |||||
| min_vec[0] = act_con->min; | |||||
| max_vec[0] = act_con->max; | |||||
| /* convert values into local object space */ | |||||
| mul_m4_v3(mat, min_vec); | |||||
| mul_m4_v3(mat, max_vec); | |||||
Done Inline ActionsWhy not write min_vec[0] = -min_vec[0]? Or write min_vec[0] *= -1? Personally I prefer the latter, as it's clear at first glance that it's only taking the negative (so no swapping min/max or changing the index). sybren: Why not write `min_vec[0] = -min_vec[0]`? Or write `min_vec[0] *= -1`? Personally I prefer the… | |||||
| float min_x_copy = min_vec[0]; | |||||
| min_vec[0] = max_vec[0] * -1; | |||||
| max_vec[0] = min_x_copy * -1; | |||||
| /* convert back to the settings space */ | |||||
| mul_m4_v3(imat, min_vec); | |||||
| mul_m4_v3(imat, max_vec); | |||||
| act_con->min = min_vec[0]; | |||||
| act_con->max = max_vec[0]; | |||||
| } | |||||
| /* See if there is any channels that uses this bone */ | |||||
| ListBase ani_curves; | |||||
| BLI_listbase_clear(&ani_curves); | |||||
| if (list_find_data_fcurves( | |||||
| &ani_curves, &act->curves, "pose.bones[", dupBone->temp.ebone->name)) { | |||||
Done Inline ActionsJust declare int i as part of the for-loop (so for (int i=0; ...). Outside of a for-loop it's a rather horribly-named variable. sybren: Just declare `int i` as part of the for-loop (so `for (int i=0; ...`). Outside of a for-loop… | |||||
Done Inline ActionsThis is because I can't initialize both i and bezt in the for loop then. I have moved down the declaration to just above the loop instead to make it better. zeddb: This is because I can't initialize both `i` and `bezt` in the for loop then.
I have moved down… | |||||
| /* Create a copy and mirror the animation */ | |||||
| for (LinkData *ld = ani_curves.first; ld; ld = ld->next) { | |||||
| FCurve *old_curve = ld->data; | |||||
| FCurve *new_curve = copy_fcurve(old_curve); | |||||
| int i; | |||||
| BezTriple *bezt; | |||||
| bActionGroup *agrp; | |||||
| char *old_path = new_curve->rna_path; | |||||
| new_curve->rna_path = BLI_str_replaceN( | |||||
| old_path, dupBone->temp.ebone->name, dupBone->name); | |||||
| MEM_freeN(old_path); | |||||
| /* Flip the animation */ | |||||
| for (i = 0, bezt = new_curve->bezt; i < new_curve->totvert; i++, bezt++) { | |||||
| const size_t slength = strlen(new_curve->rna_path); | |||||
| bool flip = false; | |||||
| if (BLI_strn_endswith(new_curve->rna_path, "location", slength) && | |||||
| new_curve->array_index == 0) { | |||||
| flip = true; | |||||
| } | |||||
| else if (BLI_strn_endswith( | |||||
| new_curve->rna_path, "rotation_quaternion", slength) && | |||||
| ELEM(new_curve->array_index, 2, 3)) { | |||||
| flip = true; | |||||
| } | |||||
| else if (BLI_strn_endswith(new_curve->rna_path, "rotation_euler", slength) && | |||||
| ELEM(new_curve->array_index, 1, 2)) { | |||||
| flip = true; | |||||
| } | |||||
| else if (BLI_strn_endswith( | |||||
| new_curve->rna_path, "rotation_axis_angle", slength) && | |||||
Done Inline ActionsJust *= -1 those sybren: Just `*= -1` those | |||||
| ELEM(new_curve->array_index, 2, 3)) { | |||||
| flip = true; | |||||
| } | |||||
| if (flip) { | |||||
| bezt->vec[0][1] = -bezt->vec[0][1]; | |||||
| bezt->vec[1][1] = -bezt->vec[1][1]; | |||||
| bezt->vec[2][1] = -bezt->vec[2][1]; | |||||
| } | |||||
| } | |||||
| /* Make sure that a action group name for the new bone exists */ | |||||
| agrp = BKE_action_group_find_name(act, dupBone->name); | |||||
| if (agrp == NULL) { | |||||
| agrp = action_groups_add_new(act, dupBone->name); | |||||
Done Inline Actionsact doesn't change within the for-loop, so you can do the DEG update outside the loop. sybren: `act` doesn't change within the `for`-loop, so you can do the DEG update outside the loop. | |||||
| } | |||||
| BLI_assert(agrp != NULL); | |||||
| // TODO might need to remove the existing channel if it already exists | |||||
| // from previous mirror syncing | |||||
| action_groups_add_channel(act, agrp, new_curve); | |||||
| /* Make deps graph aware of our changes */ | |||||
| DEG_id_tag_update(&act->id, ID_RECALC_ANIMATION_NO_FLUSH); | |||||
Done Inline ActionsIt's not clamping, it's modulo'ing. And yes, that's a good thing to do. In a separate function, in BLI_whatevermath. Once working on it anyway, you may want to add two normalisation functions, one producing a result on the interval [-180°, 180°), and one on [0°, 360°). sybren: It's not clamping, it's modulo'ing. And yes, that's a good thing to do. In a separate function… | |||||
| } | |||||
| } | |||||
| BLI_freelistN(&ani_curves); | |||||
| } | |||||
| } | |||||
| else if (curcon->type == CONSTRAINT_TYPE_KINEMATIC) { | |||||
Done Inline ActionsAdd something like "This code assumes that bRotLimitConstraint and bLocLimitConstraint have the same fields in the same memory locations." Possibly it would be prudent to add a similar warning to the types themselves, pointing back at this code. sybren: Add something like "This code assumes that bRotLimitConstraint and bLocLimitConstraint have the… | |||||
| /* IK constraint */ | |||||
| bKinematicConstraint *ik = (bKinematicConstraint *)curcon->data; | |||||
Done Inline ActionsDon't use mat or imat, but give them a descriptive name, something that is a bit closer to "constraint space conversion matrix". sybren: Don't use `mat` or `imat`, but give them a descriptive name, something that is a bit closer to… | |||||
| // TODO Perhaps clamp this to 180 degrees (soft limits of the constraint) | |||||
| ik->poleangle = -M_PI - ik->poleangle; | |||||
Done Inline ActionsUnclear why the 'min vector' and 'max vector' are relevant concepts at all. sybren: Unclear why the 'min vector' and 'max vector' are relevant concepts at all. | |||||
Done Inline ActionsThis is so we can easily transform the min/max values of the constraint. zeddb: This is so we can easily transform the min/max values of the constraint. | |||||
| } | |||||
| else if (curcon->type == CONSTRAINT_TYPE_ROTLIMIT || | |||||
Done Inline ActionsBit of a weird choice in empty lines here, possibly better grouping possible? sybren: Bit of a weird choice in empty lines here, possibly better grouping possible? | |||||
Done Inline ActionsGrouped by min/max values. I don't see any better grouping here. zeddb: Grouped by min/max values. I don't see any better grouping here. | |||||
| curcon->type == CONSTRAINT_TYPE_LOCLIMIT) { | |||||
| // TODO curcon->type == CONSTRAINT_TYPE_SIZELIMIT mirroring not needed? | |||||
| /* Both of these have the same struct data */ | |||||
| bRotLimitConstraint *limit = (bRotLimitConstraint *)curcon->data; | |||||
| float mat[4][4], imat[4][4]; | |||||
| float min_vec[3], max_vec[3]; | |||||
| min_vec[0] = limit->xmin; | |||||
| min_vec[1] = limit->ymin; | |||||
| min_vec[2] = limit->zmin; | |||||
| max_vec[0] = limit->xmax; | |||||
| max_vec[1] = limit->ymax; | |||||
| max_vec[2] = limit->zmax; | |||||
| unit_m4(mat); | |||||
| BKE_constraint_mat_convertspace( | |||||
| ob, pchan, mat, curcon->ownspace, CONSTRAINT_SPACE_LOCAL, false); | |||||
| if (curcon->type == CONSTRAINT_TYPE_ROTLIMIT) { | |||||
| /* Zero out any location translation */ | |||||
| mat[3][0] = mat[3][1] = mat[3][2] = 0; | |||||
| } | |||||
| invert_m4_m4(imat, mat); | |||||
| /* convert values into local object space */ | |||||
| mul_m4_v3(mat, min_vec); | |||||
| mul_m4_v3(mat, max_vec); | |||||
| if (curcon->type == CONSTRAINT_TYPE_ROTLIMIT) { | |||||
| float min_copy[3]; | |||||
| copy_v3_v3(min_copy, min_vec); | |||||
| min_vec[1] = max_vec[1] * -1; | |||||
| min_vec[2] = max_vec[2] * -1; | |||||
| max_vec[1] = min_copy[1] * -1; | |||||
| max_vec[2] = min_copy[2] * -1; | |||||
| } | |||||
| else { | |||||
| float min_x_copy = min_vec[0]; | |||||
| min_vec[0] = max_vec[0] * -1; | |||||
| max_vec[0] = min_x_copy * -1; | |||||
| } | |||||
| /* convert back to the settings space */ | |||||
| mul_m4_v3(imat, min_vec); | |||||
| mul_m4_v3(imat, max_vec); | |||||
| limit->xmin = min_vec[0]; | |||||
| limit->ymin = min_vec[1]; | |||||
| limit->zmin = min_vec[2]; | |||||
Done Inline Actionstar → target own, imat → something more descriptive sybren: tar → target
own, imat → something more descriptive
| |||||
Done Inline ActionsThis is what the spaces are called in the constraints themselves. zeddb: This is what the spaces are called in the constraints themselves.
So I think that these are… | |||||
| limit->xmax = max_vec[0]; | |||||
| limit->ymax = max_vec[1]; | |||||
| limit->zmax = max_vec[2]; | |||||
| } | |||||
| else if (curcon->type == CONSTRAINT_TYPE_TRANSFORM) { | |||||
| bTransformConstraint *trans = (bTransformConstraint *)curcon->data; | |||||
| float tar_mat[4][4], own_mat[4][4], imat[4][4]; | |||||
| unit_m4(own_mat); | |||||
| BKE_constraint_mat_convertspace( | |||||
| ob, pchan, own_mat, curcon->ownspace, CONSTRAINT_SPACE_LOCAL, false); | |||||
| /* ###Source map mirroring### */ | |||||
| float old_min, old_max; | |||||
| /* Source location */ | |||||
| invert_m4_m4(imat, own_mat); | |||||
Done Inline ActionsWhy not just = -old_max? sybren: Why not just ` = -old_max`? | |||||
| /* convert values into local object space */ | |||||
| mul_m4_v3(own_mat, trans->from_min); | |||||
| mul_m4_v3(own_mat, trans->from_max); | |||||
| old_min = trans->from_min[0]; | |||||
| old_max = trans->from_max[0]; | |||||
| trans->from_min[0] = old_max * -1; | |||||
| trans->from_max[0] = old_min * -1; | |||||
| /* convert back to the settings space */ | |||||
| mul_m4_v3(imat, trans->from_min); | |||||
| mul_m4_v3(imat, trans->from_max); | |||||
| /* Source rotation */ | |||||
| /* Zero out any location translation */ | |||||
| own_mat[3][0] = own_mat[3][1] = own_mat[3][2] = 0; | |||||
| invert_m4_m4(imat, own_mat); | |||||
| /* convert values into local object space */ | |||||
| mul_m4_v3(own_mat, trans->from_min_rot); | |||||
| mul_m4_v3(own_mat, trans->from_max_rot); | |||||
| old_min = trans->from_min_rot[1]; | |||||
| old_max = trans->from_max_rot[1]; | |||||
| trans->from_min_rot[1] = old_max * -1; | |||||
| trans->from_max_rot[1] = old_min * -1; | |||||
| old_min = trans->from_min_rot[2]; | |||||
| old_max = trans->from_max_rot[2]; | |||||
| trans->from_min_rot[2] = old_max * -1; | |||||
| trans->from_max_rot[2] = old_min * -1; | |||||
Done Inline ActionsDon't name things temp, but describe what it is. The fact that it's here in an if in a loop in an if already shows that's temporary. Declaring where it's needed and then naming descriptively ('copy of scale' is likely better than 'temp vector') is better. sybren: Don't name things `temp`, but describe what it is. The fact that it's here in an if in a loop… | |||||
Done Inline ActionsIn this case, the variable is used to store values temporary. It stores loc/rot/scale data depending on how the constraint is mapped. zeddb: In this case, the variable is used to store values temporary.
It stores loc/rot/scale data… | |||||
| /* convert back to the settings space */ | |||||
| mul_m4_v3(imat, trans->from_min_rot); | |||||
| mul_m4_v3(imat, trans->from_max_rot); | |||||
| /* Source scale does not require any mirroring */ | |||||
| /* ###Destination map mirroring### */ | |||||
| float temp_vec[3]; | |||||
| bPoseChannel *target_pchan = BKE_pose_channel_find_name(ob->pose, trans->subtarget); | |||||
| unit_m4(tar_mat); | |||||
| BKE_constraint_mat_convertspace( | |||||
| ob, target_pchan, tar_mat, curcon->tarspace, CONSTRAINT_SPACE_LOCAL, false); | |||||
| invert_m4_m4(imat, tar_mat); | |||||
| /* convert values into local object space */ | |||||
| mul_m4_v3(tar_mat, trans->to_min); | |||||
| mul_m4_v3(tar_mat, trans->to_max); | |||||
| mul_m4_v3(tar_mat, trans->to_min_rot); | |||||
| mul_m4_v3(tar_mat, trans->to_max_rot); | |||||
| mul_m4_v3(tar_mat, trans->to_min_scale); | |||||
| mul_m4_v3(tar_mat, trans->to_max_scale); | |||||
| // TODO maybe simplify this by using matrices (is it possible?) | |||||
Done Inline Actionsswitch! sybren: switch! | |||||
| // TODO euler order? | |||||
| // TODO zero out on rotaion matrix translation (on all constraint types!) | |||||
| if (trans->to == TRANS_SCALE) { | |||||
| copy_v3_v3(temp_vec, trans->to_max_scale); | |||||
Done Inline ActionsYou could even go further and de-uglify it locally a bit by #define AXIS_X 0 and the like (or use a const char for that). That way at least we have some understandable word there, instead of just a 0. sybren: You could even go further and de-uglify it locally a bit by `#define AXIS_X 0` and the like (or… | |||||
| for (int i = 0; i < 3; i++) { | |||||
| if ((trans->from == TRANS_LOCATION && trans->map[i] == 0) || | |||||
| (trans->from == TRANS_ROTATION && trans->map[i] != 0)) { | |||||
| /* X Loc to X/Y/Z Scale: Min/Max Flipped */ | |||||
| /* Y Rot to X/Y/Z Scale: Min/Max Flipped */ | |||||
| /* Z Rot to X/Y/Z Scale: Min/Max Flipped */ | |||||
| trans->to_max_scale[i] = trans->to_min_scale[i]; | |||||
| trans->to_min_scale[i] = temp_vec[i]; | |||||
| } | |||||
| } | |||||
| } | |||||
| else if (trans->to == TRANS_LOCATION) { | |||||
| /* Invert the X location */ | |||||
| trans->to_min[0] *= -1; | |||||
| trans->to_max[0] *= -1; | |||||
| copy_v3_v3(temp_vec, trans->to_max); | |||||
| for (int i = 0; i < 3; i++) { | |||||
| if ((trans->from == TRANS_LOCATION && trans->map[i] == 0) || | |||||
| (trans->from == TRANS_ROTATION && trans->map[i] != 0)) { | |||||
| /* X Loc to X/Y/Z Loc: Min/Max Flipped (and Inverted) | |||||
| * Y Rot to X/Y/Z Loc: Min/Max Flipped | |||||
| * Z Rot to X/Y/Z Loc: Min/Max Flipped */ | |||||
| trans->to_max[i] = trans->to_min[i]; | |||||
| trans->to_min[i] = temp_vec[i]; | |||||
| } | |||||
| } | |||||
| } | |||||
| else if (trans->to == TRANS_ROTATION) { | |||||
| /* Invert the Z rotation */ | |||||
| trans->to_min_rot[2] *= -1; | |||||
| trans->to_max_rot[2] *= -1; | |||||
| if ((trans->from == TRANS_LOCATION && trans->map[1] != 0) || | |||||
| (trans->from == TRANS_ROTATION && trans->map[1] != 1) || | |||||
| trans->from == TRANS_SCALE) { | |||||
| /* Invert the Y rotation */ | |||||
| trans->to_min_rot[1] *= -1; | |||||
| trans->to_max_rot[1] *= -1; | |||||
| } | |||||
| copy_v3_v3(temp_vec, trans->to_max_rot); | |||||
| for (int i = 0; i < 3; i++) { | |||||
| if ((trans->from == TRANS_LOCATION && trans->map[i] == 0 && i != 1) || | |||||
| (trans->from == TRANS_ROTATION && trans->map[i] == 1 && i != 1) || | |||||
| (trans->from == TRANS_ROTATION && trans->map[i] == 2)) { | |||||
| /* X Loc to X/Z Rot: Flipped | |||||
| * Y Rot to X/Z Rot: Flipped | |||||
| * Z Rot to X/Y/Z rot: Flipped */ | |||||
| trans->to_max_rot[i] = trans->to_min_rot[i]; | |||||
| trans->to_min_rot[i] = temp_vec[i]; | |||||
| } | |||||
| } | |||||
| } | |||||
| /* convert back to the settings space */ | |||||
| mul_m4_v3(imat, trans->to_min); | |||||
| mul_m4_v3(imat, trans->to_max); | |||||
| mul_m4_v3(imat, trans->to_min_rot); | |||||
| mul_m4_v3(imat, trans->to_max_rot); | |||||
| mul_m4_v3(imat, trans->to_min_scale); | |||||
| mul_m4_v3(imat, trans->to_max_scale); | |||||
| } | |||||
| } | |||||
| } | |||||
| } | |||||
| } | |||||
Done Inline ActionsSame as above, just do if (ob->pose == NULL) { return; } sybren: Same as above, just do `if (ob->pose == NULL) { return; }` | |||||
| static void updateDuplicateCustomBoneShapes(bContext *C, EditBone *dupBone, Object *ob) | |||||
| { | |||||
| if (ob->pose) { | |||||
| bPoseChannel *pchan; | |||||
| pchan = BKE_pose_channel_verify(ob->pose, dupBone->name); | |||||
| if (pchan->custom != NULL) { | |||||
| Main *bmain = CTX_data_main(C); | |||||
| char name_flip[MAX_ID_NAME - 2]; | |||||
| /* Skip the first two chars in the object name as those are used to store object type */ | |||||
| BLI_string_flip_side_name(name_flip, pchan->custom->id.name + 2, false, sizeof(name_flip)); | |||||
| Object *shape_ob = (Object *)BKE_libblock_find_name(bmain, ID_OB, name_flip); | |||||
| if (shape_ob != NULL) { | |||||
| /* A flipped shape object exists, use it! */ | |||||
| pchan->custom = shape_ob; | |||||
| } | |||||
| } | |||||
| } | |||||
| } | |||||
| static void copy_pchan(EditBone *src_bone, EditBone *dst_bone, Object *src_ob, Object *dst_ob) | |||||
| { | |||||
| /* copy the ID property */ | /* copy the ID property */ | ||||
| if (curBone->prop) { | if (src_bone->prop) { | ||||
| eBone->prop = IDP_CopyProperty(curBone->prop); | dst_bone->prop = IDP_CopyProperty(src_bone->prop); | ||||
| } | } | ||||
| /* Lets duplicate the list of constraints that the | /* Lets duplicate the list of constraints that the | ||||
| * current bone has. | * current bone has. | ||||
| */ | */ | ||||
| if (src_ob->pose) { | if (src_ob->pose) { | ||||
| bPoseChannel *chanold, *channew; | bPoseChannel *chanold, *channew; | ||||
| chanold = BKE_pose_channel_verify(src_ob->pose, curBone->name); | chanold = BKE_pose_channel_verify(src_ob->pose, src_bone->name); | ||||
| if (chanold) { | if (chanold) { | ||||
| /* WARNING: this creates a new posechannel, but there will not be an attached bone | /* WARNING: this creates a new posechannel, but there will not be an attached bone | ||||
| * yet as the new bones created here are still 'EditBones' not 'Bones'. | * yet as the new bones created here are still 'EditBones' not 'Bones'. | ||||
| */ | */ | ||||
| channew = BKE_pose_channel_verify(dst_ob->pose, eBone->name); | channew = BKE_pose_channel_verify(dst_ob->pose, dst_bone->name); | ||||
| if (channew) { | if (channew) { | ||||
| BKE_pose_channel_copy_data(channew, chanold); | BKE_pose_channel_copy_data(channew, chanold); | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| } | |||||
Done Inline ActionsCode style: use underscore instead of camel case sybren: Code style: use underscore instead of camel case | |||||
| EditBone *duplicateEditBoneObjects( | |||||
| EditBone *curBone, const char *name, ListBase *editbones, Object *src_ob, Object *dst_ob) | |||||
| { | |||||
| EditBone *eBone = MEM_mallocN(sizeof(EditBone), "addup_editbone"); | |||||
| /* Copy data from old bone to new bone */ | |||||
| memcpy(eBone, curBone, sizeof(EditBone)); | |||||
| curBone->temp.ebone = eBone; | |||||
| eBone->temp.ebone = curBone; | |||||
| if (name != NULL) { | |||||
| BLI_strncpy(eBone->name, name, sizeof(eBone->name)); | |||||
| } | |||||
| ED_armature_ebone_unique_name(editbones, eBone->name, NULL); | |||||
| BLI_addtail(editbones, eBone); | |||||
| copy_pchan(curBone, eBone, src_ob, dst_ob); | |||||
| return eBone; | return eBone; | ||||
| } | } | ||||
| EditBone *duplicateEditBone(EditBone *curBone, const char *name, ListBase *editbones, Object *ob) | EditBone *duplicateEditBone(EditBone *curBone, const char *name, ListBase *editbones, Object *ob) | ||||
| { | { | ||||
| return duplicateEditBoneObjects(curBone, name, editbones, ob, ob); | return duplicateEditBoneObjects(curBone, name, editbones, ob, ob); | ||||
| } | } | ||||
| ▲ Show 20 Lines • Show All 44 Lines • ▼ Show 20 Lines | for (ebone_iter = arm->edbo->first; ebone_iter && ebone_iter != ebone_first_dupe; | ||||
| EditBone *ebone; | EditBone *ebone; | ||||
| char new_bone_name_buff[MAXBONENAME]; | char new_bone_name_buff[MAXBONENAME]; | ||||
| char *new_bone_name = ebone_iter->name; | char *new_bone_name = ebone_iter->name; | ||||
| if (do_flip_names) { | if (do_flip_names) { | ||||
| BLI_string_flip_side_name( | BLI_string_flip_side_name( | ||||
| new_bone_name_buff, ebone_iter->name, false, sizeof(new_bone_name_buff)); | new_bone_name_buff, ebone_iter->name, false, sizeof(new_bone_name_buff)); | ||||
| /* Only use flipped name if not yet in use. Otherwise we'd get again inconsistent namings | /* Only use flipped name if not yet in use. Otherwise we'd get again inconsistent | ||||
| * (different numbers), better keep default behavior in this case. */ | * namings (different numbers), better keep default behavior in this case. */ | ||||
| if (ED_armature_ebone_find_name(arm->edbo, new_bone_name_buff) == NULL) { | if (ED_armature_ebone_find_name(arm->edbo, new_bone_name_buff) == NULL) { | ||||
| new_bone_name = new_bone_name_buff; | new_bone_name = new_bone_name_buff; | ||||
| } | } | ||||
| } | } | ||||
| ebone = duplicateEditBone(ebone_iter, new_bone_name, arm->edbo, ob); | ebone = duplicateEditBone(ebone_iter, new_bone_name, arm->edbo, ob); | ||||
| if (!ebone_first_dupe) { | if (!ebone_first_dupe) { | ||||
| ▲ Show 20 Lines • Show All 184 Lines • ▼ Show 20 Lines | for (ebone_iter = arm->edbo->first; ebone_iter; ebone_iter = ebone_iter->next) { | ||||
| ebone_src->temp.ebone = ebone_dst; | ebone_src->temp.ebone = ebone_dst; | ||||
| ebone_dst->flag &= ~(BONE_SELECTED | BONE_TIPSEL | BONE_ROOTSEL); | ebone_dst->flag &= ~(BONE_SELECTED | BONE_TIPSEL | BONE_ROOTSEL); | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
Done Inline ActionsSame as before, probably can use continue here. Patch wasn't submitted with Arcanist, so I can't see outside it (no context available), so maybe this is not the case. sybren: Same as before, probably can use `continue` here. Patch wasn't submitted with Arcanist, so I… | |||||
Done Inline ActionsCan't use continue here. I have resubmitted the new revision of the patch with context. zeddb: Can't use `continue` here. I have resubmitted the new revision of the patch with context. | |||||
| /* Find the selected bones and duplicate them as needed, with mirrored name */ | /* Find the selected bones and duplicate them as needed, with mirrored name */ | ||||
| for (ebone_iter = arm->edbo->first; ebone_iter && ebone_iter != ebone_first_dupe; | for (ebone_iter = arm->edbo->first; ebone_iter && ebone_iter != ebone_first_dupe; | ||||
| ebone_iter = ebone_iter->next) { | ebone_iter = ebone_iter->next) { | ||||
| if (EBONE_VISIBLE(arm, ebone_iter) && (ebone_iter->flag & BONE_SELECTED) && | if (EBONE_VISIBLE(arm, ebone_iter) && (ebone_iter->flag & BONE_SELECTED)) { | ||||
| /* will be set if the mirror bone already exists (no need to make a new one) */ | if (ebone_iter->temp.ebone != NULL) { | ||||
| (ebone_iter->temp.ebone == NULL)) { | /* will be set if the mirror bone already exists (no need to make a new one) | ||||
| * but we do need to make sure that the pchan settings (constraints etc) is syncronized | |||||
| */ | |||||
| bPoseChannel *pchan; | |||||
| /* Make sure we clean up the old data before overwriting it */ | |||||
| pchan = BKE_pose_channel_verify(obedit->pose, ebone_iter->temp.ebone->name); | |||||
| BKE_pose_channel_free(pchan); | |||||
| /* Sync pchan data */ | |||||
| copy_pchan(ebone_iter, ebone_iter->temp.ebone, obedit, obedit); | |||||
| continue; | |||||
| } | |||||
| char name_flip[MAXBONENAME]; | char name_flip[MAXBONENAME]; | ||||
| BLI_string_flip_side_name(name_flip, ebone_iter->name, false, sizeof(name_flip)); | BLI_string_flip_side_name(name_flip, ebone_iter->name, false, sizeof(name_flip)); | ||||
| /* bones must have a side-suffix */ | /* bones must have a side-suffix */ | ||||
| if (!STREQ(name_flip, ebone_iter->name)) { | if (!STREQ(name_flip, ebone_iter->name)) { | ||||
| EditBone *ebone; | EditBone *ebone; | ||||
| Show All 30 Lines | for (ebone_iter = arm->edbo->first; ebone_iter && ebone_iter != ebone_first_dupe; | ||||
| EditBone *ebone_parent = get_symmetrized_bone(arm, ebone_iter->parent); | EditBone *ebone_parent = get_symmetrized_bone(arm, ebone_iter->parent); | ||||
| if (ebone_parent == ebone_iter->parent) { | if (ebone_parent == ebone_iter->parent) { | ||||
| /* If the mirror lookup failed, (but the current bone has a parent) | /* If the mirror lookup failed, (but the current bone has a parent) | ||||
| * then we can assume the parent has no L/R but is a center bone. | * then we can assume the parent has no L/R but is a center bone. | ||||
| * So just use the same parent for both. | * So just use the same parent for both. | ||||
| */ | */ | ||||
| ebone->flag &= ~BONE_CONNECTED; | ebone->flag &= ~BONE_CONNECTED; | ||||
| } | } | ||||
Done Inline ActionsShouldn't this use head[axis] instead? sybren: Shouldn't this use `head[axis]` instead? | |||||
Done Inline ActionsRight you are! zeddb: Right you are! | |||||
| ebone->parent = ebone_parent; | ebone->parent = ebone_parent; | ||||
| } | } | ||||
| /* Update custom handle links. */ | /* Update custom handle links. */ | ||||
| ebone->bbone_prev = get_symmetrized_bone(arm, ebone_iter->bbone_prev); | ebone->bbone_prev = get_symmetrized_bone(arm, ebone_iter->bbone_prev); | ||||
| ebone->bbone_next = get_symmetrized_bone(arm, ebone_iter->bbone_next); | ebone->bbone_next = get_symmetrized_bone(arm, ebone_iter->bbone_next); | ||||
| /* Lets try to fix any constraint subtargets that might | /* Lets try to fix any constraint subtargets that might | ||||
| * have been duplicated | * have been duplicated | ||||
| */ | */ | ||||
| updateDuplicateSubtarget(ebone, arm->edbo, obedit); | updateDuplicateSubtarget(ebone, arm->edbo, obedit); | ||||
| /* Try to update constraint options so that they are mirrored as well */ | |||||
| updateDuplicateConstraintSettings(ebone, obedit); | |||||
| /* Mirror bone shapes if possible */ | |||||
| updateDuplicateCustomBoneShapes(C, ebone, obedit); | |||||
| } | } | ||||
| } | } | ||||
| ED_armature_edit_transform_mirror_update(obedit); | ED_armature_edit_transform_mirror_update(obedit); | ||||
| /* Selected bones now have their 'temp' pointer set, | /* Selected bones now have their 'temp' pointer set, | ||||
| * so we don't need this anymore */ | * so we don't need this anymore */ | ||||
| ▲ Show 20 Lines • Show All 430 Lines • Show Last 20 Lines | |||||
There used to be a comment here with a bit of explanation of what parameter is what. I think that's still a good idea here (but possibly a completely different comment because behaviour changed).