Remove unnecesssary values Vec from DynamicUniformBuffer and DynamicStorageBuffer (#8299)

# Objective
Fixes #8284. `values` is being pushed to separately from the actual
scratch buffer in `DynamicUniformBuffer::push` and
`DynamicStorageBuffer::push`. In both types, `values` is really only
used to track the number of elements being added to the buffer, yet is
causing extra allocations, size increments and excess copies.

## Solution
Remove it and its remaining uses. Replace it with accesses to `scratch`
instead.

I removed the `len` accessor, as it may be non-trivial to compute just
from `scratch`. If this is still desirable to have, we can keep a `len`
member field to track it instead of relying on `scratch`.
This commit is contained in:
James Liu 2023-04-04 13:12:19 -07:00 committed by GitHub
parent ed50c8b4d9
commit 63d89d31ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 12 additions and 24 deletions

View File

@ -1,5 +1,7 @@
#![allow(clippy::doc_markdown)]
use std::marker::PhantomData;
use super::Buffer;
use crate::renderer::{RenderDevice, RenderQueue};
use encase::{
@ -160,25 +162,25 @@ impl<T: ShaderType + WriteInto> StorageBuffer<T> {
///
/// [std430 alignment/padding requirements]: https://www.w3.org/TR/WGSL/#address-spaces-storage
pub struct DynamicStorageBuffer<T: ShaderType> {
values: Vec<T>,
scratch: DynamicStorageBufferWrapper<Vec<u8>>,
buffer: Option<Buffer>,
capacity: usize,
label: Option<String>,
changed: bool,
buffer_usage: BufferUsages,
_marker: PhantomData<fn() -> T>,
}
impl<T: ShaderType> Default for DynamicStorageBuffer<T> {
fn default() -> Self {
Self {
values: Vec::new(),
scratch: DynamicStorageBufferWrapper::new(Vec::new()),
buffer: None,
capacity: 0,
label: None,
changed: false,
buffer_usage: BufferUsages::COPY_DST | BufferUsages::STORAGE,
_marker: PhantomData,
}
}
}
@ -198,21 +200,14 @@ impl<T: ShaderType + WriteInto> DynamicStorageBuffer<T> {
}))
}
#[inline]
pub fn len(&self) -> usize {
self.values.len()
}
#[inline]
pub fn is_empty(&self) -> bool {
self.values.is_empty()
self.scratch.as_ref().is_empty()
}
#[inline]
pub fn push(&mut self, value: T) -> u32 {
let offset = self.scratch.write(&value).unwrap() as u32;
self.values.push(value);
offset
self.scratch.write(&value).unwrap() as u32
}
pub fn set_label(&mut self, label: Option<&str>) {
@ -258,7 +253,6 @@ impl<T: ShaderType + WriteInto> DynamicStorageBuffer<T> {
#[inline]
pub fn clear(&mut self) {
self.values.clear();
self.scratch.as_mut().clear();
self.scratch.set_offset(0);
}

View File

@ -1,3 +1,5 @@
use std::marker::PhantomData;
use crate::{
render_resource::Buffer,
renderer::{RenderDevice, RenderQueue},
@ -153,25 +155,25 @@ impl<T: ShaderType + WriteInto> UniformBuffer<T> {
///
/// [std140 alignment/padding requirements]: https://www.w3.org/TR/WGSL/#address-spaces-uniform
pub struct DynamicUniformBuffer<T: ShaderType> {
values: Vec<T>,
scratch: DynamicUniformBufferWrapper<Vec<u8>>,
buffer: Option<Buffer>,
capacity: usize,
label: Option<String>,
changed: bool,
buffer_usage: BufferUsages,
_marker: PhantomData<fn() -> T>,
}
impl<T: ShaderType> Default for DynamicUniformBuffer<T> {
fn default() -> Self {
Self {
values: Vec::new(),
scratch: DynamicUniformBufferWrapper::new(Vec::new()),
buffer: None,
capacity: 0,
label: None,
changed: false,
buffer_usage: BufferUsages::COPY_DST | BufferUsages::UNIFORM,
_marker: PhantomData,
}
}
}
@ -191,22 +193,15 @@ impl<T: ShaderType + WriteInto> DynamicUniformBuffer<T> {
}))
}
#[inline]
pub fn len(&self) -> usize {
self.values.len()
}
#[inline]
pub fn is_empty(&self) -> bool {
self.values.is_empty()
self.scratch.as_ref().is_empty()
}
/// Push data into the `DynamicUniformBuffer`'s internal vector (residing on system RAM).
#[inline]
pub fn push(&mut self, value: T) -> u32 {
let offset = self.scratch.write(&value).unwrap() as u32;
self.values.push(value);
offset
self.scratch.write(&value).unwrap() as u32
}
pub fn set_label(&mut self, label: Option<&str>) {
@ -257,7 +252,6 @@ impl<T: ShaderType + WriteInto> DynamicUniformBuffer<T> {
#[inline]
pub fn clear(&mut self) {
self.values.clear();
self.scratch.as_mut().clear();
self.scratch.set_offset(0);
}