Skip to content

Commit c54a159

Browse files
authored
Improve binding error (#6553)
* Improve binding error * Introduce a new BindingTypeName enum * Fix formatting * Use From trait instead of map functions for BindingTypeName * Update CHANGELOG.md
1 parent b75f46c commit c54a159

File tree

2 files changed

+69
-7
lines changed

2 files changed

+69
-7
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148]
116116

117117
- Make `Surface::as_hal` take an immutable reference to the surface. By @jerzywilczek in [#9999](https://github.com/gfx-rs/wgpu/pull/9999)
118118
- Add actual sample type to `CreateBindGroupError::InvalidTextureSampleType` error message. By @ErichDonGubler in [#6530](https://github.com/gfx-rs/wgpu/pull/6530).
119+
- 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).
119120

120121
#### HAL
121122

wgpu-core/src/validation.rs

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,37 @@ enum ResourceType {
2020
AccelerationStructure,
2121
}
2222

23+
#[derive(Clone, Debug)]
24+
pub enum BindingTypeName {
25+
Buffer,
26+
Texture,
27+
Sampler,
28+
AccelerationStructure,
29+
}
30+
31+
impl From<&ResourceType> for BindingTypeName {
32+
fn from(ty: &ResourceType) -> BindingTypeName {
33+
match ty {
34+
ResourceType::Buffer { .. } => BindingTypeName::Buffer,
35+
ResourceType::Texture { .. } => BindingTypeName::Texture,
36+
ResourceType::Sampler { .. } => BindingTypeName::Sampler,
37+
ResourceType::AccelerationStructure { .. } => BindingTypeName::AccelerationStructure,
38+
}
39+
}
40+
}
41+
42+
impl From<&BindingType> for BindingTypeName {
43+
fn from(ty: &BindingType) -> BindingTypeName {
44+
match ty {
45+
BindingType::Buffer { .. } => BindingTypeName::Buffer,
46+
BindingType::Texture { .. } => BindingTypeName::Texture,
47+
BindingType::StorageTexture { .. } => BindingTypeName::Texture,
48+
BindingType::Sampler { .. } => BindingTypeName::Sampler,
49+
BindingType::AccelerationStructure { .. } => BindingTypeName::AccelerationStructure,
50+
}
51+
}
52+
}
53+
2354
#[derive(Debug)]
2455
struct Resource {
2556
#[allow(unused)]
@@ -140,13 +171,20 @@ pub enum BindingError {
140171
Missing,
141172
#[error("Visibility flags don't include the shader stage")]
142173
Invisible,
143-
#[error("Type on the shader side does not match the pipeline binding")]
144-
WrongType,
174+
#[error(
175+
"Type on the shader side ({shader:?}) does not match the pipeline binding ({binding:?})"
176+
)]
177+
WrongType {
178+
binding: BindingTypeName,
179+
shader: BindingTypeName,
180+
},
145181
#[error("Storage class {binding:?} doesn't match the shader {shader:?}")]
146182
WrongAddressSpace {
147183
binding: naga::AddressSpace,
148184
shader: naga::AddressSpace,
149185
},
186+
#[error("Address space {space:?} is not a valid Buffer address space")]
187+
WrongBufferAddressSpace { space: naga::AddressSpace },
150188
#[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}")]
151189
WrongBufferSize {
152190
buffer_size: wgt::BufferSize,
@@ -378,7 +416,12 @@ impl Resource {
378416
}
379417
min_binding_size
380418
}
381-
_ => return Err(BindingError::WrongType),
419+
_ => {
420+
return Err(BindingError::WrongType {
421+
binding: (&entry.ty).into(),
422+
shader: (&self.ty).into(),
423+
})
424+
}
382425
};
383426
match min_size {
384427
Some(non_zero) if non_zero < size => {
@@ -396,7 +439,12 @@ impl Resource {
396439
return Err(BindingError::WrongSamplerComparison);
397440
}
398441
}
399-
_ => return Err(BindingError::WrongType),
442+
_ => {
443+
return Err(BindingError::WrongType {
444+
binding: (&entry.ty).into(),
445+
shader: (&self.ty).into(),
446+
})
447+
}
400448
},
401449
ResourceType::Texture {
402450
dim,
@@ -478,7 +526,12 @@ impl Resource {
478526
access: naga_access,
479527
}
480528
}
481-
_ => return Err(BindingError::WrongType),
529+
_ => {
530+
return Err(BindingError::WrongType {
531+
binding: (&entry.ty).into(),
532+
shader: (&self.ty).into(),
533+
})
534+
}
482535
};
483536
if class != expected_class {
484537
return Err(BindingError::WrongTextureClass {
@@ -487,7 +540,15 @@ impl Resource {
487540
});
488541
}
489542
}
490-
ResourceType::AccelerationStructure => (),
543+
ResourceType::AccelerationStructure => match entry.ty {
544+
BindingType::AccelerationStructure => (),
545+
_ => {
546+
return Err(BindingError::WrongType {
547+
binding: (&entry.ty).into(),
548+
shader: (&self.ty).into(),
549+
})
550+
}
551+
},
491552
};
492553

493554
Ok(())
@@ -504,7 +565,7 @@ impl Resource {
504565
naga::AddressSpace::Storage { access } => wgt::BufferBindingType::Storage {
505566
read_only: access == naga::StorageAccess::LOAD,
506567
},
507-
_ => return Err(BindingError::WrongType),
568+
_ => return Err(BindingError::WrongBufferAddressSpace { space: self.class }),
508569
},
509570
has_dynamic_offset: false,
510571
min_binding_size: Some(size),

0 commit comments

Comments
 (0)