Skip to content

Commit

Permalink
remove hal::Device::exit, add Drop implementations to Device an…
Browse files Browse the repository at this point in the history
…d `Queue` instead
  • Loading branch information
teoxoy committed Nov 13, 2024
1 parent 7a44aeb commit 2635531
Show file tree
Hide file tree
Showing 14 changed files with 62 additions and 72 deletions.
8 changes: 2 additions & 6 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use thiserror::Error;
use super::{life::LifetimeTracker, Device};

pub struct Queue {
raw: ManuallyDrop<Box<dyn hal::DynQueue>>,
raw: Box<dyn hal::DynQueue>,
pub(crate) device: Arc<Device>,
pub(crate) pending_writes: Mutex<ManuallyDrop<PendingWrites>>,
life_tracker: Mutex<LifetimeTracker>,
Expand All @@ -60,7 +60,6 @@ impl Queue {
let pending_encoder = match pending_encoder {
Ok(pending_encoder) => pending_encoder,
Err(e) => {
device.release_queue(raw);
return Err(e);
}
};
Expand Down Expand Up @@ -93,7 +92,7 @@ impl Queue {
);

Ok(Queue {
raw: ManuallyDrop::new(raw),
raw,
device,
pending_writes,
life_tracker: Mutex::new(rank::QUEUE_LIFE_TRACKER, LifetimeTracker::new()),
Expand Down Expand Up @@ -198,9 +197,6 @@ impl Drop for Queue {
// SAFETY: We are in the Drop impl and we don't use self.pending_writes anymore after this point.
let pending_writes = unsafe { ManuallyDrop::take(&mut self.pending_writes.lock()) };
pending_writes.dispose(self.device.raw());
// SAFETY: we never access `self.raw` beyond this point.
let queue = unsafe { ManuallyDrop::take(&mut self.raw) };
self.device.release_queue(queue);

closures.fire();
}
Expand Down
22 changes: 6 additions & 16 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,9 @@ use super::{
/// Structure describing a logical device. Some members are internally mutable,
/// stored behind mutexes.
pub struct Device {
raw: ManuallyDrop<Box<dyn hal::DynDevice>>,
raw: Box<dyn hal::DynDevice>,
pub(crate) adapter: Arc<Adapter>,
pub(crate) queue: OnceLock<Weak<Queue>>,
queue_to_drop: OnceLock<Box<dyn hal::DynQueue>>,
pub(crate) zero_buffer: ManuallyDrop<Box<dyn hal::DynBuffer>>,
/// The `label` from the descriptor used to create the resource.
label: String,
Expand Down Expand Up @@ -154,23 +153,19 @@ impl Drop for Device {
closure.call(DeviceLostReason::Dropped, String::from("Device dropped."));
}

// SAFETY: We are in the Drop impl and we don't use self.raw anymore after this point.
let raw = unsafe { ManuallyDrop::take(&mut self.raw) };
// SAFETY: We are in the Drop impl and we don't use self.zero_buffer anymore after this point.
let zero_buffer = unsafe { ManuallyDrop::take(&mut self.zero_buffer) };
// SAFETY: We are in the Drop impl and we don't use self.fence anymore after this point.
let fence = unsafe { ManuallyDrop::take(&mut self.fence.write()) };
self.command_allocator.dispose(raw.as_ref());
self.command_allocator.dispose(self.raw.as_ref());
#[cfg(feature = "indirect-validation")]
self.indirect_validation
.take()
.unwrap()
.dispose(raw.as_ref());
.dispose(self.raw.as_ref());
unsafe {
raw.destroy_buffer(zero_buffer);
raw.destroy_fence(fence);
let queue = self.queue_to_drop.take().unwrap();
raw.exit(queue);
self.raw.destroy_buffer(zero_buffer);
self.raw.destroy_fence(fence);
}
}
}
Expand Down Expand Up @@ -249,10 +244,9 @@ impl Device {
};

Ok(Self {
raw: ManuallyDrop::new(raw_device),
raw: raw_device,
adapter: adapter.clone(),
queue: OnceLock::new(),
queue_to_drop: OnceLock::new(),
zero_buffer: ManuallyDrop::new(zero_buffer),
label: desc.label.to_string(),
command_allocator,
Expand Down Expand Up @@ -325,10 +319,6 @@ impl Device {
DeviceError::from_hal(error)
}

pub(crate) fn release_queue(&self, queue: Box<dyn hal::DynQueue>) {
assert!(self.queue_to_drop.set(queue).is_ok());
}

/// Run some destroy operations that were deferred.
///
/// Destroying the resources requires taking a write lock on the device's snatch lock,
Expand Down
3 changes: 2 additions & 1 deletion wgpu-hal/examples/halmark/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,8 @@ impl<A: hal::Api> Example<A> {
self.device.destroy_pipeline_layout(self.pipeline_layout);

self.surface.unconfigure(&self.device);
self.device.exit(self.queue);
drop(self.queue);
drop(self.device);
drop(self.surface);
drop(self.adapter);
}
Expand Down
3 changes: 2 additions & 1 deletion wgpu-hal/examples/ray-traced-triangle/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,8 @@ impl<A: hal::Api> Example<A> {
self.device.destroy_shader_module(self.shader_module);

self.surface.unconfigure(&self.device);
self.device.exit(self.queue);
drop(self.queue);
drop(self.device);
drop(self.surface);
drop(self.adapter);
}
Expand Down
4 changes: 0 additions & 4 deletions wgpu-hal/src/dx12/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,10 +391,6 @@ impl super::Device {
impl crate::Device for super::Device {
type A = super::Api;

unsafe fn exit(self, _queue: super::Queue) {
self.rtv_pool.lock().free_handle(self.null_rtv_handle);
}

unsafe fn create_buffer(
&self,
desc: &crate::BufferDescriptor,
Expand Down
6 changes: 6 additions & 0 deletions wgpu-hal/src/dx12/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,12 @@ pub struct Device {
counters: wgt::HalCounters,
}

impl Drop for Device {
fn drop(&mut self) {
self.rtv_pool.lock().free_handle(self.null_rtv_handle);
}
}

unsafe impl Send for Device {}
unsafe impl Sync for Device {}

Expand Down
6 changes: 0 additions & 6 deletions wgpu-hal/src/dynamic/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ use super::{
};

pub trait DynDevice: DynResource {
unsafe fn exit(self: Box<Self>, queue: Box<dyn DynQueue>);

unsafe fn create_buffer(
&self,
desc: &BufferDescriptor,
Expand Down Expand Up @@ -166,10 +164,6 @@ pub trait DynDevice: DynResource {
}

impl<D: Device + DynResource> DynDevice for D {
unsafe fn exit(self: Box<Self>, queue: Box<dyn DynQueue>) {
unsafe { D::exit(*self, queue.unbox()) }
}

unsafe fn create_buffer(
&self,
desc: &BufferDescriptor,
Expand Down
1 change: 0 additions & 1 deletion wgpu-hal/src/empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ impl crate::Queue for Context {
impl crate::Device for Context {
type A = Api;

unsafe fn exit(self, queue: Context) {}
unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<Resource> {
Ok(Resource)
}
Expand Down
8 changes: 0 additions & 8 deletions wgpu-hal/src/gles/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,14 +501,6 @@ impl super::Device {
impl crate::Device for super::Device {
type A = super::Api;

unsafe fn exit(self, queue: super::Queue) {
let gl = &self.shared.context.lock();
unsafe { gl.delete_vertex_array(self.main_vao) };
unsafe { gl.delete_framebuffer(queue.draw_fbo) };
unsafe { gl.delete_framebuffer(queue.copy_fbo) };
unsafe { gl.delete_buffer(queue.zero_buffer) };
}

unsafe fn create_buffer(
&self,
desc: &crate::BufferDescriptor,
Expand Down
16 changes: 16 additions & 0 deletions wgpu-hal/src/gles/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,13 @@ pub struct Device {
counters: wgt::HalCounters,
}

impl Drop for Device {
fn drop(&mut self) {
let gl = &self.shared.context.lock();
unsafe { gl.delete_vertex_array(self.main_vao) };
}
}

pub struct ShaderClearProgram {
pub program: glow::Program,
pub color_uniform_location: glow::UniformLocation,
Expand All @@ -316,6 +323,15 @@ pub struct Queue {
current_index_buffer: Mutex<Option<glow::Buffer>>,
}

impl Drop for Queue {
fn drop(&mut self) {
let gl = &self.shared.context.lock();
unsafe { gl.delete_framebuffer(self.draw_fbo) };
unsafe { gl.delete_framebuffer(self.copy_fbo) };
unsafe { gl.delete_buffer(self.zero_buffer) };
}
}

#[derive(Clone, Debug)]
pub struct Buffer {
raw: Option<glow::Buffer>,
Expand Down
4 changes: 1 addition & 3 deletions wgpu-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ pub trait Adapter: WasmNotSendSync {
/// 1) Free resources with methods like [`Device::destroy_texture`] or
/// [`Device::destroy_shader_module`].
///
/// 1) Shut down the device by calling [`Device::exit`].
/// 1) Drop the device.
///
/// [`vkDevice`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VkDevice
/// [`ID3D12Device`]: https://learn.microsoft.com/en-us/windows/win32/api/d3d12/nn-d3d12-id3d12device
Expand Down Expand Up @@ -706,8 +706,6 @@ pub trait Adapter: WasmNotSendSync {
pub trait Device: WasmNotSendSync {
type A: Api;

/// Exit connection to this logical device.
unsafe fn exit(self, queue: <Self::A as Api>::Queue);
/// Creates a new buffer.
///
/// The initial usage is `BufferUses::empty()`.
Expand Down
2 changes: 0 additions & 2 deletions wgpu-hal/src/metal/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,6 @@ impl super::Device {
impl crate::Device for super::Device {
type A = super::Api;

unsafe fn exit(self, _queue: super::Queue) {}

unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<super::Buffer> {
let map_read = desc.usage.contains(crate::BufferUses::MAP_READ);
let map_write = desc.usage.contains(crate::BufferUses::MAP_WRITE);
Expand Down
24 changes: 0 additions & 24 deletions wgpu-hal/src/vulkan/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,18 +289,6 @@ impl super::DeviceShared {
.size((range.end - range.start + mask) & !mask)
}))
}

unsafe fn free_resources(&self) {
for &raw in self.render_passes.lock().values() {
unsafe { self.raw.destroy_render_pass(raw, None) };
}
for &raw in self.framebuffers.lock().values() {
unsafe { self.raw.destroy_framebuffer(raw, None) };
}
if self.drop_guard.is_none() {
unsafe { self.raw.destroy_device(None) };
}
}
}

impl gpu_alloc::MemoryDevice<vk::DeviceMemory> for super::DeviceShared {
Expand Down Expand Up @@ -1023,18 +1011,6 @@ impl super::Device {
impl crate::Device for super::Device {
type A = super::Api;

unsafe fn exit(self, queue: super::Queue) {
unsafe { self.mem_allocator.into_inner().cleanup(&*self.shared) };
unsafe { self.desc_allocator.into_inner().cleanup(&*self.shared) };
unsafe {
queue
.relay_semaphores
.into_inner()
.destroy(&self.shared.raw)
};
unsafe { self.shared.free_resources() };
}

unsafe fn create_buffer(
&self,
desc: &crate::BufferDescriptor,
Expand Down
27 changes: 27 additions & 0 deletions wgpu-hal/src/vulkan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,20 @@ struct DeviceShared {
memory_allocations_counter: InternalCounter,
}

impl Drop for DeviceShared {
fn drop(&mut self) {
for &raw in self.render_passes.lock().values() {
unsafe { self.raw.destroy_render_pass(raw, None) };
}
for &raw in self.framebuffers.lock().values() {
unsafe { self.raw.destroy_framebuffer(raw, None) };
}
if self.drop_guard.is_none() {
unsafe { self.raw.destroy_device(None) };
}
}
}

pub struct Device {
shared: Arc<DeviceShared>,
mem_allocator: Mutex<gpu_alloc::GpuAllocator<vk::DeviceMemory>>,
Expand All @@ -658,6 +672,13 @@ pub struct Device {
counters: wgt::HalCounters,
}

impl Drop for Device {
fn drop(&mut self) {
unsafe { self.mem_allocator.lock().cleanup(&*self.shared) };
unsafe { self.desc_allocator.lock().cleanup(&*self.shared) };
}
}

/// Semaphores for forcing queue submissions to run in order.
///
/// The [`wgpu_hal::Queue`] trait promises that if two calls to [`submit`] are
Expand Down Expand Up @@ -741,6 +762,12 @@ pub struct Queue {
relay_semaphores: Mutex<RelaySemaphores>,
}

impl Drop for Queue {
fn drop(&mut self) {
unsafe { self.relay_semaphores.lock().destroy(&self.device.raw) };
}
}

#[derive(Debug)]
pub struct Buffer {
raw: vk::Buffer,
Expand Down

0 comments on commit 2635531

Please sign in to comment.