Revise comments; discuss platform considerations in hal

Author: andyleiserson

wgpu-core/src/device/resource.rs

@@ -1214,18 +1214,20 @@ impl Device {
 
         // The great thing about buffer sizes is there are so many to choose from!
         //  - The application may request an arbitrary byte-aligned size.
-        //  - We add an extra byte if it's a vertex or index buffer so that we can simulate
-        //    binding an empty range at the end of the buffer, which Vulkan does not natively
-        //    allow.
-        //  - The overall buffer size must generally be a non-zero multiple of
-        //    COPY_BUFFER_ALIGNMENT (4), but in special cases an additional platform-dependent
-        //    requirement may apply (e.g. 256 for D3D12 constant buffers).
+        //  - If the buffer supports usage as a vertex or index buffer, we must ensure there
+        //    is a naturally aligned block of 4B at the end of the buffer. Zero-size bindings
+        //    are not supported by hal and must be redirected to this block.
+        //  - The overall buffer size must be a non-zero multiple of COPY_BUFFER_ALIGNMENT (4).
+        //    In some cases additional platform-dependent requirements may apply. The actual
+        //    allocated size of the buffer is returned by hal along with the buffer object from
+        //    `create_buffer`. `wgpu-core` is responsible for ensuring that any region of the
+        //    buffer, including hal-added padding, is initialized before it may be accessed.
         //
-        // Because initialization operates at multiples of COPY_BUFFER_ALIGNMENT, and
-        // initialization tracking will never determine that it is necessary to initialize a
-        // region outside the application-visible range, round the tracked initialization range
-        // down to a multiple of COPY_BUFFER_ALIGNMENT (tail_start), and eagerly zero-initialize
-        // from there to the buffer's true end offset.
+        // Because buffer writes for initialization operate at multiples of COPY_BUFFER_ALIGNMENT,
+        // and because initialization tracking will never determine that it is necessary to
+        // initialize a region outside the application-visible range, round the tracked
+        // initialization range down to a multiple of COPY_BUFFER_ALIGNMENT (tail_start), and
+        // eagerly zero-initialize from there to the buffer's true end.
 
         let actual_size = if desc.size == 0 {
             wgt::COPY_BUFFER_ALIGNMENT

wgpu-core/src/resource.rs

@@ -632,23 +632,11 @@ impl Buffer {
 
     /// Resolve the binding range for a vertex or index buffer.
     ///
-    /// These may have zero size. For storage and uniform buffer bindings, which
-    /// must have non-zero size, use [`Buffer::binding`].
+    /// This function is for vertex and index buffer bindings, which WebGPU
+    /// allows to have zero size. For storage and uniform buffer bindings,
+    /// which must have non-zero size, use [`Buffer::binding`].
     ///
     /// Returns an error if the binding would overflow the buffer.
-    ///
-    /// Vulkan's `vkCmdBindVertexBuffers` and `vkCmdBindIndexBuffer` have
-    /// some important restrictions:
-    ///
-    ///  1. There is no ability to specify the size of the binding.
-    ///  2. The binding offset must be strictly less than the buffer size.
-    ///
-    /// Due to (1), resolving a zero-size binding in-place could let a
-    /// shader read non-zero data outside the bounds of the binding.
-    /// To avoid that, zero-size bindings are relocated to the end of the buffer.
-    /// Due to (2), buffers with [`wgt::BufferUses::INDEX`] or
-    /// [`wgt::BufferUses::VERTEX`] must be padded with zeros on creation to
-    /// support binding them at the end.
     pub fn resolve_vertex_or_index_binding_range(
         &self,
         offset: wgt::BufferAddress,

wgpu-hal/src/lib.rs

@@ -905,6 +905,18 @@ pub trait Device: WasmNotSendSync {
     /// Creates a new buffer.
     ///
     /// The initial usage is `wgt::BufferUses::empty()`.
+    ///
+    /// `wgpu_hal` may adjust the size in `desc` to a larger value if required
+    /// by the platform. On success, it returns a tuple of the buffer itself
+    /// and its actual allocated size. `wgpu-core` is responsible for
+    /// initializing any portion of the buffer that may be accessed, including
+    /// any padding added by `create_buffer`.
+    ///
+    /// Platform-dependent padding is currently required for uniform buffers on
+    /// dx12. Support for zero-size vertex and index bindings is also platform
+    /// dependent, but presently, `wgpu-core` adds padding to the end of all
+    /// buffers with vertex or index usage, and redirects all zero-size bindings
+    /// to that padding region, regardless of platform.
     unsafe fn create_buffer(
         &self,
         desc: &BufferDescriptor,
@@ -1655,11 +1667,39 @@ pub trait CommandEncoder: WasmNotSendSync + fmt::Debug {
 
     unsafe fn set_render_pipeline(&mut self, pipeline: &<Self::A as Api>::RenderPipeline);
 
+    /// Register an index buffer binding.
+    ///
+    /// The binding offset must be 4B-aligned and strictly less than the buffer
+    /// size. On some backends, the binding size is ignored. This means that
+    /// zero-size bindings must be simulated by binding a region of zeros
+    /// spanning from the provided offset to the end of the buffer. See
+    /// [`CommandEncoder::set_vertex_buffer`] for more detail.
     unsafe fn set_index_buffer<'a>(
         &mut self,
         binding: BufferBinding<'a, <Self::A as Api>::Buffer, wgt::BufferAddress>,
         format: wgt::IndexFormat,
     );
+    /// Register a vertex buffer binding.
+    ///
+    /// The binding offset must be 4B-aligned and strictly less than the buffer
+    /// size. On some backends, the binding size is ignored. This means that
+    /// zero-size bindings must be simulated by binding a region of zeros
+    /// spanning from the provided offset to the end of the buffer.
+    ///
+    /// These restrictions arise from Vulkan's `vkCmdBindVertexBuffers` and
+    /// `vkCmdBindIndexBuffer`, which:
+    ///
+    ///  1. Do not support specifying the size of the binding.
+    ///  2. Require that the binding offset is strictly less than the buffer size.
+    ///
+    /// A read at any offset from a zero-size binding is out-of-bounds, and
+    /// should return zero. Because the binding size is not respected, this
+    /// means there may not be non-zero data between the binding offset and
+    /// the end of the buffer.
+    ///
+    /// Because the binding offset must be strictly less than the buffer size,
+    /// supporting zero-size bindings requires zero padding at the end of the
+    /// buffer.
     unsafe fn set_vertex_buffer<'a>(
         &mut self,
         index: u32,