diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 19235ce697..421e351636 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1159,6 +1159,10 @@ impl Global { } } + log::trace!("Merging {:?} with the render pass", encoder_id); + unsafe { + raw.end_render_pass(); + } super::CommandBuffer::insert_barriers( cmb.raw.last_mut().unwrap(), &mut cmb.trackers, @@ -1168,7 +1172,6 @@ impl Global { ); unsafe { cmb.raw.last_mut().unwrap().finish(); - raw.end_render_pass(); } cmb.raw.push(raw); } diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 1c05127a3a..dae5275868 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -1503,6 +1503,8 @@ impl Global { // execute resource transitions let mut transit = device.com_allocator.extend(comb); unsafe { + // the last buffer was open, closing now + comb.raw.last_mut().unwrap().finish(); transit.begin_primary(hal::command::CommandBufferFlags::ONE_TIME_SUBMIT); } log::trace!("Stitching command buffer {:?} before submission", cmb_id); @@ -1517,9 +1519,6 @@ impl Global { transit.finish(); } comb.raw.insert(0, transit); - unsafe { - comb.raw.last_mut().unwrap().finish(); - } } log::debug!("Device after submission {}: {:#?}", submit_index, trackers); diff --git a/wgpu-core/src/id.rs b/wgpu-core/src/id.rs index 897829e56d..a2bae01dc7 100644 --- a/wgpu-core/src/id.rs +++ b/wgpu-core/src/id.rs @@ -23,7 +23,10 @@ pub struct Id(NonZeroU64, PhantomData); // required for PeekPoke impl Default for Id { fn default() -> Self { - Id(unsafe { NonZeroU64::new_unchecked(!0) }, PhantomData) + Id( + unsafe { NonZeroU64::new_unchecked(!0 >> BACKEND_BITS) }, + PhantomData, + ) } } diff --git a/wgpu-core/src/track/buffer.rs b/wgpu-core/src/track/buffer.rs index fb882c3c68..fbeb3d640e 100644 --- a/wgpu-core/src/track/buffer.rs +++ b/wgpu-core/src/track/buffer.rs @@ -55,22 +55,28 @@ impl ResourceState for BufferState { output: Option<&mut Vec>>, ) -> Result<(), PendingTransition> { let old = self.last; - if usage != old || !BufferUsage::ORDERED.contains(usage) { + if old != usage || !BufferUsage::ORDERED.contains(usage) { let pending = PendingTransition { id, selector: (), usage: old..usage, }; - self.last = match output { - None => pending.collapse()?, + *self = match output { + None => { + assert_eq!( + self.first, None, + "extending a state that is already a transition" + ); + Unit::new(pending.collapse()?) + } Some(transitions) => { transitions.push(pending); - if self.first.is_none() { - self.first = Some(old); + Unit { + first: self.first.or(Some(old)), + last: usage, } - usage } - } + }; } Ok(()) } @@ -83,28 +89,33 @@ impl ResourceState for BufferState { ) -> Result<(), PendingTransition> { let old = self.last; let new = other.port(); - self.last = if old == new && BufferUsage::ORDERED.contains(new) { - if self.first.is_none() { + if old == new && BufferUsage::ORDERED.contains(new) { + if output.is_some() && self.first.is_none() { self.first = Some(old); } - other.last } else { let pending = PendingTransition { id, selector: (), usage: old..new, }; - match output { - None => pending.collapse()?, + *self = match output { + None => { + assert_eq!( + self.first, None, + "extending a state that is already a transition" + ); + Unit::new(pending.collapse()?) + } Some(transitions) => { transitions.push(pending); - if self.first.is_none() { - self.first = Some(old); + Unit { + first: self.first.or(Some(old)), + last: other.last, } - other.last } - } - }; + }; + } Ok(()) } @@ -114,19 +125,71 @@ impl ResourceState for BufferState { #[cfg(test)] mod test { use super::*; - use crate::id::TypedId; + use crate::id::Id; #[test] - fn change() { + fn change_extend() { let mut bs = Unit { - first: Some(BufferUsage::INDEX), + first: None, + last: BufferUsage::INDEX, + }; + let id = Id::default(); + assert_eq!( + bs.change(id, (), BufferUsage::STORAGE, None), + Err(PendingTransition { + id, + selector: (), + usage: BufferUsage::INDEX..BufferUsage::STORAGE, + }), + ); + bs.change(id, (), BufferUsage::VERTEX, None).unwrap(); + bs.change(id, (), BufferUsage::INDEX, None).unwrap(); + assert_eq!(bs, Unit::new(BufferUsage::VERTEX | BufferUsage::INDEX)); + } + + #[test] + fn change_replace() { + let mut bs = Unit { + first: None, last: BufferUsage::STORAGE, }; - let id = TypedId::zip(1, 0, wgt::Backend::Empty); - assert!(bs.change(id, (), BufferUsage::VERTEX, None).is_err()); - bs.change(id, (), BufferUsage::VERTEX, Some(&mut Vec::new())) + let id = Id::default(); + let mut list = Vec::new(); + bs.change(id, (), BufferUsage::VERTEX, Some(&mut list)) .unwrap(); - bs.change(id, (), BufferUsage::INDEX, None).unwrap(); - assert_eq!(bs.last, BufferUsage::VERTEX | BufferUsage::INDEX); + assert_eq!( + &list, + &[PendingTransition { + id, + selector: (), + usage: BufferUsage::STORAGE..BufferUsage::VERTEX, + }], + ); + assert_eq!( + bs, + Unit { + first: Some(BufferUsage::STORAGE), + last: BufferUsage::VERTEX, + } + ); + + list.clear(); + bs.change(id, (), BufferUsage::STORAGE, Some(&mut list)) + .unwrap(); + assert_eq!( + &list, + &[PendingTransition { + id, + selector: (), + usage: BufferUsage::VERTEX..BufferUsage::STORAGE, + }], + ); + assert_eq!( + bs, + Unit { + first: Some(BufferUsage::STORAGE), + last: BufferUsage::STORAGE, + } + ); } } diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index 1faefeb4c7..576a3d1f12 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -111,7 +111,7 @@ struct Resource { /// A structure containing all the information about a particular resource /// transition. User code should be able to generate a pipeline barrier /// based on the contents. -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub struct PendingTransition { pub id: S::Id, pub selector: S::Selector, diff --git a/wgpu-core/src/track/range.rs b/wgpu-core/src/track/range.rs index 0aa9a3602a..13376d97b0 100644 --- a/wgpu-core/src/track/range.rs +++ b/wgpu-core/src/track/range.rs @@ -9,7 +9,7 @@ use std::{cmp::Ordering, fmt::Debug, iter, ops::Range, slice::Iter}; /// Structure that keeps track of a I -> T mapping, /// optimized for a case where keys of the same values /// are often grouped together linearly. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub struct RangedStates { /// List of ranges, each associated with a singe value. /// Ranges of keys have to be non-intersecting and ordered. diff --git a/wgpu-core/src/track/texture.rs b/wgpu-core/src/track/texture.rs index d5010a8291..17ec6ac49f 100644 --- a/wgpu-core/src/track/texture.rs +++ b/wgpu-core/src/track/texture.rs @@ -13,7 +13,7 @@ use std::{iter, ops::Range}; //TODO: store `hal::image::State` here to avoid extra conversions type PlaneStates = RangedStates>; -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq)] pub struct TextureState { mips: ArrayVec<[PlaneStates; MAX_MIP_LEVELS]>, /// True if we have the information about all the subresources here @@ -116,14 +116,20 @@ impl ResourceState for TextureState { usage: unit.last..usage, }; - unit.last = match output { - None => pending.collapse()?, + *unit = match output { + None => { + assert_eq!( + unit.first, None, + "extending a state that is already a transition" + ); + Unit::new(pending.collapse()?) + } Some(ref mut out) => { out.push(pending); - if unit.first.is_none() { - unit.first = Some(unit.last); + Unit { + first: unit.first.or(Some(unit.last)), + last: usage, } - usage } }; } @@ -172,7 +178,10 @@ impl ResourceState for TextureState { let to_usage = end.port(); if start.last == to_usage && TextureUsage::ORDERED.contains(to_usage) { Unit { - first: start.first, + first: match output { + None => start.first, + Some(_) => start.first.or(Some(start.last)), + }, last: end.last, } } else { @@ -191,14 +200,18 @@ impl ResourceState for TextureState { }; match output { - None => Unit { - first: start.first, - last: pending.collapse()?, - }, + None => { + assert_eq!( + start.first, None, + "extending a state that is already a transition" + ); + Unit::new(pending.collapse()?) + } Some(ref mut out) => { out.push(pending); Unit { - first: Some(start.last), + // this has to leave a valid `first` state + first: start.first.or(Some(start.last)), last: end.last, } } @@ -222,9 +235,9 @@ impl ResourceState for TextureState { #[cfg(test)] mod test { - //TODO: change() and merge() tests - //use crate::TypedId; + //TODO: change() tests use super::*; + use crate::id::Id; use hal::{format::Aspects, image::SubresourceRange}; #[test] @@ -274,4 +287,151 @@ mod test { None, ); } + + #[test] + fn merge() { + let id = Id::default(); + let mut ts1 = TextureState::default(); + ts1.mips.push(PlaneStates::from_slice(&[( + 1..3, + Unit::new(TextureUsage::SAMPLED), + )])); + let mut ts2 = TextureState::default(); + assert_eq!( + ts1.merge(id, &ts2, None), + Ok(()), + "failed to merge with an emtpy" + ); + + ts2.mips.push(PlaneStates::from_slice(&[( + 1..2, + Unit::new(TextureUsage::COPY_SRC), + )])); + assert_eq!( + ts1.merge(Id::default(), &ts2, None), + Ok(()), + "failed to extend a compatible state" + ); + assert_eq!( + ts1.mips[0].query(&(1..2), |&v| v), + Some(Ok(Unit { + first: None, + last: TextureUsage::SAMPLED | TextureUsage::COPY_SRC, + })), + "wrong extension result" + ); + + ts2.mips[0] = PlaneStates::from_slice(&[(1..2, Unit::new(TextureUsage::COPY_DST))]); + assert_eq!( + ts1.clone().merge(Id::default(), &ts2, None), + Err(PendingTransition { + id, + selector: SubresourceRange { + aspects: Aspects::empty(), + levels: 0..1, + layers: 1..2, + }, + usage: TextureUsage::SAMPLED | TextureUsage::COPY_SRC..TextureUsage::COPY_DST, + }), + "wrong error on extending with incompatible state" + ); + + let mut list = Vec::new(); + ts2.mips[0] = PlaneStates::from_slice(&[ + (1..2, Unit::new(TextureUsage::COPY_DST)), + ( + 2..3, + Unit { + first: Some(TextureUsage::COPY_SRC), + last: TextureUsage::OUTPUT_ATTACHMENT, + }, + ), + ]); + ts1.merge(Id::default(), &ts2, Some(&mut list)).unwrap(); + assert_eq!( + &list, + &[ + PendingTransition { + id, + selector: SubresourceRange { + aspects: Aspects::empty(), + levels: 0..1, + layers: 1..2, + }, + usage: TextureUsage::SAMPLED | TextureUsage::COPY_SRC..TextureUsage::COPY_DST, + }, + PendingTransition { + id, + selector: SubresourceRange { + aspects: Aspects::empty(), + levels: 0..1, + layers: 2..3, + }, + // the transition links the end of the base rage (..SAMPLED) + // with the start of the next range (COPY_SRC..) + usage: TextureUsage::SAMPLED..TextureUsage::COPY_SRC, + }, + ], + "replacing produced wrong transitions" + ); + assert_eq!( + ts1.mips[0].query(&(1..2), |&v| v), + Some(Ok(Unit { + first: Some(TextureUsage::SAMPLED | TextureUsage::COPY_SRC), + last: TextureUsage::COPY_DST, + })), + "wrong final layer 1 state" + ); + assert_eq!( + ts1.mips[0].query(&(2..3), |&v| v), + Some(Ok(Unit { + first: Some(TextureUsage::SAMPLED), + last: TextureUsage::OUTPUT_ATTACHMENT, + })), + "wrong final layer 2 state" + ); + + list.clear(); + ts2.mips[0] = PlaneStates::from_slice(&[( + 2..3, + Unit { + first: Some(TextureUsage::OUTPUT_ATTACHMENT), + last: TextureUsage::COPY_SRC, + }, + )]); + ts1.merge(Id::default(), &ts2, Some(&mut list)).unwrap(); + assert_eq!(&list, &[], "unexpected replacing transition"); + + list.clear(); + ts2.mips[0] = PlaneStates::from_slice(&[( + 2..3, + Unit { + first: Some(TextureUsage::COPY_DST), + last: TextureUsage::COPY_DST, + }, + )]); + ts1.merge(Id::default(), &ts2, Some(&mut list)).unwrap(); + assert_eq!( + &list, + &[PendingTransition { + id, + selector: SubresourceRange { + aspects: Aspects::empty(), + levels: 0..1, + layers: 2..3, + }, + usage: TextureUsage::COPY_SRC..TextureUsage::COPY_DST, + },], + "invalid replacing transition" + ); + assert_eq!( + ts1.mips[0].query(&(2..3), |&v| v), + Some(Ok(Unit { + // the initial state here is never expected to change + first: Some(TextureUsage::SAMPLED), + last: TextureUsage::COPY_DST, + })), + "wrong final layer 2 state" + ); + } }