Skip to content

Commit

Permalink
Merge pull request #814 from dimforge/quaternion_partial_eq_fix
Browse files Browse the repository at this point in the history
Fix the PartialEq impl for quaternions.
  • Loading branch information
sebcrozet committed Dec 18, 2020
2 parents 44be05d + ed74ff7 commit 88145b7
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 55 deletions.
27 changes: 25 additions & 2 deletions src/base/unit.rs
Expand Up @@ -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.
///
Expand All @@ -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<T> {
pub(crate) value: T,
}
Expand Down Expand Up @@ -64,6 +65,28 @@ impl<T: Abomonation> Abomonation for Unit<T> {
}
}

impl<N, R, C, S> PartialEq for Unit<Matrix<N, R, C, S>>
where
N: Scalar + PartialEq,
R: Dim,
C: Dim,
S: Storage<N, R, C>,
{
#[inline]
fn eq(&self, rhs: &Self) -> bool {
self.value.eq(&rhs.value)
}
}

impl<N, R, C, S> Eq for Unit<Matrix<N, R, C, S>>
where
N: Scalar + Eq,
R: Dim,
C: Dim,
S: Storage<N, R, C>,
{
}

/// Trait implemented by entities scan be be normalized and put in an `Unit` struct.
pub trait Normed {
/// The type of the norm.
Expand Down
54 changes: 19 additions & 35 deletions src/geometry/quaternion.rs
Expand Up @@ -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};
Expand All @@ -23,17 +23,18 @@ use crate::base::{
};

use crate::geometry::{Point3, Rotation};
use std::ops::Neg;

/// A quaternion. See the type alias `UnitQuaternion = Unit<Quaternion>` for a quaternion
/// that may be used as a rotation.
#[repr(C)]
#[derive(Debug)]
pub struct Quaternion<N: Scalar + SimdValue> {
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub struct Quaternion<N: Scalar> {
/// This quaternion as a 4D vector of coordinates in the `[ x, y, z, w ]` storage order.
pub coords: Vector4<N>,
}

impl<N: RealField> Default for Quaternion<N> {
impl<N: Scalar + Zero> Default for Quaternion<N> {
fn default() -> Self {
Quaternion {
coords: Vector4::zeros(),
Expand All @@ -42,7 +43,7 @@ impl<N: RealField> Default for Quaternion<N> {
}

#[cfg(feature = "abomonation-serialize")]
impl<N: SimdRealField> Abomonation for Quaternion<N>
impl<N: Scalar> Abomonation for Quaternion<N>
where
Vector4<N>: Abomonation,
{
Expand All @@ -59,36 +60,8 @@ where
}
}

impl<N: SimdRealField + Eq> Eq for Quaternion<N> where N::Element: SimdRealField {}

impl<N: SimdRealField> PartialEq for Quaternion<N>
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<N: SimdRealField + hash::Hash> hash::Hash for Quaternion<N> {
fn hash<H: hash::Hasher>(&self, state: &mut H) {
self.coords.hash(state)
}
}

impl<N: Scalar + Copy + SimdValue> Copy for Quaternion<N> {}

impl<N: Scalar + SimdValue> Clone for Quaternion<N> {
#[inline]
fn clone(&self) -> Self {
Self::from(self.coords.clone())
}
}

#[cfg(feature = "serde-serialize")]
impl<N: SimdRealField> Serialize for Quaternion<N>
impl<N: Scalar> Serialize for Quaternion<N>
where
Owned<N, U4>: Serialize,
{
Expand All @@ -101,7 +74,7 @@ where
}

#[cfg(feature = "serde-serialize")]
impl<'a, N: SimdRealField> Deserialize<'a> for Quaternion<N>
impl<'a, N: Scalar> Deserialize<'a> for Quaternion<N>
where
Owned<N, U4>: Deserialize<'a>,
{
Expand Down Expand Up @@ -980,6 +953,17 @@ impl<N: RealField + fmt::Display> fmt::Display for Quaternion<N> {
/// A unit quaternions. May be used to represent a rotation.
pub type UnitQuaternion<N> = Unit<Quaternion<N>>;

impl<N: Scalar + ClosedNeg + PartialEq> PartialEq for UnitQuaternion<N> {
#[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<N: Scalar + ClosedNeg + Eq> Eq for UnitQuaternion<N> {}

impl<N: SimdRealField> Normed for Quaternion<N> {
type Norm = N::SimdRealField;

Expand Down
2 changes: 1 addition & 1 deletion src/geometry/quaternion_construction.rs
Expand Up @@ -19,7 +19,7 @@ use crate::{Scalar, SimdRealField};

use crate::geometry::{Quaternion, Rotation3, UnitQuaternion};

impl<N: Scalar + SimdValue> Quaternion<N> {
impl<N: Scalar> Quaternion<N> {
/// Creates a quaternion from a 4D vector. The quaternion scalar part corresponds to the `w`
/// vector component.
#[inline]
Expand Down
26 changes: 13 additions & 13 deletions src/geometry/quaternion_conversion.rs
Expand Up @@ -184,36 +184,36 @@ impl<N1: RealField, N2: RealField + SupersetOf<N1>> SubsetOf<Matrix4<N2>> for Un
}

#[cfg(feature = "mint")]
impl<N: SimdRealField> From<mint::Quaternion<N>> for Quaternion<N> {
impl<N: Scalar> From<mint::Quaternion<N>> for Quaternion<N> {
fn from(q: mint::Quaternion<N>) -> Self {
Self::new(q.s, q.v.x, q.v.y, q.v.z)
}
}

#[cfg(feature = "mint")]
impl<N: SimdRealField> Into<mint::Quaternion<N>> for Quaternion<N> {
impl<N: Scalar> Into<mint::Quaternion<N>> for Quaternion<N> {
fn into(self) -> mint::Quaternion<N> {
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<N: SimdRealField> Into<mint::Quaternion<N>> for UnitQuaternion<N> {
impl<N: Scalar + SimdValue> Into<mint::Quaternion<N>> for UnitQuaternion<N> {
fn into(self) -> mint::Quaternion<N> {
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(),
}
}
}
Expand Down Expand Up @@ -258,14 +258,14 @@ where
}
}

impl<N: Scalar + SimdValue> From<Vector4<N>> for Quaternion<N> {
impl<N: Scalar> From<Vector4<N>> for Quaternion<N> {
#[inline]
fn from(coords: Vector4<N>) -> Self {
Self { coords }
}
}

impl<N: Scalar + SimdValue> From<[N; 4]> for Quaternion<N> {
impl<N: Scalar> From<[N; 4]> for Quaternion<N> {
#[inline]
fn from(coords: [N; 4]) -> Self {
Self {
Expand Down
6 changes: 3 additions & 3 deletions src/geometry/quaternion_ops.rs
Expand Up @@ -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<N: SimdRealField> Index<usize> for Quaternion<N> {
impl<N: Scalar> Index<usize> for Quaternion<N> {
type Output = N;

#[inline]
Expand All @@ -71,7 +71,7 @@ impl<N: SimdRealField> Index<usize> for Quaternion<N> {
}
}

impl<N: SimdRealField> IndexMut<usize> for Quaternion<N> {
impl<N: Scalar> IndexMut<usize> for Quaternion<N> {
#[inline]
fn index_mut(&mut self, i: usize) -> &mut N {
&mut self.coords[i]
Expand Down
11 changes: 11 additions & 0 deletions src/geometry/unit_complex.rs
Expand Up @@ -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.
///
Expand All @@ -29,6 +31,15 @@ use simba::simd::SimdRealField;
/// * [Conversion to a matrix <span style="float:right;">`to_rotation_matrix`, `to_homogeneous`…</span>](#conversion-to-a-matrix)
pub type UnitComplex<N> = Unit<Complex<N>>;

impl<N: Scalar + PartialEq> PartialEq for UnitComplex<N> {
#[inline]
fn eq(&self, rhs: &Self) -> bool {
(**self).eq(&**rhs)
}
}

impl<N: Scalar + Eq> Eq for UnitComplex<N> {}

impl<N: SimdRealField> Normed for Complex<N> {
type Norm = N::SimdRealField;

Expand Down
1 change: 0 additions & 1 deletion tests/geometry/quaternion.rs
Expand Up @@ -114,7 +114,6 @@ quickcheck!(
*/
fn unit_quaternion_double_covering(q: UnitQuaternion<f64>) -> bool {
let mq = UnitQuaternion::new_unchecked(-q.into_inner());

mq == q && mq.angle() == q.angle() && mq.axis() == q.axis()
}

Expand Down

0 comments on commit 88145b7

Please sign in to comment.