Skip to content

Commit

Permalink
Remove IDs from wgpu traits (#6134)
Browse files Browse the repository at this point in the history 
Remove `wgpu`'s `.global_id()` getters.

Implement `PartialEq`, `Eq`, `Hash`, `PartialOrd` and `Ord` for wgpu resources.
  • Loading branch information
@teoxoy
teoxoy committed Aug 27, 2024
1 parent c7e5d07 commit 338678a
Show file tree
Hide file tree
Showing 31 changed files with 1,273 additions and 3,592 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ which we're hoping to build performance improvements upon in the future.

By @wumpf in [#6069](https://github.com/gfx-rs/wgpu/pull/6069), [#6099](https://github.com/gfx-rs/wgpu/pull/6099), [#6100](https://github.com/gfx-rs/wgpu/pull/6100).

#### `wgpu`'s resources no longer have `.global_id()` getters

`wgpu-core`'s internals no longer use nor need IDs and we are moving towards removing IDs
completely. This is a step in that direction.

Current users of `.global_id()` are encouraged to make use of the `PartialEq`, `Eq`, `Hash`, `PartialOrd` and `Ord`
traits that have now been implemented for `wgpu` resources.

By @teoxoy [#6134](https://github.com/gfx-rs/wgpu/pull/6134).

### New Features

#### Naga
Expand Down
162 changes: 55 additions & 107 deletions tests/tests/bind_group_layout_dedup.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,129 +31,77 @@ static BIND_GROUP_LAYOUT_DEDUPLICATION: GpuTestConfiguration = GpuTestConfigurat
.run_async(bgl_dedupe);

async fn bgl_dedupe(ctx: TestingContext) {
let entries_1 = &[];

let entries_2 = &[ENTRY];

// Block so we can force all resource to die.
{
let bgl_1a = ctx
.device
.create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor {
label: None,
entries: entries_1,
});

let bgl_2 = ctx
.device
.create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor {
label: None,
entries: entries_2,
});

let bgl_1b = ctx
.device
.create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor {
label: None,
entries: entries_1,
});

let bg_1a = ctx.device.create_bind_group(&wgpu::BindGroupDescriptor {
let entries = &[];

let bgl_1a = ctx
.device
.create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor {
label: None,
layout: &bgl_1a,
entries: &[],
entries,
});

let bg_1b = ctx.device.create_bind_group(&wgpu::BindGroupDescriptor {
let bgl_1b = ctx
.device
.create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor {
label: None,
layout: &bgl_1b,
entries: &[],
entries,
});

let pipeline_layout = ctx
.device
.create_pipeline_layout(&wgpu::PipelineLayoutDescriptor {
label: None,
bind_group_layouts: &[&bgl_1b],
push_constant_ranges: &[],
});

let module = ctx
.device
.create_shader_module(wgpu::ShaderModuleDescriptor {
label: None,
source: wgpu::ShaderSource::Wgsl(SHADER_SRC.into()),
});

let desc = wgpu::ComputePipelineDescriptor {
label: None,
layout: Some(&pipeline_layout),
module: &module,
entry_point: Some("no_resources"),
compilation_options: Default::default(),
cache: None,
};
let bg_1a = ctx.device.create_bind_group(&wgpu::BindGroupDescriptor {
label: None,
layout: &bgl_1a,
entries: &[],
});

let pipeline = ctx.device.create_compute_pipeline(&desc);
let bg_1b = ctx.device.create_bind_group(&wgpu::BindGroupDescriptor {
label: None,
layout: &bgl_1b,
entries: &[],
});

let mut encoder = ctx.device.create_command_encoder(&Default::default());
let pipeline_layout = ctx
.device
.create_pipeline_layout(&wgpu::PipelineLayoutDescriptor {
label: None,
bind_group_layouts: &[&bgl_1b],
push_constant_ranges: &[],
});

let mut pass = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor {
let module = ctx
.device
.create_shader_module(wgpu::ShaderModuleDescriptor {
label: None,
timestamp_writes: None,
source: wgpu::ShaderSource::Wgsl(SHADER_SRC.into()),
});

pass.set_bind_group(0, &bg_1b, &[]);
pass.set_pipeline(&pipeline);
pass.dispatch_workgroups(1, 1, 1);
let desc = wgpu::ComputePipelineDescriptor {
label: None,
layout: Some(&pipeline_layout),
module: &module,
entry_point: Some("no_resources"),
compilation_options: Default::default(),
cache: None,
};

pass.set_bind_group(0, &bg_1a, &[]);
pass.dispatch_workgroups(1, 1, 1);
let pipeline = ctx.device.create_compute_pipeline(&desc);

drop(pass);
let mut encoder = ctx.device.create_command_encoder(&Default::default());

ctx.queue.submit(Some(encoder.finish()));
let mut pass = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor {
label: None,
timestamp_writes: None,
});

// Abuse the fact that global_id is really just the bitpacked ids when targeting wgpu-core.
if ctx.adapter_info.backend != wgt::Backend::BrowserWebGpu {
let bgl_1a_idx = bgl_1a.global_id().inner() & 0xFFFF_FFFF;
assert_eq!(bgl_1a_idx, 0);
let bgl_2_idx = bgl_2.global_id().inner() & 0xFFFF_FFFF;
assert_eq!(bgl_2_idx, 1);
let bgl_1b_idx = bgl_1b.global_id().inner() & 0xFFFF_FFFF;
assert_eq!(bgl_1b_idx, 2);
}
}

ctx.async_poll(wgpu::Maintain::wait())
.await
.panic_on_timeout();

if ctx.adapter_info.backend != wgt::Backend::BrowserWebGpu {
// Indices are made reusable as soon as the handle is dropped so we keep them around
// for the duration of the loop.
let mut bgls = Vec::new();
let mut indices = Vec::new();
// Now all of the BGL ids should be dead, so we should get the same ids again.
for _ in 0..=2 {
let test_bgl = ctx
.device
.create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor {
label: None,
entries: entries_1,
});

let test_bgl_idx = test_bgl.global_id().inner() & 0xFFFF_FFFF;
bgls.push(test_bgl);
indices.push(test_bgl_idx);
}
// We don't guarantee that the IDs will appear in the same order. Sort them
// and check that they all appear exactly once.
indices.sort();
for (i, index) in indices.iter().enumerate() {
assert_eq!(*index, i as u64);
}
}
pass.set_bind_group(0, &bg_1b, &[]);
pass.set_pipeline(&pipeline);
pass.dispatch_workgroups(1, 1, 1);

pass.set_bind_group(0, &bg_1a, &[]);
pass.dispatch_workgroups(1, 1, 1);

drop(pass);

ctx.queue.submit(Some(encoder.finish()));
}

#[gpu_test]
Expand Down
53 changes: 20 additions & 33 deletions wgpu/src/api/adapter.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{future::Future, sync::Arc, thread};

use crate::context::{DeviceRequest, DynContext, ObjectId};
use crate::context::{DeviceRequest, DynContext};
use crate::*;

/// Handle to a physical graphics and/or compute device.
Expand All @@ -14,7 +14,6 @@ use crate::*;
#[derive(Debug)]
pub struct Adapter {
pub(crate) context: Arc<C>,
pub(crate) id: ObjectId,
pub(crate) data: Box<Data>,
}
#[cfg(send_sync)]
Expand All @@ -23,7 +22,7 @@ static_assertions::assert_impl_all!(Adapter: Send, Sync);
impl Drop for Adapter {
fn drop(&mut self) {
if !thread::panicking() {
self.context.adapter_drop(&self.id, self.data.as_ref())
self.context.adapter_drop(self.data.as_ref())
}
}
}
Expand All @@ -40,14 +39,6 @@ pub type RequestAdapterOptions<'a, 'b> = RequestAdapterOptionsBase<&'a Surface<'
static_assertions::assert_impl_all!(RequestAdapterOptions<'_, '_>: Send, Sync);

impl Adapter {
/// Returns a globally-unique identifier for this `Adapter`.
///
/// Calling this method multiple times on the same object will always return the same value.
/// The returned value is guaranteed to be different for all resources created from the same `Instance`.
pub fn global_id(&self) -> Id<Self> {
Id::new(self.id)
}

/// Requests a connection to a physical device, creating a logical device.
///
/// Returns the [`Device`] together with a [`Queue`] that executes command buffers.
Expand Down Expand Up @@ -80,28 +71,23 @@ impl Adapter {
let context = Arc::clone(&self.context);
let device = DynContext::adapter_request_device(
&*self.context,
&self.id,
self.data.as_ref(),
desc,
trace_path,
);
async move {
device.await.map(
|DeviceRequest {
device_id,
device_data,
queue_id,
queue_data,
}| {
(
Device {
context: Arc::clone(&context),
id: device_id,
data: device_data,
},
Queue {
context,
id: queue_id,
data: queue_data,
},
)
Expand Down Expand Up @@ -131,18 +117,21 @@ impl Adapter {
// Part of the safety requirements is that the device was generated from the same adapter.
// Therefore, unwrap is fine here since only WgpuCoreContext based adapters have the ability to create hal devices.
.unwrap()
.create_device_from_hal(&self.id.into(), hal_device, desc, trace_path)
.create_device_from_hal(
crate::context::downcast_ref(&self.data),
hal_device,
desc,
trace_path,
)
}
.map(|(device, queue)| {
(
Device {
context: Arc::clone(&context),
id: device.id().into(),
data: Box::new(device),
},
Queue {
context,
id: queue.id().into(),
data: Box::new(queue),
},
)
Expand Down Expand Up @@ -178,7 +167,12 @@ impl Adapter {
.as_any()
.downcast_ref::<crate::backend::ContextWgpuCore>()
{
unsafe { ctx.adapter_as_hal::<A, F, R>(self.id.into(), hal_adapter_callback) }
unsafe {
ctx.adapter_as_hal::<A, F, R>(
crate::context::downcast_ref(&self.data),
hal_adapter_callback,
)
}
} else {
hal_adapter_callback(None)
}
Expand All @@ -188,44 +182,37 @@ impl Adapter {
pub fn is_surface_supported(&self, surface: &Surface<'_>) -> bool {
DynContext::adapter_is_surface_supported(
&*self.context,
&self.id,
self.data.as_ref(),
&surface.id,
surface.surface_data.as_ref(),
)
}

/// The features which can be used to create devices on this adapter.
pub fn features(&self) -> Features {
DynContext::adapter_features(&*self.context, &self.id, self.data.as_ref())
DynContext::adapter_features(&*self.context, self.data.as_ref())
}

/// The best limits which can be used to create devices on this adapter.
pub fn limits(&self) -> Limits {
DynContext::adapter_limits(&*self.context, &self.id, self.data.as_ref())
DynContext::adapter_limits(&*self.context, self.data.as_ref())
}

/// Get info about the adapter itself.
pub fn get_info(&self) -> AdapterInfo {
DynContext::adapter_get_info(&*self.context, &self.id, self.data.as_ref())
DynContext::adapter_get_info(&*self.context, self.data.as_ref())
}

/// Get info about the adapter itself.
pub fn get_downlevel_capabilities(&self) -> DownlevelCapabilities {
DynContext::adapter_downlevel_capabilities(&*self.context, &self.id, self.data.as_ref())
DynContext::adapter_downlevel_capabilities(&*self.context, self.data.as_ref())
}

/// Returns the features supported for a given texture format by this adapter.
///
/// Note that the WebGPU spec further restricts the available usages/features.
/// To disable these restrictions on a device, request the [`Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES`] feature.
pub fn get_texture_format_features(&self, format: TextureFormat) -> TextureFormatFeatures {
DynContext::adapter_get_texture_format_features(
&*self.context,
&self.id,
self.data.as_ref(),
format,
)
DynContext::adapter_get_texture_format_features(&*self.context, self.data.as_ref(), format)
}

/// Generates a timestamp using the clock used by the presentation engine.
Expand All @@ -250,6 +237,6 @@ impl Adapter {
//
/// [Instant]: std::time::Instant
pub fn get_presentation_timestamp(&self) -> PresentationTimestamp {
DynContext::adapter_get_presentation_timestamp(&*self.context, &self.id, self.data.as_ref())
DynContext::adapter_get_presentation_timestamp(&*self.context, self.data.as_ref())
}
}
Loading

0 comments on commit 338678a

Please sign in to comment.