Add optional depth fog to Environment

[HybridEidolon]

This is a feature addition that accomplishes godotengine/godot-proposals#4619, addresses the needs of godotengine/godot-proposals#3429, and supercedes the implementation in #55388 by partially restoring some of the depth fog parameters in the Godot 3.x Environment.

The following properties are added to the Environment resource:

• fog_depth_enabled: used in a specialization constant in the relevant scene shaders. This addresses the performance caveat discussed in Automatically fade to Z far in environment depth fog #55388 by ensuring that when only exponential fog is used, the branch is never present in the compiled program. It is not possible to use quadratic fog exclusively at the shader level in this implementation, but you can set the parameters of the environment such that the exponential fog density is 0 while still having quadratic fog if you must have no exponential fog at all. During project conversion, this property should be set to true if fog was enabled, the new fog density should be set to 0, and the sky affect scale should also be set to 0.
• fog_depth_curve: Same as in Godot 3.x
• fog_depth_density: replaces the alpha value of fog_depth_color in Godot 3.x. Project conversion should take the fog color's alpha and put it in this property instead.
• fog_depth_begin: Same as in Godot 3.x
• fog_depth_end: Same as in Godot 3.x

The depth fog interacts with the new fog model gracefully, supporting sun scattering and the improved light transmission. It does not affect the behavior of exponential fog with depth fog disabled at all.

Implementations are provided for gles3, vulkan mobile and vulkan clustered. Although, in the commit this branch is based on, vulkan mobile's support for fog is broken (there is already an MR to fix it) and gles3 seems to be very broken regardless.

Todo:

• Documentation entries for new Environment properties
• Changes to the project convertor to migrate the old fog properties

[mrjustaguy]

Do note that this implementation fails to hide Z-far effectively.

Here is a Plane (default camera settings, fog settings in pic)
See banding visible on the Left and Right of the Fog Circle in this Image which is actually not fully faded Plane:

The effect is clearer when inverting the Fog like here:

Do note that this is also the case in the above mentioned PR, and the issue seems to be due to aerial perspective.

[Calinou]

Do note that this is also the case in the above mentioned PR, and the issue seems to be due to aerial perspective.

Aerial perspective uses a blurred version of the background sky, not an exact copy of the background sky. It's meant to mimic fog light scattering.

[HybridEidolon]

After talking with a friend about the lost fog features in the 4.0 beta, there is one more 3.x fog property that isn't present: light transmittance (fog_transmit_enabled, fog_transmit_curve). Volumetric fog appears to support this, but not distance fog anymore.

From a cursory glance on how fog light transmittance is implemented in 3.x, it relies on the light information, but fog is evaluated before lighting in 4.0 rather than after it in 3.x; a comment notes that this is to maximize VGPR occupancy. I could perform the transmittance after lighting in a specialization constant branch to restore the property, but I am wary to add another specialization permutation to the standard scene shader too. Curious what y'all's thoughts are.

[clayjohn]

We discussed this in today's PR review meeting. The main idea has been approved for implementation but the consensus was to expose the feature a little bit differently. Instead of exposing both exponential fog and depth fog side by side they should be exposed as options toggleable in the shader with a specialization constant (so only one version is compiled).

In other words, we would add a FogType enum that can be used to select between exponential and depth-based fog. In the Environment you would only see the settings related to the type of fog you have selected. Then in the shader the specialization constant would be used to toggle between exponential fog and the depth fog so only one is enabled at a time and the user doesn't pay the cost for having both.

After talking with a friend about the lost fog features in the 4.0 beta, there is one more 3.x fog property that isn't present: light transmittance (fog_transmit_enabled, fog_transmit_curve). Volumetric fog appears to support this, but not distance fog anymore.

From a cursory glance on how fog light transmittance is implemented in 3.x, it relies on the light information, but fog is evaluated before lighting in 4.0 rather than after it in 3.x; a comment notes that this is to maximize VGPR occupancy. I could perform the transmittance after lighting in a specialization constant branch to restore the property, but I am wary to add another specialization permutation to the standard scene shader too. Curious what y'all's thoughts are.

fog_transmit was renamed tosun_scatter in 4.0. It works essentially the same way as it used to and is used to produce the same effect. So there is not need to add fog_transmit back in.

[ettiSurreal]

We discussed this in today's PR review meeting. The main idea has been approved for implementation but the consensus was to expose the feature a little bit differently. Instead of exposing both exponential fog and depth fog side by side they should be exposed as options toggleable in the shader with a specialization constant (so only one version is compiled).

In other words, we would add a FogType enum that can be used to select between exponential and depth-based fog. In the Environment you would only see the settings related to the type of fog you have selected. Then in the shader the specialization constant would be used to toggle between exponential fog and the depth fog so only one is enabled at a time and the user doesn't pay the cost for having both.

After talking with a friend about the lost fog features in the 4.0 beta, there is one more 3.x fog property that isn't present: light transmittance (fog_transmit_enabled, fog_transmit_curve). Volumetric fog appears to support this, but not distance fog anymore.
From a cursory glance on how fog light transmittance is implemented in 3.x, it relies on the light information, but fog is evaluated before lighting in 4.0 rather than after it in 3.x; a comment notes that this is to maximize VGPR occupancy. I could perform the transmittance after lighting in a specialization constant branch to restore the property, but I am wary to add another specialization permutation to the standard scene shader too. Curious what y'all's thoughts are.

fog_transmit was renamed tosun_scatter in 4.0. It works essentially the same way as it used to and is used to produce the same effect. So there is not need to add fog_transmit back in.

Sounds good, just make sure to try to implement sky affect from exponential fog to depth fog.

[clayjohn]

@HybridEidolon Do you think you will have a chance to update this in the next couple of weeks?

[Calinou]

Do you think you will have a chance to update this in the next couple of weeks?

@HybridEidolon Bump 🙂

[HybridEidolon]

Sorry for the long delay on this. I'm split between multiple projects at the moment. I do have a vested interested in finishing this out with the FogType enum request, but I don't think I'll really be able to work on it until after 4.0 is released.

[clayjohn]

Sorry for the long delay on this. I'm split between multiple projects at the moment. I do have a vested interested in finishing this out with the FogType enum request, but I don't think I'll really be able to work on it until after 4.0 is released.

No worries! Take your time

[Calinou]

@HybridEidolon Do you have time to rebase and update this pull request? If you don't, it's fine – let us know and someone else should be able to salvage this PR while keeping you as a co-author.

[HybridEidolon]

I do not. Anyone is free to take over if they'd like! The only major changes needed are the shader specialization to be either-or for the fog type and setting up the appropriate enum needed in the Environment.

[Uradamus]

Will there be a way to exclude meshes when using the depth fog, like was possible in 3.x by setting a mesh to unshaded? This is a very important feature for anyone using mesh based sky boxes/clouds, which were really common with old school map designs and has been the biggest problem some friends and I have run into with a porting project we've been working on for some old PS1 games when considering moving it over to 4.x.

[Calinou]

Will there be a way to exclude meshes when using the depth fog, like was possible in 3.x by setting a mesh to unshaded?

See #56374. Given how controversial this is in practice, I think this needs a dedicated render_mode rather than relying on shading mode being Unshaded. There's render_mode disable_fog in sky shaders, but not in spatial shaders.

[st4rdog]

Here's use-case example. Lack of basic "linear fog" with start/end fade is currently stopping this game being ported from Unity. It's a basic feature you can even see in Starfield's interiors. Default Godot 4.1 fog overlaps far too close to the camera.

[Calinou]

Here's use-case example. Lack of basic "linear fog" with start/end fade is currently stopping this game being ported from Unity. It's a basic feature you can even see in Starfield's interiors. Default Godot 4.1 fog overlaps far too close to the camera.

We are well-aware of example use cases now, but we still need someone to salvage this pull request 🙂

[scriptsengineer]

I tested a simple merge with adjusting values in the conflict I had, everything came out fine here.

my branch: https://github.com/expressobits/godot/tree/distance-fog

However, these values were added to main

uint32_t camera_visible_layers;
uint32_t pad1;
uint32_t pad2;
uint32_t pad3;

And these to distance_fog

float fog_sun_scatter;
float pad1;
float pad3;
float pad4;

Which makes me think that the editor values might be editing something already used elsewhere in the shader, I don't know exactly what the rule is for these pad* properties

[Calinou]

Which makes me think that the editor values might be editing something already used elsewhere in the shader, I don't know exactly what the rule is for these pad* properties

You only have to keep pad1 and pad2 in the shader – you don't need 6 pad values. Alignment needs to be performed over 16-byte regions. A uint32_t or float is 4 bytes.

[clayjohn]

Implementation note for whoever takes over this PR. The last remaining details are:

1. Rebase on top of master and resolve conflicts
2. Change the API a bit to add a "fog mode" instead of "depth enabled" Then use either the depth mode or the physically based fog based on that setting, don't use both types of fog at once.

[Calinou]

@scriptsengineer Are you available to perform the changes requested above?

[scriptsengineer]

@scriptsengineer Are you available to perform the changes requested above?

Yes, I intend to take it easy this week.

[scriptsengineer]

1. Rebase on top of master and resolve conflicts

I resolved the conflict with master again, updated with the latest version of master
https://github.com/expressobits/godot/tree/distance-fog
Still trying to study ways to remove the excessive pads.

2. Change the API a bit to add a "fog mode" instead of "depth enabled" Then use either the depth mode or the physically based fog based on that setting, don't use both types of fog at once.

I intend to study more about how to create the option, but my understanding of the source code is still small, I want to leave this open in case anyone wants to take it.

[scriptsengineer]

Update:
✅Cleaned unnecessary pads
🚧I put an fog mode option with an enum, the only thing left to do is connect it with shaders.

[scriptsengineer]

@clayjohn Is there a justification for a user not to use both fogs together? It seems to me that the fog by distance is classified as the fog by height works, something that is added to the current fog.

[clayjohn]

@clayjohn Is there a justification for a user not to use both fogs together? It seems to me that the fog by distance is classified as the fog by height works, something that is added to the current fog.

A mix of performance and simplicity in the editor. Exposing both together is kind of messy, bad for performance, and so far no one has suggested that they need it for their project.

[st4rdog]

Depth Begin should ideally support negative numbers, to simulate being in the fog, like Exponential. Without that, someone might need both enabled.

[scriptsengineer]

Finished 🚀

✅ Merged successfully

✅ Removed unnecessary pads

✅ Added enum option in the environment inspector

✅ Added conditional in shader to execute correct fog mode
I was wondering if there is a better way to define this if
scene.glsl:

#ifndef DISABLE_FOG_DEPTH
	if (scene_data.fog_mode == 1) {
		float fog_far = scene_data.fog_depth_end > 0.0 ? scene_data.fog_depth_end : scene_data.z_far;
		float fog_z = smoothstep(scene_data.fog_depth_begin, fog_far, length(vertex));

		float fog_quad_amount = pow(fog_z, scene_data.fog_depth_curve) * scene_data.fog_density;
		fog_amount = fog_quad_amount;
	}
#else
	if (scene_data.fog_mode == 0) {
		fog_amount = 1 - exp(min(0.0, -length(vertex) * scene_data.fog_density));
	}

✅ Possible to add negative values in fog begin

godot.windows.editor.x86_64_cGJmj07bao.mp4

[jcostello]

Looking really nice. Does height works with depth?

[scriptsengineer]

Looking really nice. Does height works with depth?

Yes, it works, in reality any attribute is basically a sum

@clayjohn I wonder if this height attribute should be in depth?

[sketchyfun]

Been looking forward to getting the old fog back for a while :)

Does a new pull request need to be opened so it can be reviewed and merged? (if it's done that is)

[scriptsengineer]

Been looking forward to getting the old fog back for a while :)

Does a new pull request need to be opened so it can be reviewed and merged? (if it's done that is)

I'm also waiting for a review or response if I should open a new PR

[HybridEidolon]

I'm also waiting for a review or response if I should open a new PR

Go ahead and make a new PR; I won't be able to get back to this until probably early next year for the 4.3 dev cycle

[scriptsengineer]

Também estou aguardando uma análise ou resposta se devo abrir um novo PR

Vá em frente e faça um novo PR; Não poderei voltar a isso até provavelmente no início do próximo ano para o ciclo de desenvolvimento 4.3

Wouldn't it be better to just change the current pr configuration? it doesn't seem good to duplicate the pr

[sketchyfun]

Também estou aguardando uma análise ou resposta se devo abrir um novo PR

Vá em frente e faça um novo PR; Não poderei voltar a isso até provavelmente no início do próximo ano para o ciclo de desenvolvimento 4.3

Wouldn't it be better to just change the current pr configuration? it doesn't seem good to duplicate the pr

I could be wrong, but I don't think you can change the remote repository of a pull request, so you'll need to create a new one that points to your repository and mentions that you've taken over finishing this PR in the comment

[Calinou]

Indeed, you need to open a new PR (and link to this PR in its description).

[Calinou]

Superseded by #84792. Thanks for the contribution nonetheless!