Functions in shaders can't shadow global variable names

[hmans]

Godot version

4.0.stable

System information

Apple M1

Issue description

Summary

When writing a function in a shader program, the function is not allowed to shadow existing global names (consts, uniforms, varyings) with its parameter signature or local variables.

Example 1

Using a parameter name that shadows an existing global:

Example 2

Using a local variable that shadows an existing global:

Bonus Example 3

The issue does not seem to appear when shadowing global function names:

Impact

• Shader authoring as a whole gets a little more complicated because the author needs to make sure that there are no collisions. In the case of a monolithic shader file that is entirely under the control of the author or team, this problem is largely offset by them being able to easily work around it through more verbose scoped variable names.
• Worse, though, is the issue's impact on shader includes. If someone is authoring shader includes (possibly with the intention to distribute them), it is now on them to use parameter and local variable names that are obfuscated enough to minimize a chance of collision with any global name that might be in use by the shaders that include them. (For example, you won't be able to use this shader include if your shader has a uniform named width, and so on.)

Steps to reproduce

• Write a shader with uniforms, varyings, and/or consts
• Add a function that uses a parameter (or local variable) of the same name

I've attached an MRP with an example shader that demonstrates this.

Minimal reproduction project

MRP-Shader-Variable-Shadowing.zip

[anhvu2001ct]

Any information about fixing this issue? I think a function parameter should be able to shadow a variable. Atm I must name all function parameter in my .gdshaderinc file with prefix like _ or func_ which looks strange to me.

[Calinou]

Atm I must name all function parameter in my .gdshaderinc file with prefix like _ or func_ which looks strange to me.

It's not necessarily a bad idea to prefix parameter names with p_, as this makes it easy to distinguish them from local variables and uniforms. (Some people also use u_ prefixes for uniforms for this reason.)

For reference, all parameter names in Godot's C++ codebase have a p_ prefix :slightly_smiling_face:

[guessy42]

This will also occur with shader uniforms without causing any warnings:

This code will cause the texture to scale:

render_mode unshaded;

vec2 rotate(vec2 uv, vec2 pivot, float angle)
{
    mat2 rotation = mat2(vec2(sin(angle), -cos(angle)),
                        vec2(cos(angle), sin(angle)));
    
    uv -= pivot;
    uv = uv * rotation;
    uv += pivot;
    return uv;
}

uniform sampler2D main_texture;
uniform vec2 pivot = vec2(0.5);
uniform float rotation = 1.5;

void fragment() {
	ALBEDO = texture(main_texture, rotate(UV, pivot, rotation)).rgb;
}

Whereas this code, that just changes the name of the uniform float rotation, will correctly rotate:

render_mode unshaded;

vec2 rotate(vec2 uv, vec2 pivot, float angle)
{
    mat2 rotation = mat2(vec2(sin(angle), -cos(angle)),
                        vec2(cos(angle), sin(angle)));
    
    uv -= pivot;
    uv = uv * rotation;
    uv += pivot;
    return uv;
}

uniform sampler2D main_texture;
uniform vec2 pivot = vec2(0.5);
uniform float rotation_amount = 1.5;

void fragment() {
	ALBEDO = texture(main_texture, rotate(UV, pivot, rotation_amount )).rgb;
}

[hmans]

For reference, all parameter names in Godot's C++ codebase have a p_ prefix 🙂

Please keep in mind that the issue described here doesn't only affect parameters, but also local variables inside functions, which, in order to minimize risk of colliding with root-level symbols, would then have to be named l_*?

Should the recommended use of p_* and possibly l_* be added to the documentation, particularly the Converting GLSL to Godot shaders guide?

[OhiraKyou]

I just ran into this while converting a project from the Compatibility render to Forward+, which, alongside the Mobile renderer, handles shadowing differently. I use a bunch of shader include files and, after a lot of debugging, found that function parameter and local variable values in the include files were being replaced with the values of uniforms in the consuming shaders—but only in Forward+ and Mobile.

MRP: godot-4.4-beta4-renderer-variable-shadowing.zip

Steps:

1. Open the MRP in Godot 4.4-beta4.
2. Observe that the cubes in test_scene_1.tscn are green in the Compatibility renderer.
3. Change the renderer to Forward+ or Mobile.
4. Observe that the cubes become red.