Optimize AgX tonemapper's handling of negative values

[allenwp]

This PR has three parts:

1. Remove max call by introducing insignificant error.
2. Remove unnecessary final negative clipping.
3. Remove support for negative sRGB input values.

I can separate these into two separate PRs if that is preferred, as the first two parts are simply optimizations and improvements while the third part affects how renders appear for some scenarios.

1. Remove max call by introducing error

Both the Rec. 2020 and Blender AgX inset matrices are entirely positive values. This makes it quite easy to reason about them. For the log2 function:

The result is undefined if x≤0

To prevent undefined behaviour, I originally had a separate color = max(color, 1e-10), but I've realized that I can simply add an insignificant error to the RGB value to ensure that, even after the color is transformed, it will still be a positive non-zero value:

// A value of 2e-10 intentionally introduces insignificant error to prevent
// log2(0.0) after the inset matrix is applied. color will be >= 1e-10 after
// the matrix.
color = max(color, 2e-10);

color = agx_inset_matrix * color;

color = clamp(log2(color), min_ev, max_ev);

I have reviewed images generated with this change in all three renderers and compared them to the current master build. I could not find a single pixel in any of the images that was different, even by a single bit in a 24-bit sRGB gamma space image. If my reasoning and math is correct, I believe this is a safe and simple optimization. Given that tonemapping functions like this one are typically hard-coded to specific matrices and do not change over time with new versions of the engine, I don't see the increased complexity of changing matrices in the code to be a downside.

master	Insignificant error added

2. Remove unnecessary final clipping

In Blender's AgX LUT generation, the final tonemapped value is put through the complex lusRGB.compensate_low_side function. This function takes the negative values and smoothly converts them to non-negative values, while maintaining vibrancy to the colours. Since we cannot implement this function in a realtime shader, it actually makes most sense to simply return the negative RGB values from this tone mapping function. This way, the hue of the final color is not affected by clipping and the value will be better suited for post processing effects. In the end, if it remains negative it will be clipped when it is presented to the user anyway.

To further support this decision to remove this final clipping, the existing ACES tone mapper also returns negative values, even when the input is entirely positive sRGB values. In the following images, any RGB value returned from the tonemapper less than -1e-6 is highlighted as green:

ACES	AgX without final clipping

3. Remove support for negative sRGB input values

This change improves performance of the AgX tonemapper by allowing two matrix multiplications to be combined into one. This comes at the cost of loss of color information that could be correctly interpreted as positive RGB values in the Rec. 2020 color space.

Negative input values are most common with HDR images (such as an HDR skybox?) or negative lights. There might be other scenarios, so please comment if you know of any. Currently, no tonemapper except for the Linear tonemapper supports negative RGB input values.

The decision was made to use a highly simplified version of Blender's AgX that removes the logic that prevents hue shifts. Additionally, smooth handling of negative input and output values (lusRGB.compensate_low_side) is not included in Godot's AgX. To get these features, a LUT texture and a new custom tonemapping feature must be added to Godot. I believe that removing support for negative input values is in the spirit of the decisions that were made up until this point. I expect only a few users would truly benefit and prefer Godot's AgX to support negative input values that can be represented as positive values in Rec. 2020 space. It seems wrong to add an extra matrix multiplication that would benefit few users.

This change has no impact on many actual game project renders in the Forward+ renderer, because it seems uncommon for substantial (< -0.01) negative RGB values to be given as an input to the tonemapping function outside of the examples I gave...

I can write more and provide comparison images, but I'd rather get this PR published sooner rather than later.

Other notes

I also regenerated both combined matrices to ensure the colour space conversions match accurately. The original rec 709 to 2020 and vice-versa matrices were of low precision, so I recalculated them using the high precision matrices from Blender/EaryChow's code.

[allenwp]

For posterity, these are the matrices:

xyz to 709:
3.240437356492007 -0.9692674982687597 0.0556434463874256
-1.5371305409000549 1.8760136862977035 -0.2040259700872272
-0.4985288240756933 0.0415560804587997 1.0572254813610864

709 to xyz:
0.41245857817966206788 0.21267395437388827496 0.019333995852171663522
0.35757553616579056842 0.71515107233158101283 0.1191918453885968322
0.18043743323632757227 0.072174973294531013715 0.95030381504465871254

xyz to 2020:
1.7166634277958805 -0.6666738361988869 0.0176424817849772
-0.3556733197301399 1.6164557398246981 -0.0427769763827532
-0.2533680878902478 0.0157682970961337 0.9422432810184308

2020 to xyz:
0.63695350678507403952 0.26269833895655600358 0.000000000000000075315551199785676699
0.14461918466923311511 0.67800876577281646979 0.028073135847556982249
0.16885585392287339361 0.059292895270627329266 1.0608272349505708227

709 to 2020:
0.62751148770907403411 0.069107555861366290094 0.016396571916060692158
0.32927727964735428428 0.91950325371517433606 0.088024234821922748922
0.04330296975639152724 0.011359406931309376229 0.89551332165316518758

2020 to 709:
1.6602062978811019111 -0.12455265278490989962 -0.018154995139836493422
-0.58755383980194848639 1.1329456581856140455 -0.10060465175595819539
-0.07282705725221054784 -0.008348386143453457498 1.1188320152896576747

Blender AgX inset:
0.856627153315983 0.137318972929847 0.11189821299995
0.0951212405381588 0.761241990602591 0.0767994186031903
0.0482516061458583 0.101439036467562 0.811302368396859

Blender AgX outset:

0.899796955911611 0.11142098895748 0.11142098895748
0.0871996192028351 0.875575586156966 0.0871996192028349
0.013003424885555 0.0130034248855548 0.801379391839686

Blender AgX outset inverse:
1.1271005818144366432 -0.14132976349843826565 -0.14132976349843824772
-0.1106066430966032116 1.1578237022162717623 -0.11060664309660291788
-0.016493938717834568157 -0.01649393871783425265 1.2519364065950402828

combined 709 to 2020 and inset:
0.54490813676363087053 0.14044005884001287035 0.088827411851915368603
0.37377945959812267119 0.75410959864013760045 0.17887712465043811023
0.081384976686407536266 0.10543358536857773485 0.73224999956948382528

combined inverse outset and 2020 to 709:
1.9645509602733325934 -0.29932243390911083839 -0.16436833806080403409
-0.85585845117807513559 1.3264510741502356555 -0.23822464068860595117
-0.10886710826831608324 -0.027084020983874825605 1.402665347143271889

[EaryChow]

I don't know how far ahead Godot has planned in terms of color, but we chose to use Rec.2020 as AgX's formation space was because we believe Blender will one day fix the remaining color management issue and move on to a wider gamut rendering space for the PBR engines to work in. This means when the time comes, the light transport renderer will natively generate out-of-rec.709-gamut values. The decision to use Rec.2020 was for future proofing. Please consider the future of Godot, are you sure Godot will stay not having color management system forever?

[allenwp]

Please consider the future of Godot, are you sure Godot will stay not having color management system forever?

Regardless of this, the implementation of how non-sRGB input will be passed to the tone mapping functions is not yet known. This PR does not preclude handling other colour spaces in the future. All tone mapping, including AgX, will likely need to be reworked (if only for performance reasons) to support other color spaces.

If we knew exactly how we were going to handle other colour spaces in the future, then we could design with that in mind, but right now it's impossible, and honestly a bad idea, to try and take a guess at how that might unfold in the future. Because these decisions and the code is well documented, it will be easy to adjust AgX when it comes time.

A "Custom tone mapping" feature definitely needs to be added to the engine. This will allow users to have more bespoke tone mapping functionality, including LUT support so the Blender AgX LUT can be used without much modification. For the built-in features of a general purpose game engine, it's very important that performance is not sacrificed for a relatively few number of users.

[allenwp]

I also want to clarify to others who looking at this PR that all AgX transformations and calculations are still performed in rec 2020 space, just like in Blender.

The only difference is how negative RGB inputs to the tone mapper are clipped. In many games, there are no negative RGB inputs at all, so this PR makes literally zero difference besides improving performance. The only common cases where a visual difference might be seen is in some very vibrant HDR image files (and only in the Forward+ renderer) and negative lights.

Please post here if you know of other common scenarios that would result in negative RGB values that should be clipped in color space other than sRGB.

[EaryChow]

If we knew exactly how we were going to handle other colour spaces in the future, then we could design with that in mind, but right now it's impossible, and honestly a bad idea, to try and take a guess at how that might unfold in the future.

In general, a good idea is to leave room for a configurable color management system like OCIO in the future, when the time comes, your biggest enemy is "hardcoded sRGB assumptions scattered everywhere accross the entire software throughout the years", and by "remaining color management issues Blender needs to fix", I meant "histroical sRGB hardcoded assumptions in many parts of the Blender". For instance Blender's texture paint and video sequencer is still having weird color management issues that nobody really knows what is going on.

Once OCIO is supported, the Linear RGB rendering space that the light transport PBR engine renders in, can be configured by modifying 1 single line in a txt file. But then you would see a lot of the parts in the software are still assuming the sRGB working space because the devs back then assumed sRGB and thought it was a good idea for "optimization". For instance, Blender's Blackbody emission was hardcoded to clip to the Rec.709 gamut boundry (quite similar to what you are trying to do) before I reported it, and it's since then fixed by having the Blackbody chromaticity data re-generated and dynamically clipped to the current configurable working space.

I just feel the need to warn, if hardcoding sRGB asumption is still a good idea to you then feel free.

[allenwp]

Yes, balancing maintainable code for the future of other colour spaces and performance today on low-spec mobile devices is definitely an important consideration.

Thanks for your thoughts!

[Repiteo]

Thanks!