From 0345c5675870761ca90d6dfe55b849679d8feb44 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Mon, 21 Sep 2020 23:28:52 -0400 Subject: [PATCH] Properly implement compute pass usage scopes --- wgpu-core/src/command/bind.rs | 7 +++ wgpu-core/src/command/bundle.rs | 6 ++- wgpu-core/src/command/compute.rs | 84 ++++++++++++++++++++------------ wgpu-core/src/command/mod.rs | 8 --- wgpu-core/src/command/render.rs | 8 +-- wgpu-core/src/track/mod.rs | 37 ++++++++++++-- 6 files changed, 103 insertions(+), 47 deletions(-) diff --git a/wgpu-core/src/command/bind.rs b/wgpu-core/src/command/bind.rs index a9c1be7a3d..a62b38d5b7 100644 --- a/wgpu-core/src/command/bind.rs +++ b/wgpu-core/src/command/bind.rs @@ -219,6 +219,13 @@ impl Binder { } } + pub(super) fn list_active(&self) -> impl Iterator> + '_ { + 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) { diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 39d31c4bc6..b522efce9d 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/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 Global { } 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 diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 72d6598852..f46dc29f79 100644 --- a/wgpu-core/src/command/compute.rs +++ b/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( + &mut self, + raw_cmd_buf: &mut B::CommandBuffer, + base_trackers: &mut TrackerSet, + bind_group_guard: &Storage, id::BindGroupId>, + buffer_guard: &Storage, id::BufferId>, + texture_guard: &Storage, 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 Global { 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 Global { .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 Global { } 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 Global { 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 } => { diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 14b1993322..db8c2e7ef6 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -251,14 +251,6 @@ impl Global { } } -#[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(offset: u32, size_bytes: u32, mut push_fn: PushFn) where PushFn: FnMut(u32, &[u32]), diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 6bdfc1b265..1e9456c770 100644 --- a/wgpu-core/src/command/render.rs +++ b/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 Global { .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 Global { ) } - trackers.merge_extend(&bundle.used); + trackers.merge_extend(&bundle.used)?; state.reset_bundle(); } } diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index 17af28280a..b96e954143 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -480,6 +480,24 @@ impl ResourceState for PhantomData { 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, + array_layers: ops::Range, + 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 {