From f2cdf1fb56b45ebbbf0aa384e19d71fa5b5e4151 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89lie=20Michel?= Date: Fri, 15 Nov 2024 19:57:20 +0100 Subject: [PATCH 1/4] Improve binding error --- wgpu-core/src/validation.rs | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 52273b4f85..5af33db684 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -140,8 +140,13 @@ 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: BindingType, + shader: ResourceType, + }, #[error("Storage class {binding:?} doesn't match the shader {shader:?}")] WrongAddressSpace { binding: naga::AddressSpace, @@ -378,7 +383,12 @@ impl Resource { } min_binding_size } - _ => return Err(BindingError::WrongType), + _ => { + return Err(BindingError::WrongType { + binding: entry.ty, + shader: self.ty, + }) + } }; match min_size { Some(non_zero) if non_zero < size => { @@ -396,7 +406,12 @@ impl Resource { return Err(BindingError::WrongSamplerComparison); } } - _ => return Err(BindingError::WrongType), + _ => { + return Err(BindingError::WrongType { + binding: entry.ty, + shader: self.ty, + }) + } }, ResourceType::Texture { dim, @@ -478,7 +493,12 @@ impl Resource { access: naga_access, } } - _ => return Err(BindingError::WrongType), + _ => { + return Err(BindingError::WrongType { + binding: entry.ty, + shader: self.ty, + }) + } }; if class != expected_class { return Err(BindingError::WrongTextureClass { @@ -504,7 +524,12 @@ impl Resource { naga::AddressSpace::Storage { access } => wgt::BufferBindingType::Storage { read_only: access == naga::StorageAccess::LOAD, }, - _ => return Err(BindingError::WrongType), + _ => { + return Err(BindingError::WrongType { + binding: entry.ty, + shader: self.ty, + }) + } }, has_dynamic_offset: false, min_binding_size: Some(size), From 424f416126782eee98ec1abe2cd3f3260815ff03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89lie=20Michel?= Date: Fri, 15 Nov 2024 20:23:23 +0100 Subject: [PATCH 2/4] Introduce a new BindingTypeName enum --- wgpu-core/src/validation.rs | 66 ++++++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 5af33db684..eb48ce9205 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -20,6 +20,33 @@ enum ResourceType { AccelerationStructure, } +#[derive(Clone, Debug)] +pub enum BindingTypeName { + Buffer, + Texture, + Sampler, + AccelerationStructure, +} + +fn map_resource_to_type_name(ty: &ResourceType) -> BindingTypeName { + match ty { + ResourceType::Buffer{ .. } => BindingTypeName::Buffer, + ResourceType::Texture{ .. } => BindingTypeName::Texture, + ResourceType::Sampler{ .. } => BindingTypeName::Sampler, + ResourceType::AccelerationStructure{ .. } => BindingTypeName::AccelerationStructure, + } +} + +fn map_binding_to_type_name(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,18 +167,20 @@ pub enum BindingError { Missing, #[error("Visibility flags don't include the shader stage")] Invisible, - #[error( - "Type on the shader side ({shader:?}) does not match the pipeline binding ({binding:?})" - )] + #[error("Type on the shader side ({shader:?}) does not match the pipeline binding ({binding:?})")] WrongType { - binding: BindingType, - shader: ResourceType, + 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, @@ -385,8 +414,8 @@ impl Resource { } _ => { return Err(BindingError::WrongType { - binding: entry.ty, - shader: self.ty, + binding: map_binding_to_type_name(&entry.ty), + shader: map_resource_to_type_name(&self.ty), }) } }; @@ -408,8 +437,8 @@ impl Resource { } _ => { return Err(BindingError::WrongType { - binding: entry.ty, - shader: self.ty, + binding: map_binding_to_type_name(&entry.ty), + shader: map_resource_to_type_name(&self.ty), }) } }, @@ -495,8 +524,8 @@ impl Resource { } _ => { return Err(BindingError::WrongType { - binding: entry.ty, - shader: self.ty, + binding: map_binding_to_type_name(&entry.ty), + shader: map_resource_to_type_name(&self.ty), }) } }; @@ -507,7 +536,15 @@ impl Resource { }); } } - ResourceType::AccelerationStructure => (), + ResourceType::AccelerationStructure => match entry.ty { + BindingType::AccelerationStructure => (), + _ => { + return Err(BindingError::WrongType { + binding: map_binding_to_type_name(&entry.ty), + shader: map_resource_to_type_name(&self.ty), + }) + } + }, }; Ok(()) @@ -525,9 +562,8 @@ impl Resource { read_only: access == naga::StorageAccess::LOAD, }, _ => { - return Err(BindingError::WrongType { - binding: entry.ty, - shader: self.ty, + return Err(BindingError::WrongBufferAddressSpace { + space: self.class, }) } }, From 4a9c7bfa26eba6b1df4d8fa28a61fc071b4f7b62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89lie=20Michel?= Date: Fri, 15 Nov 2024 20:32:56 +0100 Subject: [PATCH 3/4] Fix formatting --- wgpu-core/src/validation.rs | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index eb48ce9205..8280d0169d 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -30,20 +30,20 @@ pub enum BindingTypeName { fn map_resource_to_type_name(ty: &ResourceType) -> BindingTypeName { match ty { - ResourceType::Buffer{ .. } => BindingTypeName::Buffer, - ResourceType::Texture{ .. } => BindingTypeName::Texture, - ResourceType::Sampler{ .. } => BindingTypeName::Sampler, - ResourceType::AccelerationStructure{ .. } => BindingTypeName::AccelerationStructure, + ResourceType::Buffer { .. } => BindingTypeName::Buffer, + ResourceType::Texture { .. } => BindingTypeName::Texture, + ResourceType::Sampler { .. } => BindingTypeName::Sampler, + ResourceType::AccelerationStructure { .. } => BindingTypeName::AccelerationStructure, } } fn map_binding_to_type_name(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, + BindingType::Buffer { .. } => BindingTypeName::Buffer, + BindingType::Texture { .. } => BindingTypeName::Texture, + BindingType::StorageTexture { .. } => BindingTypeName::Texture, + BindingType::Sampler { .. } => BindingTypeName::Sampler, + BindingType::AccelerationStructure { .. } => BindingTypeName::AccelerationStructure, } } @@ -167,7 +167,9 @@ pub enum BindingError { Missing, #[error("Visibility flags don't include the shader stage")] Invisible, - #[error("Type on the shader side ({shader:?}) does not match the pipeline binding ({binding:?})")] + #[error( + "Type on the shader side ({shader:?}) does not match the pipeline binding ({binding:?})" + )] WrongType { binding: BindingTypeName, shader: BindingTypeName, @@ -178,9 +180,7 @@ pub enum BindingError { shader: naga::AddressSpace, }, #[error("Address space {space:?} is not a valid Buffer address space")] - WrongBufferAddressSpace { - space: naga::AddressSpace, - }, + 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, @@ -561,11 +561,7 @@ impl Resource { naga::AddressSpace::Storage { access } => wgt::BufferBindingType::Storage { read_only: access == naga::StorageAccess::LOAD, }, - _ => { - return Err(BindingError::WrongBufferAddressSpace { - space: self.class, - }) - } + _ => return Err(BindingError::WrongBufferAddressSpace { space: self.class }), }, has_dynamic_offset: false, min_binding_size: Some(size), From 6045e88398899c9eccd017ea96727bf99cc5d864 Mon Sep 17 00:00:00 2001 From: Elie Michel Date: Fri, 15 Nov 2024 22:35:00 +0100 Subject: [PATCH 4/4] Use From trait instead of map functions for BindingTypeName --- wgpu-core/src/validation.rs | 46 ++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 8280d0169d..b1c0051902 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -28,22 +28,26 @@ pub enum BindingTypeName { AccelerationStructure, } -fn map_resource_to_type_name(ty: &ResourceType) -> BindingTypeName { - match ty { - ResourceType::Buffer { .. } => BindingTypeName::Buffer, - ResourceType::Texture { .. } => BindingTypeName::Texture, - ResourceType::Sampler { .. } => BindingTypeName::Sampler, - ResourceType::AccelerationStructure { .. } => BindingTypeName::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, + } } } -fn map_binding_to_type_name(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, +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, + } } } @@ -414,8 +418,8 @@ impl Resource { } _ => { return Err(BindingError::WrongType { - binding: map_binding_to_type_name(&entry.ty), - shader: map_resource_to_type_name(&self.ty), + binding: (&entry.ty).into(), + shader: (&self.ty).into(), }) } }; @@ -437,8 +441,8 @@ impl Resource { } _ => { return Err(BindingError::WrongType { - binding: map_binding_to_type_name(&entry.ty), - shader: map_resource_to_type_name(&self.ty), + binding: (&entry.ty).into(), + shader: (&self.ty).into(), }) } }, @@ -524,8 +528,8 @@ impl Resource { } _ => { return Err(BindingError::WrongType { - binding: map_binding_to_type_name(&entry.ty), - shader: map_resource_to_type_name(&self.ty), + binding: (&entry.ty).into(), + shader: (&self.ty).into(), }) } }; @@ -540,8 +544,8 @@ impl Resource { BindingType::AccelerationStructure => (), _ => { return Err(BindingError::WrongType { - binding: map_binding_to_type_name(&entry.ty), - shader: map_resource_to_type_name(&self.ty), + binding: (&entry.ty).into(), + shader: (&self.ty).into(), }) } },