-
Notifications
You must be signed in to change notification settings - Fork 724
Auto enable vkb enable portability #497
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
Merged
marty-johnson59
merged 13 commits into
KhronosGroup:master
from
gpx1000:Auto_enable_VKB_ENABLE_PORTABILITY
Aug 19, 2022
Merged
Auto enable vkb enable portability #497
marty-johnson59
merged 13 commits into
KhronosGroup:master
from
gpx1000:Auto_enable_VKB_ENABLE_PORTABILITY
Aug 19, 2022
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…f and only if Vulkan_VERSION is at least 1.3 which should include the required portability extension which will enable the instance to find the non-conformant vulkan for moltenVK.
TomAtkinsonArm
previously approved these changes
Jul 19, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Tested on Monterey 12.4
…As in the case when extensions required are not present.
NB: This *might* mean that Android on OSX would have an issue due to the version of CMake that Android SDK might ship with is 3.18. Revisit this once most people are on Vulkan 1.3 and/or confirmation that android on OSX is a problem.
NB: This *might* mean that Android on OSX would have an issue due to the version of CMake that Android SDK might ship with is 3.18. Revisit this once most people are on Vulkan 1.3 and/or confirmation that android on OSX is a problem.
…es which don't agree with the CICD white space results.
Merging as this is a simple fix for a significant blocking issue. LMK if there are any problems - we can revert if needed. |
marty-johnson59
added a commit
that referenced
this pull request
Sep 12, 2022
* Use <vulkan/vulkan.hpp> as precompiled header. * Auto enable vkb enable portability (#497) * Fix issue #452 * Attempt some logic to make setting of VKB_ENABLE_PORTABILITY happen if and only if Vulkan_VERSION is at least 1.3 which should include the required portability extension which will enable the instance to find the non-conformant vulkan for moltenVK. * update copyright. * make the compile definition explicit. * Validated as working with Vulkan 1.3.216.0 on MacBook Air M2. * enable vulkan.hpp samples to work. * don't crash open_gl_interop.cpp if a gl_context doesn't get created. As in the case when extensions required are not present. * revert the 3.19 cmake version upgrade for platforms other than OSX. NB: This *might* mean that Android on OSX would have an issue due to the version of CMake that Android SDK might ship with is 3.18. Revisit this once most people are on Vulkan 1.3 and/or confirmation that android on OSX is a problem. * revert the 3.19 cmake version upgrade for platforms other than OSX. NB: This *might* mean that Android on OSX would have an issue due to the version of CMake that Android SDK might ship with is 3.18. Revisit this once most people are on Vulkan 1.3 and/or confirmation that android on OSX is a problem. * update the Copyright * update with clang-format * oddly running clang-format locally resulted in some white space changes which don't agree with the CICD white space results. * Test: remove vulkan_samples_pch from framework/CMakeLists.txt * Add transcoding of api sample hlsl_shaders based on vulkan.hpp (#512) * Change vkb::core::HPPPhysicalDevice and vkb::core::HPPInstance from facades over vkb::PhysicalDevice and vkb::Instance, respectively, to self-contained classes using vulkan.hpp * Explicitly assign HPPStructurType::structureType to a variable to be usable in std::map::find(). * Change vkb::core::HPPVulkanResource from a facade over vkb::core::VulkanResource to a self-contained class using vulkan.hpp * Add transcoding of api sample terrain_tessellation based on vulkan.hpp * Change vkb::core::HPPQueue from a facade over vkb::Queue to a self-contained class using vulkan.hpp * Add transcoding of api sample hlsl_shaders based on vulkan.hpp * Prepend "HPP" in NAME in CMakeLists.txt plus modified hpp_hello_triangle/CMakeLists.txt accordingly to have a consistent naming scheme. * Fix for graphics pipeline library sample (set required flags for fragment shader output pipeline library) (#508) * Set required flags for fragment shader output pipeline library Properly clean up fragment shaders created at runtime * Removed trailing spaces * Clangformat * Update build.md Updating recommended Android CMake and NDK versions to avoid recent build issues * Use <vulkan/vulkan.hpp> as precompiled header. * Test: remove vulkan_samples_pch from framework/CMakeLists.txt * Re-enable vulkan_samples_pch; changing cmake_minimum_required to 3.16. * Adjust copyright years. * Reuse the framework's precompiled headers in the hpp-samples. Co-authored-by: Steven Winston <[email protected]> Co-authored-by: Sascha Willems <[email protected]> Co-authored-by: Marty Johnson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Attempt to make the Framework requirement of enabling the Portability extension if on an OSX device and running Vulkan greater than or equal to 1.3 (when the Portability extension is introduced). Without this, the moltenvk instance wont be detected in instance creation. Please check instance.cpp for details.
Fixes #476
General Checklist:
Please ensure the following points are checked:
Sample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: