more fence/semaphore refactor

crates/renderer/src/commands.rs

@@ -288,7 +288,7 @@ impl SingleUseCommand {
         wait: Option<(vk::Semaphore, vk::PipelineStageFlags)>,
         signal: Option<vk::Semaphore>,
     ) -> crate::Result<()> {
-        let fence = Arc::new(sync::Fence::new_pooled(&self.device().pools.fences, None)?);
+        let fence = Arc::new(sync::Fence::from_pool(&self.device().pools.fences, None)?);
         let future = self.submit_async(wait, signal, fence)?;
         future.block()?;
         Ok(())

crates/renderer/src/device.rs

@@ -20,9 +20,8 @@ use tinyvec::{ArrayVec, array_vec};
 use crate::{
     Instance, PhysicalDeviceFeatures, PhysicalDeviceInfo, Result,
     instance::InstanceInner,
-    queue::Queue,
-    queue::{DeviceQueueInfos, DeviceQueues},
-    sync,
+    queue::{DeviceQueueInfos, DeviceQueues, Queue},
+    sync::{self, BinarySemaphore, TimelineSemaphore},
 };
 
 #[derive(Debug, Default)]
@@ -410,9 +409,7 @@ impl PhysicalDeviceInfo {
 
         let shared = Arc::new(inner);
         Ok(Device {
-            pools: Arc::new(DevicePools {
-                fences: Pool::new(shared.clone()),
-            }),
+            pools: Arc::new(DevicePools::new(shared.clone())),
             shared,
         })
     }
@@ -445,6 +442,18 @@ impl PhysicalDeviceInfo {
 #[derive(Clone, Debug)]
 pub(crate) struct DevicePools {
     pub(crate) fences: Pool<vk::Fence>,
+    pub(crate) binary_semaphores: Pool<BinarySemaphore>,
+    pub(crate) timeline_semaphores: Pool<TimelineSemaphore>,
+}
+
+impl DevicePools {
+    pub fn new(device: Arc<DeviceInner>) -> Self {
+        Self {
+            fences: Pool::new(device.clone()),
+            binary_semaphores: Pool::new(device.clone()),
+            timeline_semaphores: Pool::new(device),
+        }
+    }
 }
 
 #[derive(Clone, Debug)]
@@ -599,7 +608,8 @@ pub struct DeviceOwnedDebugObject<T> {
 impl<T: Eq> Eq for DeviceOwnedDebugObject<T> {}
 impl<T: PartialEq> PartialEq for DeviceOwnedDebugObject<T> {
     fn eq(&self, other: &Self) -> bool {
-        std::sync::Arc::ptr_eq(&self.device.0, &other.device.0) && self.object == other.object
+        std::sync::Arc::ptr_eq(&self.device.shared, &other.device.shared)
+            && self.object == other.object
     }
 }
 
@@ -752,18 +762,18 @@ pub trait DeviceOwned<T> {
     fn handle(&self) -> T;
 }
 
-pub trait Pooled: vk::Handle + Sized {
+pub trait Pooled: Sized {
     fn create_from_pool(pool: &Pool<Self>) -> Result<Self>;
 }
 
-pub struct PoolObject<T: Pooled + Clone> {
-    inner: ManuallyDrop<T>,
-    pool: Pool<T>,
+pub struct PoolObject<T: Pooled + vk::Handle + Clone> {
+    pub(crate) inner: ManuallyDrop<T>,
+    pub(crate) pool: Pool<T>,
     #[cfg(debug_assertions)]
-    name: Option<Cow<'static, str>>,
+    pub(crate) name: Option<Cow<'static, str>>,
 }
 
-impl<T: Pooled + Clone> PoolObject<T> {
+impl<T: Pooled + vk::Handle + Clone> PoolObject<T> {
     pub fn name_object(&mut self, name: impl Into<Cow<'static, str>>) {
         #[cfg(debug_assertions)]
         unsafe {
@@ -779,7 +789,7 @@ impl<T: Pooled + Clone> PoolObject<T> {
     }
 }
 
-impl<T: Pooled + Clone> Drop for PoolObject<T> {
+impl<T: Pooled + vk::Handle + Clone> Drop for PoolObject<T> {
     fn drop(&mut self) {
         let handle = unsafe { ManuallyDrop::take(&mut self.inner) };
         #[cfg(debug_assertions)]
@@ -791,7 +801,7 @@ impl<T: Pooled + Clone> Drop for PoolObject<T> {
     }
 }
 
-impl<T: Pooled + Clone> Deref for PoolObject<T> {
+impl<T: Pooled + vk::Handle + Clone> Deref for PoolObject<T> {
     type Target = T;
 
     fn deref(&self) -> &Self::Target {
@@ -815,9 +825,12 @@ impl<T> Pool<T> {
             device,
         }
     }
+    pub fn pop(&self) -> Option<T> {
+        self.pool.lock().pop()
+    }
 }
 
-impl<T: Pooled + Clone> Pool<T> {
+impl<T: Pooled + vk::Handle + Clone> Pool<T> {
     pub fn get(&self) -> Result<PoolObject<T>> {
         let item = if let Some(item) = self.pool.lock().pop() {
             item
@@ -832,6 +845,14 @@ impl<T: Pooled + Clone> Pool<T> {
             name: None,
         })
     }
+
+    pub fn get_named(&self, name: Option<impl Into<Cow<'static, str>>>) -> Result<PoolObject<T>> {
+        let mut obj = self.get()?;
+        if let Some(name) = name {
+            obj.name_object(name);
+        }
+        Ok(obj)
+    }
 }
 
 // Macro for helping create and destroy Vulkan objects which are owned by a device.

crates/renderer/src/images.rs

@@ -6,7 +6,7 @@ use std::{
 
 use crate::{
     define_device_owned_handle,
-    device::{DeviceObject, DeviceOwned, QueueFlags},
+    device::{DeviceHandle, DeviceObject, DeviceOwned, QueueFlags},
 };
 
 use super::Device;
@@ -128,11 +128,43 @@ enum ImageInner {
     Allocated(vk::Image, vk_mem::Allocation),
 }
 
-pub struct Image2 {
-    image: DeviceObject<vk::Image>,
+#[derive(Debug, Clone, Copy)]
+struct ImageAlloc {
+    image: vk::Image,
     alloc: Option<vk_mem::Allocation>,
 }
 
+impl vk::Handle for ImageAlloc {
+    const TYPE: vk::ObjectType = <vk::Image as vk::Handle>::TYPE;
+
+    fn as_raw(self) -> u64 {
+        self.image.as_raw()
+    }
+
+    fn from_raw(_: u64) -> Self {
+        unimplemented!()
+    }
+}
+
+impl DeviceHandle for ImageAlloc {
+    fn destroy(mut self, device: &Device) {
+        unsafe {
+            match &mut self.alloc {
+                Some(alloc) => {
+                    device.alloc.destroy_image(self.image, alloc);
+                }
+                None => {
+                    device.raw.destroy_image(self.image, None);
+                }
+            }
+        }
+    }
+}
+
+pub struct Image2 {
+    image: DeviceObject<ImageAlloc>,
+}
+
 define_device_owned_handle! {
     #[derive(Debug)]
     pub Image(vk::Image) {

crates/renderer/src/lib.rs

@@ -990,13 +990,32 @@ impl Renderer2 {
         &self,
         window: RawWindowHandle,
         extent: vk::Extent2D,
-    ) -> Result<swapchain::WindowSurface> {
-        swapchain::WindowSurface::new(self.device.clone(), extent, window, self.display)
+    ) -> Result<swapchain::Surface> {
+        let surface = unsafe {
+            swapchain::Surface::new_from_raw_window_handle(
+                &self.device.instance,
+                self.display,
+                window,
+            )?
+        };
+        surface.configure(
+            &self.device,
+            swapchain::SwapchainConfiguration {
+                present_mode: vk::PresentModeKHR::MAILBOX,
+                format: vk::Format::R8G8B8A8_UNORM,
+                color_space: vk::ColorSpaceKHR::SRGB_NONLINEAR,
+                image_count: 3,
+                extent,
+                composite_alpha_mode: vk::CompositeAlphaFlagsKHR::OPAQUE,
+                usage: vk::ImageUsageFlags::COLOR_ATTACHMENT | vk::ImageUsageFlags::TRANSFER_DST,
+            },
+        )?;
+        Ok(surface)
     }
 
     pub async fn draw_graph<T, F: FnOnce(&mut Renderer2, &mut render_graph::RenderGraph) -> T>(
         &mut self,
-        surface: &swapchain::WindowSurface,
+        surface: &swapchain::Surface,
         cb: F,
     ) -> Result<T> {
         let frame = surface.acquire_image().await?;
@@ -1011,7 +1030,7 @@ impl Renderer2 {
         let future = cmds.submit(
             Some((frame.acquire, vk::PipelineStageFlags::TRANSFER)),
             Some(frame.release),
-            Arc::new(sync::Fence::new_pooled(&self.device.pools.fences, None)?),
+            Arc::new(sync::Fence::from_pool(&self.device.pools.fences, None)?),
         )?;
 
         future.await;

crates/renderer/src/rendering/mod.rs

@@ -6,17 +6,17 @@ use glam::{f32::Mat4, vec3};
 pub use crate::egui_pass::{egui_pass, egui_pre_pass};
 
 use crate::{
+    Result,
     buffers::{Buffer, BufferDesc},
     commands::{self, traits::CommandBufferExt},
     device::{Device, DeviceOwned},
     images::ImageViewDesc,
     pipeline,
     render_graph::{
-        buffer_barrier, Access, GraphResourceId, PassDesc, RecordFn, RenderContext, RenderGraph,
+        Access, GraphResourceId, PassDesc, RecordFn, RenderContext, RenderGraph, buffer_barrier,
     },
     sync,
     util::Rgba8,
-    Result,
 };
 
 pub struct Wireframe {
@@ -162,7 +162,11 @@ impl Wireframe {
             );
         }
 
-        let future = cmd.submit_async(None, None, Arc::new(sync::Fence::create(dev.clone())?))?;
+        let future = cmd.submit_async(
+            None,
+            None,
+            Arc::new(sync::Fence::from_pool(&dev.pools.fences, None)?),
+        )?;
 
         let (pipeline, layout) = Self::create_pipeline(dev.clone())?;
 

crates/renderer/src/swapchain.rs

@@ -500,7 +500,7 @@ impl Swapchain {
         tracing::trace!(frame, "acquiring image for frame {frame}");
 
         async move {
-            let fence = Fence::new_pooled(&self.swapchain.device().pools.fences, None)?;
+            let fence = Fence::from_pool(&self.swapchain.device().pools.fences, None)?;
             let acquire = self.acquire_semaphores[frame];
             let release = self.release_semaphores[frame];
 

crates/renderer/src/sync.rs

@@ -1,6 +1,9 @@
+#[cfg(debug_assertions)]
+use std::borrow::Cow;
 use std::{
     future::Future,
     marker::PhantomData,
+    mem::ManuallyDrop,
     sync::{Arc, atomic::AtomicU32},
     time::Duration,
 };
@@ -152,11 +155,6 @@ impl SyncThreadpool {
     }
 }
 
-pub struct Semaphore {
-    device: Device,
-    inner: vk::Semaphore,
-}
-
 pub enum Fence {
     Dedicated { fence: DeviceObject<vk::Fence> },
     Pooled { fence: PoolObject<vk::Fence> },
@@ -191,7 +189,7 @@ impl Fence {
             fence: DeviceObject::new(fence, device, name.map(Into::into)),
         })
     }
-    pub fn new_pooled(pool: &Pool<vk::Fence>, name: Option<&'static str>) -> Result<Fence> {
+    pub fn from_pool(pool: &Pool<vk::Fence>, name: Option<&'static str>) -> Result<Fence> {
         let mut fence = pool.get()?;
         #[cfg(debug_assertions)]
         if let Some(name) = name {
@@ -248,34 +246,182 @@ impl Fence {
     }
 }
 
-#[allow(dead_code)]
-impl Semaphore {
-    pub fn new(device: Device) -> VkResult<Self> {
+#[derive(Debug, Clone, Copy)]
+enum SemaphoreType {
+    Binary,
+    Timeline(u64),
+}
+
+pub enum Semaphore {
+    Dedicated {
+        semaphore_type: SemaphoreType,
+        semaphore: DeviceObject<vk::Semaphore>,
+    },
+    Pooled {
+        semaphore_type: SemaphoreType,
+        semaphore: vk::Semaphore,
+        device: Device,
+        #[cfg(debug_assertions)]
+        name: Option<Cow<'static, str>>,
+    },
+}
+
+#[derive(Debug, Clone)]
+pub(crate) struct BinarySemaphore(vk::Semaphore);
+#[derive(Debug, Clone)]
+pub(crate) struct TimelineSemaphore(vk::Semaphore);
+
+// This is just so that ash can name these semaphore newtypes
+impl vk::Handle for BinarySemaphore {
+    const TYPE: vk::ObjectType = <vk::Semaphore as vk::Handle>::TYPE;
+
+    fn as_raw(self) -> u64 {
+        self.0.as_raw()
+    }
+
+    fn from_raw(_: u64) -> Self {
+        unimplemented!("BinarySemaphore cannot be created from raw handle")
+    }
+}
+
+impl vk::Handle for TimelineSemaphore {
+    const TYPE: vk::ObjectType = <vk::Semaphore as vk::Handle>::TYPE;
+
+    fn as_raw(self) -> u64 {
+        self.0.as_raw()
+    }
+
+    fn from_raw(_: u64) -> Self {
+        unimplemented!("TimelineSemaphore cannot be created from raw handle")
+    }
+}
+
+impl Pooled for BinarySemaphore {
+    fn create_from_pool(pool: &Pool<Self>) -> Result<Self> {
         let mut type_info =
             vk::SemaphoreTypeCreateInfo::default().semaphore_type(vk::SemaphoreType::BINARY);
         let create_info = vk::SemaphoreCreateInfo::default().push_next(&mut type_info);
-        let inner = unsafe { device.dev().create_semaphore(&create_info, None)? };
-
-        Ok(Self { device, inner })
+        let inner = unsafe { pool.device.raw.create_semaphore(&create_info, None)? };
+        Ok(Self(inner))
     }
-    pub fn new_timeline(device: Device, value: u64) -> VkResult<Self> {
+}
+
+impl Pooled for TimelineSemaphore {
+    fn create_from_pool(pool: &Pool<Self>) -> Result<Self> {
         let mut type_info = vk::SemaphoreTypeCreateInfo::default()
             .semaphore_type(vk::SemaphoreType::TIMELINE)
-            .initial_value(value);
+            .initial_value(0);
         let create_info = vk::SemaphoreCreateInfo::default().push_next(&mut type_info);
-        let inner = unsafe { device.dev().create_semaphore(&create_info, None)? };
-
-        Ok(Self { device, inner })
-    }
-    pub fn semaphore(&self) -> vk::Semaphore {
-        self.inner
+        let inner = unsafe { pool.device.raw.create_semaphore(&create_info, None)? };
+        Ok(Self(inner))
     }
 }
 
 impl Drop for Semaphore {
     fn drop(&mut self) {
+        if let Semaphore::Pooled {
+            device,
+            semaphore_type,
+            semaphore,
+            name,
+        } = self
+        {
+            #[cfg(debug_assertions)]
+            if name.is_some() {
+                // reset the name to avoid confusion in case this semaphore is re-used
+                unsafe { device.debug_name_object(*semaphore, "") };
+            }
+            match semaphore_type {
+                SemaphoreType::Binary => device
+                    .pools
+                    .binary_semaphores
+                    .push(BinarySemaphore(*semaphore)),
+                SemaphoreType::Timeline(_) => {
+                    device
+                        .pools
+                        .timeline_semaphores
+                        .push(TimelineSemaphore(*semaphore));
+                }
+            }
+        }
+    }
+}
+
+impl Semaphore {
+    pub fn new_dedicated(
+        device: Device,
+        semaphore_type: SemaphoreType,
+        name: Option<&'static str>,
+    ) -> Result<Self> {
+        let mut type_info = vk::SemaphoreTypeCreateInfo::default();
+        match semaphore_type {
+            SemaphoreType::Binary => {
+                type_info = type_info.semaphore_type(vk::SemaphoreType::BINARY);
+            }
+            SemaphoreType::Timeline(value) => {
+                type_info = type_info
+                    .semaphore_type(vk::SemaphoreType::TIMELINE)
+                    .initial_value(value);
+            }
+        }
+        let create_info = vk::SemaphoreCreateInfo::default().push_next(&mut type_info);
+        let inner = unsafe { device.dev().create_semaphore(&create_info, None)? };
+
+        Ok(Self::Dedicated {
+            semaphore_type,
+            semaphore: DeviceObject::new(inner, device, name.map(Into::into)),
+        })
+    }
+
+    pub fn from_pool(
+        device: Device,
+        semaphore_type: SemaphoreType,
+        name: Option<&'static str>,
+    ) -> Result<Self> {
+        let semaphore = match semaphore_type {
+            SemaphoreType::Binary => {
+                if let Some(semaphore) = device.pools.binary_semaphores.pop() {
+                    semaphore.0
+                } else {
+                    let mut type_info = vk::SemaphoreTypeCreateInfo::default()
+                        .semaphore_type(vk::SemaphoreType::BINARY);
+                    let create_info = vk::SemaphoreCreateInfo::default().push_next(&mut type_info);
+                    unsafe { device.raw.create_semaphore(&create_info, None)? }
+                }
+            }
+            SemaphoreType::Timeline(value) => {
+                if let Some(semaphore) = device.pools.binary_semaphores.pop() {
+                    semaphore.0
+                } else {
+                    let mut type_info = vk::SemaphoreTypeCreateInfo::default()
+                        .semaphore_type(vk::SemaphoreType::TIMELINE)
+                        .initial_value(value);
+                    let create_info = vk::SemaphoreCreateInfo::default().push_next(&mut type_info);
+                    unsafe { device.raw.create_semaphore(&create_info, None)? }
+                }
+            }
+        };
+
+        #[cfg(debug_assertions)]
+        if let Some(name) = name {
             unsafe {
-            self.device.dev().destroy_semaphore(self.inner, None);
+                device.debug_name_object(semaphore, name);
+            }
+        }
+
+        Ok(Self::Pooled {
+            semaphore_type,
+            semaphore,
+            device,
+            #[cfg(debug_assertions)]
+            name: name.map(Into::into),
+        })
+    }
+
+    pub fn semaphore(&self) -> vk::Semaphore {
+        match self {
+            Semaphore::Dedicated { semaphore, .. } => **semaphore,
+            Semaphore::Pooled { semaphore, .. } => *semaphore,
         }
     }
 }