Use "volk" instead of statically linked Vulkan loader.

[bruvzg]

Tested so far:

• Building and running on macOS (with and without use_static_mvk)
• Building for Windows (volk build only, using MinGW), and running it on wine and Windows 10
• Building for iOS (with static MoltenVK only), and running on simulator.
• Building for Linux (tested volk build only) and running (using swiftshader).
• Building for Android (both with volk and NDK loader), did not tested it on device.

Fixes #51444

[bruvzg]

I have replaced use_static_mvk flag with use_volk flag, which is disabled by default on Android and iOS, and enabled for the rest of platforms.

On iOS and macOS disabling it will use static MoltenVK, on other platforms use system loader (NDK loader and headers in case of Android).

[akien-mga]

The variable should be renamed as it's not just VMA anymore. Could likely be just thirdparty_sources.

But there's something weird, if using volk we build thirdparty/vulkan/vk_mem_alloc.cpp and otherwise we build thirdparty/vulkan/android/vk_mem_alloc.cpp. What's the difference and why does volk let us use to the common version instead of the Android one?

This needs at least a comment to clarify.

[bruvzg]

android/vk_mem_alloc.cpp uses VMA header from NDK, no idea why was used (was added in the #37745), but it definitely won't work with volk, since it require replacing vulkan.h includes with volk.h to work.

[akien-mga]

Based on https://github.com/zeux/volk#basic-usage:

If some files in your application include vulkan/vulkan.h and don't include volk.h, this can result in symbol conflicts; consider defining VK_NO_PROTOTYPES when compiling code that uses Vulkan to make sure this doesn't happen.

and:

https://github.com/zeux/volk/blob/master/volk.h#L13-L15

Doesn't that mean that it should work provided that we include volk.h before including vulkan/vulkan.h? Or if we define VK_NO_PROTOTYPES?

[akien-mga]

But also, if it works, we might be able to get rid of the android/vk_mem_alloc.cpp now and just use the vendored one? It seems the custom one was just to get rid of some warnings, there might be a better way to do it? #37729 (comment)

[akien-mga]

Doesn't that mean that it should work provided that we include volk.h before including vulkan/vulkan.h? Or if we define VK_NO_PROTOTYPES?

BTW I think that means you don't need to patch thirdparty code to use volk, you just need to define VK_NO_PROTOTYPES in their build env I think. Or we have to make sure to include volk.h before including them.

[bruvzg]

This repository contains the Vulkan loader that is used for Linux, Windows, MacOS, and iOS. There is also a separate loader, maintained by Google, which is used on Android.

Seems like Android uses its custom loader, but we probably do not need to use NDK VMA. It's building fine with built-in one, and was added only to suppress some warnings, there are none now. I'll replace it with the built-on one and keep only custom loader headers. That's should fix #51524 build issues as well.

[bruvzg]

VK_NO_PROTOTYPES in their build env I think. Or we have to make sure to include volk.h before including them.

Neither of this works, it's failing with "error: redefinition of ... as different kind of symbol" errors.

[akien-mga]

I guess that means that there's still a file which includes vulkan/vulkan.h before volk.h or before a definition of VK_NO_PROTOTYPES. But it's not a big deal, we can have another look at this later, or see with upstream if indeed what they described in the README no longer works with the current SDK version.

[akien-mga]

So the new default for macOS is to use volk, and if disabled it's to use static MoltenVK IIUC, with no option anymore for dynamic MoltenVK. That works for me 👍

How does volk handle MoltenVK? Does it find it from the Vulkan SDK if installed, and from our bundled .xcframework otherwise?

[bruvzg]

volk will load try loading libvulkan.1.dylib (which is installed as part of SDK), if it's not found it will try libMoltenVK.dylib.

xcframework if only for static linking only.

[akien-mga]

Small refactoring proposal for the SCsub, should be equivalent but it flows better IMO:

#!/usr/bin/env python

Import("env")

thirdparty_obj = []
thirdparty_dir = "#thirdparty/vulkan"
thirdparty_volk_dir = "#thirdparty/volk"

if env["use_volk"]:
    env.AppendUnique(CPPDEFINES=["USE_VOLK"])
    env.Prepend(CPPPATH=[thirdparty_volk_dir])

if env["platform"] == "android" and not env["use_volk"]:
    # Use NDK Vulkan headers
    ndk_vulkan_dir = env["ANDROID_NDK_ROOT"] + "/sources/third_party/vulkan/src"
    thirdparty_includes = [
        ndk_vulkan_dir,
        ndk_vulkan_dir + "/include",
        ndk_vulkan_dir + "/layers",
        ndk_vulkan_dir + "/layers/generated",
    ]
    env.Prepend(CPPPATH=thirdparty_includes)
else:
    # Use bundled Vulkan headers
    env.Prepend(CPPPATH=[thirdparty_dir, thirdparty_dir + "/include"])

if env["platform"] == "android":
    env.AppendUnique(CPPDEFINES=["VK_USE_PLATFORM_ANDROID_KHR"])
elif env["platform"] == "iphone":
    env.AppendUnique(CPPDEFINES=["VK_USE_PLATFORM_IOS_MVK"])
elif env["platform"] == "linuxbsd":
    env.AppendUnique(CPPDEFINES=["VK_USE_PLATFORM_XLIB_KHR"])
elif env["platform"] == "osx":
    env.AppendUnique(CPPDEFINES=["VK_USE_PLATFORM_MACOS_MVK"])
elif env["platform"] == "windows":
    env.AppendUnique(CPPDEFINES=["VK_USE_PLATFORM_WIN32_KHR"])

# Build Vulkan memory allocator and volk
env_thirdparty_vma = env.Clone()
env_thirdparty_vma.disable_warnings()
thirdparty_sources_vma = [thirdparty_dir + "/vk_mem_alloc.cpp"]

if env["use_volk"]:
    env_thirdparty_vma.AppendUnique(CPPDEFINES=["VMA_STATIC_VULKAN_FUNCTIONS=1"])
    env_thirdparty_volk = env.Clone()
    env_thirdparty_volk.disable_warnings()

    thirdparty_sources_volk = [thirdparty_volk_dir + "/volk.c"]
    env_thirdparty_volk.add_source_files(thirdparty_obj, thirdparty_sources_volk)

env_thirdparty_vma.add_source_files(thirdparty_obj, thirdparty_sources_vma)


env.drivers_sources += thirdparty_obj


# Godot source files

driver_obj = []

env.add_source_files(driver_obj, "*.cpp")
env.drivers_sources += driver_obj

# Needed to force rebuilding the driver files when the thirdparty code is updated.
env.Depends(driver_obj, thirdparty_obj)

[akien-mga]

I edited my above proposal with a couple tiny edits.

[akien-mga]

Last thing and it's good to go:

diff --git a/COPYRIGHT.txt b/COPYRIGHT.txt
index b2a930a4bb..5bd67960da 100644
--- a/COPYRIGHT.txt
+++ b/COPYRIGHT.txt
@@ -438,12 +438,17 @@ Copyright: 2011, Khaled Mamou
   2003-2009, Erwin Coumans
 License: BSD-3-clause
 
+Files: ./thirdparty/volk/
+Comment: volk
+Copyright: 2018-2019, Arseny Kapoulkine
+License: Expat
+
 Files: ./thirdparty/vulkan/
-Comment: Vulkan Ecosystem Components (Vulkan ICD loader and headers)
-Copyright: 2014-2020, The Khronos Group Inc.
-  2014-2020, Valve Corporation
-  2014-2020, LunarG, Inc.
-  2015-2020, Google Inc.
+Comment: Vulkan Headers
+Copyright: 2014-2021, The Khronos Group Inc.
+  2014-2021, Valve Corporation
+  2014-2021, LunarG, Inc.
+  2015-2021, Google Inc.
 License: Apache-2.0
 
 Files: ./thirdparty/vulkan/vk_mem_alloc.h

[akien-mga]

Looks great! Thanks a lot.

[akien-mga]

Tested on Linux with both use_volk=yes (default) and use_volk=no (using system Vulkan Loader, needs to be installed) and both work fine 👍

[Calinou]

Is it possible to implement https://github.com/zeux/volk#optimizing-device-calls in Godot?

[bruvzg]

Is it possible to implement https://github.com/zeux/volk#optimizing-device-calls in Godot?

Probably, I do not see any reason why it won't work, but I have not tested it.

[zeux]

FWIW Roblox ships with volk on Android as well (valuable for reduced overhead and extension support), so that should work the same as desktop platforms.

[akien-mga]

FWIW Roblox ships with volk on Android as well (valuable for reduced overhead and extension support), so that should work the same as desktop platforms.

Thanks for the input! We'll do that, that simplifies things and avoids limiting us to whichever version of the headers our current NDK provides. (Done in #51592.)