[Bf-blender-cvs] [65a8070] master: Fix T46010: Bone offset between Rest Pose and Edit mode.
Bastien Montagne
noreply at git.blender.org
Fri Sep 4 16:50:44 CEST 2015
Commit: 65a80708d4ca0d28a9c6e35bf7429693b806e0b4
Author: Bastien Montagne
Date: Fri Sep 4 16:38:24 2015 +0200
Branches: master
https://developer.blender.org/rB65a80708d4ca0d28a9c6e35bf7429693b806e0b4
Fix T46010: Bone offset between Rest Pose and Edit mode.
That one was hairy... To summarize:
* We were setting Bone.head/tail (aka **local** rest location of bone) from EditBone data, using **EditBone's parent computed armature space**.
* We use those local head/tail to define Bone's restpose (in `BKE_armature_where_is_bone()`), using **Bone's parent armature space** (aka parent's arm_mat).
* Because of bone's roll nightmare, the two above parent's matrices will often not be the same.
In an ideal world, this should not affect locations (head/tail), but in real world of float it does - noticeably, in some extreme cases.
So! This commit cleans up things a bit (`fix_bonelist_roll()` was already doing much more than just fixing roll mess, has been renamed
to `armature_finalize_restpose()`), and ensures we do use (final!) parent's arm_mat local space to compute children's local head/tail as well.
This allows us to avoid too much imprecision here.
Checked the patch also with a complete Victor's rig from Gooseberry, seems to have no nasty side effects - fingers crossed!
===================================================================
M source/blender/editors/armature/armature_utils.c
===================================================================
diff --git a/source/blender/editors/armature/armature_utils.c b/source/blender/editors/armature/armature_utils.c
index 8251740..4fe79b3 100644
--- a/source/blender/editors/armature/armature_utils.c
+++ b/source/blender/editors/armature/armature_utils.c
@@ -477,49 +477,78 @@ EditBone *make_boneList(ListBase *edbo, ListBase *bones, EditBone *parent, Bone
return eBoneAct;
}
-/* nasty stuff for converting roll in editbones into bones */
-/* also sets restposition in armature (arm_mat) */
-static void fix_bonelist_roll(ListBase *bonelist, ListBase *editbonelist)
+/* This function:
+ * - sets local head/tail rest locations using parent bone's arm_mat.
+ * - calls BKE_armature_where_is_bone() which uses parent's transform (arm_mat) to define this bone's transform.
+ * - fixes (converts) EditBone roll into Bone roll.
+ * - calls again BKE_armature_where_is_bone(), since roll fidling may have changed things for our bone...
+ * Note that order is crucial here, we can only handle child if all its parents in chain have already been handled
+ * (this is ensured by recursive process). */
+static void armature_finalize_restpose(ListBase *bonelist, ListBase *editbonelist)
{
Bone *curBone;
EditBone *ebone;
- float premat[3][3];
- float postmat[3][3];
- float difmat[3][3];
- float imat[3][3];
-
+
for (curBone = bonelist->first; curBone; curBone = curBone->next) {
- /* sets local matrix and arm_mat (restpos).
- * Do not recurse into children here, fix_bonelist_roll is already recursive. */
+ /* Set bone's local head/tail.
+ * Note that it's important to use final parent's restpose (arm_mat) here, instead of setting those values
+ * from editbone's matrix (see T46010). */
+ if (curBone->parent) {
+ float parmat_inv[4][4];
+
+ invert_m4_m4(parmat_inv, curBone->parent->arm_mat);
+
+ /* Get the new head and tail */
+ sub_v3_v3v3(curBone->head, curBone->arm_head, curBone->parent->arm_tail);
+ sub_v3_v3v3(curBone->tail, curBone->arm_tail, curBone->parent->arm_tail);
+
+ mul_mat3_m4_v3(parmat_inv, curBone->head);
+ mul_mat3_m4_v3(parmat_inv, curBone->tail);
+ }
+ else {
+ copy_v3_v3(curBone->head, curBone->arm_head);
+ copy_v3_v3(curBone->tail, curBone->arm_tail);
+ }
+
+ /* Set local matrix and arm_mat (restpose).
+ * Do not recurse into children here, armature_finalize_restpose() is already recursive. */
BKE_armature_where_is_bone(curBone, curBone->parent, false);
-
+
/* Find the associated editbone */
- for (ebone = editbonelist->first; ebone; ebone = ebone->next)
- if (ebone->temp.bone == curBone)
- break;
-
- if (ebone) {
- /* Get the ebone premat */
- ED_armature_ebone_to_mat3(ebone, premat);
-
- /* Get the bone postmat */
- copy_m3_m4(postmat, curBone->arm_mat);
-
- invert_m3_m3(imat, premat);
- mul_m3_m3m3(difmat, imat, postmat);
+ for (ebone = editbonelist->first; ebone; ebone = ebone->next) {
+ if (ebone->temp.bone == curBone) {
+ float premat[3][3];
+ float postmat[3][3];
+ float difmat[3][3];
+ float imat[3][3];
+
+ /* Get the ebone premat and its inverse. */
+ ED_armature_ebone_to_mat3(ebone, premat);
+ invert_m3_m3(imat, premat);
+
+ /* Get the bone postmat. */
+ copy_m3_m4(postmat, curBone->arm_mat);
+
+ mul_m3_m3m3(difmat, imat, postmat);
+
#if 0
- printf("Bone %s\n", curBone->name);
- print_m4("premat", premat);
- print_m4("postmat", postmat);
- print_m4("difmat", difmat);
- printf("Roll = %f\n", RAD2DEGF(-atan2(difmat[2][0], difmat[2][2])));
+ printf("Bone %s\n", curBone->name);
+ print_m4("premat", premat);
+ print_m4("postmat", postmat);
+ print_m4("difmat", difmat);
+ printf("Roll = %f\n", RAD2DEGF(-atan2(difmat[2][0], difmat[2][2])));
#endif
- curBone->roll = -atan2f(difmat[2][0], difmat[2][2]);
-
- /* and set restposition again */
- BKE_armature_where_is_bone(curBone, curBone->parent, false);
+
+ curBone->roll = -atan2f(difmat[2][0], difmat[2][2]);
+
+ /* and set restposition again */
+ BKE_armature_where_is_bone(curBone, curBone->parent, false);
+ break;
+ }
}
- fix_bonelist_roll(&curBone->childbase, editbonelist);
+
+ /* Recurse into children... */
+ armature_finalize_restpose(&curBone->childbase, editbonelist);
}
}
@@ -588,48 +617,29 @@ void ED_armature_from_edit(bArmature *arm)
newBone->prop = IDP_CopyProperty(eBone->prop);
}
- /* Fix parenting in a separate pass to ensure ebone->bone connections
- * are valid at this point */
+ /* Fix parenting in a separate pass to ensure ebone->bone connections are valid at this point.
+ * Do not set bone->head/tail here anymore, using EditBone data for that is not OK since our later fidling
+ * with parent's arm_mat (for roll conversion) may have some small but visible impact on locations (T46010). */
for (eBone = arm->edbo->first; eBone; eBone = eBone->next) {
newBone = eBone->temp.bone;
if (eBone->parent) {
newBone->parent = eBone->parent->temp.bone;
BLI_addtail(&newBone->parent->childbase, newBone);
-
- {
- float M_parentRest[3][3];
- float iM_parentRest[3][3];
-
- /* Get the parent's matrix (rotation only) */
- ED_armature_ebone_to_mat3(eBone->parent, M_parentRest);
-
- /* Invert the parent matrix */
- invert_m3_m3(iM_parentRest, M_parentRest);
-
- /* Get the new head and tail */
- sub_v3_v3v3(newBone->head, eBone->head, eBone->parent->tail);
- sub_v3_v3v3(newBone->tail, eBone->tail, eBone->parent->tail);
-
- mul_m3_v3(iM_parentRest, newBone->head);
- mul_m3_v3(iM_parentRest, newBone->tail);
- }
}
/* ...otherwise add this bone to the armature's bonebase */
else {
- copy_v3_v3(newBone->head, eBone->head);
- copy_v3_v3(newBone->tail, eBone->tail);
BLI_addtail(&arm->bonebase, newBone);
}
}
- /* Make a pass through the new armature to fix rolling */
- /* also builds restposition again (like BKE_armature_where_is) */
- fix_bonelist_roll(&arm->bonebase, arm->edbo);
+ /* Finalize definition of restpose data (roll, bone_mat, arm_mat, head/tail...). */
+ armature_finalize_restpose(&arm->bonebase, arm->edbo);
/* so all users of this armature should get rebuilt */
for (obt = G.main->object.first; obt; obt = obt->id.next) {
- if (obt->data == arm)
+ if (obt->data == arm) {
BKE_pose_rebuild(obt, arm);
+ }
}
DAG_id_tag_update(&arm->id, 0);
More information about the Bf-blender-cvs
mailing list