712: RODS part 2 r=cwfitzgerald a=kvark

**Connections**
Unblocks https://github.com/gfx-rs/wgpu-rs/issues/359
Is a follow-up to RODS part 1 - #685

**Description**
There is a few things in here.

Thing 1: Stronger assertions on the load/store ops of the depth & stencil.

### Thing 2: rewritten tracking of render attachments
Background: from the usage tracker point of view, each subresource can be in either "extend" mode, where it accumulates a single combined usage, or in the "replace" mode, where it goes from one usage to another, producing the relevant transitions on the way.

The problem turned out to come from the fact that the render pass attachments were always tracked in "replace" mode. This is needed because their track don't have a single state: render pass itself encodes a transition of attachments. However, it also means that there was no way to "extend" the usage in RODS scenarios...

So I could see two ways to address this:
  - re-achitecture the tracking a bit more in general, representing it as a sequence of merges.
  - introduce the "prepend()" tracking operator that's *only* used for render pass attachments

I opted for the latter as it seems much less intrusive. The render pass attachments accumulate their usage like everything else in the "extend mode". But right before we are inserting the transitions (between the active command buffer and the pass), we turn the tracking of the attachments from "extend" into "replace" mode by installing the "first" usage according to what we expect it to be.

### Thing 3: missing API for RODS bind groups
The original RODS design missed a problem with Vulkan image layouts. When creating a bind group, one has to specify what layout the image will be in. We always used `ShaderReadOnlyOptimal` until now for texture views. However, in RODS scenarios this has to be `DepthStencilReadOnlyOptimal`. Luckily, it's compatible with sampling from the shader, but we still need to know about this when creating the bind group.

**Testing**
Tested on the modified water example provided in https://github.com/gfx-rs/wgpu-rs/issues/359#issuecomment-642167269 

Added a few tests to the buffer implementation of the new `prepend()` operator.

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
This commit is contained in:
bors[bot] 2020-06-12 23:58:36 +00:00 committed by GitHub
commit 0357dd80af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 179 additions and 40 deletions

View File

@ -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<G: GlobalIdentityHandlerFactory> Global<G> {
// 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<id::TextureId>,
range: &'a hal::image::SubresourceRange,
previous_use: Option<TextureUse>,
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<G: GlobalIdentityHandlerFactory> Global<G> {
"Attachment sample_count must be supported by physical device limits"
);
const MAX_TOTAL_ATTACHMENTS: usize = 10;
struct OutputAttachment<'a> {
texture_id: &'a Stored<id::TextureId>,
range: &'a hal::image::SubresourceRange,
previous_use: Option<TextureUse>,
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<G: GlobalIdentityHandlerFactory> Global<G> {
}
};
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<G: GlobalIdentityHandlerFactory> Global<G> {
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<G: GlobalIdentityHandlerFactory> Global<G> {
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,

View File

@ -79,6 +79,25 @@ impl ResourceState for BufferState {
Ok(())
}
fn prepend(
&mut self,
id: Self::Id,
_selector: Self::Selector,
usage: Self::Usage,
) -> Result<(), PendingTransition<Self>> {
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,
}
);
}
}

View File

@ -80,6 +80,14 @@ pub trait ResourceState: Clone + Default {
output: Option<&mut Vec<PendingTransition<Self>>>,
) -> Result<(), PendingTransition<Self>>;
/// 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<Self>>;
/// Merge the state of this resource tracked by a different instance
/// with the current one.
///
@ -321,6 +329,21 @@ impl<S: ResourceState> ResourceTracker<S> {
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<S>> {
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<S>> {
@ -415,6 +438,15 @@ impl<I: Copy + fmt::Debug + TypedId> ResourceState for PhantomData<I> {
Ok(())
}
fn prepend(
&mut self,
_id: Self::Id,
_selector: Self::Selector,
_usage: Self::Usage,
) -> Result<(), PendingTransition<Self>> {
Ok(())
}
fn merge(
&mut self,
_id: Self::Id,

View File

@ -136,6 +136,41 @@ impl ResourceState for TextureState {
Ok(())
}
fn prepend(
&mut self,
id: Self::Id,
selector: Self::Selector,
usage: Self::Usage,
) -> Result<(), PendingTransition<Self>> {
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,