From 91924fb6034408c14a7e75691fd9bedf24bf03e1 Mon Sep 17 00:00:00 2001
From: teoxoy <28601907+teoxoy@users.noreply.github.com>
Date: Wed, 17 Jul 2024 11:29:18 +0200
Subject: [PATCH] [wgpu-core] make `implicit_pipeline_ids` arg optional for
users that don't provide IDs
---
deno_webgpu/pipeline.rs | 25 +-------
player/src/lib.rs | 8 +--
wgpu-core/src/device/global.rs | 114 ++++++++++++++++-----------------
wgpu-core/src/device/mod.rs | 8 +--
wgpu-core/src/id.rs | 12 ----
wgpu-core/src/pipeline.rs | 2 +
wgpu/src/backend/wgpu_core.rs | 18 +-----
7 files changed, 70 insertions(+), 117 deletions(-)
diff --git a/deno_webgpu/pipeline.rs b/deno_webgpu/pipeline.rs
index 75bd9b3ef..f92570511 100644
--- a/deno_webgpu/pipeline.rs
+++ b/deno_webgpu/pipeline.rs
@@ -14,8 +14,6 @@ use std::rc::Rc;
use super::error::WebGpuError;
use super::error::WebGpuResult;
-const MAX_BIND_GROUPS: usize = 8;
-
pub(crate) struct WebGpuPipelineLayout(
pub(crate) crate::Instance,
pub(crate) wgpu_core::id::PipelineLayoutId,
@@ -118,21 +116,12 @@ pub fn op_webgpu_create_compute_pipeline(
},
cache: None,
};
- let implicit_pipelines = match layout {
- GPUPipelineLayoutOrGPUAutoLayoutMode::Layout(_) => None,
- GPUPipelineLayoutOrGPUAutoLayoutMode::Auto(GPUAutoLayoutMode::Auto) => {
- Some(wgpu_core::device::ImplicitPipelineIds {
- root_id: None,
- group_ids: &[None; MAX_BIND_GROUPS],
- })
- }
- };
let (compute_pipeline, maybe_err) = gfx_select!(device => instance.device_create_compute_pipeline(
device,
&descriptor,
None,
- implicit_pipelines
+ None,
));
let rid = state
@@ -397,21 +386,11 @@ pub fn op_webgpu_create_render_pipeline(
cache: None,
};
- let implicit_pipelines = match args.layout {
- GPUPipelineLayoutOrGPUAutoLayoutMode::Layout(_) => None,
- GPUPipelineLayoutOrGPUAutoLayoutMode::Auto(GPUAutoLayoutMode::Auto) => {
- Some(wgpu_core::device::ImplicitPipelineIds {
- root_id: None,
- group_ids: &[None; MAX_BIND_GROUPS],
- })
- }
- };
-
let (render_pipeline, maybe_err) = gfx_select!(device => instance.device_create_render_pipeline(
device,
&descriptor,
None,
- implicit_pipelines
+ None,
));
let rid = state
diff --git a/player/src/lib.rs b/player/src/lib.rs
index 8acdcd043..de56b1688 100644
--- a/player/src/lib.rs
+++ b/player/src/lib.rs
@@ -256,8 +256,8 @@ impl GlobalPlay for wgc::global::Global {
implicit_context
.as_ref()
.map(|ic| wgc::device::ImplicitPipelineIds {
- root_id: Some(ic.root_id),
- group_ids: wgc::id::as_option_slice(&ic.group_ids),
+ root_id: ic.root_id,
+ group_ids: &ic.group_ids,
});
let (_, error) =
self.device_create_compute_pipeline::(device, &desc, Some(id), implicit_ids);
@@ -277,8 +277,8 @@ impl GlobalPlay for wgc::global::Global {
implicit_context
.as_ref()
.map(|ic| wgc::device::ImplicitPipelineIds {
- root_id: Some(ic.root_id),
- group_ids: wgc::id::as_option_slice(&ic.group_ids),
+ root_id: ic.root_id,
+ group_ids: &ic.group_ids,
});
let (_, error) =
self.device_create_render_pipeline::(device, &desc, Some(id), implicit_ids);
diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs
index e5643a3da..e6a925129 100644
--- a/wgpu-core/src/device/global.rs
+++ b/wgpu-core/src/device/global.rs
@@ -1385,12 +1385,18 @@ impl Global {
let hub = A::hub(self);
+ 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 implicit_context = implicit_pipeline_ids.map(|ipi| ipi.prepare(hub));
- let is_auto_layout = desc.layout.is_none();
-
let error = 'error: {
+ if missing_implicit_pipeline_ids {
+ // TODO: categorize this error as API misuse
+ break 'error pipeline::ImplicitLayoutError::MissingImplicitPipelineIds.into();
+ }
+
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
@@ -1505,23 +1511,18 @@ impl Global {
Err(e) => break 'error e,
};
- if is_auto_layout {
- // TODO: categorize the errors below as API misuse
- let ids = 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
- );
- break 'error pipeline::ImplicitLayoutError::MissingIds(group_count as _)
- .into();
- }
- ids
- } else {
- break 'error pipeline::ImplicitLayoutError::MissingIds(0).into();
- };
+ 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();
@@ -1552,16 +1553,14 @@ impl Global {
let id = fid.assign_error();
- if is_auto_layout {
- // 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_error(ids.root_id);
- for bgl_id in ids.group_ids {
- bgl_guard.insert_error(bgl_id);
- }
+ // 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_error(ids.root_id);
+ for bgl_id in ids.group_ids {
+ bgl_guard.insert_error(bgl_id);
}
}
@@ -1629,12 +1628,18 @@ impl Global {
let hub = A::hub(self);
+ 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 implicit_context = implicit_pipeline_ids.map(|ipi| ipi.prepare(hub));
- let is_auto_layout = desc.layout.is_none();
-
let error = 'error: {
+ if missing_implicit_pipeline_ids {
+ // TODO: categorize this error as API misuse
+ break 'error pipeline::ImplicitLayoutError::MissingImplicitPipelineIds.into();
+ }
+
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
@@ -1703,23 +1708,18 @@ impl Global {
Err(e) => break 'error e,
};
- if is_auto_layout {
- // TODO: categorize the errors below as API misuse
- let ids = 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
- );
- break 'error pipeline::ImplicitLayoutError::MissingIds(group_count as _)
- .into();
- }
- ids
- } else {
- break 'error pipeline::ImplicitLayoutError::MissingIds(0).into();
- };
+ 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();
@@ -1750,16 +1750,14 @@ impl Global {
let id = fid.assign_error();
- if is_auto_layout {
- // 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_error(ids.root_id);
- for bgl_id in ids.group_ids {
- bgl_guard.insert_error(bgl_id);
- }
+ // 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_error(ids.root_id);
+ for bgl_id in ids.group_ids {
+ bgl_guard.insert_error(bgl_id);
}
}
diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs
index 03d9adcc6..e37291ef2 100644
--- a/wgpu-core/src/device/mod.rs
+++ b/wgpu-core/src/device/mod.rs
@@ -433,18 +433,18 @@ pub struct ImplicitPipelineContext {
}
pub struct ImplicitPipelineIds<'a> {
- pub root_id: Option,
- pub group_ids: &'a [Option],
+ pub root_id: PipelineLayoutId,
+ pub group_ids: &'a [BindGroupLayoutId],
}
impl ImplicitPipelineIds<'_> {
fn prepare(self, hub: &Hub) -> ImplicitPipelineContext {
ImplicitPipelineContext {
- root_id: hub.pipeline_layouts.prepare(self.root_id).into_id(),
+ root_id: hub.pipeline_layouts.prepare(Some(self.root_id)).into_id(),
group_ids: self
.group_ids
.iter()
- .map(|id_in| hub.bind_group_layouts.prepare(*id_in).into_id())
+ .map(|id_in| hub.bind_group_layouts.prepare(Some(*id_in)).into_id())
.collect(),
}
}
diff --git a/wgpu-core/src/id.rs b/wgpu-core/src/id.rs
index 05efbd2e4..c795063da 100644
--- a/wgpu-core/src/id.rs
+++ b/wgpu-core/src/id.rs
@@ -77,18 +77,6 @@ impl RawId {
}
}
-/// Coerce a slice of identifiers into a slice of optional raw identifiers.
-///
-/// There's two reasons why we know this is correct:
-/// * `Option` is guaranteed to be niche-filled to 0's.
-/// * The `T` in `Option` can inhabit any representation except 0's, since
-/// its underlying representation is `NonZero*`.
-pub fn as_option_slice(ids: &[Id]) -> &[Option>] {
- // SAFETY: Any Id is repr(transparent) over `Option`, since both
- // are backed by non-zero types.
- unsafe { std::slice::from_raw_parts(ids.as_ptr().cast(), ids.len()) }
-}
-
/// An identifier for a wgpu object.
///
/// An `Id` value identifies a value stored in a [`Global`]'s [`Hub`].
diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs
index b422ced5e..6366279ef 100644
--- a/wgpu-core/src/pipeline.rs
+++ b/wgpu-core/src/pipeline.rs
@@ -186,6 +186,8 @@ pub type ImplicitBindGroupCount = u8;
#[derive(Clone, Debug, Error)]
#[non_exhaustive]
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")]
diff --git a/wgpu/src/backend/wgpu_core.rs b/wgpu/src/backend/wgpu_core.rs
index 6485aefcd..5e1282379 100644
--- a/wgpu/src/backend/wgpu_core.rs
+++ b/wgpu/src/backend/wgpu_core.rs
@@ -1162,13 +1162,6 @@ impl crate::Context for ContextWgpuCore {
})
.collect();
- let implicit_pipeline_ids = match desc.layout {
- Some(_) => None,
- None => Some(wgc::device::ImplicitPipelineIds {
- root_id: None,
- group_ids: &[None; wgc::MAX_BIND_GROUPS],
- }),
- };
let descriptor = pipe::RenderPipelineDescriptor {
label: desc.label.map(Borrowed),
layout: desc.layout.map(|l| l.id.into()),
@@ -1211,7 +1204,7 @@ impl crate::Context for ContextWgpuCore {
*device,
&descriptor,
None,
- implicit_pipeline_ids
+ None,
));
if let Some(cause) = error {
if let wgc::pipeline::CreateRenderPipelineError::Internal { stage, ref error } = cause {
@@ -1235,13 +1228,6 @@ impl crate::Context for ContextWgpuCore {
) -> (Self::ComputePipelineId, Self::ComputePipelineData) {
use wgc::pipeline as pipe;
- let implicit_pipeline_ids = match desc.layout {
- Some(_) => None,
- None => Some(wgc::device::ImplicitPipelineIds {
- root_id: None,
- group_ids: &[None; wgc::MAX_BIND_GROUPS],
- }),
- };
let descriptor = pipe::ComputePipelineDescriptor {
label: desc.label.map(Borrowed),
layout: desc.layout.map(|l| l.id.into()),
@@ -1261,7 +1247,7 @@ impl crate::Context for ContextWgpuCore {
*device,
&descriptor,
None,
- implicit_pipeline_ids
+ None,
));
if let Some(cause) = error {
if let wgc::pipeline::CreateComputePipelineError::Internal(ref error) = cause {