Skip to content

Commerce tone mapper#4495

Merged
elalish merged 3 commits into
masterfrom
toneMapper
Oct 5, 2023
Merged

Commerce tone mapper#4495
elalish merged 3 commits into
masterfrom
toneMapper

Conversation

@elalish

@elalish elalish commented Oct 4, 2023

Copy link
Copy Markdown
Contributor

This adds an experimental feature, as we're still soliciting feedback on the subtleties of the equation before it will become an official part of our API. The new feature is a tone mapping function designed for 3D Commerce applications as an alternative to our default, ACES. The Commerce tone mapper is designed to retain color accuracy as much as possible without producing noticeable artifacts in highlights (ACES and Linear/Clamped/No tone mapping both have poor color accuracy, though in different areas).

Note: the Commerce option has less contrast than ACES (intentionally), which may make it look "flat" in a side-by-side comparison with ACES. In general, higher contrast lighting should be used with Commerce tone mapping than with ACES to make up the difference. We may add a related option for our neutral lighting in the future, since the current default was designed for ACES.

This PR also adds an option for testing this in our Editor, as well as an additional model, Macbeth, that helps to show the color issues we're attempting to resolve. Macbeth includes unlit (and therefore un-tone-mapped) spheres with the same base color as the shiny and matte spheres to allow comparison.

ACES:
image
Commerce:
image

@elalish elalish self-assigned this Oct 4, 2023
@elalish elalish requested a review from diegoteran October 5, 2023 00:04
@elalish elalish merged commit 4f10202 into master Oct 5, 2023
@elalish elalish deleted the toneMapper branch October 5, 2023 16:58
@hybridherbst

Copy link
Copy Markdown
Contributor

I'm seeing a color shift on higher exposures; e.g. this model starts to become red-ish at exposure = 3 or higher:
(the editor only allows up to 2 right now)
van-veer-original-custom.zip

Exposure=1 Exposure=3 Exposure=10
image image image

Is this intended?

And an additional question: why is peak calculated with max(r, max(g,b)) instead of using a perceptual luminance value like float peak = dot(color, vec3(0.299, 0.587, 0.114)) or so?

@elalish

elalish commented Oct 26, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback! This appears intentional to me, since brown is also red-ish, just darker. With white light, PBR will only multiply your base color by a scalar factor and add some white - this tone mapper does the same: multiplies the color by a scalar factor and mixes toward white (desaturates for high brightness). Do you have any examples of what you would expect this color to look like when overexposed?

@elalish

elalish commented Oct 26, 2023

Copy link
Copy Markdown
Contributor Author

As to perceptual luminance, I tried that route first and found it actually introduced a lot of problems. I finally realized it was because the purpose of a tone mapper is to fit into the sRGB unit cube, whose bounds do not have uniform perceptual luminance. That vector is useful when working in a luminance space, but I am not.

JL-Vidinoti pushed a commit to vidinoti/model-viewer that referenced this pull request Apr 22, 2024
* added commerce tonemapping

* added tone mapping selector to editor

* added Macbeth model to editor
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.

2 participants