Godot's Vulkan is non-compliant (broken) on arm32

[darksylinc]

Tested versions

4.4-master

System information

Godot v4.4.dev (44fa552) - Ubuntu 24.04.1 LTS 24.04 on X11 - X11 display driver, Multi-window, 2 monitors - Vulkan (Mobile) - dedicated AMD Radeon RX 6800 XT (RADV NAVI21) - AMD Ryzen 9 5900X 12-Core Processor (24 threads)

Issue description

RenderingDeviceDriver::id is defined as size_t:

class RenderingDeviceDriver : public RenderingDeviceCommons {
public:
    struct ID {
        size_t id = 0;
        _ALWAYS_INLINE_ ID() = default;
        _ALWAYS_INLINE_ ID(size_t p_id) :
                id(p_id) {}
    };
}

The problem is that in ARM32 sizeof( size_t ) == 4.

However VkPipeline (and possibly others) is defined like this:

#define VK_DEFINE_NON_DISPATCHABLE_HANDLE(object) typedef uint64_t object;
VK_DEFINE_NON_DISPATCHABLE_HANDLE(VkPipeline)

That is, VkPipeline is not a pointer. It is a uint64_t value. Yet Godot does this:

void RenderingDeviceDriverVulkan::pipeline_free(PipelineID p_pipeline) {
    static_assert(sizeof(p_pipeline.id) == sizeof(VkPipeline), "Wrong ABI"); // This will trigger.
    vkDestroyPipeline(vk_device, (VkPipeline)p_pipeline.id, VKC::get_allocation_callbacks(VK_OBJECT_TYPE_PIPELINE));
}

Basically it casts p_pipeline.id to VkPipeline; which is not legal. This will work in driver implementations that only use the first 32 bits of the variable. But it will break in implementations that actually use the latter 32 bits.

This affects all Vulkan handles that are declared via VK_DEFINE_NON_DISPATCHABLE_HANDLE. Vulkan objects declared via VK_DEFINE_HANDLE, like VkCommandBuffer, are meant to be actual pointers:

#define VK_DEFINE_HANDLE(object) typedef struct object##_T* object;
VK_DEFINE_HANDLE(VkCommandBuffer)

So those are safe.

The easiest/quickest solution I see is to just make RenderingDeviceDriver::id uint64_t on all platforms. I don't know what @reduz thinks.

Another one would be to go one by one on these vulkan "non-handles" and encapsulate them in a 32 bit pointer (an extra indirection).

Steps to reproduce

1. Compile with platform=android target=template_release arch=arm32
2. Game Over.

Minimal reproduction project (MRP)

N / A

[Calinou]

I'd go with uint64_t personally. Not that 32-bit devices currently have good Vulkan performance due to their weak GPU anyway...

[darksylinc]

I agree.

I've been discussing it with other members of the team and it seems we all agree. I had a few worries about certain possible casting horrors Godot could be doing, but it seems like my fears are unfounded.

I guess the only way to find out is to try it: Make RenderingDeviceDriver::ID (note: this is not the same as an RID) an uint64 and see if it breaks.