From 8ebfdb0c343760f24a27f74f19de6f29ddb24bbc Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Wed, 10 Jun 2020 20:06:33 -0400 Subject: [PATCH] Rewrite the render attachment tracking using the new prepend() operation. --- wgpu-core/src/command/render.rs | 107 ++++++++++++++++++++------------ wgpu-core/src/track/buffer.rs | 45 ++++++++++++++ wgpu-core/src/track/mod.rs | 32 ++++++++++ wgpu-core/src/track/texture.rs | 35 +++++++++++ 4 files changed, 179 insertions(+), 40 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 6c7e32ecdd..2e0eff92cc 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -41,8 +41,27 @@ fn is_depth_stencil_read_only( desc: &RenderPassDepthStencilAttachmentDescriptor, aspects: hal::format::Aspects, ) -> bool { - (desc.depth_read_only || !aspects.contains(hal::format::Aspects::DEPTH)) - && (desc.stencil_read_only || !aspects.contains(hal::format::Aspects::STENCIL)) + if aspects.contains(hal::format::Aspects::DEPTH) { + if !desc.depth_read_only { + return false; + } + assert_eq!( + (desc.depth_load_op, desc.depth_store_op), + (wgt::LoadOp::Load, wgt::StoreOp::Store), + "Unable to clear read-only depth" + ); + } + if aspects.contains(hal::format::Aspects::STENCIL) { + if !desc.stencil_read_only { + return false; + } + assert_eq!( + (desc.stencil_load_op, desc.stencil_store_op), + (wgt::LoadOp::Load, wgt::StoreOp::Store), + "Unable to clear read-only stencil" + ); + } + true } #[repr(C)] @@ -427,6 +446,15 @@ impl Global { // instead of the special read-only one, which would be `None`. let mut is_ds_read_only = false; + struct OutputAttachment<'a> { + texture_id: &'a Stored, + range: &'a hal::image::SubresourceRange, + previous_use: Option, + new_use: TextureUse, + } + const MAX_TOTAL_ATTACHMENTS: usize = 2 * MAX_COLOR_TARGETS + 1; + let mut output_attachments = ArrayVec::<[OutputAttachment; MAX_TOTAL_ATTACHMENTS]>::new(); + let context = { use hal::device::Device as _; @@ -445,16 +473,6 @@ impl Global { "Attachment sample_count must be supported by physical device limits" ); - const MAX_TOTAL_ATTACHMENTS: usize = 10; - struct OutputAttachment<'a> { - texture_id: &'a Stored, - range: &'a hal::image::SubresourceRange, - previous_use: Option, - new_use: TextureUse, - } - let mut output_attachments = - ArrayVec::<[OutputAttachment; MAX_TOTAL_ATTACHMENTS]>::new(); - log::trace!( "Encoding render pass begin in command buffer {:?}", encoder_id @@ -668,33 +686,6 @@ impl Global { } }; - for ot in output_attachments { - let texture = &texture_guard[ot.texture_id.value]; - assert!( - texture.usage.contains(TextureUsage::OUTPUT_ATTACHMENT), - "Texture usage {:?} must contain the usage flag OUTPUT_ATTACHMENT", - texture.usage - ); - - // this is important to record the `first` state. - let _ = trackers.textures.change_replace( - ot.texture_id.value, - &ot.texture_id.ref_count, - ot.range.clone(), - ot.previous_use.unwrap_or(ot.new_use), - ); - if ot.previous_use.is_some() { - // If we expect the texture to be transited to a new state by the - // render pass configuration, make the tracker aware of that. - let _ = trackers.textures.change_replace( - ot.texture_id.value, - &ot.texture_id.ref_count, - ot.range.clone(), - ot.new_use, - ); - }; - } - let mut render_pass_cache = device.render_passes.lock(); let render_pass = match render_pass_cache.entry(rp_key.clone()) { Entry::Occupied(e) => e.into_mut(), @@ -740,7 +731,7 @@ impl Global { let depth_id = depth_stencil_attachment.map(|at| { let aspects = view_guard[at.attachment].range.aspects; - let usage = if is_depth_stencil_read_only(at, aspects) { + let usage = if is_ds_read_only { TextureUse::ATTACHMENT_READ } else { TextureUse::ATTACHMENT_WRITE @@ -1365,6 +1356,42 @@ impl Global { unsafe { raw.end_render_pass(); } + + for ot in output_attachments { + let texture = &texture_guard[ot.texture_id.value]; + assert!( + texture.usage.contains(TextureUsage::OUTPUT_ATTACHMENT), + "Texture usage {:?} must contain the usage flag OUTPUT_ATTACHMENT", + texture.usage + ); + + // the tracker set of the pass is always in "extend" mode + trackers + .textures + .change_extend( + ot.texture_id.value, + &ot.texture_id.ref_count, + ot.range.clone(), + ot.new_use, + ) + .unwrap(); + + if let Some(usage) = ot.previous_use { + // Make the attachment tracks to be aware of the internal + // transition done by the render pass, by registering the + // previous usage as the initial state. + trackers + .textures + .prepend( + ot.texture_id.value, + &ot.texture_id.ref_count, + ot.range.clone(), + usage, + ) + .unwrap(); + } + } + super::CommandBuffer::insert_barriers( cmb.raw.last_mut().unwrap(), &mut cmb.trackers, diff --git a/wgpu-core/src/track/buffer.rs b/wgpu-core/src/track/buffer.rs index 1c41d36116..5b8bd1a958 100644 --- a/wgpu-core/src/track/buffer.rs +++ b/wgpu-core/src/track/buffer.rs @@ -79,6 +79,25 @@ impl ResourceState for BufferState { Ok(()) } + fn prepend( + &mut self, + id: Self::Id, + _selector: Self::Selector, + usage: Self::Usage, + ) -> Result<(), PendingTransition> { + match self.first { + Some(old) if old != usage => Err(PendingTransition { + id, + selector: (), + usage: old..usage, + }), + _ => { + self.first = Some(usage); + Ok(()) + } + } + } + fn merge( &mut self, id: Self::Id, @@ -190,4 +209,30 @@ mod test { } ); } + + #[test] + fn prepend() { + let mut bs = Unit { + first: None, + last: BufferUse::VERTEX, + }; + let id = Id::default(); + bs.prepend(id, (), BufferUse::INDEX).unwrap(); + bs.prepend(id, (), BufferUse::INDEX).unwrap(); + assert_eq!( + bs.prepend(id, (), BufferUse::STORAGE_LOAD), + Err(PendingTransition { + id, + selector: (), + usage: BufferUse::INDEX..BufferUse::STORAGE_LOAD, + }) + ); + assert_eq!( + bs, + Unit { + first: Some(BufferUse::INDEX), + last: BufferUse::VERTEX, + } + ); + } } diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index 95965590a1..a97ac33a25 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -80,6 +80,14 @@ pub trait ResourceState: Clone + Default { output: Option<&mut Vec>>, ) -> Result<(), PendingTransition>; + /// Sets up the first usage of the selected sub-resources. + fn prepend( + &mut self, + id: Self::Id, + selector: Self::Selector, + usage: Self::Usage, + ) -> Result<(), PendingTransition>; + /// Merge the state of this resource tracked by a different instance /// with the current one. /// @@ -321,6 +329,21 @@ impl ResourceTracker { self.temp.drain(..) } + /// Turn the tracking from the "expand" mode into the "replace" one, + /// installing the selected usage as the "first". + /// This is a special operation only used by the render pass attachments. + pub fn prepend( + &mut self, + id: S::Id, + ref_count: &RefCount, + selector: S::Selector, + usage: S::Usage, + ) -> Result<(), PendingTransition> { + Self::get_or_insert(self.backend, &mut self.map, id, ref_count) + .state + .prepend(id, selector, usage) + } + /// Merge another tracker into `self` by extending the current states /// without any transitions. pub fn merge_extend(&mut self, other: &Self) -> Result<(), PendingTransition> { @@ -415,6 +438,15 @@ impl ResourceState for PhantomData { Ok(()) } + fn prepend( + &mut self, + _id: Self::Id, + _selector: Self::Selector, + _usage: Self::Usage, + ) -> Result<(), PendingTransition> { + Ok(()) + } + fn merge( &mut self, _id: Self::Id, diff --git a/wgpu-core/src/track/texture.rs b/wgpu-core/src/track/texture.rs index 4fc2720d02..fd70e1b62f 100644 --- a/wgpu-core/src/track/texture.rs +++ b/wgpu-core/src/track/texture.rs @@ -136,6 +136,41 @@ impl ResourceState for TextureState { Ok(()) } + fn prepend( + &mut self, + id: Self::Id, + selector: Self::Selector, + usage: Self::Usage, + ) -> Result<(), PendingTransition> { + assert!(self.mips.len() >= selector.levels.end as usize); + for (mip_id, mip) in self.mips[selector.levels.start as usize..selector.levels.end as usize] + .iter_mut() + .enumerate() + { + let level = selector.levels.start + mip_id as hal::image::Level; + let layers = mip.isolate(&selector.layers, Unit::new(usage)); + for &mut (ref range, ref mut unit) in layers { + match unit.first { + Some(old) if old != usage => { + return Err(PendingTransition { + id, + selector: hal::image::SubresourceRange { + aspects: hal::format::Aspects::empty(), + levels: level..level + 1, + layers: range.clone(), + }, + usage: old..usage, + }); + } + _ => { + unit.first = Some(usage); + } + } + } + } + Ok(()) + } + fn merge( &mut self, id: Self::Id,