Recent regression in TPS demo

[lawnjelly]

Godot version:
Latest 3.x 365ab88
Worked in 3.3 stable
Works in 3.3.1 (so maybe not cherry picked)
Bisected to @lyuma commits on May 14th 2021.

OS/device including version:
Linux Mint 20, Intel HD Graphics 630

Issue description:

Open scene red_robot.tscn from TPS demo. Laser was showing when it should be hidden.

Broken (shows laser)

Working (laser not showing)

Just bisecting now, I thought it was due to BVH, but it seems to work after @pouleyKetchoupp 's May 4th PR so I'm not yet sure what is causing it.

[lawnjelly]

Looks like it is the @Chaosus / @lyuma commits on May 14th. Ah .. they are all part of #48075.

[Chaosus]

 shader_type spatial;
2 render_mode cull_disabled;
3 
4 uniform sampler2D laser_body_noise;
5 uniform vec2 uv_scale = vec2(5.0, 1.0);
6 uniform float offset_uv_x = 0.0;
7 uniform float cutoff_value : hint_range(0.0, 1.0) = 0.6;
8 uniform float traversal_scale = 1.0;
9 uniform float energy = 1.0;
10 
11 uniform float clip = 4.0;
12 
13 uniform vec4 color1 : hint_color = vec4(0.0, 0.0, 1.0, 1.0);
14 
15 // Z coordinate of the vertex, in local space.
16 varying float z_local_coordinates;
17 
18 void vertex(){
19 	// Because of how the UVs are laid out, the UVs at the right-most place
20 	// are at the end of the beam. We use that to decide wether or not to
21 	// apply the clip distance to the laser.
22 	float should_displace_vertex = step(0.9, UV.x);
23 	// Here we mix between the vertex original position and the clip position
24 	// because we use step, only the ring of vertices at the end will have a
25 	// coefficient of 1.
26 	float original_z = VERTEX.z;
27 	VERTEX.z = mix(VERTEX.z, -clip, should_displace_vertex);
28 	UV.x *= VERTEX.z / original_z;
29 	z_local_coordinates = VERTEX.z;
30 	VERTEX.xy *= traversal_scale;
31 }
32 
33 void fragment() {
34 	vec2 transformed_uv = UV * uv_scale;
35 	transformed_uv.x += offset_uv_x;
36 	float noise_val = texture(laser_body_noise, transformed_uv).r; //noise textures are grayscale
37 	ALBEDO = color1.rgb;
38 	if (noise_val < cutoff_value) {
39 		discard;
40 	}
41 	EMISSION = color1.rgb * energy;
42 }
43 

:16 - Varying must only be used in two different stages, which can be 'vertex' 'fragment' and 'light'

Seems like an error related to the new varying system...

[Chaosus]

It's because z_local_coordinates is used only in vertex but not in fragment - but the new default behavior is to mandatory use it if it's assigned in the vertex function.

I guess removing varying float z_local_coordinates; is enough to fix this issue (this varying have not used (only assigned) anyway)

[lawnjelly]

Ah fantastic, so a bit of a wild goose chase, down to exposing an error in the shader. But is good to have it fixed in the demo. 😄 👍

[lyuma]

This does seem like a regression in 3.x, caused by that unnecessary check added in the fragment-to-light varying code.

My opinion is that the "Varying must only be used in two different stages" check could be removed and it will act like it used to in this regard.

[Chaosus]

This check is originally suggested by @reduz so I've implemented it, but for 3.x this could be disabled to preserve the compatibility, I agreed.