Environment "Glow" should be put before the "Tonemap"

[EaryChow]

Tested versions

Tested on a Godot 4.4 nightly build I downloaded yesterday (Jan 12 2025)

System information

Godot v4.4.dev (5b52b4b) - Windows 11 (build 22631) - Multi-window, 1 monitor - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3050 Laptop GPU (NVIDIA; 32.0.15.6109) - 11th Gen Intel(R) Core(TM) i5-11400H @ 2.70GHz (12 threads)

Issue description

I saw the AgX PR(#87260) has been merged, so I decided to download Godot for the first time and take a look. I downloaded this demo scene: https://github.com/perfoon/Abandoned-Spaceship-Godot-Demo

Here is what I see:

It looks to me that the "Glow" effect is being put after the Image Formation (or view transform or tonemap), while it should be put to before. A rule of thumb is that any processing that can generate Open Domain (higher than 1.0) values need to be put to pre-formation (or pre-tonemapping), this is why the entire Blender compositor is before the view transform. Godot is not putting the Image formation/view transform/tone mapping at the end of these processings, which causes this problem. I feel the need to report this. You might also want to check whether there are other similar issues with other settings apart from fixing this.

Steps to reproduce

1. Open this demo project: https://github.com/perfoon/Abandoned-Spaceship-Godot-Demo
2. Select the WorldEnvironment, in the inspector, change the "Tonemap" setting to AgX
3. See how a clipping issue that shouldn't be there when AgX is enabled remains
4. Disable the "Glow", see how the issue disappears.

Minimal reproduction project (MRP)

N/A

[clayjohn]

For further detail, we handle Glow in a very weird way. We tonemap the input color, then we tonemap the glow result, then we add them together in sRGB space

godot/servers/rendering/renderer_rd/shaders/effects/tonemap.glsl

Lines 575 to 580 in d19147e

	glow = apply_tonemapping(glow, params.white);
	if (bool(params.flags & FLAG_CONVERT_TO_SRGB)) {
	glow = linear_to_srgb(glow);
	}
	color.rgb = apply_glow(color.rgb, glow);

This means that additive glow ends up with all the negative effects of adding light in sRGB space. Using any of the other glow modes would look better.

[Calinou]

I recall there was a reason why we perform glow after tonemapping, but I don't remember why exactly. It might have to do with MSAA/FXAA quality.

[Calinou]

I've attempted this: https://github.com/Calinou/godot/tree/glow-perform-before-tonemapping

When using AgX, it looks nice with the Additive glow mode, but using Screen or Softlight means glowing pixels will never be able to be fully white:

Additive	Screen	Softlight

For reference, Replace looks like this:

Additionally, the Screen mode will sometimes look strange, creating an unexpected shift towards green in this scene:

glow_screen_green.mp4

Here are videos before/after the change (first one is master, second one is with this change):

old_glow_after_tonemap.mp4

new_glow_before_tonemap.mp4

In Compatibility, glow already seems to be applied before tonemapping, so my branch affects Forward+ and Mobile only. FXAA and MSAA still seem to work correctly so far, along with TAA and FSR2.

[beicause]

This seems to be the reason why glow is applied in the current way #19361 (comment)

Honestly, I don't see the benefits of the current weird approach.

Just for reference, Unity documents bloom should be applied before tonemapping: https://docs.unity3d.com/530/Documentation/Manual/script-Tonemapping.html#:~:text=Make%20sure%20that%20Bloom%20should,Depth%20of%20Field%20image%20effect

And Bevy also applys bloom before tonemapping: bevyengine/bevy#6655 (comment)

[beicause]

I've attempted this: https://github.com/Calinou/godot/tree/glow-perform-before-tonemapping
When using AgX, it looks nice with the Additive glow mode, but using Screen or Softlight means glowing pixels will never be able to be fully white:

@Calinou Note that Screen or Softlight mode clamps glow to [0, 1]. Could this cause the glow to be too weak when mixed with color?

godot/servers/rendering/renderer_rd/shaders/effects/tonemap.glsl

Lines 403 to 422 in b89c47b

	vec3 apply_glow(vec3 color, vec3 glow) { // apply glow using the selected blending mode
	if (params.glow_mode == GLOW_MODE_ADD) {
	return color + glow;
	} else if (params.glow_mode == GLOW_MODE_SCREEN) {
	// Needs color clamping.
	glow.rgb = clamp(glow.rgb, vec3(0.0f), vec3(1.0f));
	return max((color + glow) - (color * glow), vec3(0.0));
	} else if (params.glow_mode == GLOW_MODE_SOFTLIGHT) {
	// Needs color clamping.
	glow.rgb = clamp(glow.rgb, vec3(0.0f), vec3(1.0f));
	glow = glow * vec3(0.5f) + vec3(0.5f);
	color.r = (glow.r <= 0.5f) ? (color.r - (1.0f - 2.0f * glow.r) * color.r * (1.0f - color.r)) : (((glow.r > 0.5f) && (color.r <= 0.25f)) ? (color.r + (2.0f * glow.r - 1.0f) * (4.0f * color.r * (4.0f * color.r + 1.0f) * (color.r - 1.0f) + 7.0f * color.r)) : (color.r + (2.0f * glow.r - 1.0f) * (sqrt(color.r) - color.r)));
	color.g = (glow.g <= 0.5f) ? (color.g - (1.0f - 2.0f * glow.g) * color.g * (1.0f - color.g)) : (((glow.g > 0.5f) && (color.g <= 0.25f)) ? (color.g + (2.0f * glow.g - 1.0f) * (4.0f * color.g * (4.0f * color.g + 1.0f) * (color.g - 1.0f) + 7.0f * color.g)) : (color.g + (2.0f * glow.g - 1.0f) * (sqrt(color.g) - color.g)));
	color.b = (glow.b <= 0.5f) ? (color.b - (1.0f - 2.0f * glow.b) * color.b * (1.0f - color.b)) : (((glow.b > 0.5f) && (color.b <= 0.25f)) ? (color.b + (2.0f * glow.b - 1.0f) * (4.0f * color.b * (4.0f * color.b + 1.0f) * (color.b - 1.0f) + 7.0f * color.b)) : (color.b + (2.0f * glow.b - 1.0f) * (sqrt(color.b) - color.b)));
	return color;
	} else { //replace
	return glow;
	}
	}

Edit: Screen and Softlight doesn't seem to support HDR colors. It might be a reason why glow is applied after tonemapping.