refactor: move more types to new VulkanObject system

src/client/rendering/mod.rs

@@ -2,6 +2,7 @@ use std::{ffi::CStr, ops::Deref};
 
 use ash::vk;
 use thiserror::Error;
+use vulkan::WeakObject;
 use winit::{event_loop::ActiveEventLoop, raw_window_handle::{HandleError, HasDisplayHandle}};
 
 use crate::*;
@@ -154,7 +155,7 @@ pub fn init(app: &mut App, event_loop: &ActiveEventLoop) -> RenderResult<()> {
 					.iter()
 					.map(|image| {
 						vk::ImageViewCreateInfo::default()
-							.image(**image)
+							.image(unsafe { *image.object() })
 							.format(format)
 							.view_type(vk::ImageViewType::TYPE_2D)
 							.components(

src/client/rendering/vulkan/commands.rs

@@ -7,6 +7,8 @@ use ash::{prelude::VkResult, vk};
 
 use crate::constants;
 
+use super::WeakObject;
+
 /// A collection of a frame's Vulkan commands.
 pub struct Frame {
 	command_pool_handle: vk::CommandPool,
@@ -109,7 +111,7 @@ impl Frame {
 	#[inline]
 	pub fn cmd_clear_color_image(&self, image: &super::Image, image_layout: vk::ImageLayout, clear_color_value: vk::ClearColorValue, ranges: &[vk::ImageSubresourceRange]) {
 		// SAFETY: The device is available at this point.
-		unsafe { self.device.cmd_clear_color_image(self.command_buffer_handle, **image, image_layout, &clear_color_value, ranges); }
+		unsafe { self.device.cmd_clear_color_image(self.command_buffer_handle, *image.object(), image_layout, &clear_color_value, ranges); }
 	}
 
 	#[inline]
@@ -148,7 +150,7 @@ impl Frame {
 			.old_layout(old_layout)
 			.new_layout(new_layout)
 			.subresource_range(subresource_range)
-			.image(image.0);
+			.image(unsafe { *image.object() });
 		let image_barriers = [image_barrier];
 		let dependency_info = vk::DependencyInfo::default()
 			.image_memory_barriers(&image_barriers);

src/client/rendering/vulkan/image.rs

@@ -3,6 +3,8 @@
 
 use ash::{prelude::VkResult, vk};
 
+use super::WeakObject;
+
 pub struct AllocatedImage {
 	image: super::Image,
 	image_view: super::ImageView,
@@ -15,7 +17,7 @@ impl AllocatedImage {
 	pub(super) fn new(device: &super::Device, image_create_info: &vk::ImageCreateInfo, image_view_create_info: &vk::ImageViewCreateInfo, extent: vk::Extent3D, format: vk::Format) -> VkResult<Self> {
 		let image = device.create_image(image_create_info)?;
 		let image_view_create_info = image_view_create_info
-			.image(*image);
+			.image(unsafe { *image.object() });
 		let image_view = device.create_image_view(&image_view_create_info)?;
 		Ok(
 			Self {

src/client/rendering/vulkan/mod.rs

@@ -2,12 +2,16 @@
 //! This module provides safe abstractions for Vulkan objects.
 //!
 //! See [`VulkanObject`] and [`Instance`].
+//! 
+//! # Safety Guarantees
+//! For creation methods, the object is automatically dropped, and the data is initialized.
+//! 
+//! For extension getters, the extensions are already initialized.
 
-use std::{any::Any, borrow::BorrowMut, collections::HashMap, mem::ManuallyDrop, ops::Deref, path::PathBuf, ptr::drop_in_place, rc::Rc};
+use std::{any::Any, borrow::BorrowMut, collections::HashMap, mem::ManuallyDrop, ptr::drop_in_place, rc::Rc};
 
 use ash::{ext, khr, prelude::VkResult, vk};
 use sigill_derive::{Deref, DerefMut};
-use vk_mem::Alloc;
 use winit::raw_window_handle::{RawDisplayHandle, RawWindowHandle};
 
 use super::RenderResult;
@@ -34,30 +38,28 @@ pub type QueueIndex = u32;
 /// 
 /// See [`VulkanObjectType`].
 #[derive(Deref, DerefMut)]
-pub struct LegacyVulkanObject<T, D>(T, D, fn(&T, &mut D));
+pub struct CustomVulkanObject<T, D>(T, D, fn(&T, &mut D));
 
-impl<T, D> LegacyVulkanObject<T, D> {
+impl<T, D> CustomVulkanObject<T, D> {
 	pub fn new(object: T, data: D, destructor: fn(&T, &mut D)) -> Self {
 		Self(object, data, destructor)
 	}
 }
 
-impl<T, D> LegacyVulkanObject<T, Option<D>> {
+impl<T, D> CustomVulkanObject<T, Option<D>> {
 	fn undropped(object: T) -> Self {
 		Self(object, None, |_, _| {})
 	}
 }
 
-impl<T, D> Drop for LegacyVulkanObject<T, D> {
+impl<T, D> Drop for CustomVulkanObject<T, D> {
 	fn drop(&mut self) {
 		(self.2)(&self.0, &mut self.1);
 	}
 }
 
 // Some types for Object
-pub type Surface = LegacyVulkanObject<vk::SurfaceKHR, khr::surface::Instance>;
-pub type ImageView = LegacyVulkanObject<vk::ImageView, ash::Device>;
-pub type Image = LegacyVulkanObject<vk::Image, Option<(Rc<vk_mem::Allocator>, vk_mem::Allocation)>>;
+pub type Surface = CustomVulkanObject<vk::SurfaceKHR, khr::surface::Instance>;
 
 /// A type of Vulkan object that is automatically dropped in order of dependency.
 /// # Safety
@@ -158,33 +160,28 @@ impl Instance {
 
 	#[inline]
 	pub fn get_physical_device_surface_support(&self, physical_device: vk::PhysicalDevice, queue_family_index: QueueFamilyIndex, surface: &Surface) -> VkResult<bool> {
-		// SAFETY: The object needs no additional allocation.
 		unsafe { self.extensions.surface.get_physical_device_surface_support(physical_device, queue_family_index, surface.0) }
 	}
 
 	#[inline]
 	pub fn get_physical_device_surface_capabilities(&self, physical_device: vk::PhysicalDevice, surface: &Surface) -> VkResult<vk::SurfaceCapabilitiesKHR> {
-		// SAFETY: The object needs no additional allocation function.
 		unsafe { self.extensions.surface.get_physical_device_surface_capabilities(physical_device, surface.0) }
 	}
 
 	#[inline]
 	pub fn get_physical_device_surface_formats(&self, physical_device: vk::PhysicalDevice, surface: &Surface) -> VkResult<Vec<vk::SurfaceFormatKHR>> {
-		// SAFETY: The object needs no additional allocation function.
 		unsafe { self.extensions.surface.get_physical_device_surface_formats(physical_device, surface.0) }
 	}
 
 	#[inline]
 	pub fn get_physical_device_surface_present_modes(&self, physical_device: vk::PhysicalDevice, surface: &Surface) -> VkResult<Vec<vk::PresentModeKHR>> {
-		// SAFETY: The object needs no additional allocation function.
 		unsafe { self.extensions.surface.get_physical_device_surface_present_modes(physical_device, surface.0) }
 	}
 
 	// Vulkan Object Creation for Extensions
 
 	#[inline]
 	pub fn create_debug_utils_messenger_ext(&mut self, create_info: &vk::DebugUtilsMessengerCreateInfoEXT) -> VkResult<&DebugUtilsMessenger> {
-		// SAFETY: The object is automatically dropped.
 		self.set_object(
 			VulkanObjectType::DebugUtilsMessenger,
 			unsafe {
@@ -201,15 +198,18 @@ impl Instance {
 	#[inline]
 	pub fn create_swapchain<'a>(&mut self, create_info: &vk::SwapchainCreateInfoKHR, image_view_provider: impl FnOnce(&Vec<Image>, vk::Format) -> Vec<vk::ImageViewCreateInfo<'a>>) -> VkResult<&swapchain::Swapchain> {
 		let swapchain_device = khr::swapchain::Device::new(&self.inner, &self.device().inner);
-		// SAFETY: The object is automatically dropped.
 		self.set_object(
 			VulkanObjectType::Swapchain,
-			unsafe {
-				let handle = swapchain_device.create_swapchain(create_info, None)?;
-				let images = swapchain_device.get_swapchain_images(handle)?
-					.into_iter()
-					.map(|image| LegacyVulkanObject::undropped(image))
-					.collect::<Vec<_>>();
+			{
+				// SAFETY: The object is automatically destroyed.
+				let handle = unsafe { swapchain_device.create_swapchain(create_info, None)? };
+				// SAFETY: The extension is already initialized.
+				let images = unsafe {
+					swapchain_device.get_swapchain_images(handle)?
+						.into_iter()
+						.map(|image| Image::from_object(image))
+						.collect::<Vec<_>>()
+				};
 				let image_view = image_view_provider(&images, create_info.image_format)
 					.into_iter()
 					.map(|create_info| self.device().create_image_view(&create_info))
@@ -235,7 +235,7 @@ impl Instance {
 		self.set_object(
 			VulkanObjectType::Surface, 
 			unsafe {
-				LegacyVulkanObject::new(
+				CustomVulkanObject::new(
 					ash_window::create_surface(self.entry(), &self.inner, display_handle, window_handle, None)?,
 					khr::surface::Instance::new(self.entry(), &self.inner),
 					|surface, instance| instance.destroy_surface(*surface, None),
@@ -404,7 +404,6 @@ impl Device {
 
 	// Object Creation
 
-	#[inline]
 	pub fn create_image(&self, create_info: &vk::ImageCreateInfo) -> VkResult<Image> {
 		// SAFETY: The object is automatically destroyed.
 		unsafe {
@@ -413,32 +412,15 @@ impl Device {
 				required_flags: vk::MemoryPropertyFlags::DEVICE_LOCAL,
 				..Default::default()
 			};
-			let image = self.allocator.create_image(create_info, &allocation_create_info)?;
-			Ok(
-				LegacyVulkanObject::new(
-					image.0,
-					Some((self.allocator.clone(), image.1)),
-					|image, data| {
-						let (allocator, allocation) = data.as_mut().unwrap();
-						allocator.destroy_image(*image, allocation);
-					},
-				)
+			Image::new(
+				&(*create_info, allocation_create_info),
+				self.allocator.clone(),
 			)
 		}
 	}
 
-	#[inline]
 	pub fn create_image_view(&self, create_info: &vk::ImageViewCreateInfo) -> VkResult<ImageView> {
-		// SAFETY: The object is automatically destroyed.
-		unsafe {
-			Ok(
-				LegacyVulkanObject::new(
-					self.inner.create_image_view(create_info, None)?,
-					self.inner.clone(),
-					|image_view, device| device.destroy_image_view(*image_view, None),
-				)
-			)
-		}
+		unsafe { ImageView::new(create_info, self.inner.clone()) }
 	}
 }
 

src/client/rendering/vulkan/object.rs

@@ -1,26 +1,41 @@
 //! A set of macros and abstractions for creating new Vulkan Objects.
 
-use ash::{ext, vk};
+use std::rc::Rc;
+
+use ash::{ext, khr, vk};
+use sigill_derive::{Deref, DerefMut};
+use vk_mem::Alloc;
+use winit::raw_window_handle::{RawDisplayHandle, RawWindowHandle};
 
 #[macro_export]
+/// A macro that reduces boilerplate by defining a struct with [`VulkanObject`].
+/// 
+/// Please note that any code inside `create` and `destroy` are marked as unsafe for convenience.
+/// Additionally, the destroy method should *always* look like the code below. That is, the
+/// parameter should never be anything but `self`. This is due to macro hygiene requirements.
+/// ```no_run
+/// fn destroy(self) {
+/// 	// ...
+/// }
+/// ```
 macro_rules! vulkan_object {
 	(
 		$name:ident<$create_info:ty, $object:ty, $data:ty>:
 		fn create($create_info_name:ident, $data_name:ident) $create:block;
-		fn destroy($self:ident) $destroy:block;
+		fn destroy($self:ident) $destroy:block; // The $self is a hack that is required for macro hygiene. Thanks John Rust.
 	) => {
 		#[derive(sigill_derive::Deref, sigill_derive::DerefMut)]
 		pub struct $name {
 			object: $object,
-			data: $data,
+			$data_name: $data,
 		}
 
 		impl $crate::client::rendering::vulkan::object::VulkanObject<$create_info, $object, $data> for $name {
-			unsafe fn new(create_info: &$create_info, data: $data) -> ash::prelude::VkResult<Self> {
+			unsafe fn new($create_info_name: &$create_info, $data_name: $data) -> ash::prelude::VkResult<Self> {
 				Ok(
 					Self {
-						object: unsafe { Self::create(create_info, &data)? },
-						data,
+						object: unsafe { Self::create($create_info_name, &$data_name)? },
+						$data_name,
 					}
 				)
 			}
@@ -42,6 +57,20 @@ macro_rules! vulkan_object {
 	};
 }
 
+/// A weak reference to a Vulkan object.
+/// Typically, this type is used when the lifetime of another type is tied to the underlying
+/// Vulkan object, so using [`VulkanObject`] is unnecessary or unsound. Thus, [`WeakRef`]
+/// fills that niche.
+/// 
+/// # Implementation
+/// The underlying type is the inner object of a [`VulkanObject`].
+/// 
+/// # Safety
+/// You must ensure that the underlying Vulkan object is still
+/// initialized in Vulkan before using it.
+#[derive(Deref, DerefMut)]
+pub struct WeakRef<T>(T);
+
 /// An object with a custom destructor.
 /// This struct is used for Vulkan objects that require special allocation handling.
 /// # Necessity
@@ -63,6 +92,18 @@ pub trait VulkanObject<C, T, D>: Sized {
 	unsafe fn destroy(&mut self);
 }
 
+/// A "functional trait" for [`VulkanObject`] implementors that returns a weak reference
+/// to the underlying Vulkan object.
+pub trait WeakObject<T> {
+	/// # Safety
+	/// Using [`WeakRef`] has some extra safety concerns.
+	unsafe fn object(&self) -> WeakRef<T>;
+
+	/// # Safety
+	/// See [`WeakRef`].
+	unsafe fn from_object(object: T) -> Self;
+}
+
 vulkan_object!{
 	DebugUtilsMessenger<vk::DebugUtilsMessengerCreateInfoEXT<'_>, vk::DebugUtilsMessengerEXT, ext::debug_utils::Instance>:
 	fn create(create_info, data) {
@@ -72,3 +113,79 @@ vulkan_object!{
 		self.data.destroy_debug_utils_messenger(self.object, None)
 	};
 }
+
+vulkan_object!{
+	Surface<(RawDisplayHandle, RawWindowHandle), vk::SurfaceKHR, (khr::surface::Instance, ash::Entry, ash::Instance)>:
+	fn create(handles, data) {
+		ash_window::create_surface(&data.1, &data.2, handles.0, handles.1, None)
+	};
+	fn destroy(self) {
+		self.data.0.destroy_surface(self.object, None);
+	};
+}
+
+vulkan_object!{
+	ImageView<vk::ImageViewCreateInfo<'_>, vk::ImageView, ash::Device>:
+	fn create(create_info, device) {
+		device.create_image_view(create_info, None)
+	};
+	fn destroy(self) {
+		self.device.destroy_image_view(self.object, None);
+	};
+}
+
+pub struct Image {
+	object: vk::Image,
+	allocation: Option<vk_mem::Allocation>,
+	allocator: Option<Rc<vk_mem::Allocator>>,
+}
+
+impl VulkanObject<
+	(vk::ImageCreateInfo<'_>, vk_mem::AllocationCreateInfo),
+	(vk::Image, vk_mem::Allocation),
+	Rc<vk_mem::Allocator>,
+> for Image {
+	unsafe fn new(
+		create_info: &(vk::ImageCreateInfo<'_>, vk_mem::AllocationCreateInfo),
+		allocator: Rc<vk_mem::Allocator>,
+	) -> ash::prelude::VkResult<Self> {
+		let (object, allocation) = unsafe { Self::create(create_info, &allocator)? };
+		Ok(
+			Self {
+				object,
+				allocation: Some(allocation),
+				allocator: Some(allocator),
+			}
+		)
+	}
+
+	unsafe fn create(
+		create_info: &(vk::ImageCreateInfo<'_>, vk_mem::AllocationCreateInfo),
+		allocator: &Rc<vk_mem::Allocator>
+	) -> ash::prelude::VkResult<(vk::Image, vk_mem::Allocation)> {
+		let (create_info, allocation_create_info) = create_info;
+		unsafe { allocator.create_image(create_info, allocation_create_info) }
+	}
+
+	unsafe fn destroy(&mut self) {
+		if let Some(allocator) = &self.allocator {
+			if let Some(allocation) = &mut self.allocation {
+				unsafe { allocator.destroy_image(self.object, allocation); }
+			}
+		}
+	}
+}
+
+impl WeakObject<vk::Image> for Image {
+	unsafe fn object(&self) -> WeakRef<vk::Image> {
+		WeakRef(self.object.clone())
+	}
+
+	unsafe fn from_object(object: vk::Image) -> Self {
+		Self {
+			object,
+			allocation: None,
+			allocator: None,
+		}
+	}
+}