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: Add Support for Khronos Neutral Tone Mapping #27668

Merged
merged 7 commits into from
Feb 9, 2024

Conversation

elalish
Copy link
Contributor

@elalish elalish commented Feb 2, 2024

This is the tone mapper I designed for color-accuracy and that we are working to make a standard through the Khronos 3D Commerce group. In-depth writeup here. The summary is this is an improved alternative to No/Linear/Clamped tone mapping. It preserves the baseColor sRGB values as faithfully as possible through to the final render under uncolored lighting.

Improvements over No tone mapping:

  • Avoids clamping artifacts in highlights
  • Has a uniform path to white
  • Avoids hue skews
  • Avoids saturation loss especially on darker dielectric materials

Improvements over ACES/AgX:

  • Maintains color saturation
  • Has a larger reachable set of sRGB
  • Avoids hue skews

Situations where AgX is a better choice:

  • Out-of-gamut colors: if your textures or lights contain colors outside of the Rec.709 (sRGB) gamut.
  • If you have important visual detail across a wide dynamic range (e.g. landscape photography). The HDR is aggressively compressed in this tone mapper, which is most appropriate for highlights.
  • If your output will not be directly compared to images, so absolute sRGB values are less important than scene-relative perception.

I've already shipped this in <model-viewer> and I intend to make it our default in v4.0. You can test it yourself by drag-and-dropping any GLB and HDR into this Glitch.

FYI @mrdoob @WestLangley @donmccurdy @bhouston @hybridherbst

Copy link

github-actions bot commented Feb 2, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
676.2 kB (167.9 kB) 676.9 kB (168.1 kB) +677 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
456.8 kB (110.7 kB) 457.4 kB (111 kB) +646 B

@WestLangley WestLangley changed the title added Commerce tone mapping WebGLRenderer: Add Support for Commerce Tone Mapping Feb 2, 2024
@WestLangley WestLangley added this to the r162 milestone Feb 2, 2024
Add support for `CommereceToneMapping`.
Add support for `CommerceToneMapping`.
@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 5, 2024

@elalish I've added two minor changes to your branch to make sure CommerceToneMapping also works with post processing.

Add reference for `CommerceToneMapping`.
@mrdoob
Copy link
Owner

mrdoob commented Feb 5, 2024

Great tone mapper!

However, as discussed offline... I'm not a fan of the "Commerce" name and would very much prefer renaming it to something like "Neutral" before it's not too late 😇

Maybe "Khronos' Neutral Tone Mapper"?

@hybridherbst
Copy link
Contributor

hybridherbst commented Feb 5, 2024

For anyone curious, the tonemapping can also be tried out here, across a wide range of Khronos sample models:

  1. go to https://viewer.needle.tools
  2. either drop files or select Sample Assets > pick models
    • close the Sample Assets menu again
  3. select View Options > Tonemapping > Commerce (model-viewer)

@elalish elalish changed the title WebGLRenderer: Add Support for Commerce Tone Mapping WebGLRenderer: Add Support for Khronos Neutral Tone Mapping Feb 7, 2024
@elalish
Copy link
Contributor Author

elalish commented Feb 7, 2024

@mrdoob - renamed. Thanks @Mugen87 for the update.

@WestLangley
Copy link
Collaborator

To be less verbose, maybe NeutralToneMapping, and in the docs refer to the Khronos 3D Commerce Group?

Anyway, the docs should be updated.

@mrdoob
Copy link
Owner

mrdoob commented Feb 8, 2024

What @WestLangley said 👍

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 9, 2024

I'll merge and clean up.

@Mugen87 Mugen87 merged commit 996903f into mrdoob:dev Feb 9, 2024
12 checks passed
@elalish elalish deleted the commerceToneMapping branch February 9, 2024 16:28
@elalish
Copy link
Contributor Author

elalish commented Feb 9, 2024

Thanks!

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.

7 participants