Remove SCREEN_TEXTURE, DEPTH_TEXTURE, and NORMAL_ROUGHNESS_TEXTURE

[clayjohn]

Follow up to https://twitter.com/reduzio/status/1609687033450229760

Reduz and I have discussed this at great length. I was in favour of keeping SCREEN_TEXTURE, DEPTH_TEXTURE, and NORMAL_ROUGHNESS_TEXTURE as an easy-to-use pathway for screen-space sampling. Reduz preferred to remove them as the new pathway is not complicated and can be much better for performance.

This PR removes SCREEN_TEXTURE, DEPTH_TEXTURE, and NORMAL_ROUGHNESS_TEXTURE, and exposes the new hint to visual shaders (no changes were needed to text shaders). This PR also cleans up a few bugs related to using these textures.

Now users will always have to use the hint syntax so this:

shader_type spatial

void fragment() {
    ALBEDO = texture(SCREEN_TEXTURE, SCREEN_UV).rgb;
}

becomes this

shader_type spatial

uniform sampler2D screen_texture : hint_screen_texture, repeat_disable, filter_linear_mipmap;

void fragment() {
    ALBEDO = texture(screen_texture, SCREEN_UV).rgb;
}

Or for the others:

uniform sampler2D depth_texture : hint_depth_texture;
uniform sampler2D normal_roughness_texture : hint_normal_roughness_texture;

The repeat mode and filter setting can be whatever you want.

CC @reduz

[reduz]

I think we are good without them. If users complain and make a good case we can add them back, but the new syntax to do this should still be easy enough.

[clayjohn]

@reduz pushed requested changes and added doc updates for CI

[clayjohn]

This PR will conflict with #71455, so whichever one is merged first the other will need second will need a rebase.

Also, we need to explicitly mention the removal of SCREEN_TEXTURE, DEPTH_TEXTURE, and NORMAL_ROUGHNESS_TEXTURE in the next Beta release post following this PR

[BastiaanOlij]

Indeed, we're going to have to make sure we keep the functionality for making this work in stereo and switching the sampler type.

I have to admit I'm worried about the impact of this especially so close to 4.0s release, but at the same time this also isn't a change we want to introduce halfway through a major release cycle.

I wonder if an intermediate solution would be to at least have a proper warning generated if someone is still using the old syntax, if it just gives a bland syntax error it's going to throw off a lot of people converting their Godot 3 projects or with existing Godot 4 projects if they don't read the release notes carefully.

[clayjohn]

@BastiaanOlij Updated to do 2 more things:

1. Remove the dependencies on the old names internally. Now we only check the uniform hint to see its type. This made it easier to avoid errors from things like uniform sampler2D SCREEN_TEXTURE : hint_depth_texture;
2. Add a nice in-editor warning message when users rely on the old names (unfortunately, it gets a little bit cut off, but I think more text is better than less here)

[BastiaanOlij]

@clayjohn I LOVE IT!

[akien-mga]

Looks great!

unfortunately, it gets a little bit cut off, but I think more text is better than less here

I believe we should enable line wrapping and either let the error panel span more than two lines, or add a vertical scrollbar when needed. But that can be done in a separate PR.

[akien-mga]

Thanks!

[mournguard]

So this seems to break everything that used it which is fine except that it also includes the StandardMaterial (eg proximity fade is broken)

Edit; Come to think of it I guess things like the proximity fade node in visual shaders might be broken as well.

[pixezy]

SCREEN_TEXTURE just doesn't work. Am I doing something wrong?
uniform sampler2D SCREEN_TEXTURE : hint_screen_texture, repeat_disable, filter_linear_mipmap;

[JohnPandich]

SCREEN_TEXTURE just doesn't work. Am I doing something wrong? uniform sampler2D SCREEN_TEXTURE : hint_screen_texture, repeat_disable, filter_linear_mipmap;

I did some testing and the issue seems to be with the "hint_screen_texture" hint. Using this results in the shader not giving any output. (v4.0.beta14.official)

[jonSP12]

Is there a way to solve this error ?
something to do with the PI ?

the word PI is always marked in blue...

Also the way of getting the shader paramenter is always returning NULL
var shdBlur = $CanvasLayer/ColorRect.material.get("shader_param/amount");

[akien-mga]

The error tells you that you are redefining PI, which means that PI was already defined. It's a built-in constant.

[jonSP12]

The error tells you that you are redefining PI, which means that PI was already defined. It's a built-in constant.

thanks, changed it to "const float PII = 3.14159265359;"
btw do you know the new script to 'get' and 'set' a shader parameter in gdScript ?

something like:
var aa = get_shader_paramenter("amount");

if ( key_space ):
set_shader_paramenter aa + 1;

it should be an easy one, but cant find in the manual

[akien-mga]

thanks, changed it to "const float PII = 3.14159265359;"

My point was that you can simply remove this line... PI is already defined so you can simply use it.

For the rest, please use other community platforms (e.g. Discord or the Q&A) to ask support questions, this is a merged PR and the discussion should only be about this PR.

[elvisish]

I think we are good without them. If users complain and make a good case we can add them back, but the new syntax to do this should still be easy enough.

I think it makes it awkward to use shaders that were written before GD4, it might even be more convenient if the default shader template contained the hint as so many shaders use it and screen_texture is one of the most important uses of a shader.