From d74c8cb5ae78b91bd2bd668b47898ea7b9eac2dc Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Mon, 16 Dec 2019 22:15:00 -0500 Subject: [PATCH] Refactor tracker stitches --- wgpu-core/src/command/compute.rs | 3 +- wgpu-core/src/command/mod.rs | 8 ++-- wgpu-core/src/command/render.rs | 3 +- wgpu-core/src/device.rs | 3 +- wgpu-core/src/track/buffer.rs | 48 +++++++++++--------- wgpu-core/src/track/mod.rs | 38 +++------------- wgpu-core/src/track/texture.rs | 76 +++++++++++++++----------------- 7 files changed, 76 insertions(+), 103 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 3079e03e86..01553e6168 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -11,7 +11,7 @@ use crate::{ hub::{GfxBackend, Global, IdentityFilter, Token}, id::{BindGroupId, BufferId, CommandBufferId, ComputePassId, ComputePipelineId}, resource::BufferUsage, - track::{Stitch, TrackerSet}, + track::TrackerSet, BufferAddress, Stored, }; @@ -112,7 +112,6 @@ impl Global { &mut pass.raw, &mut pass.trackers, &bind_group.used, - Stitch::Last, &*buffer_guard, &*texture_guard, ); diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index c0e923e037..96d10e90d6 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -34,7 +34,7 @@ use crate::{ TextureViewId, }, resource::{Buffer, Texture, TextureUsage, TextureViewInner}, - track::{Stitch, TrackerSet}, + track::TrackerSet, Color, Features, LifeGuard, @@ -118,17 +118,15 @@ impl CommandBuffer { raw: &mut B::CommandBuffer, base: &mut TrackerSet, head: &TrackerSet, - stitch: Stitch, buffer_guard: &Storage, BufferId>, texture_guard: &Storage, TextureId>, ) { - log::trace!("\tstitch {:?}", stitch); debug_assert_eq!(B::VARIANT, base.backend()); debug_assert_eq!(B::VARIANT, head.backend()); let buffer_barriers = base .buffers - .merge_replace(&head.buffers, stitch) + .merge_replace(&head.buffers) .map(|pending| { log::trace!("\tbuffer -> {:?}", pending); hal::memory::Barrier::Buffer { @@ -140,7 +138,7 @@ impl CommandBuffer { }); let texture_barriers = base .textures - .merge_replace(&head.textures, stitch) + .merge_replace(&head.textures) .map(|pending| { log::trace!("\ttexture -> {:?}", pending); hal::memory::Barrier::Image { diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index d75220cfd4..059830c6a9 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -13,7 +13,7 @@ use crate::{ id::{BindGroupId, BufferId, CommandBufferId, RenderPassId, RenderPipelineId}, pipeline::{IndexFormat, InputStepMode, PipelineFlags}, resource::BufferUsage, - track::{Stitch, TrackerSet}, + track::TrackerSet, BufferAddress, Color, Stored, @@ -199,7 +199,6 @@ impl> Global { last, &mut cmb.trackers, &pass.trackers, - Stitch::Last, &*buffer_guard, &*texture_guard, ); diff --git a/wgpu-core/src/device.rs b/wgpu-core/src/device.rs index d55c8d2d29..b743bb10fc 100644 --- a/wgpu-core/src/device.rs +++ b/wgpu-core/src/device.rs @@ -29,7 +29,7 @@ use crate::{ pipeline, resource, swap_chain, - track::{SEPARATE_DEPTH_STENCIL_STATES, Stitch, TrackerSet}, + track::{SEPARATE_DEPTH_STENCIL_STATES, TrackerSet}, BufferAddress, FastHashMap, Features, @@ -1618,7 +1618,6 @@ impl> Global { &mut transit, &mut *trackers, &comb.trackers, - Stitch::Init, &*buffer_guard, &*texture_guard, ); diff --git a/wgpu-core/src/track/buffer.rs b/wgpu-core/src/track/buffer.rs index 62d48cb160..9a4feee77a 100644 --- a/wgpu-core/src/track/buffer.rs +++ b/wgpu-core/src/track/buffer.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use super::{PendingTransition, ResourceState, Stitch, Unit}; +use super::{PendingTransition, ResourceState, Unit}; use crate::{conv, id::BufferId, resource::BufferUsage}; use std::ops::Range; @@ -14,6 +14,17 @@ impl PendingTransition { pub fn to_states(&self) -> Range { conv::map_buffer_state(self.usage.start) .. conv::map_buffer_state(self.usage.end) } + + fn collapse(self) -> Result { + if self.usage.start.is_empty() + || self.usage.start == self.usage.end + || !BufferUsage::WRITE_ALL.intersects(self.usage.start | self.usage.end) + { + Ok(self.usage.start | self.usage.end) + } else { + Err(self) + } + } } impl ResourceState for BufferState { @@ -23,7 +34,7 @@ impl ResourceState for BufferState { fn new(_full_selector: &Self::Selector) -> Self { BufferState { - init: BufferUsage::empty(), + first: None, last: BufferUsage::empty(), } } @@ -47,20 +58,15 @@ impl ResourceState for BufferState { usage: old .. usage, }; self.last = match output { + None => pending.collapse()?, Some(transitions) => { transitions.push(pending); + if self.first.is_none() { + self.first = Some(old); + } usage } - None => { - if !old.is_empty() - && old != usage - && BufferUsage::WRITE_ALL.intersects(old | usage) - { - return Err(pending); - } - old | usage - } - }; + } } Ok(()) } @@ -69,12 +75,14 @@ impl ResourceState for BufferState { &mut self, id: Self::Id, other: &Self, - stitch: Stitch, output: Option<&mut Vec>>, ) -> Result<(), PendingTransition> { let old = self.last; - let new = other.select(stitch); + let new = other.port(); self.last = if old == new && BufferUsage::ORDERED.contains(new) { + if self.first.is_none() { + self.first = Some(old); + } other.last } else { let pending = PendingTransition { @@ -83,15 +91,13 @@ impl ResourceState for BufferState { usage: old .. new, }; match output { + None => pending.collapse()?, Some(transitions) => { transitions.push(pending); - other.last - } - None => { - if !old.is_empty() && BufferUsage::WRITE_ALL.intersects(old | new) { - return Err(pending); + if self.first.is_none() { + self.first = Some(old); } - old | new + other.last } } }; @@ -109,7 +115,7 @@ mod test { #[test] fn change() { let mut bs = Unit { - init: BufferUsage::INDEX, + first: Some(BufferUsage::INDEX), last: BufferUsage::STORAGE, }; let id = TypedId::zip(0, 0, Backend::Empty); diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index 8decc623a3..55fc1c5433 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -35,7 +35,7 @@ pub const SEPARATE_DEPTH_STENCIL_STATES: bool = false; /// usage as well as the last/current one, similar to `Range`. #[derive(Clone, Copy, Debug, PartialEq)] pub struct Unit { - init: U, + first: Option, last: U, } @@ -43,34 +43,17 @@ impl Unit { /// Create a new unit from a given usage. fn new(usage: U) -> Self { Unit { - init: usage, + first: None, last: usage, } } - /// Select one of the ends of the usage, based on the - /// given `Stitch`. - /// - /// In some scenarios, when merging two trackers - /// A and B for a resource, we want to connect A to the initial state - /// of B. In other scenarios, we want to reach the last state of B. - fn select(&self, stitch: Stitch) -> U { - match stitch { - Stitch::Init => self.init, - Stitch::Last => self.last, - } + /// Return a usage to link to. + fn port(&self) -> U { + self.first.unwrap_or(self.last) } } -/// Mode of stitching to states together. -#[derive(Clone, Copy, Debug, PartialEq)] -pub enum Stitch { - /// Stitch to the init state of the other resource. - Init, - /// Stitch to the last state of the other resource. - Last, -} - /// The main trait that abstracts away the tracking logic of /// a particular resource type, like a buffer or a texture. pub trait ResourceState: Clone { @@ -119,15 +102,10 @@ pub trait ResourceState: Clone { /// `PendingTransition` pushed to this vector, or extended with the /// other read-only usage, unless there is a usage conflict, and /// the error is generated (returning the conflict). - /// - /// `stitch` only defines the end points of generated transitions. - /// Last states of `self` are nevertheless updated to the *last* states - /// of `other`, if `output` is provided. fn merge( &mut self, id: Self::Id, other: &Self, - stitch: Stitch, output: Option<&mut Vec>>, ) -> Result<(), PendingTransition>; @@ -331,7 +309,7 @@ impl ResourceTracker { let id = S::Id::zip(index, new.epoch, self.backend); e.into_mut() .state - .merge(id, &new.state, Stitch::Last, None)?; + .merge(id, &new.state, None)?; } } } @@ -343,7 +321,6 @@ impl ResourceTracker { pub fn merge_replace<'a>( &'a mut self, other: &'a Self, - stitch: Stitch, ) -> Drain> { for (&index, new) in other.map.iter() { match self.map.entry(index) { @@ -355,7 +332,7 @@ impl ResourceTracker { let id = S::Id::zip(index, new.epoch, self.backend); e.into_mut() .state - .merge(id, &new.state, stitch, Some(&mut self.temp)) + .merge(id, &new.state, Some(&mut self.temp)) .ok(); //TODO: unwrap? } } @@ -426,7 +403,6 @@ impl ResourceState for PhantomData { &mut self, _id: Self::Id, _other: &Self, - _stitch: Stitch, _output: Option<&mut Vec>>, ) -> Result<(), PendingTransition> { Ok(()) diff --git a/wgpu-core/src/track/texture.rs b/wgpu-core/src/track/texture.rs index 9a00245cbc..b37e19ad7b 100644 --- a/wgpu-core/src/track/texture.rs +++ b/wgpu-core/src/track/texture.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use super::{SEPARATE_DEPTH_STENCIL_STATES, range::RangedStates, PendingTransition, ResourceState, Stitch, Unit}; +use super::{SEPARATE_DEPTH_STENCIL_STATES, range::RangedStates, PendingTransition, ResourceState, Unit}; use crate::{conv, device::MAX_MIP_LEVELS, id::TextureId, resource::TextureUsage}; use arrayvec::ArrayVec; @@ -26,34 +26,14 @@ impl PendingTransition { .. conv::map_texture_state(self.usage.end, self.selector.aspects) } - //TODO: make this less awkward! - /// Check for the validity of `self` with regards to the presence of `output`. - /// - /// Return the end usage if the `output` is provided and pushes self to it. - /// Otherwise, return the extended usage, or an error if extension is impossible. - /// - /// When a transition is generated, returns the specified `replace` usage. - fn record( - self, - output: Option<&mut &mut Vec>, - replace: TextureUsage, - ) -> Result { - let u = self.usage.clone(); - match output { - Some(out) => { - out.push(self); - Ok(replace) - } - None => { - if !u.start.is_empty() - && u.start != u.end - && TextureUsage::WRITE_ALL.intersects(u.start | u.end) - { - Err(self) - } else { - Ok(u.start | u.end) - } - } + fn collapse(self) -> Result { + if self.usage.start.is_empty() + || self.usage.start == self.usage.end + || !TextureUsage::WRITE_ALL.intersects(self.usage.start | self.usage.end) + { + Ok(self.usage.start | self.usage.end) + } else { + Err(self) } } } @@ -137,7 +117,17 @@ impl ResourceState for TextureState { }, usage: unit.last .. usage, }; - unit.last = pending.record(output.as_mut(), usage)?; + + unit.last = match output { + None => pending.collapse()?, + Some(ref mut out) => { + out.push(pending); + if unit.first.is_none() { + unit.first = Some(unit.last); + } + usage + } + }; } } } @@ -148,11 +138,8 @@ impl ResourceState for TextureState { &mut self, id: Self::Id, other: &Self, - stitch: Stitch, mut output: Option<&mut Vec>>, ) -> Result<(), PendingTransition> { - assert!(output.is_some() || stitch == Stitch::Last); - let mut temp = Vec::new(); while self.mips.len() < other.mips.len() as usize { self.mips.push(MipState::default()); @@ -183,9 +170,9 @@ impl ResourceState for TextureState { start: Some(start), end: Some(end), } => { - let mut final_usage = end.select(stitch); - if start.last != final_usage - || !TextureUsage::ORDERED.contains(final_usage) + let to_usage = end.port(); + let final_usage = if start.last != to_usage + || !TextureUsage::ORDERED.contains(to_usage) { let pending = PendingTransition { id, @@ -194,12 +181,21 @@ impl ResourceState for TextureState { levels: level .. level + 1, layers: layers.clone(), }, - usage: start.last .. final_usage, + usage: start.last .. to_usage, }; - final_usage = pending.record(output.as_mut(), end.last)?; - } + + match output { + None => pending.collapse()?, + Some(ref mut out) => { + out.push(pending); + end.last + } + } + } else { + end.last + }; Unit { - init: start.init, + first: Some(start.port()), last: final_usage, } }