diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dc805e977..3d04c3c1e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,10 @@ By @teoxoy [#6134](https://github.com/gfx-rs/wgpu/pull/6134). - Reduce the amount of debug and trace logs emitted by wgpu-core and wgpu-hal. By @nical in [#6065](https://github.com/gfx-rs/wgpu/issues/6065) - `Rg11b10Float` is renamed to `Rg11b10UFloat`. By @sagudev in [#6108](https://github.com/gfx-rs/wgpu/pull/6108) +#### HAL + +- Change the inconsistent `DropGuard` based API on Vulkan and GLES to a consistent, callback-based one. By @jerzywilczek in [#6164](https://github.com/gfx-rs/wgpu/pull/6164) + ### Dependency Updates #### GLES diff --git a/wgpu-hal/src/gles/device.rs b/wgpu-hal/src/gles/device.rs index ad092307e9..490f73259b 100644 --- a/wgpu-hal/src/gles/device.rs +++ b/wgpu-hal/src/gles/device.rs @@ -110,22 +110,21 @@ impl super::Device { /// /// - `name` must be created respecting `desc` /// - `name` must be a texture - /// - If `drop_guard` is [`None`], wgpu-hal will take ownership of the texture. If `drop_guard` is - /// [`Some`], the texture must be valid until the drop implementation - /// of the drop guard is called. + /// - If `drop_callback` is [`None`], wgpu-hal will take ownership of the texture. If + /// `drop_callback` is [`Some`], the texture must be valid until the callback is called. #[cfg(any(native, Emscripten))] pub unsafe fn texture_from_raw( &self, name: std::num::NonZeroU32, desc: &crate::TextureDescriptor, - drop_guard: Option, + drop_callback: Option, ) -> super::Texture { super::Texture { inner: super::TextureInner::Texture { raw: glow::NativeTexture(name), target: super::Texture::get_info_from_desc(desc), }, - drop_guard, + drop_guard: crate::DropGuard::from_option(drop_callback), mip_level_count: desc.mip_level_count, array_layer_count: desc.array_layer_count(), format: desc.format, @@ -138,21 +137,20 @@ impl super::Device { /// /// - `name` must be created respecting `desc` /// - `name` must be a renderbuffer - /// - If `drop_guard` is [`None`], wgpu-hal will take ownership of the renderbuffer. If `drop_guard` is - /// [`Some`], the renderbuffer must be valid until the drop implementation - /// of the drop guard is called. + /// - If `drop_callback` is [`None`], wgpu-hal will take ownership of the renderbuffer. If + /// `drop_callback` is [`Some`], the renderbuffer must be valid until the callback is called. #[cfg(any(native, Emscripten))] pub unsafe fn texture_from_raw_renderbuffer( &self, name: std::num::NonZeroU32, desc: &crate::TextureDescriptor, - drop_guard: Option, + drop_callback: Option, ) -> super::Texture { super::Texture { inner: super::TextureInner::Renderbuffer { raw: glow::NativeRenderbuffer(name), }, - drop_guard, + drop_guard: crate::DropGuard::from_option(drop_callback), mip_level_count: desc.mip_level_count, array_layer_count: desc.array_layer_count(), format: desc.format, diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index b62a6b5962..92afa71d43 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -303,8 +303,35 @@ pub type MemoryRange = Range; pub type FenceValue = u64; pub type AtomicFenceValue = std::sync::atomic::AtomicU64; -/// Drop guard to signal wgpu-hal is no longer using an externally created object. -pub type DropGuard = Box; +/// A callback to signal that wgpu is no longer using a resource. +#[cfg(any(gles, vulkan))] +pub type DropCallback = Box; + +#[cfg(any(gles, vulkan))] +pub struct DropGuard { + callback: DropCallback, +} + +#[cfg(all(any(gles, vulkan), any(native, Emscripten)))] +impl DropGuard { + fn from_option(callback: Option) -> Option { + callback.map(|callback| Self { callback }) + } +} + +#[cfg(any(gles, vulkan))] +impl Drop for DropGuard { + fn drop(&mut self) { + (self.callback)(); + } +} + +#[cfg(any(gles, vulkan))] +impl std::fmt::Debug for DropGuard { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("DropGuard").finish() + } +} #[derive(Clone, Debug, PartialEq, Eq, Error)] pub enum DeviceError { diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index edeeca5e91..25d0d75049 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -1603,11 +1603,13 @@ impl super::Adapter { /// - `raw_device` must be created from this adapter. /// - `raw_device` must be created using `family_index`, `enabled_extensions` and `physical_device_features()` /// - `enabled_extensions` must be a superset of `required_device_extensions()`. + /// - If `drop_callback` is [`None`], wgpu-hal will take ownership of `raw_device`. If + /// `drop_callback` is [`Some`], `raw_device` must be valid until the callback is called. #[allow(clippy::too_many_arguments)] pub unsafe fn device_from_raw( &self, raw_device: ash::Device, - handle_is_owned: bool, + drop_callback: Option, enabled_extensions: &[&'static CStr], features: wgt::Features, memory_hints: &wgt::MemoryHints, @@ -1822,12 +1824,14 @@ impl super::Adapter { 0, 0, 0, 0, ]; + let drop_guard = crate::DropGuard::from_option(drop_callback); + let shared = Arc::new(super::DeviceShared { raw: raw_device, family_index, queue_index, raw_queue, - handle_is_owned, + drop_guard, instance: Arc::clone(&self.instance), physical_device: self.raw, enabled_extensions: enabled_extensions.into(), @@ -2015,7 +2019,7 @@ impl crate::Adapter for super::Adapter { unsafe { self.device_from_raw( raw_device, - true, + None, &enabled_extensions, features, memory_hints, diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index 6c9ddfe807..4283f844a9 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -290,7 +290,7 @@ impl super::DeviceShared { for &raw in self.framebuffers.lock().values() { unsafe { self.raw.destroy_framebuffer(raw, None) }; } - if self.handle_is_owned { + if self.drop_guard.is_none() { unsafe { self.raw.destroy_device(None) }; } } @@ -649,13 +649,13 @@ impl super::Device { /// # Safety /// /// - `vk_image` must be created respecting `desc` - /// - If `drop_guard` is `Some`, the application must manually destroy the image handle. This - /// can be done inside the `Drop` impl of `drop_guard`. + /// - If `drop_callback` is [`None`], wgpu-hal will take ownership of `vk_image`. If + /// `drop_callback` is [`Some`], `vk_image` must be valid until the callback is called. /// - If the `ImageCreateFlags` does not contain `MUTABLE_FORMAT`, the `view_formats` of `desc` must be empty. pub unsafe fn texture_from_raw( vk_image: vk::Image, desc: &crate::TextureDescriptor, - drop_guard: Option, + drop_callback: Option, ) -> super::Texture { let mut raw_flags = vk::ImageCreateFlags::empty(); let mut view_formats = vec![]; @@ -674,6 +674,8 @@ impl super::Device { raw_flags |= vk::ImageCreateFlags::MUTABLE_FORMAT; } + let drop_guard = crate::DropGuard::from_option(drop_callback); + super::Texture { raw: vk_image, drop_guard, diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 832c74b030..124fd0abcc 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -310,6 +310,8 @@ impl super::Instance { /// - `extensions` must be a superset of `desired_extensions()` and must be created from the /// same entry, `instance_api_version`` and flags. /// - `android_sdk_version` is ignored and can be `0` for all platforms besides Android + /// - If `drop_callback` is [`None`], wgpu-hal will take ownership of `raw_instance`. If + /// `drop_callback` is [`Some`], `raw_instance` must be valid until the callback is called. /// /// If `debug_utils_user_data` is `Some`, then the validation layer is /// available, so create a [`vk::DebugUtilsMessengerEXT`]. @@ -323,7 +325,7 @@ impl super::Instance { extensions: Vec<&'static CStr>, flags: wgt::InstanceFlags, has_nv_optimus: bool, - drop_guard: Option, + drop_callback: Option, ) -> Result { log::debug!("Instance version: 0x{:x}", instance_api_version); @@ -364,6 +366,8 @@ impl super::Instance { None }; + let drop_guard = crate::DropGuard::from_option(drop_callback); + Ok(Self { shared: Arc::new(super::InstanceShared { raw: raw_instance, @@ -555,7 +559,7 @@ impl Drop for super::InstanceShared { .destroy_debug_utils_messenger(du.messenger, None); du }); - if let Some(_drop_guard) = self.drop_guard.take() { + if self.drop_guard.is_none() { self.raw.destroy_instance(None); } } @@ -829,7 +833,7 @@ impl crate::Instance for super::Instance { extensions, desc.flags, has_nv_optimus, - Some(Box::new(())), // `Some` signals that wgpu-hal is in charge of destroying vk_instance + None, ) } } diff --git a/wgpu-hal/src/vulkan/mod.rs b/wgpu-hal/src/vulkan/mod.rs index 9e57f77d03..5ab214e0c7 100644 --- a/wgpu-hal/src/vulkan/mod.rs +++ b/wgpu-hal/src/vulkan/mod.rs @@ -595,7 +595,7 @@ struct DeviceShared { family_index: u32, queue_index: u32, raw_queue: vk::Queue, - handle_is_owned: bool, + drop_guard: Option, instance: Arc, physical_device: vk::PhysicalDevice, enabled_extensions: Vec<&'static CStr>,