#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 20, 2024 · 1002863 · 1002863
1 parent 6b0d20e
commit 1002863
Show file tree

Hide file tree

Showing 34 changed files with 94 additions and 58 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/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
@@ -117,21 +117,24 @@ namespace {
   } 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.
@@ -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);

diff --git a/test/animation/runtime/blending_job_tests.cc b/test/animation/runtime/blending_job_tests.cc
@@ -27,6 +27,7 @@
 
 #include "gtest/gtest.h"
 #include "ozz/animation/runtime/blending_job.h"
+#include "ozz/base/log.h"
 #include "ozz/base/maths/gtest_math_helper.h"
 #include "ozz/base/maths/soa_transform.h"
 
@@ -782,15 +783,8 @@ TEST(AdditiveWeight, BlendingJob) {
 }
 
 TEST(AdditiveRotationFromNonIdentity, BlendingJob) {
-  using ozz::math::GetW;
-  using ozz::math::GetX;
-  using ozz::math::GetY;
-  using ozz::math::GetZ;
-
-  const ozz::math::SoaTransform identity = ozz::math::SoaTransform::identity();
-
   // Initialize rest pose.
-  ozz::math::SoaTransform rest_poses[1] = {identity};
+  ozz::math::SoaTransform rest_poses[1] = {ozz::math::SoaTransform::identity()};
   rest_poses[0].rotation = ozz::math::SoaQuaternion::Load(
       ozz::math::simd_float4::Load(0.f, .70710677f, .382683432f, 0.f),
       ozz::math::simd_float4::Load(0.f, 0.f, 0.f, .70710677f),
@@ -803,14 +797,15 @@ TEST(AdditiveRotationFromNonIdentity, BlendingJob) {
       ozz::math::simd_float4::Load(0.f, 0.f, .70710677f, 0.f),
       ozz::math::simd_float4::Load(0.f, 0.f, 0.f, 0.f),
       ozz::math::simd_float4::Load(.70710677f, 1.f, -.70710677f, .9238795f));
-  ozz::math::SoaTransform input_transforms[1] = {identity};
+  ozz::math::SoaTransform input_transforms[1] = {
+      ozz::math::SoaTransform::identity()};
   input_transforms[0].rotation = input_half_rotation * input_half_rotation;
 
+  // Equivalent to additive blending with -1 weight.
   ozz::math::SoaQuaternion expected_rotation =
-      rest_poses[0].rotation *
-      Conjugate(input_half_rotation * input_half_rotation);
-  constexpr float weights[]{-1.0f, -0.5f, 0.0f, 0.5f, 1.0f};
-  for (float weight : weights) {
+      rest_poses[0].rotation * Conjugate(input_transforms[0].rotation);
+
+  for (float weight : {-1.0f, -0.5f, 0.0f, 0.5f, 1.0f}) {
     BlendingJob::Layer layers[1];
     layers[0].transform = input_transforms;
 
@@ -824,11 +819,23 @@ TEST(AdditiveRotationFromNonIdentity, BlendingJob) {
     layers[0].weight = weight;
     EXPECT_TRUE(job.Run());
 
-    ozz::math::SimdFloat4 dot =
-        ozz::math::Dot(expected_rotation, output_transforms[0].rotation);
-    ozz::math::SimdFloat4 distance =
-        ozz::math::simd_float4::one() - (dot * dot);
-    EXPECT_SIMDFLOAT_EQ_EST(distance, 0.0f, 0.0f, 0.0f, 0.0f);
+    ozz::log::Log() << "Additive weight: " << weight << std::endl;
+
+    for (size_t i = 0; i < 4; ++i) {
+      ozz::log::Log() << i << ": "
+                      << ozz::math::GetX((&output_transforms[0].rotation.x)[i])
+                      << ", "
+                      << ozz::math::GetY((&output_transforms[0].rotation.x)[i])
+                      << ", "
+                      << ozz::math::GetZ((&output_transforms[0].rotation.x)[i])
+                      << ", "
+                      << ozz::math::GetW((&output_transforms[0].rotation.x)[i])
+                      << std::endl;
+    }
+
+    const auto same = Compare(expected_rotation, output_transforms[0].rotation,
+                              ozz::math::simd_float4::Load1(.99f));
+    EXPECT_TRUE(ozz::math::AreAllTrue(same));
 
     expected_rotation = expected_rotation * input_half_rotation;
   }

diff --git a/test/base/maths/soa_quaternion_tests.cc b/test/base/maths/soa_quaternion_tests.cc
@@ -25,16 +25,15 @@
 //                                                                            //
 //----------------------------------------------------------------------------//
 
-#include "ozz/base/maths/soa_quaternion.h"
-
 #include "gtest/gtest.h"
-
 #include "ozz/base/gtest_helper.h"
 #include "ozz/base/maths/gtest_math_helper.h"
+#include "ozz/base/maths/soa_quaternion.h"
 
-using ozz::math::SoaQuaternion;
-using ozz::math::SoaFloat4;
+using ozz::math::SimdInt4;
 using ozz::math::SoaFloat3;
+using ozz::math::SoaFloat4;
+using ozz::math::SoaQuaternion;
 
 TEST(SoaQuaternionConstant, ozz_soa_math) {
   EXPECT_SOAQUATERNION_EQ(SoaQuaternion::identity(), 0.f, 0.f, 0.f, 0.f, 0.f,
@@ -166,4 +165,18 @@ TEST(SoaQuaternionArithmetic, ozz_soa_math) {
                               0.f, 0.f, .70710677f, .70710677f, .70710677f,
                               .97047764f);
   EXPECT_TRUE(ozz::math::AreAllTrue(IsNormalizedEst(nlerp_est_m)));
+
+  const SimdInt4 same_aa = Compare(a, a, ozz::math::simd_float4::Load1(.99f));
+  EXPECT_TRUE(ozz::math::AreAllTrue(same_aa));
+
+  const SimdInt4 same_ana =
+      Compare(a, Normalize(a), ozz::math::simd_float4::Load1(.99f));
+  EXPECT_TRUE(ozz::math::AreAllTrue(same_ana));
+
+  const SimdInt4 same_anega =
+      Compare(a, -a, ozz::math::simd_float4::Load1(.99f));
+  EXPECT_TRUE(ozz::math::AreAllTrue(same_anega));
+
+  const SimdInt4 same_ab = Compare(a, b, ozz::math::simd_float4::Load1(.99f));
+  EXPECT_SIMDINT_EQ(same_ab, 0, 0, 0xffffffff, 0);
 }