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

WebGLRenderer: Consider .outputEncoding for background, clear color, and fog #23937

Merged
merged 3 commits into from
Nov 9, 2022

Conversation

donmccurdy
Copy link
Collaborator

@donmccurdy donmccurdy commented Apr 23, 2022

Currently, at least three Color properties (renderer clear color, scene.background color, and scene.fog color) are exempt from the WebGLRenderer specified output color space. These exceptions create confusion — to make the ground plane match the background or fog color, for example, you need to specify the same color in two color spaces, for reasons that are mostly based on how WebGLRenderer applies the output color space conversion. However, if the user provides post-processing that handles color space conversion, the color space used for background and fog colors must change.

This PR is an idea for how to clear up that inconsistency, applied only when ColorManagement.legacyMode = false. In this case — as with THREE.Color setters — hexadecimal and CSS-string inputs are understood to be sRGB, and converted automatically to Linear-sRGB. Then internally, WebGLRenderer (which now knows it is dealing with something in the working color space, Linear-sRGB) can make the conversion appropriate for the output color space.

With this change, all of these colors will match:

document.body.style.background = '#123456';
scene.background.color.setHex( 0x123456 );
renderer.setClearColor( 0x123456 );
material.color.setHex( 0x123456 );

NOTE: The statement above assumes no post-processing, and renderer output to sRGB. One thing I'm not sure about is how renderer.outputEncoding and renderTarget.encoding are (or ideally should be?) respected in a post-processing chain. This relates to @WestLangley's comments #23614 (comment) and #18942 (comment).

@LeviPesin
Copy link
Contributor

One thing I'm not sure about is how renderer.outputEncoding and renderTarget.encoding are (or ideally should be?) respected in a post-processing chain.

Can this be made with #18322? Then renderer can have list of all post-processing effects and apply the encoding after applying them...

@donmccurdy
Copy link
Collaborator Author

Depends on answers to #23614 (comment), I'm not sure. 🤔

@donmccurdy
Copy link
Collaborator Author

/cc @drcmda this is a possible solution to the confusion with background and clear color we've discussed in Discord. It does not yet account for post-processing, still some open questions there.

@drcmda
Copy link
Contributor

drcmda commented Apr 23, 2022

/cc @drcmda this is a possible solution to the confusion with background and clear color we've discussed in Discord. It does not yet account for post-processing, still some open questions there.

yes, it is absolutely welcome. and as for pp @vanruesc will know ...

@vanruesc
Copy link
Contributor

I'd like to provide some information on how this is handled in postprocessing.

sRGB Encoding

The main problem is that WebGL doesn't support hardware sRGB encoding for the back buffer / canvas. Therefore, all materials in postprocessing that may need to perform manual sRGB encoding include the encodings_fragment shader chunk from three. Those materials are CopyMaterial and EffectMaterial first and foremost as well as some blur material variations that may be rendered to screen. Materials such as DepthDownsamplingMaterial or EdgeDetectionMaterial always produce linear output and can safely be ignored.

With this setup, fullscreen passes can either render linear output or sRGB output based on the encoding of the render target and renderer.outputEncoding:

  • If the fullscreen pass renders to a linear render target, output will be linear.
  • If it renders to an sRGB render target, it will either use hardware sRGB encoding via SRGB8_ALPHA8 in WebGL 2 or manual sRGB encoding as fallback.
  • And if it renders to screen it will output linear colors or manually encoded sRGB colors based on renderer.outputEncoding.

I'd argue that the user shouldn't have to use or worry about an additional fullscreen pass just to peform the sRGB conversion since this can easily be done at the end of any pass. However, if you're using several isolated fullscreen passes, then you'd have to include the sRGB encoding shader chunk in all of the fullscreen materials that are used in the respective passes because they could all potentially render to the canvas, which doesn't support hardware encoding.

Background, Fog & Clear Color

My current approach for the background, fog and clear color is to manually convert them to linear via convertSRGBToLinear. This is something users currently have to be aware of since three uses these colors as-is. This wouldn't be an issue if they were properly included in the whole linear-sRGB pipeline which, I believe, this and the other color management tickets are currently discussing.

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Apr 24, 2022

The main problem is that WebGL doesn't support hardware sRGB encoding for the back buffer / canvas...

Is this only true for devices that both (1) do not support WebGL 2.0, and (2) do not support EXT_sRGB? Or is it more common than that?

@gkjohnson and @Mugen87 you've mentioned that you don't find the current model particularly confusing, so perhaps you can help me clear up one thing I'm confused by. I understand that the post-processing typically uses Linear-sRGB until the final pass, and target.encoding should be LinearEncoding, that part seems fine. But is renderer.outputEncoding just completely ignored when post-processing is used? As far as I can see that's the case for three.js post-processing, but not pmndrs/postprocessing, and I do get confused by that.

Within three.js the requirement of adding GammaCorrectionShader is not ideal, but it's mostly fine, and (if we keep it) the name should be changed somewhere in the roadmap of #23614. I'd prefer if the need for that pass could be removed like @vanruesc describes, and handled with SRGB8_ALPHA8 instead, but I'd understand if we don't like the encoding fragments going everywhere.

@LeviPesin
Copy link
Contributor

Does this actually is a two separate problems - one being when encoding should be applied (e.g. by renderer after getting the output from the linear-color-space post-processing effects and before passing the output to the display-color-space effects, as I proposed) and other one being how encoding should be applied (e.g. by SRGB8_ALPHA8 or by <encodings_fragment>)?

@vanruesc
Copy link
Contributor

Is this only true for devices that both (1) do not support WebGL 2.0, and (2) do not support EXT_sRGB? Or is it more common than that?

@donmccurdy Hardware sRGB encoding is only available for render targets. The canvas doesn't support it.

Related info: https://www.khronos.org/webgl/public-mailing-list/public_webgl/1707/msg00015.php, relevant portion: "there's presently no way to do [sRGB canvas] in WebGL, and adding a way may turn out to be more complex than assumed, so what you've got to do is re-encode the linear value manually to sRGB for output."

@vanruesc
Copy link
Contributor

But is renderer.outputEncoding just completely ignored when post-processing is used? As far as I can see that's the case for three.js post-processing, but not pmndrs/postprocessing, and I do get confused by that.

That's correct; three/postprocessing ignores renderer.outputEncoding while pmndrs/postprocessing makes use of three's sRGB encoding shader chunk mechanism which respects the setting.

I understand that the post-processing typically uses Linear-sRGB until the final pass, and target.encoding should be LinearEncoding, that part seems fine.

This is true in theory. You can store linear colors in render targets, but you'll need at least 12 bits per channel or HalfFloatType buffers to prevent color degradation and banding. In practice, it's better to use UnsignedByteType sRGB frame buffers to minimize color degradation because they are more widely supported and more efficient despite the back and forth conversions from linear to sRGB and back to linear.

@gkjohnson
Copy link
Collaborator

But is renderer.outputEncoding just completely ignored when post-processing is used? As far as I can see that's the case for three.js post-processing, but not pmndrs/postprocessing, and I do get confused by that.

The three.js postprocessing stack has not been rethought for years afaict. There are definitely ways to address this from a user-facing perspective.

One thing to keep in mind, as @vanruesc mentioned, automatic hardware support for interpreting textures as linear / sRGB is only supported for render targets. When we're talking about the canvas it's still a bit of a wild west and requires the user to explicit perform the conversion which is why the gamma shader is required in the first place. This is why I think changing outputEncoding to canvasEncoding or canvasColorSpace would make things more clear.

In terms of making post processing less confusing then the post processing stack could implicitly add the Gamma conversion as a last step only if the renderer canvas color space was set to sRGB.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 25, 2022

Hardware sRGB encoding is only available for render targets. The canvas doesn't support it.

This topic was also discussed here some time ago: #23019

@donmccurdy
Copy link
Collaborator Author

This is why I think changing outputEncoding to canvasEncoding or canvasColorSpace would make things more clear.

A few questions in #23936 (comment).

it's better to use UnsignedByteType sRGB frame buffers to minimize color degradation...

I see, thanks — I knew that UnsignedByteType Linear-sRGB was very lossy but didn't know the device support situation vs. HalfFloatType. Unfortunately, this approach won't work with tone mapping, correct? We'd need to preserve open-domain [0,∞] values until the tone mapping step, so (for that purpose) 8 bits isn't enough in sRGB or Linear-sRGB. 😕

three/postprocessing ignores renderer.outputEncoding while pmndrs/postprocessing makes use of three's sRGB encoding shader chunk mechanism which respects the setting.

I think this is what has confused me then. All three names (outputEncoding, outputColorSpace, canvasColorSpace) would make most sense if they worked at the end of a post-processing chain.

@vanruesc
Copy link
Contributor

Unfortunately, this approach won't work with tone mapping, correct?

Right, Uint8 sRGB buffers are a compromise; colors are clamped to [0.0, 1.0] and you still lose plenty of information in the darker spectrum which leads to noticable banding in dark scenes. Linear, high precision HalfFloatType buffers don't have these issues and are the way to go for HDR-like workflows on desktop devices.

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Apr 27, 2022

OK, I think what this PR would need to do, in order to support post-processing with render targets ("rt" below), would be:

renderer.outputEncoding rt rt.encoding scene.background
sRGB sRGB
sRGB Linear-sRGB Linear-sRGB
sRGB sRGB Linear-sRGB
Linear-sRGB Linear-sRGB
Linear-sRGB Linear-sRGB Linear-sRGB
Linear-sRGB sRGB Linear-sRGB

Note that WebGLRenderer cannot know whether a GammaCorrectionShader is included somewhere in the post-processing chain, while it does know the render target encoding. Fortunately the choice of background color space does not depend on either.

tl;dr — if rendering to a render target, set background color to working color space (Linear-sRGB) because we know whatever color space transform is at the end of the post-processing chain will affect the background color. If rendering to canvas, set background color in renderer.outputEncoding color space because we know it won't be affected by the usual encoding fragment.

@vanruesc
Copy link
Contributor

@donmccurdy That looks correct to me.

@eviltik

This comment was marked as off-topic.

@vlucendo
Copy link
Contributor

vlucendo commented May 6, 2022

Does anyone have performance tests or information about the performance implications of using HalfFloatType buffers?
For me it seems more convenient and cleaner to keep everything in linear space (specially when writing custom shaders) and perform the last srgb conversion at the end of the rendering.

@donmccurdy
Copy link
Collaborator Author

For me it seems more convenient and cleaner to keep everything in linear space (specially when writing custom shaders) ...

Your shaders can be exactly the same either way. Converting to/from sRGB is performed automatically (by WebGL) when reading/writing a render target.

@takahirox
Copy link
Collaborator

takahirox commented Jul 19, 2022

#24362 may be a good example showing a color space problem originally caused by the issue this PR wants to resolve.

Fog color is not affected by renderer.outputEncoding to match the clear color that is not affected by renderer.outputEncoding.

But the fog color rendered to Mirror (Reflector or WebGLRenderTarget) set to material.map is affected when rendering to the screen. Therefore, fog colors in mirror and in main scene can mismatch.

Please refer to #24362 (comment) for the details.

I hope all the colors will be under outputEncoding controls. It'll be less problematic for render targets and post-processings.

@donmccurdy donmccurdy force-pushed the feat/background-clear-color-output branch from 83d2135 to cf97ad7 Compare October 24, 2022 21:24
@donmccurdy donmccurdy changed the title WebGLRenderer: Consider .outputEncoding for background and clear color WebGLRenderer: Consider .outputEncoding for background, clear color, and fog Oct 24, 2022
@donmccurdy donmccurdy marked this pull request as ready for review October 24, 2022 23:16
@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Oct 24, 2022

This PR is now ready for review. I've updated it to include fog color as well, because matching the background color to the fog color is otherwise difficult. The PR has no effect when ColorManagement.legacyMode === true (default), but it does change the behavior of background, clear, and fog colors under .legacyMode = false.

@Mugen87 Mugen87 added this to the r147 milestone Nov 8, 2022
@mrdoob mrdoob merged commit 22f434c into mrdoob:dev Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants