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

ACES tonemapper uses incorrect gamma curve #87251

Open
Bangmouse opened this issue Jan 16, 2024 · 9 comments
Open

ACES tonemapper uses incorrect gamma curve #87251

Bangmouse opened this issue Jan 16, 2024 · 9 comments

Comments

@Bangmouse
Copy link

Bangmouse commented Jan 16, 2024

Tested versions

Godot v4.2.stable
Godot v4.2.1.stable

System information

Godot v4.2.1.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3070 (NVIDIA; 31.0.15.3742) - Intel(R) Core(TM) i5-8600K CPU @ 3.60GHz (6 Threads)

Issue description

Scenes displayed with the ACES tonemapper show extremely dark mids, with overblown contrast and saturation in the highlights.

linear:
image

Reinhard:
image

filmic:
image

ACES:
image

It appears that the tonecurve is assuming an encoding gamma, while the data is in fact linear. To demonstrate, here is the screenshot in a color managed After Effects project using the ACES/rec709 display transform:

image
Still very dark (no exposure compensation), but much more realistic and manageable. This is what I would expect.

Here is the same but using After Effect's ACES/sRGB display transform:

image
This is behaving similarly to Godot's tonemapper, accounting for the 1.8f exposure bias in Godot's tonemap code making it darker. sRGB and rec709 use the same gamut but have different gamma curves.

To emphasise, the issue is not that the scene is too dark. It's a dark scene and I would expect to have to compensate somehow when using ACES anyway. the issue is unmanageably high contrast and oversaturated colors between highlights and midtones, indicative of overcorrected gamma. Here is the scene in Godot's ACES tonemapper with brighter lights and higher exposure respectively:

image
image
The contrast is way too high, highlights are way too blown out, high mids are way too saturated. As it stands the tonemapper is neither ACES compliant nor useable aesthetically.

Steps to reproduce

  • Apply ACES tonemapper to any 3d scene

Minimal reproduction project (MRP)

ACES tonemap gamma mrp.zip

@Calinou
Copy link
Member

Calinou commented Jan 16, 2024

Please upload a minimal reproduction project1 to make this easier to troubleshoot.

  • Apply ACES tonemapper to any 3d scene

The screenshots show tonemapping being applied to a 2D scene 🙂

Note that any changes to the gamma curve would break compatibility with existing projects, so if anything needs to be changed, it'll have to be an opt-in setting. See also godotengine/godot-proposals#7545 and godotengine/godot-proposals#6613.

Footnotes

  1. A small Godot project which reproduces the issue, with no unnecessary files included. Be sure to not include the .godot folder in the archive (but keep project.godot).

    Drag and drop a ZIP archive to upload it. Do not select another field until the project is done uploading.

    Note for C# users: If your issue is not Mono-specific, please upload a minimal reproduction project written in GDScript or VisualScript. This will make it easier for contributors to reproduce the issue locally as not everyone has a Mono setup available.

@Bangmouse
Copy link
Author

Bangmouse commented Jan 16, 2024

Sorry, the WorldEnvironment node is nested in a 3d scene which projects to a viewport in 2d. Here's the problem visible directly through the 3d renderer
image

and here's an mrp
ACES tonemap gamma mrp.zip

@Bangmouse
Copy link
Author

updated original post to account for testing in 4.2.1

@clayjohn
Copy link
Member

The current implementation of ACES tonemapping comes from https://github.com/TheRealMJP/BakingLab/blob/master/BakingLab/ACES.hlsl

It would be interesting to see what color space their input is in. In the code comments in the file they describe it as an sRGB->intermediate format -> sRGB transform. But its unclear if that means gamma sRGB or linear sRGB for either case.

@MJacred
Copy link
Contributor

MJacred commented Jun 23, 2024

This is not my field of expertise, but maybe the ACES implementation/handling is correct, and it's just more affected by Godot's lack of color profile management? That would at least explain the oversaturation.

@betalars
Copy link
Contributor

I agree ACES looks like it uses the incorrect gamma curve. The ACES implementations I've seen so far tend to be more punchy than other transforms, so maybe it's not totally wrong, but godot has the strongest contrast and for me personally it is kind of unusable.

But if I understand it correctly, ACES is meant as a compatibility space, so maybe the formula godot is using is not meant for the color space?

Maybe I will investigate this a bit further. The least I could do is create a color management test scene ...

@betalars
Copy link
Contributor

Yeah, having looked into this, I don't entirely understand the point of using ACES to begin with.

ACES exists as a unified color space so you can match the Footage of different Camera Manufacturers and have a unified workflow with consistent view-transform across multiple post processing pipelines. None of which is relevant for Godot Rendering mostly. I mean that could be useful for AR compositing potentially.

So if we want to properly support ACES, we need to know what color space Godot renders into, so linear, and then look up the right ACES transform for our Display space.

So the first step to actually supporting ACES is creating a proper Godot Input Color Profile for ACES. At this point I think @MJacred may be right about the lacking general color management might be the first hurdle to overcome.

@betalars
Copy link
Contributor

Okay, as i suspected importing ACES reference images into Godot shows that the importer does not recognize the ACES color space.

An Image exported directly from blender to sRGB and then rendered in Godot however looks fine.
Image

But setting the Output Tonemap to ACES makes everything kind of weird again:
Image

I think this is probably a good point of reference for a proper ACES implementation:
https://github.com/Apress/physically-based-shader-dev-for-unity-2017/blob/master/PostProcessingv2/Assets/PostProcessing/PostProcessing/Shaders/ACES.hlsl

And of course:

Until then I would say as far as I can tell I can confirm that the linear RGB to ACES tonemap in Godot right now is not working as intended.

@betalars
Copy link
Contributor

So I just catched a godot dev and turns out they are working on re-implementing the ACES tonemapping and they agree with me that godot does not have proper ACES support.

However they also said actual ACES support is not practical for godot core, but they are working on making it possible via extensions.

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

No branches or pull requests

5 participants