Skip to content

Commit

Permalink
Implement Clone on Api Types and Arc Dispatcher (#6665)
Browse files Browse the repository at this point in the history
  • Loading branch information
cwfitzgerald authored Jan 3, 2025
1 parent fb17ee8 commit 03ff99e
Show file tree
Hide file tree
Showing 30 changed files with 374 additions and 206 deletions.
43 changes: 43 additions & 0 deletions tests/tests/cloneable_types.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use wgpu_test::{gpu_test, TestingContext};

#[gpu_test]
static CLONEABLE_BUFFERS: GpuTestConfiguration =
wgpu_test::GpuTestConfiguration::new().run_sync(cloneable_buffers);

// Test a basic case of cloneable types where you clone the buffer to be able
// to access the buffer inside the callback as well as outside.
fn cloneable_buffers(ctx: TestingContext) {
let buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor {
label: None,
size: 32,
usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::MAP_READ,
mapped_at_creation: true,
});

let buffer_contents: Vec<u8> = (0..32).collect();

buffer
.slice(..)
.get_mapped_range_mut()
.copy_from_slice(&buffer_contents);

buffer.unmap();

// This is actually a bug, we should not need to call submit to make the buffer contents visible.
ctx.queue.submit([]);

let cloned_buffer = buffer.clone();
let cloned_buffer_contents = buffer_contents.clone();

buffer.slice(..).map_async(wgpu::MapMode::Read, move |_| {
let data = cloned_buffer.slice(..).get_mapped_range();

assert_eq!(&*data, &cloned_buffer_contents);
});

ctx.device.poll(wgpu::Maintain::Wait);

let data = buffer.slice(..).get_mapped_range();

assert_eq!(&*data, &buffer_contents);
}
1 change: 1 addition & 0 deletions tests/tests/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod buffer;
mod buffer_copy;
mod buffer_usages;
mod clear_texture;
mod cloneable_types;
mod compute_pass_ownership;
mod create_surface_error;
mod device;
Expand Down
23 changes: 15 additions & 8 deletions wgpu/src/api/adapter.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::future::Future;
use std::{future::Future, sync::Arc};

use crate::*;

Expand All @@ -13,9 +13,9 @@ use crate::*;
/// Does not have to be kept alive.
///
/// Corresponds to [WebGPU `GPUAdapter`](https://gpuweb.github.io/gpuweb/#gpu-adapter).
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct Adapter {
pub(crate) inner: dispatch::DispatchAdapter,
pub(crate) inner: Arc<dispatch::DispatchAdapter>,
}
#[cfg(send_sync)]
static_assertions::assert_impl_all!(Adapter: Send, Sync);
Expand Down Expand Up @@ -65,9 +65,16 @@ impl Adapter {
) -> impl Future<Output = Result<(Device, Queue), RequestDeviceError>> + WasmNotSend {
let device = self.inner.request_device(desc, trace_path);
async move {
device
.await
.map(|(device, queue)| (Device { inner: device }, Queue { inner: queue }))
device.await.map(|(device, queue)| {
(
Device {
inner: Arc::new(device),
},
Queue {
inner: Arc::new(queue),
},
)
})
}
}

Expand All @@ -93,10 +100,10 @@ impl Adapter {

Ok((
Device {
inner: device.into(),
inner: Arc::new(device.into()),
},
Queue {
inner: queue.into(),
inner: Arc::new(queue.into()),
},
))
}
Expand Down
6 changes: 4 additions & 2 deletions wgpu/src/api/bind_group.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::sync::Arc;

use crate::*;

/// Handle to a binding group.
Expand All @@ -8,9 +10,9 @@ use crate::*;
/// [`ComputePass`] with [`ComputePass::set_bind_group`].
///
/// Corresponds to [WebGPU `GPUBindGroup`](https://gpuweb.github.io/gpuweb/#gpubindgroup).
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct BindGroup {
pub(crate) inner: dispatch::DispatchBindGroup,
pub(crate) inner: Arc<dispatch::DispatchBindGroup>,
}
#[cfg(send_sync)]
static_assertions::assert_impl_all!(BindGroup: Send, Sync);
Expand Down
6 changes: 4 additions & 2 deletions wgpu/src/api/bind_group_layout.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::sync::Arc;

use crate::*;

/// Handle to a binding group layout.
Expand All @@ -11,9 +13,9 @@ use crate::*;
///
/// Corresponds to [WebGPU `GPUBindGroupLayout`](
/// https://gpuweb.github.io/gpuweb/#gpubindgrouplayout).
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct BindGroupLayout {
pub(crate) inner: dispatch::DispatchBindGroupLayout,
pub(crate) inner: Arc<dispatch::DispatchBindGroupLayout>,
}
#[cfg(send_sync)]
static_assertions::assert_impl_all!(BindGroupLayout: Send, Sync);
Expand Down
20 changes: 7 additions & 13 deletions wgpu/src/api/blas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ static_assertions::assert_impl_all!(CreateBlasDescriptor<'_>: Send, Sync);
/// [TlasPackage]: crate::TlasPackage
#[derive(Debug, Clone)]
pub struct TlasInstance {
pub(crate) blas: Arc<BlasShared>,
pub(crate) blas: Arc<dispatch::DispatchBlas>,
/// Affine transform matrix 3x4 (rows x columns, row major order).
pub transform: [f32; 12],
/// Custom index for the instance used inside the shader.
Expand All @@ -71,7 +71,7 @@ impl TlasInstance {
/// generate a validation error.
pub fn new(blas: &Blas, transform: [f32; 12], custom_index: u32, mask: u8) -> Self {
Self {
blas: blas.shared.clone(),
blas: blas.inner.clone(),
transform,
custom_index,
mask,
Expand All @@ -83,7 +83,7 @@ impl TlasInstance {
/// See the note on [TlasInstance] about the
/// guarantees of keeping a BLAS alive.
pub fn set_blas(&mut self, blas: &Blas) {
self.blas = blas.shared.clone();
self.blas = blas.inner.clone();
}
}

Expand Down Expand Up @@ -128,13 +128,7 @@ pub struct BlasBuildEntry<'a> {
}
static_assertions::assert_impl_all!(BlasBuildEntry<'_>: WasmNotSendSync);

#[derive(Debug)]
pub(crate) struct BlasShared {
pub(crate) inner: dispatch::DispatchBlas,
}
static_assertions::assert_impl_all!(BlasShared: WasmNotSendSync);

#[derive(Debug)]
#[derive(Debug, Clone)]
/// Bottom Level Acceleration Structure (BLAS).
///
/// A BLAS is a device-specific raytracing acceleration structure that contains geometry data.
Expand All @@ -144,11 +138,11 @@ static_assertions::assert_impl_all!(BlasShared: WasmNotSendSync);
/// [Tlas]: crate::Tlas
pub struct Blas {
pub(crate) handle: Option<u64>,
pub(crate) shared: Arc<BlasShared>,
pub(crate) inner: Arc<dispatch::DispatchBlas>,
}
static_assertions::assert_impl_all!(Blas: WasmNotSendSync);

crate::cmp::impl_eq_ord_hash_proxy!(Blas => .shared.inner);
crate::cmp::impl_eq_ord_hash_proxy!(Blas => .inner);

impl Blas {
/// Raw handle to the acceleration structure, used inside raw instance buffers.
Expand All @@ -157,7 +151,7 @@ impl Blas {
}
/// Destroy the associated native resources as soon as possible.
pub fn destroy(&self) {
self.shared.inner.destroy();
self.inner.destroy();
}
}

Expand Down
67 changes: 46 additions & 21 deletions wgpu/src/api/buffer.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
use std::{
error, fmt,
ops::{Bound, Deref, DerefMut, Range, RangeBounds},
sync::Arc,
};

use parking_lot::Mutex;

use crate::*;

#[derive(Debug)]
pub(crate) struct BufferShared {
pub inner: dispatch::DispatchBuffer,
pub map_context: Mutex<MapContext>,
pub size: wgt::BufferAddress,
pub usage: BufferUsages,
// Todo: missing map_state https://www.w3.org/TR/webgpu/#dom-gpubuffer-mapstate
}

/// Handle to a GPU-accessible buffer.
///
/// Created with [`Device::create_buffer`] or
Expand Down Expand Up @@ -167,18 +177,14 @@ use crate::*;
/// [mac]: BufferDescriptor::mapped_at_creation
/// [`MAP_READ`]: BufferUsages::MAP_READ
/// [`MAP_WRITE`]: BufferUsages::MAP_WRITE
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct Buffer {
pub(crate) inner: dispatch::DispatchBuffer,
pub(crate) map_context: Mutex<MapContext>,
pub(crate) size: wgt::BufferAddress,
pub(crate) usage: BufferUsages,
// Todo: missing map_state https://www.w3.org/TR/webgpu/#dom-gpubuffer-mapstate
pub(crate) shared: Arc<BufferShared>,
}
#[cfg(send_sync)]
static_assertions::assert_impl_all!(Buffer: Send, Sync);

crate::cmp::impl_eq_ord_hash_proxy!(Buffer => .inner);
crate::cmp::impl_eq_ord_hash_proxy!(Buffer => .shared.inner);

impl Buffer {
/// Return the binding view of the entire buffer.
Expand Down Expand Up @@ -206,7 +212,7 @@ impl Buffer {
&self,
hal_buffer_callback: F,
) -> R {
if let Some(buffer) = self.inner.as_core_opt() {
if let Some(buffer) = self.shared.inner.as_core_opt() {
unsafe {
buffer
.context
Expand All @@ -233,7 +239,7 @@ impl Buffer {
/// end of the buffer.
pub fn slice<S: RangeBounds<BufferAddress>>(&self, bounds: S) -> BufferSlice<'_> {
let (offset, size) = range_to_offset_size(bounds);
check_buffer_bounds(self.size, offset, size);
check_buffer_bounds(self.shared.size, offset, size);
BufferSlice {
buffer: self,
offset,
Expand All @@ -243,27 +249,27 @@ impl Buffer {

/// Flushes any pending write operations and unmaps the buffer from host memory.
pub fn unmap(&self) {
self.map_context.lock().reset();
self.inner.unmap();
self.shared.map_context.lock().reset();
self.shared.inner.unmap();
}

/// Destroy the associated native resources as soon as possible.
pub fn destroy(&self) {
self.inner.destroy();
self.shared.inner.destroy();
}

/// Returns the length of the buffer allocation in bytes.
///
/// This is always equal to the `size` that was specified when creating the buffer.
pub fn size(&self) -> BufferAddress {
self.size
self.shared.size
}

/// Returns the allowed usages for this `Buffer`.
///
/// This is always equal to the `usage` that was specified when creating the buffer.
pub fn usage(&self) -> BufferUsages {
self.usage
self.shared.usage
}
}

Expand Down Expand Up @@ -330,7 +336,7 @@ impl<'a> BufferSlice<'a> {
mode: MapMode,
callback: impl FnOnce(Result<(), BufferAsyncError>) + WasmNotSend + 'static,
) {
let mut mc = self.buffer.map_context.lock();
let mut mc = self.buffer.shared.map_context.lock();
assert_eq!(mc.initial_range, 0..0, "Buffer is already mapped");
let end = match self.size {
Some(s) => self.offset + s.get(),
Expand All @@ -339,6 +345,7 @@ impl<'a> BufferSlice<'a> {
mc.initial_range = self.offset..end;

self.buffer
.shared
.inner
.map_async(mode, self.offset..end, Box::new(callback));
}
Expand All @@ -358,8 +365,13 @@ impl<'a> BufferSlice<'a> {
///
/// [mapped]: Buffer#mapping-buffers
pub fn get_mapped_range(&self) -> BufferView<'a> {
let end = self.buffer.map_context.lock().add(self.offset, self.size);
let range = self.buffer.inner.get_mapped_range(self.offset..end);
let end = self
.buffer
.shared
.map_context
.lock()
.add(self.offset, self.size);
let range = self.buffer.shared.inner.get_mapped_range(self.offset..end);
BufferView {
slice: *self,
inner: range,
Expand All @@ -376,9 +388,15 @@ impl<'a> BufferSlice<'a> {
/// This is only available on WebGPU, on any other backends this will return `None`.
#[cfg(webgpu)]
pub fn get_mapped_range_as_array_buffer(&self) -> Option<js_sys::ArrayBuffer> {
let end = self.buffer.map_context.lock().add(self.offset, self.size);
let end = self
.buffer
.shared
.map_context
.lock()
.add(self.offset, self.size);

self.buffer
.shared
.inner
.get_mapped_range_as_array_buffer(self.offset..end)
}
Expand All @@ -398,12 +416,17 @@ impl<'a> BufferSlice<'a> {
///
/// [mapped]: Buffer#mapping-buffers
pub fn get_mapped_range_mut(&self) -> BufferViewMut<'a> {
let end = self.buffer.map_context.lock().add(self.offset, self.size);
let range = self.buffer.inner.get_mapped_range(self.offset..end);
let end = self
.buffer
.shared
.map_context
.lock()
.add(self.offset, self.size);
let range = self.buffer.shared.inner.get_mapped_range(self.offset..end);
BufferViewMut {
slice: *self,
inner: range,
readable: self.buffer.usage.contains(BufferUsages::MAP_READ),
readable: self.buffer.shared.usage.contains(BufferUsages::MAP_READ),
}
}
}
Expand Down Expand Up @@ -628,6 +651,7 @@ impl Drop for BufferView<'_> {
fn drop(&mut self) {
self.slice
.buffer
.shared
.map_context
.lock()
.remove(self.slice.offset, self.slice.size);
Expand All @@ -638,6 +662,7 @@ impl Drop for BufferViewMut<'_> {
fn drop(&mut self) {
self.slice
.buffer
.shared
.map_context
.lock()
.remove(self.slice.offset, self.slice.size);
Expand Down
Loading

0 comments on commit 03ff99e

Please sign in to comment.