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

CMake: Reintroduced #2075 #2083

Merged
merged 3 commits into from
Feb 24, 2025
Merged

Conversation

M2-TE
Copy link
Contributor

@M2-TE M2-TE commented Feb 19, 2025

FindVulkan is now optional, as generators do not need it

@asuessenbach
Copy link
Contributor

As already said in #2075: VulkanHppGenerator doesn't really need that include directory

That is, wouldn't it be a better fit to add this functionality to the vulkan_hpp__setup_vulkan_include function (currently lines 337-348 in CMakeLists.txt), which is used for every library, sample and test?

@M2-TE
Copy link
Contributor Author

M2-TE commented Feb 20, 2025

Including the Vulkan C headers in vulkan_hpp__setup_vulkan_include would work for all local things, but VulkanHpp and the module would no longer work as standalone targets outside the scope of this project (unless they call that function manually, which is usually not inteded for external projects in CMake) and requires users to include/link the C headers manually

Though, Vulkan-Headers is probably the better repo to use as a primary way to include all vulkan headers (albeit losing the ability to customize the generator or formatting and with currently worse CMake support), so I agree with putting the C includes into the function

@asuessenbach asuessenbach merged commit 2860728 into KhronosGroup:main Feb 24, 2025
58 checks passed
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