Merge Vulkan Video module into main module

[sharadhr]

Resolves #2310 and resolves #2311.

[M2-TE]

The import name in the tests needs to be changed as well.
Might also be good to update the readme:

Vulkan-Hpp/README.md

Line 800 in 63a3afa

Vulkan-Hpp provides a [C++ named module](https://en.cppreference.com/w/cpp/language/modules), `vulkan_hpp` in [`vulkan.cppm`](vulkan/vulkan.cppm).

Vulkan-Hpp/README.md

Line 885 in 63a3afa

import vulkan_hpp;

[M2-TE]

How about we update the minimal example in readme for import vulkan with dynamic dispatching as well?
Notably, we fix the import/include order (breaks modules on gcc otherwise) and provide some more info.
The previously used macros were also not really necessary in the example:

// required to access macros (otherwise, use vk::detail::DispatchLoaderDynamic directly)
#include <vulkan/vulkan_hpp_macros.hpp>
// import after all other #includes
import vulkan;

// when vulkan_hpp_macros.hpp was included
VULKAN_HPP_DEFAULT_DISPATCH_LOADER_DYNAMIC_STORAGE

auto main2(int argc, char* const argv[]) -> int
{
    // initialize minimal set of function pointers
    VULKAN_HPP_DEFAULT_DISPATCHER.init();

    auto appInfo = vk::ApplicationInfo("My App", 1, "My Engine", 1, vk::makeApiVersion(1, 0, 0, 0));
    // ...
}

[sharadhr]

Thanks for the insight. I've been meaning to update the readmes for a while, including the guidance on different compilers, build systems, etc (the module requires latest everything).

[asuessenbach]

I'm sorry, a new test has materialized: NoDefaultDispatcher.
Would you please adjust that as well, to import vulkan, instead of vulkan_hpp?

[sharadhr]

@asuessenbach this is ready.

@M2-TE I have made the minimum necessary edits to the readme (module name only). All other changes I intend to work in as part of #2358.

[theHamsta]

Hi! This looks good to me. I enabled individual tests like in #2292 and they worked fine with the corresponding changes in C++23 mode with CMake's experimental import std-support and clang-21.

Would it be possible to enable at least one configuration in CI that compiles at least one test with C++ modules enabled. It does not need to be all of #2292. We could also rebase #2292 on this to have a CI for these changes.

Is C++ modules support bound to C++23 import std support (seems to be used in the header when c++ modules are enabled)? Please pardon me ignorance. I just came back to this project after a longer time.

We also would need to update a C++20 modules test in the Vulkan-Header repository after merging this change.

[sharadhr]

Hi @theHamsta! Long time, no see.

Would it be possible to enable at least one configuration in CI that compiles at least one test with C++ modules enabled.

Yes, that makes sense, to avoid embarrassments like #2362 slipping in.

As for #2292, I feel it has fallen afoul of scope creep and really needs to be broken up, into at least two PRs. One for testing module std, another for CTest.

Is C++ modules support bound to C++23 import std support (seems to be used in the header when c++ modules are enabled)?

For now, yes. Proper module support across all three toolchains (GCC, Clang, MSVC) is still quite experimental, and we're demanding the latest toolchains anyway, so might as well ask for the latest standard as well. It also massively simplifies how the module is used by the end user, and also simplifies the CMake build logic. As for C++20, all three compiler vendors have pledged to backport import std. Some build systems support this out-of-the-box (xmake, build2), some others don't (CMake). End-users can still manually set it up, we ship the module interface file anyway.

We also would need to update a C++20 modules test in the Vulkan-Header repository after merging this change.

That test has been disabled, but after I write a CI test for this, we can re-enable that so we get modules compiling properly.

[M2-TE]

Edit: Looks like @sharadhr was just a second faster.. but yes, perhaps #2292 should be broken into several smaller PRs.

Would it be possible to enable at least one configuration in CI that compiles at least one test with C++ modules enabled. It does not need to be all of #2292. We could also rebase #2292 on this to have a CI for these changes.

I had benched that PR for a little while until I figured out how to get CI working with modules. As mentioned in #2362, @YaaZ has a working workflow for this, so I'll check that out and try to incorporate it.

Is C++ modules support bound to C++23 import std support

Yes and no. You could use modules in C++20, but that would mean doing half things by compiling modules and still including headers. We decided to make import std it a hard requirement for the "true module way", which was implemented in #2303.

We also would need to update a C++20 modules test in the Vulkan-Header repository after merging this change.

Indeed, but that will require more thorough testing, so that it does not block releases again.

[theHamsta]

Hi @theHamsta! Long time, no see.

Since this is breaking change, I would prefer @asuessenbach to merge. He will be back next week.

I had benched that PR for a little while until I figured out how to get CI working with modules. As mentioned in #2362, @YaaZ has a working workflow for this, so I'll check that out and try to incorporate it.

So maybe a good strategy would then be to leave this PR as is and have a minimal follow-up that enables CI for modules. Just building some module tests for some configuration might be enough as a first step.

[asuessenbach]

I know, I should have asked this quite a while ago...
But could you split this PR in two, one to rename the module from vulkan_hpp to vulkan, and one to actually merge the video module into main?

[sharadhr]

I can, but at least one of the PRs will run into a conflict and will have to be correctly rebased, one on top of another.

[asuessenbach]

I can, but at least one of the PRs will run into a conflict and will have to be correctly rebased, one on top of another.

Yes, that's to be expected.
But I think, merging in video and renaming vulkan_hpp to vulkan are two separate things that earn their own PR, each.
Moreover, the renaming part would need some mentioning in the Breaking Changes section of the readme.

[sharadhr]

@asuessenbach I will close this; I have opened #2372 and #2373 instead.