Merge #935

935: Properly implement compute pass usage scopes r=cwfitzgerald a=kvark

**Connections**
Fixes #934 
Closes #933

**Description**
The old barriers in a compute pass were totally wrong. They didn't combine usages across the bind groups, or with indirect buffer.
This change implements the *actual* semantics as seen by the spec. At dispatch time we gather the usages across bind groups needed by the active pipeline, possibly add the indirect buffer into the mix, and that combined used is what we transition into.

I think we can find a few interesting ideas on how to make this faster. In particular - how to avoid a separate `TrackerSet` per pass. But for now correctness is more important.

**Testing**
Tested on wgpu-rs examples

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

wgpu-core/src/command/bind.rs

@@ -219,6 +219,13 @@ impl Binder {
         }
     }
 
+    pub(super) fn list_active(&self) -> impl Iterator<Item = Valid<BindGroupId>> + '_ {
+        self.entries.iter().filter_map(|e| match e.provided {
+            Some(ref pair) if e.expected_layout_id.is_some() => Some(pair.group_id.value),
+            _ => None,
+        })
+    }
+
     pub(super) fn invalid_mask(&self) -> BindGroupMask {
         self.entries.iter().enumerate().fold(0, |mask, (i, entry)| {
             if entry.is_valid().unwrap_or(true) {

wgpu-core/src/command/bundle.rs

@@ -47,7 +47,7 @@ use crate::{
     id,
     resource::BufferUse,
     span,
-    track::TrackerSet,
+    track::{TrackerSet, UsageConflict},
     validation::check_buffer_usage,
     Label, LifeGuard, RefCount, Stored, MAX_BIND_GROUPS,
 };
@@ -630,6 +630,8 @@ pub enum RenderBundleError {
     #[error(transparent)]
     RenderCommand(#[from] RenderCommandError),
     #[error(transparent)]
+    ResourceUsageConflict(#[from] UsageConflict),
+    #[error(transparent)]
     Draw(#[from] DrawError),
 }
 
@@ -710,7 +712,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         }
 
                         state.set_bind_group(index, bind_group_id, bind_group.layout_id, offsets);
-                        state.trackers.merge_extend(&bind_group.used);
+                        state.trackers.merge_extend(&bind_group.used)?;
                     }
                     RenderCommand::SetPipeline(pipeline_id) => {
                         let pipeline = state

wgpu-core/src/command/compute.rs

@@ -3,16 +3,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use crate::{
-    binding_model::{BindError, PushConstantUploadError},
+    binding_model::{BindError, BindGroup, PushConstantUploadError},
     command::{
         bind::{Binder, LayoutChange},
-        BasePass, BasePassRef, CommandBuffer, CommandEncoderError, UsageConflict,
+        BasePass, BasePassRef, CommandBuffer, CommandEncoderError,
     },
-    device::all_buffer_stages,
-    hub::{GfxBackend, Global, GlobalIdentityHandlerFactory, Token},
+    hub::{GfxBackend, Global, GlobalIdentityHandlerFactory, Storage, Token},
     id,
-    resource::BufferUse,
+    resource::{Buffer, BufferUse, Texture},
     span,
+    track::{TrackerSet, UsageConflict},
     validation::{check_buffer_usage, MissingBufferUsageError},
     MAX_BIND_GROUPS,
 };
@@ -124,7 +124,7 @@ pub enum ComputePassError {
     #[error("indirect buffer {0:?} is invalid")]
     InvalidIndirectBuffer(id::BufferId),
     #[error(transparent)]
-    ResourceUsageConflict(UsageConflict),
+    ResourceUsageConflict(#[from] UsageConflict),
     #[error(transparent)]
     MissingBufferUsage(#[from] MissingBufferUsageError),
     #[error("cannot pop debug group, because number of pushed debug groups is zero")]
@@ -147,6 +147,7 @@ enum PipelineState {
 struct State {
     binder: Binder,
     pipeline: PipelineState,
+    trackers: TrackerSet,
     debug_scope_depth: u32,
 }
 
@@ -165,6 +166,32 @@ impl State {
         }
         Ok(())
     }
+
+    fn flush_states<B: GfxBackend>(
+        &mut self,
+        raw_cmd_buf: &mut B::CommandBuffer,
+        base_trackers: &mut TrackerSet,
+        bind_group_guard: &Storage<BindGroup<B>, id::BindGroupId>,
+        buffer_guard: &Storage<Buffer<B>, id::BufferId>,
+        texture_guard: &Storage<Texture<B>, id::TextureId>,
+    ) -> Result<(), UsageConflict> {
+        for id in self.binder.list_active() {
+            self.trackers.merge_extend(&bind_group_guard[id].used)?;
+        }
+
+        tracing::trace!("Encoding dispatch barriers");
+
+        CommandBuffer::insert_barriers(
+            raw_cmd_buf,
+            base_trackers,
+            &self.trackers,
+            buffer_guard,
+            texture_guard,
+        );
+
+        self.trackers.clear();
+        Ok(())
+    }
 }
 
 // Common routines between render/compute
@@ -212,6 +239,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         let mut state = State {
             binder: Binder::new(cmd_buf.limits.max_bind_groups),
             pipeline: PipelineState::Required,
+            trackers: TrackerSet::new(B::VARIANT),
             debug_scope_depth: 0,
         };
         let mut temp_offsets = Vec::new();
@@ -243,19 +271,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         .map_err(|_| ComputePassError::InvalidBindGroup(bind_group_id))?;
                     bind_group.validate_dynamic_bindings(&temp_offsets)?;
 
-                    tracing::trace!(
-                        "Encoding barriers on binding of {:?} to {:?}",
-                        bind_group_id,
-                        encoder_id
-                    );
-                    CommandBuffer::insert_barriers(
-                        raw,
-                        &mut cmd_buf.trackers,
-                        &bind_group.used,
-                        &*buffer_guard,
-                        &*texture_guard,
-                    );
-
                     if let Some((pipeline_layout_id, follow_ups)) = state.binder.provide_entry(
                         index as usize,
                         id::Valid(bind_group_id),
@@ -379,6 +394,13 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                 }
                 ComputeCommand::Dispatch(groups) => {
                     state.is_ready()?;
+                    state.flush_states(
+                        raw,
+                        &mut cmd_buf.trackers,
+                        &*bind_group_guard,
+                        &*buffer_guard,
+                        &*texture_guard,
+                    )?;
                     unsafe {
                         raw.dispatch(groups);
                     }
@@ -386,22 +408,22 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                 ComputeCommand::DispatchIndirect { buffer_id, offset } => {
                     state.is_ready()?;
 
-                    let (src_buffer, src_pending) = cmd_buf
+                    let indirect_buffer = state
                         .trackers
                         .buffers
-                        .use_replace(&*buffer_guard, buffer_id, (), BufferUse::INDIRECT)
-                        .map_err(ComputePassError::InvalidIndirectBuffer)?;
-                    check_buffer_usage(src_buffer.usage, BufferUsage::INDIRECT)?;
-
-                    let barriers = src_pending.map(|pending| pending.into_hal(src_buffer));
+                        .use_extend(&*buffer_guard, buffer_id, (), BufferUse::INDIRECT)
+                        .map_err(|_| ComputePassError::InvalidIndirectBuffer(buffer_id))?;
+                    check_buffer_usage(indirect_buffer.usage, BufferUsage::INDIRECT)?;
 
+                    state.flush_states(
+                        raw,
+                        &mut cmd_buf.trackers,
+                        &*bind_group_guard,
+                        &*buffer_guard,
+                        &*texture_guard,
+                    )?;
                     unsafe {
-                        raw.pipeline_barrier(
-                            all_buffer_stages()..all_buffer_stages(),
-                            hal::memory::Dependencies::empty(),
-                            barriers,
-                        );
-                        raw.dispatch_indirect(&src_buffer.raw, offset);
+                        raw.dispatch_indirect(&indirect_buffer.raw, offset);
                     }
                 }
                 ComputeCommand::PushDebugGroup { color, len } => {

wgpu-core/src/command/mod.rs

@@ -251,14 +251,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
     }
 }
 
-#[derive(Clone, Debug, Error)]
-pub enum UsageConflict {
-    #[error("buffer {0:?} combined usage is {1:?}")]
-    Buffer(id::BufferId, wgt::BufferUsage),
-    #[error("texture {0:?} combined usage is {1:?}")]
-    Texture(id::TextureId, wgt::TextureUsage),
-}
-
 fn push_constant_clear<PushFn>(offset: u32, size_bytes: u32, mut push_fn: PushFn)
 where
     PushFn: FnMut(u32, &[u32]),

wgpu-core/src/command/render.rs

@@ -19,7 +19,7 @@ use crate::{
     pipeline::PipelineFlags,
     resource::{BufferUse, TextureUse, TextureView, TextureViewInner},
     span,
-    track::{TextureSelector, TrackerSet},
+    track::{TextureSelector, TrackerSet, UsageConflict},
     validation::{
         check_buffer_usage, check_texture_usage, MissingBufferUsageError, MissingTextureUsageError,
     },
@@ -361,6 +361,8 @@ pub enum RenderPassError {
     },
     #[error("cannot pop debug group, because number of pushed debug groups is zero")]
     InvalidPopDebugGroup,
+    #[error(transparent)]
+    ResourceUsageConflict(#[from] UsageConflict),
     #[error("render bundle is incompatible, {0}")]
     IncompatibleRenderBundle(#[from] RenderPassCompatibilityError),
     #[error(transparent)]
@@ -983,7 +985,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         .validate_dynamic_bindings(&temp_offsets)
                         .map_err(RenderPassError::from)?;
 
-                    trackers.merge_extend(&bind_group.used);
+                    trackers.merge_extend(&bind_group.used)?;
 
                     if let Some((pipeline_layout_id, follow_ups)) = state.binder.provide_entry(
                         index as usize,
@@ -1527,7 +1529,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         )
                     }
 
-                    trackers.merge_extend(&bundle.used);
+                    trackers.merge_extend(&bundle.used)?;
                     state.reset_bundle();
                 }
             }

wgpu-core/src/track/mod.rs

@@ -480,6 +480,24 @@ impl<I: Copy + fmt::Debug + TypedId> ResourceState for PhantomData<I> {
 
 pub const DUMMY_SELECTOR: () = ();
 
+#[derive(Clone, Debug, Error)]
+pub enum UsageConflict {
+    #[error(
+        "Attempted to use buffer {id:?} as a combination of {combined_use:?} within a usage scope."
+    )]
+    Buffer {
+        id: id::BufferId,
+        combined_use: resource::BufferUse,
+    },
+    #[error("Attempted to use texture {id:?} mips {mip_levels:?} layers {array_layers:?} as a combination of {combined_use:?} within a usage scope.")]
+    Texture {
+        id: id::TextureId,
+        mip_levels: ops::Range<u32>,
+        array_layers: ops::Range<u32>,
+        combined_use: resource::TextureUse,
+    },
+}
+
 /// A set of trackers for all relevant resources.
 #[derive(Debug)]
 pub(crate) struct TrackerSet {
@@ -534,9 +552,21 @@ impl TrackerSet {
 
     /// Merge all the trackers of another instance by extending
     /// the usage. Panics on a conflict.
-    pub fn merge_extend(&mut self, other: &Self) {
-        self.buffers.merge_extend(&other.buffers).unwrap();
-        self.textures.merge_extend(&other.textures).unwrap();
+    pub fn merge_extend(&mut self, other: &Self) -> Result<(), UsageConflict> {
+        self.buffers
+            .merge_extend(&other.buffers)
+            .map_err(|e| UsageConflict::Buffer {
+                id: e.id.0,
+                combined_use: e.usage.end,
+            })?;
+        self.textures
+            .merge_extend(&other.textures)
+            .map_err(|e| UsageConflict::Texture {
+                id: e.id.0,
+                mip_levels: e.selector.levels.start as u32..e.selector.levels.end as u32,
+                array_layers: e.selector.layers.start as u32..e.selector.layers.end as u32,
+                combined_use: e.usage.end,
+            })?;
         self.views.merge_extend(&other.views).unwrap();
         self.bind_groups.merge_extend(&other.bind_groups).unwrap();
         self.samplers.merge_extend(&other.samplers).unwrap();
@@ -545,6 +575,7 @@ impl TrackerSet {
             .unwrap();
         self.render_pipes.merge_extend(&other.render_pipes).unwrap();
         self.bundles.merge_extend(&other.bundles).unwrap();
+        Ok(())
     }
 
     pub fn backend(&self) -> wgt::Backend {