ALPHA and discard in light() have no effect in Compatibility or Forward+ renderers

[Wolfe2x7]

Godot version

v4.2.rc2.official [1ba920f]

System information

Godot v4.2.rc2 - Windows 10.0.19045 - GLES3 (Compatibility) - NVIDIA GeForce GTX 1660 Ti (NVIDIA; 31.0.15.2802) - Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz (12 Threads)

Issue description

Between 4.1.3 and 4.2, something changed so that writing to ALPHA in the light() function of a Spatial shader has no effect, if using the OpenGL Compatibility renderer. Writing to ALPHA still works in the fragment() function, or when using a Vulkan renderer.

EDIT: I failed to check for this before, but discard also similarly does nothing.

Here is a comparison of my project in 4.1.3 and 4.2 rc2 (I've already fixed it now by moving things to fragment()):

Steps to reproduce

1. Write a Spatial shader that sets ALPHA to a value lower than 1.0 in the light() function
2. Nothing happens to ALPHA

Minimal reproduction project

min-repro-11-25-23.zip

[Chaosus]

cc @clayjohn

[akien-mga]

I can reproduce the bug, but not the fact that it would be a regression. With the MRP, the bug is reproducible in 4.2-rc2, 4.1.3-stable and 4.1-stable.

[Wolfe2x7]

I can reproduce the bug, but not the fact that it would be a regression. With the MRP, the bug is reproducible in 4.2-rc2, 4.1.3-stable and 4.1-stable.

You're right; that's strange. My 4.1.3-stable project worked fine. I moved the necessary code to fragment() without issue, so whatever the problem is, I don't have to count on it behaving itself in the meantime.

[daustria]

i looked into this for a bit, posting what I make of the problem here. I am kind of new so I will spell things out more unnecessarily.

The key files here are scene.glsl and the actions.usage_defines in gles3\storage\material_storage.cpp.

In scene.glsl, line 953 is #CODE : LIGHT. This is where the user defined light shader code (that is, the code we write in light() in the godot shader) will go. An important thing to notice is that this code is only used if we do something to the built-in variables DIFFUSE_LIGHT and SPECULAR_LIGHT inside light(). This is because of the lines in material_storage.cpp:

actions.usage_defines["DIFFUSE_LIGHT"] = "#define USE_LIGHT_SHADER_CODE\n";
actions.usage_defines["SPECULAR_LIGHT"] = "#define USE_LIGHT_SHADER_CODE\n";

So if these variables arent used then USE_LIGHT_SHADER_CODE is never defined.

Now suppose that in light() we set alpha to some value, as well as the variable DIFFUSE_LIGHT so that our code actually gets used. Returning our focus back to the scene.glsl file, note that #CODE : LIGHT is inside the function light_compute which takes alpha as an inout parameter. What this means is that alpha will be modified everytime light_compute is called in scene.glsl. Importantly, light_compute is also called whenever light_process_spot and light_process_omni are called.

If you follow the code in scene.glsl, you will notice light_compute is called for every directional, omni, and spot light. So actually if you have any one of these types in your scene, the alpha value will be set.

To illustrate what's happening, I made a scene where the cube has its alpha value set in fragment(), and the sphere has its alpha value set in light(). I also wrote the line DIFFUSE_LIGHT = vec3(0.0, 0.0, 0.0) for the sphere's light() function, so that USE_LIGHT_SHADER_CODE is defined.

Here is a screenshot of the scene where there is no directional light.

Here is the scene when I add a directional light. the sphere's alpha value is properly set now, and both objects are transparent

[daustria]

another thing i am wondering is that, why is alpha a writable variable in light()? it doesnt seem important for any lighting calculations

[Wolfe2x7]

another thing i am wondering is that, why is alpha a writable variable in light()? it doesnt seem important for any lighting calculations. it seems like light() is intended just to set DIFFUSE_LIGHT and SPECULAR_LIGHT.

light() has several other built-ins for effects, including ones that can make ALPHA relevant, such as ATTENUATION. The "shadow to opacity" effect in a StandardMaterial3D draws only shadows, using those built-ins; eg. for shadows in augmented reality (AR) video.

Instead of for AR, I am drawing shadows onto a glassy reflective plane that is actually just holes in the ground*. Unfortunately, for that purpose, I've found that discard isn't working yet in the Compatibility renderer either. Everything is fine in Vulkan.

(* - That's a new shader I wrote since opening this issue, before discovering that this issue breaks that effect in OpenGL too.)

[jsjtxietian]

The doc mentioned it pretty well "To write a light function, assign something to DIFFUSE_LIGHT or SPECULAR_LIGHT. Assigning nothing means no light is processed." ( https://docs.godotengine.org/en/latest/tutorials/shaders/shader_reference/spatial_shader.html#light-built-ins )

Three thoughts :

1. I think we can add "and the light function is not called" after the doc to make it clearer.
2. Furthermore, we can warn the user during shader compiling that light function without any assignment to DIFFUSE_LIGHT or SPECULAR_LIGHT will not take effect
3. USE_LIGHT_SHADER_CODE is only used in gles3's scene shader, should the same be applied to forwardplus and mobile?

cc @clayjohn Any thoughts or suggestions ? I'm happy to help if the suggestions are appropriate.

[Wolfe2x7]

@jsjtxietian The documentation is misleading and something is wrong, because light() shaders that do not write to DIFFUSE_LIGHT or SPECULAR_LIGHT work in Mobile. Adding DIFFUSE_LIGHT has no effect at all in the other two renderers; the issue persists.

Can't the other two renderers operate the same way that Mobile does?

[clayjohn]

@jsjtxietian It looks like this is an area that needs a lot of cleanup as there is a lot of leftovers from the 3.x era.

First of all:

1. The docs are out of date, the custom light shader should be used if the function is defined even if DIFFUSE_LIGHT or SPECULAR_LIGHT are not written to
2. The Compatibility renderer is wrongly using the 3.x style of defining the light shader function

The compatibility renderer should us LIGHT_CODE_USED instead of USE_LIGHT_SHADER_CODE. LIGHT_CODE_USED is emitted automatically by the shader_gles3 compiler:

godot/drivers/gles3/shader_gles3.cpp

Line 179 in 0bcc0e9

builder.append(String("#define ") + String(E.key) + "_CODE_USED\n");

The usage define for USE_LIGHT_SHADER_CODE should be removed:

godot/drivers/gles3/storage/material_storage.cpp

Lines 1329 to 1330 in 0bcc0e9

	actions.usage_defines["DIFFUSE_LIGHT"] = "#define USE_LIGHT_SHADER_CODE\n";
	actions.usage_defines["SPECULAR_LIGHT"] = "#define USE_LIGHT_SHADER_CODE\n";

Ity should also be removed from the other renderers as USE_LIGHT_SHADER_CODE was being defined but never used:

godot/servers/rendering/renderer_rd/forward_clustered/scene_shader_forward_clustered.cpp

Lines 683 to 684 in 0bcc0e9

	actions.usage_defines["DIFFUSE_LIGHT"] = "#define USE_LIGHT_SHADER_CODE\n";
	actions.usage_defines["SPECULAR_LIGHT"] = "#define USE_LIGHT_SHADER_CODE\n";

godot/servers/rendering/renderer_rd/forward_mobile/scene_shader_forward_mobile.cpp

Lines 587 to 588 in 0bcc0e9

	actions.usage_defines["DIFFUSE_LIGHT"] = "#define USE_LIGHT_SHADER_CODE\n";
	actions.usage_defines["SPECULAR_LIGHT"] = "#define USE_LIGHT_SHADER_CODE\n";

There is also LIGHT_SHADER_CODE_USED which was defined but never used by the canvas renderers:

godot/drivers/gles3/storage/material_storage.cpp

Line 1181 in 0bcc0e9

actions.usage_defines["LIGHT"] = "#define LIGHT_SHADER_CODE_USED\n";

godot/servers/rendering/renderer_rd/renderer_canvas_render_rd.cpp

Line 2582 in 0bcc0e9

actions.usage_defines["LIGHT"] = "#define LIGHT_SHADER_CODE_USED\n";