Skip to content

Commit 3037a97

Browse files
IceSentryjames7132
andauthored
Don't reserve in write_buffer_range (#20687)
# Objective - If you push new data to the cpu side buffer and it becomes bigger than the gpu buffer the call to reserve would wipe the old data. It was an acceptable behaviour for write_buffer but if you are only writing a range it would be surprising if suddenly all the other data was gone without warning ## Solution - Don't call reserve and return an error letting users decide how to handle it. ## Testing Not tested, not sure how to easily test this. Closes #20686 --------- Co-authored-by: James Liu <[email protected]>
1 parent 72238e3 commit 3037a97

File tree

1 file changed

+36
-12
lines changed

1 file changed

+36
-12
lines changed

crates/bevy_render/src/render_resource/buffer_vec.rs

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use encase::{
99
internal::{WriteInto, Writer},
1010
ShaderType,
1111
};
12+
use thiserror::Error;
1213
use wgpu::{BindingResource, BufferAddress, BufferUsages};
1314

1415
use super::GpuArrayBufferable;
@@ -186,25 +187,30 @@ impl<T: NoUninit> RawBufferVec<T> {
186187
/// Queues writing of data from system RAM to VRAM using the [`RenderDevice`]
187188
/// and the provided [`RenderQueue`].
188189
///
189-
/// Before queuing the write, a [`reserve`](RawBufferVec::reserve) operation
190-
/// is executed.
190+
/// If the buffer is not initialized on the GPU or the range is bigger than the capacity it will
191+
/// return an error. You'll need to either reserve a new buffer which will lose data on the GPU
192+
/// or create a new buffer and copy the old data to it.
191193
///
192194
/// This will only write the data contained in the given range. It is useful if you only want
193195
/// to update a part of the buffer.
194196
pub fn write_buffer_range(
195197
&mut self,
196-
device: &RenderDevice,
197198
render_queue: &RenderQueue,
198199
range: core::ops::Range<usize>,
199-
) {
200+
) -> Result<(), WriteBufferRangeError> {
200201
if self.values.is_empty() {
201-
return;
202+
return Err(WriteBufferRangeError::NoValuesToUpload);
203+
}
204+
if range.end > self.item_size * self.capacity {
205+
return Err(WriteBufferRangeError::RangeBiggerThanBuffer);
202206
}
203-
self.reserve(self.values.len(), device);
204207
if let Some(buffer) = &self.buffer {
205208
// Cast only the bytes we need to write
206209
let bytes: &[u8] = must_cast_slice(&self.values[range.start..range.end]);
207210
render_queue.write_buffer(buffer, (range.start * self.item_size) as u64, bytes);
211+
Ok(())
212+
} else {
213+
Err(WriteBufferRangeError::BufferNotInitialized)
208214
}
209215
}
210216

@@ -417,25 +423,30 @@ where
417423
/// Queues writing of data from system RAM to VRAM using the [`RenderDevice`]
418424
/// and the provided [`RenderQueue`].
419425
///
420-
/// Before queuing the write, a [`reserve`](BufferVec::reserve) operation
421-
/// is executed.
426+
/// If the buffer is not initialized on the GPU or the range is bigger than the capacity it will
427+
/// return an error. You'll need to either reserve a new buffer which will lose data on the GPU
428+
/// or create a new buffer and copy the old data to it.
422429
///
423430
/// This will only write the data contained in the given range. It is useful if you only want
424431
/// to update a part of the buffer.
425432
pub fn write_buffer_range(
426433
&mut self,
427-
device: &RenderDevice,
428434
render_queue: &RenderQueue,
429435
range: core::ops::Range<usize>,
430-
) {
436+
) -> Result<(), WriteBufferRangeError> {
431437
if self.data.is_empty() {
432-
return;
438+
return Err(WriteBufferRangeError::NoValuesToUpload);
433439
}
434440
let item_size = u64::from(T::min_size()) as usize;
435-
self.reserve(self.data.len() / item_size, device);
441+
if range.end > item_size * self.capacity {
442+
return Err(WriteBufferRangeError::RangeBiggerThanBuffer);
443+
}
436444
if let Some(buffer) = &self.buffer {
437445
let bytes = &self.data[range.start..range.end];
438446
render_queue.write_buffer(buffer, (range.start * item_size) as u64, bytes);
447+
Ok(())
448+
} else {
449+
Err(WriteBufferRangeError::BufferNotInitialized)
439450
}
440451
}
441452

@@ -561,3 +572,16 @@ where
561572
}
562573
}
563574
}
575+
576+
/// Error returned when `write_buffer_range` fails
577+
///
578+
/// See [`RawBufferVec::write_buffer_range`] [`BufferVec::write_buffer_range`]
579+
#[derive(Debug, Eq, PartialEq, Copy, Clone, Error)]
580+
pub enum WriteBufferRangeError {
581+
#[error("the range is bigger than the capacity of the buffer")]
582+
RangeBiggerThanBuffer,
583+
#[error("the gpu buffer is not initialized")]
584+
BufferNotInitialized,
585+
#[error("there are no values to upload")]
586+
NoValuesToUpload,
587+
}

0 commit comments

Comments
 (0)