Update iOS build and CI to support standardization of FindVulkan.cmake

[SRSaunders]

Description

This PR updates the iOS build and CI to support future standardization of FindVulkan.cmake where there may be fewer project customizations present for iOS build targets. With these changes the project will build and run for iOS and iOS Simulator with no project-specific FindVulkan.cmake present in the tree, instead relying on the cmake's standard version of the module.

However, one downside in this scenario is the Vulkan Validation Layer would not be found for iOS and samples that rely on it would not run (e.g. shader_debugprintf). In addition, iOS debug builds would run but would not benefit from validation messages (now fixed by item 4 below). A separate effort is pushing project additions to the upstream FindVulkan.cmake to allow eventual migration to the standard cmake version. In the mean time the project FindVulkan.cmake will remain in place and this PR will not result in iOS regressions in any scenario.

The specific items that have changed are:

1. A new ios.cmake toochain file has been introduced that sets common iOS cmake build variables and assists cmake's FindVulkan module in locating the Vulkan iOS frameworks. Thanks to @fgiancane8 for this idea and initial code. Co-Authored-By: Giancane, Francesco fgiancan@qti.qualcomm.com
2. Updated CI command and End-user build docs for iOS and iOS Simulator using the new ios.cmake toolchain file.
3. The Vulkan_Target_SDK cmake variable is now defined locally within Vulkan-Samples/app/CmakeLists.txt vs. depending on its definition within the project's FindVulkan.cmake. This approach makes more sense since this was a project-specific customization of FindVulkan.cmake and is the only location in the project build system (outside of FindVulkan.cmake itself) where this variable is used. Note the purpose of this variable is to locate the directory where Vulkan icd and layer json files can be found - required for packaging an iOS app bundle.
4. The Vulkan_Layer_VALIDATION cmake variable is now defined within the iOS-specific section in Vulkan-Samples/bldsys/cmake/global_options.cmake if it's not already defined or found by FindVulkan.cmake. This acts as a fail-safe fallback that avoids any regressions in finding the validation layer for iOS within the project.

I have also taken the opportunity to address three four other related items here:

5. Prevent Apple (macOS, iOS, iOS Simulator) builds from compiling the shaders for every new build instance. Because Apple platforms always require the VULKAN_SDK, the current project build logic will define the shader compilers and force a build of the shaders. This is not optimal since the project already provides pre-compiled shaders and the extra build steps on Apple platforms adds complexity and time to the project build for end-users. This could also affect CI but I believe other steps have already been taken by @SaschaWillems to prevent that (at least for slang, not sure about other shader langs). Note on Windows platforms the end-user has control over this by choosing to run with the SDK installed or not. On Apple platforms the end-user has no choice since the SDK must be installed to provide the Vulkan libraries needed at runtime.
6. Change VK_LAYER_KHRONOS_validation to be Optional for debug builds vs. Required. This applies to all platforms but allows debug builds to run even when the validation layer is not available. This is important for several use cases: a) Windows users who do not install the SDK, and b) on the iOS Simulator where the VVL library is not built for that target. I debated this change before making it, but I believe it is correct given that Windows build docs do not mention or require installation of the SDK, and the default build config for Visual Studio is debug. I am interested in feedback from maintainers on this regarding project intentions: i.e. Required forces a runtime error that prevents samples from running in debug mode, or Optional allows execution with a runtime warning message instead. I chose the latter.
7. Updated Vulkan SDK to 1.4.341.1 for iOS CI builds. This matches the project and I thought this would be a good time to update given the other iOS CI changes.
8. Updated some samples (texture_loading, texture_mipmap_generation, hpp_pipeline_cache, pipeline_cache) to guard against an undefined device on exit. These samples assume the device is defined and attempt cleanup without checking first, which can result in a crash on exit when the sample does not run successfully. I found this in testing item 6 above, when running in debug mode with VK_LAYER_KHRONOS_validation set to Required.

Fixes iOS build issues associated with pull request #1490. See additional discussion there.

Tested on Windows 11 and macOS Sequoia with macOS x86_64, iOS arm64, and iOS Simulator x86_64 targets.

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

[fgiancane8]

For reference: my MR on upstream CMake to import layer variables back into FindVulkan module.

https://gitlab.kitware.com/cmake/cmake/-/merge_requests/11808

We should be on feature parity once this lands.

[fgiancane8]

Tested on latest CMake. Vulkan frameworks and related libraries are found and processed as expected.

[SRSaunders]

I added one additional commit to:

1. Revert the ios.cmake license back to the standard Apache 2.0 license as per instructions from @fgiancane8.
2. Change the CMAKE_FIND_ROOT_PATH definition in ios.cmake to a "set" vs. "append". According to CMake docs, this is best practice for toolchain files since they are run multiple times during cmake configuration. A "set" asserts the value and prevents multiple "appends" from occuring. For iOS, there are no other root paths needed or defined within CMAKE_FIND_ROOT_PATH so a "set" to the SDK location should be fine. On Apple platforms (including iOS) the system paths are defined separately via CMAKE_OSX_SYSROOT on the command line.

Ready for review.

[fgiancane8]

LGTM.

[fgiancane8]

For reference: my MR on upstream CMake to import layer variables back into FindVulkan module.

https://gitlab.kitware.com/cmake/cmake/-/merge_requests/11808

We should be on feature parity once this lands.

FYI @SRSaunders this landed and it's targeting CMake 4.4.0 release. All the available layers in SDK are enumerated and given their own variables. This should close the loop on upstream CMake requirements to be able to build successfully for iOS as well.

Let me know if something else is missing.

[SRSaunders]

... this landed and it's targeting CMake 4.4.0 release. All the available layers in SDK are enumerated and given their own variables. This should close the loop on upstream CMake requirements to be able to build successfully for iOS as well.

Thanks @fgiancane8 for the upstream status. Note this PR now contains a fail-safe (see item 4 above in the PR description) for the Vulkan_Layer_VALIDATION variable. Even if it's not defined by any FindVulkan.cmake, the project will have back up logic to search for the layer on iOS. If the variable is already defined by FIndVulkan.cmake it will use that definition, otherwise the internal backup search logic will be used.

Therefore this PR effectively decouples the project's iOS build from the timing around CMake 4.4.0, as well as possible deletion of the the project's downstream FindVulkan.cmake. If/when this PR is merged, these items should become independent decisions going forward.

[fgiancane8]

... this landed and it's targeting CMake 4.4.0 release. All the available layers in SDK are enumerated and given their own variables. This should close the loop on upstream CMake requirements to be able to build successfully for iOS as well.

Thanks @fgiancane8 for the upstream status. Note this PR now contains a fail-safe (see item 4 above in the PR description) for the Vulkan_Layer_VALIDATION variable. Even if it's not defined by any FindVulkan.cmake, the project will have back up logic to search for the layer on iOS. If the variable is already defined by FIndVulkan.cmake it will use that definition, otherwise the internal backup search logic will be used.

Therefore this PR effectively decouples the project's iOS build from the timing around CMake 4.4.0, as well as possible deletion of the the project's downstream FindVulkan.cmake. If/when this PR is merged, these items should become independent decisions going forward.

Sounds good to me. Just FYI please note that upstream has adopted a slightly different naming scheme for these variables:
https://gitlab.kitware.com/cmake/cmake/-/merge_requests/11808/diffs

We may want to align them before removing the downstream version.
Thanks!

[SRSaunders]

Just FYI please note that upstream has adopted a slightly different naming scheme for these variables:

Thanks for the heads up. Even if the Vulkan_Layer_VALIDATION variable name changes in the upstream version, this PR will handle that gracefully and fall back to searching itself. But yes, if/when the downstream project version of FindVulkan.cmake is removed and CMake 4.4 is generally available (e.g. via homebrew), I can update this variable name to the new upstream name (Vulkan_Layer_KHRONOS_VALIDATION_LIBRARY).

No need to update this PR at this time. Thanks @fgiancane8 for your initiative with this effort.

[fgiancane8]

Just FYI please note that upstream has adopted a slightly different naming scheme for these variables:

Thanks for the heads up. Even if the Vulkan_Layer_VALIDATION variable name changes in the upstream version, this PR will handle that gracefully and fall back to searching itself. But yes, if/when the downstream project version of FindVulkan.cmake is removed and CMake 4.4 is generally available (e.g. via homebrew), I can update this variable name to the new upstream name (Vulkan_Layer_KHRONOS_VALIDATION_LIBRARY).

No need to update this PR at this time. Thanks @fgiancane8 for your initiative with this effort.

Sure, no worries at all. We can follow up once we're ready to move away completely from the vendored version here.

[fgiancane8]

Suggested change

	Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
	Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. Not a contribution.

@SRSaunders can you please accept this change? Legal team reviewed this and requested the copyright to be stated like this, according to Apache-2.0 license.

Otherwise, LGTM.

[SRSaunders]

Ok, but if it's not a contribution, then what is it? I am not sure I understand the intent of this change from your legal folks. Can you clarify what this means?

[fgiancane8]

From Apache 2 license:

      "Contribution" shall mean any work of authorship, including
      the original version of the Work and any modifications or additions
      to that Work or Derivative Works thereof, that is intentionally
      submitted to Licensor for inclusion in the Work by the copyright owner
      or by an individual or Legal Entity authorized to submit on behalf of
      the copyright owner. For the purposes of this definition, "submitted"
      means any form of electronic, verbal, or written communication sent
      to the Licensor or its representatives, including but not limited to
      communication on electronic mailing lists, source code control systems,
      and issue tracking systems that are managed by, or on behalf of, the
      Licensor for the purpose of discussing and improving the Work, but
      excluding communication that is conspicuously marked or otherwise
      designated in writing by the copyright owner as "Not a Contribution."

Last line in the license explains the "Not a Contribution" usage in copyrights. I relayed your question to the Legal team to provide more clarity here.

[SRSaunders]

Well, I am not a maintainer on this project so I cannot say whether this can be accepted or not. You will have to ask @gpx1000 about that.

However, based on the above text, "Contribution" and "submitted" are exactly what we are doing here, which is described by "communication sent to the Licensor or its representatives, including but not limited to communication on electronic mailing lists, source code control systems, and issue tracking systems that are managed by, or on behalf of, the Licensor for the purpose of discussing and improving the Work".

The next clause "but excluding communication that is conspicuously marked or otherwise designated in writing by the copyright owner as "Not a Contribution." means that we should NOT include in the license any further improvements discussed by you if you mark a new communication as such. Note the exact wording here: the license talks about marking "communication as Not a Contribution", not the licensed work itself. That means in future discussion that you could mark a new idea as Not a Contribution and it would not be incorporated in the code and not included in the original license. That would be more reasonable.

However, these kinds of communication clauses are typically put in licensing agreements when the IP licensing is 1-way only, with no follow-on discussion desired by the licensee. That approach is not very typical of open source where ongoing contribution and discussion is typically welcomed and encouraged.

[fgiancane8]

@SRSaunders thanks for all the clarifications. As you can imagine, I am not a legal person myself so I am following what folks from legal team told me to do. Please bear with me some more time to see if I can get some more information and/or inputs on how we can better address this.

@gpx1000 since you're maintainer here, any history/inputs/etc would be very much appreciated.

Thanks all folks, hopefully we'll get it sorted out quickly.

[SRSaunders]

Thanks for looking into this. At the simplest level, you can't make a contribution and then call it "Not a contribution". However, new stuff can be discussed but not contributed as long as you mark it that way.

[gpx1000]

@marty-johnson59 Could you take a look? I think we're fine but I'm not a lawyer, and have no wish to have an opinion.

[marty-johnson59]

I'll bring this up with Khronos legal, but my read is that this was a "contribution", so not sure we're OK with marking it as "not a contribution"... @fgiancane8, can you help clarify how/why this should not be considered a contribution? That might help our legal team sort this out. Thanks