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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a new high quality tonemapper: ACES Fitted (3.x) #52477

Merged
merged 1 commit into from
Sep 13, 2021

Conversation

Lauson1ex
Copy link
Contributor

3.x version of #52476.

Adds a new high quality, tonemapper mode which is more physically accurate. This one better represents how lighting works in the real world. High energy lights or emissive materials will become lighter as they get brighter, eventually becoming white if its color is bright enough to saturate the camera sensor.

Closes #3214 馃檪

Current Godot implementation:
image

ACES Fitted:
image

Current Godot implementation:
image

ACES Fitted:
image

Current Godot implementation:
image

ACES Fitted:
image

Additionally, fixes a small bug in the tonemapper that clamped colors even when tonemapping was disabled. Fixes the issue found while reviewing PR #51708. Now the PR should work as expected. 馃檪

@clayjohn
Copy link
Member

clayjohn commented Sep 8, 2021

Implementation looks good, I'm not sure what is going on with the documentation checks though, its saying the new enum should not be added.

@atirut-w
Copy link
Contributor

atirut-w commented Sep 8, 2021

Implementation looks good, I'm not sure what is going on with the documentation checks though, its saying the new enum should not be added.

My attempt at an educated guess: forgot to update docs, maybe?

Checked just now and now I'm stumped too.

@bruvzg
Copy link
Member

bruvzg commented Sep 8, 2021

I'm not sure what is going on with the documentation checks

BIND_ENUM_CONSTANT(TONE_MAPPER_ACES_FITTED) is missing from the VisualServer::_bind_methods

@Lauson1ex
Copy link
Contributor Author

BIND_ENUM_CONSTANT(TONE_MAPPER_ACES_FITTED) is missing from the VisualServer::_bind_methods

Thanks. Updated 馃檪

@akien-mga
Copy link
Member

Thanks!

@ghost
Copy link

ghost commented Sep 25, 2021

Sorry to necropost, but shouldn鈥檛 this visual tweak be also available to the other Tonemap modes, like Filmic and the others? Being it just the way emissive materials behaves regardless of how you tonemap your scene. Is it possible?

@clayjohn
Copy link
Member

@Leocesar3D this is not a change to the way emissive materials are handled. It is a different, more correct, way of implementing tone mapping using the ACES colour space.

@atirut-w
Copy link
Contributor

Giving Filmic its own "Fitted" version still sounds good, though.

@Calinou
Copy link
Member

Calinou commented Oct 20, 2021

Giving Filmic its own "Fitted" version still sounds good, though.

I don't know if this is feasible due to how the filmic tonemapper works. How do other engines handle this? I believe Godot already uses Uncharted 2's formula for filmic tonemapping.

@Lauson1ex
Copy link
Contributor Author

I don't know if this is feasible due to how the filmic tonemapper works. How do other engines handle this? I believe Godot already uses Uncharted 2's formula for filmic tonemapping.

Other engines implement the industry standard algorithms: Filmic operator, Reinhard operator, Hable's Unchard 2 operator, Narkowicz's approximate ACES operator, and finally, the ACES operator performed in the ACES color space.

Unreal Engine 4 deprecated the previous tonemapping operators, and UE5 ditched them altogether in favor of ACES:

With Unreal Engine 4.15, the Filmic tonemapper using the ACES standard is enabled by default. Because of this change, there may be differences in appearance to your existing content. While there is no timeframe in which we plan to remove the legacy tonemapper, it will be deprecated and removed at some point in a future Unreal Engine release.

The take away here is: previous tonemapping techniques try to approximate what ACES achieves accurately, which is real filmic color grading. But since the previous tonemapping operators have become standards (have been widely used), they are still offered as options for legacy content, or if you want to achieve a particular look.

We can't achieve the same look of ACES on previous tonemappers because previous tonemapper perform operations on the RGB color space, while ACES (fitted) transforms from RGB to the ACES color space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants