surface/swapchain refactor

crates/renderer/src/commands.rs

@@ -9,7 +9,7 @@ use crate::{
     util::{self, FormatExt, MutexExt},
 };
 
-use super::{Device, Queue};
+use super::{Device, queue::Queue};
 use ash::{prelude::*, vk};
 use parking_lot::Mutex;
 
@@ -643,8 +643,9 @@ mod command_pools {
     use thread_local::ThreadLocal;
 
     use crate::{
-        Queue, define_device_owned_handle,
+        define_device_owned_handle,
         device::{Device, DeviceOwned},
+        queue::Queue,
         sync,
         util::MutexExt,
     };

crates/renderer/src/device.rs

@@ -2,6 +2,7 @@ use std::{
     borrow::Cow,
     collections::{BTreeSet, HashMap, HashSet},
     ffi::CStr,
+    ops::Deref,
     sync::Arc,
 };
 
@@ -102,20 +103,21 @@ impl Drop for DeviceDrop {
 }
 
 struct DeviceExtensions {
-    debug_utils: ext::debug_utils::Device,
+    pub(crate) debug_utils: ext::debug_utils::Device,
-    mesh_shader: Option<ext::mesh_shader::Device>,
+    pub(crate) swapchain: Option<khr::swapchain::Device>,
+    pub(crate) mesh_shader: Option<ext::mesh_shader::Device>,
 }
 
 #[allow(unused)]
 pub struct DeviceInner {
-    alloc: vk_mem::Allocator,
+    pub(crate) alloc: vk_mem::Allocator,
-    raw: ash::Device,
+    pub(crate) raw: ash::Device,
-    adapter: PhysicalDeviceInfo,
+    pub(crate) adapter: PhysicalDeviceInfo,
-    instance: Arc<InstanceInner>,
+    pub(crate) instance: Instance,
-    queues: DeviceQueues,
+    pub(crate) queues: DeviceQueues,
-    sync_threadpool: sync::SyncThreadpool,
+    pub(crate) sync_threadpool: sync::SyncThreadpool,
-    device_extensions: DeviceExtensions,
+    pub(crate) device_extensions: DeviceExtensions,
-    enabled_extensions: Vec<&'static CStr>,
+    pub(crate) enabled_extensions: Vec<&'static CStr>,
     _drop: DeviceDrop,
 }
 
@@ -157,63 +159,6 @@ impl<'a> std::hash::Hash for Extension<'a> {
     }
 }
 
-#[derive(Debug)]
-pub struct DeviceDesc<'a> {
-    pub app_name: Option<&'a str>,
-    pub app_version: u32,
-    pub layers: &'a [&'a CStr],
-    pub layer_settings: &'a [vk::LayerSettingEXT<'a>],
-    pub instance_extensions: &'a [Extension<'a>],
-    pub display_handle: Option<RawDisplayHandle>,
-}
-
-const VALIDATION_LAYER_NAME: &core::ffi::CStr = c"VK_LAYER_KHRONOS_validation";
-const DEBUG_LAYERS: [&core::ffi::CStr; 1] = [VALIDATION_LAYER_NAME];
-impl DeviceDesc<'_> {
-    fn debug_layer_settings() -> &'static [vk::LayerSettingEXT<'static>; 3] {
-        static SETTINGS: std::sync::LazyLock<[vk::LayerSettingEXT; 3]> =
-            std::sync::LazyLock::new(|| {
-                [
-                    vk::LayerSettingEXT::default()
-                        .layer_name(VALIDATION_LAYER_NAME)
-                        .setting_name(c"VK_KHRONOS_VALIDATION_VALIDATE_BEST_PRACTICES")
-                        .ty(vk::LayerSettingTypeEXT::BOOL32)
-                        .values(&[1]),
-                    vk::LayerSettingEXT::default()
-                        .layer_name(VALIDATION_LAYER_NAME)
-                        .setting_name(c"VK_KHRONOS_VALIDATION_VALIDATE_BEST_PRACTICES_AMD")
-                        .ty(vk::LayerSettingTypeEXT::BOOL32)
-                        .values(&[1]),
-                    vk::LayerSettingEXT::default()
-                        .layer_name(VALIDATION_LAYER_NAME)
-                        .setting_name(c"VK_KHRONOS_VALIDATION_VALIDATE_SYNC")
-                        .ty(vk::LayerSettingTypeEXT::BOOL32)
-                        .values(&[1]),
-                ]
-            });
-        &SETTINGS
-    }
-}
-
-impl<'a> Default for DeviceDesc<'a> {
-    fn default() -> Self {
-        Self {
-            app_name: Default::default(),
-            app_version: Default::default(),
-            #[cfg(debug_assertions)]
-            layers: &DEBUG_LAYERS,
-            #[cfg(debug_assertions)]
-            layer_settings: Self::debug_layer_settings(),
-            #[cfg(not(debug_assertions))]
-            layers: &[],
-            #[cfg(not(debug_assertions))]
-            layer_settings: &[],
-            instance_extensions: Default::default(),
-            display_handle: Default::default(),
-        }
-    }
-}
-
 pub(crate) fn get_available_extensions(
     entry: &ash::Entry,
     layers: &[&CStr],
@@ -430,6 +375,11 @@ impl PhysicalDeviceInfo {
 
         let device_extensions = DeviceExtensions {
             debug_utils: ext::debug_utils::Device::new(&instance.inner.raw, &device),
+            swapchain: if enabled_extensions.contains(&khr::swapchain::NAME) {
+                Some(khr::swapchain::Device::new(&instance.inner.raw, &device))
+            } else {
+                None
+            },
             mesh_shader: if enabled_extensions.contains(&ext::mesh_shader::NAME) {
                 Some(ext::mesh_shader::Device::new(&instance.inner.raw, &device))
             } else {
@@ -446,7 +396,7 @@ impl PhysicalDeviceInfo {
                     self.pdev,
                 ))?
             },
-            instance: instance.inner.clone(),
+            instance: instance.clone(),
             adapter: self,
             queues: device_queues,
             device_extensions,
@@ -486,6 +436,14 @@ impl PhysicalDeviceInfo {
 #[derive(Clone, Debug)]
 pub struct Device(Arc<DeviceInner>);
 
+impl PartialEq for Device {
+    fn eq(&self, other: &Self) -> bool {
+        Arc::ptr_eq(&self.0, &other.0)
+    }
+}
+
+impl Eq for Device {}
+
 impl core::ops::Deref for Device {
     type Target = DeviceInner;
 
@@ -493,54 +451,18 @@ impl core::ops::Deref for Device {
         &self.0
     }
 }
-pub type WeakDevice = std::sync::Weak<DeviceInner>;
 
 impl Device {
-    pub fn new_from_desc(desc: DeviceDesc) -> crate::Result<Self> {
-        let instance = Instance::new(&crate::instance::InstanceDesc {
-            app_name: desc.app_name,
-            app_version: desc.app_version,
-            instance_extensions: &desc
-                .instance_extensions
-                .iter()
-                .map(|ext| ext.name)
-                .collect::<Vec<_>>(),
-            layers: desc.layers,
-            layer_settings: desc.layer_settings,
-            display_handle: desc.display_handle,
-        })?;
-
-        // //these are required for the renderpass
-        // let features13 = features.physical_features_13.get_or_insert_default();
-        // features13.synchronization2 = vk::TRUE;
-        // features13.dynamic_rendering = vk::TRUE;
-        // features13.maintenance4 = vk::TRUE;
-        let features = PhysicalDeviceFeatures {
-            core13: vk::PhysicalDeviceVulkan13Features {
-                synchronization2: vk::TRUE,
-                dynamic_rendering: vk::TRUE,
-                maintenance4: vk::TRUE,
-                ..Default::default()
-            },
-            ..Default::default()
-        };
-
-        todo!()
-    }
-
     pub fn sync_threadpool(&self) -> &sync::SyncThreadpool {
         &self.0.sync_threadpool
     }
-    pub fn weak(&self) -> WeakDevice {
-        Arc::downgrade(&self.0)
-    }
     pub fn alloc(&self) -> &vk_mem::Allocator {
         &self.0.alloc
     }
     pub fn dev(&self) -> &ash::Device {
         &self.0.raw
     }
-    pub fn instance(&self) -> &Arc<InstanceInner> {
+    pub fn instance(&self) -> &Instance {
         &self.0.instance
     }
     pub fn queues(&self) -> &DeviceQueues {
@@ -600,18 +522,50 @@ impl Device {
         Ok(())
     }
 
-    pub fn debug_name_object<T: vk::Handle>(&self, handle: T, name: &str) -> VkResult<()> {
+    /// # Safety
-        let name = std::ffi::CString::new(name.as_bytes()).unwrap_or(c"invalid name".to_owned());
+    ///
+    /// This method inherits the safety contract from [`vkSetDebugUtilsObjectName`]. In particular:
+    ///
+    /// - `object` must be a valid handle for one of the following:
+    ///   - An instance-level object from the same instance as this device.
+    ///   - A physical-device-level object that descends from the same physical device as this
+    ///     device.
+    ///   - A device-level object that descends from this device.
+    /// - `object` must be externally synchronized—only the calling thread should access it during
+    ///   this call.
+    ///
+    /// [`vkSetDebugUtilsObjectName`]: https://registry.khronos.org/vulkan/specs/latest/man/html/vkSetDebugUtilsObjectNameEXT.html
+    pub unsafe fn debug_name_object<T: vk::Handle>(&self, object: T, name: &str) {
+        // avoid heap allocation for short names
+        let mut buffer = [0u8; 64];
+        let buffer_vec: Vec<u8>;
+
+        let name_bytes = if name.len() < buffer.len() {
+            buffer[..name.len()].copy_from_slice(name.as_bytes());
+            &buffer[..]
+        } else {
+            buffer_vec = name
+                .as_bytes()
+                .iter()
+                .cloned()
+                .chain(std::iter::once(0))
+                .collect();
+            &buffer_vec
+        };
+
+        let name = CStr::from_bytes_with_nul(name_bytes)
+            .expect("there is always a nul terminator because we added one");
+
         unsafe {
-            self.device_extensions
+            _ = self
+                .device_extensions
                 .debug_utils
                 .set_debug_utils_object_name(
                     &vk::DebugUtilsObjectNameInfoEXT::default()
-                        .object_handle(handle)
+                        .object_handle(object)
-                        .object_name(&name),
+                        .object_name(name),
-                )?;
+                );
         }
-        Ok(())
     }
 }
 
@@ -658,7 +612,9 @@ impl<T> DeviceOwnedDebugObject<T> {
         T: vk::Handle + Copy,
     {
         if let Some(name) = name.as_ref() {
-            device.debug_name_object(object, name)?;
+            unsafe {
+                device.debug_name_object(object, name);
+            }
         }
 
         Ok(Self {
@@ -680,6 +636,98 @@ impl<T> DeviceOwnedDebugObject<T> {
     }
 }
 
+#[derive(Debug)]
+pub struct DeviceObject<T: DeviceHandle> {
+    inner: T,
+    device: Device,
+    #[cfg(debug_assertions)]
+    name: Option<Cow<'static, str>>,
+}
+
+impl<T: DeviceHandle> Deref for DeviceObject<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.inner
+    }
+}
+
+impl<T: DeviceHandle> DeviceObject<T> {
+    pub fn new(inner: T, device: Device, name: Option<Cow<'static, str>>) -> Self {
+        unsafe {
+            if let Some(name) = name.as_ref() {
+                device.debug_name_object(inner, &name);
+            }
+        }
+
+        Self {
+            inner,
+            device,
+            #[cfg(debug_assertions)]
+            name,
+        }
+    }
+    pub fn device(&self) -> &Device {
+        &self.device
+    }
+    pub fn name(&self) -> Option<&str> {
+        #[cfg(debug_assertions)]
+        {
+            self.name.as_deref().map(|cow| cow.as_ref())
+        }
+        #[cfg(not(debug_assertions))]
+        {
+            None
+        }
+    }
+}
+
+impl<T: DeviceHandle> Drop for DeviceObject<T> {
+    fn drop(&mut self) {
+        self.destroy(&self.device);
+    }
+}
+
+pub trait DeviceHandle: vk::Handle + Copy {
+    fn destroy(self, device: &Device);
+}
+
+impl DeviceHandle for vk::Semaphore {
+    fn destroy(self, device: &Device) {
+        unsafe {
+            device.dev().destroy_semaphore(self, None);
+        }
+    }
+}
+
+impl DeviceHandle for vk::Fence {
+    fn destroy(self, device: &Device) {
+        unsafe {
+            device.dev().destroy_fence(self, None);
+        }
+    }
+}
+
+impl DeviceHandle for vk::Buffer {
+    fn destroy(self, device: &Device) {
+        unsafe {
+            device.dev().destroy_buffer(self, None);
+        }
+    }
+}
+
+impl DeviceHandle for vk::SwapchainKHR {
+    fn destroy(self, device: &Device) {
+        unsafe {
+            device
+                .device_extensions
+                .swapchain
+                .as_ref()
+                .map(|swapchain| swapchain.destroy_swapchain(self, None));
+        }
+    }
+}
+
 pub trait DeviceOwned<T> {
     fn device(&self) -> &Device;
     fn handle(&self) -> T;

crates/renderer/src/instance.rs

@@ -1,4 +1,5 @@
 use std::{
+    cell::OnceCell,
     cmp::Ordering,
     ffi::{CStr, CString},
     ops::Deref,
@@ -9,7 +10,7 @@ use ash::{Entry, ext, khr, vk};
 use raw_window_handle::RawDisplayHandle;
 
 use crate::{
-    Error, PhysicalDeviceFeatures, PhysicalDeviceInfo,
+    Error, PhysicalDeviceFeatures, PhysicalDeviceInfo, SurfaceCapabilities,
     device::{Extension, get_extensions, get_layers},
     get_physical_device_features, get_physical_device_properties, make_extension,
     swapchain::Surface,
@@ -47,15 +48,61 @@ impl core::fmt::Debug for Instance {
     }
 }
 
+#[derive(Debug)]
 pub struct InstanceDesc<'a> {
     pub app_name: Option<&'a str>,
     pub app_version: u32,
-    pub instance_extensions: &'a [&'a CStr],
+    pub instance_extensions: &'a [Extension<'a>],
     pub layer_settings: &'a [vk::LayerSettingEXT<'a>],
     pub layers: &'a [&'a CStr],
     pub display_handle: Option<RawDisplayHandle>,
 }
 
+pub const VALIDATION_LAYER: &CStr = c"VK_LAYER_KHRONOS_validation";
+
+#[cfg(not(debug_assertions))]
+const DEBUG_LAYER_SETTINGS: &[vk::LayerSettingEXT] = &[];
+#[cfg(debug_assertions)]
+const DEBUG_LAYER_SETTINGS: &[vk::LayerSettingEXT] = &[
+    vk::LayerSettingEXT {
+        p_layer_name: VALIDATION_LAYER.as_ptr(),
+        p_setting_name: c"VK_KHRONOS_VALIDATION_VALIDATE_BEST_PRACTICES".as_ptr(),
+        value_count: 1,
+        p_values: &[1u8; 1] as *const u8 as _,
+        ty: vk::LayerSettingTypeEXT::BOOL32,
+        _marker: core::marker::PhantomData,
+    },
+    vk::LayerSettingEXT {
+        p_layer_name: VALIDATION_LAYER.as_ptr(),
+        p_setting_name: c"VK_KHRONOS_VALIDATION_VALIDATE_BEST_PRACTICES_AMD".as_ptr(),
+        value_count: 1,
+        p_values: &[1u8; 1] as *const u8 as _,
+        ty: vk::LayerSettingTypeEXT::BOOL32,
+        _marker: core::marker::PhantomData,
+    },
+    vk::LayerSettingEXT {
+        p_layer_name: VALIDATION_LAYER.as_ptr(),
+        p_setting_name: c"VK_KHRONOS_VALIDATION_VALIDATE_SYNC".as_ptr(),
+        value_count: 1,
+        p_values: &[1u8; 1] as *const u8 as _,
+        ty: vk::LayerSettingTypeEXT::BOOL32,
+        _marker: core::marker::PhantomData,
+    },
+];
+
+impl Default for InstanceDesc<'_> {
+    fn default() -> Self {
+        Self {
+            app_name: None,
+            app_version: vk::make_api_version(0, 1, 0, 0),
+            instance_extensions: &[make_extension!(khr::surface)],
+            layer_settings: DEBUG_LAYER_SETTINGS,
+            layers: &[VALIDATION_LAYER],
+            display_handle: None,
+        }
+    }
+}
+
 impl Instance {
     pub fn new<'a>(desc: &InstanceDesc<'a>) -> crate::Result<Self> {
         let entry = unsafe { ash::Entry::load()? };
@@ -95,11 +142,7 @@ impl Instance {
             }
         };
 
-        let mut requested_extensions = desc
+        let mut requested_extensions = desc.instance_extensions.to_vec();
-            .instance_extensions
-            .iter()
-            .map(|name| Extension { name, version: 0 })
-            .collect::<Vec<_>>();
 
         requested_extensions.push(make_extension!(ext::debug_utils as 1));
         #[cfg(debug_assertions)]
@@ -170,17 +213,24 @@ impl Instance {
 
     fn choose_adapter(
         &self,
-        surface: Option<&Surface>,
         mut discriminator: impl FnMut(&PhysicalDeviceInfo, &PhysicalDeviceInfo) -> Ordering,
     ) -> crate::Result<PhysicalDeviceInfo> {
         let pdevs = unsafe { self.inner.raw.enumerate_physical_devices()? };
         let mut pdevs = pdevs
             .into_iter()
-            .map(|pdev| -> crate::Result<PhysicalDeviceInfo> {
+            .map(|pdev| -> crate::Result<PhysicalDeviceInfo> { self.expose_adapter(pdev) })
-                let properties = get_physical_device_properties(&self.inner, pdev)?;
+            .filter_map(Result::ok)
-                let features = get_physical_device_features(&self.inner, pdev, &properties)?;
+            .collect::<Vec<_>>();
 
-                let surface_capabilities = if let Some(surface) = surface {
+        pdevs.sort_unstable_by(&mut discriminator);
+        pdevs.pop().ok_or(Error::NoAdapter)
+    }
+
+    pub(crate) fn get_adapter_surface_capabilities(
+        &self,
+        pdev: vk::PhysicalDevice,
+        surface: &Surface,
+    ) -> crate::Result<SurfaceCapabilities> {
         let capabilities = unsafe {
             surface
                 .functor
@@ -193,23 +243,31 @@ impl Instance {
                 .get_physical_device_surface_formats(pdev, surface.raw)?
         };
 
-                    Some((capabilities, formats))
+        let present_modes = unsafe {
-                } else {
+            surface
-                    None
+                .functor
+                .get_physical_device_surface_present_modes(pdev, surface.raw)?
         };
 
+        Ok(SurfaceCapabilities {
+            capabilities,
+            formats,
+            present_modes,
+        })
+    }
+
+    pub(crate) fn expose_adapter(
+        &self,
+        pdev: vk::PhysicalDevice,
+    ) -> crate::Result<PhysicalDeviceInfo> {
+        let properties = get_physical_device_properties(&self.inner, pdev)?;
+        let features = get_physical_device_features(&self.inner, pdev, &properties)?;
+
         Ok(PhysicalDeviceInfo {
             pdev,
             properties,
             features,
-                    surface_capabilities,
         })
-            })
-            .filter_map(Result::ok)
-            .collect::<Vec<_>>();
-
-        pdevs.sort_unstable_by(&mut discriminator);
-        pdevs.pop().ok_or(Error::NoAdapter)
     }
 
     pub(crate) fn choose_adapter_default(
@@ -218,7 +276,7 @@ impl Instance {
         required_extensions: &[Extension],
         required_features: Option<&PhysicalDeviceFeatures>,
     ) -> crate::Result<PhysicalDeviceInfo> {
-        self.choose_adapter(surface, |a, b| {
+        self.choose_adapter(|a, b| {
             // Extensions: we definitely need swapchain.
             match a
                 .properties
@@ -240,6 +298,7 @@ impl Instance {
 
             // Check surface compatibility
             // TODO
+            _ = surface;
 
             if let Some(ref required_features) = required_features {
                 if b.features.superset_of(&required_features)

crates/renderer/src/lib.rs

@@ -6,7 +6,9 @@
     slice_partition_dedup
 )]
 
-use std::{collections::HashMap, ffi::CStr, fmt::Debug, marker::PhantomData, sync::Arc};
+use std::{
+    cell::OnceCell, collections::HashMap, ffi::CStr, fmt::Debug, marker::PhantomData, sync::Arc,
+};
 
 use bitflags::bitflags;
 use raw_window_handle::{RawDisplayHandle, RawWindowHandle};
@@ -103,6 +105,16 @@ pub enum Error {
     NoPhysicalDevice,
     #[error(transparent)]
     Io(#[from] std::io::Error),
+    #[error(
+        "Image dimensions ({width}x{height}) exceed the maximum allowed size of {max_size}x{max_size}."
+    )]
+    ImageTooLarge {
+        width: u32,
+        height: u32,
+        max_size: u32,
+    },
+    #[error("Image dimensions cannot be zero.")]
+    ImageZeroSized,
 }
 
 pub type Result<T> = core::result::Result<T, Error>;
@@ -160,34 +172,6 @@ fn compatible_extension_properties(
 
 pub mod queue;
 
-// Queues must be externally synchronised for calls to `vkQueueSubmit` and `vkQueuePresentKHR`.
-#[derive(Clone, Debug)]
-pub struct Queue(Arc<Mutex<vk::Queue>>, u32);
-
-impl Queue {
-    fn new(device: &ash::Device, family: u32, index: u32) -> Self {
-        Self(
-            Arc::new(Mutex::new(unsafe {
-                device.get_device_queue(family, index)
-            })),
-            family,
-        )
-    }
-
-    pub fn family(&self) -> u32 {
-        self.1
-    }
-
-    pub fn with_locked<T, F: FnOnce(vk::Queue) -> T>(&self, map: F) -> T {
-        let lock = self.0.lock();
-        map(*lock)
-    }
-
-    pub fn lock(&self) -> MutexGuard<'_, vk::Queue> {
-        self.0.lock()
-    }
-}
-
 pub trait ExtendsDeviceFeatures2Debug:
     vk::ExtendsPhysicalDeviceFeatures2 + Debug + Send + Sync
 {
@@ -211,7 +195,13 @@ pub struct PhysicalDeviceInfo {
     pub pdev: vk::PhysicalDevice,
     pub properties: PhysicalDeviceProperties,
     pub features: PhysicalDeviceFeatures,
-    pub surface_capabilities: Option<(SurfaceCapabilitiesKHR, Vec<vk::SurfaceFormatKHR>)>,
+}
+
+#[derive(Debug)]
+pub struct SurfaceCapabilities {
+    pub capabilities: SurfaceCapabilitiesKHR,
+    pub formats: Vec<vk::SurfaceFormatKHR>,
+    pub present_modes: Vec<vk::PresentModeKHR>,
 }
 
 #[derive(Default, Debug)]
@@ -960,7 +950,16 @@ pub struct Renderer2 {
 
 impl Renderer2 {
     pub fn new(display: RawDisplayHandle) -> Result<Self> {
-        let device = Device::new_from_default_desc(Some(display), &[])?;
+        let instance = Instance::new(&InstanceDesc {
+            ..Default::default()
+        })?;
+        let adapter = instance.choose_adapter_default(None, &[], None)?;
+        let device = adapter.create_logical_device(
+            &instance,
+            &[],
+            PhysicalDeviceFeatures::default(),
+            Some(display),
+        )?;
 
         Ok(Self {
             samplers: SamplerCache::new(device.clone()),
@@ -1027,7 +1026,10 @@ impl Renderer2 {
 
 pub use vk::Extent2D;
 
-use crate::{device::Extension, instance::InstanceInner};
+use crate::{
+    device::Extension,
+    instance::{InstanceDesc, InstanceInner},
+};
 
 pub mod utils {
     #![allow(dead_code)]

crates/renderer/src/pipeline.rs

@@ -255,7 +255,7 @@ impl DescriptorPool {
 
         for (&set, desc) in sets.iter().zip(descs) {
             if let Some(name) = desc.name.as_ref() {
-                self.device().debug_name_object(set, &name)?;
+                unsafe { self.device().debug_name_object(set, &name) };
             }
         }
 

crates/renderer/src/queue.rs

@@ -133,6 +133,10 @@ impl DeviceQueues {
         &self.transfer
     }
 
+    pub fn swapchain_family_indices(&self) -> &[u32] {
+        core::slice::from_ref(&self.graphics.family.index)
+    }
+
     pub unsafe fn lock(&self) {
         core::mem::forget((
             self.graphics.inner.lock.lock(),