Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work-around macOS/iOS blank screen issues on swapchain recreation #1150

Merged
merged 4 commits into from
Sep 19, 2024

Conversation

SRSaunders
Copy link
Contributor

@SRSaunders SRSaunders commented Sep 3, 2024

Description

This PR does two three things:

  1. Adds workarounds on macOS/iOS only for swapchain recreation when only image_count, image_usage_flags, or transform parameters are changing (i.e. without resizing). This workaround recreates the swapchain a second time within [HPP]RenderContext::begin_frame() specifically in cases when vkAcquireNextImageKHR() returns VK_SUBOPTIMAL_KHR. This fixes the blank screen problem when options are selected in certain samples (afbc, msaa, swapchain_images, hpp_swapchain_images, and surface_rotation).
  2. Reverts the previous workarounds from PR Add Layer Settings API to framework, improve batch mode error recovery, fix macOS issues #1084 which permitted VK_SUBOPTIMAL_KHR return codes within [HPP]RenderContext::begin_frame(). These VK_SUBOPTIMAL_KHR return codes were due to swapchain recreation problems in item 1 above. With these now addressed, the code allowing VK_SUBOPTIMAL_KHR is no longer needed.
  3. UPDATE: Fixes 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.

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:

  • My code follows the coding style
  • I have reviewed file licenses
  • I have commented any added functions (in line with Doxygen)
  • I have commented any code that could be hard to understand
  • My changes do not add any new compiler warnings
  • My changes do not add any new validation layer errors or warnings
  • I have used existing framework/helper functions where possible
  • My changes do not add any regressions
  • I have tested every sample to ensure everything runs correctly
  • This PR describes the scope and expected impact of the changes I am making

Note: The Samples CI runs a number of checks including:

  • I have updated the header Copyright to reflect the current year (CI build will fail if Copyright is out of date)
  • My changes build on Windows, Linux, macOS and Android. Otherwise I have documented any exceptions

If this PR contains framework changes:

  • I did a full batch run using the batch command line argument to make sure all samples still work properly

Sample Checklist

If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist:

  • I have tested the sample on at least one compliant Vulkan implementation
  • If the sample is vendor-specific, I have tagged it appropriately
  • I have stated on what implementation the sample has been tested so that others can test on different implementations and platforms
  • Any dependent assets have been merged and published in downstream modules
  • For new samples, I have added a paragraph with a summary to the appropriate chapter in the readme of the folder that the sample belongs to e.g. api samples readme
  • For new samples, I have added a tutorial README.md file to guide users through what they need to know to implement code using this feature. For example, see conditional_rendering
  • For new samples, I have added a link to the Antora navigation so that the sample will be listed at the Vulkan documentation site

@asuessenbach
Copy link
Contributor

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?

@SRSaunders
Copy link
Contributor Author

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.

@SRSaunders
Copy link
Contributor Author

SRSaunders commented Sep 4, 2024

@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.

Copy link
Contributor

@asuessenbach asuessenbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks much, well, more logical. Thanks.
Just one remaining issue.

framework/rendering/hpp_render_context.cpp Show resolved Hide resolved
@marty-johnson59
Copy link
Contributor

Merging - 3 approvals

@marty-johnson59 marty-johnson59 merged commit ef2a690 into KhronosGroup:main Sep 19, 2024
23 checks passed
@SRSaunders SRSaunders deleted the vksubopt-fixes branch September 19, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apple devices can display blank screens following swapchain recreation
5 participants