Vulkan: return to more conventional swapchain sync method, encapsulate more code #525

Merged
goeiecool9999 merged 10 commits from fixchainimagecount into main 2022-11-25 05:51:47 -03:00
goeiecool9999 commented 2022-11-22 09:54:26 -03:00 (Migrated from github.com)

#502 adopted a swapchain sync method that is correct in theory but is not commonly used.
This changes the sync method back to what was used before.

The validation layer appears to not have made up their mind about whether using only a fence is valid and one of the authors of the vulkan specification says it is unintended and basically advises against it here

This PR also ups the minimum swapchain image count to at least two images (not min+1 which might be > 2 causing unnecessary input lag and wasting resources). Using only one swapchain image caused a freeze on AMD windows drivers when switching to fullscreen.
One of the issues that popped up after removing the acquire semaphore is that on macOS the fonts in the imgui overlay disappeared.
I do not have a concrete answer as to why moltenVK has issues while other platforms do not. It may be that moltenVK assumes swapchain image access is guarded with a semaphore.

Additionally some more swapchain code is moved from VulkanRenderer into the SwapchainInfoVk class. There is likely more code that could be moved there before the encapsulation is optimal but that would make the scope of the PR a bit too large.

#502 adopted a swapchain sync method that is correct in theory but is not commonly used. This changes the sync method back to what was used before. The [validation layer](https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/12) appears to not have made up their mind about whether using only a fence is valid and one of the authors of the vulkan specification says it is unintended and basically advises against it [here](https://github.com/KhronosGroup/Vulkan-Docs/issues/117#issuecomment-200357476) This PR also ups the minimum swapchain image count to at least two images (not min+1 which might be > 2 causing unnecessary input lag and wasting resources). Using only one swapchain image caused a freeze on AMD windows drivers when switching to fullscreen. One of the issues that popped up after removing the acquire semaphore is that on macOS the fonts in the imgui overlay disappeared. I do not have a concrete answer as to why moltenVK has issues while other platforms do not. It may be that moltenVK assumes swapchain image access is guarded with a semaphore. Additionally some more swapchain code is moved from VulkanRenderer into the SwapchainInfoVk class. There is likely more code that could be moved there before the encapsulation is optimal but that would make the scope of the PR a bit too large.
Exzap commented 2022-11-24 06:59:40 -03:00 (Migrated from github.com)

Since you already introduced SwapchainInfoVk you should use methods and not expose m_imageAvailableFence to VulkanRenderer directly.

I'd also prefer if we could avoid the usage of a timeout for vkWaitForFences in RecreateSwapchain(). Whether the fence is going to be signaled or not should always be a well defined state?

Since you already introduced `SwapchainInfoVk` you should use methods and not expose `m_imageAvailableFence` to VulkanRenderer directly. I'd also prefer if we could avoid the usage of a timeout for `vkWaitForFences` in `RecreateSwapchain()`. Whether the fence is going to be signaled or not should always be a well defined state?
goeiecool9999 commented 2022-11-24 09:55:18 -03:00 (Migrated from github.com)

I think this moves the encapsulation of the swapchain in the right direction. I think it could go further but that would require more effort

I think this moves the encapsulation of the swapchain in the right direction. I think it could go further but that would require more effort
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: cemu-project_Mirror/Cemu-2024-03-05#525
No description provided.