-
-
Notifications
You must be signed in to change notification settings - Fork 23.3k
Improve AgX tonemapping curve. #102435
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
Improve AgX tonemapping curve. #102435
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me.
6094e84 to
4493e06
Compare
|
Previously I had not updated the |
4493e06 to
e7504cf
Compare
e7504cf to
28167ab
Compare
|
I've updated this PR to remove the docs changes, as those were merged separately in #102820. |
28167ab to
c76154e
Compare
|
After around 2 months of R&D, I've got AgX into a good state where it is both high performance and high quality, matching the reference curve much closer than before. This is now ready to merge with addition of white parameter in a future PR. |
c76154e to
fa3449f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me, thanks for taking the time to dive deeper into AGX.
fa3449f to
05bc9f3
Compare
|
Stephen Hill, author of the ACES tonemapping curve approximation that Godot uses, took the time to review this as well and suggested a clever optimization: Where Stephen explained that The full breakdown starts by replacing ...is equivalent to:
In my test, I found this improved performance of the tonemapper from 0.26 ms to 0.23 ms. I've updated the PR to include this optimization. |
|
I’m temporarily putting this PR back in a draft state, as I’ve discovered a piecewise polynomial equation that may be better for approximating AgX in terms of performance, future |
|
Update on my progress: After playing around with it more, I've found that there are actually four goals for the new AgX tone curve:
The first two goals are well achieved by the current state of this PR. The third goal of the The fourth goal is simply not achieved with the current state of this PR. The contrast (toe strength) is greatly influenced by manipulation of the dynamic range. I thought it would work well enough, but the contrast (toe strength) increases as dynamic range increases. In practice, artists would typically want the opposite behaviour, because an increase in dynamic range is naturally an increase in contrast already. The reason this is such an issue is that the sort of HDR that Godot is aiming to support is actually described by Apple as "Extended Dynamic Range". That is, values can simply extend beyond 100% (1.0). If the user turns their brightness to maximum, it's possible that there will be no extra dynamic range left to work with, so the maximum value will be 1.0. Or, the user may have their brightness low, so the maximum value would be something like 5.0 or 10.0. This behaviour is effectively the same on Windows for Godot's purposes, but Apple does a better job of explaining it.
All that to say, there is more maths to be done to achieve all of the goals for the new AgX tone curve. All of this should be very possible, but it's not something that's well documented (most HDR tonemapping is fixed range instead of variable/extended range). This PR could be merged, but since the curve will need to be adjusted again in the future to support variable dynamic range, it might be better to only merge the curve after all goals have been met. |
|
I've created a new PR that doesn't have as good of performance as this PR, but is extremely stable and predicable across all variable dynamic ranges, making it suitable for SDR, HDR, and variable Extended Dynamic Range (EDR): #106940 I'll leave this in draft for now, but will close it if and when the new PR is merged. |
Superseded by #106940
Not cherry-pickable to 4.3, as AgX is only in 4.4.
Tonemapping Curve
This PR changes the tonemapping curve of AgX to use an approximate in linear encoding. Timothy Lottes' tonemapping curve equation is used to approximate the AgX curve. This new approximation introduces a higher contrast than the original Blender AgX by EaryChow, but greatly improves performance and flexibility.
Performance
Using the visual profiler, Calinou's tonemapping test scene, ~4K window, and an NVIDIA 980 Ti on Windows 11, I recorded the following performance stats:
AgX (exact curve with white parameter): 1.35 ms
Tony McMapface: 1.29 ms
AgX (exact curve): 1.22 ms
AgX (Godot 4.4 beta 4): 1.19 ms
ACES: 1.07 ms
AgX (this PR): 1.05 ms
Filmic: 1.0 ms
Rienhard: 0.93 ms
Linear: 0.82 ms
Comparison
White parameter and HDR output
Because this AgX tonemapping curve approximation uses Timothy Lottes' equation, it will be possible to control the maximum value (white parameter) and also adjust brightness and exposure in the future. A separate PR will add support for the white parameter and further features can be added relating to controlling the middle grey input and output mappings for HDR output. More information about this tonemapping equation can be found in the GDC slides or video.