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

FOG writing in shaders fails with Compatibility renderer unless enabled via WorldEnvironment #94183

Closed
Tracked by #66458
Zorochase opened this issue Jul 10, 2024 · 3 comments · Fixed by #94564
Closed
Tracked by #66458

Comments

@Zorochase
Copy link

Tested versions

4.2.2, 4.3.beta3

System information

Windows 11, Compatibility

Issue description

Writing to FOG in a shader works fine in Forward+ and Mobile, regardless of whether there is a WorldEnvironment node with fog enabled in the scene. This leads me to believe writing to FOG is intended to entirely override fog from any WorldEnvironment node. However, with the Compatibility renderer, writing to FOG only works if there is a WorldEnvironment node with fog enabled.

Steps to reproduce

Add a node, like a MeshInstance3D, to a scene. Add a ShaderMaterial to it and give that a shader which writes to FOG in the fragment function. If you're using the Forward+ or Mobile renderers, you should see the fog color of the node change. However, with the Compatibility renderer, you'll need to add a WorldEnvironment node, give it an Environment resource, and enable fog; only then will you see the fog color change to what you instructed it to in the shader.

Minimal reproduction project (MRP)

Fog Shader Compatibility Issue MRP.zip

@clayjohn
Copy link
Member

The problem here is we use a pre-processor macro DISABLE_FOG to tell the shader when to not render fog

if (render_data.environment.is_null() || (render_data.environment.is_valid() && !environment_get_fog_enabled(render_data.environment))) {
spec_constant_base_flags |= SceneShaderGLES3::DISABLE_FOG;
}

DISABLE_FOG should stop fog from being calculated, it shouldn't stop custom fog from being used.

What makes this complicated is that there is also a FOG_DISABLED macro which should disable all sources of fog Including custom fog. It seems that in the compatibility renderer we are using both DISABLE_FOG and FOG_DISABLED in places where on FOG_DISABLED should be used

We need to go through the shader and clean up places where DISABLE_FOG shouldn't be used:

#ifndef FOG_DISABLED
fog.xy = unpackHalf2x16(fog_rg);
fog.zw = unpackHalf2x16(fog_ba);
#ifndef DISABLE_FOG
if (scene_data.fog_enabled) {
frag_color.rgb = mix(frag_color.rgb, fog.rgb, fog.a);
}
#endif // !DISABLE_FOG
#endif // !FOG_DISABLED

@clayjohn clayjohn added this to the 4.x milestone Jul 19, 2024
@rothej
Copy link
Contributor

rothej commented Jul 20, 2024

Going to take a crack at this.

Edit: Writing my debug notes below since I'm new to contributing here. If any of the more experienced contributors want to tell me I am barking up the wrong tree, please do so.

Started trying to debug and ended up using grep -r and commenting out every instance of FOG_DISABLED, DISABLE_FOG, and USE_CUSTOM_FOG where it is used in an #ifndef block, the issue persisted.

Changed the shader script to:

shader_type spatial;

void fragment() {
    ALBEDO = vec3(0.0, 1.0, 0.0); 
}

and verified this was green across all three modes, so it appeared to not like fog.rgb in compatibility mode. (Is that a correct assessment?)

Using ALBEDO = vec3(FOG.r, FOG.g, FOG.b); turns the cube red so it looks like fog.rgb is being assigned correctly at the moment. So issue may be somewhere in the fog blending logic. (Is that also a correct assessment?)

Adding the WorldEnvironment node back in flipped it, where Compatibility is red but the other two don't work. Not sure at which point this started.

Commenting out the if (scene_data.fog_enabled) brackets in scene.glsl almost fixed the error, so I think I've potentially located where the issue is. Seeing a red cube in all three modes with no WorldEnvironment node, but when I add that node back in only Compatibility mode works as desired. Will do a fresh re-pull of the repo and try playing with that more to isolate the issue.

@rothej
Copy link
Contributor

rothej commented Jul 20, 2024

OK I've drilled it down further, current code works in all modes, with and without the world node, aside from these changes:

From scene.glsl, line 1923:

#ifndef FOG_DISABLED
	fog.xy = unpackHalf2x16(fog_rg);
	fog.zw = unpackHalf2x16(fog_ba);

//#ifndef DISABLE_FOG // debug - this breaks compatibility mode w no World node
	//if (scene_data.fog_enabled) { // debug - same issue as above 
	frag_color.rgb = mix(frag_color.rgb, fog.rgb, fog.a);
	//}
//#endif // !DISABLE_FOG
#endif // !FOG_DISABLED // Commenting out FOG_DISABLED and leaving the if statement in does not fix.

scene.glsl, line 2134:

#ifndef FOG_DISABLED
	fog.xy = unpackHalf2x16(fog_rg);
	fog.zw = unpackHalf2x16(fog_ba);

//#ifndef DISABLE_FOG // debug - uncommenting this makes compatibility mode have 3 red sides, 3 white sides
	//if (scene_data.fog_enabled) { // debug - same issue here
	additive_light_color *= (1.0 - fog.a);
	//}
//#endif // !DISABLE_FOG
#endif // !FOG_DISABLED // same issue if you uncomment scene_data but comment this. DISABLE_FOG remains commented out.

So #ifndef DISABLE_FOG needs to be removed from these two spots.

Additionally, there is no way it is letting me leave if(scene_data.fog_enabled) in. If I set bool fog_enabled = true; in scene.glsl just before each of these if statements, it breaks the shader in a different way (entire cube turns grey). I'm going to take that to mean that, in this case, fog_enabled should not be set high, but we still want to set frag_color and additive_light_color, so I will go ahead and remove the if statements.

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

Successfully merging a pull request may close this issue.

5 participants