Skip to content

Commit

Permalink
Only create changed buffer if it already exists (#13242)
Browse files Browse the repository at this point in the history
# Objective

- `DynamicUniformBuffer` tries to create a buffer as soon as the changed
flag is set to true. This doesn't work correctly when the buffer wasn't
already created. This currently creates a crash because it's trying to
create a buffer of size 0 if the flag is set but there's no buffer yet.

## Solution

- Don't create a changed buffer until there's data that needs to be
written to a buffer.

## Testing

- run `cargo run --example scene_viewer` and see that it doesn't crash
anymore

Fixes #13235
  • Loading branch information
IceSentry authored May 5, 2024
1 parent 088960f commit a22eced
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 9 deletions.
12 changes: 6 additions & 6 deletions crates/bevy_render/src/render_resource/buffer_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub struct RawBufferVec<T: NoUninit> {
item_size: usize,
buffer_usage: BufferUsages,
label: Option<String>,
label_changed: bool,
changed: bool,
}

impl<T: NoUninit> RawBufferVec<T> {
Expand All @@ -55,7 +55,7 @@ impl<T: NoUninit> RawBufferVec<T> {
item_size: std::mem::size_of::<T>(),
buffer_usage,
label: None,
label_changed: false,
changed: false,
}
}

Expand Down Expand Up @@ -93,7 +93,7 @@ impl<T: NoUninit> RawBufferVec<T> {
let label = label.map(str::to_string);

if label != self.label {
self.label_changed = true;
self.changed = true;
}

self.label = label;
Expand All @@ -115,16 +115,16 @@ impl<T: NoUninit> RawBufferVec<T> {
/// the `RawBufferVec` was created, the buffer on the [`RenderDevice`]
/// is marked as [`BufferUsages::COPY_DST`](BufferUsages).
pub fn reserve(&mut self, capacity: usize, device: &RenderDevice) {
if capacity > self.capacity || self.label_changed {
let size = self.item_size * capacity;
if capacity > self.capacity || (self.changed && size > 0) {
self.capacity = capacity;
let size = self.item_size * capacity;
self.buffer = Some(device.create_buffer(&wgpu::BufferDescriptor {
label: self.label.as_deref(),
size: size as BufferAddress,
usage: BufferUsages::COPY_DST | self.buffer_usage,
mapped_at_creation: false,
}));
self.label_changed = false;
self.changed = false;
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/render_resource/storage_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ impl<T: ShaderType + WriteInto> DynamicStorageBuffer<T> {
let capacity = self.buffer.as_deref().map(wgpu::Buffer::size).unwrap_or(0);
let size = self.scratch.as_ref().len() as u64;

if capacity < size || self.changed {
if capacity < size || (self.changed && size > 0) {
self.buffer = Some(device.create_buffer_with_data(&BufferInitDescriptor {
label: self.label.as_deref(),
usage: self.buffer_usage,
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_render/src/render_resource/uniform_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ impl<T: ShaderType + WriteInto> DynamicUniformBuffer<T> {
.checked_mul(max_count as u64)
.unwrap();

if capacity < size || self.changed {
if capacity < size || (self.changed && size > 0) {
let buffer = device.create_buffer(&BufferDescriptor {
label: self.label.as_deref(),
usage: self.buffer_usage,
Expand Down Expand Up @@ -336,7 +336,7 @@ impl<T: ShaderType + WriteInto> DynamicUniformBuffer<T> {
let capacity = self.buffer.as_deref().map(wgpu::Buffer::size).unwrap_or(0);
let size = self.scratch.as_ref().len() as u64;

if capacity < size || self.changed {
if capacity < size || (self.changed && size > 0) {
self.buffer = Some(device.create_buffer_with_data(&BufferInitDescriptor {
label: self.label.as_deref(),
usage: self.buffer_usage,
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_render/src/view/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ pub struct ViewUniforms {
impl FromWorld for ViewUniforms {
fn from_world(world: &mut World) -> Self {
let mut uniforms = DynamicUniformBuffer::default();
uniforms.set_label(Some("view_uniforms_buffer"));

let render_device = world.resource::<RenderDevice>();
if render_device.limits().max_storage_buffers_per_shader_stage > 0 {
Expand Down

0 comments on commit a22eced

Please sign in to comment.