vulkan: Update all components to Vulkan SDK 1.3.283.0

[akien-mga]

Draft as I need help to rediff the SPIRV-Reflect specialization constant patch. CC @godotengine/rendering

You can help me by checking out this PR, trying to apply thirdparty/spirv-reflect/patches/specialization-constants.patch with patch -p3 < thirdparty/spirv-reflect/patches/specialization-constants.patch, and then figuring out how to resolve the hunks that couldn't be applied. Once the changes are good, you can do git diff > mydiff and copy that file on top of thirdparty/spirv-reflect/patches/specialization-constants.patch, to include in the commit.

It seems like upstream added a (more minimal?) spec constant struct with the same name, so we likely need to fold our changes into their new API:

• Add baseline Spec Constant API support KhronosGroup/SPIRV-Reflect#242

Worth noting that there are two open PRs upstream to add the kind of spec constants support we have, which we could help drive to completion / merge:

• Adding preliminary SpecConstant support KhronosGroup/SPIRV-Reflect#154 (derived from @reduz's Implement specialization constants #50325)
• Added support for specialization constants. They are now automaticall… KhronosGroup/SPIRV-Reflect#197 (which was made independently later on, missing the fact there was already a PR for that)

[RandomShaper]

Re-done patch to spirv-reflect:
specialization-constants.patch

[akien-mga]

Re-done patch to spirv-reflect: specialization-constants.patch

Thanks, added to the PR. Should be good to go then.

[akien-mga]

@bruvzg The macOS and iOS builds for glslang are failing with:

In file included from thirdparty/glslang/glslang/GenericCodeGen/Link.cpp:40:
In file included from thirdparty/glslang/glslang/GenericCodeGen/../Include/ShHandle.h:48:
thirdparty/glslang/glslang/GenericCodeGen/../Include/InfoSink.h:104:37In file included from thirdparty/glslang/glslang/GenericCodeGen/CodeGen.cpp:36:
In file included from thirdparty/glslang/glslang/GenericCodeGen/../Include/ShHandle.h:48:
thirdparty/glslang/glslang/GenericCodeGen/../Include/InfoSink.h:104:37: error: 'absolute' is unavailable: introduced in macOS 10.15
            append(std::filesystem::absolute(shaderFileName).string());
                                    ^
: error: 'absolute' is unavailable: introduced in macOS 10.15
/Applications/Xcode_15.0.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__filesystem/operations.h:65:35: note: 'absolute' has been explicitly marked unavailable here
inline _LIBCPP_HIDE_FROM_ABI path absolute(const path& __p) { return __absolute(__p); }
                                  ^

In file included from thirdparty/glslang/glslang/GenericCodeGen/Link.cpp:40:
In file included from thirdparty/glslang/glslang/GenericCodeGen/../Include/ShHandle.h:48:
thirdparty/glslang/glslang/GenericCodeGen/../Include/InfoSink.h:104:37: error: 'absolute' is unavailable: introduced in iOS 13.0 [2]
             append(std::filesystem::absolute(shaderFileName).string());
                                     ^

Sounds like using this glslang version would force us to bump our min macOS and iOS releases.
Should we send a patch upstream to restore support for older versions?

Edit: Filed a bug report upstream:

• Usage of std::filesystem::absolute in glslang/Include/InfoSink.h removes support for macOS < 10.15 and iOS < 13.0 KhronosGroup/glslang#3604

[bruvzg]

Sounds like using this glslang version would force us to bump our min macOS and iOS releases.
Should we send a patch upstream to restore support for older versions?

Vulkan (with a feature set used by Godot) won't properly work on macOS 10.14 and iOS 12, it's only for OpenGL backend. Both versions are unsupported for a long time (but current Xcode still can target both), so I'm not sure if there's much reason to keep supporting it, or bump the min versions.

[akien-mga]

Sounds like using this glslang version would force us to bump our min macOS and iOS releases.
Should we send a patch upstream to restore support for older versions?

Vulkan (with a feature set used by Godot) won't properly work on macOS 10.14 and iOS 12, it's only for OpenGL backend. Both versions are unsupported for a long time (but current Xcode still can target both), so I'm not sure if there's much reason to keep supporting it, or bump the min versions.

It probably doesn't make sense to attempt using our Vulkan/Metal renderers on macOS 10.14 or iOS 12 indeed, but for OpenGL I think it may still make sense if we can preserve compatibility.

Judging by https://gs.statcounter.com/ios-version-market-share/ stats, iOS 12.5 (which seemed to be a LTS and only got EOL'ed in Jan 2023) still has around 1.5% market share among iOS users:

For macOS, stats from the same website: https://gs.statcounter.com/macos-version-market-share/

So we'd lose 2.5% of macOS users by dropping 10.13 and 10.14.

[bruvzg]

Seems like it only for output prints with the specific flag, so we can patch it locally by removing if (absolute) condition.

[akien-mga]

Made this small hack to restore compat for older Apple platforms, until the issue is resolved upstream or we decide to increase our min supported versions to drop iOS 12.0 and macOS 10.13 and 10.14.

[bruvzg]

For macOS, stats from the same website

That's some strange data, seem to include only outdated and unsupported versions of macOS, currently only Monterey (12) and newer are getting security updates.

[akien-mga]

For macOS, stats from the same website

That's some strange data, seem to include only outdated and unsupported versions of macOS, currently only Monterey (12) and newer are getting security updates.

Yeah I think they simply didn't update their heuristics when macOS bumped from 10.15 to 11.0, so they seem to track all >= 10.15 into "Catalina". The data for older releases may still be somewhat reliable.

[akien-mga]

Getting a test failure which does sound related to this PR:

#################### RenderSceneBuffersRD ####################

RenderSceneBuffersRD.has_texture --- executing with 2 parameters [&"", &""]
GDSCRIPT CODE:     RenderSceneBuffersRD.new().has_texture(StringName(""), StringName(""))

RenderSceneBuffersRD.create_texture_from_format --- executing with 5 parameters [&"", &"", <RDTextureFormat#-9223371657488353317>, <RDTextureView#-9223371657471576100>, false]
GDSCRIPT CODE:     RenderSceneBuffersRD.new().create_texture_from_format(StringName(""), StringName(""), RDTextureFormat.new(), RDTextureView.new(), false)
godot.linuxbsd.editor.dev.double.x86_64.san: thirdparty/vulkan/vk_mem_alloc.h:3931: bool FindMemoryPreferences(bool, const VmaAllocationCreateInfo&, VmaBufferImageUsage, VkMemoryPropertyFlags&, VkMemoryPropertyFlags&, VkMemoryPropertyFlags&): Assertion `0 && "VMA_MEMORY_USAGE_AUTO* values can only be used with functions like vmaCreateBuffer, vmaCreateImage so that the details of the created resource are known." " Maybe you use VkBufferUsageFlags2CreateInfoKHR but forgot to use VMA_ALLOCATOR_CREATE_KHR_MAINTENANCE5_BIT?"' failed.
Aborted (core dumped)

[akien-mga]

I just rebased, and updated VMA to the newly tagged 3.1.0 release (no change compared to previous git snapshot in that PR). So I assume the CI will still fail similarly.

Then I'll try reverting VMA to the previous version (that we have in the master branch) to see if that's indeed what causes our issue.

[akien-mga]

So reverting VMA to an older commit fixed it, i.e. this commit (which I'll now remove): 2b466bd

The error on CI is:

godot.linuxbsd.editor.dev.double.x86_64.san: thirdparty/vulkan/vk_mem_alloc.h:3931: bool FindMemoryPreferences(bool, const VmaAllocationCreateInfo&, VmaBufferImageUsage, VkMemoryPropertyFlags&, VkMemoryPropertyFlags&, VkMemoryPropertyFlags&): Assertion `0 && "VMA_MEMORY_USAGE_AUTO* values can only be used with functions like vmaCreateBuffer, vmaCreateImage so that the details of the created resource are known." " Maybe you use VkBufferUsageFlags2CreateInfoKHR but forgot to use VMA_ALLOCATOR_CREATE_KHR_MAINTENANCE5_BIT?"' failed.

And that's consistent with the fact that the VMA update mostly adds support for KHR_MAINTENANCE5.

I don't know much about that, but maybe @DarioSamo or @RandomShaper could have a look at what we're missing?

[RandomShaper]

This patch may do the trick:

diff --git a/drivers/vulkan/rendering_device_driver_vulkan.cpp b/drivers/vulkan/rendering_device_driver_vulkan.cpp
index 131e0e4a8a..d38eb5a88d 100644
--- a/drivers/vulkan/rendering_device_driver_vulkan.cpp
+++ b/drivers/vulkan/rendering_device_driver_vulkan.cpp
@@ -942,6 +942,10 @@ Error RenderingDeviceDriverVulkan::_initialize_allocator() {
 	allocator_info.physicalDevice = physical_device;
 	allocator_info.device = vk_device;
 	allocator_info.instance = context_driver->instance_get();
+	const bool use_1_3_features = physical_device_properties.apiVersion >= VK_API_VERSION_1_3;
+	if (use_1_3_features) {
+		allocator_info.flags |= VMA_ALLOCATOR_CREATE_KHR_MAINTENANCE5_BIT;
+	}
 	VkResult err = vmaCreateAllocator(&allocator_info, &allocator);
 	ERR_FAIL_COND_V_MSG(err, ERR_CANT_CREATE, "vmaCreateAllocator failed with error " + itos(err) + ".");

Explanation: with Vulkan >= 1.3, KHR_MAINTENANCE5 extension is implicitly enabled. VMA needs to know if that's the case so it uses the right code path in the algo to find the right memory type.

Not tested, so, well, TIWAGOS.

PS: There are other VMA creation flags related to extensions that we may want to set in case those are either implicitly enabled or explicitly by us in case they're available so maybe VMA can do a better job somehow.

[akien-mga]

I gave it a try in the latest commit (7479f84), but that doesn't seem to solve it, it still fails at the same API test:

RenderSceneBuffersRD.create_texture_from_format --- executing with 5 parameters [&"", &"", <RDTextureFormat#-9223371657404467237>, <RDTextureView#-9223371657387690020>, false]
GDSCRIPT CODE:     RenderSceneBuffersRD.new().create_texture_from_format(StringName(""), StringName(""), RDTextureFormat.new(), RDTextureView.new(), false)
godot.linuxbsd.editor.dev.double.x86_64.san: thirdparty/vulkan/vk_mem_alloc.h:3931: bool FindMemoryPreferences(bool, const VmaAllocationCreateInfo&, VmaBufferImageUsage, VkMemoryPropertyFlags&, VkMemoryPropertyFlags&, VkMemoryPropertyFlags&): Assertion `0 && "VMA_MEMORY_USAGE_AUTO* values can only be used with functions like vmaCreateBuffer, vmaCreateImage so that the details of the created resource are known." " Maybe you use VkBufferUsageFlags2CreateInfoKHR but forgot to use VMA_ALLOCATOR_CREATE_KHR_MAINTENANCE5_BIT?"' failed.

[RandomShaper]

I can't reproduce on Windows. However, I've found that this is something else we'd want to do when initializing allocator_info:
allocator_info.vulkanApiVersion = physical_device_properties.apiVersion;

It stops VMA from assuming we're using Vulkan 1.0, and so lets it collect pointers to certain functions that make some operations more efficient.

Can you test it and see if it interacts with the issue at hand?

[akien-mga]

Tried it, that doesn't seem to fix it either.

I managed to reproduce the issue locally on Linux with a regular dev build from this branch, running the command from the test project in a new project:

RenderSceneBuffersRD.new().create_texture_from_format(StringName(""), StringName(""), RDTextureFormat.new(), RDTextureView.new(), false)

It's not showing me the assert message for some reason but it crashes all the same:

(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x00000000073e3052 in vmaFindMemoryTypeIndexForImageInfo (allocator=0xbc830e0, pImageCreateInfo=0x7fffffff3c40, pAllocationCreateInfo=0x7fffffff3c00, pMemoryTypeIndex=0x7fffffff3b54)
    at thirdparty/vulkan/vk_mem_alloc.h:15223
#2  0x00000000073f7caa in RenderingDeviceDriverVulkan::texture_create (this=0xbc37df0, p_format=..., p_view=...) at drivers/vulkan/rendering_device_driver_vulkan.cpp:1464
#3  0x0000000009cd0e78 in RenderingDevice::texture_create (this=0xbc37110, p_format=..., p_view=..., p_data=...) at ./servers/rendering/rendering_device.cpp:778
#4  0x0000000009ffe5d8 in RendererRD::TextureStorage::TextureStorage (this=0xb345730) at ./servers/rendering/renderer_rd/storage_rd/texture_storage.cpp:114
#5  0x0000000009ec9f79 in RendererCompositorRD::RendererCompositorRD (this=0xbf551f0) at ./servers/rendering/renderer_rd/renderer_compositor_rd.cpp:305
#6  0x0000000005b14cdc in RendererCompositorRD::_create_current () at ./servers/rendering/renderer_rd/renderer_compositor_rd.h:138
#7  0x0000000009cacf11 in RendererCompositor::create () at ./servers/rendering/renderer_compositor.cpp:42
#8  0x0000000009d2acbd in RenderingServerDefault::_init (this=0xb347750) at ./servers/rendering/rendering_server_default.cpp:220
#9  0x0000000009d2b0f2 in RenderingServerDefault::init (this=0xb347750) at ./servers/rendering/rendering_server_default.cpp:259
#10 0x0000000005b8e872 in Main::setup2 () at main/main.cpp:2790
#11 0x0000000005b8ca8b in Main::setup (execpath=0x7fffffffdba9 "/home/akien/.local/bin/godot-git", argc=1, argv=0x7fffffffd730, p_second_phase=true) at main/main.cpp:2432
#12 0x0000000005ada225 in main (argc=2, argv=0x7fffffffd728) at platform/linuxbsd/godot_linuxbsd.cpp:74

[RandomShaper]

That's been useful! The problem is that piece of script is trying to create a texture with not a single usage bit specified. See #92587.

[akien-mga]

That did the trick! Should I keep the changes we made to allocator_info, or revert them for now?

[RandomShaper]

That did the trick! Should I keep the changes we made to allocator_info, or revert them for now?

Good question! The one in the patch is needed as part of the update. However, the other line of code is not strictly part of the scope of this PR. It may be better to keep it as a separate commit. Besides, it's good to have the ability to frame that other change in a bisect.