Restore unintentional support for zero-size buffers

Author: andyleiserson

wgpu-core/src/binding_model.rs

@@ -106,7 +106,7 @@ pub enum BindingError {
         binding_size: u64,
         buffer_size: u64,
     },
-    #[error("Buffer {buffer}: Binding offset {offset} is greater than or equal to buffer size {buffer_size}")]
+    #[error("Buffer {buffer}: Binding offset {offset} is greater than buffer size {buffer_size}")]
     BindingOffsetTooLarge {
         buffer: ResourceErrorIdent,
         offset: wgt::BufferAddress,

wgpu-core/src/command/bundle.rs

@@ -93,6 +93,7 @@ use core::{
 use arrayvec::ArrayVec;
 use thiserror::Error;
 
+use wgpu_hal::ShouldBeNonZeroExt;
 use wgt::error::{ErrorType, WebGpuError};
 
 use crate::{
@@ -504,7 +505,7 @@ impl RenderBundleEncoder {
             buffer_id,
             index_format,
             offset,
-            size,
+            size: size.map(NonZeroU64::get),
         });
     }
 }
@@ -609,7 +610,7 @@ fn set_index_buffer(
     buffer_id: id::Id<id::markers::Buffer>,
     index_format: wgt::IndexFormat,
     offset: u64,
-    size: Option<NonZeroU64>,
+    size: Option<wgt::BufferSizeOrZero>,
 ) -> Result<(), RenderBundleErrorInner> {
     let buffer = buffer_guard.get(buffer_id).get()?;
 
@@ -641,7 +642,7 @@ fn set_vertex_buffer(
     slot: u32,
     buffer_id: id::Id<id::markers::Buffer>,
     offset: u64,
-    size: Option<NonZeroU64>,
+    size: Option<wgt::BufferSizeOrZero>,
 ) -> Result<(), RenderBundleErrorInner> {
     let max_vertex_buffers = state.device.limits.max_vertex_buffers;
     if slot >= max_vertex_buffers {
@@ -1166,19 +1167,16 @@ impl IndexState {
             .range
             .end
             .checked_sub(self.range.start)
-            .and_then(wgt::BufferSize::new);
-        assert!(
-            self.range.end <= self.buffer.size && binding_size.is_some(),
-            "index buffer range must have non-zero size and be contained in buffer",
-        );
+            .filter(|_| self.range.end <= self.buffer.size)
+            .expect("index range must be contained in buffer");
 
         if self.is_dirty {
             self.is_dirty = false;
             Some(ArcRenderCommand::SetIndexBuffer {
                 buffer: self.buffer.clone(),
                 index_format: self.format,
                 offset: self.range.start,
-                size: binding_size,
+                size: Some(binding_size),
             })
         } else {
             None
@@ -1221,24 +1219,20 @@ impl VertexState {
     ///
     /// `slot` is the index of the vertex buffer slot that `self` tracks.
     fn flush(&mut self, slot: u32) -> Option<ArcRenderCommand> {
-        // This was all checked before, but let's check again just in case.
         let binding_size = self
             .range
             .end
             .checked_sub(self.range.start)
-            .and_then(wgt::BufferSize::new);
-        assert!(
-            self.range.end <= self.buffer.size && binding_size.is_some(),
-            "vertex buffer range must have non-zero size and be contained in buffer",
-        );
+            .filter(|_| self.range.end <= self.buffer.size)
+            .expect("vertex range must be contained in buffer");
 
         if self.is_dirty {
             self.is_dirty = false;
             Some(ArcRenderCommand::SetVertexBuffer {
                 slot,
                 buffer: self.buffer.clone(),
                 offset: self.range.start,
-                size: binding_size,
+                size: Some(binding_size),
             })
         } else {
             None
@@ -1602,7 +1596,7 @@ where
 pub mod bundle_ffi {
     use super::{RenderBundleEncoder, RenderCommand};
     use crate::{id, RawString};
-    use core::{convert::TryInto, slice};
+    use core::{convert::TryInto, num::NonZeroU64, slice};
     use wgt::{BufferAddress, BufferSize, DynamicOffset, IndexFormat};
 
     /// # Safety
@@ -1661,7 +1655,7 @@ pub mod bundle_ffi {
             slot,
             buffer_id,
             offset,
-            size,
+            size: size.map(NonZeroU64::get),
         });
     }
 

wgpu-core/src/command/render.rs

@@ -1,12 +1,17 @@
 use alloc::{borrow::Cow, sync::Arc, vec::Vec};
-use core::{fmt, num::NonZeroU32, str};
+use core::{
+    fmt,
+    num::{NonZeroU32, NonZeroU64},
+    str,
+};
+use hal::ShouldBeNonZeroExt;
 
 use arrayvec::ArrayVec;
 use thiserror::Error;
 use wgt::{
     error::{ErrorType, WebGpuError},
-    BufferAddress, BufferSize, BufferUsages, Color, DynamicOffset, IndexFormat, ShaderStages,
-    TextureSelector, TextureUsages, TextureViewDimension, VertexStepMode,
+    BufferAddress, BufferSize, BufferSizeOrZero, BufferUsages, Color, DynamicOffset, IndexFormat,
+    ShaderStages, TextureSelector, TextureUsages, TextureViewDimension, VertexStepMode,
 };
 
 use crate::command::{
@@ -2333,7 +2338,7 @@ fn set_index_buffer(
     buffer: Arc<crate::resource::Buffer>,
     index_format: IndexFormat,
     offset: u64,
-    size: Option<BufferSize>,
+    size: Option<BufferSizeOrZero>,
 ) -> Result<(), RenderPassErrorInner> {
     api_log!("RenderPass::set_index_buffer {}", buffer.error_ident());
 
@@ -2373,7 +2378,7 @@ fn set_vertex_buffer(
     slot: u32,
     buffer: Arc<crate::resource::Buffer>,
     offset: u64,
-    size: Option<BufferSize>,
+    size: Option<BufferSizeOrZero>,
 ) -> Result<(), RenderPassErrorInner> {
     api_log!(
         "RenderPass::set_vertex_buffer {slot} {}",
@@ -3084,7 +3089,7 @@ impl Global {
             buffer: pass_try!(base, scope, self.resolve_render_pass_buffer_id(buffer_id)),
             index_format,
             offset,
-            size,
+            size: size.map(NonZeroU64::get),
         });
 
         Ok(())
@@ -3105,7 +3110,7 @@ impl Global {
             slot,
             buffer: pass_try!(base, scope, self.resolve_render_pass_buffer_id(buffer_id)),
             offset,
-            size,
+            size: size.map(NonZeroU64::get),
         });
 
         Ok(())

wgpu-core/src/command/render_command.rs

@@ -1,6 +1,6 @@
 use alloc::sync::Arc;
 
-use wgt::{BufferAddress, BufferSize, Color};
+use wgt::{BufferAddress, BufferSizeOrZero, Color};
 
 use super::{Rect, RenderBundle};
 use crate::{
@@ -24,13 +24,13 @@ pub enum RenderCommand {
         buffer_id: id::BufferId,
         index_format: wgt::IndexFormat,
         offset: BufferAddress,
-        size: Option<BufferSize>,
+        size: Option<BufferSizeOrZero>,
     },
     SetVertexBuffer {
         slot: u32,
         buffer_id: id::BufferId,
         offset: BufferAddress,
-        size: Option<BufferSize>,
+        size: Option<BufferSizeOrZero>,
     },
     SetBlendConstant(Color),
     SetStencilReference(u32),
@@ -418,21 +418,18 @@ pub enum ArcRenderCommand {
         offset: BufferAddress,
 
         // For a render pass, this reflects the argument passed by the
-        // application, which may be `None`. For a render bundle, this reflects
-        // the validated size of the binding, and will be populated even in the
-        // case that the application omitted the size.
-        size: Option<BufferSize>,
+        // application, which may be `None`. For a finished render bundle, this
+        // reflects the validated size of the binding, and will be populated
+        // even in the case that the application omitted the size.
+        size: Option<BufferSizeOrZero>,
     },
     SetVertexBuffer {
         slot: u32,
         buffer: Arc<Buffer>,
         offset: BufferAddress,
 
-        // For a render pass, this reflects the argument passed by the
-        // application, which may be `None`. For a render bundle, this reflects
-        // the validated size of the binding, and will be populated even in the
-        // case that the application omitted the size.
-        size: Option<BufferSize>,
+        // See comment in `SetIndexBuffer`.
+        size: Option<BufferSizeOrZero>,
     },
     SetBlendConstant(Color),
     SetStencilReference(u32),

wgpu-core/src/device/resource.rs

@@ -8,9 +8,10 @@ use alloc::{
 use core::{
     fmt,
     mem::{self, ManuallyDrop},
-    num::NonZeroU32,
+    num::{NonZeroU32, NonZeroU64},
     sync::atomic::{AtomicBool, Ordering},
 };
+use hal::ShouldBeNonZeroExt;
 
 use arrayvec::ArrayVec;
 use bitflags::Flags;
@@ -2197,7 +2198,7 @@ impl Device {
 
         buffer.check_usage(pub_usage)?;
 
-        let bb = buffer.binding(bb.offset, bb.size, snatch_guard)?;
+        let bb = buffer.binding(bb.offset, bb.size.map(NonZeroU64::get), snatch_guard)?;
         let bind_size = bb.size.get();
 
         if bind_size > range_limit as u64 {

wgpu-core/src/resource.rs

@@ -490,35 +490,32 @@ impl Buffer {
     /// If `size` is `None`, then the remainder of the buffer starting from
     /// `offset` is used.
     ///
-    /// If the binding would overflow the buffer or is empty (see
-    /// [`hal::BufferBinding`]), then an error is returned.
+    /// If the binding would overflow the buffer, then an error is returned.
+    ///
+    /// Zero-size bindings are permitted here for historical reasons. Although
+    /// zero-size bindings are permitted by WebGPU, they are not permitted by
+    /// some backends. See [`Buffer::binding`] and
+    /// [#3170](https://github.com/gfx-rs/wgpu/issues/3170).
     pub fn resolve_binding_size(
         &self,
         offset: wgt::BufferAddress,
-        binding_size: Option<wgt::BufferSize>,
-    ) -> Result<wgt::BufferSize, BindingError> {
+        binding_size: Option<wgt::BufferSizeOrZero>,
+    ) -> Result<wgt::BufferSizeOrZero, BindingError> {
         let buffer_size = self.size;
 
         match binding_size {
-            Some(binding_size) => {
-                match offset.checked_add(binding_size.get()) {
-                    // `binding_size` is not zero which means `end == buffer_size` is ok.
-                    Some(end) if end <= buffer_size => Ok(binding_size),
-                    _ => Err(BindingError::BindingRangeTooLarge {
-                        buffer: self.error_ident(),
-                        offset,
-                        binding_size: binding_size.get(),
-                        buffer_size,
-                    }),
-                }
-            }
+            Some(binding_size) => match offset.checked_add(binding_size) {
+                Some(end) if end <= buffer_size => Ok(binding_size),
+                _ => Err(BindingError::BindingRangeTooLarge {
+                    buffer: self.error_ident(),
+                    offset,
+                    binding_size,
+                    buffer_size,
+                }),
+            },
             None => {
-                // We require that `buffer_size - offset` converts to
-                // `BufferSize` (`NonZeroU64`) because bindings must not be
-                // empty.
                 buffer_size
                     .checked_sub(offset)
-                    .and_then(wgt::BufferSize::new)
                     .ok_or_else(|| BindingError::BindingOffsetTooLarge {
                         buffer: self.error_ident(),
                         offset,
@@ -534,12 +531,20 @@ impl Buffer {
     /// If `size` is `None`, then the remainder of the buffer starting from
     /// `offset` is used.
     ///
-    /// If the binding would overflow the buffer or is empty (see
-    /// [`hal::BufferBinding`]), then an error is returned.
+    /// If the binding would overflow the buffer, then an error is returned.
+    ///
+    /// Zero-size bindings are permitted here for historical reasons. Although
+    /// zero-size bindings are permitted by WebGPU, they are not permitted by
+    /// some backends. Previous documentation for `hal::BufferBinding`
+    /// disallowed zero-size bindings, but this restriction was not honored
+    /// elsewhere in the code. Zero-size bindings need to be quashed or remapped
+    /// to a non-zero size, either universally in wgpu-core, or in specific
+    /// backends that do not support them. See
+    /// [#3170](https://github.com/gfx-rs/wgpu/issues/3170).
     pub fn binding<'a>(
         &'a self,
         offset: wgt::BufferAddress,
-        binding_size: Option<wgt::BufferSize>,
+        binding_size: Option<wgt::BufferSizeOrZero>,
         snatch_guard: &'a SnatchGuard,
     ) -> Result<hal::BufferBinding<'a, dyn hal::DynBuffer>, BindingError> {
         let buf_raw = self.try_raw(snatch_guard)?;

wgpu-hal/examples/halmark/main.rs

@@ -447,11 +447,7 @@ impl<A: hal::Api> Example<A> {
         let global_group = {
             let global_buffer_binding = unsafe {
                 // SAFETY: This is the same size that was specified for buffer creation.
-                hal::BufferBinding::new_unchecked(
-                    &global_buffer,
-                    0,
-                    global_buffer_desc.size.try_into().unwrap(),
-                )
+                hal::BufferBinding::new_unchecked(&global_buffer, 0, global_buffer_desc.size)
             };
             let texture_binding = hal::TextureBinding {
                 view: &texture_view,

wgpu-hal/src/dx12/command.rs

@@ -1136,7 +1136,7 @@ impl crate::CommandEncoder for super::CommandEncoder {
     ) {
         let ibv = Direct3D12::D3D12_INDEX_BUFFER_VIEW {
             BufferLocation: binding.resolve_address(),
-            SizeInBytes: binding.resolve_size() as u32,
+            SizeInBytes: binding.size.try_into().unwrap(),
             Format: auxil::dxgi::conv::map_index_format(format),
         };
 
@@ -1149,7 +1149,7 @@ impl crate::CommandEncoder for super::CommandEncoder {
     ) {
         let vb = &mut self.pass.vertex_buffers[index as usize];
         vb.BufferLocation = binding.resolve_address();
-        vb.SizeInBytes = binding.resolve_size() as u32;
+        vb.SizeInBytes = binding.size.try_into().unwrap();
         self.pass.dirty_vertex_buffers |= 1 << index;
     }
 

wgpu-hal/src/dx12/device.rs

@@ -1442,7 +1442,7 @@ impl crate::Device for super::Device {
                     let end = start + entry.count as usize;
                     for data in &desc.buffers[start..end] {
                         let gpu_address = data.resolve_address();
-                        let mut size = data.resolve_size() as u32;
+                        let mut size = data.size.try_into().unwrap();
 
                         if has_dynamic_offset {
                             match ty {

wgpu-hal/src/dx12/mod.rs

@@ -865,13 +865,6 @@ unsafe impl Sync for Buffer {}
 impl crate::DynBuffer for Buffer {}
 
 impl crate::BufferBinding<'_, Buffer> {
-    fn resolve_size(&self) -> wgt::BufferAddress {
-        match self.size {
-            Some(size) => size.get(),
-            None => self.buffer.size - self.offset,
-        }
-    }
-
     // TODO: Return GPU handle directly?
     fn resolve_address(&self) -> wgt::BufferAddress {
         (unsafe { self.buffer.resource.GetGPUVirtualAddress() }) + self.offset