Repository: bitshifter/glam-rs
Pull request #522 (2024-06-04 11:47:36)

Replace impl From<Vec4> for Vec3A with Vec3A::from_vec4 method.
The From impl violated the trait contract that the conversion should be lossless.


Patch:
diff --git a/codegen/templates/mat.rs.tera b/codegen/templates/mat.rs.tera
index 9fdcd0d..c6045f4 100644
--- a/codegen/templates/mat.rs.tera
+++ b/codegen/templates/mat.rs.tera
@@ -452,19 +452,11 @@ impl {{ self_t }} {
     #[inline]
     #[must_use]
     pub fn from_mat4(m: {{ mat4_t }}) -> Self {
-        {% if self_t == "Mat3A" %}
-            Self::from_cols(
-                m.x_axis.into(),
-                m.y_axis.into(),
-                m.z_axis.into(),
-            )
-        {% else %}
-            Self::from_cols(
-                m.x_axis.xyz(),
-                m.y_axis.xyz(),
-                m.z_axis.xyz(),
-            )
-        {% endif %}
+        Self::from_cols(
+            {{ col_t }}::from_vec4(m.x_axis),
+            {{ col_t }}::from_vec4(m.y_axis),
+            {{ col_t }}::from_vec4(m.z_axis),
+        )
     }
 
     /// Creates a 3D rotation matrix from the given quaternion.
@@ -1991,7 +1983,7 @@ impl {{ self_t }} {
             res = self.y_axis.mul(rhs.yyyy()).add(res);
             res = self.z_axis.mul(rhs.zzzz()).add(res);
             res = self.w_axis.add(res);
-            res.into()
+            Vec3A::from_vec4(res)
         {% endif %}
     }
 
@@ -2008,7 +2000,7 @@ impl {{ self_t }} {
             let mut res = self.x_axis.mul(rhs.xxxx());
             res = self.y_axis.mul(rhs.yyyy()).add(res);
             res = self.z_axis.mul(rhs.zzzz()).add(res);
-            res.into()
+            Vec3A::from_vec4(res)
         {% endif %}
     }
 {% endif %}
diff --git a/codegen/templates/vec.rs.tera b/codegen/templates/vec.rs.tera
index f6fea83..38e5dff 100644
--- a/codegen/templates/vec.rs.tera
+++ b/codegen/templates/vec.rs.tera
@@ -505,6 +505,20 @@ impl {{ self_t }} {
         {{ vec3_t }}::new(self.x, self.y, z)
     }
 {% elif dim == 3 %}
+    {% if self_t == "Vec3A" %}
+    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
+    ///
+    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
+    #[inline]
+    #[must_use]
+    pub fn from_vec4(v: Vec4) -> Self {
+        {% if is_scalar %}
+            Self { x: v.x, y: v.y, z: v.z }
+        {% else %}
+            Self(v.0)
+        {% endif %}
+    }
+    {% else %}
     /// Internal method for creating a 3D vector from a 4D vector, discarding `w`.
     #[allow(dead_code)]
     #[inline]
@@ -516,6 +530,7 @@ impl {{ self_t }} {
             Self(v.0)
         {% endif %}
     }
+    {% endif %}
 
     /// Creates a 4D vector from `self` and the given `w` value.
     #[inline]
@@ -3481,24 +3496,6 @@ impl From<Vec3> for Vec3A {
     }
 }
 
-impl From<Vec4> for Vec3A {
-    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
-    ///
-    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
-    #[inline]
-    fn from(v: Vec4) -> Self {
-        {% if is_scalar %}
-            Self {
-                x: v.x,
-                y: v.y,
-                z: v.z,
-            }
-        {% else %}
-            Self(v.0)
-        {% endif %}
-    }
-}
-
 impl From<Vec3A> for Vec3 {
     #[inline]
     fn from(v: Vec3A) -> Self {
diff --git a/src/f32/coresimd/mat3a.rs b/src/f32/coresimd/mat3a.rs
index 4e87f36..b5fb94f 100644
--- a/src/f32/coresimd/mat3a.rs
+++ b/src/f32/coresimd/mat3a.rs
@@ -152,7 +152,11 @@ impl Mat3A {
     #[inline]
     #[must_use]
     pub fn from_mat4(m: Mat4) -> Self {
-        Self::from_cols(m.x_axis.into(), m.y_axis.into(), m.z_axis.into())
+        Self::from_cols(
+            Vec3A::from_vec4(m.x_axis),
+            Vec3A::from_vec4(m.y_axis),
+            Vec3A::from_vec4(m.z_axis),
+        )
     }
 
     /// Creates a 3D rotation matrix from the given quaternion.
diff --git a/src/f32/coresimd/mat4.rs b/src/f32/coresimd/mat4.rs
index 04f9b8e..7745dcb 100644
--- a/src/f32/coresimd/mat4.rs
+++ b/src/f32/coresimd/mat4.rs
@@ -1126,7 +1126,7 @@ impl Mat4 {
         res = self.y_axis.mul(rhs.yyyy()).add(res);
         res = self.z_axis.mul(rhs.zzzz()).add(res);
         res = self.w_axis.add(res);
-        res.into()
+        Vec3A::from_vec4(res)
     }
 
     /// Transforms the give [`Vec3A`] as 3D vector.
@@ -1139,7 +1139,7 @@ impl Mat4 {
         let mut res = self.x_axis.mul(rhs.xxxx());
         res = self.y_axis.mul(rhs.yyyy()).add(res);
         res = self.z_axis.mul(rhs.zzzz()).add(res);
-        res.into()
+        Vec3A::from_vec4(res)
     }
 
     /// Transforms a 4D vector.
diff --git a/src/f32/coresimd/vec3a.rs b/src/f32/coresimd/vec3a.rs
index b7c3495..61c82cb 100644
--- a/src/f32/coresimd/vec3a.rs
+++ b/src/f32/coresimd/vec3a.rs
@@ -138,11 +138,12 @@ impl Vec3A {
         slice[2] = self.z;
     }
 
-    /// Internal method for creating a 3D vector from a 4D vector, discarding `w`.
-    #[allow(dead_code)]
+    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
+    ///
+    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
     #[inline]
     #[must_use]
-    pub(crate) fn from_vec4(v: Vec4) -> Self {
+    pub fn from_vec4(v: Vec4) -> Self {
         Self(v.0)
     }
 
@@ -1289,16 +1290,6 @@ impl From<Vec3> for Vec3A {
     }
 }
 
-impl From<Vec4> for Vec3A {
-    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
-    ///
-    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
-    #[inline]
-    fn from(v: Vec4) -> Self {
-        Self(v.0)
-    }
-}
-
 impl From<Vec3A> for Vec3 {
     #[inline]
     fn from(v: Vec3A) -> Self {
diff --git a/src/f32/mat3.rs b/src/f32/mat3.rs
index 24c5ee9..e82465e 100644
--- a/src/f32/mat3.rs
+++ b/src/f32/mat3.rs
@@ -153,7 +153,11 @@ impl Mat3 {
     #[inline]
     #[must_use]
     pub fn from_mat4(m: Mat4) -> Self {
-        Self::from_cols(m.x_axis.xyz(), m.y_axis.xyz(), m.z_axis.xyz())
+        Self::from_cols(
+            Vec3::from_vec4(m.x_axis),
+            Vec3::from_vec4(m.y_axis),
+            Vec3::from_vec4(m.z_axis),
+        )
     }
 
     /// Creates a 3D rotation matrix from the given quaternion.
diff --git a/src/f32/neon/mat3a.rs b/src/f32/neon/mat3a.rs
index 94151e2..b87bada 100644
--- a/src/f32/neon/mat3a.rs
+++ b/src/f32/neon/mat3a.rs
@@ -152,7 +152,11 @@ impl Mat3A {
     #[inline]
     #[must_use]
     pub fn from_mat4(m: Mat4) -> Self {
-        Self::from_cols(m.x_axis.into(), m.y_axis.into(), m.z_axis.into())
+        Self::from_cols(
+            Vec3A::from_vec4(m.x_axis),
+            Vec3A::from_vec4(m.y_axis),
+            Vec3A::from_vec4(m.z_axis),
+        )
     }
 
     /// Creates a 3D rotation matrix from the given quaternion.
diff --git a/src/f32/neon/mat4.rs b/src/f32/neon/mat4.rs
index 9828f85..b3e12c5 100644
--- a/src/f32/neon/mat4.rs
+++ b/src/f32/neon/mat4.rs
@@ -1131,7 +1131,7 @@ impl Mat4 {
         res = self.y_axis.mul(rhs.yyyy()).add(res);
         res = self.z_axis.mul(rhs.zzzz()).add(res);
         res = self.w_axis.add(res);
-        res.into()
+        Vec3A::from_vec4(res)
     }
 
     /// Transforms the give [`Vec3A`] as 3D vector.
@@ -1144,7 +1144,7 @@ impl Mat4 {
         let mut res = self.x_axis.mul(rhs.xxxx());
         res = self.y_axis.mul(rhs.yyyy()).add(res);
         res = self.z_axis.mul(rhs.zzzz()).add(res);
-        res.into()
+        Vec3A::from_vec4(res)
     }
 
     /// Transforms a 4D vector.
diff --git a/src/f32/neon/vec3a.rs b/src/f32/neon/vec3a.rs
index e2d2ddc..f1675ef 100644
--- a/src/f32/neon/vec3a.rs
+++ b/src/f32/neon/vec3a.rs
@@ -143,11 +143,12 @@ impl Vec3A {
         slice[2] = self.z;
     }
 
-    /// Internal method for creating a 3D vector from a 4D vector, discarding `w`.
-    #[allow(dead_code)]
+    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
+    ///
+    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
     #[inline]
     #[must_use]
-    pub(crate) fn from_vec4(v: Vec4) -> Self {
+    pub fn from_vec4(v: Vec4) -> Self {
         Self(v.0)
     }
 
@@ -1356,16 +1357,6 @@ impl From<Vec3> for Vec3A {
     }
 }
 
-impl From<Vec4> for Vec3A {
-    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
-    ///
-    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
-    #[inline]
-    fn from(v: Vec4) -> Self {
-        Self(v.0)
-    }
-}
-
 impl From<Vec3A> for Vec3 {
     #[inline]
     fn from(v: Vec3A) -> Self {
diff --git a/src/f32/scalar/mat3a.rs b/src/f32/scalar/mat3a.rs
index 167c662..2801251 100644
--- a/src/f32/scalar/mat3a.rs
+++ b/src/f32/scalar/mat3a.rs
@@ -153,7 +153,11 @@ impl Mat3A {
     #[inline]
     #[must_use]
     pub fn from_mat4(m: Mat4) -> Self {
-        Self::from_cols(m.x_axis.into(), m.y_axis.into(), m.z_axis.into())
+        Self::from_cols(
+            Vec3A::from_vec4(m.x_axis),
+            Vec3A::from_vec4(m.y_axis),
+            Vec3A::from_vec4(m.z_axis),
+        )
     }
 
     /// Creates a 3D rotation matrix from the given quaternion.
diff --git a/src/f32/scalar/vec3a.rs b/src/f32/scalar/vec3a.rs
index 0bbd1ec..feb92ea 100644
--- a/src/f32/scalar/vec3a.rs
+++ b/src/f32/scalar/vec3a.rs
@@ -145,11 +145,12 @@ impl Vec3A {
         slice[2] = self.z;
     }
 
-    /// Internal method for creating a 3D vector from a 4D vector, discarding `w`.
-    #[allow(dead_code)]
+    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
+    ///
+    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
     #[inline]
     #[must_use]
-    pub(crate) fn from_vec4(v: Vec4) -> Self {
+    pub fn from_vec4(v: Vec4) -> Self {
         Self {
             x: v.x,
             y: v.y,
@@ -1407,20 +1408,6 @@ impl From<Vec3> for Vec3A {
     }
 }
 
-impl From<Vec4> for Vec3A {
-    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
-    ///
-    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
-    #[inline]
-    fn from(v: Vec4) -> Self {
-        Self {
-            x: v.x,
-            y: v.y,
-            z: v.z,
-        }
-    }
-}
-
 impl From<Vec3A> for Vec3 {
     #[inline]
     fn from(v: Vec3A) -> Self {
diff --git a/src/f32/sse2/mat3a.rs b/src/f32/sse2/mat3a.rs
index 730f10e..24b5ce8 100644
--- a/src/f32/sse2/mat3a.rs
+++ b/src/f32/sse2/mat3a.rs
@@ -155,7 +155,11 @@ impl Mat3A {
     #[inline]
     #[must_use]
     pub fn from_mat4(m: Mat4) -> Self {
-        Self::from_cols(m.x_axis.into(), m.y_axis.into(), m.z_axis.into())
+        Self::from_cols(
+            Vec3A::from_vec4(m.x_axis),
+            Vec3A::from_vec4(m.y_axis),
+            Vec3A::from_vec4(m.z_axis),
+        )
     }
 
     /// Creates a 3D rotation matrix from the given quaternion.
diff --git a/src/f32/sse2/mat4.rs b/src/f32/sse2/mat4.rs
index 4da52e8..c499b8e 100644
--- a/src/f32/sse2/mat4.rs
+++ b/src/f32/sse2/mat4.rs
@@ -1135,7 +1135,7 @@ impl Mat4 {
         res = self.y_axis.mul(rhs.yyyy()).add(res);
         res = self.z_axis.mul(rhs.zzzz()).add(res);
         res = self.w_axis.add(res);
-        res.into()
+        Vec3A::from_vec4(res)
     }
 
     /// Transforms the give [`Vec3A`] as 3D vector.
@@ -1148,7 +1148,7 @@ impl Mat4 {
         let mut res = self.x_axis.mul(rhs.xxxx());
         res = self.y_axis.mul(rhs.yyyy()).add(res);
         res = self.z_axis.mul(rhs.zzzz()).add(res);
-        res.into()
+        Vec3A::from_vec4(res)
     }
 
     /// Transforms a 4D vector.
diff --git a/src/f32/sse2/vec3a.rs b/src/f32/sse2/vec3a.rs
index 858b840..918fe05 100644
--- a/src/f32/sse2/vec3a.rs
+++ b/src/f32/sse2/vec3a.rs
@@ -151,11 +151,12 @@ impl Vec3A {
         slice[2] = self.z;
     }
 
-    /// Internal method for creating a 3D vector from a 4D vector, discarding `w`.
-    #[allow(dead_code)]
+    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
+    ///
+    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
     #[inline]
     #[must_use]
-    pub(crate) fn from_vec4(v: Vec4) -> Self {
+    pub fn from_vec4(v: Vec4) -> Self {
         Self(v.0)
     }
 
@@ -1364,16 +1365,6 @@ impl From<Vec3> for Vec3A {
     }
 }
 
-impl From<Vec4> for Vec3A {
-    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
-    ///
-    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
-    #[inline]
-    fn from(v: Vec4) -> Self {
-        Self(v.0)
-    }
-}
-
 impl From<Vec3A> for Vec3 {
     #[inline]
     fn from(v: Vec3A) -> Self {
diff --git a/src/f32/wasm32/mat3a.rs b/src/f32/wasm32/mat3a.rs
index a8d6a8a..9cd6573 100644
--- a/src/f32/wasm32/mat3a.rs
+++ b/src/f32/wasm32/mat3a.rs
@@ -152,7 +152,11 @@ impl Mat3A {
     #[inline]
     #[must_use]
     pub fn from_mat4(m: Mat4) -> Self {
-        Self::from_cols(m.x_axis.into(), m.y_axis.into(), m.z_axis.into())
+        Self::from_cols(
+            Vec3A::from_vec4(m.x_axis),
+            Vec3A::from_vec4(m.y_axis),
+            Vec3A::from_vec4(m.z_axis),
+        )
     }
 
     /// Creates a 3D rotation matrix from the given quaternion.
diff --git a/src/f32/wasm32/mat4.rs b/src/f32/wasm32/mat4.rs
index c80e962..0fd7308 100644
--- a/src/f32/wasm32/mat4.rs
+++ b/src/f32/wasm32/mat4.rs
@@ -1126,7 +1126,7 @@ impl Mat4 {
         res = self.y_axis.mul(rhs.yyyy()).add(res);
         res = self.z_axis.mul(rhs.zzzz()).add(res);
         res = self.w_axis.add(res);
-        res.into()
+        Vec3A::from_vec4(res)
     }
 
     /// Transforms the give [`Vec3A`] as 3D vector.
@@ -1139,7 +1139,7 @@ impl Mat4 {
         let mut res = self.x_axis.mul(rhs.xxxx());
         res = self.y_axis.mul(rhs.yyyy()).add(res);
         res = self.z_axis.mul(rhs.zzzz()).add(res);
-        res.into()
+        Vec3A::from_vec4(res)
     }
 
     /// Transforms a 4D vector.
diff --git a/src/f32/wasm32/vec3a.rs b/src/f32/wasm32/vec3a.rs
index 095ff4c..c91b71b 100644
--- a/src/f32/wasm32/vec3a.rs
+++ b/src/f32/wasm32/vec3a.rs
@@ -137,11 +137,12 @@ impl Vec3A {
         slice[2] = self.z;
     }
 
-    /// Internal method for creating a 3D vector from a 4D vector, discarding `w`.
-    #[allow(dead_code)]
+    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
+    ///
+    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
     #[inline]
     #[must_use]
-    pub(crate) fn from_vec4(v: Vec4) -> Self {
+    pub fn from_vec4(v: Vec4) -> Self {
         Self(v.0)
     }
 
@@ -1313,16 +1314,6 @@ impl From<Vec3> for Vec3A {
     }
 }
 
-impl From<Vec4> for Vec3A {
-    /// Creates a [`Vec3A`] from the `x`, `y` and `z` elements of `self` discarding `w`.
-    ///
-    /// On architectures where SIMD is supported such as SSE2 on `x86_64` this conversion is a noop.
-    #[inline]
-    fn from(v: Vec4) -> Self {
-        Self(v.0)
-    }
-}
-
 impl From<Vec3A> for Vec3 {
     #[inline]
     fn from(v: Vec3A) -> Self {
diff --git a/src/f64/dmat3.rs b/src/f64/dmat3.rs
index 7c9564d..25162f9 100644
--- a/src/f64/dmat3.rs
+++ b/src/f64/dmat3.rs
@@ -153,7 +153,11 @@ impl DMat3 {
     #[inline]
     #[must_use]
     pub fn from_mat4(m: DMat4) -> Self {
-        Self::from_cols(m.x_axis.xyz(), m.y_axis.xyz(), m.z_axis.xyz())
+        Self::from_cols(
+            DVec3::from_vec4(m.x_axis),
+            DVec3::from_vec4(m.y_axis),
+            DVec3::from_vec4(m.z_axis),
+        )
     }
 
     /// Creates a 3D rotation matrix from the given quaternion.
diff --git a/src/lib.rs b/src/lib.rs
index 085bfc3..24c378e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -76,7 +76,8 @@ use glam::{Vec3, Vec3A, Vec4};
 let v4 = Vec4::new(1.0, 2.0, 3.0, 4.0);
 
 // Convert from `Vec4` to `Vec3A`, this is a no-op if SIMD is supported.
-let v3a = Vec3A::from(v4);
+// We use an explicit method here instead of a From impl as data is lost in the conversion.
+let v3a = Vec3A::from_vec4(v4);
 assert_eq!(Vec3A::new(1.0, 2.0, 3.0), v3a);
 
 // Convert from `Vec3A` to `Vec3`.
@@ -187,7 +188,7 @@ assert_eq!(Vec3::new(2.0, 3.0, 4.0), yzw);
 // To swizzle a `Vec4` into a `Vec3A` swizzle the `Vec4` first then convert to
 // `Vec3A`. If SIMD is supported this will use a vector shuffle. The last
 // element of the shuffled `Vec4` is ignored by the `Vec3A`.
-let yzw = Vec3A::from(v.yzwx());
+let yzw = Vec3A::from_vec4(v.yzwx());
 assert_eq!(Vec3A::new(2.0, 3.0, 4.0), yzw);
 
 // You can swizzle from a `Vec4` to a `Vec2`


Test patch:
diff --git a/tests/vec3.rs b/tests/vec3.rs
index 4a5c166..cf3ac92 100644
--- a/tests/vec3.rs
+++ b/tests/vec3.rs
@@ -1462,7 +1462,7 @@ mod vec3a {
     glam_test!(test_mask_align16, {
         // make sure the unused 'w' value doesn't break Vec3Ab behaviour
         let a = Vec4::ZERO;
-        let mut b = Vec3A::from(a);
+        let mut b = Vec3A::from_vec4(a);
         b.x = 1.0;
         b.y = 1.0;
         b.z = 1.0;
@@ -1508,9 +1508,9 @@ mod vec3a {
 
     glam_test!(test_min_max_from_vec4, {
         // checks that the 4th element is unused.
-        let v1 = Vec3A::from(Vec4::new(1.0, 2.0, 3.0, 4.0));
+        let v1 = Vec3A::from_vec4(Vec4::new(1.0, 2.0, 3.0, 4.0));
         assert_eq!(v1.max_element(), 3.0);
-        let v2 = Vec3A::from(Vec4::new(4.0, 3.0, 2.0, 1.0));
+        let v2 = Vec3A::from_vec4(Vec4::new(4.0, 3.0, 2.0, 1.0));
         assert_eq!(v2.min_element(), 2.0);
     });