remove hal::Device::exit, add Drop implementations to Device and Queue instead

This commit is contained in:
teoxoy 2024-11-07 12:13:25 +01:00 committed by Teodor Tanasoaia
parent 394bf37f98
commit d489e4c2e8
14 changed files with 62 additions and 72 deletions

View File

@ -41,7 +41,7 @@ use thiserror::Error;
use super::{life::LifetimeTracker, Device}; use super::{life::LifetimeTracker, Device};
pub struct Queue { pub struct Queue {
raw: ManuallyDrop<Box<dyn hal::DynQueue>>, raw: Box<dyn hal::DynQueue>,
pub(crate) device: Arc<Device>, pub(crate) device: Arc<Device>,
pub(crate) pending_writes: Mutex<ManuallyDrop<PendingWrites>>, pub(crate) pending_writes: Mutex<ManuallyDrop<PendingWrites>>,
life_tracker: Mutex<LifetimeTracker>, life_tracker: Mutex<LifetimeTracker>,
@ -60,7 +60,6 @@ impl Queue {
let pending_encoder = match pending_encoder { let pending_encoder = match pending_encoder {
Ok(pending_encoder) => pending_encoder, Ok(pending_encoder) => pending_encoder,
Err(e) => { Err(e) => {
device.release_queue(raw);
return Err(e); return Err(e);
} }
}; };
@ -93,7 +92,7 @@ impl Queue {
); );
Ok(Queue { Ok(Queue {
raw: ManuallyDrop::new(raw), raw,
device, device,
pending_writes, pending_writes,
life_tracker: Mutex::new(rank::QUEUE_LIFE_TRACKER, LifetimeTracker::new()), life_tracker: Mutex::new(rank::QUEUE_LIFE_TRACKER, LifetimeTracker::new()),
@ -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. // 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()) }; let pending_writes = unsafe { ManuallyDrop::take(&mut self.pending_writes.lock()) };
pending_writes.dispose(self.device.raw()); 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(); closures.fire();
} }

View File

@ -57,10 +57,9 @@ use super::{
/// Structure describing a logical device. Some members are internally mutable, /// Structure describing a logical device. Some members are internally mutable,
/// stored behind mutexes. /// stored behind mutexes.
pub struct Device { pub struct Device {
raw: ManuallyDrop<Box<dyn hal::DynDevice>>, raw: Box<dyn hal::DynDevice>,
pub(crate) adapter: Arc<Adapter>, pub(crate) adapter: Arc<Adapter>,
pub(crate) queue: OnceLock<Weak<Queue>>, pub(crate) queue: OnceLock<Weak<Queue>>,
queue_to_drop: OnceLock<Box<dyn hal::DynQueue>>,
pub(crate) zero_buffer: ManuallyDrop<Box<dyn hal::DynBuffer>>, pub(crate) zero_buffer: ManuallyDrop<Box<dyn hal::DynBuffer>>,
/// The `label` from the descriptor used to create the resource. /// The `label` from the descriptor used to create the resource.
label: String, label: String,
@ -154,23 +153,19 @@ impl Drop for Device {
closure.call(DeviceLostReason::Dropped, String::from("Device dropped.")); 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. // 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) }; 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. // 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()) }; 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")] #[cfg(feature = "indirect-validation")]
self.indirect_validation self.indirect_validation
.take() .take()
.unwrap() .unwrap()
.dispose(raw.as_ref()); .dispose(self.raw.as_ref());
unsafe { unsafe {
raw.destroy_buffer(zero_buffer); self.raw.destroy_buffer(zero_buffer);
raw.destroy_fence(fence); self.raw.destroy_fence(fence);
let queue = self.queue_to_drop.take().unwrap();
raw.exit(queue);
} }
} }
} }
@ -249,10 +244,9 @@ impl Device {
}; };
Ok(Self { Ok(Self {
raw: ManuallyDrop::new(raw_device), raw: raw_device,
adapter: adapter.clone(), adapter: adapter.clone(),
queue: OnceLock::new(), queue: OnceLock::new(),
queue_to_drop: OnceLock::new(),
zero_buffer: ManuallyDrop::new(zero_buffer), zero_buffer: ManuallyDrop::new(zero_buffer),
label: desc.label.to_string(), label: desc.label.to_string(),
command_allocator, command_allocator,
@ -325,10 +319,6 @@ impl Device {
DeviceError::from_hal(error) 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. /// Run some destroy operations that were deferred.
/// ///
/// Destroying the resources requires taking a write lock on the device's snatch lock, /// Destroying the resources requires taking a write lock on the device's snatch lock,

View File

@ -579,7 +579,8 @@ impl<A: hal::Api> Example<A> {
self.device.destroy_pipeline_layout(self.pipeline_layout); self.device.destroy_pipeline_layout(self.pipeline_layout);
self.surface.unconfigure(&self.device); self.surface.unconfigure(&self.device);
self.device.exit(self.queue); drop(self.queue);
drop(self.device);
drop(self.surface); drop(self.surface);
drop(self.adapter); drop(self.adapter);
} }

View File

@ -1068,7 +1068,8 @@ impl<A: hal::Api> Example<A> {
self.device.destroy_shader_module(self.shader_module); self.device.destroy_shader_module(self.shader_module);
self.surface.unconfigure(&self.device); self.surface.unconfigure(&self.device);
self.device.exit(self.queue); drop(self.queue);
drop(self.device);
drop(self.surface); drop(self.surface);
drop(self.adapter); drop(self.adapter);
} }

View File

@ -391,10 +391,6 @@ impl super::Device {
impl crate::Device for super::Device { impl crate::Device for super::Device {
type A = super::Api; 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( unsafe fn create_buffer(
&self, &self,
desc: &crate::BufferDescriptor, desc: &crate::BufferDescriptor,

View File

@ -602,6 +602,12 @@ pub struct Device {
counters: wgt::HalCounters, 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 Send for Device {}
unsafe impl Sync for Device {} unsafe impl Sync for Device {}

View File

@ -16,8 +16,6 @@ use super::{
}; };
pub trait DynDevice: DynResource { pub trait DynDevice: DynResource {
unsafe fn exit(self: Box<Self>, queue: Box<dyn DynQueue>);
unsafe fn create_buffer( unsafe fn create_buffer(
&self, &self,
desc: &BufferDescriptor, desc: &BufferDescriptor,
@ -166,10 +164,6 @@ pub trait DynDevice: DynResource {
} }
impl<D: Device + DynResource> DynDevice for D { 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( unsafe fn create_buffer(
&self, &self,
desc: &BufferDescriptor, desc: &BufferDescriptor,

View File

@ -163,7 +163,6 @@ impl crate::Queue for Context {
impl crate::Device for Context { impl crate::Device for Context {
type A = Api; type A = Api;
unsafe fn exit(self, queue: Context) {}
unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<Resource> { unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<Resource> {
Ok(Resource) Ok(Resource)
} }

View File

@ -501,14 +501,6 @@ impl super::Device {
impl crate::Device for super::Device { impl crate::Device for super::Device {
type A = super::Api; 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( unsafe fn create_buffer(
&self, &self,
desc: &crate::BufferDescriptor, desc: &crate::BufferDescriptor,

View File

@ -295,6 +295,13 @@ pub struct Device {
counters: wgt::HalCounters, 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 struct ShaderClearProgram {
pub program: glow::Program, pub program: glow::Program,
pub color_uniform_location: glow::UniformLocation, pub color_uniform_location: glow::UniformLocation,
@ -316,6 +323,15 @@ pub struct Queue {
current_index_buffer: Mutex<Option<glow::Buffer>>, 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)] #[derive(Clone, Debug)]
pub struct Buffer { pub struct Buffer {
raw: Option<glow::Buffer>, raw: Option<glow::Buffer>,

View File

@ -676,7 +676,7 @@ pub trait Adapter: WasmNotSendSync {
/// 1) Free resources with methods like [`Device::destroy_texture`] or /// 1) Free resources with methods like [`Device::destroy_texture`] or
/// [`Device::destroy_shader_module`]. /// [`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 /// [`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 /// [`ID3D12Device`]: https://learn.microsoft.com/en-us/windows/win32/api/d3d12/nn-d3d12-id3d12device
@ -706,8 +706,6 @@ pub trait Adapter: WasmNotSendSync {
pub trait Device: WasmNotSendSync { pub trait Device: WasmNotSendSync {
type A: Api; type A: Api;
/// Exit connection to this logical device.
unsafe fn exit(self, queue: <Self::A as Api>::Queue);
/// Creates a new buffer. /// Creates a new buffer.
/// ///
/// The initial usage is `BufferUses::empty()`. /// The initial usage is `BufferUses::empty()`.

View File

@ -320,8 +320,6 @@ impl super::Device {
impl crate::Device for super::Device { impl crate::Device for super::Device {
type A = super::Api; type A = super::Api;
unsafe fn exit(self, _queue: super::Queue) {}
unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<super::Buffer> { unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<super::Buffer> {
let map_read = desc.usage.contains(crate::BufferUses::MAP_READ); let map_read = desc.usage.contains(crate::BufferUses::MAP_READ);
let map_write = desc.usage.contains(crate::BufferUses::MAP_WRITE); let map_write = desc.usage.contains(crate::BufferUses::MAP_WRITE);

View File

@ -289,18 +289,6 @@ impl super::DeviceShared {
.size((range.end - range.start + mask) & !mask) .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 { impl gpu_alloc::MemoryDevice<vk::DeviceMemory> for super::DeviceShared {
@ -1023,18 +1011,6 @@ impl super::Device {
impl crate::Device for super::Device { impl crate::Device for super::Device {
type A = super::Api; 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( unsafe fn create_buffer(
&self, &self,
desc: &crate::BufferDescriptor, desc: &crate::BufferDescriptor,

View File

@ -646,6 +646,20 @@ struct DeviceShared {
memory_allocations_counter: InternalCounter, 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 { pub struct Device {
shared: Arc<DeviceShared>, shared: Arc<DeviceShared>,
mem_allocator: Mutex<gpu_alloc::GpuAllocator<vk::DeviceMemory>>, mem_allocator: Mutex<gpu_alloc::GpuAllocator<vk::DeviceMemory>>,
@ -658,6 +672,13 @@ pub struct Device {
counters: wgt::HalCounters, 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. /// Semaphores for forcing queue submissions to run in order.
/// ///
/// The [`wgpu_hal::Queue`] trait promises that if two calls to [`submit`] are /// The [`wgpu_hal::Queue`] trait promises that if two calls to [`submit`] are
@ -741,6 +762,12 @@ pub struct Queue {
relay_semaphores: Mutex<RelaySemaphores>, relay_semaphores: Mutex<RelaySemaphores>,
} }
impl Drop for Queue {
fn drop(&mut self) {
unsafe { self.relay_semaphores.lock().destroy(&self.device.raw) };
}
}
#[derive(Debug)] #[derive(Debug)]
pub struct Buffer { pub struct Buffer {
raw: vk::Buffer, raw: vk::Buffer,