Repository: bitshifter/glam-rs
Pull request #282 (2022-04-09 16:37:45)

Ensure a positive dot product in `Quat::slerp`
Fixes https://github.com/bitshifter/glam-rs/issues/276.

This also has the benefit of no longer requiring the user to check
 the dot product and negating one of the quaternions if the dot
product is negative.




Patch:
diff --git a/src/core/scalar/quaternion.rs b/src/core/scalar/quaternion.rs
index a483ed1..60d1259 100644
--- a/src/core/scalar/quaternion.rs
+++ b/src/core/scalar/quaternion.rs
@@ -26,13 +26,24 @@ impl<T: FloatEx> Quaternion<T> for XYZW<T> {
     }
 
     #[inline]
-    fn slerp(self, end: Self, s: T) -> Self {
+    fn slerp(self, mut end: Self, s: T) -> Self {
         // http://number-none.com/product/Understanding%20Slerp,%20Then%20Not%20Using%20It/
 
         glam_assert!(FloatVector4::is_normalized(self));
         glam_assert!(FloatVector4::is_normalized(end));
 
-        let dot = self.dot(end);
+        let mut dot = self.dot(end);
+
+        // Note that a rotation can be represented by two quaternions: `q` and
+        // `-q`. The slerp path between `q` and `end` will be different from the
+        // path between `-q` and `end`. One path will take the long way around and
+        // one will take the short way. In order to correct for this, the `dot`
+        // product between `self` and `end` should be positive. If the `dot`
+        // product is negative, slerp between `self` and `-end`.
+        if dot < T::ZERO {
+            end = end.mul_scalar(T::NEG_ONE);
+            dot = -dot;
+        }
 
         if dot > T::from_f32(0.9995) {
             // assumes lerp returns a normalized quaternion
diff --git a/src/core/sse2/quaternion.rs b/src/core/sse2/quaternion.rs
index 6ffea3b..196eb24 100644
--- a/src/core/sse2/quaternion.rs
+++ b/src/core/sse2/quaternion.rs
@@ -40,14 +40,26 @@ impl Quaternion<f32> for __m128 {
     }
 
     #[inline]
-    fn slerp(self, end: Self, s: f32) -> Self {
+    fn slerp(self, mut end: Self, s: f32) -> Self {
         // http://number-none.com/product/Understanding%20Slerp,%20Then%20Not%20Using%20It/
         glam_assert!(FloatVector4::is_normalized(self));
         glam_assert!(FloatVector4::is_normalized(end));
 
         const DOT_THRESHOLD: f32 = 0.9995;
 
-        let dot = Vector4::dot(self, end);
+        let mut dot = Vector4::dot(self, end);
+
+        // Note that a rotation can be represented by two quaternions: `q` and
+        // `-q`. The slerp path between `q` and `end` will be different from the
+        // path between `-q` and `end`. One path will take the long way around and
+        // one will take the short way. In order to correct for this, the `dot`
+        // product between `self` and `end` should be positive. If the `dot`
+        // product is negative, slerp between `self` and `-end`.
+        if dot < 0.0 {
+            const PS_NEGATIVE_ONE: __m128 = const_f32x4!([-1.0; 4]);
+            end = unsafe { _mm_mul_ps(PS_NEGATIVE_ONE, end) };
+            dot = -dot;
+        }
 
         if dot > DOT_THRESHOLD {
             // assumes lerp returns a normalized quaternion
diff --git a/src/quat.rs b/src/quat.rs
index 135c942..e0fa1b2 100644
--- a/src/quat.rs
+++ b/src/quat.rs
@@ -437,13 +437,6 @@ macro_rules! impl_quat_methods {
         /// When `s` is `0.0`, the result will be equal to `self`.  When `s`
         /// is `1.0`, the result will be equal to `end`.
         ///
-        /// Note that a rotation can be represented by two quaternions: `q` and
-        /// `-q`. The slerp path between `q` and `end` will be different from the
-        /// path between `-q` and `end`. One path will take the long way around and
-        /// one will take the short way. In order to correct for this, the `dot`
-        /// product between `self` and `end` should be positive. If the `dot`
-        /// product is negative, slerp between `-self` and `end`.
-        ///
         /// # Panics
         ///
         /// Will panic if `self` or `end` are not normalized when `glam_assert` is enabled.


Test patch:
diff --git a/tests/quat.rs b/tests/quat.rs
index 831bf0f..b76a1c0 100644
--- a/tests/quat.rs
+++ b/tests/quat.rs
@@ -280,15 +280,45 @@ macro_rules! impl_quat_tests {
             while s <= 1.0 {
                 let q0 = $quat::from_rotation_y(deg(0.0));
                 let q1 = $quat::from_rotation_y(deg(90.0));
-                assert_approx_eq!(
-                    $quat::from_rotation_y(deg(s * 90.0)),
-                    q0.slerp(q1, s),
-                    1.0e-3
-                );
+                let result = q0.slerp(q1, s);
+                assert_approx_eq!($quat::from_rotation_y(deg(s * 90.0)), result, 1.0e-3);
+                assert!(result.is_normalized());
                 s += step;
             }
         });
 
+        glam_test!(test_slerp_tau, {
+            let q1 = $quat::IDENTITY;
+            let q2 = $quat::from_rotation_x(core::$t::consts::TAU);
+            let s = q1.slerp(q2, 1.);
+            assert!(s.is_finite());
+            assert!(s.is_normalized());
+        });
+
+        glam_test!(test_slerp_negative_tau, {
+            let q1 = $quat::IDENTITY;
+            let q2 = $quat::from_rotation_x(-core::$t::consts::TAU);
+            let s = q1.slerp(q2, 1.);
+            assert!(s.is_finite());
+            assert!(s.is_normalized());
+        });
+
+        glam_test!(test_slerp_pi, {
+            let q1 = $quat::IDENTITY;
+            let q2 = $quat::from_rotation_x(core::$t::consts::PI);
+            let s = q1.slerp(q2, 1.);
+            assert!(s.is_finite());
+            assert!(s.is_normalized());
+        });
+
+        glam_test!(test_slerp_negative_pi, {
+            let q1 = $quat::IDENTITY;
+            let q2 = $quat::from_rotation_x(-core::$t::consts::PI);
+            let s = q1.slerp(q2, 1.);
+            assert!(s.is_finite());
+            assert!(s.is_normalized());
+        });
+
         glam_test!(test_fmt, {
             let a = $quat::IDENTITY;
             assert_eq!(