Skip to content

Commit

Permalink
#630 transmute_vec_as_bytes potential fix (#662)
Browse files Browse the repository at this point in the history
* chore(#630): reformatting transmute_vec_as_bytes to properly handle padding for usize and f32 types

* chore(630): added bytemuck to fyrox-core  and fyrox-impl . Updated transmute_vec_as_bytes to have the generic T implement pod trait

* chore(630): removing commented statement

* general cleanup

* removed podvector3 struct and enabled bytemuck on half dependency.

---------

Co-authored-by: Ouroboros <nathanael@Ouroboross-Mac-Studio.local>
  • Loading branch information
NathanaelG1 and Ouroboros committed Jul 4, 2024
1 parent 31344ef commit 474e3b0
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 12 deletions.
3 changes: 2 additions & 1 deletion fyrox-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ byteorder = "1.4.3"
rand = "0.8.4"
memoffset = "0.9.0"
lazy_static = "1.4.0"
nalgebra = "0.32.3"
nalgebra = { version = "0.32.3", features = ["bytemuck"] }
arrayvec = "0.7.2"
futures = {version = "0.3.17", features = ["thread-pool"] }
uuid = { version = "1", features = ["v4", "js"] }
Expand All @@ -35,6 +35,7 @@ once_cell = "1.17.1"
notify = "6"
serde = { version = "1", features = ["derive"] }
bincode = "1.3.3"
bytemuck = "1.16.1"

[target.'cfg(target_arch = "wasm32")'.dependencies]
web-sys = { version = "0.3.53", features = ["Request", "Window", "Response", "AudioContext", "AudioBuffer", "AudioContextOptions", "AudioNode", "AudioBufferSourceNode", "AudioDestinationNode"] }
Expand Down
28 changes: 22 additions & 6 deletions fyrox-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use std::{
path::{Path, PathBuf},
};

use bytemuck::Pod;
pub mod color;
pub mod color_gradient;
pub mod io;
Expand Down Expand Up @@ -66,7 +67,6 @@ pub use wasm_bindgen_futures;
pub use web_sys;

pub use type_traits::prelude::*;

/// Defines as_(variant), as_mut_(variant) and is_(variant) methods.
#[macro_export]
macro_rules! define_is_as {
Expand Down Expand Up @@ -321,7 +321,7 @@ pub fn value_as_u8_slice<T: Sized>(v: &T) -> &'_ [u8] {
}

/// Takes a vector of trivially-copyable values and turns it into a vector of bytes.
pub fn transmute_vec_as_bytes<T: Copy>(vec: Vec<T>) -> Vec<u8> {
pub fn transmute_vec_as_bytes<T: Pod>(vec: Vec<T>) -> Vec<u8> {
unsafe {
let mut vec = std::mem::ManuallyDrop::new(vec);
Vec::from_raw_parts(
Expand Down Expand Up @@ -449,15 +449,15 @@ where
mod test {
use std::path::Path;

use fxhash::FxHashMap;
use uuid::uuid;

use crate::{
append_extension, cmp_strings_case_insensitive, combine_uuids, hash_combine,
make_relative_path,
make_relative_path, transmute_vec_as_bytes,
visitor::{Visit, Visitor},
BiDirHashMap,
};
use fxhash::FxHashMap;
use std::mem::size_of;
use uuid::uuid;

#[test]
fn test_combine_uuids() {
Expand Down Expand Up @@ -627,4 +627,20 @@ mod test {
assert!(!cmp_strings_case_insensitive("FooBaz", "FOOBaR"));
assert!(cmp_strings_case_insensitive("foobar", "foobar"));
}

#[test]
fn test_transmute_vec_as_bytes_length_new_f32() {
let vec = vec![1.0f32, 2.0, 3.0];
let byte_vec = transmute_vec_as_bytes(vec.clone());
let expected_length = vec.len() * size_of::<f32>();
assert_eq!(byte_vec.len(), expected_length);
}

#[test]
fn test_transmute_vec_as_bytes_length_new_usize() {
let vec = vec![1usize, 2, 3];
let byte_vec = transmute_vec_as_bytes(vec.clone());
let expected_length = vec.len() * size_of::<usize>();
assert_eq!(byte_vec.len(), expected_length);
}
}
6 changes: 3 additions & 3 deletions fyrox-impl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ strum = "0.26.1"
strum_macros = "0.26.1"
clap = { version = "4", features = ["derive"] }
winit = { version = "0.29.2", features = ["serde"] }
half = "2.2.1"
half = { version = "2.2.1", features = ["bytemuck"] }
fast_image_resize = "4.0.0"
base64 = "0.22.1"
uvgen = "0.1.0"
lightmap = "0.1.1"
libloading = "0.8.1"
gltf = { version = "1.4.0", optional = true, default-features = false, features = ["names", "utils"] }

bytemuck = { version = "1.16.1", features = ["derive"]}
# These dependencies isn't actually used by the engine, but it is needed to prevent cargo from rebuilding
# the engine lib on different packages.
hashbrown = { version = "0.14.3", features = ["raw"] }
Expand All @@ -67,4 +67,4 @@ raw-window-handle = "0.5.0"
serde-wasm-bindgen = "0.6.3"

[target.'cfg(target_os = "android")'.dependencies]
winit = { version = "0.29.2", features = ["android-native-activity"] }
winit = { version = "0.29.2", features = ["android-native-activity"] }
5 changes: 3 additions & 2 deletions fyrox-impl/src/scene/mesh/surface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use crate::{
},
utils::raw_mesh::{RawMesh, RawMeshBuilder},
};
use bytemuck::{Pod, Zeroable};
use fxhash::{FxHashMap, FxHasher};
use half::f16;
use lazy_static::lazy_static;
Expand Down Expand Up @@ -101,7 +102,7 @@ impl BlendShapesContainer {
input_blend_shapes: &[InputBlendShapeData],
) -> Self {
#[repr(C)]
#[derive(Default, Clone, Copy)]
#[derive(Default, Clone, Copy, Pod, Zeroable)]
struct VertexData {
position: Vector3<f16>,
normal: Vector3<f16>,
Expand Down Expand Up @@ -167,7 +168,7 @@ impl BlendShapesContainer {
}
}

let bytes = crate::core::transmute_vec_as_bytes(vertex_data);
let bytes = crate::core::transmute_vec_as_bytes::<VertexData>(vertex_data);

assert_eq!(
bytes.len(),
Expand Down

0 comments on commit 474e3b0

Please sign in to comment.