Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ By @SupaMaggie70Incorporated in [#8206](https://github.com/gfx-rs/wgpu/pull/8206

### Changes

- Buffer slices with a length of 0 can now be created, but not mapped or used. By @beholdnec in [#8505] (https://github.com/gfx-rs/wgpu/pull/8505).

#### General

- Lower `max_blas_primitive_count` due to a bug in llvmpipe. By @Vecvec in [#8446](https://github.com/gfx-rs/wgpu/pull/8446).
Expand Down
4 changes: 2 additions & 2 deletions tests/tests/wgpu-validation/api/buffer_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fn getters() {
slice_with_size.offset(),
slice_with_size.size()
),
(&buffer, 10, NonZero::new(80).unwrap())
(&buffer, 10, 80)
);

let slice_without_size = buffer.slice(10..);
Expand All @@ -46,7 +46,7 @@ fn getters() {
slice_without_size.offset(),
slice_without_size.size()
),
(&buffer, 10, NonZero::new(90).unwrap())
(&buffer, 10, 90)
);
}

Expand Down
100 changes: 44 additions & 56 deletions wgpu/src/api/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ impl Buffer {
/// # Panics
///
/// - If `bounds` is outside of the bounds of `self`.
/// - If `bounds` has a length less than 1.
#[track_caller]
pub fn slice<S: RangeBounds<BufferAddress>>(&self, bounds: S) -> BufferSlice<'_> {
let (offset, size) = range_to_offset_size(bounds, self.size);
Expand Down Expand Up @@ -495,7 +494,7 @@ impl Buffer {
pub struct BufferSlice<'a> {
pub(crate) buffer: &'a Buffer,
pub(crate) offset: BufferAddress,
pub(crate) size: BufferSize,
pub(crate) size: BufferAddress,
}
#[cfg(send_sync)]
static_assertions::assert_impl_all!(BufferSlice<'_>: Send, Sync);
Expand All @@ -512,11 +511,10 @@ impl<'a> BufferSlice<'a> {
/// # Panics
///
/// - If `bounds` is outside of the bounds of `self`.
/// - If `bounds` has a length less than 1.
#[track_caller]
pub fn slice<S: RangeBounds<BufferAddress>>(&self, bounds: S) -> BufferSlice<'a> {
let (offset, size) = range_to_offset_size(bounds, self.size.get());
check_buffer_bounds(self.size.get(), offset, size);
let (offset, size) = range_to_offset_size(bounds, self.size);
check_buffer_bounds(self.size, offset, size);
BufferSlice {
buffer: self.buffer,
offset: self.offset + offset, // check_buffer_bounds ensures this does not overflow
Expand Down Expand Up @@ -570,7 +568,8 @@ impl<'a> BufferSlice<'a> {
) {
let mut mc = self.buffer.map_context.lock();
assert_eq!(mc.mapped_range, 0..0, "Buffer is already mapped");
let end = self.offset + self.size.get();
// FIXME: should self.size == 0 be forbidden here?
let end = self.offset + self.size;
mc.mapped_range = self.offset..end;

self.buffer
Expand Down Expand Up @@ -598,15 +597,16 @@ impl<'a> BufferSlice<'a> {
/// [mapped]: Buffer#mapping-buffers
#[track_caller]
pub fn get_mapped_range(&self) -> BufferView {
let subrange = Subrange::new(self.offset, self.size, RangeMappingKind::Immutable);
let slice_size = self.size_expect_nonzero();
let subrange = Subrange::new(self.offset, slice_size, RangeMappingKind::Immutable);
self.buffer
.map_context
.lock()
.validate_and_add(subrange.clone());
let range = self.buffer.inner.get_mapped_range(subrange.index);
BufferView {
buffer: self.buffer.clone(),
size: self.size,
size: slice_size,
offset: self.offset,
inner: range,
}
Expand All @@ -632,15 +632,16 @@ impl<'a> BufferSlice<'a> {
/// [mapped]: Buffer#mapping-buffers
#[track_caller]
pub fn get_mapped_range_mut(&self) -> BufferViewMut {
let subrange = Subrange::new(self.offset, self.size, RangeMappingKind::Mutable);
let slice_size = self.size_expect_nonzero();
let subrange = Subrange::new(self.offset, slice_size, RangeMappingKind::Mutable);
self.buffer
.map_context
.lock()
.validate_and_add(subrange.clone());
let range = self.buffer.inner.get_mapped_range(subrange.index);
BufferViewMut {
buffer: self.buffer.clone(),
size: self.size,
size: slice_size,
offset: self.offset,
inner: range,
readable: self.buffer.usage.contains(BufferUsages::MAP_READ),
Expand All @@ -662,9 +663,13 @@ impl<'a> BufferSlice<'a> {
}

/// Returns the size of this slice.
pub fn size(&self) -> BufferSize {
pub fn size(&self) -> BufferAddress {
self.size
}

pub(crate) fn size_expect_nonzero(&self) -> BufferSize {
BufferSize::new(self.size).expect("buffer slices can not be empty")
}
}

impl<'a> From<BufferSlice<'a>> for crate::BufferBinding<'a> {
Expand All @@ -674,7 +679,7 @@ impl<'a> From<BufferSlice<'a>> for crate::BufferBinding<'a> {
BufferBinding {
buffer: value.buffer,
offset: value.offset,
size: Some(value.size),
size: Some(value.size_expect_nonzero()),
}
}
}
Expand Down Expand Up @@ -993,24 +998,23 @@ impl Drop for BufferViewMut {

#[track_caller]
fn check_buffer_bounds(
buffer_size: BufferAddress,
whole_size: BufferAddress,
slice_offset: BufferAddress,
slice_size: BufferSize,
slice_size: BufferAddress,
) {
// A slice of length 0 is invalid, so the offset must not be equal to or greater than the buffer size.
if slice_offset >= buffer_size {
if slice_offset > whole_size {
panic!(
"slice offset {} is out of range for buffer of size {}",
slice_offset, buffer_size
slice_offset, whole_size
);
}

// Detect integer overflow.
let end = slice_offset.checked_add(slice_size.get());
if end.is_none_or(|end| end > buffer_size) {
let end = slice_offset.checked_add(slice_size);
if end.is_none_or(|end| end > whole_size) {
panic!(
"slice offset {} size {} is out of range for buffer of size {}",
slice_offset, slice_size, buffer_size
slice_offset, slice_size, whole_size
);
}
}
Expand All @@ -1019,75 +1023,59 @@ fn check_buffer_bounds(
pub(crate) fn range_to_offset_size<S: RangeBounds<BufferAddress>>(
bounds: S,
whole_size: BufferAddress,
) -> (BufferAddress, BufferSize) {
) -> (BufferAddress, BufferAddress) {
let offset = match bounds.start_bound() {
Bound::Included(&bound) => bound,
Bound::Excluded(&bound) => bound + 1,
Bound::Unbounded => 0,
};
let size = BufferSize::new(match bounds.end_bound() {
let size = match bounds.end_bound() {
Bound::Included(&bound) => bound + 1 - offset,
Bound::Excluded(&bound) => bound - offset,
Bound::Unbounded => whole_size - offset,
})
.expect("buffer slices can not be empty");
};

(offset, size)
}

#[cfg(test)]
mod tests {
use super::{
check_buffer_bounds, range_overlaps, range_to_offset_size, BufferAddress, BufferSize,
};

fn bs(value: BufferAddress) -> BufferSize {
BufferSize::new(value).unwrap()
}
use super::{check_buffer_bounds, range_overlaps, range_to_offset_size};

#[test]
fn range_to_offset_size_works() {
let whole = 100;

assert_eq!(range_to_offset_size(0..2, whole), (0, bs(2)));
assert_eq!(range_to_offset_size(2..5, whole), (2, bs(3)));
assert_eq!(range_to_offset_size(.., whole), (0, bs(whole)));
assert_eq!(range_to_offset_size(21.., whole), (21, bs(whole - 21)));
assert_eq!(range_to_offset_size(0.., whole), (0, bs(whole)));
assert_eq!(range_to_offset_size(..21, whole), (0, bs(21)));
}

#[test]
#[should_panic = "buffer slices can not be empty"]
fn range_to_offset_size_panics_for_empty_range() {
range_to_offset_size(123..123, 200);
}

#[test]
#[should_panic = "buffer slices can not be empty"]
fn range_to_offset_size_panics_for_unbounded_empty_range() {
range_to_offset_size(..0, 100);
assert_eq!(range_to_offset_size(0..2, whole), (0, 2));
assert_eq!(range_to_offset_size(2..5, whole), (2, 3));
assert_eq!(range_to_offset_size(.., whole), (0, whole));
assert_eq!(range_to_offset_size(21.., whole), (21, whole - 21));
assert_eq!(range_to_offset_size(0.., whole), (0, whole));
assert_eq!(range_to_offset_size(..21, whole), (0, 21));
}

#[test]
fn check_buffer_bounds_works_for_end_in_range() {
check_buffer_bounds(200, 100, bs(50));
check_buffer_bounds(200, 100, bs(100));
check_buffer_bounds(u64::MAX, u64::MAX - 100, bs(100));
check_buffer_bounds(u64::MAX, 0, bs(u64::MAX));
check_buffer_bounds(u64::MAX, 1, bs(u64::MAX - 1));
check_buffer_bounds(200, 100, 50);
check_buffer_bounds(200, 100, 100);
check_buffer_bounds(u64::MAX, u64::MAX - 100, 100);
check_buffer_bounds(u64::MAX, 0, u64::MAX);
check_buffer_bounds(u64::MAX, 1, u64::MAX - 1);
// Test empty buffer slices
check_buffer_bounds(0, 0, 0);
check_buffer_bounds(u64::MAX, u64::MAX, 0);
}

#[test]
#[should_panic]
fn check_buffer_bounds_panics_for_end_over_size() {
check_buffer_bounds(200, 100, bs(101));
check_buffer_bounds(200, 100, 101);
}

#[test]
#[should_panic]
fn check_buffer_bounds_panics_for_end_wraparound() {
check_buffer_bounds(u64::MAX, 1, bs(u64::MAX));
check_buffer_bounds(u64::MAX, 1, u64::MAX);
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions wgpu/src/api/command_buffer_actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ macro_rules! impl_deferred_command_buffer_actions {
callback: impl FnOnce(Result<(), BufferAsyncError>) + WasmNotSend + 'static,
) {
let (offset, size) = range_to_offset_size(bounds, buffer.size);
let size = BufferSize::new(size).expect("buffer slices can not be empty");
self.actions.lock().buffer_mappings.push(
crate::api::command_buffer_actions::DeferredBufferMapping {
buffer: buffer.clone(),
Expand Down
4 changes: 2 additions & 2 deletions wgpu/src/api/render_bundle_encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl<'a> RenderBundleEncoder<'a> {
&buffer_slice.buffer.inner,
index_format,
buffer_slice.offset,
Some(buffer_slice.size),
Some(buffer_slice.size_expect_nonzero()),
);
}

Expand All @@ -112,7 +112,7 @@ impl<'a> RenderBundleEncoder<'a> {
slot,
&buffer_slice.buffer.inner,
buffer_slice.offset,
Some(buffer_slice.size),
Some(buffer_slice.size_expect_nonzero()),
);
}

Expand Down
4 changes: 2 additions & 2 deletions wgpu/src/api/render_pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl RenderPass<'_> {
&buffer_slice.buffer.inner,
index_format,
buffer_slice.offset,
Some(buffer_slice.size),
Some(buffer_slice.size_expect_nonzero()),
);
}

Expand All @@ -126,7 +126,7 @@ impl RenderPass<'_> {
slot,
&buffer_slice.buffer.inner,
buffer_slice.offset,
Some(buffer_slice.size),
Some(buffer_slice.size_expect_nonzero()),
);
}

Expand Down
2 changes: 1 addition & 1 deletion wgpu/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl DownloadBuffer {
buffer: &super::BufferSlice<'_>,
callback: impl FnOnce(Result<Self, super::BufferAsyncError>) + Send + 'static,
) {
let size = buffer.size.into();
let size = buffer.size;

let download = device.create_buffer(&super::BufferDescriptor {
size,
Expand Down
Loading