nordic-dev.net/wgpu
nordic-dev.net/wgpu
2
0
Fork 0
mirror of https://github.com/gfx-rs/wgpu.git synced 2024-11-22 14:55:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix tracking of the initial state during replacement.

Browse Source 
When multiple "replace" style transitions are happening,
we weren't properly retaining the "first" state, which
is required for proper stitching of command buffers.

This logic is fixed and fortified with a new set of
"change" and "merge" tests in the track module.
This commit is contained in:
Dzmitry Malyshau 2020-04-09 23:25:16 -04:00
parent 74e9e89fe7
commit 920c00931d
7 changed files with 274 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
wgpu-core/src/command/render.rs
View File

@ -1159,6 +1159,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}
}
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<G: GlobalIdentityHandlerFactory> Global<G> {
);
unsafe {
cmb.raw.last_mut().unwrap().finish();
raw.end_render_pass();
}
cmb.raw.push(raw);
}

5
wgpu-core/src/device/mod.rs
View File

@ -1503,6 +1503,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
// 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<G: GlobalIdentityHandlerFactory> Global<G> {
transit.finish();
}
comb.raw.insert(0, transit);
unsafe {
comb.raw.last_mut().unwrap().finish();
}
}
log::debug!("Device after submission {}: {:#?}", submit_index, trackers);

5
wgpu-core/src/id.rs
View File

@ -23,7 +23,10 @@ pub struct Id<T>(NonZeroU64, PhantomData<T>);
// required for PeekPoke
impl<T> Default for Id<T> {
fn default() -> Self {
Id(unsafe { NonZeroU64::new_unchecked(!0) }, PhantomData)
Id(
unsafe { NonZeroU64::new_unchecked(!0 >> BACKEND_BITS) },
PhantomData,
)
}
}

113
wgpu-core/src/track/buffer.rs
View File

@ -55,22 +55,28 @@ impl ResourceState for BufferState {
output: Option<&mut Vec<PendingTransition<Self>>>,
) -> Result<(), PendingTransition<Self>> {
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<Self>> {
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,
}
);
}
}

2
wgpu-core/src/track/mod.rs
View File

@ -111,7 +111,7 @@ struct Resource<S> {
/// 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<S: ResourceState> {
pub id: S::Id,
pub selector: S::Selector,

2
wgpu-core/src/track/range.rs
View File

@ -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<I, T> {
/// List of ranges, each associated with a singe value.
/// Ranges of keys have to be non-intersecting and ordered.

188
wgpu-core/src/track/texture.rs
View File

@ -13,7 +13,7 @@ use std::{iter, ops::Range};
//TODO: store `hal::image::State` here to avoid extra conversions
type PlaneStates = RangedStates<hal::image::Layer, Unit<TextureUsage>>;
#[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"
);
}
}