From 08089f18f94bbdc74cf7c1325b1be93824821962 Mon Sep 17 00:00:00 2001 From: Mikko Lehtonen Date: Mon, 4 Dec 2023 18:05:17 +0200 Subject: [PATCH 1/5] validation: More detailed on incompatible BGL When encountering a incompatible BindGroupLayout on RenderPipelines, attempt to report the actual difference causing the incompatibility. --- wgpu-core/src/command/bind.rs | 58 +++++++++++++++++++++++++++++++++ wgpu-core/src/command/draw.rs | 3 +- wgpu-core/src/command/render.rs | 6 ++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/wgpu-core/src/command/bind.rs b/wgpu-core/src/command/bind.rs index 66b95a6df9..7b694729af 100644 --- a/wgpu-core/src/command/bind.rs +++ b/wgpu-core/src/command/bind.rs @@ -53,6 +53,50 @@ mod compat { fn is_incompatible(&self) -> bool { self.expected.is_none() || !self.is_valid() } + + fn bgl_diff(&self) -> Vec { + let mut diff = vec![]; + + if let Some(expected_bgl) = self.expected.as_ref() { + if let Some(assigned_bgl) = self.assigned.as_ref() { + + 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("Assigned bindgroup layout is not implicit, expected implicit".to_owned()); + } + + + diff + } } #[derive(Debug, Default)] @@ -121,6 +165,16 @@ mod compat { } }) } + + pub fn bgl_diff(&self) -> Vec { + for e in &self.entries { + if !e.is_valid() { + + return e.bgl_diff() + } + } + vec![format!("huh?!")] + } } } @@ -274,6 +328,10 @@ impl Binder { self.manager.invalid_mask() } + pub(super) fn bgl_diff(&self) -> Vec { + self.manager.bgl_diff() + } + /// Scan active buffer bindings corresponding to layouts without `min_binding_size` specified. pub(super) fn check_late_buffer_bindings( &self, diff --git a/wgpu-core/src/command/draw.rs b/wgpu-core/src/command/draw.rs index c5b0bb7527..a51f8fcb58 100644 --- a/wgpu-core/src/command/draw.rs +++ b/wgpu-core/src/command/draw.rs @@ -25,9 +25,10 @@ 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}")] + #[error("The pipeline layout, associated with the current render pipeline, contains a incompatible bind group layout at index {index}")] IncompatibleBindGroup { index: u32, + diff: Vec //expected: BindGroupLayoutId, //provided: Option<(BindGroupLayoutId, BindGroupId)>, }, diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 0ed8f46aa9..4bd0540f92 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -441,6 +441,7 @@ impl State { //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() { @@ -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); + } + }; } } From 8bdb0f2471742bed7593541588e1f4f047e14ec0 Mon Sep 17 00:00:00 2001 From: Mikko Lehtonen Date: Mon, 11 Dec 2023 16:43:31 +0200 Subject: [PATCH 2/5] Nits --- wgpu-core/src/command/bind.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/wgpu-core/src/command/bind.rs b/wgpu-core/src/command/bind.rs index 7b694729af..01d7f22252 100644 --- a/wgpu-core/src/command/bind.rs +++ b/wgpu-core/src/command/bind.rs @@ -55,7 +55,7 @@ mod compat { } fn bgl_diff(&self) -> Vec { - let mut diff = vec![]; + let mut diff = Vec::new(); if let Some(expected_bgl) = self.expected.as_ref() { if let Some(assigned_bgl) = self.assigned.as_ref() { @@ -169,11 +169,10 @@ mod compat { pub fn bgl_diff(&self) -> Vec { for e in &self.entries { if !e.is_valid() { - return e.bgl_diff() } } - vec![format!("huh?!")] + vec![String::from("No differences detected?")] } } } From 526148dd30b2d4d4748b6624fa5ac1f4369c3a5e Mon Sep 17 00:00:00 2001 From: Mikko Lehtonen Date: Mon, 11 Dec 2023 16:43:53 +0200 Subject: [PATCH 3/5] fmt --- wgpu-core/src/command/bind.rs | 53 ++++++++++++++++++++------------- wgpu-core/src/command/draw.rs | 5 ++-- wgpu-core/src/command/render.rs | 2 +- 3 files changed, 35 insertions(+), 25 deletions(-) diff --git a/wgpu-core/src/command/bind.rs b/wgpu-core/src/command/bind.rs index 01d7f22252..fb2a91890e 100644 --- a/wgpu-core/src/command/bind.rs +++ b/wgpu-core/src/command/bind.rs @@ -59,24 +59,33 @@ mod compat { if let Some(expected_bgl) = self.expected.as_ref() { if let Some(assigned_bgl) = self.assigned.as_ref() { - 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} 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")) } } @@ -86,15 +95,17 @@ mod compat { diff.push(format!("Entry {id} not found in expected bindgroup layout")) } }); - } else { - diff.push("Assigned bindgroup layout is implicit, expected explicit".to_owned()); + diff.push( + "Assigned bindgroup layout is implicit, expected explicit".to_owned(), + ); } } else if let Some(_assigned_bgl) = self.assigned.as_ref() { - diff.push("Assigned bindgroup layout is not implicit, expected implicit".to_owned()); + diff.push( + "Assigned bindgroup layout is not implicit, expected implicit".to_owned(), + ); } - diff } } @@ -169,8 +180,8 @@ mod compat { pub fn bgl_diff(&self) -> Vec { for e in &self.entries { if !e.is_valid() { - return e.bgl_diff() - } + return e.bgl_diff(); + } } vec![String::from("No differences detected?")] } diff --git a/wgpu-core/src/command/draw.rs b/wgpu-core/src/command/draw.rs index a51f8fcb58..e091acf804 100644 --- a/wgpu-core/src/command/draw.rs +++ b/wgpu-core/src/command/draw.rs @@ -28,9 +28,8 @@ pub enum DrawError { #[error("The pipeline layout, associated with the current render pipeline, contains a incompatible bind group layout at index {index}")] IncompatibleBindGroup { index: u32, - diff: Vec - //expected: BindGroupLayoutId, - //provided: Option<(BindGroupLayoutId, BindGroupId)>, + diff: Vec, //expected: BindGroupLayoutId, + //provided: Option<(BindGroupLayoutId, BindGroupId)>, }, #[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 { diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 4bd0540f92..2e902ecf2f 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -441,7 +441,7 @@ impl State { //let (expected, provided) = self.binder.entries[index as usize].info(); return Err(DrawError::IncompatibleBindGroup { index: bind_mask.trailing_zeros(), - diff: self.binder.bgl_diff() + diff: self.binder.bgl_diff(), }); } if self.pipeline.is_none() { From 88dfad73270f774161159a3181e3f2f3eb268565 Mon Sep 17 00:00:00 2001 From: Mikko Lehtonen Date: Sun, 17 Dec 2023 22:23:12 +0200 Subject: [PATCH 4/5] Improve error messages --- wgpu-core/src/command/bind.rs | 22 ++++++++++++++++++++-- wgpu-core/src/command/draw.rs | 8 ++------ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/wgpu-core/src/command/bind.rs b/wgpu-core/src/command/bind.rs index fb2a91890e..6bf849a42a 100644 --- a/wgpu-core/src/command/bind.rs +++ b/wgpu-core/src/command/bind.rs @@ -54,11 +54,21 @@ mod compat { self.expected.is_none() || !self.is_valid() } + // Describe how bind group layouts are incompatible, for validation + // error message. fn bgl_diff(&self) -> Vec { 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 { @@ -100,12 +110,20 @@ mod compat { "Assigned bindgroup layout is implicit, expected explicit".to_owned(), ); } - } else if let Some(_assigned_bgl) = self.assigned.as_ref() { + } 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 } } @@ -183,7 +201,7 @@ mod compat { return e.bgl_diff(); } } - vec![String::from("No differences detected?")] + vec![String::from("No differences detected? (internal error)")] } } } diff --git a/wgpu-core/src/command/draw.rs b/wgpu-core/src/command/draw.rs index e091acf804..ab3b6fa475 100644 --- a/wgpu-core/src/command/draw.rs +++ b/wgpu-core/src/command/draw.rs @@ -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 incompatible bind group layout at index {index}")] - IncompatibleBindGroup { - index: u32, - diff: Vec, //expected: BindGroupLayoutId, - //provided: Option<(BindGroupLayoutId, BindGroupId)>, - }, + #[error("Incompatible bind group at index {index} in the current render pipeline")] + IncompatibleBindGroup { index: u32, diff: Vec }, #[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, From 5cd3c585947f51bc1b8a75ad490fb7206cc135fe Mon Sep 17 00:00:00 2001 From: Mikko Lehtonen Date: Sun, 17 Dec 2023 22:23:39 +0200 Subject: [PATCH 5/5] Implement bgl_diff for compute --- wgpu-core/src/command/compute.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 8bc8f43cd4..0fd80f2f21 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -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 }, #[error( "Each current dispatch group size dimension ({current:?}) must be less or equal to {limit}" )] @@ -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); + } + } _ => {} }; } @@ -291,8 +292,11 @@ impl State { 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() {