Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - use bytemuck crate instead of Byteable trait #2183

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion crates/bevy_core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
162 changes: 13 additions & 149 deletions crates/bevy_core/src/bytes.rs
Original file line number Diff line number Diff line change
@@ -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.
Copy link
Contributor

@NathanSWard NathanSWard May 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make an issue for this (once this PR gets merged) that way we we're able to track the progress for this TODO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but It's not really part of this PR. There is some progress in #1931 and afaik cart has some private branch going on as well.


/// 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
Expand All @@ -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`
Expand All @@ -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!(
Expand All @@ -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 {

Expand All @@ -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));
Expand All @@ -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]);
}
}
8 changes: 4 additions & 4 deletions crates/bevy_core/src/float_ord.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::bytes::AsBytes;
use crate::bytes_of;
use std::{
cmp::Ordering,
hash::{Hash, Hasher},
Expand Down Expand Up @@ -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));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_math/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
5 changes: 5 additions & 0 deletions crates/bevy_pbr/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand All @@ -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"] }
10 changes: 3 additions & 7 deletions crates/bevy_pbr/src/light.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down
26 changes: 14 additions & 12 deletions crates/bevy_pbr/src/render_graph/lights_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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)));
}
},
);
Expand Down
Loading