WebGLRenderer: Fix incorrect background color space when setting scene.background to color

[gkjohnson]

Related issue: #28423

Description

Previously the transmission buffer was rendering the background with an incorrect color space causing it to be lighter. I'm not entirely clear where the background color was being rendered for the transmission pass previously. Here's a demo showing the incorrect colors through a transmissive object.

• Ensures the transmission buffer is assigned a color space.
• Calls background.render when rendering the transmission pass which converts the color apropriately.

Before	After

Edit: Possibly related to #28118, as well?

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev	Filesize PR	Diff
678.9 kB (168.2 kB)	679 kB (168.3 kB)	+80 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev	Filesize PR	Diff
456.9 kB (110.3 kB)	457 kB (110.3 kB)	+57 B

[donmccurdy]

Hmm, it hadn't occurred to me that the default color space of a render target is THREE.NoColorSpace! Good catch, and I expect that there my be some other places we may need to fix similar issues.

[donmccurdy]

I'm not sure I understand the motivation for this part of the change, why would XR presentation and depth sensing affect whether the background is drawn?

[gkjohnson]

I don't know the history of some of these decisions, unfortunately - just that it's already done elsewhere in WebGLRenderer to determine whether to draw the background. There's probably some clean up that should happen in the render function in this respect.

See https://github.com/mrdoob/three.js/blob/dev/src/renderers/WebGLRenderer.js#L1156

[donmccurdy]

Ok! I don't know much about that part but will approve the PR in any case, thanks again!

[Mugen87]

Do you mind passing the existing renderBackground flag from the render() method into renderTransmissionPass(). In this way, it's not necessary to make the same evaluation twice.

[Mugen87]

You can also consider to rename it to _renderBackground and store it in the same scope like _clippingEnabled. Then it is not necessary to change the signature of renderTransmissionPass().

[gkjohnson]

Done!

[Mugen87]

I'm not entirely clear where the background color was being rendered for the transmission pass previously.

I think it didn't at all. Before #28118, the background was renderer to the default framebuffer (or current bound render target) before any render pass were executed. This approach introduces performance issue though as explain in #28118.

Now, the background is rendered after the transmission pass. However, before and after #28118 the background was never rendered with the transmission render target bound. This should be corrected with this PR.

[gkjohnson]

Sounds good - as I mentioned previously I'm a bit confused by some of the logic relating to the transmission pass and I don't know enough about the reasoning to make changes to it. This code block, for example, is really odd to me:

			_this.getClearColor( _currentClearColor );
			_currentClearAlpha = _this.getClearAlpha();
			if ( _currentClearAlpha < 1 ) _this.setClearColor( 0xffffff, 0.5 );

			_this.clear();

Why are we forcing a 50% transparent white background if the background is at all partially transparent of any color, for example?

[Mugen87]

The reasoning behind this is explained in #25819.

[Mugen87]

After reading the code again I think _this.clear(); can be removed now. Otherwise clear() is effectively called twice.

[Mugen87]

The previous approach only honored the clear color set via setClearColor(). But if you trigger the clear indirectly via background.render( scene ), then both clear color and Scene.background = new Color() are honored.

[gkjohnson]

I don't think it's necessarily the case? Again the rationale behind the current transmission clearing logic isn't super clear to me and may deserve some more comments (especially for the white clear color, for example). If renderBackground is set to false then this call to "clear" will still be run, for example. I guess it can at least go in an "else" branch?

[Mugen87]

If renderBackground is set to false then this call to "clear" will still be run, for example. I guess it can at least go in an "else" branch?

That makes sense. This particular clear should be retained in XR.