godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
86.9k stars 19.45k forks source link

ACES tonemapper uses incorrect gamma curve #87251

Open Bangmouse opened 6 months ago

Bangmouse commented 6 months ago

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

Minimal reproduction project (MRP)

ACES tonemap gamma mrp.zip

Calinou commented 6 months ago

Please upload a minimal reproduction project[^1] to make this easier to troubleshoot.

[^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.

  • Apply ACES tonemapper to any 3d scene

The screenshots show tonemapping being applied to a 2D scene :slightly_smiling_face:

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 https://github.com/godotengine/godot-proposals/discussions/7545 and https://github.com/godotengine/godot-proposals/issues/6613.

Bangmouse commented 6 months ago

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 commented 6 months ago

updated original post to account for testing in 4.2.1

clayjohn commented 6 months ago

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 commented 3 weeks ago

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.