godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.04k stars 21.18k forks source link

Regression: Simple shader was working fine in 4.2.1, broken in 4.3 dev 1 #86425

Closed RebelliousX closed 10 months ago

RebelliousX commented 10 months ago

Tested versions

System information

Windows 11, Godot 4.3 dev 1, RTX 3070, i9 9900KF

Issue description

A simple shader like this to re-color non-transparent pixels will fail in 4.3 dev 1 using the Forward+ renderer. It works in Compatibility renderer. In Godot 4.2.1, it works fine using the Forward+ renderer.

shader_type canvas_item;

uniform vec4 dashed_color : source_color = vec4(0.0, 1.0, 1.0, 1.0);

void fragment() 
{
    vec4 pixel_color = texture(TEXTURE, UV);
    pixel_color = vec4(mix(pixel_color.rgb, dashed_color.rgb, dashed_color.a), pixel_color.a);
    COLOR = pixel_color;
}

Steps to reproduce

Add this shader to a textured sprite with some transparency. Try to change the parameter of the dashed color. Nothing happens.

Minimal reproduction project (MRP)

Shader 4.2.1.zip <- Working Shader 4.3 Dev 1.zip <- ( Forward+ Not working, Compatibility Renderer is working)

Worth mentioning error I got when I started the new minimal project. I Which I didn't get when I upgraded my main project from 4.21 -> 4.3 Dev 1. And it is the official 4.3 Dev 1 build, I didn't build it myself.

Warning

This video shows the regression I am talking about.

https://github.com/godotengine/godot/assets/1888186/f95abb44-381b-4f24-8105-80a7b444eb52

YuriSizov commented 10 months ago

I cannot reproduce it. Is this not what you expect to happen?

https://github.com/godotengine/godot/assets/11782833/90adbf44-f3cf-4718-9213-5150131e15a7

RebelliousX commented 10 months ago

@YuriSizov No, it is not supposed to be like that. Here is a video that shows the difference between 4.2.1 and 4.3 Dev 1. ( I had to compress the video to 720p to make it under 10MB limit for Github, sorry for the bad quality).

https://github.com/godotengine/godot/assets/1888186/f95abb44-381b-4f24-8105-80a7b444eb52

LimestaX commented 10 months ago

Tested this, you have to make the material unique. They will still share the same shader, but can independently change values, Did you confirm when upgrading from 4.2.1 -> 4.3 a specific project that the "unique" stays persistent? Or is it lost when upgrading? Currently seems intended.

RebelliousX commented 10 months ago

@LimestaX In the video above, they are 2 different file names. They are unique. In my main project I duplicate() the shader material through code. It has been working fine since Godot 3.

It seems you didn't see the video I posted above.

LimestaX commented 10 months ago

@RebelliousX If you could provide an MRP, I can test it out, the video wasn't conveying that information, and I haven't been able to replicate it on my end.

RebelliousX commented 10 months ago

@LimestaX @YuriSizov I updated the first post with 2 different projects, one for 4.2.1 (working), and another for 4.3 Dev (not working).

Lippanon commented 10 months ago

I can reproduce this in v4.3.dev1.official [9d1cbab1c]. If you hide the Background Sprite2D, then the color appears correct on the Foreground one. I don't see any shared UUIDs and deleting the .godot folder doesn't solve it either. Introducing this line: uniform float test = 1.0; In the Foreground.tres shader fixes it. I think when the shader code is the same, some regression has made it so subsequent shaders get overwritten. If you change the order of the nodes in the tree (Foreground as first child), the color will swap and both sprites will use the Foreground color.

@RebelliousX You can try bisecting the regression to help speed up the fixing process.

Followup note: The simplest shader code is enough to demonstrate the problem:

shader_type canvas_item;

uniform vec4 a_color : source_color = vec4(0.0, 1.0, 1.0, 1.0);

void fragment() {
    COLOR = a_color;
}

It also seems to only affect nodes that are in the same scene, and I can't reproduce it with QuadMesh in 3D so it might be canvas_item only (tested TextureRect also affected).

bitsawer commented 10 months ago

I can also repro this on current master at 13a0d6e9b253654f5cc2a44f3d0b3cae10440443 on Forward+ renderer, works on Compatibility. Adding any uniform like Lippanon suggested also makes the bug go away.

Bisecting this would be very useful if anyone has the time, changelog for 4.3 dev1 is here: https://godotengine.github.io/godot-interactive-changelog/#4.3-dev1.