Fixed C++Module with better dependency management in CMake and newer VMA submodule commit hash

[M2-TE]

The module was still broken when using the workflow provided in the current CMakeLists.txt, so I went ahead and fixed it alongside a few other things. Will resolve #65. Changes include:

• BREAKING CHANGE: renamed module target from ::VulkanMemoryAllocator-Hpp to ::HppModule (same as Vulkan naming scheme). Better to do it now, while there are already breaking changes for the 3.3.0 release.
• updated VMA submodule to newer commit to include module patches (required for compilation to succeed)
• improved dependency fetching in the root CMakeLists.txt file; it checks for existing targets before finding or fetching.
  -> vulkan in FetchContent was renamed to vulkan-headers to match the "fetch content module name should match git URL" guideline
• VMA is now always linked to as INTERFACE, while also performing the link outside the module logic (normal vma-hpp target also needs it)
• add_subdirectory now properly checks whether the VMA submodule has even been checked out. If not, falls back to FetchContent. VMA dep logic for module has been simplified

[M2-TE]

The Generate Headers workflow fails, since the submodule hash has changed. Not sure why the gitdiff for it exists, what should I change @YaaZ?

[YaaZ]

Hi @M2-TE, thank you. I have some concerns about your approach, but I need some time to re-read it and come up with a sensible feedback.

In the meantime, could you please explain the exact issues you faced and fixed with this PR? I see what the changes are, but I don't understand what problems are those meant to solve.

Answering your question, "generate headers" fails because VMA submodule is not meant to be updated manually - DO_UPDATE synchronizes the submodule with current project version. It checks out VMA 3.3.0 and sees the mismatch.
If the reason for updating the submodule was only to fix #65, I suggest doing it in a separate branch, because master currently serves as a stable branch, from which 3.3.0 releases are created.

[M2-TE]

Yup the submodule was updated to fix the related issue, hadn't known how it was handled in this repo, sorry!
I use this branch to get modules working in my own projects, so I figured I might as well put it here.

As for the CMake changes, it is all related to module usage:

1. It automatically fetched Vulkan-Headers and created conflicts when it already existed in the project. In my case, only the Vulkan::HppModule target existed so previous checks were insufficient and it would fetch vulkan anew (duplicate definition errors followed).
2. VMA dep resolve wasn't quite right. The check for vk_mem_alloc.h would always fail. find_package was also likely to never succeed (it's not a standard package to have, without installing it manually via cmake). I've fixed the check for the header by looking in the right place, while also providing another FetchContent fallback.
3. The module would break since dep resolve as mentioned in 2) was broken. The linkage as INTERFACE is an easy way to provide a stable VMA include directory for both the normal and module target (target guaranteed to exist, as this is handled in the main CMake now). Though, I should have removed the currently present target_include_directories line, as that is no longer necessary.
4. This is very subjective, but the ALIAS name for the library is way too long.

Theres probably two things I should do after looking at this PR again:

1. Remove the superfluous target_include_directories for C VMA
2. Keep the previous alias, but mark it as deprecated (if you agree with this change in the first place)

[YaaZ]

I think you're missing the (new) intended usage idea. A lot has changed with the latest big updates and the README was rewritten to make it more clear, hopefully.

The most important part is that root CMakeLists is now for the generator and samples. You don't need it when using VMA-Hpp as a dependency in your project - you only need the one in the include directory.

Also, it's recommended to use GH releases instead of checking out the whole repo. Release tarball contains only include directory and also bundles the proper vk_mem_alloc.h header. That's why there is a check for that header - it's the default case when using a minimal release archive.

I agree that the ALIAS is ugly, but it also doesn't look fair to me to take the "VulkanMemoryAllocator" namespace because we're not the "original" VMA. Especially given that even VMA itself doesn't use that namespace, I feel this name may be misleading.
I would be happy to change the alias to something better, but we would need to think more about the new name.

[M2-TE]

Oh I see what you intended in that case. Though, checking out dependencies like this repo with FetchContent is a very common use case and it would be nice if the root CMake was capable of supporting this (providing the target and nothing more).

As for the alias, it would make sense to be in the same namespace, no?
It is the Hpp version in the namespace, basically the variant. Vulkan and Vulkan-Hpp handle it in the same way.

[YaaZ]

We can check PROJECT_IS_TOP_LEVEL and skip all the unrelevant code in the root CMakeLists, or something like this.
But is there any reason to check out the whole repo at all? I mean, when doing FetchContent anyway, why choose a bunch of unused files instead of a minimal archive?

I partially agree, if VMA used "VulkanMemoryAllocator::" namespace, we could also use it for Hpp bindings (asking author's consent first). But VMA uses "GPUOpen::" namespace and we are not related to that in any way. And imo using "VulkanMemoryAllocator::" could be misleading because there would only be C++ and no C headers.

I can see two solutions here:

1. Use current safe namespace VulkanMemoryAllocator-Hpp:: and shorten the target name
2. Collaborate with VMA to add nice aliases like VulkanMemoryAllocator::Headers, VulkanMemoryAllocator::Hpp and VulkanMemoryAllocator::HppModule

[M2-TE]

PROJECT_IS_TOP_LEVEL seems like a good solution as well. As for why one would FetchContent, its really just convenience. All you need is 3 LoC in your CMake and you dont have to copy anything into your project manually.

You make a good point about the alias namespace though.. using GPUOpen doesn't fit (even if allowed) and I forgot VMA does not use VulkanMemoryAllocator as their namespace. Your second idea of adding this to VMA might be pretty good. It won't break anything, as it is just an additional alias. I'll shoot them a question regarding that!

[YaaZ]

But using FetchContent with the release tarball is as convenient as with the git repo, 3 similar LOC and no manual copies, see the example in https://github.com/YaaZ/VulkanMemoryAllocator-Hpp?tab=readme-ov-file#get-the-library
Moreover, you don't only get rid of the unnecessary files, you also get a bundled VMA header, which you don't get when checking out the repo.

Regarding alias - I would be happy to merge it if we manage to sort it out with VMA.
UPD. But in that case, let's just deprecate the old alias, not remove it right away.

[M2-TE]

I will check out FetchContent with release tarballs, maybe I've been missing out. I figured using tarballs wouldn't supply me with the CMake targets.

As for the alias, of course, deprecation is much better than immediate removal.

I'll close this PR in the meantime. The alias changes should be a separate PR if anything. Thanks for your inputs!