Skip to content

Commit d2f8c44

Browse files
teoxoyteoxoy
authored
[wgpu-core] remove implicit PL & BGL IDs from pipeline creation (#7967)
1 parent 4ef4f5d commit d2f8c44

File tree

8 files changed

+18
-194
lines changed

8 files changed

+18
-194
lines changed

‎deno_webgpu/device.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,7 @@ impl GPUDevice {
651651

652652
let (id, err) =
653653
self.instance
654-
.device_create_compute_pipeline(self.id, &wgpu_descriptor, None, None);
654+
.device_create_compute_pipeline(self.id, &wgpu_descriptor, None);
655655

656656
self.error_handler.push_error(err);
657657

@@ -818,7 +818,7 @@ impl GPUDevice {
818818

819819
let (id, err) =
820820
self.instance
821-
.device_create_render_pipeline(self.id, &wgpu_descriptor, None, None);
821+
.device_create_render_pipeline(self.id, &wgpu_descriptor, None);
822822

823823
self.error_handler.push_error(err);
824824

‎player/src/lib.rs‎

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -298,41 +298,17 @@ impl GlobalPlay for wgc::global::Global {
298298
Action::DestroyShaderModule(id) => {
299299
self.shader_module_drop(id);
300300
}
301-
Action::CreateComputePipeline {
302-
id,
303-
desc,
304-
implicit_context,
305-
} => {
306-
let implicit_ids =
307-
implicit_context
308-
.as_ref()
309-
.map(|ic| wgc::device::ImplicitPipelineIds {
310-
root_id: ic.root_id,
311-
group_ids: &ic.group_ids,
312-
});
313-
let (_, error) =
314-
self.device_create_compute_pipeline(device, &desc, Some(id), implicit_ids);
301+
Action::CreateComputePipeline { id, desc } => {
302+
let (_, error) = self.device_create_compute_pipeline(device, &desc, Some(id));
315303
if let Some(e) = error {
316304
panic!("{e}");
317305
}
318306
}
319307
Action::DestroyComputePipeline(id) => {
320308
self.compute_pipeline_drop(id);
321309
}
322-
Action::CreateRenderPipeline {
323-
id,
324-
desc,
325-
implicit_context,
326-
} => {
327-
let implicit_ids =
328-
implicit_context
329-
.as_ref()
330-
.map(|ic| wgc::device::ImplicitPipelineIds {
331-
root_id: ic.root_id,
332-
group_ids: &ic.group_ids,
333-
});
334-
let (_, error) =
335-
self.device_create_render_pipeline(device, &desc, Some(id), implicit_ids);
310+
Action::CreateRenderPipeline { id, desc } => {
311+
let (_, error) = self.device_create_render_pipeline(device, &desc, Some(id));
336312
if let Some(e) = error {
337313
panic!("{e}");
338314
}

‎wgpu-core/src/device/global.rs‎

Lines changed: 1 addition & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use crate::{
3030

3131
use wgt::{BufferAddress, TextureFormat};
3232

33-
use super::{ImplicitPipelineIds, UserClosures};
33+
use super::UserClosures;
3434

3535
impl Global {
3636
pub fn adapter_is_surface_supported(
@@ -1228,7 +1228,6 @@ impl Global {
12281228
device_id: DeviceId,
12291229
desc: &pipeline::RenderPipelineDescriptor,
12301230
id_in: Option<id::RenderPipelineId>,
1231-
implicit_pipeline_ids: Option<ImplicitPipelineIds<'_>>,
12321231
) -> (
12331232
id::RenderPipelineId,
12341233
Option<pipeline::CreateRenderPipelineError>,
@@ -1237,26 +1236,16 @@ impl Global {
12371236

12381237
let hub = &self.hub;
12391238

1240-
let missing_implicit_pipeline_ids =
1241-
desc.layout.is_none() && id_in.is_some() && implicit_pipeline_ids.is_none();
1242-
12431239
let fid = hub.render_pipelines.prepare(id_in);
1244-
let implicit_context = implicit_pipeline_ids.map(|ipi| ipi.prepare(hub));
12451240

12461241
let error = 'error: {
1247-
if missing_implicit_pipeline_ids {
1248-
// TODO: categorize this error as API misuse
1249-
break 'error pipeline::ImplicitLayoutError::MissingImplicitPipelineIds.into();
1250-
}
1251-
12521242
let device = self.hub.devices.get(device_id);
12531243

12541244
#[cfg(feature = "trace")]
12551245
if let Some(ref mut trace) = *device.trace.lock() {
12561246
trace.add(trace::Action::CreateRenderPipeline {
12571247
id: fid.id(),
12581248
desc: desc.clone(),
1259-
implicit_context: implicit_context.clone(),
12601249
});
12611250
}
12621251

@@ -1357,40 +1346,6 @@ impl Global {
13571346
Err(e) => break 'error e,
13581347
};
13591348

1360-
if let Some(ids) = implicit_context.as_ref() {
1361-
let group_count = pipeline.layout.bind_group_layouts.len();
1362-
if ids.group_ids.len() < group_count {
1363-
log::error!(
1364-
"Not enough bind group IDs ({}) specified for the implicit layout ({})",
1365-
ids.group_ids.len(),
1366-
group_count
1367-
);
1368-
// TODO: categorize this error as API misuse
1369-
break 'error pipeline::ImplicitLayoutError::MissingIds(group_count as _)
1370-
.into();
1371-
}
1372-
1373-
let mut pipeline_layout_guard = hub.pipeline_layouts.write();
1374-
let mut bgl_guard = hub.bind_group_layouts.write();
1375-
pipeline_layout_guard.insert(ids.root_id, Fallible::Valid(pipeline.layout.clone()));
1376-
let mut group_ids = ids.group_ids.iter();
1377-
// NOTE: If the first iterator is longer than the second, the `.zip()` impl will still advance the
1378-
// the first iterator before realizing that the second iterator has finished.
1379-
// The `pipeline.layout.bind_group_layouts` iterator will always be shorter than `ids.group_ids`,
1380-
// so using it as the first iterator for `.zip()` will work properly.
1381-
for (bgl, bgl_id) in pipeline
1382-
.layout
1383-
.bind_group_layouts
1384-
.iter()
1385-
.zip(&mut group_ids)
1386-
{
1387-
bgl_guard.insert(*bgl_id, Fallible::Valid(bgl.clone()));
1388-
}
1389-
for bgl_id in group_ids {
1390-
bgl_guard.insert(*bgl_id, Fallible::Invalid(Arc::new(String::new())));
1391-
}
1392-
}
1393-
13941349
let id = fid.assign(Fallible::Valid(pipeline));
13951350
api_log!("Device::create_render_pipeline -> {id:?}");
13961351

@@ -1399,17 +1354,6 @@ impl Global {
13991354

14001355
let id = fid.assign(Fallible::Invalid(Arc::new(desc.label.to_string())));
14011356

1402-
// We also need to assign errors to the implicit pipeline layout and the
1403-
// implicit bind group layouts.
1404-
if let Some(ids) = implicit_context {
1405-
let mut pipeline_layout_guard = hub.pipeline_layouts.write();
1406-
let mut bgl_guard = hub.bind_group_layouts.write();
1407-
pipeline_layout_guard.insert(ids.root_id, Fallible::Invalid(Arc::new(String::new())));
1408-
for bgl_id in ids.group_ids {
1409-
bgl_guard.insert(bgl_id, Fallible::Invalid(Arc::new(String::new())));
1410-
}
1411-
}
1412-
14131357
(id, Some(error))
14141358
}
14151359

@@ -1467,7 +1411,6 @@ impl Global {
14671411
device_id: DeviceId,
14681412
desc: &pipeline::ComputePipelineDescriptor,
14691413
id_in: Option<id::ComputePipelineId>,
1470-
implicit_pipeline_ids: Option<ImplicitPipelineIds<'_>>,
14711414
) -> (
14721415
id::ComputePipelineId,
14731416
Option<pipeline::CreateComputePipelineError>,
@@ -1476,26 +1419,16 @@ impl Global {
14761419

14771420
let hub = &self.hub;
14781421

1479-
let missing_implicit_pipeline_ids =
1480-
desc.layout.is_none() && id_in.is_some() && implicit_pipeline_ids.is_none();
1481-
14821422
let fid = hub.compute_pipelines.prepare(id_in);
1483-
let implicit_context = implicit_pipeline_ids.map(|ipi| ipi.prepare(hub));
14841423

14851424
let error = 'error: {
1486-
if missing_implicit_pipeline_ids {
1487-
// TODO: categorize this error as API misuse
1488-
break 'error pipeline::ImplicitLayoutError::MissingImplicitPipelineIds.into();
1489-
}
1490-
14911425
let device = self.hub.devices.get(device_id);
14921426

14931427
#[cfg(feature = "trace")]
14941428
if let Some(ref mut trace) = *device.trace.lock() {
14951429
trace.add(trace::Action::CreateComputePipeline {
14961430
id: fid.id(),
14971431
desc: desc.clone(),
1498-
implicit_context: implicit_context.clone(),
14991432
});
15001433
}
15011434

@@ -1545,40 +1478,6 @@ impl Global {
15451478
Err(e) => break 'error e,
15461479
};
15471480

1548-
if let Some(ids) = implicit_context.as_ref() {
1549-
let group_count = pipeline.layout.bind_group_layouts.len();
1550-
if ids.group_ids.len() < group_count {
1551-
log::error!(
1552-
"Not enough bind group IDs ({}) specified for the implicit layout ({})",
1553-
ids.group_ids.len(),
1554-
group_count
1555-
);
1556-
// TODO: categorize this error as API misuse
1557-
break 'error pipeline::ImplicitLayoutError::MissingIds(group_count as _)
1558-
.into();
1559-
}
1560-
1561-
let mut pipeline_layout_guard = hub.pipeline_layouts.write();
1562-
let mut bgl_guard = hub.bind_group_layouts.write();
1563-
pipeline_layout_guard.insert(ids.root_id, Fallible::Valid(pipeline.layout.clone()));
1564-
let mut group_ids = ids.group_ids.iter();
1565-
// NOTE: If the first iterator is longer than the second, the `.zip()` impl will still advance the
1566-
// the first iterator before realizing that the second iterator has finished.
1567-
// The `pipeline.layout.bind_group_layouts` iterator will always be shorter than `ids.group_ids`,
1568-
// so using it as the first iterator for `.zip()` will work properly.
1569-
for (bgl, bgl_id) in pipeline
1570-
.layout
1571-
.bind_group_layouts
1572-
.iter()
1573-
.zip(&mut group_ids)
1574-
{
1575-
bgl_guard.insert(*bgl_id, Fallible::Valid(bgl.clone()));
1576-
}
1577-
for bgl_id in group_ids {
1578-
bgl_guard.insert(*bgl_id, Fallible::Invalid(Arc::new(String::new())));
1579-
}
1580-
}
1581-
15821481
let id = fid.assign(Fallible::Valid(pipeline));
15831482
api_log!("Device::create_compute_pipeline -> {id:?}");
15841483

@@ -1587,17 +1486,6 @@ impl Global {
15871486

15881487
let id = fid.assign(Fallible::Invalid(Arc::new(desc.label.to_string())));
15891488

1590-
// We also need to assign errors to the implicit pipeline layout and the
1591-
// implicit bind group layouts.
1592-
if let Some(ids) = implicit_context {
1593-
let mut pipeline_layout_guard = hub.pipeline_layouts.write();
1594-
let mut bgl_guard = hub.bind_group_layouts.write();
1595-
pipeline_layout_guard.insert(ids.root_id, Fallible::Invalid(Arc::new(String::new())));
1596-
for bgl_id in ids.group_ids {
1597-
bgl_guard.insert(bgl_id, Fallible::Invalid(Arc::new(String::new())));
1598-
}
1599-
}
1600-
16011489
(id, Some(error))
16021490
}
16031491

‎wgpu-core/src/device/mod.rs‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ use core::{fmt, num::NonZeroU32};
33

44
use crate::{
55
binding_model,
6-
hub::Hub,
7-
id::{BindGroupLayoutId, PipelineLayoutId},
86
ray_tracing::BlasCompactReadyPendingClosure,
97
resource::{
108
Buffer, BufferAccessError, BufferAccessResult, BufferMapOperation, Labeled,
@@ -385,31 +383,6 @@ impl WebGpuError for MissingDownlevelFlags {
385383
}
386384
}
387385

388-
#[derive(Clone, Debug)]
389-
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
390-
pub struct ImplicitPipelineContext {
391-
pub root_id: PipelineLayoutId,
392-
pub group_ids: ArrayVec<BindGroupLayoutId, { hal::MAX_BIND_GROUPS }>,
393-
}
394-
395-
pub struct ImplicitPipelineIds<'a> {
396-
pub root_id: PipelineLayoutId,
397-
pub group_ids: &'a [BindGroupLayoutId],
398-
}
399-
400-
impl ImplicitPipelineIds<'_> {
401-
fn prepare(self, hub: &Hub) -> ImplicitPipelineContext {
402-
ImplicitPipelineContext {
403-
root_id: hub.pipeline_layouts.prepare(Some(self.root_id)).id(),
404-
group_ids: self
405-
.group_ids
406-
.iter()
407-
.map(|id_in| hub.bind_group_layouts.prepare(Some(*id_in)).id())
408-
.collect(),
409-
}
410-
}
411-
}
412-
413386
/// Create a validator with the given validation flags.
414387
pub fn create_validator(
415388
features: wgt::Features,

‎wgpu-core/src/device/trace.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,15 +90,11 @@ pub enum Action<'a> {
9090
CreateComputePipeline {
9191
id: id::ComputePipelineId,
9292
desc: crate::pipeline::ComputePipelineDescriptor<'a>,
93-
#[cfg_attr(feature = "replay", serde(default))]
94-
implicit_context: Option<super::ImplicitPipelineContext>,
9593
},
9694
DestroyComputePipeline(id::ComputePipelineId),
9795
CreateRenderPipeline {
9896
id: id::RenderPipelineId,
9997
desc: crate::pipeline::RenderPipelineDescriptor<'a>,
100-
#[cfg_attr(feature = "replay", serde(default))]
101-
implicit_context: Option<super::ImplicitPipelineContext>,
10298
},
10399
DestroyRenderPipeline(id::RenderPipelineId),
104100
CreatePipelineCache {

‎wgpu-core/src/pipeline.rs‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,6 @@ pub type ImplicitBindGroupCount = u8;
190190
#[derive(Clone, Debug, Error)]
191191
#[non_exhaustive]
192192
pub enum ImplicitLayoutError {
193-
#[error("The implicit_pipeline_ids arg is required")]
194-
MissingImplicitPipelineIds,
195-
#[error("Missing IDs for deriving {0} bind groups")]
196-
MissingIds(ImplicitBindGroupCount),
197193
#[error("Unable to reflect the shader {0:?} interface")]
198194
ReflectionError(wgt::ShaderStages),
199195
#[error(transparent)]
@@ -205,9 +201,7 @@ pub enum ImplicitLayoutError {
205201
impl WebGpuError for ImplicitLayoutError {
206202
fn webgpu_error_type(&self) -> ErrorType {
207203
let e: &dyn WebGpuError = match self {
208-
Self::MissingImplicitPipelineIds | Self::MissingIds(_) | Self::ReflectionError(_) => {
209-
return ErrorType::Validation
210-
}
204+
Self::ReflectionError(_) => return ErrorType::Validation,
211205
Self::BindGroup(e) => e,
212206
Self::Pipeline(e) => e,
213207
};

‎wgpu-core/src/registry.rs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use alloc::sync::Arc;
33
use crate::{
44
id::Id,
55
identity::IdentityManager,
6-
lock::{rank, RwLock, RwLockReadGuard, RwLockWriteGuard},
6+
lock::{rank, RwLock, RwLockReadGuard},
77
storage::{Element, Storage, StorageItem},
88
};
99

@@ -55,6 +55,7 @@ pub(crate) struct FutureId<'a, T: StorageItem> {
5555
}
5656

5757
impl<T: StorageItem> FutureId<'_, T> {
58+
#[cfg(feature = "trace")]
5859
pub fn id(&self) -> Id<T::Marker> {
5960
self.id
6061
}
@@ -87,10 +88,6 @@ impl<T: StorageItem> Registry<T> {
8788
pub(crate) fn read<'a>(&'a self) -> RwLockReadGuard<'a, Storage<T>> {
8889
self.storage.read()
8990
}
90-
#[track_caller]
91-
pub(crate) fn write<'a>(&'a self) -> RwLockWriteGuard<'a, Storage<T>> {
92-
self.storage.write()
93-
}
9491
pub(crate) fn remove(&self, id: Id<T::Marker>) -> T {
9592
let value = self.storage.write().remove(id);
9693
// This needs to happen *after* removing it from the storage, to maintain the

‎wgpu/src/backend/wgpu_core.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1321,10 +1321,10 @@ impl dispatch::DeviceInterface for CoreDevice {
13211321
cache: desc.cache.map(|cache| cache.inner.as_core().id),
13221322
};
13231323

1324-
let (id, error) =
1325-
self.context
1326-
.0
1327-
.device_create_render_pipeline(self.id, &descriptor, None, None);
1324+
let (id, error) = self
1325+
.context
1326+
.0
1327+
.device_create_render_pipeline(self.id, &descriptor, None);
13281328
if let Some(cause) = error {
13291329
if let wgc::pipeline::CreateRenderPipelineError::Internal { stage, ref error } = cause {
13301330
log::error!("Shader translation error for stage {stage:?}: {error}");
@@ -1372,10 +1372,10 @@ impl dispatch::DeviceInterface for CoreDevice {
13721372
cache: desc.cache.map(|cache| cache.inner.as_core().id),
13731373
};
13741374

1375-
let (id, error) =
1376-
self.context
1377-
.0
1378-
.device_create_compute_pipeline(self.id, &descriptor, None, None);
1375+
let (id, error) = self
1376+
.context
1377+
.0
1378+
.device_create_compute_pipeline(self.id, &descriptor, None);
13791379
if let Some(cause) = error {
13801380
if let wgc::pipeline::CreateComputePipelineError::Internal(ref error) = cause {
13811381
log::error!(

0 commit comments

Comments
 (0)