Skip to content

Commit

Permalink
Bind only the written parts of storage buffers. (#16405)
Browse files Browse the repository at this point in the history
# Objective
- Fix part of #15920

## Solution

- Keep track of the last written amount of bytes, and bind only that
much of the buffer.

## Testing

- Did you test these changes? If so, how? No
- Are there any parts that need more testing?
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?

---

## Migration Guide

- Fixed a bug with StorageBuffer and DynamicStorageBuffer binding data
from the previous frame(s) due to caching GPU buffers between frames.
  • Loading branch information
JMS55 authored and mockersf committed Nov 17, 2024
1 parent d0f755d commit a8ee620
Showing 1 changed file with 18 additions and 10 deletions.
28 changes: 18 additions & 10 deletions crates/bevy_render/src/render_resource/storage_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use encase::{
internal::WriteInto, DynamicStorageBuffer as DynamicStorageBufferWrapper, ShaderType,
StorageBuffer as StorageBufferWrapper,
};
use wgpu::{util::BufferInitDescriptor, BindingResource, BufferBinding, BufferUsages};
use wgpu::{util::BufferInitDescriptor, BindingResource, BufferBinding, BufferSize, BufferUsages};

use super::IntoBinding;

Expand Down Expand Up @@ -39,6 +39,7 @@ pub struct StorageBuffer<T: ShaderType> {
label: Option<String>,
changed: bool,
buffer_usage: BufferUsages,
last_written_size: Option<BufferSize>,
}

impl<T: ShaderType> From<T> for StorageBuffer<T> {
Expand All @@ -50,6 +51,7 @@ impl<T: ShaderType> From<T> for StorageBuffer<T> {
label: None,
changed: false,
buffer_usage: BufferUsages::COPY_DST | BufferUsages::STORAGE,
last_written_size: None,
}
}
}
Expand All @@ -63,6 +65,7 @@ impl<T: ShaderType + Default> Default for StorageBuffer<T> {
label: None,
changed: false,
buffer_usage: BufferUsages::COPY_DST | BufferUsages::STORAGE,
last_written_size: None,
}
}
}
Expand All @@ -75,9 +78,11 @@ impl<T: ShaderType + WriteInto> StorageBuffer<T> {

#[inline]
pub fn binding(&self) -> Option<BindingResource> {
Some(BindingResource::Buffer(
self.buffer()?.as_entire_buffer_binding(),
))
Some(BindingResource::Buffer(BufferBinding {
buffer: self.buffer()?,
offset: 0,
size: self.last_written_size,
}))
}

pub fn set(&mut self, value: T) {
Expand Down Expand Up @@ -137,16 +142,15 @@ impl<T: ShaderType + WriteInto> StorageBuffer<T> {
} else if let Some(buffer) = &self.buffer {
queue.write_buffer(buffer, 0, self.scratch.as_ref());
}

self.last_written_size = BufferSize::new(size);
}
}

impl<'a, T: ShaderType + WriteInto> IntoBinding<'a> for &'a StorageBuffer<T> {
#[inline]
fn into_binding(self) -> BindingResource<'a> {
self.buffer()
.expect("Failed to get buffer")
.as_entire_buffer_binding()
.into_binding()
self.binding().expect("Failed to get buffer")
}
}

Expand Down Expand Up @@ -180,6 +184,7 @@ pub struct DynamicStorageBuffer<T: ShaderType> {
label: Option<String>,
changed: bool,
buffer_usage: BufferUsages,
last_written_size: Option<BufferSize>,
_marker: PhantomData<fn() -> T>,
}

Expand All @@ -191,6 +196,7 @@ impl<T: ShaderType> Default for DynamicStorageBuffer<T> {
label: None,
changed: false,
buffer_usage: BufferUsages::COPY_DST | BufferUsages::STORAGE,
last_written_size: None,
_marker: PhantomData,
}
}
Expand All @@ -207,7 +213,7 @@ impl<T: ShaderType + WriteInto> DynamicStorageBuffer<T> {
Some(BindingResource::Buffer(BufferBinding {
buffer: self.buffer()?,
offset: 0,
size: Some(T::min_size()),
size: self.last_written_size,
}))
}

Expand Down Expand Up @@ -260,6 +266,8 @@ impl<T: ShaderType + WriteInto> DynamicStorageBuffer<T> {
} else if let Some(buffer) = &self.buffer {
queue.write_buffer(buffer, 0, self.scratch.as_ref());
}

self.last_written_size = BufferSize::new(size);
}

#[inline]
Expand All @@ -272,6 +280,6 @@ impl<T: ShaderType + WriteInto> DynamicStorageBuffer<T> {
impl<'a, T: ShaderType + WriteInto> IntoBinding<'a> for &'a DynamicStorageBuffer<T> {
#[inline]
fn into_binding(self) -> BindingResource<'a> {
self.binding().unwrap()
self.binding().expect("Failed to get buffer")
}
}

0 comments on commit a8ee620

Please sign in to comment.