Allow assigning a varying in shader light function

[hippogyz]

Describe the project you are working on

A 3D game with cel shading.

Describe the problem or limitation you are having in your project

As the issue #484 said, godot shader handles each light independently, which makes it almost impossible for doing things like color ramp after all lights computed.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

A workaround is storing mid-data between light process and treating each output of light() as the final result calculated with the mid-data. The varying variable could act as the role for storing those mid-data if it is allowed assigning varying in light() function.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

For example, achieving color ramp after all lights computed can be done with code below:

varying vec3 light_influence;

void fragment() {
  light_influence = vec3(0);
}

void light() {
  // store mid-data in varying variable
  light_influence += clamp(dot(NORMAL, LIGHT), 0.0, 1.0) * ATTENUATION * LIGHT_COLOR;

  // treat OUTPUT as final result since we don't know which light() is the last call
  // sample from the ramp depending on the light intensity
  vec2 ramp_uv =  vec2(get_hsv_v(light_influence), 0.5);
  DIFFUSE_LIGHT = texture(ramp, ramp_uv);
}

If this enhancement will not be used often, can it be worked around with a few lines of script?

No.

The workaround I found for current version is quite complex and unrobust:

1. Copy all meshes and use a custom shader which output mid-data with DIFFUSE_LIGHT.
2. Use a sub-viewport to render these copied meshes.
3. Get mid-data from the viewport texture.

In short, I need a two-pass rendering for achieve the effect just like mentioned in #484

If the proposal is adopted, the complex workaround can be easily achieved by using a varying to store the mid-data.

Is there a reason why this should be core and not an add-on in the asset library?

It needs to modify the compilation rule of shader language. I cannot image how it can be replaced by an add-on.

[hippogyz]

BTW, I have no idea why assigning a varying in light() is not allowed in current version.

The content of light() is always(?) put after fragment() in compiled glsl. So if a varying has already been assigned in fragment(), it should be allowed to be assigned in light()?

I tried to modify the code for test varying assign validation at https://github.com/godotengine/godot/blob/568589c9d8c763bfb3a4348174d53b42d7c59f21/servers/rendering/shader_language.cpp#L4629

bool ShaderLanguage::_validate_varying_assign(ShaderNode::Varying &p_varying, String *r_message) {
  if (current_function != "vertex" && current_function != "fragment") {
    // allow assigning in the 'light' function if varying has been assigned in the 'fragment' function
    if (!(current_function == "light" && p_varying.stage == ShaderNode::Varying::STAGE_FRAGMENT)) {
      *r_message = vformat(RTR("Varying may not be assigned in the '%s' function."), current_function);
      return false;
    }
  }
...

It works for my proposal.

The only problem is the light() function in gdshader must be put behind the 'fragment()'.

[AThousandShips]

The reason for this is explained here, and varyings are designed to pass information forward, so it wouldn't have anywhere to go?