Skip to content

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

Conversation

gpx1000
Copy link
Collaborator

@gpx1000 gpx1000 commented Jul 19, 2022

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:

  • My code follows the coding style
  • I have reviewed file licenses
  • I have updated the header Copyright to reflect the current year (CI build will fail if Copyright is out of date)
  • 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 build on Windows, Linux, macOS and Android. Otherwise I have documented any exceptions
  • 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

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 samples readme

gpx1000 added 4 commits May 18, 2022 18:52
…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
TomAtkinsonArm previously approved these changes Jul 19, 2022
Copy link
Contributor

@TomAtkinsonArm TomAtkinsonArm left a 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

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.
@marty-johnson59
Copy link
Contributor

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 marty-johnson59 merged commit 9198f24 into KhronosGroup:master Aug 19, 2022
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OSX, Could not create Vulkan instance : ERROR_INCOMPATIBLE_DRIVER
3 participants