Skip to content

Commit 5585308

Browse files
committed
StorageBuffer uses wrong type to calculate the buffer size. (#4557)
# Objective Fixes #4556 ## Solution StorageBuffer must use the Size of the std430 representation to calculate the buffer size, as the std430 representation is the data that will be written to it.
1 parent 82d849d commit 5585308

File tree

1 file changed

+75
-2
lines changed

1 file changed

+75
-2
lines changed

crates/bevy_render/src/render_resource/storage_buffer.rs

+75-2
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ impl<T: AsStd430, U: AsStd430 + Default> Default for StorageBuffer<T, U> {
3030
impl<T: AsStd430, U: AsStd430> StorageBuffer<T, U> {
3131
// NOTE: AsStd430::std430_size_static() uses size_of internally but trait functions cannot be
3232
// marked as const functions
33-
const BODY_SIZE: usize = std::mem::size_of::<U>();
34-
const ITEM_SIZE: usize = std::mem::size_of::<T>();
33+
const BODY_SIZE: usize = std::mem::size_of::<U::Output>();
34+
const ITEM_SIZE: usize = std::mem::size_of::<T::Output>();
3535

3636
/// Gets the reference to the underlying buffer, if one has been allocated.
3737
#[inline]
@@ -139,3 +139,76 @@ impl<T: AsStd430, U: AsStd430> StorageBuffer<T, U> {
139139
self.values.append(values);
140140
}
141141
}
142+
143+
#[cfg(test)]
144+
mod tests {
145+
use super::StorageBuffer;
146+
use bevy_crevice::std430;
147+
use bevy_crevice::std430::AsStd430;
148+
use bevy_crevice::std430::Std430;
149+
use bevy_math::Vec3;
150+
use bevy_math::Vec4;
151+
152+
//Note:
153+
//A Vec3 has 12 bytes and needs to be padded to 16 bytes, when converted to std430
154+
//https://www.w3.org/TR/WGSL/#alignment-and-size
155+
#[derive(AsStd430, Default)]
156+
struct NotInherentlyAligned {
157+
data: Vec3,
158+
}
159+
160+
//Note:
161+
//A Vec4 has 16 bytes and does not need to be padded to fit in std430
162+
//https://www.w3.org/TR/WGSL/#alignment-and-size
163+
#[derive(AsStd430)]
164+
struct InherentlyAligned {
165+
data: Vec4,
166+
}
167+
168+
#[test]
169+
fn storage_buffer_correctly_sized_nonaligned() {
170+
let mut buffer: StorageBuffer<NotInherentlyAligned> = StorageBuffer::default();
171+
buffer.push(NotInherentlyAligned { data: Vec3::ONE });
172+
173+
let actual_size = buffer.size();
174+
175+
let data = [NotInherentlyAligned { data: Vec3::ONE }].as_std430();
176+
let data_as_bytes = data.as_bytes();
177+
178+
assert_eq!(actual_size, data_as_bytes.len());
179+
}
180+
181+
#[test]
182+
fn storage_buffer_correctly_sized_aligned() {
183+
let mut buffer: StorageBuffer<InherentlyAligned> = StorageBuffer::default();
184+
buffer.push(InherentlyAligned { data: Vec4::ONE });
185+
186+
let actual_size = buffer.size();
187+
188+
let data = [InherentlyAligned { data: Vec4::ONE }].as_std430();
189+
let data_as_bytes = data.as_bytes();
190+
191+
assert_eq!(actual_size, data_as_bytes.len());
192+
}
193+
194+
#[test]
195+
fn storage_buffer_correctly_sized_item_and_body() {
196+
let mut buffer: StorageBuffer<NotInherentlyAligned, NotInherentlyAligned> =
197+
StorageBuffer::default();
198+
buffer.push(NotInherentlyAligned { data: Vec3::ONE });
199+
buffer.set_body(NotInherentlyAligned { data: Vec3::ONE });
200+
201+
let calculated_size = buffer.size();
202+
203+
//Emulate Write
204+
let mut scratch = Vec::<u8>::new();
205+
scratch.resize(calculated_size, 0);
206+
let mut writer = std430::Writer::new(&mut scratch[0..calculated_size]);
207+
writer
208+
.write(&buffer.body)
209+
.expect("Buffer has enough space to write the body.");
210+
writer
211+
.write(buffer.values.as_slice())
212+
.expect("Buffer has enough space to write the values.");
213+
}
214+
}

0 commit comments

Comments
 (0)