Vulkan: Add macros to avoid compilation issues with missing symbols in Vulkan 1.0

[panreyes]

Fixes #111088

Added 2 macros to avoid compilation issues with missing symbols in certain Vulkan implementations, in current SDKs.

Missing symbols:
VK_DRIVER_ID_MESA_AGXV
VK_BUFFER_USAGE_2_EXECUTION_GRAPH_SCRATCH_BIT_AMDX

[clayjohn]

Copying my comment from earlier:

This is third-party code that comes from the Vulkan SDK. Any changes should be reflected in the corresponding patch file (here: https://github.com/godotengine/godot/tree/master/thirdparty/vulkan/patches) and should be PRed upstream so we minimize our changes from upstream.

Since you are seeing a compilation issue though, it really sounds like you may just have an out of date SDK in your tool chain.

[panreyes]

I updated the patch file to reflect the changes, I hope it looks correct (first time doing a git diff, and my hashes look a bit shorter).

This might be related to an out of date Vulkan SDK in the toolchain, but it is being shipped as the current one in a gaming platform's SDK. I could use the Vulkan headers from the thirdparty directory instead of those provided by the platform's SDK, but I'm not sure if they are fully compatible with the platform's Vulkan drivers.

I can keep the fix in my repository, but I thought it might be useful upstream as well. If it's not relevant for others, that's totally fine too. Either way, I'm glad I got to dig into this and learned quite a bit today!

[podiki]

Running into the reported error in compilation and others. It could actually be that the Vulkan stuff in Godot is older than the system one. For instance, I see VK_DRIVER_ID_MESA_AGXV is found in vulkan-sdk-1.3.283.0 (version in thirdparty/vulkan for Godot 4.5) but not vulkan-sdk-1.4.321.0 (the version on my system, GNU Guix). For Guix we supply a system package for vulkan-loader in the build (so the dlopen code finds this library).

I tried with a vulkan-loader (and thus headers) matching the version included in thirdparty/vulkan, and that built successfully. I guess from a packaging standpoint, I could remove the thirdpart/vulkan directory and point to the relevant system package files for vulkan. Though since there are Godot patches, I don't know if that would work either.

Anyway, here were all the errors when using vulkan-loader 1.4.321.0:

In file included from drivers/vulkan/rendering_context_driver_vulkan.cpp:35:
thirdparty/vulkan/vk_enum_string_helper.h: In function ‘const char* string_VkStructureType(VkStructureType)’:
thirdparty/vulkan/vk_enum_string_helper.h:1347:14: error: ‘VK_STRUCTURE_TYPE_CUDA_MODULE_CREATE_INFO_NV’ was not declared in this scope; did you mean ‘VK_STRUCTURE_TYPE_CU_MODULE_CREATE_INFO_NVX’?
 1347 |         case VK_STRUCTURE_TYPE_CUDA_MODULE_CREATE_INFO_NV:
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |              VK_STRUCTURE_TYPE_CU_MODULE_CREATE_INFO_NVX
thirdparty/vulkan/vk_enum_string_helper.h:1349:14: error: ‘VK_STRUCTURE_TYPE_CUDA_FUNCTION_CREATE_INFO_NV’ was not declared in this scope; did you mean ‘VK_STRUCTURE_TYPE_CU_FUNCTION_CREATE_INFO_NVX’?
 1349 |         case VK_STRUCTURE_TYPE_CUDA_FUNCTION_CREATE_INFO_NV:
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |              VK_STRUCTURE_TYPE_CU_FUNCTION_CREATE_INFO_NVX
thirdparty/vulkan/vk_enum_string_helper.h:1351:14: error: ‘VK_STRUCTURE_TYPE_CUDA_LAUNCH_INFO_NV’ was not declared in this scope; did you mean ‘VK_STRUCTURE_TYPE_CU_LAUNCH_INFO_NVX’?
 1351 |         case VK_STRUCTURE_TYPE_CUDA_LAUNCH_INFO_NV:
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |              VK_STRUCTURE_TYPE_CU_LAUNCH_INFO_NVX
thirdparty/vulkan/vk_enum_string_helper.h:1353:14: error: ‘VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CUDA_KERNEL_LAUNCH_FEATURES_NV’ was not declared in this scope; did you mean ‘VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_SM_BUILTINS_FEATURES_NV’?
 1353 |         case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CUDA_KERNEL_LAUNCH_FEATURES_NV:
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |              VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_SM_BUILTINS_FEATURES_NV
thirdparty/vulkan/vk_enum_string_helper.h:1355:14: error: ‘VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CUDA_KERNEL_LAUNCH_PROPERTIES_NV’ was not declared in this scope; did you mean ‘VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_SM_BUILTINS_PROPERTIES_NV’?
 1355 |         case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CUDA_KERNEL_LAUNCH_PROPERTIES_NV:
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |              VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_SM_BUILTINS_PROPERTIES_NV
Compiling drivers/gles3/storage/texture_storage.cpp ...
thirdparty/vulkan/vk_enum_string_helper.h: In function ‘const char* string_VkObjectType(VkObjectType)’:
thirdparty/vulkan/vk_enum_string_helper.h:2081:14: error: ‘VK_OBJECT_TYPE_CUDA_MODULE_NV’ was not declared in this scope; did you mean ‘VK_OBJECT_TYPE_CU_MODULE_NVX’?
 2081 |         case VK_OBJECT_TYPE_CUDA_MODULE_NV:
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |              VK_OBJECT_TYPE_CU_MODULE_NVX
thirdparty/vulkan/vk_enum_string_helper.h:2083:14: error: ‘VK_OBJECT_TYPE_CUDA_FUNCTION_NV’ was not declared in this scope; did you mean ‘VK_OBJECT_TYPE_CU_FUNCTION_NVX’?
 2083 |         case VK_OBJECT_TYPE_CUDA_FUNCTION_NV:
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |              VK_OBJECT_TYPE_CU_FUNCTION_NVX
thirdparty/vulkan/vk_enum_string_helper.h: In function ‘const char* string_VkDriverId(VkDriverId)’:
thirdparty/vulkan/vk_enum_string_helper.h:3521:14: error: ‘VK_DRIVER_ID_MESA_AGXV’ was not declared in this scope; did you mean ‘VK_DRIVER_ID_MESA_V3DV’?
 3521 |         case VK_DRIVER_ID_MESA_AGXV:
      |              ^~~~~~~~~~~~~~~~~~~~~~
      |              VK_DRIVER_ID_MESA_V3DV
thirdparty/vulkan/vk_enum_string_helper.h: In function ‘const char* string_VkBufferUsageFlagBits2KHR(uint64_t)’:
thirdparty/vulkan/vk_enum_string_helper.h:8290:24: error: ‘VK_BUFFER_USAGE_2_EXECUTION_GRAPH_SCRATCH_BIT_AMDX’ was not declared in this scope
 8290 |     if (input_value == VK_BUFFER_USAGE_2_EXECUTION_GRAPH_SCRATCH_BIT_AMDX) return "VK_BUFFER_USAGE_2_EXECUTION_GRAPH_SCRATCH_BIT_AMDX";
      |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Compiling drivers/gles3/storage/utilities.cpp ...
scons: *** [bin/obj/drivers/vulkan/rendering_context_driver_vulkan.linuxbsd.editor.x86_64.o] Error 1
scons: building terminated because of errors.

[clayjohn]

@podiki what do you mean when using vulkan-loader 1.4.321.0? All of the Vulkan headers are supplied by Godot in the third-party/Vulkan folder. In what way are you using a Vulkan loader at compile time?

Are you pointing to custom Vulkan header files instead of using the matching ones provided?

[podiki]

@clayjohn I think I might know what is happening here. There is a dlopen call for libvulkan (in thirdparty/volk) which won't work in Guix as there is no global library loading (no /lib for instance, everything is specified with full paths in /gnu/store). So, we patch that call to find the system-supplied library from the vulkan-loader package. We do keep thirdparty/vulkan but my guess is that since vulkan-loader is in the build environment, its headers are found first.

Anyway, I guess that is to say that in the Guix case, it is on our end to be worked out in packaging, though this is a new problem with 4.5 (something must have changed we need to adapt to on the packaging side). That said, we would love if we could further unbundle thirdparty stuff for packaging in general; the current builtin_ options in SConstruct don't cover everything.

Sorry for hijacking, though maybe this will help with the original issue. Something may have changed from 4.4 where now system vulkan headers are picked up before the Godot ones? Because at least with the packaging in Guix, 4.4 builds fine but 4.5 hits this issue, changing nothing else.

Edit: See also #104893 (comment) probably pretty similar (Nix/Guix very similar here), so I think the below guess about CPPEXTPATH is indeed related.

[podiki]

Perhaps f25fc34 which uses CPPEXTPATH for thirdparty directories? (I'm not very familiar with the details of the Godot SCons build system, just a guess)

[clayjohn]

@podiki That's a really good insight. This could definitely be an issue caused by f25fc34. It sounds like a mismatching version of the Vulkan headers.

So I guess a better solution would be to figure out why f25fc34 caused a regression here and figure out how to avoid polluting compilation units

Any ideas @Repiteo @akien-mga?

We now have three places that have regressed in basically the same way:

1. Nix building
2. Guix building
3. Building for whatever platform panreyes is building for

[akien-mga]

I think we might need to revert #104893 and its follow-up(s).

The change was promising but it's proving to be unreliable, and I don't have a deep enough understanding of include precedence and shenanigans to fully make sure it's bulletproof.

[panreyes]

So, in the end, I conclude this:

• There are no compatibility issues from using the Vulkan headers from godot/thirdparty instead of the ones from the OS.
• Something broke from 4.4 to 4.5 because of this line from that commit:

env["EXTINCPREFIX"] = "-isystem "

It might be even more quirky than adding my macros, but it might be a good idea to either:

• Prioritize thirdparty/vulkan headers over the OS headers (env["EXTINCPREFIX"] = "-ithirdparty/vulkan -isystem " ?)
• Add ifdefs for every symbol 😅

[akien-mga]

Reverting the change (it worked fine before) is definitely better than adding more hacks with potential pitfalls.

[podiki]

Thanks for the quick responses all! I just wanted to follow up with one thing from the Guix side, as it is actually a bit of an accident we hit this.

So far we had disabled volk (via use_volk=no) yet still had vulkan-loader so we could patch the dlopen calls in thirdparty/volk.c That doesn't quite make sense (I assume it would never hit that code if volk is disabled) and since the header includes has vulkan only when not using volk, we end up finding those system libraries due to the change mentioned above. If I enable volk, it builds fine (and I think works, have to check).

[Edit: Actually, I forgot there needs to be libvulkan found via pkg-config, so vulkan-loader is always in the build environment]

Anyway, I think it is good to find this out soon after the commits and in a way that was pretty easy to figure out. Having proper precedence for thirdparty/system libraries would be good. Probably this hasn't been seen more widely because the default has volk enabled. And in Guix we try to unbundle as much as we can. But I would think building on any system where vulkan-loader/headers are a different version and volk is disabled could hit this. Well, as long as the build is properly isolated and does not include the thirdparty stuff from system, it would be fine.

I'll work around this in the Guix package for 4.5 one way or another, but feel free to ping to test any changes in building.

[akien-mga]

Superseded by #111331.