From 7aaf6725769046bd2ffecd1fceee4efe375a8356 Mon Sep 17 00:00:00 2001 From: Brad Werth Date: Mon, 6 Nov 2023 12:53:31 -0800 Subject: [PATCH] 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. --- CHANGELOG.md | 7 +++--- wgpu-core/src/device/global.rs | 43 ++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 898b83e6b2..7b1190c904 100644 --- 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 index d8f1c0f5d0..9c2e51cb1a 100644 --- 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 Global { pub fn adapter_is_surface_supported( @@ -1849,6 +1851,7 @@ impl Global { 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 Global { }; 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 Global { 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 Global { implicit_context: implicit_context.clone(), }); } - let pipeline = match device.create_compute_pipeline( device_id, desc, @@ -2066,6 +2087,24 @@ impl Global { }; 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)) }