From 5cbff59f801d7cee4a0ce0b3cb90d22c82349b80 Mon Sep 17 00:00:00 2001
From: Max Verevkin <me@maxverevkin.tk>
Date: Sun, 26 Sep 2021 19:34:19 +0300
Subject: [PATCH] simplify Matrix::is_identity while also improving performance

---
 src/base/properties.rs          | 40 ++++-----------------------------
 src/geometry/dual_quaternion.rs |  2 +-
 src/geometry/isometry.rs        |  2 +-
 src/geometry/similarity.rs      |  2 +-
 src/geometry/unit_complex.rs    |  3 +--
 5 files changed, 8 insertions(+), 41 deletions(-)

diff --git a/src/base/properties.rs b/src/base/properties.rs
index 7536a4a5..ef1dac82 100644
--- a/src/base/properties.rs
+++ b/src/base/properties.rs
@@ -63,50 +63,18 @@ impl<T: Scalar, R: Dim, C: Dim, S: RawStorage<T, R, C>> Matrix<T, R, C, S> {
         T::Epsilon: Clone,
     {
         let (nrows, ncols) = self.shape();
-        let d;
-
-        if nrows > ncols {
-            d = ncols;
-
-            for i in d..nrows {
-                for j in 0..ncols {
-                    if !relative_eq!(self[(i, j)], T::zero(), epsilon = eps.clone()) {
-                        return false;
-                    }
-                }
-            }
-        } else {
-            // nrows <= ncols
-            d = nrows;
 
+        for j in 0..ncols {
             for i in 0..nrows {
-                for j in d..ncols {
-                    if !relative_eq!(self[(i, j)], T::zero(), epsilon = eps.clone()) {
-                        return false;
-                    }
-                }
-            }
-        }
-
-        // Off-diagonal elements of the sub-square matrix.
-        for i in 1..d {
-            for j in 0..i {
-                // TODO: use unsafe indexing.
-                if !relative_eq!(self[(i, j)], T::zero(), epsilon = eps.clone())
-                    || !relative_eq!(self[(j, i)], T::zero(), epsilon = eps.clone())
+                let el = unsafe { self.get_unchecked((i, j)) };
+                if (i == j && !relative_eq!(*el, T::one(), epsilon = eps.clone()))
+                    || (i != j && !relative_eq!(*el, T::zero(), epsilon = eps.clone()))
                 {
                     return false;
                 }
             }
         }
 
-        // Diagonal elements of the sub-square matrix.
-        for i in 0..d {
-            if !relative_eq!(self[(i, i)], T::one(), epsilon = eps.clone()) {
-                return false;
-            }
-        }
-
         true
     }
 }
diff --git a/src/geometry/dual_quaternion.rs b/src/geometry/dual_quaternion.rs
index 4050fbfb..8bbf11f1 100644
--- a/src/geometry/dual_quaternion.rs
+++ b/src/geometry/dual_quaternion.rs
@@ -351,7 +351,7 @@ impl<T: RealField + UlpsEq<Epsilon = T>> UlpsEq for DualQuaternion<T> {
 
     #[inline]
     fn ulps_eq(&self, other: &Self, epsilon: Self::Epsilon, max_ulps: u32) -> bool {
-        self.clone().to_vector().ulps_eq(&other.clone().to_vector(), epsilon.clone(), max_ulps.clone()) ||
+        self.clone().to_vector().ulps_eq(&other.clone().to_vector(), epsilon.clone(), max_ulps) ||
         // Account for the double-covering of S², i.e. q = -q.
         self.clone().to_vector().iter().zip(other.clone().to_vector().iter()).all(|(a, b)| a.ulps_eq(&-b.clone(), epsilon.clone(), max_ulps))
     }
diff --git a/src/geometry/isometry.rs b/src/geometry/isometry.rs
index 4492c6c1..bdd2de60 100755
--- a/src/geometry/isometry.rs
+++ b/src/geometry/isometry.rs
@@ -629,7 +629,7 @@ where
     #[inline]
     fn ulps_eq(&self, other: &Self, epsilon: Self::Epsilon, max_ulps: u32) -> bool {
         self.translation
-            .ulps_eq(&other.translation, epsilon.clone(), max_ulps.clone())
+            .ulps_eq(&other.translation, epsilon.clone(), max_ulps)
             && self.rotation.ulps_eq(&other.rotation, epsilon, max_ulps)
     }
 }
diff --git a/src/geometry/similarity.rs b/src/geometry/similarity.rs
index 4cff61ce..1b685252 100755
--- a/src/geometry/similarity.rs
+++ b/src/geometry/similarity.rs
@@ -415,7 +415,7 @@ where
     #[inline]
     fn ulps_eq(&self, other: &Self, epsilon: Self::Epsilon, max_ulps: u32) -> bool {
         self.isometry
-            .ulps_eq(&other.isometry, epsilon.clone(), max_ulps.clone())
+            .ulps_eq(&other.isometry, epsilon.clone(), max_ulps)
             && self.scaling.ulps_eq(&other.scaling, epsilon, max_ulps)
     }
 }
diff --git a/src/geometry/unit_complex.rs b/src/geometry/unit_complex.rs
index 2c621674..63cf251c 100755
--- a/src/geometry/unit_complex.rs
+++ b/src/geometry/unit_complex.rs
@@ -458,8 +458,7 @@ impl<T: RealField> UlpsEq for UnitComplex<T> {
 
     #[inline]
     fn ulps_eq(&self, other: &Self, epsilon: Self::Epsilon, max_ulps: u32) -> bool {
-        self.re
-            .ulps_eq(&other.re, epsilon.clone(), max_ulps.clone())
+        self.re.ulps_eq(&other.re, epsilon.clone(), max_ulps)
             && self.im.ulps_eq(&other.im, epsilon, max_ulps)
     }
 }