Merge #387

387: Stop dynamic offset iteration when the bind groups stop r=grovesNL a=kvark

Fixes #386 

I imagine the code is not performance-optimal and could be improved. This PR focuses on correctness.

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

wgpu-core/src/command/bind.rs

@@ -12,7 +12,7 @@ use crate::{
 
 use smallvec::{smallvec, SmallVec};
 
-use std::convert::identity;
+use std::slice;
 
 pub const DEFAULT_BIND_GROUPS: usize = 4;
 type BindGroupMask = u8;
@@ -36,16 +36,21 @@ pub enum Provision {
     Changed { was_compatible: bool },
 }
 
-struct TakeSome<I> {
-    iter: I,
+#[derive(Clone)]
+pub struct FollowUpIter<'a> {
+    iter: slice::Iter<'a, BindGroupEntry>,
 }
-impl<T, I> Iterator for TakeSome<I>
-where
-    I: Iterator<Item = Option<T>>,
-{
-    type Item = T;
-    fn next(&mut self) -> Option<T> {
-        self.iter.next().and_then(identity)
+impl<'a> Iterator for FollowUpIter<'a> {
+    type Item = (BindGroupId, &'a [BufferAddress]);
+    fn next(&mut self) -> Option<Self::Item> {
+        self.iter
+            .next()
+            .and_then(|entry| {
+                Some((
+                    entry.actual_value()?,
+                    entry.dynamic_offsets.as_slice(),
+                ))
+            })
     }
 }
 
@@ -163,11 +168,7 @@ impl Binder {
         bind_group_id: BindGroupId,
         bind_group: &BindGroup<B>,
         offsets: &[BufferAddress],
-    ) -> Option<(
-        PipelineLayoutId,
-        impl 'a + Iterator<Item = BindGroupId>,
-        impl 'a + Iterator<Item = &'a BufferAddress>,
-    )> {
+    ) -> Option<(PipelineLayoutId, FollowUpIter<'a>)> {
         log::trace!("\tBinding [{}] = group {:?}", index, bind_group_id);
         debug_assert_eq!(B::VARIANT, bind_group_id.backend());
 
@@ -184,14 +185,9 @@ impl Binder {
                     log::trace!("\t\tbinding up to {}", end);
                     Some((
                         self.pipeline_layout_id?,
-                        TakeSome {
-                            iter: self.entries[index + 1 .. end]
-                                .iter()
-                                .map(|entry| entry.actual_value()),
-                        },
-                        self.entries[index + 1 .. end]
-                            .iter()
-                            .flat_map(|entry| entry.dynamic_offsets.as_slice()),
+                        FollowUpIter {
+                            iter: self.entries[index + 1 .. end].iter(),
+                        }
                     ))
                 } else {
                     log::trace!("\t\tskipping above compatible {}", compatible_count);

wgpu-core/src/command/compute.rs

@@ -116,12 +116,12 @@ impl<F> Global<F> {
             &*texture_guard,
         );
 
-        if let Some((pipeline_layout_id, follow_up_sets, follow_up_offsets)) = pass
+        if let Some((pipeline_layout_id, follow_ups)) = pass
             .binder
             .provide_entry(index as usize, bind_group_id, bind_group, offsets)
         {
             let bind_groups = iter::once(bind_group.raw.raw())
-                .chain(follow_up_sets.map(|bg_id| bind_group_guard[bg_id].raw.raw()));
+                .chain(follow_ups.clone().map(|(bg_id, _)| bind_group_guard[bg_id].raw.raw()));
             unsafe {
                 pass.raw.bind_compute_descriptor_sets(
                     &pipeline_layout_guard[pipeline_layout_id].raw,
@@ -129,7 +129,7 @@ impl<F> Global<F> {
                     bind_groups,
                     offsets
                         .iter()
-                        .chain(follow_up_offsets)
+                        .chain(follow_ups.flat_map(|(_, offsets)| offsets))
                         .map(|&off| off as hal::command::DescriptorSetOffset),
                 );
             }

wgpu-core/src/command/render.rs

@@ -247,12 +247,12 @@ impl<F: IdentityFilter<RenderPassId>> Global<F> {
 
         pass.trackers.merge_extend(&bind_group.used);
 
-        if let Some((pipeline_layout_id, follow_up_sets, follow_up_offsets)) = pass
+        if let Some((pipeline_layout_id, follow_ups)) = pass
             .binder
             .provide_entry(index as usize, bind_group_id, bind_group, offsets)
         {
             let bind_groups = iter::once(bind_group.raw.raw())
-                .chain(follow_up_sets.map(|bg_id| bind_group_guard[bg_id].raw.raw()));
+                .chain(follow_ups.clone().map(|(bg_id, _)| bind_group_guard[bg_id].raw.raw()));
             unsafe {
                 pass.raw.bind_graphics_descriptor_sets(
                     &&pipeline_layout_guard[pipeline_layout_id].raw,
@@ -260,7 +260,7 @@ impl<F: IdentityFilter<RenderPassId>> Global<F> {
                     bind_groups,
                     offsets
                         .iter()
-                        .chain(follow_up_offsets)
+                        .chain(follow_ups.flat_map(|(_, offsets)| offsets))
                         .map(|&off| off as hal::command::DescriptorSetOffset),
                 );
             }