Merge branch 'trunk' into ruint64

atlv24 · Nov 23, 2024 · ccc6956 · ccc6956
2 parents 9461742 + c54a159
commit ccc6956
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -118,6 +118,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/instance.rs b/wgpu-core/src/instance.rs
@@ -657,7 +657,7 @@ pub enum GetSurfaceSupportError {
 
 #[derive(Clone, Debug, Error)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-/// Error when requesting a device from the adaptor
+/// Error when requesting a device from the adapter
 #[non_exhaustive]
 pub enum RequestDeviceError {
     #[error(transparent)]

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,
@@ -380,7 +418,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 => {
@@ -398,7 +441,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,
@@ -480,7 +528,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 {
@@ -489,7 +542,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(())
@@ -506,7 +567,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),

diff --git a/wgpu-hal/src/dx12/adapter.rs b/wgpu-hal/src/dx12/adapter.rs
@@ -293,6 +293,7 @@ impl super::Adapter {
             }
         };
 
+        // these should always be available on d3d12
         let mut features = wgt::Features::empty()
             | wgt::Features::DEPTH_CLIP_CONTROL
             | wgt::Features::DEPTH32FLOAT_STENCIL8
@@ -315,7 +316,8 @@ impl super::Adapter {
             | wgt::Features::SHADER_PRIMITIVE_INDEX
             | wgt::Features::RG11B10UFLOAT_RENDERABLE
             | wgt::Features::DUAL_SOURCE_BLENDING
-            | wgt::Features::TEXTURE_FORMAT_NV12;
+            | wgt::Features::TEXTURE_FORMAT_NV12
+            | wgt::Features::FLOAT32_FILTERABLE;
 
         //TODO: in order to expose this, we need to run a compute shader
         // that extract the necessary statistics out of the D3D12 result.
@@ -410,9 +412,6 @@ impl super::Adapter {
             atomic_int64_on_typed_resource_supported,
         );
 
-        // float32-filterable should always be available on d3d12
-        features.set(wgt::Features::FLOAT32_FILTERABLE, true);
-
         // TODO: Determine if IPresentationManager is supported
         let presentation_timer = auxil::dxgi::time::PresentationTimer::new_dxgi();