vk: handle non-error cases for swapchain acquire#9909
Conversation
poweifeng
force-pushed
the
pf/vk-better-handle-swapchain-acquire
branch
from
971bf73 to
bfadb8b
Compare
| VkResult result = VK_NOT_READY; | ||
| size_t acquireTryCount = 0; | ||
|
|
||
| while (acquireTryCount++ < MAX_ACQUIRE_TRY_COUNT && result != VK_SUCCESS) { |
There was a problem hiding this comment.
why do we need to retry? and also if you flushed/waited already, why do it again?
There was a problem hiding this comment.
why do we need to retry?
I'm just trying to account for the case where a user is drag-resizing a window, in which case, I imagine we might hit multiple VK_SUBOPTIMAL_KHR or VK_ERROR_OUT_OF_DATE_KHR. But that's just my guess and not based on any particular platform.
and also if you flushed/waited already, why do it again?
That's a good point, but flush() and wait() is essentially no-op when there is no buffer being recorded. So calling it multiple times is fine.
There was a problem hiding this comment.
let me know if you prefer changes to either of the above.
There was a problem hiding this comment.
So I think we should try it once and that's it.
In the case of out of date, it it fails, we just record, submit and wont present anything.
Will that break anything?
There was a problem hiding this comment.
so I had to do more work, but I added some guards so that calls that concern the renderpass are skipped, because we cannot construct a valid renderpass when the acquire fails.
| // Check if the swapchain should be resized. | ||
| if ((resized = mPlatform->hasResized(swapChain))) { | ||
| auto recreateSwapchain = [&]() { | ||
| recreated = true; |
There was a problem hiding this comment.
recreated is only updated to true throughout the code flow. do we need to set it to false as default at the beginning of this function?
poweifeng
force-pushed
the
pf/vk-better-handle-swapchain-acquire
branch
2 times, most recently
from
4ee3d3d to
3654bfd
Compare
poweifeng
force-pushed
the
pf/vk-better-handle-swapchain-acquire
branch
from
3654bfd to
2ccecba
Compare
| // Used for cases where the backing swapchain needs to be recreated. | ||
| auto recreate = [&]() { | ||
| // Calling flush multiptle times is ok, since it's no-op if not recording. | ||
| if (mFlushAndWaitOnResize) { |
There was a problem hiding this comment.
I think its better to just move this function inside the if, easier to read it.
| auto swapChain = resource_ptr<VulkanSwapChain>::cast(&mResourceManager, sch); | ||
|
|
||
| // Present the backbuffer after the most recent command buffer submission has finished. | ||
| swapChain->present(*this); |
There was a problem hiding this comment.
what about here? in the case of skip, there's no need to do present but do we still have to be aware that things have to be submitted?
There was a problem hiding this comment.
it'll be no-op since the swapchain was not acquired. Other commands will be submitted with the flush call, or on the next frame
When acquiring an image in a swapchain, we could get VK_ERROR_OUT_OF_DATE_KHR or VK_SUBOPTIMAL_KHR for acceptable states like window resizing. For those cases, instead of failing to acquire, we recreate the swapchain and try to acquire swapchain image again. In case where acquire fails even after the recreation, then we simply don't draw(). Because acquire happens on beginRenderPass (we need backing images for the fbo), if acquire fails, then we assume the renderpass is also invalid, and subsquent calls to make use of it (e.g. bindPipeline) would just be no-op. Fixes #9732
poweifeng
force-pushed
the
pf/vk-better-handle-swapchain-acquire
branch
from
2ccecba to
1d13c81
Compare
4 participants
When acquiring an image in a swapchain, we could get VK_ERROR_OUT_OF_DATE_KHR or VK_SUBOPTIMAL_KHR for acceptable states like window resizing. For those cases, instead of failing to acquire, we recreate the swapchain and try to acquire swapchain image again
Fixes #9732