#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 · 2af18fc · 2af18fc
1 parent 6b0d20e
commit 2af18fc
Show file tree

Hide file tree

Showing 34 changed files with 48 additions and 29 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
@@ -129,7 +129,7 @@ namespace {
     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.rotation = _out.rotation * NormalizeEst(interp_quat);               \
     _out.scale =                                                             \
         _out.scale * (one_minus_weight_f3 + (_in.scale * _simd_weight));     \
   } while (void(0), 0)
@@ -147,7 +147,7 @@ namespace {
     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;      \
+    _out.rotation = _out.rotation * Conjugate(NormalizeEst(interp_quat));      \
     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)), \

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
@@ -782,15 +782,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 +796,14 @@ 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;
 
   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 +817,9 @@ 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);
+    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);
 }