Nabla icon indicating copy to clipboard operation
Nabla copied to clipboard

Circular dependency between swapchain and its images

Open achalpandeyy opened this issue 4 years ago • 11 comments

ISwapchain has refctd pointers to IGPUImage (m_images member) and CVulkanForeignImage (a derivative of IGPUImage) wants to have a refctd pointer to CVulkanSwapchain (a derivative of ISwapchain).

achalpandeyy avatar Nov 01 '21 07:11 achalpandeyy

  1. Is it plausible to make ISwapchain templated on the type of pointers (raw or refctd) it uses to keep its images? If the images are created externally (Vulkan's Presentation Engine, Unreal, Qt, whatever) we can use raw pointers to IGPUImage otherwise smart_refctd_ptr. Just a first thought.

  2. There aren't any use cases where a swapchain image needs to keep the swapchain alive so can we ditch the refctd backpointer from image to swapchain?

achalpandeyy avatar Nov 01 '21 07:11 achalpandeyy

There aren't any use cases where a swapchain image needs to keep the swapchain alive so can we ditch the refctd backpointer from image to swapchain?

I guess we need to know the pointer to the swapchain to keep on presenting it and acquiring images.

Buut, it would blow up the engine if the IGPUImage came from a swapchain, and didn't keep it alive (suddenly the IGPUImage, its views, descriptor sets that have it inside would all be dead or invalid).

https://github.com/Devsh-Graphics-Programming/Nabla/pull/306#issuecomment-1025665615

Erfan-Ahmadi avatar Jan 31 '22 14:01 Erfan-Ahmadi

From the glorious Vulkan spec.

The application must not destroy a swapchain until after completion of all outstanding operations on images that were acquired from the swapchain. swapchain and all associated VkImage handles are destroyed, and must not be acquired or used any more by the application. The memory of each VkImage will only be freed after that image is no longer used by the presentation engine. For example, if one image of the swapchain is being displayed in a window, the memory for that image may not be freed until the window is destroyed, or another swapchain is created for the window. Destroying the swapchain does not invalidate the parent VkSurfaceKHR, and a new swapchain can be created with it.

When a swapchain associated with a display surface is destroyed, if the image most recently presented to the display surface is from the swapchain being destroyed, then either any display resources modified by presenting images from any swapchain associated with the display surface must be reverted by the implementation to their state prior to the first present performed on one of these swapchains, or such resources must be left in their current state.

Further, the lifetime of presentable images is controlled by the implementation, so applications must not destroy a presentable image. See vkDestroySwapchainKHR for further details on the lifetime of presentable images.

Images can also be created by using vkCreateImage with VkImageSwapchainCreateInfoKHR and bound to swapchain memory using vkBindImageMemory2 with VkBindImageMemorySwapchainInfoKHR. These images can be used anywhere swapchain images are used, and are useful in logical devices with multiple physical devices to create peer memory bindings of swapchain memory. These images and bindings have no effect on what memory is presented. Unlike images retrieved from vkGetSwapchainImagesKHR, these images must be destroyed with vkDestroyImage.

CVulkanForeignImage needs to be rewritten from scratch

#Lifetime Tracking

Scrap CVulkanForeignImage

It doesn't really fit any use case

General Requirement

Requirement 1: We want the use of a swapchain image (retrieved view or created) to keep a swapchain alive (prevent destruction). Requirement 2: Avoid turning IReferenceCounted's grab() and drop() methods into virtuals, at all cost. Requirement 3: Swapchain cannot die until all acquired images have been presented (easy, this->grab() on acquire, this->drop() on present)

Retrieved ImageView

Requirement 1: Cannot destroy Vulkan object corresponding to self. Requirement 2: But cannot stay around after swapchain is destroyed.

Created Image

Requirement 1: Must destroy Vulkan object corresponding to self BEFORE swapchain is destroyed.

Corollaries

Make the Image Creation for Swapchain a Swapchain Method

Swapchain already knows which ILogicalDevice it belongs to, it can execute "special" image creation and memory binding functions.

Derive ISwapchain from IDeviceMemoryAllocation

It makes it easy for a "Created Image" to keep a swapchain alive, also properly signifies the semantics that the swapchain provides memory for the image.

Obviously make it report 0 length, size, mapping caps, dedicated=true, etc so it can't actually be used for binding it to anything else.

For a Retrieved Image, copy the CFileArchive "cached creation" scheme

In OpenGL we would either have to disable this option OR we could boot it from the engine entirely (or create and cache the views always).

Derive CRetrievedImageView from your desired backend class (CVulkanImageView or COpenGLImageView), solely for the purpose of having placement operator new and operator delete overloads that take an extra paramter to the swapchain.

Let the memory for the storage of the CVulkanImageViews come from a ISwapchain member (just some malloc).

When someone wants to "retrieve" an image:

  1. speculatively grab() the casted ImageView
  2. if old counter value was 0, it means object didn't exist, else return the smart pointer constructed with core::dont_grab (you already grabbed)
  3. placement new overload will grab() the swapchain and use a custom ctor which provides the VkImageView/GLuint and does not create the resource and sets a flag not to destroy it
  4. return smart pointer constructed with core::dont_grab as we always do when newing stuff (cause refcount inits to 1)

Now when all usages of all retrieved images are gone, they destruct, but they don't destruct the underlying API object (the IGPUImageView container can be recreated, but handle stays the same) so it can be retrieved again. If nothing else is holding onto the swapchain, it dies as it should.

@deprilula28 let us know of any downsides to going only the "Created Image" route

Scrap CVulkanForeignImage

It doesn't really fit any use case

General Requirement

Requirement 1: We want the use of a swapchain image (retrieved view or created) to keep a swapchain alive (prevent destruction). Requirement 2: Avoid turning IReferenceCounted's grab() and drop() methods into virtuals, at all cost. Requirement 3: Swapchain cannot die until all acquired images have been presented (easy, this->grab() on acquire, this->drop() on present)

Retrieved ImageView

Requirement 1: Cannot destroy Vulkan object corresponding to self. Requirement 2: But cannot stay around after swapchain is destroyed.

Created Image

Requirement 1: Must destroy Vulkan object corresponding to self BEFORE swapchain is destroyed.

Corollaries

Make the Image Creation for Swapchain a Swapchain Method

Swapchain already knows which ILogicalDevice it belongs to, it can execute "special" image creation and memory binding functions.

Derive ISwapchain from IDeviceMemoryAllocation

It makes it easy for a "Created Image" to keep a swapchain alive, also properly signifies the semantics that the swapchain provides memory for the image.

Obviously make it report 0 length, size, mapping caps, dedicated=true, etc so it can't actually be used for binding it to anything else.

For a Retrieved Image, copy the CFileArchive "cached creation" scheme

In OpenGL we would either have to disable this option OR we could boot it from the engine entirely (or create and cache the views always).

Derive CRetrievedImageView from your desired backend class (CVulkanImageView or COpenGLImageView), solely for the purpose of having placement operator new and operator delete overloads that take an extra paramter to the swapchain.

Let the memory for the storage of the CVulkanImageViews come from a ISwapchain member (just some malloc).

When someone wants to "retrieve" an image:

1. speculatively `grab()` the casted ImageView

2. if old counter value was 0, it means object didn't exist, else return the smart pointer constructed with `core::dont_grab` (you already grabbed)

3. placement new overload will `grab()` the swapchain and use a custom ctor which provides the `VkImageView/GLuint` and does not create the resource and sets a flag not to destroy it

4. return smart pointer constructed with `core::dont_grab` as we always do when `new`ing stuff (cause refcount inits to 1)

Now when all usages of all retrieved images are gone, they destruct, but they don't destruct the underlying API object (the IGPUImageView container can be recreated, but handle stays the same) so it can be retrieved again. If nothing else is holding onto the swapchain, it dies as it should.

outdated and obsoleted by discord conversation