From e3b5c1a33fdc35498a33f70805d2f94913ca29ec Mon Sep 17 00:00:00 2001 From: Samson <16504129+sagudev@users.noreply.github.com> Date: Wed, 24 Jul 2024 17:50:18 +0200 Subject: [PATCH] Error instead of panic in check bind (#6012) Removed zipping of binding entries introduced in 4a19ac279c4f81aacedb1d215c884c10fe115275 (to make sure binding numbers actually match) and add unknown error for fallback. # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 6 +++ wgpu-core/src/command/bind.rs | 69 ++++++++++++++++++----------------- wgpu-core/src/lib.rs | 1 - wgpu-core/src/utils.rs | 54 --------------------------- 4 files changed, 41 insertions(+), 89 deletions(-) delete mode 100644 wgpu-core/src/utils.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 25a47dd456..7a7007f6de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,12 @@ Bottom level categories: ## Unreleased +### Bug Fixes + +#### General + +- Fix function for checking bind compatibility to error instead of panic. By @sagudev [#6012](https://github.com/gfx-rs/wgpu/pull/6012) + ## 22.0.0 (2024-07-17) ### Overview diff --git a/wgpu-core/src/command/bind.rs b/wgpu-core/src/command/bind.rs index 64d534b558..73f1d9fe17 100644 --- a/wgpu-core/src/command/bind.rs +++ b/wgpu-core/src/command/bind.rs @@ -142,49 +142,50 @@ mod compat { 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 = crate::utils::ZipWithProperAdvance::new( - &mut expected_bgl_entries, - &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, expected_entry) in expected_bgl.entries.iter() { + if let Some(assigned_entry) = assigned_bgl.entries.get(binding) { + 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, + }); + } + } else { + errors.push(EntryError::ExtraExpected { binding }); } } - for (&binding, _) in expected_bgl_entries { - errors.push(EntryError::ExtraExpected { binding }); + for (&binding, _) in assigned_bgl.entries.iter() { + if !expected_bgl.entries.contains_key(binding) { + errors.push(EntryError::ExtraAssigned { binding }); + } } - for (&binding, _) in assigned_bgl_entries { - errors.push(EntryError::ExtraAssigned { binding }); - } + #[derive(Clone, Debug, Error)] + #[error("Unknown reason")] + struct Unknown(); Err(Error::Incompatible { expected_bgl: expected_bgl.error_ident(), assigned_bgl: assigned_bgl.error_ident(), - inner: MultiError::new(errors.drain(..)).unwrap(), + inner: MultiError::new(errors.drain(..)).unwrap_or_else(|| { + MultiError::new(core::iter::once(Unknown())).unwrap() + }), }) } } else { diff --git a/wgpu-core/src/lib.rs b/wgpu-core/src/lib.rs index 36105c90e6..7600436bc4 100644 --- a/wgpu-core/src/lib.rs +++ b/wgpu-core/src/lib.rs @@ -71,7 +71,6 @@ pub mod resource; mod snatch; pub mod storage; mod track; -mod utils; // This is public for users who pre-compile shaders while still wanting to // preserve all run-time checks that `wgpu-core` does. // See , after which this can be diff --git a/wgpu-core/src/utils.rs b/wgpu-core/src/utils.rs deleted file mode 100644 index cf61e797e2..0000000000 --- a/wgpu-core/src/utils.rs +++ /dev/null @@ -1,54 +0,0 @@ -/// If the first iterator is longer than the second, the zip implementation -/// in the standard library will still advance the the first iterator before -/// realizing that the second iterator has finished. -/// -/// This implementation will advance the shorter iterator first avoiding -/// the issue above. -/// -/// If you can guarantee that the first iterator is always shorter than the -/// second, you should use the zip impl in stdlib. -pub(crate) struct ZipWithProperAdvance< - A: ExactSizeIterator, - B: ExactSizeIterator, - IA, - IB, -> { - a: A, - b: B, - iter_a_first: bool, -} - -impl, B: ExactSizeIterator, IA, IB> - ZipWithProperAdvance -{ - pub(crate) fn new(a: A, b: B) -> Self { - let iter_a_first = a.len() <= b.len(); - Self { a, b, iter_a_first } - } -} - -impl, B: ExactSizeIterator, IA, IB> Iterator - for ZipWithProperAdvance -{ - type Item = (IA, IB); - - fn next(&mut self) -> Option { - if self.iter_a_first { - let a = self.a.next()?; - let b = self.b.next()?; - Some((a, b)) - } else { - let b = self.b.next()?; - let a = self.a.next()?; - Some((a, b)) - } - } -} - -impl, B: ExactSizeIterator, IA, IB> ExactSizeIterator - for ZipWithProperAdvance -{ - fn len(&self) -> usize { - self.a.len().min(self.b.len()) - } -}