Improve late buffer binding size error (#8484)

Author: cwfitzgerald

tests/tests/wgpu-gpu/buffer.rs

@@ -354,7 +354,7 @@ static MINIMUM_BUFFER_BINDING_SIZE_DISPATCH: GpuTestConfiguration = GpuTestConfi
                 drop(pass);
                 let _ = encoder.finish();
             },
-            Some("buffer is bound with size 16 where the shader expects 32 in group[0] compact index 0"),
+            Some("In bind group index 0, the buffer bound at binding index 0 is bound with size 16 where the shader expects 32"),
         );
     });
 

wgpu-core/src/binding_model.rs

@@ -1164,6 +1164,14 @@ pub(crate) fn buffer_binding_type_bounds_check_alignment(
     }
 }
 
+#[derive(Debug)]
+pub(crate) struct BindGroupLateBufferBindingInfo {
+    /// The normal binding index in the bind group.
+    pub binding_index: u32,
+    /// The size that exists at bind time.
+    pub size: wgt::BufferSize,
+}
+
 #[derive(Debug)]
 pub struct BindGroup {
     pub(crate) raw: Snatchable<Box<dyn hal::DynBindGroup>>,
@@ -1178,7 +1186,7 @@ pub struct BindGroup {
     pub(crate) dynamic_binding_info: Vec<BindGroupDynamicBindingData>,
     /// Actual binding sizes for buffers that don't have `min_binding_size`
     /// specified in BGL. Listed in the order of iteration of `BGL.entries`.
-    pub(crate) late_buffer_binding_sizes: Vec<wgt::BufferSize>,
+    pub(crate) late_buffer_binding_infos: Vec<BindGroupLateBufferBindingInfo>,
 }
 
 impl Drop for BindGroup {
@@ -1289,10 +1297,13 @@ impl WebGpuError for GetBindGroupLayoutError {
 }
 
 #[derive(Clone, Debug, Error, Eq, PartialEq)]
-#[error("Buffer is bound with size {bound_size} where the shader expects {shader_size} in group[{group_index}] compact index {compact_index}")]
+#[error(
+    "In bind group index {group_index}, the buffer bound at binding index {binding_index} \
+     is bound with size {bound_size} where the shader expects {shader_size}."
+)]
 pub struct LateMinBufferBindingSizeMismatch {
     pub group_index: u32,
-    pub compact_index: usize,
+    pub binding_index: u32,
     pub shader_size: wgt::BufferAddress,
     pub bound_size: wgt::BufferAddress,
 }

wgpu-core/src/command/bind.rs

@@ -291,6 +291,7 @@ pub enum BinderError {
 
 #[derive(Debug)]
 struct LateBufferBinding {
+    binding_index: u32,
     shader_expect_size: wgt::BufferAddress,
     bound_size: wgt::BufferAddress,
 }
@@ -347,20 +348,25 @@ impl Binder {
         self.manager.update_expectations(&new.bind_group_layouts);
 
         // Update the buffer binding sizes that are required by shaders.
+
         for (payload, late_group) in self.payloads.iter_mut().zip(late_sized_buffer_groups) {
             payload.late_bindings_effective_count = late_group.shader_sizes.len();
+            // Update entries that already exist as the bind group was bound before the pipeline
+            // was bound.
             for (late_binding, &shader_expect_size) in payload
                 .late_buffer_bindings
                 .iter_mut()
                 .zip(late_group.shader_sizes.iter())
             {
                 late_binding.shader_expect_size = shader_expect_size;
             }
+            // Add new entries for the bindings that were not known when the bind group was bound.
             if late_group.shader_sizes.len() > payload.late_buffer_bindings.len() {
                 for &shader_expect_size in
                     late_group.shader_sizes[payload.late_buffer_bindings.len()..].iter()
                 {
                     payload.late_buffer_bindings.push(LateBufferBinding {
+                        binding_index: 0,
                         shader_expect_size,
                         bound_size: 0,
                     });
@@ -389,20 +395,27 @@ impl Binder {
 
         // Fill out the actual binding sizes for buffers,
         // whose layout doesn't specify `min_binding_size`.
-        for (late_binding, late_size) in payload
+
+        // Update entries that already exist as the pipeline was bound before the group
+        // was bound.
+        for (late_binding, late_info) in payload
             .late_buffer_bindings
             .iter_mut()
-            .zip(bind_group.late_buffer_binding_sizes.iter())
+            .zip(bind_group.late_buffer_binding_infos.iter())
         {
-            late_binding.bound_size = late_size.get();
+            late_binding.binding_index = late_info.binding_index;
+            late_binding.bound_size = late_info.size.get();
         }
-        if bind_group.late_buffer_binding_sizes.len() > payload.late_buffer_bindings.len() {
-            for late_size in
-                bind_group.late_buffer_binding_sizes[payload.late_buffer_bindings.len()..].iter()
+
+        // Add new entries for the bindings that were not known when the pipeline was bound.
+        if bind_group.late_buffer_binding_infos.len() > payload.late_buffer_bindings.len() {
+            for late_info in
+                bind_group.late_buffer_binding_infos[payload.late_buffer_bindings.len()..].iter()
             {
                 payload.late_buffer_bindings.push(LateBufferBinding {
+                    binding_index: late_info.binding_index,
                     shader_expect_size: 0,
-                    bound_size: late_size.get(),
+                    bound_size: late_info.size.get(),
                 });
             }
         }
@@ -469,15 +482,13 @@ impl Binder {
     ) -> Result<(), LateMinBufferBindingSizeMismatch> {
         for group_index in self.manager.list_active() {
             let payload = &self.payloads[group_index];
-            for (compact_index, late_binding) in payload.late_buffer_bindings
-                [..payload.late_bindings_effective_count]
-                .iter()
-                .enumerate()
+            for late_binding in
+                &payload.late_buffer_bindings[..payload.late_bindings_effective_count]
             {
                 if late_binding.bound_size < late_binding.shader_expect_size {
                     return Err(LateMinBufferBindingSizeMismatch {
                         group_index: group_index as u32,
-                        compact_index,
+                        binding_index: late_binding.binding_index,
                         shader_size: late_binding.shader_expect_size,
                         bound_size: late_binding.bound_size,
                     });

wgpu-core/src/device/resource.rs

@@ -25,7 +25,9 @@ use wgt::{
 use crate::device::trace;
 use crate::{
     api_log,
-    binding_model::{self, BindGroup, BindGroupLayout, BindGroupLayoutEntryError},
+    binding_model::{
+        self, BindGroup, BindGroupLateBufferBindingInfo, BindGroupLayout, BindGroupLayoutEntryError,
+    },
     command, conv,
     device::{
         bgl, create_validator, life::WaitIdleError, map_buffer, AttachmentData,
@@ -3279,10 +3281,16 @@ impl Device {
             .map_err(|e| self.handle_hal_error(e))?;
 
         // collect in the order of BGL iteration
-        let late_buffer_binding_sizes = layout
+        let late_buffer_binding_infos = layout
             .entries
             .indices()
-            .flat_map(|binding| late_buffer_binding_sizes.get(&binding).cloned())
+            .flat_map(|binding| {
+                let size = late_buffer_binding_sizes.get(&binding).cloned()?;
+                Some(BindGroupLateBufferBindingInfo {
+                    binding_index: binding,
+                    size,
+                })
+            })
             .collect();
 
         let bind_group = BindGroup {
@@ -3295,7 +3303,7 @@ impl Device {
             used_buffer_ranges,
             used_texture_ranges,
             dynamic_binding_info,
-            late_buffer_binding_sizes,
+            late_buffer_binding_infos,
         };
 
         let bind_group = Arc::new(bind_group);