Bugfix: Bone components attached to animations now work correctly

Source/BansheeCore/Animation/BsAnimation.cpp

@@ -1428,29 +1428,28 @@ namespace bs
 						}
 					}
 
-					Vector3 rootParentPos = Vector3::ZERO;
-					Quaternion rootParentRot = Quaternion::IDENTITY;
-					Vector3 rootParentScale = Vector3::ONE;
+					while(rootSO && rootSO.isDestroyed(true))
+						rootSO = rootSO->getParent();
 
-					if(rootSO.isDestroyed(true))
+					Vector3 parentPos = Vector3::ZERO;
+					Quaternion parentRot = Quaternion::IDENTITY;
+					Vector3 parentScale = Vector3::ONE;
+
+					if(!rootSO.isDestroyed(true))
 					{
-						HSceneObject rootParent = rootSO->getParent();
-						if(!rootParent.isDestroyed(true))
-						{
-							const Transform& tfrm = rootParent->getTransform();
-							rootParentPos = tfrm.getPosition();
-							rootParentRot = tfrm.getRotation();
-							rootParentScale = tfrm.getScale();
-						}
+						const Transform& tfrm = rootSO->getTransform();
+						parentPos = tfrm.getPosition();
+						parentRot = tfrm.getRotation();
+						parentScale = tfrm.getScale();
 					}
 
 					// Transform from space relative to root's parent to world space
-					rotation = rootParentRot * rotation;
+					rotation = parentRot * rotation;
 
-					scale = rootParentScale * scale;
+					scale = parentScale * scale;
 
-					position = rootParentRot.rotate(rootParentScale * position);
-					position += rootParentPos;
+					position = parentRot.rotate(parentScale * position);
+					position += parentPos;
 
 					so->setWorldPosition(position);
 					so->setWorldRotation(rotation);

Source/BansheeCore/Animation/BsSkeleton.cpp

@@ -88,7 +88,7 @@ namespace bs
 	{ }
 
 	Skeleton::Skeleton(BONE_DESC* bones, UINT32 numBones)
-		: mNumBones(numBones), mBoneTransforms(bs_newN<Matrix4>(numBones)), mInvBindPoses(bs_newN<Matrix4>(numBones))
+		: mNumBones(numBones), mBoneTransforms(bs_newN<Transform>(numBones)), mInvBindPoses(bs_newN<Matrix4>(numBones))
 		, mBoneInfo(bs_newN<SkeletonBoneInfo>(numBones))
 	{
 		for(UINT32 i = 0; i < numBones; i++)
@@ -170,9 +170,6 @@ namespace bs
 			localPose.scales[i] = Vector3::ONE;
 		}
 
-		bool* hasAnimCurve = bs_stack_alloc<bool>(mNumBones);
-		bs_zero_out(hasAnimCurve, mNumBones);
-
 		// Note: For a possible performance improvement consider keeping an array of only active (non-disabled) bones and
 		// just iterate over them without mask checks. Possibly also a list of active curve mappings to avoid those checks
 		// as well.
@@ -209,6 +206,8 @@ namespace bs
 					if (!mask.isEnabled(k))
 						continue;
 
+					bool hasAnimCurve = false;
+
 					const AnimationCurveMapping& mapping = state.boneToCurveMapping[k];
 					UINT32 curveIdx = mapping.position;
 					if (curveIdx != (UINT32)-1)
@@ -217,7 +216,7 @@ namespace bs
 						localPose.positions[k] += curve.evaluate(state.time, state.positionCaches[curveIdx], state.loop) * normWeight;
 
 						localPose.hasOverride[k] = false;
-						hasAnimCurve[k] = true;
+						hasAnimCurve = true;
 					}
 
 					curveIdx = mapping.scale;
@@ -227,7 +226,7 @@ namespace bs
 						localPose.scales[k] *= curve.evaluate(state.time, state.scaleCaches[curveIdx], state.loop) * normWeight;
 
 						localPose.hasOverride[k] = false;
-						hasAnimCurve[k] = true;
+						hasAnimCurve = true;
 					}
 
 					if (layer.additive)
@@ -246,7 +245,7 @@ namespace bs
 
 							localPose.rotations[k] *= value;
 							localPose.hasOverride[k] = false;
-							hasAnimCurve[k] = true;
+							hasAnimCurve = true;
 						}
 					}
 					else
@@ -262,9 +261,18 @@ namespace bs
 
 							localPose.rotations[k] += value;
 							localPose.hasOverride[k] = false;
-							hasAnimCurve[k] = true;
+							hasAnimCurve = true;
 						}
 					}
+
+					// If no animation for this bone, apply its default transform
+					// (Even if this bone isn't used for the skin, the child bones need to inherit this transform)
+					if(!hasAnimCurve)
+					{
+						localPose.positions[k] = mBoneTransforms[k].getPosition();
+						localPose.rotations[k] = mBoneTransforms[k].getRotation();
+						localPose.scales[k] = mBoneTransforms[k].getScale();
+					}
 				}
 			}
 		}
@@ -291,15 +299,6 @@ namespace bs
 			pose[i] = Matrix4::TRS(localPose.positions[i], localPose.rotations[i], localPose.scales[i]);
 		}
 
-		// Apply default local tranform to non-animated bones (so that any potential child bones are transformed properly)
-		for(UINT32 i = 0; i < mNumBones; i++)
-		{
-			if(hasAnimCurve[i])
-				continue;
-
-			pose[i] = mBoneTransforms[i] * pose[i];
-		}
-
 		// Calculate global poses
 		// Note: For a possible performance improvement consider sorting bones in such order so that parents (and overrides)
 		// always come before children, we no isGlobal check is needed.
@@ -329,7 +328,6 @@ namespace bs
 			pose[i] = pose[i] * mInvBindPoses[i];
 
 		bs_stack_free(isGlobal);
-		bs_stack_free(hasAnimCurve);
 	}
 
 	UINT32 Skeleton::getRootBoneIndex() const

Source/BansheeCore/Animation/BsSkeleton.h

@@ -8,6 +8,7 @@
 #include "Math/BsVector3.h"
 #include "Math/BsQuaternion.h"
 #include "Animation/BsCurveCache.h"
+#include "Scene/BsTransform.h"
 
 namespace bs
 {
@@ -34,7 +35,7 @@ namespace bs
 		String name; /**< Unique name of the bone. */
 		UINT32 parent; /**< Index of the parent bone, if any. -1 if root bone. */
 
-		Matrix4 localTfrm; /**< Local transform of the bone, relative to other bones in the hierarchy. */
+		Transform localTfrm; /**< Local transform of the bone, relative to other bones in the hierarchy. */
 		Matrix4 invBindPose; /**< Inverse bind pose which transforms vertices from their bind pose into local space. */
 	};
 
@@ -177,7 +178,7 @@ namespace bs
 		Skeleton(BONE_DESC* bones, UINT32 numBones);
 
 		UINT32 mNumBones = 0;
-		Matrix4* mBoneTransforms = nullptr;
+		Transform* mBoneTransforms = nullptr;
 		Matrix4* mInvBindPoses = nullptr;
 		SkeletonBoneInfo* mBoneInfo = nullptr;
 

Source/BansheeCore/Components/BsCAnimation.cpp

@@ -373,7 +373,7 @@ namespace bs
 		newMapping.bone = bone;
 
 		mMappingInfos.push_back(newMapping);
-		mInternal->mapCurveToSceneObject(bone->getName(), newMapping.sceneObject);
+		mInternal->mapCurveToSceneObject(bone->getBoneName(), newMapping.sceneObject);
 	}
 
 	void CAnimation::_removeBone(const HBone& bone)

Source/BansheeCore/Private/RTTI/BsSkeletonRTTI.h

@@ -38,15 +38,15 @@ namespace bs
 			obj->mBoneInfo = bs_newN<SkeletonBoneInfo>(size);
 		}
 
-		Matrix4& getBoneTransform(Skeleton* obj, UINT32 idx) { return obj->mBoneTransforms[idx]; }
-		void setBoneTransform(Skeleton* obj, UINT32 idx, Matrix4& value) { obj->mBoneTransforms[idx] = value; }
+		Transform& getBoneTransform(Skeleton* obj, UINT32 idx) { return obj->mBoneTransforms[idx]; }
+		void setBoneTransform(Skeleton* obj, UINT32 idx, Transform& value) { obj->mBoneTransforms[idx] = value; }
 
 		void setNumBoneTransforms(Skeleton* obj, UINT32 size)
 		{
 			obj->mNumBones = size;
 			
 			assert(obj->mBoneTransforms == nullptr);
-			obj->mBoneTransforms = bs_newN<Matrix4>(size);
+			obj->mBoneTransforms = bs_newN<Transform>(size);
 		}
 
 		UINT32 getNumBones(Skeleton* obj) { return obj->mNumBones; }
@@ -57,7 +57,7 @@ namespace bs
 				&SkeletonRTTI::setBindPose, &SkeletonRTTI::setNumBindPoses);
 			addPlainArrayField("boneInfo", 1, &SkeletonRTTI::getBoneInfo, &SkeletonRTTI::getNumBones,
 				&SkeletonRTTI::setBoneInfo, &SkeletonRTTI::setNumBoneInfos);
-			addPlainArrayField("boneTransforms", 2, &SkeletonRTTI::getBoneTransform, &SkeletonRTTI::getNumBones,
+			addReflectableArrayField("boneTransforms", 3, &SkeletonRTTI::getBoneTransform, &SkeletonRTTI::getNumBones,
 				&SkeletonRTTI::setBoneTransform, &SkeletonRTTI::setNumBoneTransforms);
 		}
 

Source/BansheeCore/Scene/BsTransform.cpp

@@ -9,6 +9,10 @@ namespace bs
 		: mPosition(Vector3::ZERO), mRotation(Quaternion::IDENTITY), mScale(Vector3::ONE)
 	{ }
 
+	Transform::Transform(const Vector3& position, const Quaternion& rotation, const Vector3& scale)
+		: mPosition(position), mRotation(rotation), mScale(scale)
+	{ }
+
 	Matrix4 Transform::getMatrix() const
 	{
 		return Matrix4::TRS(mPosition, mRotation, mScale);

Source/BansheeCore/Scene/BsTransform.h

@@ -21,6 +21,7 @@ namespace bs
 	{
 	public:
 		Transform();
+		Transform(const Vector3& position, const Quaternion& rotation, const Vector3& scale);
 
 		/**	Sets the local position of the object. */
 		void setPosition(const Vector3& position) { mPosition = position; }
@@ -67,10 +68,10 @@ namespace bs
 		 */
 		void setWorldScale(const Vector3& scale, const Transform& parent);
 
-		/** Builds the transform matrix from current properties. */
+		/** Builds the transform matrix from current translation, rotation and scale properties. */
 		Matrix4 getMatrix() const;
 
-		/** Builds the inverse transform matrix from current properties. */
+		/** Builds the inverse transform matrix from current translation, rotation and scale properties. */
 		Matrix4 getInvMatrix() const;
 
 		/** 

Source/BansheeFBXImporter/BsFBXImportData.h

@@ -10,6 +10,7 @@
 #include "Math/BsQuaternion.h"
 #include "Animation/BsAnimationCurve.h"
 #include "RenderAPI/BsSubMesh.h"
+#include "Scene/BsTransform.h"
 
 namespace bs
 {
@@ -39,7 +40,7 @@ namespace bs
 		~FBXImportNode();
 
 		Matrix4 geomTransform;
-		Matrix4 localTransform;
+		Transform localTransform;
 		Matrix4 worldTransform;
 		String name;
 		FbxNode* fbxNode;
@@ -71,7 +72,7 @@ namespace bs
 	struct FBXBone
 	{
 		FBXImportNode* node;
-		Matrix4 localTfrm;
+		Transform localTfrm;
 		Matrix4 bindPose;
 	};
 

Source/BansheeFBXImporter/BsFBXImporter.cpp

@@ -302,7 +302,7 @@ namespace bs
 				BONE_DESC& bone = allBones.back();
 
 				bone.name = "MultiMeshRoot";
-				bone.localTfrm = Matrix4::IDENTITY;
+				bone.localTfrm = Transform();
 				bone.invBindPose = Matrix4::IDENTITY;
 				bone.parent = (UINT32)-1;
 
@@ -622,16 +622,16 @@ namespace bs
 
 		node->name = fbxNode->GetNameWithoutNameSpacePrefix().Buffer();
 		node->fbxNode = fbxNode;
-		node->localTransform = Matrix4::TRS(translation, rotation, scale);
+		node->localTransform = Transform(translation, rotation, scale);
 
 		if (parent != nullptr)
 		{
-			node->worldTransform = parent->worldTransform * node->localTransform;
+			node->worldTransform = parent->worldTransform * node->localTransform.getMatrix();
 
 			parent->children.push_back(node);
 		}
 		else
-			node->worldTransform = node->localTransform;
+			node->worldTransform = node->localTransform.getMatrix();
 
 		// Geometry transform is applied to geometry (mesh data) only, it is not inherited by children, so we store it
 		// separately
@@ -1586,9 +1586,11 @@ namespace bs
 
 			FbxAMatrix invLinkTransform = linkTransform.Inverse();
 			bone.localTfrm = bone.node->localTransform;
-			bone.localTfrm[0][3] *= scene.scaleFactor;
-			bone.localTfrm[1][3] *= scene.scaleFactor;
-			bone.localTfrm[2][3] *= scene.scaleFactor;
+
+			Vector3 localTfrmPos = bone.localTfrm.getPosition();
+			localTfrmPos *= scene.scaleFactor;
+
+			bone.localTfrm.setPosition(localTfrmPos);
 
 			bone.bindPose = FBXToNativeType(invLinkTransform);
 

Source/BansheeUtility/Math/BsMatrix4.h

@@ -305,9 +305,10 @@ namespace bs
 		 * Decompose a Matrix4 to translation, rotation and scale.
 		 *
 		 * @note
-		 * Matrix must consist only of translation, rotation and uniform scale transformations,
-		 * otherwise accurate results are not guaranteed. Applying non-uniform scale guarantees
-		 * results will not be accurate.
+		 * This method is unable to decompose all types of matrices, in particular these are the limitations:
+		 *  - Only translation, rotation and scale transforms are supported
+		 *  - Plain TRS matrices (that aren't composed with other matrices) can always be decomposed
+		 *  - Composed TRS matrices can be decomposed ONLY if the scaling factor is uniform
 		 */
 		void decomposition(Vector3& position, Quaternion& rotation, Vector3& scale) const;
 

Source/MBansheeEditor/Inspectors/BoneInspector.cs

@@ -44,6 +44,7 @@ namespace BansheeEditor
                     {
                         if (bone.Name == boneNames[i])
                         {
+                            selectedBoneName = bone.Name;
                             boneField.Index = i;
                             break;
                         }