Closed SRSaunders closed 1 month ago
Creating the very same handle a second time seems a bit weird! Even as a workaround. Do we really want to have such a strange pattern in this repository?
Creating the very same handle a second time seems a bit weird! Even as a workaround. Do we really want to have such a strange pattern in this repository?
@asuessenbach I understand your perspective. My approach here was to log the problem and ask for assistance from a few knowledgeable experts regarding macOS-specifics (see defect #1149). I spent some time tracing around in the debugger and it seems that swapchain old resource destruction and new resource creation can happen out-of-order. It seems to be the final destroy of old image resources (after creation of new ones) that causes the blank screen. While this is a possible clue, I don't know enough about MoltenVK internals to determine the root cause. I felt that putting this workaround up may provide additional information that could help diagnose the problem. Note that the second call does create a new handle instance, but with no memory of image resources from the previous configuration.
@asuessenbach - I'm following up here with some new information and a code update.
It seems like MoltenVK behaves differently than native Vulkan implementations when rebuilding the swapchain for reasons other than resizing. On Windows and Linux, swapchain image count, image usage flags, and transform changes seem to be transparent to vkAcquireNextImageKHR()
which returns VK_SUCCESS
on the next acquire operation. However, with MoltenVK, the next call to vkAcquireNextImageKHR()
following a swapchain change as above (i.e. not a resize) results in VK_SUBOPTIMAL_KHR
. The existing logic in Vulkan-Samples does not handle this properly, other than the temporary band-aid I applied in an earlier PR which allowed the suboptimal return code without bailing out. However that was not really the right approach and is something I am trying to correct here.
In my latest changes I have removed the awkward "double handle create" approach you rightly commented on, and instead added code to [HPP]RenderContext::begin_frame()
which calls handle_surface_changes(true)
on macOS/iOS for a forced update when VK_SUBOPTIMAL_KHR
or VK_ERROR_OUT_OF_DATE_KHR
is returned by vkAcquireNextImageKHR()
. This recreates the swapchain again and cleans up the blank screen issue. Hopefully this approach is a bit more logical and clear.
I also noticed a defect in the way [HPP]RenderContext::begin_frame()
calls swapchain->acquire_next_image()
a second time using the same semaphore from the first call. This is not correct since that semaphore will already be signaled and cannot be used again until cleared by a queue wait operation. To address this, I added code to destroy the semaphore and recreate it in a non-signaled state. This corrects the problem and I have tested to make sure it works properly.
Merging - 3 approvals
Description
This PR does
twothree things:image_count
,image_usage_flags
, ortransform
parameters are changing (i.e. without resizing). This workaround recreates the swapchain a second time within[HPP]RenderContext::begin_frame()
specifically in cases whenvkAcquireNextImageKHR()
returnsVK_SUBOPTIMAL_KHR
. This fixes the blank screen problem when options are selected in certain samples (_afbc, msaa, swapchain_images, hpp_swapchain_images, and surfacerotation).VK_SUBOPTIMAL_KHR
return codes within[HPP]RenderContext::begin_frame()
. TheseVK_SUBOPTIMAL_KHR
return codes were due to swapchain recreation problems in item 1 above. With these now addressed, the code allowingVK_SUBOPTIMAL_KHR
is no longer needed.[HPP]RenderContext::begin_frame()
callsswapchain->acquire_next_image()
a second time using the same semaphore from the first call. This is not correct since that semaphore will already be signaled and cannot be used again until cleared by a queue wait operation. To address this, I added code to destroy the semaphore and recreate it in a non-signaled state.Fixes #1149
Tested on macOS Ventura 13.6.6, iOS 17.6.1, and Windows 10 using an AMD 6600XT GPU.
General Checklist:
Please ensure the following points are checked:
[X] This PR describes the scope and expected impact of the changes I am making
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batch
command line argument to make sure all samples still work properlySample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: