diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a63afa5b8..ba78f4e4ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). diff --git a/tests/tests/wgpu-validation/api/buffer_slice.rs b/tests/tests/wgpu-validation/api/buffer_slice.rs index ac9fe9f8d6..d4c0b7e333 100644 --- a/tests/tests/wgpu-validation/api/buffer_slice.rs +++ b/tests/tests/wgpu-validation/api/buffer_slice.rs @@ -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..); @@ -46,7 +46,7 @@ fn getters() { slice_without_size.offset(), slice_without_size.size() ), - (&buffer, 10, NonZero::new(90).unwrap()) + (&buffer, 10, 90) ); } diff --git a/wgpu/src/api/buffer.rs b/wgpu/src/api/buffer.rs index afab2cf1cf..16f4177f44 100644 --- a/wgpu/src/api/buffer.rs +++ b/wgpu/src/api/buffer.rs @@ -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>(&self, bounds: S) -> BufferSlice<'_> { let (offset, size) = range_to_offset_size(bounds, self.size); @@ -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); @@ -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>(&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 @@ -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 @@ -598,7 +597,8 @@ 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() @@ -606,7 +606,7 @@ impl<'a> BufferSlice<'a> { 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, } @@ -632,7 +632,8 @@ 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() @@ -640,7 +641,7 @@ impl<'a> BufferSlice<'a> { 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), @@ -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> for crate::BufferBinding<'a> { @@ -674,7 +679,7 @@ impl<'a> From> for crate::BufferBinding<'a> { BufferBinding { buffer: value.buffer, offset: value.offset, - size: Some(value.size), + size: Some(value.size_expect_nonzero()), } } } @@ -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 ); } } @@ -1019,75 +1023,59 @@ fn check_buffer_bounds( pub(crate) fn range_to_offset_size>( 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] diff --git a/wgpu/src/api/command_buffer_actions.rs b/wgpu/src/api/command_buffer_actions.rs index 1c79b16ff8..5a29da296a 100644 --- a/wgpu/src/api/command_buffer_actions.rs +++ b/wgpu/src/api/command_buffer_actions.rs @@ -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(), diff --git a/wgpu/src/api/render_bundle_encoder.rs b/wgpu/src/api/render_bundle_encoder.rs index 483bfd084d..3bb0bd799d 100644 --- a/wgpu/src/api/render_bundle_encoder.rs +++ b/wgpu/src/api/render_bundle_encoder.rs @@ -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()), ); } @@ -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()), ); } diff --git a/wgpu/src/api/render_pass.rs b/wgpu/src/api/render_pass.rs index bd250e7b3c..110dff6492 100644 --- a/wgpu/src/api/render_pass.rs +++ b/wgpu/src/api/render_pass.rs @@ -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()), ); } @@ -126,7 +126,7 @@ impl RenderPass<'_> { slot, &buffer_slice.buffer.inner, buffer_slice.offset, - Some(buffer_slice.size), + Some(buffer_slice.size_expect_nonzero()), ); } diff --git a/wgpu/src/util/mod.rs b/wgpu/src/util/mod.rs index 2f524fc74c..345f429e0f 100644 --- a/wgpu/src/util/mod.rs +++ b/wgpu/src/util/mod.rs @@ -163,7 +163,7 @@ impl DownloadBuffer { buffer: &super::BufferSlice<'_>, callback: impl FnOnce(Result) + Send + 'static, ) { - let size = buffer.size.into(); + let size = buffer.size; let download = device.create_buffer(&super::BufferDescriptor { size,