[wgpu-core] remove implicit PL & BGL IDs from pipeline creation (#7967)

This commit is contained in:
Teodor Tanasoaia 2025-07-18 19:04:46 +02:00 committed by GitHub
parent 4ef4f5dfbe
commit d2f8c44ac6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 18 additions and 194 deletions

View File

@ -651,7 +651,7 @@ impl GPUDevice {
let (id, err) = let (id, err) =
self.instance self.instance
.device_create_compute_pipeline(self.id, &wgpu_descriptor, None, None); .device_create_compute_pipeline(self.id, &wgpu_descriptor, None);
self.error_handler.push_error(err); self.error_handler.push_error(err);
@ -818,7 +818,7 @@ impl GPUDevice {
let (id, err) = let (id, err) =
self.instance self.instance
.device_create_render_pipeline(self.id, &wgpu_descriptor, None, None); .device_create_render_pipeline(self.id, &wgpu_descriptor, None);
self.error_handler.push_error(err); self.error_handler.push_error(err);

View File

@ -298,20 +298,8 @@ impl GlobalPlay for wgc::global::Global {
Action::DestroyShaderModule(id) => { Action::DestroyShaderModule(id) => {
self.shader_module_drop(id); self.shader_module_drop(id);
} }
Action::CreateComputePipeline { Action::CreateComputePipeline { id, desc } => {
id, let (_, error) = self.device_create_compute_pipeline(device, &desc, Some(id));
desc,
implicit_context,
} => {
let implicit_ids =
implicit_context
.as_ref()
.map(|ic| wgc::device::ImplicitPipelineIds {
root_id: ic.root_id,
group_ids: &ic.group_ids,
});
let (_, error) =
self.device_create_compute_pipeline(device, &desc, Some(id), implicit_ids);
if let Some(e) = error { if let Some(e) = error {
panic!("{e}"); panic!("{e}");
} }
@ -319,20 +307,8 @@ impl GlobalPlay for wgc::global::Global {
Action::DestroyComputePipeline(id) => { Action::DestroyComputePipeline(id) => {
self.compute_pipeline_drop(id); self.compute_pipeline_drop(id);
} }
Action::CreateRenderPipeline { Action::CreateRenderPipeline { id, desc } => {
id, let (_, error) = self.device_create_render_pipeline(device, &desc, Some(id));
desc,
implicit_context,
} => {
let implicit_ids =
implicit_context
.as_ref()
.map(|ic| wgc::device::ImplicitPipelineIds {
root_id: ic.root_id,
group_ids: &ic.group_ids,
});
let (_, error) =
self.device_create_render_pipeline(device, &desc, Some(id), implicit_ids);
if let Some(e) = error { if let Some(e) = error {
panic!("{e}"); panic!("{e}");
} }

View File

@ -30,7 +30,7 @@ use crate::{
use wgt::{BufferAddress, TextureFormat}; use wgt::{BufferAddress, TextureFormat};
use super::{ImplicitPipelineIds, UserClosures}; use super::UserClosures;
impl Global { impl Global {
pub fn adapter_is_surface_supported( pub fn adapter_is_surface_supported(
@ -1228,7 +1228,6 @@ impl Global {
device_id: DeviceId, device_id: DeviceId,
desc: &pipeline::RenderPipelineDescriptor, desc: &pipeline::RenderPipelineDescriptor,
id_in: Option<id::RenderPipelineId>, id_in: Option<id::RenderPipelineId>,
implicit_pipeline_ids: Option<ImplicitPipelineIds<'_>>,
) -> ( ) -> (
id::RenderPipelineId, id::RenderPipelineId,
Option<pipeline::CreateRenderPipelineError>, Option<pipeline::CreateRenderPipelineError>,
@ -1237,18 +1236,9 @@ impl Global {
let hub = &self.hub; let hub = &self.hub;
let missing_implicit_pipeline_ids =
desc.layout.is_none() && id_in.is_some() && implicit_pipeline_ids.is_none();
let fid = hub.render_pipelines.prepare(id_in); let fid = hub.render_pipelines.prepare(id_in);
let implicit_context = implicit_pipeline_ids.map(|ipi| ipi.prepare(hub));
let error = 'error: { let error = 'error: {
if missing_implicit_pipeline_ids {
// TODO: categorize this error as API misuse
break 'error pipeline::ImplicitLayoutError::MissingImplicitPipelineIds.into();
}
let device = self.hub.devices.get(device_id); let device = self.hub.devices.get(device_id);
#[cfg(feature = "trace")] #[cfg(feature = "trace")]
@ -1256,7 +1246,6 @@ impl Global {
trace.add(trace::Action::CreateRenderPipeline { trace.add(trace::Action::CreateRenderPipeline {
id: fid.id(), id: fid.id(),
desc: desc.clone(), desc: desc.clone(),
implicit_context: implicit_context.clone(),
}); });
} }
@ -1357,40 +1346,6 @@ impl Global {
Err(e) => break 'error e, Err(e) => break 'error e,
}; };
if let Some(ids) = implicit_context.as_ref() {
let group_count = pipeline.layout.bind_group_layouts.len();
if ids.group_ids.len() < group_count {
log::error!(
"Not enough bind group IDs ({}) specified for the implicit layout ({})",
ids.group_ids.len(),
group_count
);
// TODO: categorize this error as API misuse
break 'error pipeline::ImplicitLayoutError::MissingIds(group_count as _)
.into();
}
let mut pipeline_layout_guard = hub.pipeline_layouts.write();
let mut bgl_guard = hub.bind_group_layouts.write();
pipeline_layout_guard.insert(ids.root_id, Fallible::Valid(pipeline.layout.clone()));
let mut group_ids = ids.group_ids.iter();
// NOTE: If the first iterator is longer than the second, the `.zip()` impl will still advance the
// the first iterator before realizing that the second iterator has finished.
// The `pipeline.layout.bind_group_layouts` iterator will always be shorter than `ids.group_ids`,
// so using it as the first iterator for `.zip()` will work properly.
for (bgl, bgl_id) in pipeline
.layout
.bind_group_layouts
.iter()
.zip(&mut group_ids)
{
bgl_guard.insert(*bgl_id, Fallible::Valid(bgl.clone()));
}
for bgl_id in group_ids {
bgl_guard.insert(*bgl_id, Fallible::Invalid(Arc::new(String::new())));
}
}
let id = fid.assign(Fallible::Valid(pipeline)); let id = fid.assign(Fallible::Valid(pipeline));
api_log!("Device::create_render_pipeline -> {id:?}"); api_log!("Device::create_render_pipeline -> {id:?}");
@ -1399,17 +1354,6 @@ impl Global {
let id = fid.assign(Fallible::Invalid(Arc::new(desc.label.to_string()))); let id = fid.assign(Fallible::Invalid(Arc::new(desc.label.to_string())));
// We also need to assign errors to the implicit pipeline layout and the
// implicit bind group layouts.
if let Some(ids) = implicit_context {
let mut pipeline_layout_guard = hub.pipeline_layouts.write();
let mut bgl_guard = hub.bind_group_layouts.write();
pipeline_layout_guard.insert(ids.root_id, Fallible::Invalid(Arc::new(String::new())));
for bgl_id in ids.group_ids {
bgl_guard.insert(bgl_id, Fallible::Invalid(Arc::new(String::new())));
}
}
(id, Some(error)) (id, Some(error))
} }
@ -1467,7 +1411,6 @@ impl Global {
device_id: DeviceId, device_id: DeviceId,
desc: &pipeline::ComputePipelineDescriptor, desc: &pipeline::ComputePipelineDescriptor,
id_in: Option<id::ComputePipelineId>, id_in: Option<id::ComputePipelineId>,
implicit_pipeline_ids: Option<ImplicitPipelineIds<'_>>,
) -> ( ) -> (
id::ComputePipelineId, id::ComputePipelineId,
Option<pipeline::CreateComputePipelineError>, Option<pipeline::CreateComputePipelineError>,
@ -1476,18 +1419,9 @@ impl Global {
let hub = &self.hub; let hub = &self.hub;
let missing_implicit_pipeline_ids =
desc.layout.is_none() && id_in.is_some() && implicit_pipeline_ids.is_none();
let fid = hub.compute_pipelines.prepare(id_in); let fid = hub.compute_pipelines.prepare(id_in);
let implicit_context = implicit_pipeline_ids.map(|ipi| ipi.prepare(hub));
let error = 'error: { let error = 'error: {
if missing_implicit_pipeline_ids {
// TODO: categorize this error as API misuse
break 'error pipeline::ImplicitLayoutError::MissingImplicitPipelineIds.into();
}
let device = self.hub.devices.get(device_id); let device = self.hub.devices.get(device_id);
#[cfg(feature = "trace")] #[cfg(feature = "trace")]
@ -1495,7 +1429,6 @@ impl Global {
trace.add(trace::Action::CreateComputePipeline { trace.add(trace::Action::CreateComputePipeline {
id: fid.id(), id: fid.id(),
desc: desc.clone(), desc: desc.clone(),
implicit_context: implicit_context.clone(),
}); });
} }
@ -1545,40 +1478,6 @@ impl Global {
Err(e) => break 'error e, Err(e) => break 'error e,
}; };
if let Some(ids) = implicit_context.as_ref() {
let group_count = pipeline.layout.bind_group_layouts.len();
if ids.group_ids.len() < group_count {
log::error!(
"Not enough bind group IDs ({}) specified for the implicit layout ({})",
ids.group_ids.len(),
group_count
);
// TODO: categorize this error as API misuse
break 'error pipeline::ImplicitLayoutError::MissingIds(group_count as _)
.into();
}
let mut pipeline_layout_guard = hub.pipeline_layouts.write();
let mut bgl_guard = hub.bind_group_layouts.write();
pipeline_layout_guard.insert(ids.root_id, Fallible::Valid(pipeline.layout.clone()));
let mut group_ids = ids.group_ids.iter();
// NOTE: If the first iterator is longer than the second, the `.zip()` impl will still advance the
// the first iterator before realizing that the second iterator has finished.
// The `pipeline.layout.bind_group_layouts` iterator will always be shorter than `ids.group_ids`,
// so using it as the first iterator for `.zip()` will work properly.
for (bgl, bgl_id) in pipeline
.layout
.bind_group_layouts
.iter()
.zip(&mut group_ids)
{
bgl_guard.insert(*bgl_id, Fallible::Valid(bgl.clone()));
}
for bgl_id in group_ids {
bgl_guard.insert(*bgl_id, Fallible::Invalid(Arc::new(String::new())));
}
}
let id = fid.assign(Fallible::Valid(pipeline)); let id = fid.assign(Fallible::Valid(pipeline));
api_log!("Device::create_compute_pipeline -> {id:?}"); api_log!("Device::create_compute_pipeline -> {id:?}");
@ -1587,17 +1486,6 @@ impl Global {
let id = fid.assign(Fallible::Invalid(Arc::new(desc.label.to_string()))); let id = fid.assign(Fallible::Invalid(Arc::new(desc.label.to_string())));
// We also need to assign errors to the implicit pipeline layout and the
// implicit bind group layouts.
if let Some(ids) = implicit_context {
let mut pipeline_layout_guard = hub.pipeline_layouts.write();
let mut bgl_guard = hub.bind_group_layouts.write();
pipeline_layout_guard.insert(ids.root_id, Fallible::Invalid(Arc::new(String::new())));
for bgl_id in ids.group_ids {
bgl_guard.insert(bgl_id, Fallible::Invalid(Arc::new(String::new())));
}
}
(id, Some(error)) (id, Some(error))
} }

View File

@ -3,8 +3,6 @@ use core::{fmt, num::NonZeroU32};
use crate::{ use crate::{
binding_model, binding_model,
hub::Hub,
id::{BindGroupLayoutId, PipelineLayoutId},
ray_tracing::BlasCompactReadyPendingClosure, ray_tracing::BlasCompactReadyPendingClosure,
resource::{ resource::{
Buffer, BufferAccessError, BufferAccessResult, BufferMapOperation, Labeled, Buffer, BufferAccessError, BufferAccessResult, BufferMapOperation, Labeled,
@ -385,31 +383,6 @@ impl WebGpuError for MissingDownlevelFlags {
} }
} }
#[derive(Clone, Debug)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct ImplicitPipelineContext {
pub root_id: PipelineLayoutId,
pub group_ids: ArrayVec<BindGroupLayoutId, { hal::MAX_BIND_GROUPS }>,
}
pub struct ImplicitPipelineIds<'a> {
pub root_id: PipelineLayoutId,
pub group_ids: &'a [BindGroupLayoutId],
}
impl ImplicitPipelineIds<'_> {
fn prepare(self, hub: &Hub) -> ImplicitPipelineContext {
ImplicitPipelineContext {
root_id: hub.pipeline_layouts.prepare(Some(self.root_id)).id(),
group_ids: self
.group_ids
.iter()
.map(|id_in| hub.bind_group_layouts.prepare(Some(*id_in)).id())
.collect(),
}
}
}
/// Create a validator with the given validation flags. /// Create a validator with the given validation flags.
pub fn create_validator( pub fn create_validator(
features: wgt::Features, features: wgt::Features,

View File

@ -90,15 +90,11 @@ pub enum Action<'a> {
CreateComputePipeline { CreateComputePipeline {
id: id::ComputePipelineId, id: id::ComputePipelineId,
desc: crate::pipeline::ComputePipelineDescriptor<'a>, desc: crate::pipeline::ComputePipelineDescriptor<'a>,
#[cfg_attr(feature = "replay", serde(default))]
implicit_context: Option<super::ImplicitPipelineContext>,
}, },
DestroyComputePipeline(id::ComputePipelineId), DestroyComputePipeline(id::ComputePipelineId),
CreateRenderPipeline { CreateRenderPipeline {
id: id::RenderPipelineId, id: id::RenderPipelineId,
desc: crate::pipeline::RenderPipelineDescriptor<'a>, desc: crate::pipeline::RenderPipelineDescriptor<'a>,
#[cfg_attr(feature = "replay", serde(default))]
implicit_context: Option<super::ImplicitPipelineContext>,
}, },
DestroyRenderPipeline(id::RenderPipelineId), DestroyRenderPipeline(id::RenderPipelineId),
CreatePipelineCache { CreatePipelineCache {

View File

@ -190,10 +190,6 @@ pub type ImplicitBindGroupCount = u8;
#[derive(Clone, Debug, Error)] #[derive(Clone, Debug, Error)]
#[non_exhaustive] #[non_exhaustive]
pub enum ImplicitLayoutError { pub enum ImplicitLayoutError {
#[error("The implicit_pipeline_ids arg is required")]
MissingImplicitPipelineIds,
#[error("Missing IDs for deriving {0} bind groups")]
MissingIds(ImplicitBindGroupCount),
#[error("Unable to reflect the shader {0:?} interface")] #[error("Unable to reflect the shader {0:?} interface")]
ReflectionError(wgt::ShaderStages), ReflectionError(wgt::ShaderStages),
#[error(transparent)] #[error(transparent)]
@ -205,9 +201,7 @@ pub enum ImplicitLayoutError {
impl WebGpuError for ImplicitLayoutError { impl WebGpuError for ImplicitLayoutError {
fn webgpu_error_type(&self) -> ErrorType { fn webgpu_error_type(&self) -> ErrorType {
let e: &dyn WebGpuError = match self { let e: &dyn WebGpuError = match self {
Self::MissingImplicitPipelineIds | Self::MissingIds(_) | Self::ReflectionError(_) => { Self::ReflectionError(_) => return ErrorType::Validation,
return ErrorType::Validation
}
Self::BindGroup(e) => e, Self::BindGroup(e) => e,
Self::Pipeline(e) => e, Self::Pipeline(e) => e,
}; };

View File

@ -3,7 +3,7 @@ use alloc::sync::Arc;
use crate::{ use crate::{
id::Id, id::Id,
identity::IdentityManager, identity::IdentityManager,
lock::{rank, RwLock, RwLockReadGuard, RwLockWriteGuard}, lock::{rank, RwLock, RwLockReadGuard},
storage::{Element, Storage, StorageItem}, storage::{Element, Storage, StorageItem},
}; };
@ -55,6 +55,7 @@ pub(crate) struct FutureId<'a, T: StorageItem> {
} }
impl<T: StorageItem> FutureId<'_, T> { impl<T: StorageItem> FutureId<'_, T> {
#[cfg(feature = "trace")]
pub fn id(&self) -> Id<T::Marker> { pub fn id(&self) -> Id<T::Marker> {
self.id self.id
} }
@ -87,10 +88,6 @@ impl<T: StorageItem> Registry<T> {
pub(crate) fn read<'a>(&'a self) -> RwLockReadGuard<'a, Storage<T>> { pub(crate) fn read<'a>(&'a self) -> RwLockReadGuard<'a, Storage<T>> {
self.storage.read() self.storage.read()
} }
#[track_caller]
pub(crate) fn write<'a>(&'a self) -> RwLockWriteGuard<'a, Storage<T>> {
self.storage.write()
}
pub(crate) fn remove(&self, id: Id<T::Marker>) -> T { pub(crate) fn remove(&self, id: Id<T::Marker>) -> T {
let value = self.storage.write().remove(id); let value = self.storage.write().remove(id);
// This needs to happen *after* removing it from the storage, to maintain the // This needs to happen *after* removing it from the storage, to maintain the

View File

@ -1321,10 +1321,10 @@ impl dispatch::DeviceInterface for CoreDevice {
cache: desc.cache.map(|cache| cache.inner.as_core().id), cache: desc.cache.map(|cache| cache.inner.as_core().id),
}; };
let (id, error) = let (id, error) = self
self.context .context
.0 .0
.device_create_render_pipeline(self.id, &descriptor, None, None); .device_create_render_pipeline(self.id, &descriptor, None);
if let Some(cause) = error { if let Some(cause) = error {
if let wgc::pipeline::CreateRenderPipelineError::Internal { stage, ref error } = cause { if let wgc::pipeline::CreateRenderPipelineError::Internal { stage, ref error } = cause {
log::error!("Shader translation error for stage {stage:?}: {error}"); log::error!("Shader translation error for stage {stage:?}: {error}");
@ -1372,10 +1372,10 @@ impl dispatch::DeviceInterface for CoreDevice {
cache: desc.cache.map(|cache| cache.inner.as_core().id), cache: desc.cache.map(|cache| cache.inner.as_core().id),
}; };
let (id, error) = let (id, error) = self
self.context .context
.0 .0
.device_create_compute_pipeline(self.id, &descriptor, None, None); .device_create_compute_pipeline(self.id, &descriptor, None);
if let Some(cause) = error { if let Some(cause) = error {
if let wgc::pipeline::CreateComputePipelineError::Internal(ref error) = cause { if let wgc::pipeline::CreateComputePipelineError::Internal(ref error) = cause {
log::error!( log::error!(