introduce MultiError and use it for BGL incompatibility errors

deno_webgpu/error.rs

@@ -6,6 +6,7 @@ use serde::Serialize;
 use std::convert::From;
 use std::error::Error;
 use std::fmt;
+use std::fmt::Write;
 use wgpu_core::binding_model::CreateBindGroupError;
 use wgpu_core::binding_model::CreateBindGroupLayoutError;
 use wgpu_core::binding_model::CreatePipelineLayoutError;
@@ -34,13 +35,29 @@ use wgpu_core::resource::CreateTextureViewError;
 
 fn fmt_err(err: &(dyn Error + 'static)) -> String {
     let mut output = err.to_string();
+    let mut level = 0;
 
-    let mut e = err.source();
-    while let Some(source) = e {
-        output.push_str(&format!(": {source}"));
-        e = source.source();
+    fn print_tree(output: &mut String, level: &mut usize, e: &(dyn Error + 'static)) {
+        let mut print = |e: &(dyn Error + 'static)| {
+            writeln!(output, "{}{}", " ".repeat(*level * 2), e).unwrap();
+
+            if let Some(e) = e.source() {
+                *level += 1;
+                print_tree(output, level, e);
+                *level -= 1;
+            }
+        };
+        if let Some(multi) = e.downcast_ref::<wgpu_core::error::MultiError>() {
+            for e in multi.errors() {
+                print(e);
+            }
+        } else {
+            print(e);
+        }
     }
 
+    print_tree(&mut output, &mut level, err);
+
     output
 }
 

wgpu-core/src/command/bind.rs

@@ -13,14 +13,26 @@ use thiserror::Error;
 
 mod compat {
     use arrayvec::ArrayVec;
+    use thiserror::Error;
+    use wgt::{BindingType, ShaderStages};
 
     use crate::{
         binding_model::BindGroupLayout,
         device::bgl,
+        error::MultiError,
         hal_api::HalApi,
-        resource::{Labeled, ParentDevice},
+        resource::{Labeled, ParentDevice, ResourceErrorIdent},
     };
-    use std::{ops::Range, sync::Arc};
+    use std::{num::NonZeroU32, ops::Range, sync::Arc};
+
+    pub(crate) enum Error {
+        Incompatible {
+            expected_bgl: ResourceErrorIdent,
+            assigned_bgl: ResourceErrorIdent,
+            inner: MultiError,
+        },
+        Missing,
+    }
 
     #[derive(Debug, Clone)]
     struct Entry<A: HalApi> {
@@ -55,81 +67,113 @@ 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<String> {
-            let mut diff = Vec::new();
-
+        fn check(&self) -> Result<(), Error> {
             if let Some(expected_bgl) = self.expected.as_ref() {
-                let expected_bgl_type = match expected_bgl.origin {
-                    bgl::Origin::Derived => "implicit",
-                    bgl::Origin::Pool => "explicit",
-                };
-                diff.push(format!(
-                    "Should be compatible an with an {expected_bgl_type} {}",
-                    expected_bgl.error_ident()
-                ));
                 if let Some(assigned_bgl) = self.assigned.as_ref() {
-                    let assigned_bgl_type = match assigned_bgl.origin {
-                        bgl::Origin::Derived => "implicit",
-                        bgl::Origin::Pool => "explicit",
-                    };
-                    diff.push(format!(
-                        "Assigned {assigned_bgl_type} {}",
-                        assigned_bgl.error_ident()
-                    ));
-                    for (id, e_entry) in expected_bgl.entries.iter() {
-                        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 bind group layout"
-                            ))
+                    if expected_bgl.is_equal(assigned_bgl) {
+                        Ok(())
+                    } else {
+                        #[derive(Clone, Debug, Error)]
+                        #[error("Expected an {expected_bgl_type} bind group layout, got an {assigned_bgl_type} bind group layout")]
+                        struct IncompatibleTypes {
+                            expected_bgl_type: &'static str,
+                            assigned_bgl_type: &'static str,
                         }
-                    }
 
-                    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 bind group layout"
-                            ))
+                        if expected_bgl.origin != assigned_bgl.origin {
+                            fn get_bgl_type(origin: bgl::Origin) -> &'static str {
+                                match origin {
+                                    bgl::Origin::Derived => "implicit",
+                                    bgl::Origin::Pool => "explicit",
+                                }
+                            }
+                            return Err(Error::Incompatible {
+                                expected_bgl: expected_bgl.error_ident(),
+                                assigned_bgl: assigned_bgl.error_ident(),
+                                inner: MultiError::new(core::iter::once(IncompatibleTypes {
+                                    expected_bgl_type: get_bgl_type(expected_bgl.origin),
+                                    assigned_bgl_type: get_bgl_type(assigned_bgl.origin),
+                                }))
+                                .unwrap(),
+                            });
                         }
-                    });
 
-                    if expected_bgl.origin != assigned_bgl.origin {
-                        diff.push(format!("Expected {expected_bgl_type} bind group layout, got {assigned_bgl_type}"))
+                        #[derive(Clone, Debug, Error)]
+                        enum EntryError {
+                            #[error("Entries with binding {binding} differ in visibility: expected {expected:?}, got {assigned:?}")]
+                            Visibility {
+                                binding: u32,
+                                expected: ShaderStages,
+                                assigned: ShaderStages,
+                            },
+                            #[error("Entries with binding {binding} differ in type: expected {expected:?}, got {assigned:?}")]
+                            Type {
+                                binding: u32,
+                                expected: BindingType,
+                                assigned: BindingType,
+                            },
+                            #[error("Entries with binding {binding} differ in count: expected {expected:?}, got {assigned:?}")]
+                            Count {
+                                binding: u32,
+                                expected: Option<NonZeroU32>,
+                                assigned: Option<NonZeroU32>,
+                            },
+                            #[error("Expected entry with binding {binding} not found in assigned bind group layout")]
+                            ExtraExpected { binding: u32 },
+                            #[error("Assigned entry with binding {binding} not found in expected bind group layout")]
+                            ExtraAssigned { binding: u32 },
+                        }
+
+                        let mut errors = Vec::new();
+
+                        let mut expected_bgl_entries = expected_bgl.entries.iter();
+                        let mut assigned_bgl_entries = assigned_bgl.entries.iter();
+                        let zipped = (&mut expected_bgl_entries).zip(&mut assigned_bgl_entries);
+
+                        for ((&binding, expected_entry), (_, assigned_entry)) in zipped {
+                            if assigned_entry.visibility != expected_entry.visibility {
+                                errors.push(EntryError::Visibility {
+                                    binding,
+                                    expected: expected_entry.visibility,
+                                    assigned: assigned_entry.visibility,
+                                });
+                            }
+                            if assigned_entry.ty != expected_entry.ty {
+                                errors.push(EntryError::Type {
+                                    binding,
+                                    expected: expected_entry.ty,
+                                    assigned: assigned_entry.ty,
+                                });
+                            }
+                            if assigned_entry.count != expected_entry.count {
+                                errors.push(EntryError::Count {
+                                    binding,
+                                    expected: expected_entry.count,
+                                    assigned: assigned_entry.count,
+                                });
+                            }
+                        }
+
+                        for (&binding, _) in expected_bgl_entries {
+                            errors.push(EntryError::ExtraExpected { binding });
+                        }
+
+                        for (&binding, _) in assigned_bgl_entries {
+                            errors.push(EntryError::ExtraAssigned { binding });
+                        }
+
+                        Err(Error::Incompatible {
+                            expected_bgl: expected_bgl.error_ident(),
+                            assigned_bgl: assigned_bgl.error_ident(),
+                            inner: MultiError::new(errors.drain(..)).unwrap(),
+                        })
                     }
                 } else {
-                    diff.push("Assigned bind group layout not found (internal error)".to_owned());
+                    Err(Error::Missing)
                 }
             } else {
-                diff.push("Expected bind group layout not found (internal error)".to_owned());
+                Ok(())
             }
-
-            diff
         }
     }
 
@@ -190,23 +234,32 @@ mod compat {
                 .filter_map(|(i, e)| if e.is_active() { Some(i) } else { None })
         }
 
-        pub fn get_invalid(&self) -> Option<(usize, Vec<String>)> {
+        pub fn get_invalid(&self) -> Result<(), (usize, Error)> {
             for (index, entry) in self.entries.iter().enumerate() {
-                if !entry.is_valid() {
-                    return Some((index, entry.bgl_diff()));
-                }
+                entry.check().map_err(|e| (index, e))?;
             }
-            None
+            Ok(())
         }
     }
 }
 
 #[derive(Clone, Debug, Error)]
-#[error("Bind group at index {index} is incompatible with the current set {pipeline}")]
-pub struct IncompatibleBindGroupError {
-    index: usize,
-    pipeline: ResourceErrorIdent,
-    diff: Vec<String>,
+pub enum BinderError {
+    #[error("The current set {pipeline} expects a BindGroup to be set at index {index}")]
+    MissingBindGroup {
+        index: usize,
+        pipeline: ResourceErrorIdent,
+    },
+    #[error("The {assigned_bgl} of current set {assigned_bg} at index {index} is not compatible with the corresponding {expected_bgl} of {pipeline}")]
+    IncompatibleBindGroup {
+        expected_bgl: ResourceErrorIdent,
+        assigned_bgl: ResourceErrorIdent,
+        assigned_bg: ResourceErrorIdent,
+        index: usize,
+        pipeline: ResourceErrorIdent,
+        #[source]
+        inner: crate::error::MultiError,
+    },
 }
 
 #[derive(Debug)]
@@ -356,16 +409,27 @@ impl<A: HalApi> Binder<A> {
     pub(super) fn check_compatibility<T: Labeled>(
         &self,
         pipeline: &T,
-    ) -> Result<(), IncompatibleBindGroupError> {
-        if let Some((index, diff)) = self.manager.get_invalid() {
-            Err(IncompatibleBindGroupError {
-                index,
-                pipeline: pipeline.error_ident(),
-                diff,
+    ) -> Result<(), Box<BinderError>> {
+        self.manager.get_invalid().map_err(|(index, error)| {
+            Box::new(match error {
+                compat::Error::Incompatible {
+                    expected_bgl,
+                    assigned_bgl,
+                    inner,
+                } => BinderError::IncompatibleBindGroup {
+                    expected_bgl,
+                    assigned_bgl,
+                    assigned_bg: self.payloads[index].group.as_ref().unwrap().error_ident(),
+                    index,
+                    pipeline: pipeline.error_ident(),
+                    inner,
+                },
+                compat::Error::Missing => BinderError::MissingBindGroup {
+                    index,
+                    pipeline: pipeline.error_ident(),
+                },
             })
-        } else {
-            Ok(())
-        }
+        })
     }
 
     /// Scan active buffer bindings corresponding to layouts without `min_binding_size` specified.

wgpu-core/src/command/compute.rs

@@ -35,10 +35,7 @@ use wgt::{BufferAddress, DynamicOffset};
 use std::sync::Arc;
 use std::{fmt, mem, str};
 
-use super::{
-    bind::IncompatibleBindGroupError, memory_init::CommandBufferTextureMemoryActions,
-    DynComputePass,
-};
+use super::{bind::BinderError, memory_init::CommandBufferTextureMemoryActions, DynComputePass};
 
 pub struct ComputePass<A: HalApi> {
     /// All pass data & records is stored here.
@@ -121,7 +118,7 @@ pub enum DispatchError {
     #[error("Compute pipeline must be set")]
     MissingPipeline,
     #[error(transparent)]
-    IncompatibleBindGroup(#[from] IncompatibleBindGroupError),
+    IncompatibleBindGroup(#[from] Box<BinderError>),
     #[error(
         "Each current dispatch group size dimension ({current:?}) must be less or equal to {limit}"
     )]

wgpu-core/src/command/draw.rs

@@ -11,7 +11,7 @@ use wgt::VertexStepMode;
 
 use thiserror::Error;
 
-use super::bind::IncompatibleBindGroupError;
+use super::bind::BinderError;
 
 /// Error validating a draw call.
 #[derive(Clone, Debug, Error)]
@@ -29,7 +29,7 @@ pub enum DrawError {
     #[error("Index buffer must be set")]
     MissingIndexBuffer,
     #[error(transparent)]
-    IncompatibleBindGroup(#[from] IncompatibleBindGroupError),
+    IncompatibleBindGroup(#[from] Box<BinderError>),
     #[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: u64,

wgpu-core/src/error.rs

@@ -1,5 +1,5 @@
 use core::fmt;
-use std::error::Error;
+use std::{error::Error, sync::Arc};
 
 use crate::{gfx_select, global::Global};
 
@@ -179,3 +179,44 @@ impl Error for ContextError {
         Some(self.cause.as_ref())
     }
 }
+
+/// Don't use this error type with thiserror's #[error(transparent)]
+#[derive(Clone)]
+pub struct MultiError {
+    inner: Vec<Arc<dyn Error + Send + Sync + 'static>>,
+}
+
+impl MultiError {
+    pub fn new<T: Error + Send + Sync + 'static>(
+        iter: impl ExactSizeIterator<Item = T>,
+    ) -> Option<Self> {
+        if iter.len() == 0 {
+            return None;
+        }
+        Some(Self {
+            inner: iter.map(Box::from).map(Arc::from).collect(),
+        })
+    }
+
+    pub fn errors(&self) -> Box<dyn Iterator<Item = &(dyn Error + Send + Sync + 'static)> + '_> {
+        Box::new(self.inner.iter().map(|e| e.as_ref()))
+    }
+}
+
+impl fmt::Debug for MultiError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
+        fmt::Debug::fmt(&self.inner[0], f)
+    }
+}
+
+impl fmt::Display for MultiError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
+        fmt::Display::fmt(&self.inner[0], f)
+    }
+}
+
+impl Error for MultiError {
+    fn source(&self) -> Option<&(dyn Error + 'static)> {
+        self.inner[0].source()
+    }
+}

wgpu/src/backend/wgpu_core.rs

@@ -325,19 +325,36 @@ impl ContextWgpuCore {
     fn format_error(&self, err: &(impl Error + 'static)) -> String {
         let global = self.global();
         let mut err_descs = vec![];
+        let mut level = 0;
 
-        let mut err_str = String::new();
-        wgc::error::format_pretty_any(&mut err_str, global, err);
-        err_descs.push(err_str);
+        fn print_tree(
+            err_descs: &mut Vec<String>,
+            level: &mut usize,
+            global: &wgc::global::Global,
+            e: &(dyn Error + 'static),
+        ) {
+            let mut print = |e| {
+                let mut err_str = " ".repeat(*level * 2);
+                wgc::error::format_pretty_any(&mut err_str, global, e);
+                err_descs.push(err_str);
 
-        let mut source_opt = err.source();
-        while let Some(source) = source_opt {
-            let mut source_str = String::new();
-            wgc::error::format_pretty_any(&mut source_str, global, source);
-            err_descs.push(source_str);
-            source_opt = source.source();
+                if let Some(e) = e.source() {
+                    *level += 1;
+                    print_tree(err_descs, level, global, e);
+                    *level -= 1;
+                }
+            };
+            if let Some(multi) = e.downcast_ref::<wgc::error::MultiError>() {
+                for e in multi.errors() {
+                    print(e);
+                }
+            } else {
+                print(e);
+            }
         }
 
+        print_tree(&mut err_descs, &mut level, global, err);
+
         format!("Validation Error\n\nCaused by:\n{}", err_descs.join(""))
     }
 }