From ef0ce05d3a6ffae396c3335d71ef76069a2d480d Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 10 Jul 2024 12:12:12 +0200 Subject: [PATCH] [wgpu-core] fix `.zip()` usages --- wgpu-core/src/command/bind.rs | 5 +++- wgpu-core/src/device/global.rs | 26 +++++++++++++--- wgpu-core/src/lib.rs | 1 + wgpu-core/src/utils.rs | 54 ++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 wgpu-core/src/utils.rs diff --git a/wgpu-core/src/command/bind.rs b/wgpu-core/src/command/bind.rs index a6176ac4c..64d534b55 100644 --- a/wgpu-core/src/command/bind.rs +++ b/wgpu-core/src/command/bind.rs @@ -144,7 +144,10 @@ mod compat { 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); + 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 { diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index d74e34d38..499ba6ecc 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -1532,8 +1532,17 @@ impl Global { let mut pipeline_layout_guard = hub.pipeline_layouts.write(); let mut bgl_guard = hub.bind_group_layouts.write(); pipeline_layout_guard.insert(ids.root_id, pipeline.layout.clone()); - let group_ids = &mut ids.group_ids.iter(); - for (bgl_id, bgl) in group_ids.zip(pipeline.layout.bind_group_layouts.iter()) { + let mut group_ids = ids.group_ids.iter(); + // NOTE: If the first iterator is longer than the second, the `.zip()` impl will still advance the + // the first iterator before realizing that the second iterator has finished. + // The `pipeline.layout.bind_group_layouts` iterator will always be shorter than `ids.group_ids`, + // so using it as the first iterator for `.zip()` will work properly. + for (bgl, bgl_id) in pipeline + .layout + .bind_group_layouts + .iter() + .zip(&mut group_ids) + { bgl_guard.insert(*bgl_id, bgl.clone()); } for bgl_id in group_ids { @@ -1721,8 +1730,17 @@ impl Global { let mut pipeline_layout_guard = hub.pipeline_layouts.write(); let mut bgl_guard = hub.bind_group_layouts.write(); pipeline_layout_guard.insert(ids.root_id, pipeline.layout.clone()); - let group_ids = &mut ids.group_ids.iter(); - for (bgl_id, bgl) in group_ids.zip(pipeline.layout.bind_group_layouts.iter()) { + let mut group_ids = ids.group_ids.iter(); + // NOTE: If the first iterator is longer than the second, the `.zip()` impl will still advance the + // the first iterator before realizing that the second iterator has finished. + // The `pipeline.layout.bind_group_layouts` iterator will always be shorter than `ids.group_ids`, + // so using it as the first iterator for `.zip()` will work properly. + for (bgl, bgl_id) in pipeline + .layout + .bind_group_layouts + .iter() + .zip(&mut group_ids) + { bgl_guard.insert(*bgl_id, bgl.clone()); } for bgl_id in group_ids { diff --git a/wgpu-core/src/lib.rs b/wgpu-core/src/lib.rs index 7600436bc..36105c90e 100644 --- a/wgpu-core/src/lib.rs +++ b/wgpu-core/src/lib.rs @@ -71,6 +71,7 @@ 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 new file mode 100644 index 000000000..cf61e797e --- /dev/null +++ b/wgpu-core/src/utils.rs @@ -0,0 +1,54 @@ +/// 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()) + } +}