Merge #302

302: Fixed pipeline barriers that are not transitions r=grovesNL a=kvark

The actual fix is a one-liner: `u.start != u.end` bit in `PendingTransition::record`. The case is relatively new - as of #281, which I haven't tested extensively.
The PR also improves our logging for further assistance with similar issues... but the most annoying piece is that I would find this much much earlier if I didn't ignore the result here: `let _ = state.change(...)`. Let it be the lesson to all of us :)

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

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Change Log
 
+## v0.3.1 (21-08-2019)
+  - fixed pipeline barriers that aren't transitions
+
 ## v0.3 (21-08-2019)
   - Platforms: experimental OpenGL/WebGL
   - Crates:

Cargo.lock

@@ -1419,7 +1419,7 @@ dependencies = [
 
 [[package]]
 name = "wgpu-native"
-version = "0.3.0"
+version = "0.3.1"
 dependencies = [
  "arrayvec 0.4.10 (registry+https://github.com/rust-lang/crates.io-index)",
  "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1449,7 +1449,7 @@ dependencies = [
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.91 (registry+https://github.com/rust-lang/crates.io-index)",
- "wgpu-native 0.3.0",
+ "wgpu-native 0.3.1",
 ]
 
 [[package]]

wgpu-native/Cargo.toml

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

wgpu-native/src/command/compute.rs

@@ -17,6 +17,7 @@ use crate::{
 };
 
 use hal::{self, command::RawCommandBuffer};
+use log::trace;
 
 use std::{iter, slice};
 
@@ -102,6 +103,7 @@ pub extern "C" fn wgpu_compute_pass_set_bind_group(
     let (buffer_guard, mut token) = HUB.buffers.read(&mut token);
     let (texture_guard, _) = HUB.textures.read(&mut token);
 
+    trace!("Encoding barriers on binding of {:?} in pass {:?}", bind_group_id, pass_id);
     CommandBuffer::insert_barriers(
         &mut pass.raw,
         &mut pass.trackers,

wgpu-native/src/command/mod.rs

@@ -114,11 +114,12 @@ impl CommandBufferHandle {
         buffer_guard: &Storage<BufferHandle, BufferId>,
         texture_guard: &Storage<TextureHandle, TextureId>,
     ) {
+        trace!("\tstitch {:?}", stitch);
         let buffer_barriers =
             base.buffers
                 .merge_replace(&head.buffers, stitch)
                 .map(|pending| {
-                    trace!("transit buffer {:?}", pending);
+                    trace!("\tbuffer -> {:?}", pending);
                     hal::memory::Barrier::Buffer {
                         states: pending.to_states(),
                         target: &buffer_guard[pending.id].raw,
@@ -130,7 +131,7 @@ impl CommandBufferHandle {
             .textures
             .merge_replace(&head.textures, stitch)
             .map(|pending| {
-                trace!("transit texture {:?}", pending);
+                trace!("\ttexture -> {:?}", pending);
                 hal::memory::Barrier::Image {
                     states: pending.to_states(),
                     target: &texture_guard[pending.id].raw,
@@ -221,6 +222,7 @@ pub fn command_encoder_begin_render_pass(
         }
     }
 
+    trace!("Encoding render pass begin in command buffer {:?}", command_encoder_id);
     let rp_key = {
         let trackers = &mut cmb.trackers;
         let swap_chain_links = &mut cmb.swap_chain_links;
@@ -253,11 +255,14 @@ pub fn command_encoder_begin_render_pass(
                         view.range.clone(),
                         TextureUsage::OUTPUT_ATTACHMENT,
                     );
-                    barriers.extend(pending.map(|pending| hal::memory::Barrier::Image {
-                        states: pending.to_states(),
-                        target: &texture.raw,
-                        families: None,
-                        range: pending.selector,
+                    barriers.extend(pending.map(|pending| {
+                        trace!("\tdepth-stencil {:?}", pending);
+                        hal::memory::Barrier::Image {
+                            states: pending.to_states(),
+                            target: &texture.raw,
+                            families: None,
+                            range: pending.selector,
+                        }
                     }));
                     hal::image::Layout::DepthStencilAttachmentOptimal
                 }
@@ -312,11 +317,14 @@ pub fn command_encoder_begin_render_pass(
                         view.range.clone(),
                         TextureUsage::OUTPUT_ATTACHMENT,
                     );
-                    barriers.extend(pending.map(|pending| hal::memory::Barrier::Image {
-                        states: pending.to_states(),
-                        target: &texture.raw,
-                        families: None,
-                        range: pending.selector,
+                    barriers.extend(pending.map(|pending| {
+                        trace!("\tcolor {:?}", pending);
+                        hal::memory::Barrier::Image {
+                            states: pending.to_states(),
+                            target: &texture.raw,
+                            families: None,
+                            range: pending.selector,
+                        }
                     }));
                     hal::image::Layout::ColorAttachmentOptimal
                 }
@@ -368,11 +376,14 @@ pub fn command_encoder_begin_render_pass(
                             view.range.clone(),
                             TextureUsage::OUTPUT_ATTACHMENT,
                         );
-                        barriers.extend(pending.map(|pending| hal::memory::Barrier::Image {
-                            states: pending.to_states(),
-                            target: &texture.raw,
-                            families: None,
-                            range: pending.selector,
+                        barriers.extend(pending.map(|pending| {
+                            trace!("\tresolve {:?}", pending);
+                            hal::memory::Barrier::Image {
+                                states: pending.to_states(),
+                                target: &texture.raw,
+                                families: None,
+                                range: pending.selector,
+                            }
                         }));
                         hal::image::Layout::ColorAttachmentOptimal
                     }

wgpu-native/src/command/render.rs

@@ -20,6 +20,7 @@ use crate::{
 };
 
 use hal::command::RawCommandBuffer;
+use log::trace;
 
 use std::{iter, ops::Range, slice};
 
@@ -189,6 +190,7 @@ pub extern "C" fn wgpu_render_pass_end_pass(pass_id: RenderPassId) {
 
     match cmb.raw.last_mut() {
         Some(ref mut last) => {
+            trace!("Encoding barriers before pass {:?}", pass_id);
             CommandBuffer::insert_barriers(
                 last,
                 &mut cmb.trackers,

wgpu-native/src/device.rs

@@ -1434,6 +1434,7 @@ pub extern "C" fn wgpu_queue_submit(
                         hal::command::CommandBufferInheritanceInfo::default(),
                     );
                 }
+                trace!("Stitching command buffer {:?} before submission", cmb_id);
                 command::CommandBuffer::insert_barriers(
                     &mut transit,
                     &mut *trackers,

wgpu-native/src/id.rs

@@ -1,5 +1,8 @@
 use crate::{Epoch, Index};
-use std::marker::PhantomData;
+use std::{
+    fmt,
+    marker::PhantomData,
+};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
@@ -9,7 +12,6 @@ use serde::{Deserialize, Serialize};
 pub struct Id(Index, Epoch);
 
 #[repr(transparent)]
-#[derive(Debug)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct GenericId<T>(Id, PhantomData<T>);
 
@@ -21,6 +23,12 @@ impl<T> Clone for GenericId<T> {
     }
 }
 
+impl<T> fmt::Debug for GenericId<T> {
+    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+        self.0.fmt(formatter)
+    }
+}
+
 impl<T> std::hash::Hash for GenericId<T> {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
         self.0.hash(state);

wgpu-native/src/track/buffer.rs

@@ -58,7 +58,10 @@ impl ResourceState for BufferState {
                     usage
                 }
                 None =>  {
-                    if !old.is_empty() && BufferUsage::WRITE_ALL.intersects(old | usage) {
+                    if !old.is_empty() &&
+                        old != usage &&
+                        BufferUsage::WRITE_ALL.intersects(old | usage)
+                    {
                         return Err(pending);
                     }
                     old | usage

wgpu-native/src/track/mod.rs

@@ -59,7 +59,7 @@ impl<U: Copy> Unit<U> {
 }
 
 /// Mode of stitching to states together.
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, PartialEq)]
 pub enum Stitch {
     /// Stitch to the init state of the other resource.
     Init,
@@ -209,12 +209,15 @@ impl<S: ResourceState> ResourceTracker<S> {
         default: S::Usage,
     ) -> bool {
         let mut state = S::default();
-        let _ = state.change(
+        match state.change(
             id,
             selector,
             default,
             None,
-        );
+        ) {
+            Ok(()) => (),
+            Err(_) => unreachable!(),
+        }
         self.map
             .insert(id.index(), Resource {
                 ref_count: ref_count.clone(),

wgpu-native/src/track/texture.rs

@@ -51,7 +51,10 @@ impl PendingTransition<TextureState> {
                 Ok(replace)
             }
             None => {
-                if !u.start.is_empty() && TextureUsage::WRITE_ALL.intersects(u.start | u.end) {
+                if !u.start.is_empty() &&
+                    u.start != u.end &&
+                    TextureUsage::WRITE_ALL.intersects(u.start | u.end)
+                {
                     Err(self)
                 } else {
                     Ok(u.start | u.end)
@@ -149,6 +152,8 @@ impl ResourceState for TextureState {
         stitch: Stitch,
         mut output: Option<&mut Vec<PendingTransition<Self>>>,
     ) -> Result<(), PendingTransition<Self>> {
+        assert!(output.is_some() || stitch == Stitch::Last);
+
         let mut temp = Vec::new();
         while self.mips.len() < other.mips.len() as usize {
             self.mips.push(MipState::default());
@@ -172,7 +177,7 @@ impl ResourceState for TextureState {
                     let unit = match states {
                         Range { start: None, end: None } => unreachable!(),
                         Range { start: Some(start), end: None } => start,
-                        Range { start: None, end: Some(end) } => Unit::new(end.select(stitch)),
+                        Range { start: None, end: Some(end) } => end,
                         Range { start: Some(start), end: Some(end) } => {
                             let mut final_usage = end.select(stitch);
                             if start.last != final_usage || !TextureUsage::ORDERED.contains(final_usage) {