#152 Fixes additive animation blending issues

Fixes additive animation blending issues due to wrong quaternion multiplication order in additive animation builder and blending job. Note that animations needs rebuild to benefit from the fix.
guillaumeblanc · Feb 21, 2024 · e1a871d · e1a871d
1 parent 6b0d20e
commit e1a871d
Show file tree

Hide file tree

Showing 35 changed files with 178 additions and 182 deletions.
diff --git a/AUTHORS.md b/AUTHORS.md
@@ -15,3 +15,5 @@ The following authors have all licensed their contributions to ozz-animation und
 - Mikołaj Siedlarek <mikolaj@siedlarek.net>
 - Paul Gruenbacher <pgruenbacher@gmail.com>
 - Christophe Meyer <christophe.meyer.pro@gmail.com>
+- Blaž Tomažič <blaz.tomazic@gmail.com>
+- Jan Krassnigg <jan@krassnigg.de>
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,6 +1,9 @@
 Next release
 ----------------------
 
+* Library
+  - [offline/animation] #152 Fixes additive animation blending issues due to wrong quaternion multiplication order in additive animation builder and blending job. Note that animations needs rebuild to benefit from the fix.
+
 * Build pipeline
   - Adds CI for WebAssembly.
 

diff --git a/include/ozz/base/maths/gtest_math_helper.h b/include/ozz/base/maths/gtest_math_helper.h
@@ -29,7 +29,7 @@
 #define OZZ_OZZ_BASE_MATHS_GTEST_MATH_HELPER_H_
 
 static const float kFloatNearTolerance = 1e-5f;
-static const float kFloatNearEstTolerance = 1e-3f;
+static const float kFloatNearEstTolerance = 1e-2f;
 
 // Implements "float near" test as a function. Avoids overloading compiler
 // optimizer when too much EXPECT_NEAR are used in a single compilation unit.

diff --git a/include/ozz/base/maths/soa_quaternion.h b/include/ozz/base/maths/soa_quaternion.h
@@ -72,6 +72,16 @@ OZZ_INLINE SimdFloat4 Dot(const SoaQuaternion& _a, const SoaQuaternion& _b) {
   return _a.x * _b.x + _a.y * _b.y + _a.z * _b.z + _a.w * _b.w;
 }
 
+// Returns true if the angle between _a and _b is less than _tolerance.
+OZZ_INLINE SimdInt4 Compare(const math::SoaQuaternion& _a,
+                            const math::SoaQuaternion& _b,
+                            const math::SimdFloat4 _cos_half_tolerance) {
+  // Computes w component of a-1 * b.
+  const SimdFloat4 cos_half_angle =
+      _a.x * _b.x + _a.y * _b.y + _a.z * _b.z + _a.w * _b.w;
+  return math::CmpGe(math::Abs(cos_half_angle), _cos_half_tolerance);
+}
+
 // Returns the normalized SoaQuaternion _q.
 OZZ_INLINE SoaQuaternion Normalize(const SoaQuaternion& _q) {
   const SimdFloat4 len2 = _q.x * _q.x + _q.y * _q.y + _q.z * _q.z + _q.w * _q.w;

diff --git a/media/bin/astro_max_animation.ozz b/media/bin/astro_max_animation.ozz
diff --git a/media/bin/astro_max_skeleton.ozz b/media/bin/astro_max_skeleton.ozz
diff --git a/media/bin/astro_maya_animation.ozz b/media/bin/astro_maya_animation.ozz
diff --git a/media/bin/astro_maya_skeleton.ozz b/media/bin/astro_maya_skeleton.ozz
diff --git a/media/bin/baked_animation.ozz b/media/bin/baked_animation.ozz
diff --git a/media/bin/pab_atlas_raw.ozz b/media/bin/pab_atlas_raw.ozz
diff --git a/media/bin/pab_crackhead.ozz b/media/bin/pab_crackhead.ozz
diff --git a/media/bin/pab_crackhead_additive.ozz b/media/bin/pab_crackhead_additive.ozz
diff --git a/media/bin/pab_crossarms.ozz b/media/bin/pab_crossarms.ozz
diff --git a/media/bin/pab_curl_additive.ozz b/media/bin/pab_curl_additive.ozz
diff --git a/media/bin/pab_jog.ozz b/media/bin/pab_jog.ozz
diff --git a/media/bin/pab_run.ozz b/media/bin/pab_run.ozz
diff --git a/media/bin/pab_skeleton.ozz b/media/bin/pab_skeleton.ozz
diff --git a/media/bin/pab_splay_additive.ozz b/media/bin/pab_splay_additive.ozz
diff --git a/media/bin/pab_walk.ozz b/media/bin/pab_walk.ozz
diff --git a/media/bin/robot_animation.ozz b/media/bin/robot_animation.ozz
diff --git a/media/bin/robot_skeleton.ozz b/media/bin/robot_skeleton.ozz
diff --git a/media/bin/robot_track_grasp.ozz b/media/bin/robot_track_grasp.ozz
diff --git a/media/bin/ruby_animation.ozz b/media/bin/ruby_animation.ozz
diff --git a/media/bin/ruby_mesh.ozz b/media/bin/ruby_mesh.ozz
diff --git a/media/bin/ruby_skeleton.ozz b/media/bin/ruby_skeleton.ozz
diff --git a/media/bin/seymour_animation.ozz b/media/bin/seymour_animation.ozz
diff --git a/media/bin/versioning/animation_v6_be.ozz b/media/bin/versioning/animation_v6_be.ozz
diff --git a/media/bin/versioning/animation_v6_le.ozz b/media/bin/versioning/animation_v6_le.ozz
diff --git a/media/bin/versioning/raw_animation_v3_be.ozz b/media/bin/versioning/raw_animation_v3_be.ozz
diff --git a/media/bin/versioning/raw_animation_v3_le.ozz b/media/bin/versioning/raw_animation_v3_le.ozz
diff --git a/src/animation/offline/additive_animation_builder.cc b/src/animation/offline/additive_animation_builder.cc
@@ -63,7 +63,7 @@ math::Float3 MakeDeltaTranslation(const math::Float3& _reference,
 
 math::Quaternion MakeDeltaRotation(const math::Quaternion& _reference,
                                    const math::Quaternion& _value) {
-  return _value * Conjugate(_reference);
+  return Conjugate(_reference) * _value;
 }
 
 math::Float3 MakeDeltaScale(const math::Float3& _reference,

diff --git a/src/animation/runtime/blending_job.cc b/src/animation/runtime/blending_job.cc
@@ -93,45 +93,48 @@ bool BlendingJob::Validate() const {
 namespace {
 
 // Macro that defines the process of blending the 1st pass.
-#define OZZ_BLEND_1ST_PASS(_in, _simd_weight, _out)     \
-  do {                                                  \
-    _out->translation = _in.translation * _simd_weight; \
-    _out->rotation = _in.rotation * _simd_weight;       \
-    _out->scale = _in.scale * _simd_weight;             \
+#define OZZ_BLEND_1ST_PASS(_in, _simd_weight, _out)    \
+  do {                                                 \
+    _out.translation = _in.translation * _simd_weight; \
+    _out.rotation = _in.rotation * _simd_weight;       \
+    _out.scale = _in.scale * _simd_weight;             \
   } while (void(0), 0)
 
 // Macro that defines the process of blending any pass but the first.
-#define OZZ_BLEND_N_PASS(_in, _simd_weight, _out)                              \
-  do {                                                                         \
-    /* Blends translation. */                                                  \
-    _out->translation = _out->translation + _in.translation * _simd_weight;    \
-    /* Blends rotations, negates opposed quaternions to be sure to choose*/    \
-    /* the shortest path between the two.*/                                    \
-    const math::SimdInt4 sign = math::Sign(Dot(_out->rotation, _in.rotation)); \
-    const math::SoaQuaternion rotation = {                                     \
-        math::Xor(_in.rotation.x, sign), math::Xor(_in.rotation.y, sign),      \
-        math::Xor(_in.rotation.z, sign), math::Xor(_in.rotation.w, sign)};     \
-    _out->rotation = _out->rotation + rotation * _simd_weight;                 \
-    /* Blends scales.*/                                                        \
-    _out->scale = _out->scale + _in.scale * _simd_weight;                      \
+#define OZZ_BLEND_N_PASS(_in, _simd_weight, _out)                             \
+  do {                                                                        \
+    /* Blends translation. */                                                 \
+    _out.translation = _out.translation + _in.translation * _simd_weight;     \
+    /* Blends rotations, negates opposed quaternions to be sure to choose*/   \
+    /* the shortest path between the two.*/                                   \
+    const math::SimdInt4 sign = math::Sign(Dot(_out.rotation, _in.rotation)); \
+    const math::SoaQuaternion rotation = {                                    \
+        math::Xor(_in.rotation.x, sign), math::Xor(_in.rotation.y, sign),     \
+        math::Xor(_in.rotation.z, sign), math::Xor(_in.rotation.w, sign)};    \
+    _out.rotation = _out.rotation + rotation * _simd_weight;                  \
+    /* Blends scales.*/                                                       \
+    _out.scale = _out.scale + _in.scale * _simd_weight;                       \
   } while (void(0), 0)
 
 // Macro that defines the process of adding a pass.
-#define OZZ_ADD_PASS(_in, _simd_weight, _out)                                \
-  do {                                                                       \
-    _out.translation = _out.translation + _in.translation * _simd_weight;    \
-    /* Interpolate quaternion between identity and src.rotation.*/           \
-    /* Quaternion sign is fixed up, so that lerp takes the shortest path.*/  \
-    const math::SimdInt4 sign = math::Sign(_in.rotation.w);                  \
-    const math::SoaQuaternion rotation = {                                   \
-        math::Xor(_in.rotation.x, sign), math::Xor(_in.rotation.y, sign),    \
-        math::Xor(_in.rotation.z, sign), math::Xor(_in.rotation.w, sign)};   \
-    const math::SoaQuaternion interp_quat = {                                \
-        rotation.x * _simd_weight, rotation.y * _simd_weight,                \
-        rotation.z * _simd_weight, (rotation.w - one) * _simd_weight + one}; \
-    _out.rotation = NormalizeEst(interp_quat) * _out.rotation;               \
-    _out.scale =                                                             \
-        _out.scale * (one_minus_weight_f3 + (_in.scale * _simd_weight));     \
+#define OZZ_ADD_PASS(_in, _simd_weight, _out)                               \
+  do {                                                                      \
+    _out.translation = _out.translation + _in.translation * _simd_weight;   \
+    /* Interpolate quaternion between identity and src.rotation.*/          \
+    /* Quaternion sign is fixed up, so that lerp takes the shortest path.*/ \
+    const math::SimdInt4 sign = math::Sign(_in.rotation.w);                 \
+    const math::SoaQuaternion interp_quat = {                               \
+        math::Xor(_in.rotation.x, sign) * _simd_weight,                     \
+        math::Xor(_in.rotation.y, sign) * _simd_weight,                     \
+        math::Xor(_in.rotation.z, sign) * _simd_weight,                     \
+        (math::Xor(_in.rotation.w, sign) - one) * _simd_weight + one};      \
+    _out.rotation = _out.rotation * NormalizeEst(interp_quat);              \
+    _out.scale.x = _out.scale.x *                                           \
+                   math::MAdd(_in.scale.x, _simd_weight, one_minus_weight); \
+    _out.scale.y = _out.scale.y *                                           \
+                   math::MAdd(_in.scale.y, _simd_weight, one_minus_weight); \
+    _out.scale.z = _out.scale.z *                                           \
+                   math::MAdd(_in.scale.z, _simd_weight, one_minus_weight); \
   } while (void(0), 0)
 
 // Macro that defines the process of subtracting a pass.
@@ -141,19 +144,21 @@ namespace {
     /* Interpolate quaternion between identity and src.rotation.*/             \
     /* Quaternion sign is fixed up, so that lerp takes the shortest path.*/    \
     const math::SimdInt4 sign = math::Sign(_in.rotation.w);                    \
-    const math::SoaQuaternion rotation = {                                     \
-        math::Xor(_in.rotation.x, sign), math::Xor(_in.rotation.y, sign),      \
-        math::Xor(_in.rotation.z, sign), math::Xor(_in.rotation.w, sign)};     \
     const math::SoaQuaternion interp_quat = {                                  \
-        rotation.x * _simd_weight, rotation.y * _simd_weight,                  \
-        rotation.z * _simd_weight, (rotation.w - one) * _simd_weight + one};   \
-    _out.rotation = Conjugate(NormalizeEst(interp_quat)) * _out.rotation;      \
-    const math::SoaFloat3 rcp_scale = {                                        \
-        math::RcpEst(math::MAdd(_in.scale.x, _simd_weight, one_minus_weight)), \
-        math::RcpEst(math::MAdd(_in.scale.y, _simd_weight, one_minus_weight)), \
-        math::RcpEst(                                                          \
-            math::MAdd(_in.scale.z, _simd_weight, one_minus_weight))};         \
-    _out.scale = _out.scale * rcp_scale;                                       \
+        math::Xor(_in.rotation.x, sign) * _simd_weight,                        \
+        math::Xor(_in.rotation.y, sign) * _simd_weight,                        \
+        math::Xor(_in.rotation.z, sign) * _simd_weight,                        \
+        (math::Xor(_in.rotation.w, sign) - one) * _simd_weight + one};         \
+    _out.rotation = _out.rotation * Conjugate(NormalizeEst(interp_quat));      \
+    _out.scale.x =                                                             \
+        _out.scale.x *                                                         \
+        math::RcpEst(math::MAdd(_in.scale.x, _simd_weight, one_minus_weight)); \
+    _out.scale.y =                                                             \
+        _out.scale.y *                                                         \
+        math::RcpEst(math::MAdd(_in.scale.y, _simd_weight, one_minus_weight)); \
+    _out.scale.z =                                                             \
+        _out.scale.z *                                                         \
+        math::RcpEst(math::MAdd(_in.scale.z, _simd_weight, one_minus_weight)); \
   } while (void(0), 0)
 
 // Defines parameters that are passed through blending stages.
@@ -230,7 +235,7 @@ void BlendLayers(ProcessArgs* _args) {
       if (_args->num_passes == 0) {
         for (size_t i = 0; i < _args->num_soa_joints; ++i) {
           const math::SoaTransform& src = layer.transform[i];
-          math::SoaTransform* dest = _args->job.output.begin() + i;
+          math::SoaTransform& dest = _args->job.output[i];
           const math::SimdFloat4 weight =
               layer_weight * math::Max0(layer.joint_weights[i]);
           _args->accumulated_weights[i] = weight;
@@ -239,7 +244,7 @@ void BlendLayers(ProcessArgs* _args) {
       } else {
         for (size_t i = 0; i < _args->num_soa_joints; ++i) {
           const math::SoaTransform& src = layer.transform[i];
-          math::SoaTransform* dest = _args->job.output.begin() + i;
+          math::SoaTransform& dest = _args->job.output[i];
           const math::SimdFloat4 weight =
               layer_weight * math::Max0(layer.joint_weights[i]);
           _args->accumulated_weights[i] =
@@ -252,14 +257,14 @@ void BlendLayers(ProcessArgs* _args) {
       if (_args->num_passes == 0) {
         for (size_t i = 0; i < _args->num_soa_joints; ++i) {
           const math::SoaTransform& src = layer.transform[i];
-          math::SoaTransform* dest = _args->job.output.begin() + i;
+          math::SoaTransform& dest = _args->job.output[i];
           _args->accumulated_weights[i] = layer_weight;
           OZZ_BLEND_1ST_PASS(src, layer_weight, dest);
         }
       } else {
         for (size_t i = 0; i < _args->num_soa_joints; ++i) {
           const math::SoaTransform& src = layer.transform[i];
-          math::SoaTransform* dest = _args->job.output.begin() + i;
+          math::SoaTransform& dest = _args->job.output[i];
           _args->accumulated_weights[i] =
               _args->accumulated_weights[i] + layer_weight;
           OZZ_BLEND_N_PASS(src, layer_weight, dest);
@@ -300,7 +305,7 @@ void BlendRestPose(ProcessArgs* _args) {
 
         for (size_t i = 0; i < _args->num_soa_joints; ++i) {
           const math::SoaTransform& src = _args->job.rest_pose[i];
-          math::SoaTransform* dest = _args->job.output.begin() + i;
+          math::SoaTransform& dest = _args->job.output[i];
           OZZ_BLEND_N_PASS(src, simd_bp_weight, dest);
         }
       }
@@ -316,7 +321,7 @@ void BlendRestPose(ProcessArgs* _args) {
 
     for (size_t i = 0; i < _args->num_soa_joints; ++i) {
       const math::SoaTransform& src = _args->job.rest_pose[i];
-      math::SoaTransform* dest = _args->job.output.begin() + i;
+      math::SoaTransform& dest = _args->job.output[i];
       const math::SimdFloat4 bp_weight =
           math::Max0(threshold - _args->accumulated_weights[i]);
       _args->accumulated_weights[i] =
@@ -384,15 +389,11 @@ void AddLayers(ProcessArgs* _args) {
           const math::SimdFloat4 weight =
               layer_weight * math::Max0(layer.joint_weights[i]);
           const math::SimdFloat4 one_minus_weight = one - weight;
-          const math::SoaFloat3 one_minus_weight_f3 = {
-              one_minus_weight, one_minus_weight, one_minus_weight};
           OZZ_ADD_PASS(src, weight, dest);
         }
       } else {
         // This is a full layer.
         const math::SimdFloat4 one_minus_weight = one - layer_weight;
-        const math::SoaFloat3 one_minus_weight_f3 = {
-            one_minus_weight, one_minus_weight, one_minus_weight};
 
         for (size_t i = 0; i < _args->num_soa_joints; ++i) {
           const math::SoaTransform& src = layer.transform[i];

diff --git a/test/animation/offline/additive_animation_builder_tests.cc b/test/animation/offline/additive_animation_builder_tests.cc
@@ -88,7 +88,7 @@ TEST(Build, AdditiveAnimationBuilder) {
 
   // First track is empty
   {
-      // input.tracks[0]
+    // input.tracks[0]
   }
 
   // 2nd track
@@ -202,7 +202,7 @@ TEST(BuildRefPose, AdditiveAnimationBuilder) {
 
   // First track is empty
   {
-      // input.tracks[0]
+    // input.tracks[0]
   }
 
   // 2nd track
@@ -284,7 +284,7 @@ TEST(BuildRefPose, AdditiveAnimationBuilder) {
           output.tracks[1].rotations;
       EXPECT_EQ(rotations.size(), 1u);
       EXPECT_FLOAT_EQ(rotations[0].time, 0.f);
-      EXPECT_QUATERNION_EQ(rotations[0].value, .5f, .5f, -.5f, .5f);
+      EXPECT_QUATERNION_EQ(rotations[0].value, .5f, -.5f, -.5f, .5f);
       const RawAnimation::JointTrack::Scales& scales = output.tracks[1].scales;
       EXPECT_EQ(scales.size(), 1u);
       EXPECT_FLOAT_EQ(scales[0].time, 0.f);