Repository: bitshifter/glam-rs
Pull request #146 (2021-03-10 18:51:12)

Add vector clamp function
This mirrors the new https://doc.rust-lang.org/std/primitive.f32.html#method.clamp function, with the differences:

* `assert!` has been replaced with `glam_assert!`
* treatment of `NaN` is undefined

emilk: Should we define the behavior of a `NaN` in self,min,max ? Should we protect against that with `glam_assert!`?
emilk: `glam_assert!(min.cmple(max).all());` is failing to compile because I put `clamp` in the `Vector` trait, and `.all()` is in `MaskVector3` and `MaskVector4` and rustc doesn't know which one to use. How would you suggest fixing that?
bitshifter: I made a comment on this but probably in the wrong place.

The assert complicates things a bit it turns out. For the clamp there's no Vector3/Vector4 implementation required because we just ignore lanes we don't care about (e.g. w for vec3), but for the assert we do care about the lanes, or rather we don't want say the w lane to factor into the Vec3 min/max assert. You could remove it for now and I'll deal with it later. I think to fix it. adding Vector3 and Vector4 implementations of clamp instead of Vector might fix it. It might still require the trait for all to be specified explicitly. Like I said, I'm happy for you to remove the assert and let me deal with it also.

Patch:
diff --git a/src/core/scalar/vector.rs b/src/core/scalar/vector.rs
index ad245d7..cd5d0d1 100644
--- a/src/core/scalar/vector.rs
+++ b/src/core/scalar/vector.rs
@@ -232,6 +232,18 @@ impl<T: NumEx> Vector<T> for XY<T> {
             y: self.y.max(other.y),
         }
     }
+
+    #[inline]
+    fn clamp(self, min: Self, max: Self) -> Self {
+        glam_assert!(min.x <= max.x);
+        glam_assert!(min.y <= max.y);
+        // we intentionally do not use `f32::clamp` because we don't
+        // want panics unless `glam-assert` feature is on.
+        Self {
+            x: self.x.max(min.x).min(max.x),
+            y: self.y.max(min.y).min(max.y),
+        }
+    }
 }
 
 impl<T: NumEx> Vector<T> for XYZ<T> {
@@ -385,6 +397,20 @@ impl<T: NumEx> Vector<T> for XYZ<T> {
             z: self.z.max(other.z),
         }
     }
+
+    #[inline]
+    fn clamp(self, min: Self, max: Self) -> Self {
+        glam_assert!(min.x <= max.x);
+        glam_assert!(min.y <= max.y);
+        glam_assert!(min.z <= max.z);
+        // we intentionally do not use `f32::clamp` because we don't
+        // want panics unless `glam-assert` feature is on.
+        Self {
+            x: self.x.max(min.x).min(max.x),
+            y: self.y.max(min.y).min(max.y),
+            z: self.z.max(min.z).min(max.z),
+        }
+    }
 }
 
 impl<T: NumEx> Vector<T> for XYZW<T> {
@@ -559,6 +585,22 @@ impl<T: NumEx> Vector<T> for XYZW<T> {
             w: self.w.max(other.w),
         }
     }
+
+    #[inline]
+    fn clamp(self, min: Self, max: Self) -> Self {
+        glam_assert!(min.x <= max.x);
+        glam_assert!(min.y <= max.y);
+        glam_assert!(min.z <= max.z);
+        glam_assert!(min.w <= max.w);
+        // we intentionally do not use `f32::clamp` because we don't
+        // want panics unless `glam-assert` feature is on.
+        Self {
+            x: self.x.max(min.x).min(max.x),
+            y: self.y.max(min.y).min(max.y),
+            z: self.z.max(min.z).min(max.z),
+            w: self.w.max(min.w).min(max.w),
+        }
+    }
 }
 
 impl<T: NumEx> Vector2<T> for XY<T> {
diff --git a/src/core/sse2/vector.rs b/src/core/sse2/vector.rs
index e2329b7..f129e19 100644
--- a/src/core/sse2/vector.rs
+++ b/src/core/sse2/vector.rs
@@ -261,6 +261,15 @@ impl Vector<f32> for __m128 {
     fn max(self, other: Self) -> Self {
         unsafe { _mm_max_ps(self, other) }
     }
+
+    #[inline(always)]
+    fn clamp(self, min: Self, max: Self) -> Self {
+        glam_assert!(
+            MaskVector3::all(min.cmple(max)), // TODO: also check .w for Vec4
+            "clamp: expected min <= max"
+        );
+        self.max(min).min(max)
+    }
 }
 
 impl Vector3<f32> for __m128 {
diff --git a/src/core/traits/vector.rs b/src/core/traits/vector.rs
index 887423c..bdba8e5 100644
--- a/src/core/traits/vector.rs
+++ b/src/core/traits/vector.rs
@@ -90,6 +90,7 @@ pub trait Vector<T>: Sized + Copy + Clone {
 
     fn min(self, other: Self) -> Self;
     fn max(self, other: Self) -> Self;
+    fn clamp(self, min: Self, max: Self) -> Self;
 }
 
 pub trait Vector2<T>: Vector<T> + Vector2Const {
diff --git a/src/vec.rs b/src/vec.rs
index fd09026..29fafd8 100644
--- a/src/vec.rs
+++ b/src/vec.rs
@@ -54,6 +54,16 @@ macro_rules! impl_vecn_common_methods {
             Self(self.0.max(other.0))
         }
 
+        /// Component-wise clamping of values, similar to [`std::f32::clamp`].
+        ///
+        /// Each element in `min` must be less-or-equal to the corresponing element in `max`.
+        /// If the `glam-assert` feature is enabled, the function will panic if the contract is not met,
+        /// otherwise the behavior is undefined.
+        #[inline(always)]
+        pub fn clamp(self, min: Self, max: Self) -> Self {
+            Self(self.0.clamp(min.0, max.0))
+        }
+
         /// Returns the horizontal minimum of `self`.
         ///
         /// In other words this computes `min(x, y, ..)`.


Test patch:
diff --git a/tests/vec2.rs b/tests/vec2.rs
index 4bfa56d..5612d02 100644
--- a/tests/vec2.rs
+++ b/tests/vec2.rs
@@ -122,6 +122,21 @@ macro_rules! impl_vec2_tests {
             assert_eq!($new(1 as $t, 2 as $t), b.max(a));
         }
 
+        #[test]
+        fn test_clamp() {
+            fn vec(x: i32, y: i32) -> $vec2 {
+                $vec2::new(x as $t, y as $t)
+            }
+            let min = vec(1, 3);
+            let max = vec(6, 8);
+            assert_eq!(vec(0, 0).clamp(min, max), vec(1, 3));
+            assert_eq!(vec(2, 2).clamp(min, max), vec(2, 3));
+            assert_eq!(vec(4, 5).clamp(min, max), vec(4, 5));
+            assert_eq!(vec(6, 6).clamp(min, max), vec(6, 6));
+            assert_eq!(vec(7, 7).clamp(min, max), vec(6, 7));
+            assert_eq!(vec(9, 9).clamp(min, max), vec(6, 8));
+        }
+
         #[test]
         fn test_hmin_hmax() {
             let a = $new(1 as $t, 2 as $t);
diff --git a/tests/vec3.rs b/tests/vec3.rs
index 5c766c3..968972f 100644
--- a/tests/vec3.rs
+++ b/tests/vec3.rs
@@ -137,6 +137,21 @@ macro_rules! impl_vec3_tests {
             assert_eq!((4 as $t, 5 as $t, 6 as $t), b.max(a).into());
         }
 
+        #[test]
+        fn test_clamp() {
+            fn vec(x: i32, y: i32, z: i32) -> $vec3 {
+                $vec3::new(x as $t, y as $t, z as $t)
+            }
+            let min = vec(1, 3, 3);
+            let max = vec(6, 8, 8);
+            assert_eq!(vec(0, 0, 0).clamp(min, max), vec(1, 3, 3));
+            assert_eq!(vec(2, 2, 2).clamp(min, max), vec(2, 3, 3));
+            assert_eq!(vec(4, 5, 5).clamp(min, max), vec(4, 5, 5));
+            assert_eq!(vec(6, 6, 6).clamp(min, max), vec(6, 6, 6));
+            assert_eq!(vec(7, 7, 7).clamp(min, max), vec(6, 7, 7));
+            assert_eq!(vec(9, 9, 9).clamp(min, max), vec(6, 8, 8));
+        }
+
         #[test]
         fn test_hmin_hmax() {
             let a = $new(2 as $t, 3 as $t, 1 as $t);
diff --git a/tests/vec4.rs b/tests/vec4.rs
index 3e7244b..a402c63 100644
--- a/tests/vec4.rs
+++ b/tests/vec4.rs
@@ -158,6 +158,21 @@ macro_rules! impl_vec4_tests {
             assert_eq!((5 as $t, 6 as $t, 7 as $t, 8 as $t), b.max(a).into());
         }
 
+        #[test]
+        fn test_clamp() {
+            fn vec(x: i32, y: i32, z: i32, w: i32) -> $vec4 {
+                $vec4::new(x as $t, y as $t, z as $t, w as $t)
+            }
+            let min = vec(1, 1, 3, 3);
+            let max = vec(6, 6, 8, 8);
+            assert_eq!(vec(0, 0, 0, 0).clamp(min, max), vec(1, 1, 3, 3));
+            assert_eq!(vec(2, 2, 2, 2).clamp(min, max), vec(2, 2, 3, 3));
+            assert_eq!(vec(4, 4, 5, 5).clamp(min, max), vec(4, 4, 5, 5));
+            assert_eq!(vec(6, 6, 6, 6).clamp(min, max), vec(6, 6, 6, 6));
+            assert_eq!(vec(7, 7, 7, 7).clamp(min, max), vec(6, 6, 7, 7));
+            assert_eq!(vec(9, 9, 9, 9).clamp(min, max), vec(6, 6, 8, 8));
+        }
+
         #[test]
         fn test_hmin_hmax() {
             let a = $new(3 as $t, 4 as $t, 1 as $t, 2 as $t);