use bytemuck crate instead of Byteable trait (#2183)

This gets rid of multiple unsafe blocks that we had to maintain ourselves, and instead depends on library that's commonly used and supported by the ecosystem. We also get support for glam types for free.

There is still some things to clear up with the `Bytes` trait, but that is a bit more substantial change and can be done separately. Also there are already separate efforts to use `crevice` crate, so I've just added that as a TODO.

Author: Frizi

crates/bevy_core/Cargo.toml

@@ -14,10 +14,14 @@ keywords = ["bevy"]
 
 
 [dependencies]
+# bevy
 bevy_app = { path = "../bevy_app", version = "0.5.0" }
 bevy_derive = { path = "../bevy_derive", version = "0.5.0" }
 bevy_ecs = { path = "../bevy_ecs", version = "0.5.0" }
 bevy_math = { path = "../bevy_math", version = "0.5.0" }
 bevy_reflect = { path = "../bevy_reflect", version = "0.5.0", features = ["bevy"] }
 bevy_tasks = { path = "../bevy_tasks", version = "0.5.0" }
-bevy_utils = { path = "../bevy_utils", version = "0.5.0" }
+bevy_utils = { path = "../bevy_utils", version = "0.5.0" }
+
+# other
+bytemuck = "1.5"

crates/bevy_core/src/bytes.rs

@@ -1,7 +1,14 @@
-use bevy_math::{Mat4, Vec2, Vec3, Vec4};
-
 pub use bevy_derive::Bytes;
 
+// NOTE: we can reexport common traits and methods from bytemuck to avoid requiring dependency most of
+// the time, but unfortunately we can't use derive macros that way due to hardcoded path in generated code.
+pub use bytemuck::{bytes_of, cast_slice, Pod, Zeroable};
+
+// FIXME: `Bytes` trait doesn't specify the expected encoding format,
+// which means types that implement it have to know what format is expected
+// and can only implement one encoding at a time.
+// TODO: Remove `Bytes` and `FromBytes` in favour of `crevice` crate.
+
 /// Converts the implementing type to bytes by writing them to a given buffer
 pub trait Bytes {
     /// Converts the implementing type to bytes by writing them to a given buffer
@@ -11,29 +18,19 @@ pub trait Bytes {
     fn byte_len(&self) -> usize;
 }
 
-/// A trait that indicates that it is safe to cast the type to a byte array reference.
-pub unsafe trait Byteable: Copy + Sized {}
-
 impl<T> Bytes for T
 where
-    T: Byteable,
+    T: Pod,
 {
     fn write_bytes(&self, buffer: &mut [u8]) {
-        let bytes = self.as_bytes();
-        buffer[0..self.byte_len()].copy_from_slice(bytes)
+        buffer[0..self.byte_len()].copy_from_slice(bytes_of(self))
     }
 
     fn byte_len(&self) -> usize {
         std::mem::size_of::<Self>()
     }
 }
 
-/// Reads the implementing type as a byte array reference
-pub trait AsBytes {
-    /// Reads the implementing type as a byte array reference
-    fn as_bytes(&self) -> &[u8];
-}
-
 /// Converts a byte array to `Self`
 pub trait FromBytes {
     /// Converts a byte array to `Self`
@@ -42,7 +39,7 @@ pub trait FromBytes {
 
 impl<T> FromBytes for T
 where
-    T: Byteable,
+    T: Pod,
 {
     fn from_bytes(bytes: &[u8]) -> Self {
         assert_eq!(
@@ -55,128 +52,6 @@ where
     }
 }
 
-impl<T> AsBytes for T
-where
-    T: Byteable,
-{
-    fn as_bytes(&self) -> &[u8] {
-        let len = std::mem::size_of::<T>();
-        unsafe { core::slice::from_raw_parts(self as *const Self as *const u8, len) }
-    }
-}
-
-impl<'a, T> AsBytes for [T]
-where
-    T: Byteable,
-{
-    fn as_bytes(&self) -> &[u8] {
-        let len = std::mem::size_of_val(self);
-        unsafe { core::slice::from_raw_parts(self as *const Self as *const u8, len) }
-    }
-}
-
-unsafe impl<T, const N: usize> Byteable for [T; N] where T: Byteable {}
-
-unsafe impl Byteable for u8 {}
-unsafe impl Byteable for u16 {}
-unsafe impl Byteable for u32 {}
-unsafe impl Byteable for u64 {}
-unsafe impl Byteable for usize {}
-unsafe impl Byteable for i8 {}
-unsafe impl Byteable for i16 {}
-unsafe impl Byteable for i32 {}
-unsafe impl Byteable for i64 {}
-unsafe impl Byteable for isize {}
-unsafe impl Byteable for f32 {}
-unsafe impl Byteable for f64 {}
-unsafe impl Byteable for Vec2 {}
-// NOTE: Vec3 actually takes up the size of 4 floats / 16 bytes due to SIMD. This is actually
-// convenient because GLSL uniform buffer objects pad Vec3s to be 16 bytes.
-unsafe impl Byteable for Vec3 {}
-unsafe impl Byteable for Vec4 {}
-
-impl Bytes for Mat4 {
-    fn write_bytes(&self, buffer: &mut [u8]) {
-        let array = self.to_cols_array();
-        array.write_bytes(buffer);
-    }
-
-    fn byte_len(&self) -> usize {
-        std::mem::size_of::<Self>()
-    }
-}
-
-impl FromBytes for Mat4 {
-    fn from_bytes(bytes: &[u8]) -> Self {
-        let array = <[f32; 16]>::from_bytes(bytes);
-        Mat4::from_cols_array(&array)
-    }
-}
-
-impl<T> Bytes for Option<T>
-where
-    T: Bytes,
-{
-    fn write_bytes(&self, buffer: &mut [u8]) {
-        if let Some(val) = self {
-            val.write_bytes(buffer)
-        }
-    }
-
-    fn byte_len(&self) -> usize {
-        self.as_ref().map_or(0, |val| val.byte_len())
-    }
-}
-
-impl<T> FromBytes for Option<T>
-where
-    T: FromBytes,
-{
-    fn from_bytes(bytes: &[u8]) -> Self {
-        if bytes.is_empty() {
-            None
-        } else {
-            Some(T::from_bytes(bytes))
-        }
-    }
-}
-
-impl<T> Bytes for Vec<T>
-where
-    T: Byteable,
-{
-    fn write_bytes(&self, buffer: &mut [u8]) {
-        let bytes = self.as_slice().as_bytes();
-        buffer[0..self.byte_len()].copy_from_slice(bytes)
-    }
-
-    fn byte_len(&self) -> usize {
-        self.as_slice().as_bytes().len()
-    }
-}
-
-impl<T> FromBytes for Vec<T>
-where
-    T: Byteable,
-{
-    fn from_bytes(bytes: &[u8]) -> Self {
-        assert_eq!(
-            bytes.len() % std::mem::size_of::<T>(),
-            0,
-            "Cannot convert byte slice `&[u8]` to type `Vec<{0}>`. Slice length is not a multiple of std::mem::size_of::<{0}>.",
-            std::any::type_name::<T>(),
-        );
-
-        let len = bytes.len() / std::mem::size_of::<T>();
-        let mut vec = Vec::<T>::with_capacity(len);
-        unsafe {
-            std::ptr::copy_nonoverlapping(bytes.as_ptr(), vec.as_mut_ptr() as *mut u8, bytes.len());
-            vec.set_len(len);
-        }
-        vec
-    }
-}
-
 #[cfg(test)]
 mod tests {
 
@@ -200,17 +75,6 @@ mod tests {
         test_round_trip(123f64);
     }
 
-    #[test]
-    fn test_vec_bytes_round_trip() {
-        test_round_trip(vec![1u32, 2u32, 3u32]);
-    }
-
-    #[test]
-    fn test_option_bytes_round_trip() {
-        test_round_trip(Some(123u32));
-        test_round_trip(Option::<u32>::None);
-    }
-
     #[test]
     fn test_vec2_round_trip() {
         test_round_trip(Vec2::new(1.0, 2.0));
@@ -233,7 +97,7 @@ mod tests {
 
     #[test]
     fn test_array_round_trip() {
-        test_round_trip([-10i32; 200]);
+        test_round_trip([-10i32; 1024]);
         test_round_trip([Vec2::ZERO, Vec2::ONE, Vec2::Y, Vec2::X]);
     }
 }

crates/bevy_core/src/float_ord.rs

@@ -1,4 +1,4 @@
-use crate::bytes::AsBytes;
+use crate::bytes_of;
 use std::{
     cmp::Ordering,
     hash::{Hash, Hasher},
@@ -42,12 +42,12 @@ impl Hash for FloatOrd {
     fn hash<H: Hasher>(&self, state: &mut H) {
         if self.0.is_nan() {
             // Ensure all NaN representations hash to the same value
-            state.write(f32::NAN.as_bytes())
+            state.write(bytes_of(&f32::NAN))
         } else if self.0 == 0.0 {
             // Ensure both zeroes hash to the same value
-            state.write(0.0f32.as_bytes())
+            state.write(bytes_of(&0.0f32))
         } else {
-            state.write(self.0.as_bytes());
+            state.write(bytes_of(&self.0));
         }
     }
 }

crates/bevy_math/Cargo.toml

@@ -13,5 +13,5 @@ license = "MIT"
 keywords = ["bevy"]
 
 [dependencies]
-glam = { version = "0.14.0", features = ["serde"] }
+glam = { version = "0.14.0", features = ["serde", "bytemuck"] }
 bevy_reflect = { path = "../bevy_reflect", version = "0.5.0", features = ["bevy"] }

crates/bevy_pbr/Cargo.toml

@@ -13,6 +13,7 @@ license = "MIT"
 keywords = ["bevy"]
 
 [dependencies]
+# bevy
 bevy_app = { path = "../bevy_app", version = "0.5.0" }
 bevy_asset = { path = "../bevy_asset", version = "0.5.0" }
 bevy_core = { path = "../bevy_core", version = "0.5.0" }
@@ -23,3 +24,7 @@ bevy_reflect = { path = "../bevy_reflect", version = "0.5.0", features = ["bevy"
 bevy_render = { path = "../bevy_render", version = "0.5.0" }
 bevy_transform = { path = "../bevy_transform", version = "0.5.0" }
 bevy_window = { path = "../bevy_window", version = "0.5.0" }
+
+# other
+# direct dependency required for derive macro
+bytemuck = { version = "1", features = ["derive"] }

crates/bevy_pbr/src/light.rs

@@ -1,4 +1,4 @@
-use bevy_core::Byteable;
+use bevy_core::{Pod, Zeroable};
 use bevy_ecs::reflect::ReflectComponent;
 use bevy_math::Vec3;
 use bevy_reflect::Reflect;
@@ -27,16 +27,14 @@ impl Default for PointLight {
 }
 
 #[repr(C)]
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, Pod, Zeroable)]
 pub(crate) struct PointLightUniform {
     pub pos: [f32; 4],
     pub color: [f32; 4],
     // storing as a `[f32; 4]` for memory alignement
     pub light_params: [f32; 4],
 }
 
-unsafe impl Byteable for PointLightUniform {}
-
 impl PointLightUniform {
     pub fn new(light: &PointLight, global_transform: &GlobalTransform) -> PointLightUniform {
         let (x, y, z) = global_transform.translation.into();
@@ -118,14 +116,12 @@ impl Default for DirectionalLight {
 }
 
 #[repr(C)]
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, Pod, Zeroable)]
 pub(crate) struct DirectionalLightUniform {
     pub dir: [f32; 4],
     pub color: [f32; 4],
 }
 
-unsafe impl Byteable for DirectionalLightUniform {}
-
 impl DirectionalLightUniform {
     pub fn new(light: &DirectionalLight) -> DirectionalLightUniform {
         // direction is negated to be ready for N.L

crates/bevy_pbr/src/render_graph/lights_node.rs

@@ -4,7 +4,7 @@ use crate::{
     },
     render_graph::uniform,
 };
-use bevy_core::{AsBytes, Byteable};
+use bevy_core::{bytes_of, Pod, Zeroable};
 use bevy_ecs::{
     system::{BoxedSystem, IntoSystem, Local, Query, Res, ResMut},
     world::World,
@@ -49,16 +49,14 @@ impl Node for LightsNode {
 }
 
 #[repr(C)]
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, Pod, Zeroable)]
 struct LightCount {
     // storing as a `[u32; 4]` for memory alignement
     // Index 0 is for point lights,
     // Index 1 is for directional lights
     pub num_lights: [u32; 4],
 }
 
-unsafe impl Byteable for LightCount {}
-
 impl SystemNode for LightsNode {
     fn get_system(&self) -> BoxedSystem {
         let system = lights_node_system.system().config(|config| {
@@ -160,29 +158,33 @@ pub fn lights_node_system(
         0..max_light_uniform_size as u64,
         &mut |data, _renderer| {
             // ambient light
-            data[0..ambient_light_size].copy_from_slice(ambient_light.as_bytes());
+            data[0..ambient_light_size].copy_from_slice(bytes_of(&ambient_light));
 
             // light count
-            data[ambient_light_size..light_count_size].copy_from_slice(
-                [point_light_count as u32, dir_light_count as u32, 0, 0].as_bytes(),
-            );
+            data[ambient_light_size..light_count_size].copy_from_slice(bytes_of(&[
+                point_light_count as u32,
+                dir_light_count as u32,
+                0,
+                0,
+            ]));
 
             // point light array
             for ((point_light, global_transform), slot) in point_lights.iter().zip(
                 data[point_light_uniform_start..point_light_uniform_end]
                     .chunks_exact_mut(point_light_size),
             ) {
-                slot.copy_from_slice(
-                    PointLightUniform::new(&point_light, &global_transform).as_bytes(),
-                );
+                slot.copy_from_slice(bytes_of(&PointLightUniform::new(
+                    &point_light,
+                    &global_transform,
+                )));
             }
 
             // directional light array
             for (dir_light, slot) in dir_lights.iter().zip(
                 data[dir_light_uniform_start..dir_light_uniform_end]
                     .chunks_exact_mut(dir_light_size),
             ) {
-                slot.copy_from_slice(DirectionalLightUniform::new(&dir_light).as_bytes());
+                slot.copy_from_slice(bytes_of(&DirectionalLightUniform::new(&dir_light)));
             }
         },
     );