Skip to content

Commit 1162e03

Browse files
BleachfuelTim Overbeek
andauthored
Make the get function on InstanceInputUniformBuffer less error prone (#17131)
# Objective the `get` function on [`InstanceInputUniformBuffer`] seems very error-prone. This PR hopes to fix this. ## Solution Do a few checks to ensure the index is in bounds and that the `BDI` is not removed. Return `Option<BDI>` instead of `BDI`. ## Testing - Did you test these changes? If so, how? added a test to verify that the instance buffer works correctly ## Future Work Performance decreases when using .binary_search(). However this is likely due to the fact that [`InstanceInputUniformBuffer::get`] for now is never used, and only get_unchecked. ## Migration Guide `InstanceInputUniformBuffer::get` now returns `Option<BDI>` instead of `BDI` to reduce panics. If you require the old functionality of `InstanceInputUniformBuffer::get` consider using `InstanceInputUniformBuffer::get_unchecked`. --------- Co-authored-by: Tim Overbeek <oorbeck@gmail.com>
1 parent d220ecc commit 1162e03

File tree

2 files changed

+41
-2
lines changed

2 files changed

+41
-2
lines changed

crates/bevy_pbr/src/render/mesh.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,8 @@ impl RenderMeshInstanceGpuBuilder {
957957

958958
// Save the old mesh input uniform. The mesh preprocessing
959959
// shader will need it to compute motion vectors.
960-
let previous_mesh_input_uniform = current_input_buffer.get(current_uniform_index);
960+
let previous_mesh_input_uniform =
961+
current_input_buffer.get_unchecked(current_uniform_index);
961962
let previous_input_index = previous_input_buffer.add(previous_mesh_input_uniform);
962963
mesh_input_uniform.previous_input_index = previous_input_index;
963964

crates/bevy_render/src/batching/gpu_preprocessing.rs

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,11 +217,31 @@ where
217217
}
218218

219219
/// Returns the piece of buffered data at the given index.
220-
pub fn get(&self, uniform_index: u32) -> BDI {
220+
///
221+
/// Returns [`None`] if the index is out of bounds or the data is removed.
222+
pub fn get(&self, uniform_index: u32) -> Option<BDI> {
223+
if (uniform_index as usize) >= self.buffer.len()
224+
|| self.free_uniform_indices.contains(&uniform_index)
225+
{
226+
None
227+
} else {
228+
Some(self.get_unchecked(uniform_index))
229+
}
230+
}
231+
232+
/// Returns the piece of buffered data at the given index.
233+
/// Can return data that has previously been removed.
234+
///
235+
/// # Panics
236+
/// if `uniform_index` is not in bounds of [`Self::buffer`].
237+
pub fn get_unchecked(&self, uniform_index: u32) -> BDI {
221238
self.buffer.values()[uniform_index as usize]
222239
}
223240

224241
/// Stores a piece of buffered data at the given index.
242+
///
243+
/// # Panics
244+
/// if `uniform_index` is not in bounds of [`Self::buffer`].
225245
pub fn set(&mut self, uniform_index: u32, element: BDI) {
226246
self.buffer.values_mut()[uniform_index as usize] = element;
227247
}
@@ -990,3 +1010,21 @@ pub fn write_indirect_parameters_buffer(
9901010
.write_buffer(&render_device, &render_queue);
9911011
indirect_parameters_buffer.buffer.clear();
9921012
}
1013+
1014+
#[cfg(test)]
1015+
mod tests {
1016+
use super::*;
1017+
1018+
#[test]
1019+
fn instance_buffer_correct_behavior() {
1020+
let mut instance_buffer = InstanceInputUniformBuffer::new();
1021+
1022+
let index = instance_buffer.add(2);
1023+
instance_buffer.remove(index);
1024+
assert_eq!(instance_buffer.get_unchecked(index), 2);
1025+
assert_eq!(instance_buffer.get(index), None);
1026+
1027+
instance_buffer.add(5);
1028+
assert_eq!(instance_buffer.buffer().len(), 1);
1029+
}
1030+
}

0 commit comments

Comments
 (0)