feat(tracing): Trace failed create_*_pipeline calls (gfx-rs#9209)

Author: andyleiserson

player/src/lib.rs

@@ -14,7 +14,7 @@ use wgc::{
     binding_model::BindingResource,
     command::{ArcCommand, ArcReferences, BasePass, Command, PointerReferences},
     device::trace::{self, DataKind, DataLoader},
-    id::PointerId,
+    id::{Marker, PointerId},
 };
 
 pub struct Player {
@@ -88,6 +88,28 @@ impl Default for Player {
     }
 }
 
+fn process_result<T: Marker, U>(
+    op: &str,
+    map: &mut HashMap<PointerId<T>, U>,
+    id: Option<PointerId<T>>,
+    value: Result<U, impl std::error::Error>,
+) {
+    match (id, value) {
+        (Some(id), Ok(value)) => {
+            map.insert(id, value);
+        }
+        (Some(_), Err(err)) => {
+            panic!("{op} succeeded when recording, but failed on playback: {err}");
+        }
+        (None, Ok(_)) => {
+            panic!("{op} failed when recording, but succeeded on playback");
+        }
+        (None, Err(err)) => {
+            panic!("{op} failed when recording, and failed on playback: {err}");
+        }
+    }
+}
+
 impl Player {
     pub fn process(
         &mut self,
@@ -323,10 +345,13 @@ impl Player {
             }
             Action::CreateComputePipeline { id, desc } => {
                 let resolved_desc = self.resolve_compute_pipeline_descriptor(desc);
-                let pipeline = device
-                    .create_compute_pipeline(resolved_desc)
-                    .expect("create_compute_pipeline error");
-                self.compute_pipelines.insert(id, pipeline);
+                let pipeline = device.create_compute_pipeline(resolved_desc);
+                process_result(
+                    "create_compute_pipeline",
+                    &mut self.compute_pipelines,
+                    id,
+                    pipeline,
+                );
             }
             Action::DestroyComputePipeline(id) => {
                 self.compute_pipelines
@@ -338,10 +363,13 @@ impl Player {
                 // pipeline descriptor that can represent either a conventional
                 // pipeline or a mesh shading pipeline.
                 let resolved_desc = self.resolve_render_pipeline_descriptor(desc);
-                let pipeline = device
-                    .create_render_pipeline(resolved_desc)
-                    .expect("create_render_pipeline error");
-                self.render_pipelines.insert(id, pipeline);
+                let pipeline = device.create_render_pipeline(resolved_desc);
+                process_result(
+                    "create_render_pipeline",
+                    &mut self.render_pipelines,
+                    id,
+                    pipeline,
+                );
             }
             Action::DestroyRenderPipeline(id) => {
                 self.render_pipelines

player/tests/player/data/bind-group.ron

@@ -50,7 +50,7 @@
             data: File("empty.wgsl"),
         ),
         CreateComputePipeline(
-            id: PointerId(0x10),
+            id: Some(PointerId(0x10)),
             desc: (
                 label: None,
                 layout: Some(PointerId(0x10)),

player/tests/player/data/pipeline-statistics-query.ron

@@ -23,7 +23,7 @@
             data: File("empty.wgsl"),
         ),
         CreateComputePipeline(
-            id: PointerId(0x10),
+            id: Some(PointerId(0x10)),
             desc: (
                 label: None,
                 layout: Some(PointerId(0x10)),

player/tests/player/data/quad.ron

@@ -50,7 +50,7 @@
             immediate_size: 0,
         )),
         CreateGeneralRenderPipeline(
-            id: PointerId(0x10),
+            id: Some(PointerId(0x10)),
             desc: (
                 label: None,
                 layout: Some(PointerId(0x10)),

player/tests/player/data/zero-init-buffer.ron

@@ -125,7 +125,7 @@
             immediate_size: 0,
         )),
         CreateComputePipeline(
-            id: PointerId(0x10),
+            id: Some(PointerId(0x10)),
             desc: (
                 label: None,
                 layout: Some(PointerId(0x10)),

player/tests/player/data/zero-init-texture-binding.ron

@@ -128,7 +128,7 @@
             data: File("zero-init-texture-binding.wgsl"),
         ),
         CreateComputePipeline(
-            id: PointerId(0x10),
+            id: Some(PointerId(0x10)),
             desc: (
                 label: None,
                 layout: Some(PointerId(0x10)),

wgpu-core/src/device/global.rs

@@ -1538,19 +1538,21 @@ impl Global {
             #[cfg(feature = "trace")]
             let trace_desc = desc.clone().into_trace();
 
-            let pipeline = match device.create_render_pipeline(desc) {
-                Ok(pair) => pair,
-                Err(e) => break 'error e,
-            };
+            let res = device.create_render_pipeline(desc);
 
             #[cfg(feature = "trace")]
             if let Some(ref mut trace) = *device.trace.lock() {
                 trace.add(trace::Action::CreateGeneralRenderPipeline {
-                    id: pipeline.to_trace(),
+                    id: res.as_ref().ok().map(IntoTrace::to_trace),
                     desc: trace_desc,
                 });
             }
 
+            let pipeline = match res {
+                Ok(pair) => pair,
+                Err(e) => break 'error e,
+            };
+
             let id = fid.assign(Fallible::Valid(pipeline));
             api_log!("Device::create_render_pipeline -> {id:?}");
 
@@ -1687,19 +1689,21 @@ impl Global {
             #[cfg(feature = "trace")]
             let trace_desc = desc.clone().into_trace();
 
-            let pipeline = match device.create_compute_pipeline(desc) {
-                Ok(pair) => pair,
-                Err(e) => break 'error e,
-            };
+            let res = device.create_compute_pipeline(desc);
 
             #[cfg(feature = "trace")]
             if let Some(ref mut trace) = *device.trace.lock() {
                 trace.add(trace::Action::CreateComputePipeline {
-                    id: pipeline.to_trace(),
+                    id: res.as_ref().ok().map(IntoTrace::to_trace),
                     desc: trace_desc,
                 });
             }
 
+            let pipeline = match res {
+                Ok(pair) => pair,
+                Err(e) => break 'error e,
+            };
+
             let id = fid.assign(Fallible::Valid(pipeline));
             api_log!("Device::create_compute_pipeline -> {id:?}");
 

wgpu-core/src/device/trace.rs

@@ -193,12 +193,12 @@ pub enum Action<'a, R: ReferenceType> {
     },
     DestroyShaderModule(PointerId<markers::ShaderModule>),
     CreateComputePipeline {
-        id: PointerId<markers::ComputePipeline>,
+        id: Option<PointerId<markers::ComputePipeline>>,
         desc: TraceComputePipelineDescriptor<'a>,
     },
     DestroyComputePipeline(PointerId<markers::ComputePipeline>),
     CreateGeneralRenderPipeline {
-        id: PointerId<markers::RenderPipeline>,
+        id: Option<PointerId<markers::RenderPipeline>>,
         desc: TraceGeneralRenderPipelineDescriptor<'a>,
     },
     DestroyRenderPipeline(PointerId<markers::RenderPipeline>),

wgpu-core/src/id.rs

@@ -134,7 +134,7 @@ impl TryFrom<SerialId> for RawId {
 #[derive(Debug, serde::Serialize, serde::Deserialize)]
 pub enum PointerId<T: Marker> {
     // The only variant forces RON to not ignore "Id"
-    PointerId(usize, #[serde(skip)] PhantomData<T>),
+    PointerId(core::num::NonZeroUsize, #[serde(skip)] PhantomData<T>),
 }
 
 #[cfg(feature = "serde")]
@@ -177,7 +177,10 @@ impl<T: crate::storage::StorageItem> From<&alloc::sync::Arc<T>> for PointerId<T:
         // data, not to the `ArcInner`. The `ArcInner` stores the reference
         // counts before the data, so the machine code for this conversion has
         // to add an offset to the pointer.
-        PointerId::PointerId(alloc::sync::Arc::as_ptr(arc) as usize, PhantomData)
+        PointerId::PointerId(
+            core::num::NonZeroUsize::new(alloc::sync::Arc::as_ptr(arc) as usize).unwrap(),
+            PhantomData,
+        )
     }
 }