Apple compatibility fixes for driver optionality (MoltenVK & KosmicKrisp)

[SRSaunders]

Description

This WIP PR removes the Apple = MoltenVK driver assumption and provides compatibility for the coming KosmicKrisp driver on supported platforms (macOS, arm64). In addition, it provides a simple mechanism to allow the user to build for either situation. The already-existant CMake variable VKB_ENABLE_PORTABILITY is now externalized as a CMake command line option for users to select building for MoltenVK vs. KosmicKrisp. The option is set in this PR to default = ON for backwards compatibility. To build for KosmicKrisp the user will likely have to execute a specific setup-env.sh from the SDK (or set the VK_DRIVER_FILES environment variable) and then run CMake using the macOS instructions adding the option -DVKB_ENABLE_PORTABILITY=OFF.

Specifics are:

1. Change VKB_ENABLE_PORTABILITY from being an internally set CMake variable to a cmdline option (default ON).
2. Guard a small number of Apple-specific layer settings-related code blocks used for configuring MoltenVK.
3. Remove unneeded Apple-specific #ifdef legacy code blocks from the descriptor_indexing sample.
4. Update the profiles sample to run with VKB_ENABLE_PORTABILITY set to ON or OFF on Apple platforms. The only issue here is that I had to remove a hard-coded #ifdef __APPLE__ code block from vulkan_profiles.hpp to provide this flexibility. I realize this is a generated file, but the .hpp file makes a bad assumption that Apple = PORTABILITY and turns on the portability extensions by itself. This is not correct going forward and the sample itself should control that, not the hpp file. This really should be changed in the generator script, but I do not have access to that within this project.
5. Add a few missing null object checks in the [hpp_]hello_triangle samples so they do not crash on exit if no valid driver is found. I discovered this during regression testing.
6. [new] Update comments and messages for iOS config and build framework. These changes retain the assumption that MoltenVK is used for iOS, but the framework now exists for adding alternative iOS drivers. Comments welcome.
7. [new] Update profiles sample to include previously missing items: a) enable VK_KHR_portability_subset device extension when VKB_ENABLE_PORTABILITY=ON, b) allocate descriptors from update-after-bind pool for descriptor indexing scalability, and c) destroy images and sampler on exit - all to eliminate validation errors.

Fixes #1434

Regression tested for MoltenVK on macOS Ventura on x86_64 + AMD 6600 and on macOS Sequoia on M1 Air using SDK 1.4.328.1 as well as the latest development build of MoltenVK.

@aitor-lunarg if you could review and/or test this functionality with KosmicKrisp that would be great. See macOS build instructions. Simply add -DVKB_ENABLE_PORTABILITY=OFF to the command line. There are some unknowns regarding project dependencies on components like the Vulkan DynamicLoader and volk, but I am hoping those work as-is for KosmicKrisp.

Once this PR has been verified as working by others, I will add an additional commit for macOS build instructions.

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

[SaschaWillems]

We just discussed this, also in light of what you did for my repo. If I did understand you correct, if MoltenVK could be adjusted, we could get rid of having to handle it different than KosmicKrisp? Is that correct? If so we could forward that to the people still working on MoltenVK.

[SRSaunders]

...if MoltenVK could be adjusted, we could get rid of having to handle it different than KosmicKrisp? Is that correct?

Not really. The main end-user difference between MoltenVK and KosmicKrisp is recognition of MoltenVK being a Vulkan Portability driver with the need for enabling portability extensions (VK_KHR_portability_enumeration at the instance level and VK_KHR_portability_subset at the device level). Fortunately this is encapsulated in the Vulkan-Samples project with the VKB_ENABLE_PORTABILITY CMake variable and identically-named compiler define. KosmicKrisp will not need or support these extensions as it is being built as a conformant (i.e. non-portability) driver from the outset.

What I have tried to do in this PR is:

1. Leverage the VKB_ENABLE_PORTABILITY CMake variable by changing it from hard-coded ON on Apple into an cmake option switch when configuring the project. VKB_ENABLE_PORTABILITY is set to default ON to support MoltenVK, but will need to be specified OFF on the cmake command line for KosmicKrisp. The default could change later, but this should be a good start. Once this PR is tested by the KosmicKrisp developers, I will add a doc commit here to help explain this.
2. Remove as many MoltenVK-specifics from the samples as possible and guard any remaining necessary differences with VKB_ENABLE_PORTABILITY to ensure cross-driver compatibility.
3. A special note re the profiles sample: The vulkan_profiles.hpp generated file contains a bad assumption that all Apple platforms require the portability extensions be enabled. Going forward this will be incorrect. I have taken a quick and dirty approach in this PR and removed that bad assumption from the generated file, leaving enablement of those extensions in the sample itself. However, this should really be fixed in the generator script (different project) for the vulkan_profiles.hpp file.

Regarding your comment about asking the MoltenVK folks to handle things differently (i.e. portability vs conformant), I think that may be unreasonable or perhaps impossible at this point. Portability was a major design assumption of MoltenVK and has allowed it to advance its Vulkan version support by leveraging the subsetting nature of the Vulkan spec. I think MoltenVK is fine as is, supporting macOS, iOS and arm64/x86_64 across a wide range of OS versions. My goal here was not to force change back down the pipe, but to accommodate both approaches in the Vulkan-Samples project. Note that KosmicKrisp will start with a narrower support matrix of newer macOS versions on arm64 only, and I don't believe iOS support is planned at least for now.

[SRSaunders]

Not sure why Antora doc build is failing. I didn't touch anything related with this PR. Strange...

[SaschaWillems]

The Doc build failure is our fault, see KhronosGroup/antora-ui-khronos#30

[gpx1000]

Fixed it.

[gpx1000]

Also, I agree with @SRSaunders, there's nothing for MoltenVK to fix. That's the way MoltenVK is designed to work. During the meeting today, we were under the impression that @SRSaunders found some bug in MoltenVK having to do with non-standard's compliant layer handling in Sascha's samples. I think that's what we were wondering if we needed to escalate knowledge about to get fixed in the MoltenVK team.

[aitor-lunarg]

Just tested and it works with no issues, thank you!

I do have one suggestion, but feel free to ignore it. Changing drivers is as easy as setting a new value for VK_DRIVER_FILES. Would it be possible not to have something like VKB_ENABLE_PORTABILITY and just decide at run time if portability is needed or not based on available drivers? Mainly with the idea of letting users easily switch between MoltenVK, KosmicKrisp and Lavapipe as their Vulkan backend.

[SRSaunders]

Just tested and it works with no issues, thank you!

@aitor-lunarg thanks for testing quickly and delivering excellent news! I was hoping external dependencies like the Vulkan DynamicLoader (vulkan.hpp) and volk would work as-is, and I am glad to hear they do.

Would it be possible not to have something like VKB_ENABLE_PORTABILITY and just decide at run time if portability is needed or not based on available drivers?

This is an interesting idea, but would require changing the code from being a build-time decision to a runtime one. This may indeed be possible, but I would have to investigate if there are any limitations or downsides here. On the positive side, part of this concept is already in place - the portability extensions are always checked for availability at runtime prior to enabling them. However, we also need to understand the implications of always including vulkan_beta.h and things like VkPhysicalDevicePortabilitySubsetFeaturesKHR on non-portability drivers. Please let me have a look at this and do some prototyping before I answer.

Changing drivers is as easy as setting a new value for VK_DRIVER_FILES

This is also good to hear. Can you tell me how a future SDK will handle setting up this env variable for KosmicKrisp vs. MoltenVK? Currently there is a single setup-env.sh file at the SDK root level (and a different one in the iOS subfolder) that sets up important env variables including VULKAN_SDK, VK_DRIVER_FILES, etc. Going forward will there be multiple setup-env.sh files? Or perhaps an updated script with some optionality? In other words have you figured out how SDK co-residency and env variable setup will work for selection of either KosmicKrisp or MoltenVK?

Also will CMake's find_package(Vulkan OPTIONAL_COMPONENTS ...) be able to handle the differences? I presume it will key off of VULKAN_SDK, but will there be new optional components for KosmicKrisp with new variables similar to Vulkan_MoltenVK_FOUND and Vulkan_MoltenVK_LIBRARY?

[SRSaunders]

Would it be possible not to have something like VKB_ENABLE_PORTABILITY and just decide at run time if portability is needed or not based on available drivers?

Please let me have a look at this and do some prototyping before I answer.

@aitor-lunarg I have updated this PR to ensure there are runtime checks for the portability extensions and features. However, I have left VKB_ENABLE_PORTABILITY in place as a developer build option, and have it turned ON by default for Apple and OFF by default for other platforms. There should be no need to expose this to end users or in the docs. The runtime checks should ensure KosmicKrisp can run without special configuration (i.e. can follow current instructions) other than setting a new value for VK_DRIVER_FILES.

However, you may now see runtime warning messages that VK_KHR_portability_subset or certain portability features are not available. This is a by-product of leaving VKB_ENABLE_PORTABILITY=ON. These messages would not appear with VKB_ENABLE_PORTABILITY=OFF, but I think the benefit of simplicity may be worth the trade-off. The messages are informational only and will not affect performance in any way.

Note that I am still interested in how env variables will be configured for MoltenVK vs. KosmicKrisp in a co-resident SDK via setup-env.sh. If you have information about this I could update the docs to provide instructions to Vulkan-Samples users when running on macOS. I presume iOS will not change for now.

If you want to test this updated PR with KosmicKrisp that would be great. If you come back with positive results, I could move this PR out of WIP status into ready for review. Thanks.

[gpx1000]

This works well for me on KosmicKrisp and MoltenVK. Also tested in Linux to ensure everything still works everywhere.

[aitor-lunarg]

Note that I am still interested in how env variables will be configured for MoltenVK vs. KosmicKrisp in a co-resident SDK via setup-env.sh. If you have information about this I could update the docs to provide instructions to Vulkan-Samples users when running on macOS. I presume iOS will not change for now.

My understanding is that it will just be added to the path used by the loader to search for drivers. Right now, setup-env.sh will export VK_DRIVER_FILES="$VULKAN_SDK/share/vulkan/icd.d/MoltenVK_icd.json", so that will eventually also contain KK. Then it's up to the developer to choose which driver they want to use as you would in any other environment with multiple Vulkan drivers.

[richard-lunarg]

The plan for the first Alpha release of KosmicKrisp is to not include it in the system global folders the way MoltenVK is. The rationale is it may create havok if suddenly there are two ICD's to choose from now for developers using that option. An icd json file is included and people who want to experiment can embed it in their application bundles, or copy it to the system folder where MoltenVK is themselves. KosmicKrisp also does not support direct linking the way MoltenVK does. You have to use the Vulkan Loader. The ICD can then be placde in the system folders or (preferably) in the application bundle. I hope no one is actually shipping applications or games and assuming MoltenVK (or KosmicKrisp in the future) is installed in a system folder.

[SRSaunders]

Thanks @richard-lunarg for the explanation on initial KosmicKrisp installation decisions for the SDK. For the Vulkan-Samples project at least, the end user is asked to run setup-env.sh to define the path to the driver icd file (as well as to other things like the VVL). Up to this point that has been for MoltenVK only. Similar to @aitor-lunarg's comment above, will there be a second setup-env.sh available somewhere in an upcoming SDK which specifies VK_DRIVER_FILES and defines the path to KosmicKrisp_icd.json within the SDK (e.g. in a separate KosmicKrisp folder similar to what is done for iOS)? Or will that be entirely manual and up to the developer for the Alpha release?

[SRSaunders]

Thanks @gpx1000 for testing this PR and approving. I was waiting for someone to provide feedback re the added runtime checks for portability extensions and features, which provides compatibility for MoltenVK and KosmicKrisp without prior configuration. Given your positive results, I will now remove the WIP label on this PR.

[gpx1000]

Thanks for the hard work on it! Your help on Apple has been really appreciated!

[richard-lunarg]

Or will that be entirely manual and up to the developer for the Alpha release?

Manual for now, but all they have to do is set the one environment variable. I'll put something in the release notes about it. After the alpha and some other feedback, we'll probably finesse this a bit more for the next SDK release. The response to KosmicKrisp has been really strong, we are all very gratified by how much enthusiasm the community is showing for this project.

[SRSaunders]

Manual for now, but all they have to do is set the one environment variable. I'll put something in the release notes about it.

Thanks @richard-lunarg, sounds good. I won't add any additional docs on the Vulkan-Samples side for now.

And @gpx1000 I'm happy to help out. Thanks for your ongoing feedback, testing, and support re Apple.

[richard-lunarg]

Heads up, we did have a change in plan late in testing for the next SDK. If KosmicKrisp is selected, it will be placed in the system folders along side MoltenVK. The loader sees MoltenVK first, so it should not break anything, but all you will need to do is run:

export VK_DRIVER_FILES=$VULKAN_SDK/share/vulkan/icd.d/libkosmickrisp_icd.json

To switch to KosmicKrisp from MoltenVK from the command line. There are multiple ways to use KosmicKrisp, and see the 'Getting Started Guide" for macOS in the SDK for details next week.

[SRSaunders]

If KosmicKrisp is selected, it will be placed in the system folders along side MoltenVK.

Will this be on arm64 hosts only? I presume this will not be the case for x86_64 hosts. Alternatively, will the SDK's KosmicKrisp installation option be available only on arm64?

export VK_DRIVER_FILES=$VULKAN_SDK/share/vulkan/icd.d/libkosmickrisp_icd.json

This should work well for command line invocation of Vulkan-Samples. However, for Xcode-based builds and execution, the user will have to update the Vulkan-Samples Xcode schema (under Environment Variables) to modify the definition of VK_DRIVER_FILES. That's because shell definitions are not available when running under Xcode. Alternatively, if the user executes the above line after running setup-env.sh and before cmake configuration for the project, the new definition will automatically be populated in the project's Xcode schema.

[richard-lunarg]

Will this be on arm64 hosts only? I presume this will not be the case for x86_64 hosts.
Alternatively, will the SDK's KosmicKrisp installation option be available only on arm64?

Ah, good point/question. The installer does not distinguish between Apple Silicon and x86_64, but KosmicKrisp is only for Apple Silicon. I've added a note at least to indicate this on the installer and in the release notes.

[richard-lunarg]

for Xcode-based builds and execution

They can also include KosmicKrisp in the application bundle, which is the recommended method for distributable applications as well.

[SRSaunders]

Thanks @richard-lunarg for your answers.