Reflector (WebGLRenderTarget): Fog Color problem in sRGB outputEncoding

[takahirox]

From Hubs-Foundation/hubs#5572

Describe the bug

Fog color in Reflector (WebGLRenderTarget) can mismatch the one in the main scene.

I made an online example to show the problem.

https://raw.githack.com/takahirox/three.js/MirrorFogExample/examples/webgl_mirror_fog.html

Screenshots

sRGB output encoding + fog. Fog color in the reflector looks brighter than the one in the main scene. (See the green wall.)

sRGB output encoding + no fog. Color in the reflector looks the same as the one in the main scene.

Linear output encoding + fog. Fog color in the reflector looks the same as the one in the main scene.

Live example

https://raw.githack.com/takahirox/three.js/MirrorFogExample/examples/webgl_mirror_fog.html

Expected behavior

Fog color in the reflector looks same as the one in the main scene even in sRGB outputEncoding.

Platform:

• Device: Desktop
• OS: Windows 10
• Browser: Chrome
• Three.js version: dev

[takahirox]

Probably the root issue is color space conversion. In short, the fog color in the main scene doesn't seem to be applied color space conversion when rendering to the screen.

In the example, first rendering the scene including the fog to the reflector render target, and then rendering the scene including the reflector render target texture to the screen.

Three.js does color space conversion in shader (<encodings_fragment>) when rendering to the screen.

https://github.com/mrdoob/three.js/blob/b90f614fd59e1983d59dc24c1aa05c9c2c3886cd/src/renderers/shaders/ShaderChunk/encodings_fragment.glsl.js

When rendering to the screen the fog color in the reflector render target texture set to material.map is converted in shader. But please see the shader codes in ShaderLib directory. It first applys <encodings_fragment> and then <fog_fragment>. So, the fog color in the main scene is not converted.

three.js/src/renderers/shaders/ShaderLib/meshphysical.glsl.js

Lines 210 to 211 in 12f550e

	#include <encodings_fragment>
	#include <fog_fragment>

Therefore, the fog colors in the reflector and in the main scene can mismatch.

An easy solution may be swap the order of <encodings_fragment> and <fog_fragment>. Is there any reasons why applying fog after the color space conversion?

This is the screenshot of sRGB output encoding + fog + swap the order. The fog color in the reflecor looks the same as the one in the main scene.

[WestLangley]

Is there any reasons why applying fog after the color space conversion?

I believe this explains the history, although at the moment, I am unable to locate the original discussion on the topic.

[LeviPesin]

Related: #23937

[takahirox]

Thanks @WestLangley @LeviPesin for pointing out the related threads.

[donmccurdy]

I expect there will be similar challenges with tone mapping and fog, too.

[LeviPesin]

Is this still an issue after #23937?

[takahirox]

Unfortunately the problem doesn't seem to be resolved yet

[donmccurdy]

Note that #23937 will require setting ColorManagement.legacyMode = false to have any effect. But I think there are other things going on here, this is tricky. 😕

[takahirox]

ColorManagement.legacyMode = false

https://raw.githack.com/takahirox/three.js/MirrorFogExample/examples/webgl_mirror_fog.html

[takahirox]

As I commented #24362 (comment) , swapping the order of <encodings_fragment> and <fog_fragment> (and no fog color space conversion in CPU) seems to resolve the problem.

https://raw.githack.com/takahirox/three.js/MirrorFogExample2/examples/webgl_mirror_fog.html

takahirox@4788ed5

If I understand correctly, the reason why the current order <encodings_fragment> and then <fog_fragment> was to match fog color and clear color. But with #23937 we do clear color space conversion so it may be ok to swap the order now?

[WestLangley]

Fog should be applied in scene-referred linear color space.

Since tone mapping converts from scene-referred color space to display-referred color space, the fog chunk should precede tone mapping.

#include <fog_fragment>
#include <tonemapping_fragment>
#include <encodings_fragment>

Ideally, we should adhere to that convention -- and make whatever changes are required to accommodate it.

/ping @donmccurdy (color management)
/ping @bhouston (legacy conventions)

[donmccurdy]

Fog should be applied in scene-referred linear color space.

...

#include <fog_fragment>
#include <tonemapping_fragment>
#include <encodings_fragment>

Yes, agreed. This would be simpler and easier to reason about than the current ordering. For implementation purposes it would be clear improvement, and makes bugs like this easier to address.

The troublesome part is breaking convention, and explaining that to users. Consider a user writing this today:

// (A) ColorManagement.legacyMode = true
renderer.outputEncoding = sRGBEncoding;
scene.fog.color.setRGB( 40, 40, 40 );              // sRGB → Linear-sRGB 🚨
scene.background.setRGB( 40, 40, 40 );             // sRGB
renderer.setClearColor( new Color( 40, 40, 40 ) ); // sRGB

material.color.setRGB( 40, 40, 40 );               // Linear-sRGB
light.color.setRGB( 40, 40, 40 );                  // Linear-sRGB

Or, this:

// (B) ColorManagement.legacyMode = false
renderer.outputEncoding = sRGBEncoding;
scene.fog.color.setRGB( 40, 40, 40 );              // Linear-sRGB
scene.background.setRGB( 40, 40, 40 );             // Linear-sRGB
renderer.setClearColor( new Color( 40, 40, 40 ) ); // Linear-sRGB

material.color.setRGB( 40, 40, 40 );               // Linear-sRGB
light.color.setRGB( 40, 40, 40 );                  // Linear-sRGB

Under legacyMode = true (A) it's awkward. Fog is related to background and clear color — none are affected by lighting, and you probably want them all to match — but now they're specified in different color spaces.

Under legacyMode = false (B) this should not be a breaking change. We still assume all RGB components are Linear-sRGB, and convert them internally as needed, when setting background or clear color. Only the internal conversion would change for fog.

---

Most users (and examples) are currently in case (A). I wish there were a gentler way to make the change under that scenario, but I'm not sure there is.

[donmccurdy]

^My concerns above are addressed in r152, as color management is now enabled by default.

Discussion continued in:

• Apply fog before tonemapping and encoding #26208