Ensure that pipeline creation errors register layouts as errors also (#…

…4624) Since the pipeline id is provided by the caller, the caller may presume that an implicit pipeline layout id is also created, even in error conditions such as with an invalid device. Since our registry system will panic if asked to retrieve a pipeline layout id that has never been registered, it's dangerous to leave that id unregistered. This ensures that the layout ids also get error values when the pipeline creation returns an error.
gfx-rs · Nov 6, 2023 · 7aaf672 · 7aaf672
1 parent 1d4fa81
commit 7aaf672
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -73,7 +73,7 @@ By @Zoxc in [#4248](https://github.com/gfx-rs/wgpu/pull/4248)
 Timestamp queries are now supported on both Metal and Desktop OpenGL. On Apple chips on Metal, they only support timestamp queries in command buffers or in the renderpass descriptor,
 they do not support them inside a pass.
 
-Metal: By @Wumpf in [#4008](https://github.com/gfx-rs/wgpu/pull/4008)  
+Metal: By @Wumpf in [#4008](https://github.com/gfx-rs/wgpu/pull/4008)
 OpenGL: By @Zoxc in [#4267](https://github.com/gfx-rs/wgpu/pull/4267)
 
 ### Render/Compute Pass Query Writes
@@ -196,7 +196,7 @@ let instance = wgpu::Instance::new(InstanceDescriptor {
 });
 ```
 
-`gles_minor_version`: By @PJB3005 in [#3998](https://github.com/gfx-rs/wgpu/pull/3998)  
+`gles_minor_version`: By @PJB3005 in [#3998](https://github.com/gfx-rs/wgpu/pull/3998)
 `flags`: By @nical in [#4230](https://github.com/gfx-rs/wgpu/pull/4230)
 
 ### Many New Examples!
@@ -242,7 +242,7 @@ By @teoxoy in [#4185](https://github.com/gfx-rs/wgpu/pull/4185)
 - Add trace-level logging for most entry points in wgpu-core By @nical in [4183](https://github.com/gfx-rs/wgpu/pull/4183)
 - Add `Rgb10a2Uint` format. By @teoxoy in [4199](https://github.com/gfx-rs/wgpu/pull/4199)
 - Validate that resources are used on the right device. By @nical in [4207](https://github.com/gfx-rs/wgpu/pull/4207)
-- Expose instance flags. 
+- Expose instance flags.
 - Add support for the bgra8unorm-storage feature. By @jinleili and @nical in [#4228](https://github.com/gfx-rs/wgpu/pull/4228)
 - Calls to lost devices now return `DeviceError::Lost` instead of `DeviceError::Invalid`. By @bradwerth in [#4238]([https://github.com/gfx-rs/wgpu/pull/4238])
 - Let the `"strict_asserts"` feature enable check that wgpu-core's lock-ordering tokens are unique per thread. By @jimblandy in [#4258]([https://github.com/gfx-rs/wgpu/pull/4258])
@@ -271,6 +271,7 @@ By @teoxoy in [#4185](https://github.com/gfx-rs/wgpu/pull/4185)
 - Fix `clear` texture views being leaked when `wgpu::SurfaceTexture` is dropped before it is presented. By @rajveermalviya in [#4057](https://github.com/gfx-rs/wgpu/pull/4057).
 - Add `Feature::SHADER_UNUSED_VERTEX_OUTPUT` to allow unused vertex shader outputs. By @Aaron1011 in [#4116](https://github.com/gfx-rs/wgpu/pull/4116).
 - Fix a panic in `surface_configure`. By @nical in [#4220](https://github.com/gfx-rs/wgpu/pull/4220) and [#4227](https://github.com/gfx-rs/wgpu/pull/4227)
+- Pipelines register their implicit layouts in error cases. By @bradwerth in [#4624](https://github.com/gfx-rs/wgpu/pull/4624)
 
 #### Vulkan
 

diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs
@@ -26,7 +26,9 @@ use wgt::{BufferAddress, TextureFormat};
 
 use std::{borrow::Cow, iter, mem, ops::Range, ptr};
 
-use super::{BufferMapPendingClosure, ImplicitPipelineIds, InvalidDevice, UserClosures};
+use super::{
+    BufferMapPendingClosure, ImplicitPipelineIds, InvalidDevice, UserClosures, IMPLICIT_FAILURE,
+};
 
 impl<G: GlobalIdentityHandlerFactory> Global<G> {
     pub fn adapter_is_surface_supported<A: HalApi>(
@@ -1849,6 +1851,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
         let fid = hub.render_pipelines.prepare(id_in);
         let implicit_context = implicit_pipeline_ids.map(|ipi| ipi.prepare(hub));
+        let implicit_error_context = implicit_context.clone();
 
         let (adapter_guard, mut token) = hub.adapters.read(&mut token);
         let (device_guard, mut token) = hub.devices.read(&mut token);
@@ -1897,6 +1900,24 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         };
 
         let id = fid.assign_error(desc.label.borrow_or_default(), &mut token);
+
+        // We also need to assign errors to the implicit pipeline layout and the
+        // implicit bind group layout. We have to remove any existing entries first.
+        let (mut pipeline_layout_guard, mut token) = hub.pipeline_layouts.write(&mut token);
+        let (mut bgl_guard, _token) = hub.bind_group_layouts.write(&mut token);
+        if let Some(ref ids) = implicit_error_context {
+            if pipeline_layout_guard.contains(ids.root_id) {
+                pipeline_layout_guard.remove(ids.root_id);
+            }
+            pipeline_layout_guard.insert_error(ids.root_id, IMPLICIT_FAILURE);
+            for &bgl_id in ids.group_ids.iter() {
+                if bgl_guard.contains(bgl_id) {
+                    bgl_guard.remove(bgl_id);
+                }
+                bgl_guard.insert_error(bgl_id, IMPLICIT_FAILURE);
+            }
+        }
+
         (id, Some(error))
     }
 
@@ -2022,6 +2043,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
         let fid = hub.compute_pipelines.prepare(id_in);
         let implicit_context = implicit_pipeline_ids.map(|ipi| ipi.prepare(hub));
+        let implicit_error_context = implicit_context.clone();
 
         let (device_guard, mut token) = hub.devices.read(&mut token);
         let error = loop {
@@ -2041,7 +2063,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                     implicit_context: implicit_context.clone(),
                 });
             }
-
             let pipeline = match device.create_compute_pipeline(
                 device_id,
                 desc,
@@ -2066,6 +2087,24 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         };
 
         let id = fid.assign_error(desc.label.borrow_or_default(), &mut token);
+
+        // We also need to assign errors to the implicit pipeline layout and the
+        // implicit bind group layout. We have to remove any existing entries first.
+        let (mut pipeline_layout_guard, mut token) = hub.pipeline_layouts.write(&mut token);
+        let (mut bgl_guard, _token) = hub.bind_group_layouts.write(&mut token);
+        if let Some(ref ids) = implicit_error_context {
+            if pipeline_layout_guard.contains(ids.root_id) {
+                pipeline_layout_guard.remove(ids.root_id);
+            }
+            pipeline_layout_guard.insert_error(ids.root_id, IMPLICIT_FAILURE);
+            for &bgl_id in ids.group_ids.iter() {
+                if bgl_guard.contains(bgl_id) {
+                    bgl_guard.remove(bgl_id);
+                }
+                bgl_guard.insert_error(bgl_id, IMPLICIT_FAILURE);
+            }
+        }
+
         (id, Some(error))
     }