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

Compatibility renderer breaks shader #91919

Open
Zorochase opened this issue May 13, 2024 · 4 comments · May be fixed by #92388
Open

Compatibility renderer breaks shader #91919

Zorochase opened this issue May 13, 2024 · 4 comments · May be fixed by #92388

Comments

@Zorochase
Copy link

Tested versions

4.1.4.stable, 4.2.2.stable, 4.3.dev6

System information

Windows 11, Compatibility

Issue description

I'm trying to port this shader over to Godot 4. I’ve got it working just fine with Forward+, but the shadow color breaks in Compatibility.

Here’s a comparison. The left side was rendered with Compatibility. It should look the same as the gem on the right, which was rendered with Forward+.
Comparison

I've asked on Godot Forum and a user suggested I try different Godot versions. I tried 4.1.4 and 4.3 Dev 6; neither fixed the issue. They also pointed out I shouldn't need to change anything about the code itself; if it works with Forward+, it should work with Compatibility. Is this true?

The provided MRP contains the shader code and the example gem in the image above. I tried exporting the gem as a glb file and an obj file; this made no difference either (though I doubted it would, I had to try anyway).

Steps to reproduce

Download the MRP, open scene.tscn and switch between Compatibility and Forward+ or even Mobile. Compatibility is the only renderer which breaks the shader in my testing.

Minimal reproduction project (MRP)

Compatibility Shader Issue MRP.zip

@Zorochase
Copy link
Author

roc4u1000 on Godot Forum suggested this change to the code:

ALBEDO =
    clamp(albedo_color.rgb +
    brightness +
    darkness * vec3(-shadow_intensity), .0, 1.);

which fixed the issue with the shader, but we're still curious as to why this fixed it. It might still be an issue with Godot, so I'm leaving this open for now.

@clayjohn
Copy link
Member

This comes down to a difference in how negative values are handled by vulkan and opengl given the respective framebuffer formats. Oh, and also the GPU drivers.

In OpenGL we write to a buffer that is explicitly in the 0-1 range. Some drivers will clamp negative values while others wraparound negative values.

We should probably just always clamp out the values to the 0-1 before writing them in the internal shader to avoid differences in drivers

@clayjohn clayjohn added this to the 4.x milestone May 17, 2024
sunfl0w added a commit to sunfl0w/godot that referenced this issue May 26, 2024
Using the Compatibility renderer results in wrapped albedo values if a custom shader returns albedo values outside the expected range of [0,1].

This commit fixed this issue by clamping the albedo value to [0,1] right after the custom shader is executed.

Fixes godotengine#91919
@sunfl0w
Copy link

sunfl0w commented May 26, 2024

Hello. I just looked into this issue and added a line of code clamping the albedo value to [0,1] in the shader used in the Compatibility render mode. The clamping is performed right after the execution of the user supplied fragment shader code. This small addition fixes the problem on my end, and the test scene now looks comparable in all render modes.

sunfl0w added a commit to sunfl0w/godot that referenced this issue May 26, 2024
Using the Compatibility renderer results in wrapped albedo values if a custom shader returns albedo values outside the expected range of [0,1].

This commit fixed this issue by clamping the albedo value to [0,1] right after the custom shader is executed.

Fixes godotengine#91919
@daustria
Copy link
Contributor

In OpenGL we write to a buffer that is explicitly in the 0-1 range. Some drivers will clamp negative values while others wraparound negative values.

can you expand on this, for my own understanding of whats happening? i am not sure what buffer we are talking about and what specifies it should be in the 0-1 range. maybe we are talking about the values stored in the albedo variable in the scene.glsl shader?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Up for grabs
Development

Successfully merging a pull request may close this issue.

6 participants