diff --git a/CHANGELOG.md b/CHANGELOG.md index b6871a07de..00d5d5c8be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Change Log +## v0.5.1 (10-04-2020) + - fix tracking of swapchain images that are used multiple times in a command buffer + - fix tracking of initial usage of a resource across a command buffer + ## v0.5 (06-04-2020) - Crates: - `wgpu-types`: common types between native and web targets diff --git a/Cargo.lock b/Cargo.lock index d55279d6f8..7e76dac1a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -653,7 +653,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "wgpu-core" -version = "0.5.0" +version = "0.5.1" dependencies = [ "arrayvec 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", "battery 0.7.5 (registry+https://github.com/rust-lang/crates.io-index)", @@ -687,7 +687,7 @@ dependencies = [ "objc 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", "raw-window-handle 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", - "wgpu-core 0.5.0", + "wgpu-core 0.5.1", "wgpu-types 0.5.0", ] @@ -697,7 +697,7 @@ version = "0.1.0" dependencies = [ "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", - "wgpu-core 0.5.0", + "wgpu-core 0.5.1", "wgpu-types 0.5.0", ] diff --git a/wgpu-core/Cargo.toml b/wgpu-core/Cargo.toml index 0b1185bbd5..839d752f32 100644 --- a/wgpu-core/Cargo.toml +++ b/wgpu-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "wgpu-core" -version = "0.5.0" +version = "0.5.1" authors = [ "Dzmitry Malyshau ", "Joshua Groves ", diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 41e82c7809..3bd16fd86b 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -29,9 +29,7 @@ use wgt::{ TextureUsage, BIND_BUFFER_ALIGNMENT, }; -use std::{ - borrow::Borrow, collections::hash_map::Entry, iter, marker::PhantomData, mem, ops::Range, slice, -}; +use std::{borrow::Borrow, collections::hash_map::Entry, iter, mem, ops::Range, slice}; pub type RenderPassColorAttachmentDescriptor = RenderPassColorAttachmentDescriptorBase; @@ -438,7 +436,10 @@ impl Global { let mut resolves = ArrayVec::new(); for at in &color_attachments { - let view = &view_guard[at.attachment]; + let view = trackers + .views + .use_extend(&*view_guard, at.attachment, (), ()) + .unwrap(); if let Some(ex) = extent { assert_eq!(ex, view.extent); } else { @@ -448,10 +449,6 @@ impl Global { view.samples, sample_count, "All attachments must have the same sample_count" ); - let first_use = trackers - .views - .init(at.attachment, view.life_guard.add_ref(), PhantomData) - .is_ok(); let layouts = match view.inner { TextureViewInner::Native { ref source_id, .. } => { @@ -477,10 +474,9 @@ impl Global { } let end = hal::image::Layout::Present; - let start = if first_use { - hal::image::Layout::Undefined - } else { - end + let start = match base_trackers.views.query(at.attachment, ()) { + Some(_) => end, + None => hal::image::Layout::Undefined, }; start..end } @@ -496,16 +492,15 @@ impl Global { } for resolve_target in color_attachments.iter().flat_map(|at| at.resolve_target) { - let view = &view_guard[resolve_target]; + let view = trackers + .views + .use_extend(&*view_guard, resolve_target, (), ()) + .unwrap(); assert_eq!(extent, Some(view.extent)); assert_eq!( view.samples, 1, "All resolve_targets must have a sample_count of 1" ); - let first_use = trackers - .views - .init(resolve_target, view.life_guard.add_ref(), PhantomData) - .is_ok(); let layouts = match view.inner { TextureViewInner::Native { ref source_id, .. } => { @@ -531,10 +526,9 @@ impl Global { } let end = hal::image::Layout::Present; - let start = if first_use { - hal::image::Layout::Undefined - } else { - end + let start = match base_trackers.views.query(resolve_target, ()) { + Some(_) => end, + None => hal::image::Layout::Undefined, }; start..end } @@ -1158,6 +1152,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, @@ -1167,7 +1165,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 2091b521b4..2888a2310f 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" + ); + } }