fence refactor

crates/renderer/src/commands.rs

@@ -277,7 +277,7 @@ impl SingleUseCommand {
         signal: Option<vk::Semaphore>,
         fence: Arc<sync::Fence>,
     ) -> VkResult<FenceFuture<'a>> {
-        self.submit_fence(wait, signal, Some(fence.fence()))?;
+        self.submit_fence(wait, signal, Some(fence.raw()))?;
 
         Ok(FenceFuture::new(fence))
     }
@@ -287,8 +287,8 @@ impl SingleUseCommand {
         self,
         wait: Option<(vk::Semaphore, vk::PipelineStageFlags)>,
         signal: Option<vk::Semaphore>,
-    ) -> VkResult<()> {
+    ) -> crate::Result<()> {
-        let fence = Arc::new(sync::Fence::create(self.device().clone())?);
+        let fence = Arc::new(sync::Fence::new_pooled(&self.device().pools.fences, None)?);
         let future = self.submit_async(wait, signal, fence)?;
         future.block()?;
         Ok(())

crates/renderer/src/device.rs

@@ -1,7 +1,9 @@
 use std::{
     borrow::Cow,
+    cell::UnsafeCell,
     collections::{BTreeSet, HashMap, HashSet},
     ffi::CStr,
+    mem::{ManuallyDrop, MaybeUninit},
     ops::Deref,
     sync::Arc,
 };
@@ -11,6 +13,7 @@ use ash::{
     prelude::VkResult,
     vk::{self, Handle},
 };
+use parking_lot::Mutex;
 use raw_window_handle::RawDisplayHandle;
 use tinyvec::{ArrayVec, array_vec};
 
@@ -405,7 +408,13 @@ impl PhysicalDeviceInfo {
             _drop: DeviceDrop(device),
         };
 
-        Ok(Device(Arc::new(inner)))
+        let shared = Arc::new(inner);
+        Ok(Device {
+            pools: Arc::new(DevicePools {
+                fences: Pool::new(shared.clone()),
+            }),
+            shared,
+        })
     }
 
     fn required_extensions(&self, requested_extensions: &[Extension<'static>]) -> Vec<*const i8> {
@@ -434,11 +443,19 @@ impl PhysicalDeviceInfo {
 }
 
 #[derive(Clone, Debug)]
-pub struct Device(Arc<DeviceInner>);
+pub(crate) struct DevicePools {
+    pub(crate) fences: Pool<vk::Fence>,
+}
+
+#[derive(Clone, Debug)]
+pub struct Device {
+    pub(crate) shared: Arc<DeviceInner>,
+    pub(crate) pools: Arc<DevicePools>,
+}
 
 impl PartialEq for Device {
     fn eq(&self, other: &Self) -> bool {
-        Arc::ptr_eq(&self.0, &other.0)
+        Arc::ptr_eq(&self.shared, &other.shared)
     }
 }
 
@@ -448,57 +465,57 @@ impl core::ops::Deref for Device {
     type Target = DeviceInner;
 
     fn deref(&self) -> &Self::Target {
-        &self.0
+        &self.shared
     }
 }
 
-impl Device {
+impl DeviceInner {
     pub fn sync_threadpool(&self) -> &sync::SyncThreadpool {
-        &self.0.sync_threadpool
+        &self.sync_threadpool
     }
     pub fn alloc(&self) -> &vk_mem::Allocator {
-        &self.0.alloc
+        &self.alloc
     }
     pub fn dev(&self) -> &ash::Device {
-        &self.0.raw
+        &self.raw
     }
     pub fn instance(&self) -> &Instance {
-        &self.0.instance
+        &self.instance
     }
     pub fn queues(&self) -> &DeviceQueues {
-        &self.0.queues
+        &self.queues
     }
     pub fn phy(&self) -> vk::PhysicalDevice {
-        self.0.adapter.pdev
+        self.adapter.pdev
     }
     pub fn features(&self) -> &crate::PhysicalDeviceFeatures {
-        &self.0.adapter.features
+        &self.adapter.features
     }
     pub fn properties(&self) -> &crate::PhysicalDeviceProperties {
-        &self.0.adapter.properties
+        &self.adapter.properties
     }
     pub fn physical_device(&self) -> &PhysicalDeviceInfo {
-        &self.0.adapter
+        &self.adapter
     }
     pub fn main_queue(&self) -> &Queue {
-        self.0.queues.graphics()
+        self.queues.graphics()
     }
     pub fn compute_queue(&self) -> &Queue {
-        self.0.queues.compute()
+        self.queues.compute()
     }
     pub fn transfer_queue(&self) -> &Queue {
-        self.0.queues.transfer()
+        self.queues.transfer()
     }
 
     pub unsafe fn lock_queues(&self) {
         unsafe {
-            self.0.queues.lock();
+            self.queues.lock();
         }
     }
 
     pub unsafe fn unlock_queues(&self) {
         unsafe {
-            self.0.queues.unlock();
+            self.queues.unlock();
         }
     }
 
@@ -515,7 +532,7 @@ impl Device {
         tracing::warn!("locking all queues and waiting for device to idle");
         unsafe {
             self.lock_queues();
-            self.dev().device_wait_idle()?;
+            self.raw.device_wait_idle()?;
             self.unlock_queues();
         }
         tracing::warn!("finished waiting: unlocking all queues.");
@@ -540,7 +557,9 @@ impl Device {
         let mut buffer = [0u8; 64];
         let buffer_vec: Vec<u8>;
 
-        let name_bytes = if name.len() < buffer.len() {
+        let name_bytes = if name.is_empty() {
+            &[]
+        } else if name.len() < buffer.len() {
             buffer[..name.len()].copy_from_slice(name.as_bytes());
             &buffer[..]
         } else {
@@ -733,7 +752,89 @@ pub trait DeviceOwned<T> {
     fn handle(&self) -> T;
 }
 
-/// Macro for helping create and destroy Vulkan objects which are owned by a device.
+pub trait Pooled: vk::Handle + Sized {
+    fn create_from_pool(pool: &Pool<Self>) -> Result<Self>;
+}
+
+pub struct PoolObject<T: Pooled + Clone> {
+    inner: ManuallyDrop<T>,
+    pool: Pool<T>,
+    #[cfg(debug_assertions)]
+    name: Option<Cow<'static, str>>,
+}
+
+impl<T: Pooled + Clone> PoolObject<T> {
+    pub fn name_object(&mut self, name: impl Into<Cow<'static, str>>) {
+        #[cfg(debug_assertions)]
+        unsafe {
+            self.name = Some(name.into());
+            self.pool
+                .device
+                .debug_name_object(T::clone(&self.inner), self.name.as_ref().unwrap());
+        }
+    }
+
+    pub fn device(&self) -> &Arc<DeviceInner> {
+        &self.pool.device
+    }
+}
+
+impl<T: Pooled + Clone> Drop for PoolObject<T> {
+    fn drop(&mut self) {
+        let handle = unsafe { ManuallyDrop::take(&mut self.inner) };
+        #[cfg(debug_assertions)]
+        if self.name.is_some() {
+            unsafe { self.pool.device.debug_name_object(handle.clone(), "") };
+        }
+
+        self.pool.push(handle);
+    }
+}
+
+impl<T: Pooled + Clone> Deref for PoolObject<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.inner
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct Pool<T> {
+    pub(crate) pool: Arc<Mutex<Vec<T>>>,
+    pub(crate) device: Arc<DeviceInner>,
+}
+
+impl<T> Pool<T> {
+    pub fn push(&self, item: T) {
+        self.pool.lock().push(item);
+    }
+    pub fn new(device: Arc<DeviceInner>) -> Self {
+        Self {
+            pool: Arc::new(Mutex::new(Vec::new())),
+            device,
+        }
+    }
+}
+
+impl<T: Pooled + Clone> Pool<T> {
+    pub fn get(&self) -> Result<PoolObject<T>> {
+        let item = if let Some(item) = self.pool.lock().pop() {
+            item
+        } else {
+            T::create_from_pool(self)?
+        };
+
+        Ok(PoolObject {
+            inner: ManuallyDrop::new(item),
+            pool: self.clone(),
+            #[cfg(debug_assertions)]
+            name: None,
+        })
+    }
+}
+
+// Macro for helping create and destroy Vulkan objects which are owned by a device.
 #[macro_export]
 macro_rules! define_device_owned_handle {
     ($(#[$attr:meta])*

crates/renderer/src/images.rs

@@ -6,7 +6,7 @@ use std::{
 
 use crate::{
     define_device_owned_handle,
-    device::{DeviceOwned, QueueFlags},
+    device::{DeviceObject, DeviceOwned, QueueFlags},
 };
 
 use super::Device;
@@ -123,6 +123,16 @@ impl Default for ImageDesc {
     }
 }
 
+enum ImageInner {
+    Swapchain(vk::Image),
+    Allocated(vk::Image, vk_mem::Allocation),
+}
+
+pub struct Image2 {
+    image: DeviceObject<vk::Image>,
+    alloc: Option<vk_mem::Allocation>,
+}
+
 define_device_owned_handle! {
     #[derive(Debug)]
     pub Image(vk::Image) {

crates/renderer/src/lib.rs

@@ -1011,7 +1011,7 @@ impl Renderer2 {
         let future = cmds.submit(
             Some((frame.acquire, vk::PipelineStageFlags::TRANSFER)),
             Some(frame.release),
-            Arc::new(sync::Fence::create(self.device.clone())?),
+            Arc::new(sync::Fence::new_pooled(&self.device.pools.fences, None)?),
         )?;
 
         future.await;

crates/renderer/src/swapchain.rs

@@ -20,7 +20,7 @@ use crate::{
     device::{Device, DeviceObject, DeviceOwned},
     images,
     instance::InstanceInner,
-    sync,
+    sync::{self, Fence},
     util::RawMutexGuard,
 };
 
@@ -338,9 +338,9 @@ impl Drop for Swapchain {
     fn drop(&mut self) {
         unsafe {
             self.release_resources();
-            self.functor.destroy_swapchain(*self.swapchain, None);
         }
-        todo!()
+        // the swapchain itself will be automatically destroyed by the
+        // DeviceObject's Drop implementation.
     }
 }
 
@@ -489,7 +489,7 @@ impl Swapchain {
     /// suboptimal and should be recreated.
     fn acquire_image(
         self: Arc<Self>,
-    ) -> impl std::future::Future<Output = VkResult<(SwapchainFrame, bool)>> {
+    ) -> impl std::future::Future<Output = crate::Result<(SwapchainFrame, bool)>> {
         let frame = self
             .current_frame
             .try_update(Ordering::Release, Ordering::Relaxed, |i| {
@@ -500,7 +500,7 @@ impl Swapchain {
         tracing::trace!(frame, "acquiring image for frame {frame}");
 
         async move {
-            let fence = self.fences[frame];
+            let fence = Fence::new_pooled(&self.swapchain.device().pools.fences, None)?;
             let acquire = self.acquire_semaphores[frame];
             let release = self.release_semaphores[frame];
 
@@ -510,14 +510,14 @@ impl Swapchain {
                 move || unsafe {
                     this.with_locked(|swapchain| {
                         this.functor
-                            .acquire_next_image(swapchain, u64::MAX, acquire, fence)
+                            .acquire_next_image(swapchain, u64::MAX, acquire, fence.raw())
                     })
                 }
             })
             .await?;
 
             // wait for image to become available.
-            sync::FenceFuture::new(fence.clone()).await;
+            fence.into_future().await;
 
             let idx = idx as usize;
             let image = self.images[idx].clone();

crates/renderer/src/sync.rs

@@ -5,6 +5,9 @@ use std::{
     time::Duration,
 };
 
+use crate::device::{DeviceObject, DeviceOwned, Pool, PoolObject, Pooled};
+use crate::{Result, device::DeviceInner};
+
 use super::Device;
 use ash::{prelude::*, vk};
 use crossbeam::channel::{Receiver, Sender};
@@ -154,75 +157,94 @@ pub struct Semaphore {
     inner: vk::Semaphore,
 }
 
-pub struct Fence {
+pub enum Fence {
-    dev: Device,
+    Dedicated { fence: DeviceObject<vk::Fence> },
-    fence: vk::Fence,
+    Pooled { fence: PoolObject<vk::Fence> },
+}
+
+impl Pooled for vk::Fence {
+    fn create_from_pool(pool: &Pool<Self>) -> Result<Self> {
+        let fence = unsafe {
+            pool.device
+                .raw
+                .create_fence(&vk::FenceCreateInfo::default(), None)?
+        };
+
+        Ok(fence)
+    }
 }
 
 impl std::fmt::Debug for Fence {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("Fence").field("fence", &self.fence).finish()
+        f.debug_struct("Fence").field("fence", &self.raw()).finish()
-    }
-}
-
-impl Drop for Fence {
-    fn drop(&mut self) {
-        unsafe {
-            self.dev.dev().destroy_fence(self.fence, None);
-        }
     }
 }
 
 impl Fence {
-    unsafe fn new(dev: Device, fence: vk::Fence) -> Fence {
+    pub fn new_dedicated(device: Device, name: Option<&'static str>) -> Result<Fence> {
-        Self { dev, fence }
+        let fence = unsafe {
+            device
+                .raw
+                .create_fence(&vk::FenceCreateInfo::default(), None)?
+        };
+        Ok(Self::Dedicated {
+            fence: DeviceObject::new(fence, device, name.map(Into::into)),
+        })
     }
-    pub fn create(dev: Device) -> VkResult<Fence> {
+    pub fn new_pooled(pool: &Pool<vk::Fence>, name: Option<&'static str>) -> Result<Fence> {
+        let mut fence = pool.get()?;
+        #[cfg(debug_assertions)]
+        if let Some(name) = name {
+            fence.name_object(name);
+        }
+        Ok(Self::Pooled { fence })
+    }
+
+    pub fn raw(&self) -> vk::Fence {
+        match self {
+            Fence::Dedicated { fence } => **fence,
+            Fence::Pooled { fence } => **fence,
+        }
+    }
+
+    fn device(&self) -> &Arc<DeviceInner> {
+        match self {
+            Fence::Dedicated { fence } => &fence.device().shared,
+            Fence::Pooled { fence } => fence.device(),
+        }
+    }
+
+    pub fn wait_on(&self, timeout: Option<u64>) -> Result<()> {
         unsafe {
-            Ok(Self::new(
+            self.device().raw.wait_for_fences(
-                dev.clone(),
+                core::slice::from_ref(&self.raw()),
-                dev.dev()
+                true,
-                    .create_fence(&vk::FenceCreateInfo::default(), None)?,
+                timeout.unwrap_or(u64::MAX),
-            ))
+            )?
         }
+        Ok(())
     }
-    #[allow(dead_code)]
+
-    pub fn create_signaled(dev: Device) -> VkResult<Fence> {
-        unsafe {
-            Ok(Self::new(
-                dev.clone(),
-                dev.dev().create_fence(
-                    &vk::FenceCreateInfo::default().flags(vk::FenceCreateFlags::SIGNALED),
-                    None,
-                )?,
-            ))
-        }
-    }
-    pub fn wait_on(&self, timeout: Option<u64>) -> Result<(), vk::Result> {
-        use core::slice::from_ref;
-        unsafe {
-            self.dev
-                .dev()
-                .wait_for_fences(from_ref(&self.fence), true, timeout.unwrap_or(u64::MAX))
-        }
-    }
-    pub fn fence(&self) -> vk::Fence {
-        self.fence
-    }
     pub fn is_signaled(&self) -> bool {
-        unsafe { self.dev.dev().get_fence_status(self.fence).unwrap_or(false) }
-    }
-    pub fn reset(&self) -> Result<(), vk::Result> {
         unsafe {
-            self.dev
+            self.device()
-                .dev()
+                .raw
-                .reset_fences(core::slice::from_ref(&self.fence))
+                .get_fence_status(self.raw())
+                .unwrap_or(false)
         }
     }
-}
+
-impl AsRef<vk::Fence> for Fence {
+    pub fn reset(&self) -> Result<()> {
-    fn as_ref(&self) -> &vk::Fence {
+        unsafe {
-        todo!()
+            self.device()
+                .raw
+                .reset_fences(core::slice::from_ref(&self.raw()))?
+        }
+        Ok(())
+    }
+
+    pub fn into_future<'a>(self) -> FenceFuture<'a> {
+        FenceFuture::new(Arc::new(self))
     }
 }
 
@@ -265,24 +287,16 @@ pub struct FenceFuture<'a> {
 }
 
 impl FenceFuture<'_> {
-    /// # Safety
-    /// `fence` must not be destroyed while this future is live.
-    #[allow(dead_code)]
-    pub unsafe fn from_fence(device: Device, fence: vk::Fence) -> Self {
-        Self {
-            fence: Arc::new(unsafe { Fence::new(device, fence) }),
-            _pd: PhantomData,
-        }
-    }
     pub fn new(fence: Arc<Fence>) -> Self {
         Self {
             fence,
             _pd: PhantomData,
         }
     }
-    pub fn block(&self) -> VkResult<()> {
+    pub fn block(&self) -> crate::Result<()> {
         self.fence.wait_on(None)?;
-        self.fence.reset()
+        self.fence.reset()?;
+        Ok(())
     }
 }
 
@@ -299,7 +313,7 @@ impl Future for FenceFuture<'_> {
             std::task::Poll::Ready(())
         } else {
             self.fence
-                .dev
+                .device()
                 .sync_threadpool()
                 .spawn_waiter(self.fence.clone(), cx.waker().clone());
             std::task::Poll::Pending