Merge pull request #4075 from bghgary/bone-fix

Fix issues with glTF animations that scale bones

dist/preview release/what's new.md

@@ -136,6 +136,7 @@
 - Reflection and refraction no longer apply a toLinear conversion twice when applying image processing as a post process - [#4060](https://github.com/BabylonJS/Babylon.js/issues/4060) ([trevordev](https://github.com/trevordev))
 - Fix ember.js compatibility in ```PostProcessRenderEffect``` ([sebavan](https://github.com/sebavan))
 - Fix ember.js compatibility in ```BloomEffect``` and ```Camera``` ([kaysabelle](https://github.com/kaysabelle))
+- Fix bug with glTF animation when animating bone scale. ([bghgary](https://github.com/bghgary)]
 
 ## Breaking changes
 
@@ -144,3 +145,5 @@
 - glTF 2.0 loader now creates a mesh for each primitive instead of merging the primitives together into one mesh. If a mesh only has one primitive, the behavior is the same as before. This change only affects meshes that have multiple primitives. ([bghgary](https://github.com/bghgary)]
 - Engine's onCanvasPointerOutObservable will now return a PointerEvent instead of the Engine. ([trevordev](https://github.com/trevordev))
 - Removed public references to default rendering pipeline's internal post process ([trevordev](https://github.com/trevordev))
+- Rename `setScale`, `getScale`, `getScaleToRef` of the `Bone` class to `setAdditionalScale`, `getAdditionalScale`, `getAdditionalScaleToRef`. ([bghgary](https://github.com/bghgary)]
+- Bone `scaling` now replaces the scale of the local matrix instead of adding scale. ([bghgary](https://github.com/bghgary)]

loaders/src/glTF/2.0/babylon.glTFLoader.ts

@@ -920,13 +920,12 @@ module BABYLON.GLTF2 {
                             outTangent: key.outTangent ? key.outTangent[targetIndex] : undefined
                         })));
 
-                        const multiTarget = new AnimationMultiTarget();
+                        const morphTargets = new Array<any>();
                         this._forEachPrimitive(targetNode, babylonMesh => {
-                            const morphTarget = babylonMesh.morphTargetManager!.getTarget(targetIndex);
-                            multiTarget.subTargets.push(morphTarget);
+                            morphTargets.push(babylonMesh.morphTargetManager!.getTarget(targetIndex));
                         });
 
-                        babylonAnimationGroup.addTargetedAnimation(babylonAnimation, multiTarget);
+                        babylonAnimationGroup.addTargetedAnimation(babylonAnimation, morphTargets);
                     }
                 }
                 else {
@@ -935,11 +934,7 @@ module BABYLON.GLTF2 {
                     babylonAnimation.setKeys(keys);
 
                     if (targetNode._babylonAnimationTargets) {
-                        const multiTarget = new AnimationMultiTarget();
-                        for (const target of targetNode._babylonAnimationTargets) {
-                            multiTarget.subTargets.push(target);
-                        }
-                        babylonAnimationGroup.addTargetedAnimation(babylonAnimation, multiTarget);
+                        babylonAnimationGroup.addTargetedAnimation(babylonAnimation, targetNode._babylonAnimationTargets);
                     }
                 }
             });

loaders/src/glTF/2.0/babylon.glTFLoaderUtilities.ts

@@ -14,32 +14,4 @@ module BABYLON.GLTF2 {
             }
         }
     }
-
-    export class AnimationMultiTarget {
-        public subTargets = new Array<any>();
-
-        public set position(value: Vector3) {
-            for (const subTarget of this.subTargets) {
-                subTarget.position = value;
-            }
-        }
-
-        public set rotationQuaternion(value: Quaternion) {
-            for (const subTarget of this.subTargets) {
-                subTarget.rotationQuaternion = value;
-            }
-        }
-
-        public set scaling(value: Vector3) {
-            for (const subTarget of this.subTargets) {
-                subTarget.scaling = value;
-            }
-        }
-
-        public set influence(value: number) {
-            for (const subTarget of this.subTargets) {
-                subTarget.influence = value;
-            }
-        }
-    }
 }

src/Animations/babylon.runtimeAnimation.ts

@@ -242,6 +242,17 @@
          * @param weight defines the weight to apply to this value
          */
         public setValue(currentValue: any, weight = 1.0): void {
+            if (this._target instanceof Array) {
+                for (const target of this._target) {
+                    this._setValue(target, currentValue, weight);
+                }
+            }
+            else {
+                this._setValue(this._target, currentValue, weight);
+            }
+        }
+
+        private _setValue(target: any, currentValue: any, weight = 1.0): void {
             // Set value
             var path: any;
             var destination: any;
@@ -249,7 +260,7 @@
             let targetPropertyPath = this._animation.targetPropertyPath
 
             if (targetPropertyPath.length > 1) {
-                var property = this._target[targetPropertyPath[0]];
+                var property = target[targetPropertyPath[0]];
 
                 for (var index = 1; index < targetPropertyPath.length - 1; index++) {
                     property = property[targetPropertyPath[index]];
@@ -259,7 +270,7 @@
                 destination = property;
             } else {
                 path = targetPropertyPath[0];
-                destination = this._target;
+                destination = target;
             }
 
             this._targetPath = path;
@@ -267,8 +278,8 @@
             this._weight = weight;
 
             // Blending
-            let enableBlending = this._target && this._target.animationPropertiesOverride ? this._target.animationPropertiesOverride.enableBlending : this._animation.enableBlending;
-            let blendingSpeed = this._target && this._target.animationPropertiesOverride ? this._target.animationPropertiesOverride.blendingSpeed : this._animation.blendingSpeed;
+            let enableBlending = target && target.animationPropertiesOverride ? target.animationPropertiesOverride.enableBlending : this._animation.enableBlending;
+            let blendingSpeed = target && target.animationPropertiesOverride ? target.animationPropertiesOverride.blendingSpeed : this._animation.blendingSpeed;
             
             if (enableBlending && this._blendingFactor <= 1.0) {
                 if (!this._originalBlendValue) {
@@ -337,8 +348,8 @@
                 destination[path] = this._currentValue;
             }
 
-            if (this._target.markAsDirty) {
-                this._target.markAsDirty(this._animation.targetProperty);
+            if (target.markAsDirty) {
+                target.markAsDirty(this._animation.targetProperty);
             }
         }
 

src/Bones/babylon.bone.ts

@@ -31,19 +31,15 @@
             return this._localMatrix;
         }
 
-        set _matrix(val: Matrix) {
-            if (this._localMatrix) {
-                this._localMatrix.copyFrom(val);
-            } else {
-                this._localMatrix = val;
-            }
+        set _matrix(value: Matrix) {
+            this._localMatrix.copyFrom(value);
         }
 
         constructor(public name: string, skeleton: Skeleton, parentBone: Nullable<Bone> = null, localMatrix: Nullable<Matrix> = null,
             restPose: Nullable<Matrix> = null, baseMatrix: Nullable<Matrix> = null, index: Nullable<number> = null) {
             super(name, skeleton.getScene());
             this._skeleton = skeleton;
-            this._localMatrix = localMatrix ? localMatrix : Matrix.Identity();
+            this._localMatrix = localMatrix ? localMatrix.clone() : Matrix.Identity();
             this._restPose = restPose ? restPose : this._localMatrix.clone();
             this._baseMatrix = baseMatrix ? baseMatrix : this._localMatrix.clone();
             this._index = index;
@@ -85,6 +81,8 @@
             if (updateDifferenceMatrix) {
                 this._updateDifferenceMatrix();
             }
+
+            this.markAsDirty();
         }
 
         public getLocalMatrix(): Matrix {
@@ -141,13 +139,17 @@
         }
 
         public get scaling(): Vector3 {
-            return this.getScale();
+            let value = Vector3.One();
+            this._localMatrix.decompose(value, undefined, undefined);
+            return value;
         }
 
         public set scaling(newScaling: Vector3) {
-            this.setScale(newScaling.x, newScaling.y, newScaling.z);
+            this._localMatrix.decompose(undefined, Bone._tmpQuat, Bone._tmpVecs[0]);
+            Matrix.ComposeToRef(newScaling, Bone._tmpQuat, Bone._tmpVecs[0], this._localMatrix);
+            this.markAsDirty();
         }
-        
+
         /**
          * Gets the animation properties override
          */
@@ -157,14 +159,14 @@
 
         // Methods
         public updateMatrix(matrix: Matrix, updateDifferenceMatrix = true): void {
-            this._baseMatrix = matrix.clone();
-            this._localMatrix = matrix.clone();
-
-            this._skeleton._markAsDirty();
+            this._baseMatrix.copyFrom(matrix);
+            this._localMatrix.copyFrom(matrix);
 
             if (updateDifferenceMatrix) {
                 this._updateDifferenceMatrix();
             }
+
+            this.markAsDirty();
         }
 
         public _updateDifferenceMatrix(rootMatrix?: Matrix): void {
@@ -367,13 +369,13 @@
         }
 
         /**
-         * Set the scale of the bone on the x, y and z axes.
-         * @param x The scale of the bone on the x axis.
-         * @param x The scale of the bone on the y axis.
-         * @param z The scale of the bone on the z axis.
+         * Adds an additional scale to the bone on the x, y and z axes.
+         * @param x The additional scale of the bone on the x axis.
+         * @param y The additional scale of the bone on the y axis.
+         * @param z The additional scale of the bone on the z axis.
          * @param scaleChildren Set this to true if children of the bone should be scaled.
          */
-        public setScale(x: number, y: number, z: number, scaleChildren = false): void {
+        public setAdditionalScale(x: number, y: number, z: number, scaleChildren = false): void {
 
             if (this.animations[0] && !this.animations[0].hasRunningRuntimeAnimations) {
                 if (!scaleChildren) {
@@ -406,7 +408,7 @@
             origLocMatInv.invert();
 
             var scaleMat = Bone._tmpMats[2];
-            Matrix.FromValuesToRef(x, 0, 0, 0, 0, y, 0, 0, 0, 0, z, 0, 0, 0, 0, 1, scaleMat);
+            Matrix.ScalingToRef(x, y, z, scaleMat);
             this._scaleMatrix.multiplyToRef(scaleMat, this._scaleMatrix);
             this._scaleVector.x *= x;
             this._scaleVector.y *= y;
@@ -460,15 +462,15 @@
          */
         public setYawPitchRoll(yaw: number, pitch: number, roll: number, space = Space.LOCAL, mesh?: AbstractMesh): void {
 
-            var rotMat = Bone._tmpMats[0];
-            Matrix.RotationYawPitchRollToRef(yaw, pitch, roll, rotMat);
-
-            var rotMatInv = Bone._tmpMats[1];
+            var rotMatInv = Bone._tmpMats[0];
+            if (!this._getNegativeRotationToRef(rotMatInv, space, mesh)) {
+                return;
+            }
 
-            this._getNegativeRotationToRef(rotMatInv, space, mesh);
+            var rotMat = Bone._tmpMats[1];
+            Matrix.RotationYawPitchRollToRef(yaw, pitch, roll, rotMat);
 
             rotMatInv.multiplyToRef(rotMat, rotMat);
-
             this._rotateWithMatrix(rotMat, space, mesh);
 
         }
@@ -502,11 +504,13 @@
          */
         public setAxisAngle(axis: Vector3, angle: number, space = Space.LOCAL, mesh?: AbstractMesh): void {
 
-            var rotMat = Bone._tmpMats[0];
-            Matrix.RotationAxisToRef(axis, angle, rotMat);
-            var rotMatInv = Bone._tmpMats[1];
+            var rotMatInv = Bone._tmpMats[0];
+            if (!this._getNegativeRotationToRef(rotMatInv, space, mesh)) {
+                return;
+            }
 
-            this._getNegativeRotationToRef(rotMatInv, space, mesh);
+            var rotMat = Bone._tmpMats[1];
+            Matrix.RotationAxisToRef(axis, angle, rotMat);
 
             rotMatInv.multiplyToRef(rotMat, rotMat);
             this._rotateWithMatrix(rotMat, space, mesh);
@@ -534,8 +538,9 @@
         public setRotationQuaternion(quat: Quaternion, space = Space.LOCAL, mesh?: AbstractMesh): void {
 
             var rotMatInv = Bone._tmpMats[0];
-
-            this._getNegativeRotationToRef(rotMatInv, space, mesh);
+            if (!this._getNegativeRotationToRef(rotMatInv, space, mesh)) {
+                return;
+            }
 
             var rotMat = Bone._tmpMats[1];
             Matrix.FromQuaternionToRef(quat, rotMat);
@@ -555,8 +560,9 @@
         public setRotationMatrix(rotMat: Matrix, space = Space.LOCAL, mesh?: AbstractMesh): void {
 
             var rotMatInv = Bone._tmpMats[0];
-
-            this._getNegativeRotationToRef(rotMatInv, space, mesh);
+            if (!this._getNegativeRotationToRef(rotMatInv, space, mesh)) {
+                return;
+            }
 
             var rotMat2 = Bone._tmpMats[1];
             rotMat2.copyFrom(rotMat);
@@ -616,7 +622,7 @@
 
         }
 
-        private _getNegativeRotationToRef(rotMatInv: Matrix, space = Space.LOCAL, mesh?: AbstractMesh): void {
+        private _getNegativeRotationToRef(rotMatInv: Matrix, space = Space.LOCAL, mesh?: AbstractMesh): boolean {
 
             if (space == Space.WORLD) {
                 var scaleMatrix = Bone._tmpMats[2];
@@ -631,11 +637,23 @@
                 }
 
                 rotMatInv.invert();
+                if (isNaN(rotMatInv.m[0])) {
+                    // Matrix failed to invert.
+                    // This can happen if scale is zero for example.
+                    return false;
+                }
+
                 scaleMatrix.m[0] *= this._scalingDeterminant;
                 rotMatInv.multiplyToRef(scaleMatrix, rotMatInv);
             } else {
                 rotMatInv.copyFrom(this.getLocalMatrix());
                 rotMatInv.invert();
+                if (isNaN(rotMatInv.m[0])) {
+                    // Matrix failed to invert.
+                    // This can happen if scale is zero for example.
+                    return false;
+                }
+
                 var scaleMatrix = Bone._tmpMats[2];
                 scaleMatrix.copyFrom(this._scaleMatrix);
 
@@ -651,23 +669,25 @@
                 rotMatInv.multiplyToRef(scaleMatrix, rotMatInv);
             }
 
+            return true;
+
         }
 
         /**
-         * Get the scale of the bone
-         * @returns the scale of the bone
+         * Get the additional scale of the bone
+         * @returns the additional scale of the bone
          */
-        public getScale(): Vector3 {
+        public getAdditionalScale(): Vector3 {
 
             return this._scaleVector.clone();
 
         }
 
         /**
-         * Copy the scale of the bone to a vector3.
-         * @param result The vector3 to copy the scale to
+         * Copy the additional scale of the bone to a vector3.
+         * @param result The vector3 to copy the additional scale to
          */
-        public getScaleToRef(result: Vector3): void {
+        public getAdditionalScaleToRef(result: Vector3): void {
 
             result.copyFrom(this._scaleVector);
 
@@ -909,7 +929,7 @@
 
             if (space == Space.LOCAL) {
 
-                this.getLocalMatrix().decompose(Bone._tmpVecs[0], result, Bone._tmpVecs[1]);
+                this.getLocalMatrix().decompose(undefined, result, undefined);
 
             } else {
 
@@ -926,7 +946,7 @@
                 mat.m[1] *= this._scalingDeterminant;
                 mat.m[2] *= this._scalingDeterminant;
 
-                mat.decompose(Bone._tmpVecs[0], result, Bone._tmpVecs[1]);
+                mat.decompose(undefined, result, undefined);
 
             }
         }

src/Math/babylon.math.ts

@@ -4204,13 +4204,14 @@
          * @param translation defines the translation vector3 given as a reference to update
          * @returns true if operation was successful
          */
-        public decompose(scale: Vector3, rotation?: Quaternion, translation?: Vector3): boolean {
+        public decompose(scale?: Vector3, rotation?: Quaternion, translation?: Vector3): boolean {
             if (translation) {
                 translation.x = this.m[12];
                 translation.y = this.m[13];
                 translation.z = this.m[14];
             }
 
+            scale = scale || MathTmp.Vector3[0];
             scale.x = Math.sqrt(this.m[0] * this.m[0] + this.m[1] * this.m[1] + this.m[2] * this.m[2]);
             scale.y = Math.sqrt(this.m[4] * this.m[4] + this.m[5] * this.m[5] + this.m[6] * this.m[6]);
             scale.z = Math.sqrt(this.m[8] * this.m[8] + this.m[9] * this.m[9] + this.m[10] * this.m[10]);
@@ -4219,15 +4220,18 @@
                 scale.y *= -1;
             }
 
-            if (rotation) {
-                if (scale.x === 0 || scale.y === 0 || scale.z === 0) {
+            if (scale.x === 0 || scale.y === 0 || scale.z === 0) {
+                if (rotation) {
                     rotation.x = 0;
                     rotation.y = 0;
                     rotation.z = 0;
                     rotation.w = 1;
-                    return false;
                 }
 
+                return false;
+            }
+
+            if (rotation) {
                 Matrix.FromValuesToRef(
                     this.m[0] / scale.x, this.m[1] / scale.x, this.m[2] / scale.x, 0,
                     this.m[4] / scale.y, this.m[5] / scale.y, this.m[6] / scale.y, 0,
@@ -4388,19 +4392,24 @@
         public getRotationMatrixToRef(result: Matrix): Matrix {
             var m = this.m;
 
-            var xs = m[0] * m[1] * m[2] * m[3] < 0 ? -1 : 1;
-            var ys = m[4] * m[5] * m[6] * m[7] < 0 ? -1 : 1;
-            var zs = m[8] * m[9] * m[10] * m[11] < 0 ? -1 : 1;
+            var sx = Math.sqrt(m[0] * m[0] + m[1] * m[1] + m[2] * m[2]);
+            var sy = Math.sqrt(m[4] * m[4] + m[5] * m[5] + m[6] * m[6]);
+            var sz = Math.sqrt(m[8] * m[8] + m[9] * m[9] + m[10] * m[10]);
 
-            var sx = xs * Math.sqrt(m[0] * m[0] + m[1] * m[1] + m[2] * m[2]);
-            var sy = ys * Math.sqrt(m[4] * m[4] + m[5] * m[5] + m[6] * m[6]);
-            var sz = zs * Math.sqrt(m[8] * m[8] + m[9] * m[9] + m[10] * m[10]);
+            if (this.determinant() <= 0) {
+                sy *= -1;
+            }
 
-            Matrix.FromValuesToRef(
-                m[0] / sx, m[1] / sx, m[2] / sx, 0,
-                m[4] / sy, m[5] / sy, m[6] / sy, 0,
-                m[8] / sz, m[9] / sz, m[10] / sz, 0,
-                0, 0, 0, 1, result);
+            if (sx === 0 || sy === 0 || sz === 0) {
+                Matrix.IdentityToRef(result);
+            }
+            else {
+                Matrix.FromValuesToRef(
+                    m[0] / sx, m[1] / sx, m[2] / sx, 0,
+                    m[4] / sy, m[5] / sy, m[6] / sy, 0,
+                    m[8] / sz, m[9] / sz, m[10] / sz, 0,
+                    0, 0, 0, 1, result);
+            }
 
             return this;
         }