CreateSwapChain Fix #487

MathewBensonCode · 2026-01-15T22:45:14Z

Rewrote the Swapchain logic to use the size of the window and to use these dimensions to clamp against the device capabilities.
Also used the push method to add image views
Did some housekeeping in the VulkanDevice.cpp source file to fix warnings that we're being generated.

This is an alternate approach to pull request #479.

It was also originally presented in pull request #480, but has since been reverted

it closes #486

JeanPhilippeKernel · 2026-01-16T06:57:30Z

ZEngine/ZEngine/Hardwares/VulkanDevice.cpp

-        uint32_t image_count = 0;
-        ZENGINE_VALIDATE_ASSERT(vkGetSwapchainImagesKHR(LogicalDevice, SwapchainHandle, &image_count, nullptr) == VK_SUCCESS, "Failed to get Images count from Swapchain")
-
-        if (image_count < SwapchainImageCount)
-        {
-            ZENGINE_CORE_WARN("Max Swapchain image count supported is {}, but requested {}", image_count, SwapchainImageCount);
-            SwapchainImageCount = image_count;
-            ZENGINE_CORE_WARN("Swapchain image count has changed from {} to {}", SwapchainImageCount, image_count);
-        }


SwapchainImageCount serves as a hint for the desired image count we want - likely we want 3 images for a triple-buffering.

But some GPUs we can only provide 2 images, so double-buffering. in that case we want to know - adjust and be warned that the engine will run with that since it's below the standard we desire.

I have taken some time to go over this. I have remembered why I took out this section. The problem I encountered is that the image_count obtained from the device was higher than the SwapchainImageCount that was specified at construction time. So there ended up being a mis-match between the SwapchainImages in the device and what was being specified by the application.

This is the reason I went with the option of using push as it may not be possible to know the actual number of Swapchain Images that the device would return.

oh I see -

that means we will need to re-adjust some sections of the whole code - mostly the due the scratch Arena

Here is the proposal:

PS: Note the usage of 2 Temp Arena

auto scratch = ZGetScratch(Arena); Array<uint32_t> family_indice = {}; uint32_t family_indice_count = HasSeperateTransfertQueueFamily ? 2 : 1; family_indice.init(scratch.Arena, family_indice_count, family_indice_count); family_indice[0] = GraphicFamilyIndex; if (HasSeperateTransfertQueueFamily) { family_indice[1] = TransferFamilyIndex; } swapchain_create_info.imageSharingMode = HasSeperateTransfertQueueFamily ? VK_SHARING_MODE_CONCURRENT : VK_SHARING_MODE_EXCLUSIVE; swapchain_create_info.queueFamilyIndexCount = HasSeperateTransfertQueueFamily ? 2 : 1; swapchain_create_info.pQueueFamilyIndices = family_indice.data(); ZENGINE_VALIDATE_ASSERT(vkCreateSwapchainKHR(LogicalDevice, &swapchain_create_info, nullptr, &SwapchainHandle) == VK_SUCCESS, "Failed to create Swapchain") ZReleaseScratch(scratch); uint32_t image_count = 0; ZENGINE_VALIDATE_ASSERT(vkGetSwapchainImagesKHR(LogicalDevice, SwapchainHandle, &image_count, nullptr) == VK_SUCCESS, "Failed to get Images count from Swapchain") if (image_count != SwapchainImageCount) { ZENGINE_CORE_WARN("Max Swapchain image count supported is {}, but requested {}", image_count, SwapchainImageCount); SwapchainImageCount = image_count; ZENGINE_CORE_WARN("Swapchain image count has changed from {} to {}", SwapchainImageCount, image_count); } if (SwapchainImageViews.capacity() <= 0) { SwapchainImageViews.init(Arena, SwapchainImageCount, SwapchainImageCount); } if (SwapchainFramebuffers.capacity() <= 0) { SwapchainFramebuffers.init(Arena, SwapchainImageCount, SwapchainImageCount); } scratch = ZGetScratch(Arena); Array<VkImage> SwapchainImages = {}; SwapchainImages.init(scratch.Arena, SwapchainImageCount, SwapchainImageCount); ZENGINE_VALIDATE_ASSERT(vkGetSwapchainImagesKHR(LogicalDevice, SwapchainHandle, &SwapchainImageCount, SwapchainImages.data()) == VK_SUCCESS, "Failed to get VkImages from Swapchain") /*Transition Image from Undefined to Present_src*/ auto command_buffer = GetInstantCommandBuffer(Rendering::QueueType::GRAPHIC_QUEUE); { for (int i = 0; i < SwapchainImages.size(); ++i) { Rendering::Specifications::ImageMemoryBarrierSpecification barrier_spec = {}; barrier_spec.ImageHandle = SwapchainImages[i]; barrier_spec.OldLayout = Specifications::ImageLayout::UNDEFINED; barrier_spec.NewLayout = Specifications::ImageLayout::PRESENT_SRC; barrier_spec.ImageAspectMask = VK_IMAGE_ASPECT_COLOR_BIT; barrier_spec.SourceAccessMask = 0; barrier_spec.DestinationAccessMask = VK_ACCESS_MEMORY_READ_BIT; barrier_spec.SourceStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; barrier_spec.DestinationStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; barrier_spec.LayerCount = 1; Rendering::Primitives::ImageMemoryBarrier barrier{barrier_spec}; command_buffer->TransitionImageLayout(barrier); } } EnqueueInstantCommandBuffer(command_buffer); for (int i = 0; i < SwapchainImageCount; ++i) { SwapchainImageViews[i] = CreateImageView(SwapchainImages[i], SurfaceFormat.format, VK_IMAGE_VIEW_TYPE_2D, VK_IMAGE_ASPECT_COLOR_BIT); Array<VkImageView> fb_images_views; fb_images_views.init(scratch.Arena, 1); fb_images_views.push(SwapchainImageViews[i]); SwapchainFramebuffers[i] = CreateFramebuffer(ArrayView{fb_images_views}, SwapchainAttachment->GetHandle(), SwapchainImageWidth, SwapchainImageHeight); } ZReleaseScratch(scratch);

JeanPhilippeKernel · 2026-01-16T07:16:20Z

ZEngine/ZEngine/Hardwares/VulkanDevice.cpp


        if (SwapchainImageViews.capacity() <= 0)
        {
-            SwapchainImageViews.init(Arena, SwapchainImageCount, SwapchainImageCount);


init(Arena, count, count) help to pre-allocate (and zero-ed) the needed mem.

it's useful here, in this context because, once the Temp Arena below auto scratch = ZGetScratch(Arena); is called, all operations ( eg : push(...)) will use it and will get cleaned-up after release it.

But data in SwapchainImageViews and SwapchainFramebuffers need to be persistent till we close the engine.

JeanPhilippeKernel · 2026-01-16T07:17:06Z

ZEngine/ZEngine/Hardwares/VulkanDevice.cpp


        if (SwapchainFramebuffers.capacity() <= 0)
        {
-            SwapchainFramebuffers.init(Arena, SwapchainImageCount, SwapchainImageCount);


Same comment here about the prea-allocate ops before calling the Scratch Arena

JeanPhilippeKernel · 2026-01-16T07:20:52Z

ZEngine/ZEngine/Hardwares/VulkanDevice.cpp

+        SwapchainImageWidth                            = std::clamp(CurrentWindow->GetWidth(), capabilities.minImageExtent.width, capabilities.maxImageExtent.width);
+        SwapchainImageHeight                           = std::clamp(CurrentWindow->GetHeight(), capabilities.minImageExtent.height, capabilities.maxImageExtent.height);


you can't use CurrentWindow->GetWidth() and CurrentWindow->GetHeight() as source of trust for those operations because there is a big chance that its diverge from the capabilities.currentExtent per Hardware and GPU and platforms

The idea I had here was to use clamp to check the value of the CurrentWindow is between the capabilities that we have queried from the device. so the values of GetWidth() and GetHeight() can only be used if they are valid ranges betwen the min and max specified by the capabilities.

Not too sure about this though, this was my understanding of how the clamp algorithm would work in this case.

I understand - but that is not always true , because on some platforms - mostly on macOS, and window + scaling, the reporting value from both Width/Height and capability.currentExtent can be totally diverge.

The source of trust for the swapchain size should be value from capability.currentExtent (or the Framebuffer in some case).

it's okay to do capabilities.currentExtent.width != std::numeric_limits<uint32_t>::max() as it's common in the industry before creating the swapchain

When I wrote this, I was getting some very large sizes in the capabilities.currentExtent for some reason. I just tried it today and the currentExtent, minImageExtent and maxImageExtent are all the same which is the size of the window. So I guess it was a driver bug or something. I will revert this section.

JeanPhilippeKernel · 2026-01-16T07:26:32Z

ZEngine/ZEngine/Hardwares/VulkanDevice.cpp

        for (int i = 0; i < SwapchainImageCount; ++i)
        {
-            SwapchainImageViews[i] = CreateImageView(SwapchainImages[i], SurfaceFormat.format, VK_IMAGE_VIEW_TYPE_2D, VK_IMAGE_ASPECT_COLOR_BIT);
+            SwapchainImageViews.push(CreateImageView(SwapchainImages[i], SurfaceFormat.format, VK_IMAGE_VIEW_TYPE_2D, VK_IMAGE_ASPECT_COLOR_BIT));


you can't call push(...) here because this block is part of Temp Arena, so all allocation will be clean-up once it gets release : ZReleaseScratch(scratch);

noted. will study this more

JeanPhilippeKernel · 2026-01-16T07:27:30Z

ZEngine/ZEngine/Hardwares/VulkanDevice.cpp

            fb_images_views.init(scratch.Arena, 1);
            fb_images_views.push(SwapchainImageViews[i]);
-            SwapchainFramebuffers[i] = CreateFramebuffer(ArrayView{fb_images_views}, SwapchainAttachment->GetHandle(), SwapchainImageWidth, SwapchainImageHeight);
+            SwapchainFramebuffers.push(CreateFramebuffer(ArrayView{fb_images_views}, SwapchainAttachment->GetHandle(), SwapchainImageWidth, SwapchainImageHeight));


Same comment about : can't call push(...) here -

JeanPhilippeKernel · 2026-01-16T07:32:35Z

ZEngine/ZEngine/Hardwares/VulkanDevice.cpp

+        SwapchainImageHeight                           = std::clamp(CurrentWindow->GetHeight(), capabilities.minImageExtent.height, capabilities.maxImageExtent.height);

-        auto                     min_image_count       = std::clamp(capabilities.minImageCount, capabilities.minImageCount + 1, capabilities.maxImageCount);
+        auto                     min_image_count       = SwapchainImageCount < capabilities.minImageCount ? capabilities.minImageCount : SwapchainImageCount;


Like mentioned in other comments, it's preferable to rely on capabilities.[min | max] and adjust SwapchainImageCount with the result later down

JeanPhilippeKernel · 2026-01-16T15:01:39Z

ZEngine/ZEngine/Hardwares/VulkanDevice.cpp

+        SwapchainImageWidth                            = std::clamp(CurrentWindow->GetWidth(), capabilities.minImageExtent.width, capabilities.maxImageExtent.width);
+        SwapchainImageHeight                           = std::clamp(CurrentWindow->GetHeight(), capabilities.minImageExtent.height, capabilities.maxImageExtent.height);


I understand - but that is not always true , because on some platforms - mostly on macOS, and window + scaling, the reporting value from both Width/Height and capability.currentExtent can be totally diverge.

The source of trust for the swapchain size should be value from capability.currentExtent (or the Framebuffer in some case).

it's okay to do capabilities.currentExtent.width != std::numeric_limits<uint32_t>::max() as it's common in the industry before creating the swapchain

- Guard against device capabilities with maxImageCount == 0, which is defined to mean that there is no limit to the number of images, the only limitation is memory availability. - Rearranged code to ensure consistency in the number of swapchains created to match capability of device. - We check the number of swapchains supported and allocate memory to match this number.

JeanPhilippeKernel · 2026-01-17T00:48:39Z

ZEngine/ZEngine/Hardwares/VulkanDevice.cpp

+        if (SwapchainImageViews.capacity() <= 0)
+        {
+            SwapchainImageViews.init(Arena, SwapchainImageCount, SwapchainImageCount);
+        }
+
+        if (SwapchainFramebuffers.capacity() <= 0)
+        {
+            SwapchainFramebuffers.init(Arena, SwapchainImageCount, SwapchainImageCount);
+        }
+


This will lead to a crash, because it's called under the block of Temp Arena scratch.

and you can't use the allocator Arena here till you release its Temp Arena.

------Arena mem-------|| <-----Temp Arena: Scratch (all allocation here in this zone will be ephemeral)---> || ---Arena mem------

noted. thanks for the explanation on the allocation. there was an issue on one of the vulkan calls because of the mismatch in the "original" swapchain count and the count after obtaining the image count.

I will try and look at it with the allocation in mind.

#487 (comment)

The comment here should provide the right fix -- and helps for more clarity

JeanPhilippeKernel · 2026-01-17T01:04:46Z

ZEngine/ZEngine/Hardwares/VulkanDevice.cpp

-        uint32_t image_count = 0;
-        ZENGINE_VALIDATE_ASSERT(vkGetSwapchainImagesKHR(LogicalDevice, SwapchainHandle, &image_count, nullptr) == VK_SUCCESS, "Failed to get Images count from Swapchain")
-
-        if (image_count < SwapchainImageCount)
-        {
-            ZENGINE_CORE_WARN("Max Swapchain image count supported is {}, but requested {}", image_count, SwapchainImageCount);
-            SwapchainImageCount = image_count;
-            ZENGINE_CORE_WARN("Swapchain image count has changed from {} to {}", SwapchainImageCount, image_count);
-        }


Here is the proposal:

PS: Note the usage of 2 Temp Arena

auto scratch = ZGetScratch(Arena); Array<uint32_t> family_indice = {}; uint32_t family_indice_count = HasSeperateTransfertQueueFamily ? 2 : 1; family_indice.init(scratch.Arena, family_indice_count, family_indice_count); family_indice[0] = GraphicFamilyIndex; if (HasSeperateTransfertQueueFamily) { family_indice[1] = TransferFamilyIndex; } swapchain_create_info.imageSharingMode = HasSeperateTransfertQueueFamily ? VK_SHARING_MODE_CONCURRENT : VK_SHARING_MODE_EXCLUSIVE; swapchain_create_info.queueFamilyIndexCount = HasSeperateTransfertQueueFamily ? 2 : 1; swapchain_create_info.pQueueFamilyIndices = family_indice.data(); ZENGINE_VALIDATE_ASSERT(vkCreateSwapchainKHR(LogicalDevice, &swapchain_create_info, nullptr, &SwapchainHandle) == VK_SUCCESS, "Failed to create Swapchain") ZReleaseScratch(scratch); uint32_t image_count = 0; ZENGINE_VALIDATE_ASSERT(vkGetSwapchainImagesKHR(LogicalDevice, SwapchainHandle, &image_count, nullptr) == VK_SUCCESS, "Failed to get Images count from Swapchain") if (image_count != SwapchainImageCount) { ZENGINE_CORE_WARN("Max Swapchain image count supported is {}, but requested {}", image_count, SwapchainImageCount); SwapchainImageCount = image_count; ZENGINE_CORE_WARN("Swapchain image count has changed from {} to {}", SwapchainImageCount, image_count); } if (SwapchainImageViews.capacity() <= 0) { SwapchainImageViews.init(Arena, SwapchainImageCount, SwapchainImageCount); } if (SwapchainFramebuffers.capacity() <= 0) { SwapchainFramebuffers.init(Arena, SwapchainImageCount, SwapchainImageCount); } scratch = ZGetScratch(Arena); Array<VkImage> SwapchainImages = {}; SwapchainImages.init(scratch.Arena, SwapchainImageCount, SwapchainImageCount); ZENGINE_VALIDATE_ASSERT(vkGetSwapchainImagesKHR(LogicalDevice, SwapchainHandle, &SwapchainImageCount, SwapchainImages.data()) == VK_SUCCESS, "Failed to get VkImages from Swapchain") /*Transition Image from Undefined to Present_src*/ auto command_buffer = GetInstantCommandBuffer(Rendering::QueueType::GRAPHIC_QUEUE); { for (int i = 0; i < SwapchainImages.size(); ++i) { Rendering::Specifications::ImageMemoryBarrierSpecification barrier_spec = {}; barrier_spec.ImageHandle = SwapchainImages[i]; barrier_spec.OldLayout = Specifications::ImageLayout::UNDEFINED; barrier_spec.NewLayout = Specifications::ImageLayout::PRESENT_SRC; barrier_spec.ImageAspectMask = VK_IMAGE_ASPECT_COLOR_BIT; barrier_spec.SourceAccessMask = 0; barrier_spec.DestinationAccessMask = VK_ACCESS_MEMORY_READ_BIT; barrier_spec.SourceStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; barrier_spec.DestinationStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; barrier_spec.LayerCount = 1; Rendering::Primitives::ImageMemoryBarrier barrier{barrier_spec}; command_buffer->TransitionImageLayout(barrier); } } EnqueueInstantCommandBuffer(command_buffer); for (int i = 0; i < SwapchainImageCount; ++i) { SwapchainImageViews[i] = CreateImageView(SwapchainImages[i], SurfaceFormat.format, VK_IMAGE_VIEW_TYPE_2D, VK_IMAGE_ASPECT_COLOR_BIT); Array<VkImageView> fb_images_views; fb_images_views.init(scratch.Arena, 1); fb_images_views.push(SwapchainImageViews[i]); SwapchainFramebuffers[i] = CreateFramebuffer(ArrayView{fb_images_views}, SwapchainAttachment->GetHandle(), SwapchainImageWidth, SwapchainImageHeight); } ZReleaseScratch(scratch);

JeanPhilippeKernel assigned MathewBensonCode Jan 16, 2026

JeanPhilippeKernel added critical vulkan-api area-rendering area-linux Work on Linux system area-window Work on Window system area-macOS Work on macOS system labels Jan 16, 2026

JeanPhilippeKernel added this to ZEngine Board Jan 16, 2026

JeanPhilippeKernel added this to the Born baby (0.2.0) milestone Jan 16, 2026

JeanPhilippeKernel moved this to In Progress in ZEngine Board Jan 16, 2026

JeanPhilippeKernel requested changes Jan 16, 2026

View reviewed changes

MathewBensonCode force-pushed the fix_swapchain branch from 91d76f7 to 4001ea0 Compare January 16, 2026 23:10

MathewBensonCode changed the title ~~CreateSwapChain using Window size dimensions & VulkanDevice.cpp Cleanup~~ CreateSwapChain Fix Jan 17, 2026

JeanPhilippeKernel requested changes Jan 17, 2026

View reviewed changes

		SwapchainImageWidth = std::clamp(CurrentWindow->GetWidth(), capabilities.minImageExtent.width, capabilities.maxImageExtent.width);
		SwapchainImageHeight = std::clamp(CurrentWindow->GetHeight(), capabilities.minImageExtent.height, capabilities.maxImageExtent.height);

Uh oh!

CreateSwapChain Fix #487

Are you sure you want to change the base?

CreateSwapChain Fix #487

Conversation

MathewBensonCode commented Jan 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants