Improve binding error (gfx-rs#6553)

* Improve binding error * Introduce a new BindingTypeName enum * Fix formatting * Use From trait instead of map functions for BindingTypeName * Update CHANGELOG.md
shards-lang · Nov 23, 2024 · c54a159 · c54a159
1 parent b75f46c
commit c54a159
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -116,6 +116,7 @@ By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148]
 
 - Make `Surface::as_hal` take an immutable reference to the surface. By @jerzywilczek in [#9999](https://github.com/gfx-rs/wgpu/pull/9999)
 - Add actual sample type to `CreateBindGroupError::InvalidTextureSampleType` error message. By @ErichDonGubler in [#6530](https://github.com/gfx-rs/wgpu/pull/6530).
+- Improve binding error to give a clearer message when there is a mismatch between resource binding as it is in the shader and as it is in the binding layout. By @eliemichel in [#6553](https://github.com/gfx-rs/wgpu/pull/6553).
 
 #### HAL
 

diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs
@@ -20,6 +20,37 @@ enum ResourceType {
     AccelerationStructure,
 }
 
+#[derive(Clone, Debug)]
+pub enum BindingTypeName {
+    Buffer,
+    Texture,
+    Sampler,
+    AccelerationStructure,
+}
+
+impl From<&ResourceType> for BindingTypeName {
+    fn from(ty: &ResourceType) -> BindingTypeName {
+        match ty {
+            ResourceType::Buffer { .. } => BindingTypeName::Buffer,
+            ResourceType::Texture { .. } => BindingTypeName::Texture,
+            ResourceType::Sampler { .. } => BindingTypeName::Sampler,
+            ResourceType::AccelerationStructure { .. } => BindingTypeName::AccelerationStructure,
+        }
+    }
+}
+
+impl From<&BindingType> for BindingTypeName {
+    fn from(ty: &BindingType) -> BindingTypeName {
+        match ty {
+            BindingType::Buffer { .. } => BindingTypeName::Buffer,
+            BindingType::Texture { .. } => BindingTypeName::Texture,
+            BindingType::StorageTexture { .. } => BindingTypeName::Texture,
+            BindingType::Sampler { .. } => BindingTypeName::Sampler,
+            BindingType::AccelerationStructure { .. } => BindingTypeName::AccelerationStructure,
+        }
+    }
+}
+
 #[derive(Debug)]
 struct Resource {
     #[allow(unused)]
@@ -140,13 +171,20 @@ pub enum BindingError {
     Missing,
     #[error("Visibility flags don't include the shader stage")]
     Invisible,
-    #[error("Type on the shader side does not match the pipeline binding")]
-    WrongType,
+    #[error(
+        "Type on the shader side ({shader:?}) does not match the pipeline binding ({binding:?})"
+    )]
+    WrongType {
+        binding: BindingTypeName,
+        shader: BindingTypeName,
+    },
     #[error("Storage class {binding:?} doesn't match the shader {shader:?}")]
     WrongAddressSpace {
         binding: naga::AddressSpace,
         shader: naga::AddressSpace,
     },
+    #[error("Address space {space:?} is not a valid Buffer address space")]
+    WrongBufferAddressSpace { space: naga::AddressSpace },
     #[error("Buffer structure size {buffer_size}, added to one element of an unbound array, if it's the last field, ended up greater than the given `min_binding_size`, which is {min_binding_size}")]
     WrongBufferSize {
         buffer_size: wgt::BufferSize,
@@ -378,7 +416,12 @@ impl Resource {
                         }
                         min_binding_size
                     }
-                    _ => return Err(BindingError::WrongType),
+                    _ => {
+                        return Err(BindingError::WrongType {
+                            binding: (&entry.ty).into(),
+                            shader: (&self.ty).into(),
+                        })
+                    }
                 };
                 match min_size {
                     Some(non_zero) if non_zero < size => {
@@ -396,7 +439,12 @@ impl Resource {
                         return Err(BindingError::WrongSamplerComparison);
                     }
                 }
-                _ => return Err(BindingError::WrongType),
+                _ => {
+                    return Err(BindingError::WrongType {
+                        binding: (&entry.ty).into(),
+                        shader: (&self.ty).into(),
+                    })
+                }
             },
             ResourceType::Texture {
                 dim,
@@ -478,7 +526,12 @@ impl Resource {
                             access: naga_access,
                         }
                     }
-                    _ => return Err(BindingError::WrongType),
+                    _ => {
+                        return Err(BindingError::WrongType {
+                            binding: (&entry.ty).into(),
+                            shader: (&self.ty).into(),
+                        })
+                    }
                 };
                 if class != expected_class {
                     return Err(BindingError::WrongTextureClass {
@@ -487,7 +540,15 @@ impl Resource {
                     });
                 }
             }
-            ResourceType::AccelerationStructure => (),
+            ResourceType::AccelerationStructure => match entry.ty {
+                BindingType::AccelerationStructure => (),
+                _ => {
+                    return Err(BindingError::WrongType {
+                        binding: (&entry.ty).into(),
+                        shader: (&self.ty).into(),
+                    })
+                }
+            },
         };
 
         Ok(())
@@ -504,7 +565,7 @@ impl Resource {
                     naga::AddressSpace::Storage { access } => wgt::BufferBindingType::Storage {
                         read_only: access == naga::StorageAccess::LOAD,
                     },
-                    _ => return Err(BindingError::WrongType),
+                    _ => return Err(BindingError::WrongBufferAddressSpace { space: self.class }),
                 },
                 has_dynamic_offset: false,
                 min_binding_size: Some(size),