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

Treat VK_SUBOPTIMAL_KHR as VK_SUCCESS on Android. #3525

Merged

Conversation

James2022-rgb
Copy link
Contributor

@James2022-rgb James2022-rgb commented Feb 23, 2023

Checklist

  • Run cargo clippy.
  • Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections
Addresses issue #3345.

Description
This PR makes is so that vkQueuePresentKHR's VK_SUBOPTIMAL_KHR is treated as VK_SUCCESS (i.e ignored and not logged) on Android.

On Android 10+, vkQueuePresentKHR returns VK_SUBOPTIMAL_KHR if the swapchain's VkSwapchainCreateInfoKHR::preTransform does not match the device's current orientation.
This is always the case in wgpu when the device orientation is anything other than the identity one,
because we unconditionally use VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR when creating the swapchain.

The assumption is that on Android, as discussed in the issue, VK_SUBOPTIMAL_KHR is only returned when the device's current orientation doesn't match VkSwapchainCreateInfoKHR::preTransform.

https://developer.android.com/games/optimize/vulkan-prerotation#detect_device_orientation_changes

The most reliable way to detect an orientation change in your application is to verify whether the vkQueuePresentKHR() function returns VK_SUBOPTIMAL_KHR.

The only code path that returns VK_SUBOPTIMAL_KHR in Android libvulkan's vkQueuePresentKHR implementation:
https://android.googlesource.com/platform/frameworks/native/+/master/vulkan/libvulkan/swapchain.cpp#2021

More in-detail description of the problem in the connected issue.

Testing
Tested on my Galaxy S20 Ultra 5G (Adreno 650), Android 10, that the log spam is no longer exhibited.

Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Looks good but there is one more spot where we should ignore this.
Search for acquire_next_image in the codebase.

@James2022-rgb
Copy link
Contributor Author

Added a commit to ignore VK_SUBOPTIMAL_KHR for vkAcquireNextImageKHR as well!

Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants