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

WebGPURenderer: Add HDR Support #29573

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

RenaudRohlinger
Copy link
Collaborator

@RenaudRohlinger RenaudRohlinger commented Oct 7, 2024

Description

Enjoy Threejs in HDR 🚀 (requires WebGPU support and an HDR-capable monitor).

Check out the difference:

HDR:
https://raw.githack.com/renaudrohlinger/three.js/utsubo/feat/hdr/examples/webgpu_tsl_vfx_linkedparticles.html

SDR:
https://threejs.org/examples/webgpu_tsl_vfx_linkedparticles.html

This contribution is funded by Utsubo

Copy link

github-actions bot commented Oct 7, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 688.12
170.5
688.12
170.5
+0 B
+0 B
WebGPU 807.93
217.62
808.06
217.66
+129 B
+41 B
WebGPU Nodes 807.44
217.48
807.57
217.52
+129 B
+41 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 462.84
111.78
462.84
111.78
+0 B
+0 B
WebGPU 535.68
144.56
535.81
144.6
+129 B
+46 B
WebGPU Nodes 491.79
134.31
491.92
134.35
+129 B
+45 B

@sunag sunag added this to the r170 milestone Oct 7, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2024

Related: https://github.com/ccameron-chromium/webgpu-hdr/blob/main/EXPLAINER.md

So I guess when using hdr: true, the developer should not configure any tone mapping, right? Should we check that and provide a warning if the developer attempts to do that? Meaning:

renderer = new THREE.WebGPURenderer( { antialias: true, hdr: true } );
renderer.toneMapping = THREE.ACESFilmicToneMapping; // -> produces warning

What about outputColorSpace? Is it correct to use SRGBColorSpace in the demo? It seems not, since the renderer would attempt to convert unbound HDR texels to sRGB which isn't right.

To clarify: webgpu_tsl_vfx_linkedparticles should have used tone mapping in the first place.

@RenaudRohlinger
Copy link
Collaborator Author

I agree with the proposed solution.

@CodyJasonBennett warned me about the toneMapping issue and we could indeed probably just warn the developer about the fact that both HDR and tonemapping cannot coexist yet.
As for outputColorSpace, I'd value @donmccurdy's input on this matter.

@CodyJasonBennett
Copy link
Contributor

CodyJasonBennett commented Oct 7, 2024

What about outputColorSpace? Is it correct to use SRGBColorSpace in the demo? It seems not, since the renderer would attempt to convert unbound HDR texels to sRGB which isn't right.

Your intuition is correct. When rendering out in HDR, you send the physical/lighting units (candelas, nits). The display does the rest, including conversion into the electric signal used by the display, which is what sRGBTransferOETF does in LDR. We can still do a view transform though and preserve the (de)saturation of a tonemapper, but I haven't seen an implementation that outputs HDR. As they are fitted, they would require a custom implementation and switch depending on output parameters (not enough precision to do both in one).

CodyJasonBennett warned me about the toneMapping issue and we could indeed probably just warn the developer about the fact that both HDR and tonemapping cannot coexist yet.
As for outputColorSpace, I'd value donmccurdy's input on this matter.

I've chatted with Don about this since I'm eager to see a real comparison with tonemapping in LDR and HDR (simply disabling tonemapping doesn't compare), but it's a lot of work to implement still. I'm happy to upstream tonemappers here if we can figure out an API. Just a lot of unknowns on top of historical problems and inconsistencies from display manufacturers, which complicate this. I'd be more confident with an API once we have direction here.

Comment on lines +115 to +117
if ( this.backend.parameters.hdr === true ) {

return GPUTextureFormat.RGBA16Float;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we expose RGBA16 drawing buffers independently of the HDR parameter? Perhaps something like drawingBufferType or canvasType, with possible values of HalfFloatType or UnsignedByteType?

There will certainly be interest in using high-precision output without HDR, and I believe it's also good to make HDR usage explicit about the drawing buffer configuration.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Oct 7, 2024

Some caution — our output to the drawing buffer must still be a formed image, we cannot send the stimulus (i.e. scene lighting / luminance) directly to the drawing buffer; the browser/device/OS does not provide image formation any more in “HDR” than in “SDR”. A lot of recent HDR demos on social media have made this mistake by omitting tone mapping. We do still want to consider output units (likely nits), as they relate to the viewing device/display.

WebGPU HDR, as currently shipped in Chrome, tells us nothing about the display, so we are guessing heavily. The amount of HDR headroom available may vary: turn your laptop brightness up, headroom reduces, different color management may be required. This is a major gap in the current WebGPU implementation in Chrome, and something we may need to keep tabs on for changes. And as @CodyJasonBennett says well, “a lot of unknowns on top of historical problems and inconsistencies” exist outside of Chrome's control.

I have a general idea of how to adapt AgX or ACES Filmic for use with “HDR“ output, and I'll look into that a bit. Desaturation is fortunately orthogonal: representation as “SDR” vs. “HDR” does not imply any large difference in saturation. If the comparison does diverge then something is likely wrong.1

What about outputColorSpace? Is it correct to use SRGBColorSpace in the demo? It seems not, since the renderer would attempt to convert unbound HDR texels to sRGB which isn't right.

Your intuition is correct. When rendering out in HDR, you send the physical/lighting units (candelas, nits).

A quick test here would be to render a solid MeshBasicMaterial (example: #ff8c69) with HDR mode enabled. I believe we'll get the expected result when keeping .outputColorSpace = SRGBColorspace. There is no rule that an sRGB value cannot exceed 1... and I think the WebGPU explainer is saying indeed that we must do so, but the WebGPU explainer is not as clear as I'd prefer. I wish they'd offered rec2100-hlg too, that is easier to reason about for our purposes, and hopefully that's coming.

Footnotes

  1. Historically we've made tone mapping very easy for users, and color grading we've left as advanced/DIY ... and I think this has lead to some misconceptions. Adjusting saturation above/below tone mapping defaults is reasonable — and beneficial more often than not!

@donmccurdy
Copy link
Collaborator

donmccurdy commented Oct 14, 2024

Possible API:

import { ExtendedSGRBColorSpace } from 'three/addons/math/ColorSpaces.js';

const renderer = new WebGPURenderer({ outputType: THREE.HalfFloatType });

renderer.outputColorSpace = ExtendedSGRBColorSpace;

My main concern: Extended sRGB (the only option WebGPU currently allows) is not really an appropriate output color space for lit rendering; we need to render an “HDR” image in reference to a well-defined display/medium. I'll file an issue on the WebGPU spec repo about this (EDIT: gpuweb/gpuweb#4919); perhaps there are plans to enable other output color spaces. I would prefer to have this:

import { Rec2100HLGColorSpace } from 'three/addons/math/ColorSpaces.js';

const renderer = new WebGPURenderer({ outputType: THREE.HalfFloatType });

renderer.outputColorSpace = Rec2100HLGColorSpace;

Adaptations to tone mapping are also needed, though they depend on information we do not have, and which may not be well-defined at all when using Extended sRGB.

I know others are excited about the “HDR” features though — would it be possible to start with a PR that exposes outputType: HalfFloatType and confirm that everything still works as expected, before we continue with next steps? I'm hoping to avoid major color changes for existing scenes like r152, which I'm afraid will be necessary to transition from WebGPU's current HDR API to a correct image formation pipeline.

@mrdoob mrdoob modified the milestones: r170, r171 Oct 31, 2024
@mrdoob mrdoob modified the milestones: r171, r172 Nov 29, 2024
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.

6 participants