From 67603be6ed435947f9bc24ead3cdc9df9be9f0ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crozet=20S=C3=A9bastien?= Date: Fri, 18 Dec 2020 11:38:33 +0100 Subject: [PATCH 1/2] Fix the PartialEq impl for quaternions. The double-covering property should only be taken into account for `UnitQuaternion` instead of `Quaternion` itself. --- src/base/unit.rs | 27 +++++++++++++- src/geometry/quaternion.rs | 54 ++++++++++----------------- src/geometry/quaternion_conversion.rs | 10 ++--- src/geometry/unit_complex.rs | 11 ++++++ tests/geometry/quaternion.rs | 1 - 5 files changed, 60 insertions(+), 43 deletions(-) diff --git a/src/base/unit.rs b/src/base/unit.rs index 20ca4603..2483307a 100644 --- a/src/base/unit.rs +++ b/src/base/unit.rs @@ -11,7 +11,8 @@ use abomonation::Abomonation; use crate::allocator::Allocator; use crate::base::DefaultAllocator; -use crate::{Dim, MatrixMN, RealField, Scalar, SimdComplexField, SimdRealField}; +use crate::storage::Storage; +use crate::{Dim, Matrix, MatrixMN, RealField, Scalar, SimdComplexField, SimdRealField}; /// A wrapper that ensures the underlying algebraic entity has a unit norm. /// @@ -24,7 +25,7 @@ use crate::{Dim, MatrixMN, RealField, Scalar, SimdComplexField, SimdRealField}; /// and [`UnitQuaternion`](crate::UnitQuaternion); both built on top of `Unit`. If you are interested /// in their documentation, read their dedicated pages directly. #[repr(transparent)] -#[derive(Eq, PartialEq, Clone, Hash, Debug, Copy)] +#[derive(Clone, Hash, Debug, Copy)] pub struct Unit { pub(crate) value: T, } @@ -64,6 +65,28 @@ impl Abomonation for Unit { } } +impl PartialEq for Unit> +where + N: Scalar + PartialEq, + R: Dim, + C: Dim, + S: Storage, +{ + #[inline] + fn eq(&self, rhs: &Self) -> bool { + self.value.eq(&rhs.value) + } +} + +impl Eq for Unit> +where + N: Scalar + Eq, + R: Dim, + C: Dim, + S: Storage, +{ +} + /// Trait implemented by entities scan be be normalized and put in an `Unit` struct. pub trait Normed { /// The type of the norm. diff --git a/src/geometry/quaternion.rs b/src/geometry/quaternion.rs index e860a8b8..c8fd80be 100755 --- a/src/geometry/quaternion.rs +++ b/src/geometry/quaternion.rs @@ -13,7 +13,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "abomonation-serialize")] use abomonation::Abomonation; -use simba::scalar::RealField; +use simba::scalar::{ClosedNeg, RealField}; use simba::simd::{SimdBool, SimdOption, SimdRealField, SimdValue}; use crate::base::dimension::{U1, U3, U4}; @@ -23,17 +23,18 @@ use crate::base::{ }; use crate::geometry::{Point3, Rotation}; +use std::ops::Neg; /// A quaternion. See the type alias `UnitQuaternion = Unit` for a quaternion /// that may be used as a rotation. #[repr(C)] -#[derive(Debug)] -pub struct Quaternion { +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub struct Quaternion { /// This quaternion as a 4D vector of coordinates in the `[ x, y, z, w ]` storage order. pub coords: Vector4, } -impl Default for Quaternion { +impl Default for Quaternion { fn default() -> Self { Quaternion { coords: Vector4::zeros(), @@ -42,7 +43,7 @@ impl Default for Quaternion { } #[cfg(feature = "abomonation-serialize")] -impl Abomonation for Quaternion +impl Abomonation for Quaternion where Vector4: Abomonation, { @@ -59,36 +60,8 @@ where } } -impl Eq for Quaternion where N::Element: SimdRealField {} - -impl PartialEq for Quaternion -where - N::Element: SimdRealField, -{ - fn eq(&self, rhs: &Self) -> bool { - self.coords == rhs.coords || - // Account for the double-covering of S², i.e. q = -q - self.as_vector().iter().zip(rhs.as_vector().iter()).all(|(a, b)| *a == -*b) - } -} - -impl hash::Hash for Quaternion { - fn hash(&self, state: &mut H) { - self.coords.hash(state) - } -} - -impl Copy for Quaternion {} - -impl Clone for Quaternion { - #[inline] - fn clone(&self) -> Self { - Self::from(self.coords.clone()) - } -} - #[cfg(feature = "serde-serialize")] -impl Serialize for Quaternion +impl Serialize for Quaternion where Owned: Serialize, { @@ -101,7 +74,7 @@ where } #[cfg(feature = "serde-serialize")] -impl<'a, N: SimdRealField> Deserialize<'a> for Quaternion +impl<'a, N: Scalar> Deserialize<'a> for Quaternion where Owned: Deserialize<'a>, { @@ -980,6 +953,17 @@ impl fmt::Display for Quaternion { /// A unit quaternions. May be used to represent a rotation. pub type UnitQuaternion = Unit>; +impl PartialEq for UnitQuaternion { + #[inline] + fn eq(&self, rhs: &Self) -> bool { + self.coords == rhs.coords || + // Account for the double-covering of S², i.e. q = -q + self.coords.iter().zip(rhs.coords.iter()).all(|(a, b)| *a == -b.inlined_clone()) + } +} + +impl Eq for UnitQuaternion {} + impl Normed for Quaternion { type Norm = N::SimdRealField; diff --git a/src/geometry/quaternion_conversion.rs b/src/geometry/quaternion_conversion.rs index 330c97e4..7f276e99 100644 --- a/src/geometry/quaternion_conversion.rs +++ b/src/geometry/quaternion_conversion.rs @@ -184,14 +184,14 @@ impl> SubsetOf> for Un } #[cfg(feature = "mint")] -impl From> for Quaternion { +impl From> for Quaternion { fn from(q: mint::Quaternion) -> Self { Self::new(q.s, q.v.x, q.v.y, q.v.z) } } #[cfg(feature = "mint")] -impl Into> for Quaternion { +impl Into> for Quaternion { fn into(self) -> mint::Quaternion { mint::Quaternion { v: mint::Vector3 { @@ -205,7 +205,7 @@ impl Into> for Quaternion { } #[cfg(feature = "mint")] -impl Into> for UnitQuaternion { +impl Into> for UnitQuaternion { fn into(self) -> mint::Quaternion { mint::Quaternion { v: mint::Vector3 { @@ -258,14 +258,14 @@ where } } -impl From> for Quaternion { +impl From> for Quaternion { #[inline] fn from(coords: Vector4) -> Self { Self { coords } } } -impl From<[N; 4]> for Quaternion { +impl From<[N; 4]> for Quaternion { #[inline] fn from(coords: [N; 4]) -> Self { Self { diff --git a/src/geometry/unit_complex.rs b/src/geometry/unit_complex.rs index 40148454..43ef7324 100755 --- a/src/geometry/unit_complex.rs +++ b/src/geometry/unit_complex.rs @@ -4,8 +4,10 @@ use std::fmt; use crate::base::{Matrix2, Matrix3, Normed, Unit, Vector1, Vector2}; use crate::geometry::{Point2, Rotation2}; +use crate::Scalar; use simba::scalar::RealField; use simba::simd::SimdRealField; +use std::cmp::{Eq, PartialEq}; /// A 2D rotation represented as a complex number with magnitude 1. /// @@ -29,6 +31,15 @@ use simba::simd::SimdRealField; /// * [Conversion to a matrix `to_rotation_matrix`, `to_homogeneous`…](#conversion-to-a-matrix) pub type UnitComplex = Unit>; +impl PartialEq for UnitComplex { + #[inline] + fn eq(&self, rhs: &Self) -> bool { + (**self).eq(&**rhs) + } +} + +impl Eq for UnitComplex {} + impl Normed for Complex { type Norm = N::SimdRealField; diff --git a/tests/geometry/quaternion.rs b/tests/geometry/quaternion.rs index 77456ca0..5ff20a0e 100644 --- a/tests/geometry/quaternion.rs +++ b/tests/geometry/quaternion.rs @@ -114,7 +114,6 @@ quickcheck!( */ fn unit_quaternion_double_covering(q: UnitQuaternion) -> bool { let mq = UnitQuaternion::new_unchecked(-q.into_inner()); - mq == q && mq.angle() == q.angle() && mq.axis() == q.axis() } From ed74ff7c99a4b96ff2f76e98a506b7fdc489cb20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crozet=20S=C3=A9bastien?= Date: Fri, 18 Dec 2020 12:06:27 +0100 Subject: [PATCH 2/2] Simplify trait bounds for quaternion indexing. --- src/geometry/quaternion_construction.rs | 2 +- src/geometry/quaternion_conversion.rs | 18 +++++++++--------- src/geometry/quaternion_ops.rs | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/geometry/quaternion_construction.rs b/src/geometry/quaternion_construction.rs index 63f00636..7d0cca8c 100644 --- a/src/geometry/quaternion_construction.rs +++ b/src/geometry/quaternion_construction.rs @@ -19,7 +19,7 @@ use crate::{Scalar, SimdRealField}; use crate::geometry::{Quaternion, Rotation3, UnitQuaternion}; -impl Quaternion { +impl Quaternion { /// Creates a quaternion from a 4D vector. The quaternion scalar part corresponds to the `w` /// vector component. #[inline] diff --git a/src/geometry/quaternion_conversion.rs b/src/geometry/quaternion_conversion.rs index 7f276e99..b57cc52b 100644 --- a/src/geometry/quaternion_conversion.rs +++ b/src/geometry/quaternion_conversion.rs @@ -195,25 +195,25 @@ impl Into> for Quaternion { fn into(self) -> mint::Quaternion { mint::Quaternion { v: mint::Vector3 { - x: self[0], - y: self[1], - z: self[2], + x: self[0].inlined_clone(), + y: self[1].inlined_clone(), + z: self[2].inlined_clone(), }, - s: self[3], + s: self[3].inlined_clone(), } } } #[cfg(feature = "mint")] -impl Into> for UnitQuaternion { +impl Into> for UnitQuaternion { fn into(self) -> mint::Quaternion { mint::Quaternion { v: mint::Vector3 { - x: self[0], - y: self[1], - z: self[2], + x: self[0].inlined_clone(), + y: self[1].inlined_clone(), + z: self[2].inlined_clone(), }, - s: self[3], + s: self[3].inlined_clone(), } } } diff --git a/src/geometry/quaternion_ops.rs b/src/geometry/quaternion_ops.rs index 2961362e..f9205874 100644 --- a/src/geometry/quaternion_ops.rs +++ b/src/geometry/quaternion_ops.rs @@ -57,12 +57,12 @@ use std::ops::{ use crate::base::allocator::Allocator; use crate::base::dimension::{U1, U3, U4}; use crate::base::storage::Storage; -use crate::base::{DefaultAllocator, Unit, Vector, Vector3}; +use crate::base::{DefaultAllocator, Scalar, Unit, Vector, Vector3}; use crate::SimdRealField; use crate::geometry::{Point3, Quaternion, Rotation, UnitQuaternion}; -impl Index for Quaternion { +impl Index for Quaternion { type Output = N; #[inline] @@ -71,7 +71,7 @@ impl Index for Quaternion { } } -impl IndexMut for Quaternion { +impl IndexMut for Quaternion { #[inline] fn index_mut(&mut self, i: usize) -> &mut N { &mut self.coords[i]