trace: fix reusing object ids

.github/workflows/ci.yml

@@ -28,36 +28,43 @@ jobs:
             channel: stable
             build_command: rustup target add aarch64-apple-ios; cargo clippy --target aarch64-apple-ios
             additional_core_features:
+            additional_player_features:
           - os: macos-10.15
             name: MacOS Stable
             channel: stable
             build_command: cargo clippy
             additional_core_features: trace
+            additional_player_features: winit
           - os: macos-10.15
             name: MacOS Nightly
             channel: nightly
             build_command: cargo test
             additional_core_features:
+            additional_player_features:
           - os: ubuntu-18.04
             name: Ubuntu Stable
             channel: stable
             build_command: cargo clippy
             additional_core_features: trace,replay
+            additional_player_features:
           - os: ubuntu-18.04
             name: Ubuntu Nightly
             channel: nightly
             build_command: cargo test
             additional_core_features:
+            additional_player_features: winit
           - os: windows-2019
             name: Windows Stable
             channel: stable
             build_command: rustup default stable-msvc; cargo clippy
-            additional_core_features: replay
+            additional_core_features: trace
+            additional_player_features:
           - os: windows-2019
             name: Windows Nightly
             channel: nightly
             build_command: rustup default nightly-msvc; cargo test
             additional_core_features:
+            additional_player_features:
     steps:
     - uses: actions/checkout@v2
     - if: matrix.channel == 'nightly'
@@ -72,3 +79,5 @@ jobs:
       run: ${{ matrix.build_command }}
     - if: matrix.additional_core_features != ''
       run: cargo check --manifest-path wgpu-core/Cargo.toml --features ${{ matrix.additional_core_features }}
+    - if: matrix.additional_player_features != ''
+      run: cargo check --manifest-path player/Cargo.toml --features ${{ matrix.additional_player_features }}

player/src/main.rs

@@ -2,6 +2,14 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+/*! This is a player for WebGPU traces.
+ *
+ * # Notes
+ * - we call device_maintain_ids() before creating any refcounted resource,
+ *   which is basically everything except for BGL and shader modules,
+ *   so that we don't accidentally try to use the same ID.
+!*/
+
 use wgc::device::trace;
 
 use std::{
@@ -96,6 +104,13 @@ trait GlobalExt {
         encoder: wgc::id::CommandEncoderId,
         commands: Vec<trace::Command>,
     ) -> wgc::id::CommandBufferId;
+    /*fn process<B: wgc::hub::GfxBackend>(
+        &self,
+        device: wgc::id::DeviceId,
+        action: trace::Action,
+    ) {
+
+    }*/
 }
 
 impl GlobalExt for wgc::hub::Global<IdentityPassThroughFactory> {
@@ -186,6 +201,7 @@ fn main() {
     env_logger::init();
 
     //TODO: setting for the backend bits
+    //TODO: setting for the target frame, or controls
 
     let dir = match std::env::args().nth(1) {
         Some(arg) if Path::new(&arg).is_dir() => PathBuf::from(arg),
@@ -194,7 +210,8 @@ fn main() {
 
     log::info!("Loading trace '{:?}'", dir);
     let file = File::open(dir.join(trace::FILE_NAME)).unwrap();
-    let actions: Vec<trace::Action> = ron::de::from_reader(file).unwrap();
+    let mut actions: Vec<trace::Action> = ron::de::from_reader(file).unwrap();
+    actions.reverse(); // allows us to pop from the top
     log::info!("Found {} actions", actions.len());
 
     #[cfg(feature = "winit")]
@@ -239,27 +256,32 @@ fn main() {
         )
         .unwrap();
 
-    let mut device = wgc::id::DeviceId::default();
+    log::info!("Initializing the device");
+    let device = match actions.pop() {
+        Some(trace::Action::Init { limits }) => {
+            gfx_select!(adapter => global.adapter_request_device(
+                adapter,
+                &wgt::DeviceDescriptor {
+                    extensions: wgt::Extensions {
+                        anisotropic_filtering: false,
+                    },
+                    limits,
+                },
+                wgc::id::TypedId::zip(1, 0, wgt::Backend::Empty)
+            ))
+        }
+        _ => panic!("Expected Action::Init"),
+    };
+    let mut frame_count = 0;
 
     log::info!("Executing actions");
     for action in actions {
         use wgc::device::trace::Action as A;
         match action {
-            A::Init { limits } => {
+            A::Init { .. } => panic!("Unexpected Action::Init"),
-                log::info!("Initializing the device");
-                device = gfx_select!(adapter => global.adapter_request_device(
-                    adapter,
-                    &wgt::DeviceDescriptor {
-                        extensions: wgt::Extensions {
-                            anisotropic_filtering: false,
-                        },
-                        limits,
-                    },
-                    wgc::id::TypedId::zip(1, 0, wgt::Backend::Empty)
-                ));
-            }
             A::CreateBuffer { id, desc } => {
                 let label = Label::new(&desc.label);
+                gfx_select!(device => global.device_maintain_ids(device));
                 gfx_select!(device => global.device_create_buffer(device, &desc.map_label(|_| label.as_ptr()), id));
             }
             A::DestroyBuffer(id) => {
@@ -267,6 +289,7 @@ fn main() {
             }
             A::CreateTexture { id, desc } => {
                 let label = Label::new(&desc.label);
+                gfx_select!(device => global.device_maintain_ids(device));
                 gfx_select!(device => global.device_create_texture(device, &desc.map_label(|_| label.as_ptr()), id));
             }
             A::DestroyTexture(id) => {
@@ -278,6 +301,7 @@ fn main() {
                 desc,
             } => {
                 let label = desc.as_ref().map_or(Label(None), |d| Label::new(&d.label));
+                gfx_select!(device => global.device_maintain_ids(device));
                 gfx_select!(device => global.texture_create_view(parent_id, desc.map(|d| d.map_label(|_| label.as_ptr())).as_ref(), id));
             }
             A::DestroyTextureView(id) => {
@@ -285,6 +309,7 @@ fn main() {
             }
             A::CreateSampler { id, desc } => {
                 let label = Label::new(&desc.label);
+                gfx_select!(device => global.device_maintain_ids(device));
                 gfx_select!(device => global.device_create_sampler(device, &desc.map_label(|_| label.as_ptr()), id));
             }
             A::DestroySampler(id) => {
@@ -308,6 +333,8 @@ fn main() {
                 gfx_select!(device => global.swap_chain_get_next_texture(parent_id, id)).unwrap();
             }
             A::PresentSwapChain(id) => {
+                frame_count += 1;
+                log::debug!("Presenting frame {}", frame_count);
                 gfx_select!(device => global.swap_chain_present(id));
             }
             A::CreateBindGroupLayout { id, label, entries } => {
@@ -328,6 +355,7 @@ fn main() {
                 id,
                 bind_group_layouts,
             } => {
+                gfx_select!(device => global.device_maintain_ids(device));
                 gfx_select!(device => global.device_create_pipeline_layout(
                     device,
                     &wgc::binding_model::PipelineLayoutDescriptor {
@@ -366,6 +394,7 @@ fn main() {
                         },
                     })
                     .collect::<Vec<_>>();
+                gfx_select!(device => global.device_maintain_ids(device));
                 gfx_select!(device => global.device_create_bind_group(
                     device,
                     &wgc::binding_model::BindGroupDescriptor {
@@ -398,6 +427,7 @@ fn main() {
             }
             A::CreateComputePipeline { id, desc } => {
                 let cs_stage = OwnedProgrammableStage::from(desc.compute_stage);
+                gfx_select!(device => global.device_maintain_ids(device));
                 gfx_select!(device => global.device_create_compute_pipeline(
                     device,
                     &wgc::pipeline::ComputePipelineDescriptor {
@@ -423,6 +453,7 @@ fn main() {
                         attributes_length: vb.attributes.len(),
                     })
                     .collect::<Vec<_>>();
+                gfx_select!(device => global.device_maintain_ids(device));
                 gfx_select!(device => global.device_create_render_pipeline(
                     device,
                     &wgc::pipeline::RenderPipelineDescriptor {
@@ -451,6 +482,7 @@ fn main() {
             A::WriteBuffer { id, data, range } => {
                 let bin = std::fs::read(dir.join(data)).unwrap();
                 let size = (range.end - range.start) as usize;
+                gfx_select!(device => global.device_wait_for_buffer(device, id));
                 gfx_select!(device => global.device_set_buffer_sub_data(device, id, range.start, &bin[..size]));
             }
             A::Submit(commands) => {

wgpu-core/src/device/life.rs

@@ -2,14 +2,14 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#[cfg(feature = "trace")]
+use crate::device::trace;
 use crate::{
     hub::{GfxBackend, Global, GlobalIdentityHandlerFactory, Token},
     id, resource,
     track::TrackerSet,
     FastHashMap, RefCount, Stored, SubmissionIndex,
 };
-#[cfg(feature = "trace")]
-use crate::device::trace;
 
 use copyless::VecHelper as _;
 use gfx_descriptor::{DescriptorAllocator, DescriptorSet};
@@ -290,8 +290,7 @@ impl<B: GfxBackend> LifetimeTracker<B> {
         &mut self,
         global: &Global<G>,
         trackers: &Mutex<TrackerSet>,
-        #[cfg(feature = "trace")]
+        #[cfg(feature = "trace")] trace: Option<&Mutex<trace::Trace>>,
-        trace: Option<&Mutex<trace::Trace>>,
         token: &mut Token<super::Device<B>>,
     ) {
         let hub = B::hub(global);

wgpu-core/src/device/mod.rs

@@ -604,6 +604,35 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         (id, pointer)
     }
 
+    #[cfg(feature = "replay")]
+    pub fn device_wait_for_buffer<B: GfxBackend>(
+        &self,
+        device_id: id::DeviceId,
+        buffer_id: id::BufferId,
+    ) {
+        let hub = B::hub(self);
+        let mut token = Token::root();
+        let (device_guard, mut token) = hub.devices.read(&mut token);
+        let last_submission = {
+            let (buffer_guard, _) = hub.buffers.write(&mut token);
+            buffer_guard[buffer_id]
+                .life_guard
+                .submission_index
+                .load(Ordering::Acquire)
+        };
+
+        let device = &device_guard[device_id];
+        let mut life_lock = device.lock_life(&mut token);
+        if life_lock.lowest_active_submission() <= last_submission {
+            log::info!(
+                "Waiting for submission {:?} before accessing buffer {:?}",
+                last_submission,
+                buffer_id
+            );
+            life_lock.triage_submissions(&device.raw, true);
+        }
+    }
+
     pub fn device_set_buffer_sub_data<B: GfxBackend>(
         &self,
         device_id: id::DeviceId,
@@ -2031,7 +2060,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             life_guard: LifeGuard::new(),
         };
 
-        let id = hub.render_pipelines
+        let id = hub
+            .render_pipelines
             .register_identity(id_in, pipeline, &mut token);
 
         #[cfg(feature = "trace")]
@@ -2039,21 +2069,29 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             Some(ref trace) => trace.lock().add(trace::Action::CreateRenderPipeline {
                 id,
                 desc: trace::RenderPipelineDescriptor {
-                    layout: desc.layout, 
+                    layout: desc.layout,
                     vertex_stage: trace::ProgrammableStageDescriptor::new(&desc.vertex_stage),
-                    fragment_stage: unsafe { desc.fragment_stage.as_ref() }.map(trace::ProgrammableStageDescriptor::new),
+                    fragment_stage: unsafe { desc.fragment_stage.as_ref() }
+                        .map(trace::ProgrammableStageDescriptor::new),
                     primitive_topology: desc.primitive_topology,
                     rasterization_state: unsafe { desc.rasterization_state.as_ref() }.cloned(),
                     color_states: color_states.to_vec(),
                     depth_stencil_state: depth_stencil_state.cloned(),
                     vertex_state: trace::VertexStateDescriptor {
                         index_format: desc.vertex_state.index_format,
-                        vertex_buffers: desc_vbs.iter().map(|vbl| trace::VertexBufferLayoutDescriptor {
+                        vertex_buffers: desc_vbs
-                            array_stride: vbl.array_stride,
+                            .iter()
-                            step_mode: vbl.step_mode,
+                            .map(|vbl| trace::VertexBufferLayoutDescriptor {
-                            attributes: unsafe { slice::from_raw_parts(vbl.attributes, vbl.attributes_length) }
+                                array_stride: vbl.array_stride,
-                                .iter().cloned().collect(),
+                                step_mode: vbl.step_mode,
-                        }).collect(),
+                                attributes: unsafe {
+                                    slice::from_raw_parts(vbl.attributes, vbl.attributes_length)
+                                }
+                                .iter()
+                                .cloned()
+                                .collect(),
+                            })
+                            .collect(),
                     },
                     sample_count: desc.sample_count,
                     sample_mask: desc.sample_mask,
@@ -2147,7 +2185,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             },
             life_guard: LifeGuard::new(),
         };
-        let id = hub.compute_pipelines
+        let id = hub
+            .compute_pipelines
             .register_identity(id_in, pipeline, &mut token);
 
         #[cfg(feature = "trace")]
@@ -2155,7 +2194,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             Some(ref trace) => trace.lock().add(trace::Action::CreateComputePipeline {
                 id,
                 desc: trace::ComputePipelineDescriptor {
-                    layout: desc.layout, 
+                    layout: desc.layout,
                     compute_stage: trace::ProgrammableStageDescriptor::new(&desc.compute_stage),
                 },
             }),
@@ -2295,6 +2334,23 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         sc_id
     }
 
+    #[cfg(feature = "replay")]
+    /// Only triange suspected resource IDs. This helps us to avoid ID collisions
+    /// upon creating new resources when re-plaing a trace.
+    pub fn device_maintain_ids<B: GfxBackend>(&self, device_id: id::DeviceId) {
+        let hub = B::hub(self);
+        let mut token = Token::root();
+        let (device_guard, mut token) = hub.devices.read(&mut token);
+        let device = &device_guard[device_id];
+        device.lock_life(&mut token).triage_suspected(
+            self,
+            &device.trackers,
+            #[cfg(feature = "trace")]
+            None,
+            &mut token,
+        );
+    }
+
     pub fn device_poll<B: GfxBackend>(&self, device_id: id::DeviceId, force_wait: bool) {
         let hub = B::hub(self);
         let mut token = Token::root();

wgpu-core/src/device/trace.rs

@@ -6,9 +6,9 @@ use crate::{
     command::{BufferCopyView, TextureCopyView},
     id,
 };
-use std::ops::Range;
 #[cfg(feature = "trace")]
 use std::io::Write as _;
+use std::ops::Range;
 
 //TODO: consider a readable Id that doesn't include the backend
 

wgpu-core/src/hub.rs

@@ -46,7 +46,7 @@ impl Default for IdentityManager {
 impl IdentityManager {
     pub fn from_index(min_index: u32) -> Self {
         IdentityManager {
-            free: (0 .. min_index).collect(),
+            free: (0..min_index).collect(),
             epochs: vec![1; min_index as usize],
         }
     }

wgpu-core/src/instance.rs

@@ -282,8 +282,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         };
 
         let mut token = Token::root();
-        self.surfaces
+        self.surfaces.register_identity(id_in, surface, &mut token)
-            .register_identity(id_in, surface, &mut token)
     }
 
     pub fn enumerate_adapters(&self, inputs: AdapterInputs<Input<G, AdapterId>>) -> Vec<AdapterId> {