Skip to content

Commit

Permalink
validation: More detailed on incompatible BGL (#4826)
Browse files Browse the repository at this point in the history
  • Loading branch information
scoopr authored Dec 17, 2023
1 parent ba56dd2 commit 159ac9e
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 13 deletions.
86 changes: 86 additions & 0 deletions wgpu-core/src/command/bind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,79 @@ mod compat {
fn is_incompatible(&self) -> bool {
self.expected.is_none() || !self.is_valid()
}

// Describe how bind group layouts are incompatible, for validation
// error message.
fn bgl_diff(&self) -> Vec<String> {
let mut diff = Vec::new();

if let Some(expected_bgl) = self.expected.as_ref() {
diff.push(format!(
"Should be compatible with bind group layout with label = `{}`",
expected_bgl.label()
));
if let Some(assigned_bgl) = self.assigned.as_ref() {
diff.push(format!(
"Assigned bind group layout with label = `{}`",
assigned_bgl.label()
));
for (id, e_entry) in &expected_bgl.entries {
if let Some(a_entry) = assigned_bgl.entries.get(id) {
if a_entry.binding != e_entry.binding {
diff.push(format!(
"Entry {id} binding expected {}, got {}",
e_entry.binding, a_entry.binding
));
}
if a_entry.count != e_entry.count {
diff.push(format!(
"Entry {id} count expected {:?}, got {:?}",
e_entry.count, a_entry.count
));
}
if a_entry.ty != e_entry.ty {
diff.push(format!(
"Entry {id} type expected {:?}, got {:?}",
e_entry.ty, a_entry.ty
));
}
if a_entry.visibility != e_entry.visibility {
diff.push(format!(
"Entry {id} visibility expected {:?}, got {:?}",
e_entry.visibility, a_entry.visibility
));
}
} else {
diff.push(format!("Entry {id} not found in assigned bindgroup layout"))
}
}

assigned_bgl.entries.iter().for_each(|(id, _e_entry)| {
if !expected_bgl.entries.contains_key(id) {
diff.push(format!("Entry {id} not found in expected bindgroup layout"))
}
});
} else {
diff.push(
"Assigned bindgroup layout is implicit, expected explicit".to_owned(),
);
}
} else if let Some(assigned_bgl) = self.assigned.as_ref() {
diff.push(format!(
"Assigned bind group layout = `{}`",
assigned_bgl.label()
));
diff.push(
"Assigned bindgroup layout is not implicit, expected implicit".to_owned(),
);
}

if diff.is_empty() {
diff.push("But no differences found? (internal error)".to_owned())
}

diff
}
}

#[derive(Debug, Default)]
Expand Down Expand Up @@ -121,6 +194,15 @@ mod compat {
}
})
}

pub fn bgl_diff(&self) -> Vec<String> {
for e in &self.entries {
if !e.is_valid() {
return e.bgl_diff();
}
}
vec![String::from("No differences detected? (internal error)")]
}
}
}

Expand Down Expand Up @@ -274,6 +356,10 @@ impl<A: HalApi> Binder<A> {
self.manager.invalid_mask()
}

pub(super) fn bgl_diff(&self) -> Vec<String> {
self.manager.bgl_diff()
}

/// Scan active buffer bindings corresponding to layouts without `min_binding_size` specified.
pub(super) fn check_late_buffer_bindings(
&self,
Expand Down
18 changes: 11 additions & 7 deletions wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,8 @@ pub struct ComputePassDescriptor<'a> {
pub enum DispatchError {
#[error("Compute pipeline must be set")]
MissingPipeline,
#[error("The pipeline layout, associated with the current compute pipeline, contains a bind group layout at index {index} which is incompatible with the bind group layout associated with the bind group at {index}")]
IncompatibleBindGroup {
index: u32,
//expected: BindGroupLayoutId,
//provided: Option<(BindGroupLayoutId, BindGroupId)>,
},
#[error("Incompatible bind group at index {index} in the current compute pipeline")]
IncompatibleBindGroup { index: u32, diff: Vec<String> },
#[error(
"Each current dispatch group size dimension ({current:?}) must be less or equal to {limit}"
)]
Expand Down Expand Up @@ -245,6 +241,11 @@ impl PrettyError for ComputePassErrorInner {
Self::InvalidIndirectBuffer(id) => {
fmt.buffer_label(&id);
}
Self::Dispatch(DispatchError::IncompatibleBindGroup { ref diff, .. }) => {
for d in diff {
fmt.note(&d);
}
}
_ => {}
};
}
Expand Down Expand Up @@ -291,8 +292,11 @@ impl<A: HalApi> State<A> {
let bind_mask = self.binder.invalid_mask();
if bind_mask != 0 {
//let (expected, provided) = self.binder.entries[index as usize].info();
let index = bind_mask.trailing_zeros();

return Err(DispatchError::IncompatibleBindGroup {
index: bind_mask.trailing_zeros(),
index,
diff: self.binder.bgl_diff(),
});
}
if self.pipeline.is_none() {
Expand Down
8 changes: 2 additions & 6 deletions wgpu-core/src/command/draw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,8 @@ pub enum DrawError {
MissingVertexBuffer { index: u32 },
#[error("Index buffer must be set")]
MissingIndexBuffer,
#[error("The pipeline layout, associated with the current render pipeline, contains a bind group layout at index {index} which is incompatible with the bind group layout associated with the bind group at {index}")]
IncompatibleBindGroup {
index: u32,
//expected: BindGroupLayoutId,
//provided: Option<(BindGroupLayoutId, BindGroupId)>,
},
#[error("Incompatible bind group at index {index} in the current render pipeline")]
IncompatibleBindGroup { index: u32, diff: Vec<String> },
#[error("Vertex {last_vertex} extends beyond limit {vertex_limit} imposed by the buffer in slot {slot}. Did you bind the correct `Vertex` step-rate vertex buffer?")]
VertexBeyondLimit {
last_vertex: u32,
Expand Down
6 changes: 6 additions & 0 deletions wgpu-core/src/command/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ impl<A: HalApi> State<A> {
//let (expected, provided) = self.binder.entries[index as usize].info();
return Err(DrawError::IncompatibleBindGroup {
index: bind_mask.trailing_zeros(),
diff: self.binder.bgl_diff(),
});
}
if self.pipeline.is_none() {
Expand Down Expand Up @@ -643,6 +644,11 @@ impl PrettyError for RenderPassErrorInner {
if let Self::InvalidAttachment(id) = *self {
fmt.texture_view_label_with_key(&id, "attachment");
};
if let Self::Draw(DrawError::IncompatibleBindGroup { diff, .. }) = self {
for d in diff {
fmt.note(&d);
}
};
}
}

Expand Down

0 comments on commit 159ac9e

Please sign in to comment.