godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.81k stars 20.14k forks source link

Indexing into uniform array of sampler2D in a custom shader fails with variable index #95139

Closed brndd closed 1 month ago

brndd commented 1 month ago

Tested versions

System information

Godot v4.2.2.stable.mono - Fedora Linux 40 (KDE Plasma) - Wayland - Vulkan (Forward+) - dedicated AMD Radeon RX 6700 XT (RADV NAVI22) () - 12th Gen Intel(R) Core(TM) i9-12900K (24 Threads)

Issue description

I am working on a heightmapped terrain system divided into tiles. Each tile has a 1024x1024 heightmap for 1025x1025 vertices, so the right and bottom edge vertices must sample from a neighbouring tile's heightmap.

To accomplish this, I'm using a uniform sampler2D array[4] and dynamically indexing into it in the shader. I've been taught that branching in shaders is bad, so I sought to do this without branching like this (simplified from reproduction project below):

int heightmap_index = 0; //Bitflag. 0: this tile, 1: right neighbor, 2: bottom neighbor, 3: bottom right neighbor
heightmap_index |= 1 * (texel.x / 1024);
heightmap_index |= 2 * (texel.y / 1024);
texel = texel % 1024; //wraps 1024th vertex back to zero so it's sampled from top/left edge of neighbor
h = texelFetch(u_terrain_heightmap[heightmap_index], texel, 0).r;

However, this fails to work, and after debugging it for two days I suspect I'm either dealing with a compiler bug or inadvertedly hitting UB.

In this screenshot, I've coloured vertices red or green based on their heightmap_index value (as such: debug_color.r = float(heightmap_index & 1); and debug_color.g = float((heightmap_index & 2) >> 1);) to verify that the bitflag is getting set correctly. As you can see, vertices on the right and bottom edge are red and green respectively, with the bottom right one being yellow, yet they are not being sampled from the correct sampler2D:

2024-08-04T19:37:18

Here is a screenshot where I circumvent the issue by using branching code. This is how the heightmap is supposed to look:

2024-08-04T19:37:27

In this screenshot, I have replaced the bit shifting above with these if statements:

if (((u_tile_neighbor_flag & 3) != 0) && texel.x == 1024 && texel.y == 1024) {
    debug_color = vec3(1.0, 1.0, 0.0);
    h = texelFetch(u_terrain_heightmap[3], texel % 1024, 0).r;
}
else if (((u_tile_neighbor_flag & 1) != 0) && texel.x == 1024) {
    debug_color = vec3(1.0, 0.0, 0.0);
    h = texelFetch(u_terrain_heightmap[1], texel % 1024, 0).r;
}
else if (((u_tile_neighbor_flag & 2) != 0) && texel.y == 1024) {
    debug_color = vec3(0.0, 1.0, 0.0);
    h = texelFetch(u_terrain_heightmap[2], texel % 1024, 0).r;
}
else {
    h = texelFetch(u_terrain_heightmap[0], texel % 1024, 0).r;
}

Steps to reproduce

The minimal reproduction project below contains two scenes: broken.tscn and working.tscn, which contain four PlaneMeshes that each have their own ShaderMaterial that uses the same shader (broken.gdshader in the broken one and working.gdshader in the working one) with different shader parameters (different u_tile_neighbor_flag and different textures assigned to u_terrain_heightmap).

Minimal reproduction project (MRP)

shader_repro.zip

shader_repro2.zip (simpler and lighter)

Chaosus commented 1 month ago

I don't see any difference between default working and broken scene (Windows 11/GTX 4080):

изображение

brndd commented 1 month ago

Interesting, thanks. That to me suggests that this is AMD or Linux specific. Bit-twiddling is probably somewhat unorthodox in a shader, so it being a driver issue or something sounds plausible... but I'm not familiar enough with the stack involved here to know how to approach something like that. My understanding is that Godot would compile the shader into SPIR-V after which the driver takes over, is that right?

brndd commented 1 month ago

FWIW this also occurs on my laptop which has an Intel 5300U iGPU, so it's probably either something Linux-specific or Mesa-specific. Would appreciate if someone with an Nvidia GPU on Linux could test this.

brndd commented 1 month ago

Sorry for the triple post...

Is there a way to get the SPIR-V code of the shader out of Godot? I think that would help with debugging. I tried looking in the shader cache but the files there seem to be in a Godot-specific format that I don't know how to read.

brndd commented 1 month ago

I created a simpler repro [shader_repro2.zip] for this that only uses one PlaneMesh and two textures, then indexes into the textures alternatingly like this:

ivec2 texel = ivec2(VERTEX.xz);
index |= 1 * ((texel.x + texel.y) % 2);
float h = texelFetch(height[index], texel, 0).r;

I then tried to reproduce this with a simple C++ OpenGL application (https://gist.github.com/brndd/2c94d3d3d76cc8b5426e00e8c0ce5e69), but a comparable shader on a comparable mesh seems to work fine there (but I had ChatGPT write all the boilerplate for me, so there could be differences I am not accounting for -- not least that Godot uses Vulkan):

2024-08-05T01:21:55


I also learnt that in OpenGL lingo, what I'm doing is indexing into a sampler array using a dynamically uniform expression, for whatever that is worth.

Chaosus commented 1 month ago

I then tried to reproduce this with a simple C++ OpenGL application

You can check it on ShadeRED (https://shadered.org/) or shadertoy (https://www.shadertoy.com) rather than create an own OpenGL app.

brndd commented 1 month ago

Reproducing it with Shadertoy isn't possible because this syntax isn't legal in GLSL versions before 4.00. Judging from the fact that I couldn't reproduce it with OpenGL, it's probably a Vulkan-specific issue. I suspect it's probably a bug in Mesa, but I'll hold off for a few days before reporting this upstream to see if someone with an Nvidia card on Linux wants to test the MRP.

Calinou commented 1 month ago

Is there a way to get the SPIR-V code of the shader out of Godot? I think that would help with debugging. I tried looking in the shader cache but the files there seem to be in a Godot-specific format that I don't know how to read.

You can run Godot with the --generate-spirv-debug-info command line argument. Note that this argument isn't forwarded from the project manager to the editor, or from the editor to the running project, so make sure to add it to the Main Run Args project setting if running from the editor.

--help says:

Generate SPIR-V debug information. This allows source-level shader debugging with RenderDoc.

brndd commented 1 month ago

Thanks. Debugging the shader with RenderDoc was mostly going over my head, so I elected to pass this on to the Mesa folks, as I feel it's pretty likely at this point to be a Mesa issue: https://gitlab.freedesktop.org/mesa/mesa/-/issues/11675

Let's see what they have to say.

brndd commented 1 month ago

The Mesa folks told me that the way I'm doing this is in fact UB.

However, there's a GLSL hint/function that makes this not UB: nonuniformEXT(). This currently doesn't seem to be supported in Godot's shading language.

This hint is part of a Vulkan feature called descriptor indexing, which used to be an extension but was promoted into core in Vulkan 1.2. It seems quite well supported in practice (desktop hardware newer than Intel's Skylake iGPUs seem to support it), so perhaps it could be enabled in Godot? It seems like a quite useful feature.

This blog post seems to explain it quite well: https://anki3d.org/resource-uniformity-bindless-access-in-vulkan/

brndd commented 1 month ago

After researching this a bit more I felt confident enough to post a feature proposal: https://github.com/godotengine/godot-proposals/issues/10423. Closing this to continue the discussion there.