Merge gfx-rs#226

226: Tracking Rewrite r=grovesNL a=kvark Fixes gfx-rs#44 The idea is to support independent tracking of sub-resources. Today, this is needed for textures, which can have individual layers and mipmap levels in different states at a time. Tomorrow, this will be needed for buffer sub-ranges. The intent to hack it in grew into a complete rewrite of the tracker... The new approach is cleaner in a few places (e.g. `TrackPermit` is gone), but the implementation is obviously more complex. I tried to separate the levels from each other (see `ResourceState` and `RangedStates`) to fight complexity, but it requires a whole lot of testing infrastructure to be solid. Also regresses gfx-rs#216 a bit, cc @arashikou : tracker is a relatively complex structure. I somehow doubt it's useful to look at it in debug spew. We may need to implement `Debug` manually for it before re-adding `Debug` derives on passes and command buffers. TODO: - [x] documentation of tracking types - [x] unit tests for tracking logic - [x] actual testing with existing apps, ensure no regressions - [x] write a mipmap generation example Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
adamnemecek · Jun 16, 2019 · a667d50 · a667d50
2 parents 6b655f5 + 3753309
commit a667d50
Show file tree

Hide file tree

Showing 20 changed files with 1,615 additions and 608 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,6 +1,7 @@
 /target
 **/*.rs.bk
 #Cargo.lock
+.DS_Store
 .vscode
 .vs
 build

diff --git a/.travis.yml b/.travis.yml
@@ -57,6 +57,7 @@ before_install:
 
 script:
   - cargo test
+  - if [[ $TRAVIS_OS_NAME == "linux" ]]; then cargo check --release; fi
   - if [[ $TRAVIS_RUST_VERSION == "nightly" ]]; then cargo +nightly install cbindgen; fi
   - if [[ $TRAVIS_RUST_VERSION == "nightly" ]] && [[ $TRAVIS_OS_NAME == "windows" ]]; then
       wget -nc -O glfw.zip https://github.com/glfw/glfw/archive/3.3.zip &&

diff --git a/ffi/wgpu-remote.h b/ffi/wgpu-remote.h
@@ -19,8 +19,6 @@ typedef struct WGPUClientFactory WGPUClientFactory;
 
 typedef struct WGPUServer WGPUServer;
 
-typedef struct WGPUTrackPermit WGPUTrackPermit;
-
 typedef uint32_t WGPUIndex;
 
 typedef uint32_t WGPUEpoch;
@@ -57,10 +55,6 @@ typedef struct {
   const uint8_t *error;
 } WGPUInfrastructure;
 
-
-
-
-
 WGPUDeviceId wgpu_client_adapter_create_device(const WGPUClient *client,
                                                WGPUAdapterId adapter_id,
                                                const WGPUDeviceDescriptor *desc);

diff --git a/ffi/wgpu.h b/ffi/wgpu.h
@@ -11,6 +11,8 @@
 
 #define WGPUMAX_COLOR_TARGETS 4
 
+#define WGPUMAX_MIP_LEVELS 16
+
 #define WGPUMAX_VERTEX_BUFFERS 8
 
 typedef enum {
@@ -230,8 +232,6 @@ typedef enum {
   WGPUVertexFormat_Int4 = 48,
 } WGPUVertexFormat;
 
-typedef struct WGPUTrackPermit WGPUTrackPermit;
-
 typedef uint32_t WGPUIndex;
 
 typedef uint32_t WGPUEpoch;
@@ -615,10 +615,6 @@ typedef struct {
   uint32_t array_count;
 } WGPUTextureViewDescriptor;
 
-
-
-
-
 #if defined(WGPU_LOCAL)
 WGPUDeviceId wgpu_adapter_request_device(WGPUAdapterId adapter_id,
                                          const WGPUDeviceDescriptor *desc);

diff --git a/wgpu-native/src/binding_model.rs b/wgpu-native/src/binding_model.rs
@@ -105,7 +105,6 @@ pub struct BindGroupDescriptor {
     pub bindings_length: usize,
 }
 
-#[derive(Debug)]
 pub struct BindGroup<B: hal::Backend> {
     pub(crate) raw: DescriptorSet<B>,
     pub(crate) device_id: Stored<DeviceId>,

diff --git a/wgpu-native/src/command/allocator.rs b/wgpu-native/src/command/allocator.rs
@@ -24,7 +24,6 @@ impl<B: hal::Backend> CommandPool<B> {
     }
 }
 
-#[derive(Debug)]
 struct Inner<B: hal::Backend> {
     pools: HashMap<thread::ThreadId, CommandPool<B>>,
     pending: Vec<CommandBuffer<B>>,
@@ -42,7 +41,6 @@ impl<B: hal::Backend> Inner<B> {
     }
 }
 
-#[derive(Debug)]
 pub struct CommandAllocator<B: hal::Backend> {
     queue_family: hal::queue::QueueFamilyId,
     inner: Mutex<Inner<B>>,

diff --git a/wgpu-native/src/command/compute.rs b/wgpu-native/src/command/compute.rs
@@ -17,7 +17,6 @@ use hal::{self, command::RawCommandBuffer};
 
 use std::{iter, slice};
 
-#[derive(Debug)]
 pub struct ComputePass<B: hal::Backend> {
     raw: B::CommandBuffer,
     cmb_id: Stored<CommandBufferId>,

diff --git a/wgpu-native/src/command/mod.rs b/wgpu-native/src/command/mod.rs
@@ -21,7 +21,7 @@ use crate::{
     hub::{Storage, HUB},
     resource::TexturePlacement,
     swap_chain::{SwapChainLink, SwapImageEpoch},
-    track::{DummyUsage, Stitch, TrackerSet},
+    track::{Stitch, TrackerSet},
     BufferHandle,
     BufferId,
     Color,
@@ -88,7 +88,6 @@ pub struct RenderPassDescriptor {
     pub depth_stencil_attachment: *const RenderPassDepthStencilAttachmentDescriptor<TextureViewId>,
 }
 
-#[derive(Debug)]
 pub struct CommandBuffer<B: hal::Backend> {
     pub(crate) raw: Vec<B::CommandBuffer>,
     is_recording: bool,
@@ -110,34 +109,29 @@ impl CommandBufferHandle {
     ) {
         let buffer_barriers =
             base.buffers
-                .consume_by_replace(&head.buffers, stitch)
-                .map(|(id, transit)| {
-                    let b = &buffer_guard[id];
-                    trace!("transit buffer {:?} {:?}", id, transit);
+                .merge_replace(&head.buffers, stitch)
+                .map(|pending| {
+                    trace!("transit buffer {:?}", pending);
                     hal::memory::Barrier::Buffer {
-                        states: conv::map_buffer_state(transit.start)
-                            .. conv::map_buffer_state(transit.end),
-                        target: &b.raw,
+                        states: pending.to_states(),
+                        target: &buffer_guard[pending.id].raw,
                         range: None .. None,
                         families: None,
                     }
                 });
         let texture_barriers = base
             .textures
-            .consume_by_replace(&head.textures, stitch)
-            .map(|(id, transit)| {
-                let t = &texture_guard[id];
-                trace!("transit texture {:?} {:?}", id, transit);
-                let aspects = t.full_range.aspects;
+            .merge_replace(&head.textures, stitch)
+            .map(|pending| {
+                trace!("transit texture {:?}", pending);
                 hal::memory::Barrier::Image {
-                    states: conv::map_texture_state(transit.start, aspects)
-                        .. conv::map_texture_state(transit.end, aspects),
-                    target: &t.raw,
-                    range: t.full_range.clone(), //TODO?
+                    states: pending.to_states(),
+                    target: &texture_guard[pending.id].raw,
+                    range: pending.selector,
                     families: None,
                 }
             });
-        base.views.consume_by_extend(&head.views).unwrap();
+        base.views.merge_extend(&head.views).unwrap();
 
         let stages = all_buffer_stages() | all_image_stages();
         unsafe {
@@ -175,6 +169,7 @@ pub fn command_encoder_begin_render_pass(
     let mut cmb_guard = HUB.command_buffers.write();
     let cmb = &mut cmb_guard[command_encoder_id];
     let device = &device_guard[cmb.device_id.value];
+    let texture_guard = HUB.textures.read();
     let view_guard = HUB.texture_views.read();
 
     let mut current_comb = device.com_allocator.extend(cmb);
@@ -185,6 +180,7 @@ pub fn command_encoder_begin_render_pass(
         );
     }
     let mut extent = None;
+    let mut barriers = Vec::new();
 
     let color_attachments =
         unsafe { slice::from_raw_parts(desc.color_attachments, desc.color_attachments_length) };
@@ -195,38 +191,63 @@ pub fn command_encoder_begin_render_pass(
         let swap_chain_links = &mut cmb.swap_chain_links;
 
         let depth_stencil = depth_stencil_attachment.map(|at| {
-            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 {
                 extent = Some(view.extent);
             }
-            trackers
-                .views
-                .query(at.attachment, &view.life_guard.ref_count, DummyUsage);
-            let query = trackers.textures.query(
+            let old_layout = match trackers.textures.query(
                 view.texture_id.value,
-                &view.texture_id.ref_count,
-                TextureUsage::empty(),
-            );
-            let (_, layout) = conv::map_texture_state(
-                query.usage,
-                hal::format::Aspects::DEPTH | hal::format::Aspects::STENCIL,
-            );
+                view.range.clone(),
+            ) {
+                Some(usage) => {
+                    conv::map_texture_state(
+                        usage,
+                        hal::format::Aspects::DEPTH | hal::format::Aspects::STENCIL,
+                    ).1
+                }
+                None => {
+                    // Required sub-resources have inconsistent states, we need to
+                    // issue individual barriers instead of relying on the render pass.
+                    let (texture, pending) = trackers.textures.use_replace(
+                        &*texture_guard,
+                        view.texture_id.value,
+                        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,
+                    }));
+                    hal::image::Layout::DepthStencilAttachmentOptimal
+                }
+            };
             hal::pass::Attachment {
                 format: Some(conv::map_texture_format(view.format)),
                 samples: view.samples,
                 ops: conv::map_load_store_ops(at.depth_load_op, at.depth_store_op),
                 stencil_ops: conv::map_load_store_ops(at.stencil_load_op, at.stencil_store_op),
-                layouts: layout .. layout,
+                layouts: old_layout .. hal::image::Layout::DepthStencilAttachmentOptimal,
             }
         });
 
         let color_keys = color_attachments.iter().map(|at| {
-            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 {
+                extent = Some(view.extent);
+            }
 
             if view.is_owned_by_swap_chain {
-                let link = match HUB.textures.read()[view.texture_id.value].placement {
+                let link = match texture_guard[view.texture_id.value].placement {
                     TexturePlacement::SwapChain(ref link) => SwapChainLink {
                         swap_chain_id: link.swap_chain_id.clone(),
                         epoch: *link.epoch.lock(),
@@ -237,26 +258,37 @@ pub fn command_encoder_begin_render_pass(
                 swap_chain_links.push(link);
             }
 
-            if let Some(ex) = extent {
-                assert_eq!(ex, view.extent);
-            } else {
-                extent = Some(view.extent);
-            }
-            trackers
-                .views
-                .query(at.attachment, &view.life_guard.ref_count, DummyUsage);
-            let query = trackers.textures.query(
+            let old_layout = match trackers.textures.query(
                 view.texture_id.value,
-                &view.texture_id.ref_count,
-                TextureUsage::empty(),
-            );
-            let (_, layout) = conv::map_texture_state(query.usage, hal::format::Aspects::COLOR);
+                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 (texture, pending) = trackers.textures.use_replace(
+                        &*texture_guard,
+                        view.texture_id.value,
+                        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,
+                    }));
+                    hal::image::Layout::ColorAttachmentOptimal
+                }
+            };
             hal::pass::Attachment {
                 format: Some(conv::map_texture_format(view.format)),
                 samples: view.samples,
                 ops: conv::map_load_store_ops(at.load_op, at.store_op),
                 stencil_ops: hal::pass::AttachmentOps::DONT_CARE,
-                layouts: layout .. layout,
+                layouts: old_layout .. hal::image::Layout::ColorAttachmentOptimal,
             }
         });
 
@@ -266,6 +298,16 @@ pub fn command_encoder_begin_render_pass(
         }
     };
 
+    if !barriers.is_empty() {
+        unsafe {
+            current_comb.pipeline_barrier(
+                all_image_stages() .. all_image_stages(),
+                hal::memory::Dependencies::empty(),
+                barriers,
+            );
+        }
+    }
+
     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(),

diff --git a/wgpu-native/src/command/render.rs b/wgpu-native/src/command/render.rs
@@ -109,7 +109,6 @@ impl VertexState {
     }
 }
 
-#[derive(Debug)]
 pub struct RenderPass<B: hal::Backend> {
     raw: B::CommandBuffer,
     cmb_id: Stored<CommandBufferId>,
@@ -177,6 +176,7 @@ pub extern "C" fn wgpu_render_pass_end_pass(pass_id: RenderPassId) -> CommandBuf
     unsafe {
         pass.raw.end_render_pass();
     }
+    pass.trackers.optimize();
     let cmb = &mut cmb_guard[pass.cmb_id.value];
 
     match cmb.raw.last_mut() {
@@ -192,7 +192,7 @@ pub extern "C" fn wgpu_render_pass_end_pass(pass_id: RenderPassId) -> CommandBuf
             unsafe { last.finish() };
         }
         None => {
-            cmb.trackers.consume_by_extend(&pass.trackers);
+            cmb.trackers.merge_extend(&pass.trackers);
         }
     }
 
@@ -232,7 +232,7 @@ pub extern "C" fn wgpu_render_pass_set_bind_group(
         }
     }
 
-    pass.trackers.consume_by_extend(&bind_group.used);
+    pass.trackers.merge_extend(&bind_group.used);
 
     if let Some((pipeline_layout_id, follow_up)) =
         pass.binder
@@ -282,7 +282,7 @@ pub extern "C" fn wgpu_render_pass_set_index_buffer(
     let buffer = pass
         .trackers
         .buffers
-        .get_with_extended_usage(&*buffer_guard, buffer_id, BufferUsage::INDEX)
+        .use_extend(&*buffer_guard, buffer_id, (), BufferUsage::INDEX)
         .unwrap();
 
     let range = offset .. buffer.size;
@@ -316,7 +316,7 @@ pub extern "C" fn wgpu_render_pass_set_vertex_buffers(
     for (vbs, (&id, &offset)) in pass.vertex_state.inputs.iter_mut().zip(buffers.iter().zip(offsets)) {
         let buffer = pass.trackers
             .buffers
-            .get_with_extended_usage(&*buffer_guard, id, BufferUsage::VERTEX)
+            .use_extend(&*buffer_guard, id, (), BufferUsage::VERTEX)
             .unwrap();
         vbs.total_size = buffer.size - offset;
     }
@@ -450,7 +450,7 @@ pub extern "C" fn wgpu_render_pass_set_pipeline(
             let buffer = pass
                 .trackers
                 .buffers
-                .get_with_extended_usage(&*buffer_guard, buffer_id, BufferUsage::INDEX)
+                .use_extend(&*buffer_guard, buffer_id, (), BufferUsage::INDEX)
                 .unwrap();
 
             let view = hal::buffer::IndexBufferView {