Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accessing COLOR as input variable breaks shader unintuitively #5545

Open
SyntaxPartnership opened this issue Nov 13, 2018 · 10 comments
Open
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation enhancement topic:shaders
Milestone

Comments

@SyntaxPartnership
Copy link

Godot version:
3.1 Alpha 2

OS/device including version:
Windows 7 Professional Service Pack 1

Issue description:
Shader turns entire sprite white.

Tried both of the following shader coding:

shader_type canvas_item;

//Set target colors.
uniform vec4 target_color_1 : hint_color;
uniform vec4 target_color_2 : hint_color;
uniform vec4 target_color_3 : hint_color;

//Set replacement colors.
uniform vec4 replace_color_1 : hint_color;
uniform vec4 replace_color_2 : hint_color;
uniform vec4 replace_color_3 : hint_color;

void fragment() {
    if (COLOR == target_color_1)
    {
        COLOR = replace_color_1
    }
    if (COLOR == target_color_2)
    {
        COLOR = replace_color_2
    }
    if (COLOR == target_color_3)
    {
        COLOR = replace_color_3
    }
}

And:

shader_type canvas_item;
uniform float threshold : hint_range(0,1) = 0.01;
uniform vec4 T0:hint_color = vec4(0.058823, 0.058823, 0.058823, 1);
uniform vec4 T1:hint_color = vec4(0.254902, 0.254902, 0.254902, 1);
uniform vec4 T2:hint_color = vec4(0.45098, 0.45098, 0.45098, 1);


uniform vec4 R0 : hint_color;
uniform vec4 R1 : hint_color;
uniform vec4 R2 : hint_color;


void fragment() {
	vec3 c = COLOR.rgb;
	
	if (abs(T0.r-c.r) <= threshold && abs(T0.g-c.g) <= threshold && abs(T0.b-c.b) <= threshold) {COLOR.rgb = R0.rgb;}
	if (abs(T1.r-c.r) <= threshold && abs(T1.g-c.g) <= threshold && abs(T1.b-c.b) <= threshold) {COLOR.rgb = R1.rgb;}
	if (abs(T2.r-c.r) <= threshold && abs(T2.g-c.g) <= threshold && abs(T2.b-c.b) <= threshold) {COLOR.rgb = R2.rgb;}

}

test2
(Ignore the green pixel.)

Steps to reproduce:
Run app.

@DavidSichma
Copy link

COLOR is intended to be only primary used as an output. Related godotengine/godot#23179.

Try vec4 col = texture(TEXTURE, UV);.

EDIT: COLOR is actually the output of the vertex shader. Unless you use vertex colors this will be white.

@akien-mga akien-mga changed the title GLES2 Shader Bug Accessing COLOR as input variable breaks shader unintuitively Nov 13, 2018
@akien-mga
Copy link
Member

This is indeed expected as mentioned by @DavidSichma. Yet many users seem to be hit by this, so we should see if we can improve the UX, e.g. by getting a compilation error when trying to read COLOR before it has been assigned.

@clayjohn
Copy link
Member

@akien-mga In the fragment shader COLOR is both an input and output. It is the vertex color output from the vertex shader into the fragment shader and it is the output color from the fragment shader. So it would not make sense to throw a compilation error when it is used. I think the best thing to do is just document COLOR better and explain that it does not automatically read from the texture.

@akien-mga
Copy link
Member

CC @Chaosus

@Chaosus
Copy link
Member

Chaosus commented Dec 13, 2019

By default, vertex color is always white so comparison like if (COLOR == target_color_1) doesn't make much sense. More commonly, If you need to replace a texture pixel color in a fragment shader you should use texture(sampler, UV) instead COLOR...

@Chaosus
Copy link
Member

Chaosus commented Dec 13, 2019

@akien-mga I don't understand what unintuitively and wrong in this behavior. Do you want me to create a PR to warn users about this? Of course, I could :)

@clayjohn
Copy link
Member

As I said above, I don't think there should be an editor warning. I think this is just a documentation issue.

@Chaosus
Copy link
Member

Chaosus commented Dec 13, 2019

Phew... Then I think so too. It's rather hard to implement such warn system anyways 😄

@akien-mga akien-mga transferred this issue from godotengine/godot Jan 25, 2022
@akien-mga
Copy link
Member

Transferred to godot-docs as this seems to be a documentation issue.

From a quick look at https://docs.godotengine.org/en/stable/tutorials/shading/shading_reference/canvas_item_shader.html#vertex-built-ins, I'd say that it might still not be crystal clear that one can't read from COLOR in the vertex function (it's specified as inout, without particular caveat).

@skyace65 skyace65 added the area:manual Issues and PRs related to the Manual/Tutorials section of the documentation label Dec 28, 2022
@tetrapod00
Copy link
Contributor

Is this still relevant? I think after #10107 this has been thoroughly clarified in 4.x/master (and the behavior is different in 4.x, anyway). Does this issue still need to exist for a potential docs change to 3.x?

@tetrapod00 tetrapod00 added this to the 3.x milestone Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation enhancement topic:shaders
Projects
None yet
Development

No branches or pull requests

7 participants