Merge #410

410: [0.4] Fix tracking of render pass attachments r=grovesNL a=kvark

Fixes #407
Also improves the debug output of the tracker.

Filed #409 for addressing this better.

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Change Log
 
+## v0.4.2 (15-12-2019)
+  - fixed render pass transitions
+
 ## v0.4.1 (28-11-2019)
   - fixed depth/stencil transitions
   - fixed dynamic offset iteration

wgpu-native/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "wgpu-native"
-version = "0.4.1"
+version = "0.4.2"
 authors = [
 	"Dzmitry Malyshau <kvark@mozilla.com>",
 	"Joshua Groves <josh@joshgroves.com>",

wgpu-native/src/command/compute.rs

@@ -63,6 +63,7 @@ pub fn compute_pass_end_pass<B: GfxBackend>(global: &Global, pass_id: ComputePas
 
     // There are no transitions to be made: we've already been inserting barriers
     // into the parent command buffer while recording this compute pass.
+    log::debug!("Compute pass {:?} tracker: {:#?}", pass_id, pass.trackers);
     cmb.trackers = pass.trackers;
     cmb.raw.push(pass.raw);
 }

wgpu-native/src/command/mod.rs

@@ -201,6 +201,7 @@ pub fn command_encoder_finish<B: GfxBackend>(
     if let Some((ref view_id, _)) = comb.used_swap_chain {
         comb.trackers.views.remove(view_id.value);
     }
+    log::debug!("Command buffer {:?} tracker: {:#?}", encoder_id, comb.trackers);
     encoder_id
 }
 
@@ -292,8 +293,17 @@ pub fn command_encoder_begin_render_pass<B: GfxBackend>(
                 let texture = &texture_guard[texture_id];
                 assert!(texture.usage.contains(TextureUsage::OUTPUT_ATTACHMENT));
 
-                let old_layout = match trackers.textures.query(texture_id, view.range.clone()) {
+                let consistent_usage = trackers.textures.query(texture_id, view.range.clone());
+                let pending = trackers.textures.change_replace(
+                    texture_id,
+                    &texture.life_guard.ref_count,
+                    view.range.clone(),
+                    TextureUsage::OUTPUT_ATTACHMENT,
+                );
+
+                let old_layout = match consistent_usage {
                     Some(usage) => {
+                        // Using render pass for transition.
                         conv::map_texture_state(
                             usage,
                             hal::format::Aspects::DEPTH | hal::format::Aspects::STENCIL,
@@ -303,13 +313,6 @@ pub fn command_encoder_begin_render_pass<B: GfxBackend>(
                     None => {
                         // Required sub-resources have inconsistent states, we need to
                         // issue individual barriers instead of relying on the render pass.
-                        let pending = trackers.textures.change_replace(
-                            texture_id,
-                            &texture.life_guard.ref_count,
-                            view.range.clone(),
-                            TextureUsage::OUTPUT_ATTACHMENT,
-                        );
-
                         barriers.extend(pending.map(|pending| {
                             log::trace!("\tdepth-stencil {:?}", pending);
                             hal::memory::Barrier::Image {
@@ -355,32 +358,34 @@ pub fn command_encoder_begin_render_pass<B: GfxBackend>(
                         let texture = &texture_guard[source_id.value];
                         assert!(texture.usage.contains(TextureUsage::OUTPUT_ATTACHMENT));
 
-                        let old_layout =
-                            match trackers.textures.query(source_id.value, view.range.clone()) {
-                                Some(usage) => {
-                                    conv::map_texture_state(usage, hal::format::Aspects::COLOR).1
-                                }
-                                None => {
-                                    // Required sub-resources have inconsistent states, we need to
-                                    // issue individual barriers instead of relying on the render pass.
-                                    let pending = trackers.textures.change_replace(
-                                        source_id.value,
-                                        &texture.life_guard.ref_count,
-                                        view.range.clone(),
-                                        TextureUsage::OUTPUT_ATTACHMENT,
-                                    );
-                                    barriers.extend(pending.map(|pending| {
-                                        log::trace!("\tcolor {:?}", pending);
-                                        hal::memory::Barrier::Image {
-                                            states: pending.to_states(),
-                                            target: &texture.raw,
-                                            families: None,
-                                            range: pending.selector,
-                                        }
-                                    }));
-                                    hal::image::Layout::ColorAttachmentOptimal
-                                }
-                            };
+                        let consistent_usage = trackers.textures.query(source_id.value, view.range.clone());
+                        let pending = trackers.textures.change_replace(
+                            source_id.value,
+                            &texture.life_guard.ref_count,
+                            view.range.clone(),
+                            TextureUsage::OUTPUT_ATTACHMENT,
+                        );
+
+                        let old_layout = match consistent_usage {
+                            Some(usage) => {
+                                // Using render pass for transition.
+                                conv::map_texture_state(usage, hal::format::Aspects::COLOR).1
+                            }
+                            None => {
+                                // Required sub-resources have inconsistent states, we need to
+                                // issue individual barriers instead of relying on the render pass.
+                                barriers.extend(pending.map(|pending| {
+                                    log::trace!("\tcolor {:?}", pending);
+                                    hal::memory::Barrier::Image {
+                                        states: pending.to_states(),
+                                        target: &texture.raw,
+                                        families: None,
+                                        range: pending.selector,
+                                    }
+                                }));
+                                hal::image::Layout::ColorAttachmentOptimal
+                            }
+                        };
                         old_layout .. hal::image::Layout::ColorAttachmentOptimal
                     }
                     TextureViewInner::SwapChain { .. } => {
@@ -433,32 +438,34 @@ pub fn command_encoder_begin_render_pass<B: GfxBackend>(
                         let texture = &texture_guard[source_id.value];
                         assert!(texture.usage.contains(TextureUsage::OUTPUT_ATTACHMENT));
 
-                        let old_layout =
-                            match trackers.textures.query(source_id.value, view.range.clone()) {
-                                Some(usage) => {
-                                    conv::map_texture_state(usage, hal::format::Aspects::COLOR).1
-                                }
-                                None => {
-                                    // Required sub-resources have inconsistent states, we need to
-                                    // issue individual barriers instead of relying on the render pass.
-                                    let pending = trackers.textures.change_replace(
-                                        source_id.value,
-                                        &texture.life_guard.ref_count,
-                                        view.range.clone(),
-                                        TextureUsage::OUTPUT_ATTACHMENT,
-                                    );
-                                    barriers.extend(pending.map(|pending| {
-                                        log::trace!("\tresolve {:?}", pending);
-                                        hal::memory::Barrier::Image {
-                                            states: pending.to_states(),
-                                            target: &texture.raw,
-                                            families: None,
-                                            range: pending.selector,
-                                        }
-                                    }));
-                                    hal::image::Layout::ColorAttachmentOptimal
-                                }
-                            };
+                        let consistent_usage = trackers.textures.query(source_id.value, view.range.clone());
+                        let pending = trackers.textures.change_replace(
+                            source_id.value,
+                            &texture.life_guard.ref_count,
+                            view.range.clone(),
+                            TextureUsage::OUTPUT_ATTACHMENT,
+                        );
+
+                        let old_layout = match consistent_usage {
+                            Some(usage) => {
+                                // Using render pass for transition.
+                                conv::map_texture_state(usage, hal::format::Aspects::COLOR).1
+                            }
+                            None => {
+                                // Required sub-resources have inconsistent states, we need to
+                                // issue individual barriers instead of relying on the render pass.
+                                barriers.extend(pending.map(|pending| {
+                                    log::trace!("\tresolve {:?}", pending);
+                                    hal::memory::Barrier::Image {
+                                        states: pending.to_states(),
+                                        target: &texture.raw,
+                                        families: None,
+                                        range: pending.selector,
+                                    }
+                                }));
+                                hal::image::Layout::ColorAttachmentOptimal
+                            }
+                        };
                         old_layout .. hal::image::Layout::ColorAttachmentOptimal
                     }
                     TextureViewInner::SwapChain { .. } => {

wgpu-native/src/command/render.rs

@@ -190,6 +190,8 @@ pub fn render_pass_end_pass<B: GfxBackend>(global: &Global, pass_id: RenderPassI
         pass.raw.end_render_pass();
     }
     pass.trackers.optimize();
+    log::debug!("Render pass {:?} tracker: {:#?}", pass_id, pass.trackers);
+
     let cmb = &mut cmb_guard[pass.cmb_id.value];
     let (buffer_guard, mut token) = hub.buffers.read(&mut token);
     let (texture_guard, _) = hub.textures.read(&mut token);

wgpu-native/src/device.rs

@@ -1413,6 +1413,8 @@ pub fn device_create_bind_group<B: GfxBackend>(
         dynamic_count: bind_group_layout.dynamic_count,
     };
     let (id, id_out) = hub.bind_groups.new_identity(id_in);
+
+    log::debug!("Bind group {:?} tracker: {:#?}", id, bind_group.used);
     let ok = device
         .trackers
         .lock()
@@ -1650,6 +1652,8 @@ pub fn queue_submit<B: GfxBackend>(
             }
         }
 
+        log::debug!("Device tracker after submission: {:#?}", trackers);
+
         // now prepare the GPU submission
         let fence = device.raw.create_fence(false).unwrap();
         let submission = hal::queue::Submission::<_, _, Vec<&B::Semaphore>> {

wgpu-native/src/track/mod.rs

@@ -22,7 +22,7 @@ use crate::{
 use std::{
     borrow::Borrow,
     collections::hash_map::Entry,
-    fmt::Debug,
+    fmt,
     marker::PhantomData,
     ops::Range,
     vec::Drain,
@@ -76,11 +76,11 @@ pub enum Stitch {
 /// a particular resource type, like a buffer or a texture.
 pub trait ResourceState: Clone + Default {
     /// Corresponding `HUB` identifier.
-    type Id: Copy + Debug + TypedId;
+    type Id: Copy + fmt::Debug + TypedId;
     /// A type specifying the sub-resources.
-    type Selector: Debug;
+    type Selector: fmt::Debug;
     /// Usage type for a `Unit` of a sub-resource.
-    type Usage: Debug;
+    type Usage: fmt::Debug;
 
     /// Check if all the selected sub-resources have the same
     /// usage, and return it.
@@ -135,7 +135,7 @@ pub trait ResourceState: Clone + Default {
 
 /// Structure wrapping the abstract tracking state with the relevant resource
 /// data, such as the reference count and the epoch.
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 struct Resource<S> {
     ref_count: RefCount,
     state: S,
@@ -153,7 +153,6 @@ pub struct PendingTransition<S: ResourceState> {
 }
 
 /// A tracker for all resources of a given type.
-#[derive(Debug)]
 pub struct ResourceTracker<S: ResourceState> {
     /// An association of known resource indices with their tracked states.
     map: FastHashMap<Index, Resource<S>>,
@@ -163,6 +162,18 @@ pub struct ResourceTracker<S: ResourceState> {
     backend: Backend,
 }
 
+impl<S: ResourceState + fmt::Debug> fmt::Debug for ResourceTracker<S> {
+    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+        self.map
+            .iter()
+            .map(|(&index, res)| {
+                ((index, res.epoch), &res.state)
+            })
+            .collect::<FastHashMap<_, _>>()
+            .fmt(formatter)
+    }
+}
+
 impl<S: ResourceState> ResourceTracker<S> {
     /// Create a new empty tracker.
     pub fn new(backend: Backend) -> Self {
@@ -383,7 +394,7 @@ impl<S: ResourceState> ResourceTracker<S> {
 }
 
 
-impl<I: Copy + Debug + TypedId> ResourceState for PhantomData<I> {
+impl<I: Copy + fmt::Debug + TypedId> ResourceState for PhantomData<I> {
     type Id = I;
     type Selector = ();
     type Usage = ();