Add white, contrast, and future HDR support to the AgX tonemapper.

[allenwp]

Summary

This PR adds agx_white and agx_contrast parameters and HDR output support to the AgX tonemapper. It also optimizes existing tonemappers to reduce unnecessary per-pixel math operations.

SDR output is currently hardcoded; minimal changes are required to support future HDR output. Try out HDR output with this PR here: #110701.

1. Before this PR, AgX is not usable with Mobile renderer and HDR 2D disabled. This PR fixes this by adding an agx_white parameter that can be set as low as 2.0.
   • Fixes AgX has low dynamic range output with Mobile renderer #101558
2. Before this PR, AgX has low shader performance due to converting to and from log2 space when applying the curve. This PR improves performance by applying the curve directly to linear values instead.
3. This PR further improves shader performance for all tonemappers by moving math calculations from the shader to the CPU when these calculations are the same for all pixels on the screen.
4. Before this PR, AgX uses a low quality and low performance polynomial approximation curve that produces slightly lower contrast and lower saturation than Blender's AgX. This PR improves visual quality by replacing the polynomial approximation with a high quality curve that better matches Blender's AgX curve.
   • Fixes AgX: Oscillations in sigmoid approximation #102416
5. Before this PR, AgX used a hardcoded contrast curve that was designed for Blender, where it would be used in tandem with high quality colour grading effects that were applied before the tonemapper. This PR adapts the design from Blender to be better suited for a real time game engine by exposing an agx_contrast parameter that has no performance cost and produces higher quality results than the existing contrast adjustment.
   • Closes Add HDR support and configuration parameters to AgX tonemapper godot-proposals#12317
6. Before this PR, AgX could not be used with the upcoming HDR output. This PR adds support for HDR output with this tonemapper.
   • Closes Add HDR support and configuration parameters to AgX tonemapper godot-proposals#12317

Note: unlike white, agx_white is limited to a minimum of 2.0 and defaults to 16.29 to match Blender's configuration and the existing behaviour in Godot. When using a RGB10A2 render buffer, agx_white will be ignored and a value of 2.0 will be used instead to ensure good behavior on the Mobile renderer.

This PR uses a new bespoke tonemapping curve in place of the AgX curve because the AgX curve is not suitable for real time use and is also not suitable for variable dynamic range output (EDR). The new tonemapping curve very closely matches the existing Blender AgX curve for dark-to-mid values with the default agx_contrast and agx_white. There is deviation from Blender and Godot 4.4 in the brightest input values, but this is required to ensure stable behaviour across all variable dynamic ranges.

I have also re-done the math on the combined colour space and Blender AgX matrices to correct an issue where colourless values were having colour added to them by the matrices. I traced this back to the Rec. 709 to/from Rec. 2020 matrices that I had calculated based on the Blender OCIO files. The new matrices were taken from RGB Colourspace Transformation Matrix app of Colour Science, which is clearly where I should have sourced them to begin with.

New AgX contrast parameter

The new AgX contrast is applied during the tonemapping process, so it produces higher quality results than Adjustments contrast. The AgX contrast is specifically better at maintaining the hue of bright colours than Adjustments contrast which is prone to hue shifts towards yellow, cyan, and magenta from channels hard-clipping.

AgX default contrast	AgX contrast of 1.53

The following compares AgX contrast with Adjustments contrast in Forward+ with HDR 2D disabled:

Review considerations

• I don't like the public scripting API for agx_white; it would be much nicer to have this be the same white as all of the other tonemappers, but I don't know any other way to get the AgX white to default to 16.29 to match existing behaviour in Godot 4.4 and Blender.
• Tonemapping parameters are calculated in EnvironmentStorage, which doesn't seem like the best place for this, but logically is the nicest spot of the current classes. With future HDR output, the parameters will change every frame based on max output value, so they are calculated on demand instead of when tonemap parameters changed by the user.
• agx_white could go as low as 1.0 instead of 2.0, but as pointed out by the primary Blender AgX author, it really doesn't perform well when white is set to 1.0.

Performance

Using the visual profiler, Calinou's tonemapping test scene, ~4K window, and an NVIDIA 980 Ti on Windows 11 with Forward+, I recorded the following performance stats:

Tonemapper	b89c47b	this PR
linear	0.83	0.84
reinhard	0.94	0.95
filmic	1.03	1.00
ACES	1.07	1.07
AgX	1.2	1.14

Using the visual profiler, Calinou's tonemapping test scene, 1/2 of a ~4K window, and Intel Graphics 770 on Windows 11 with Forward+, I recorded the following performance stats:

(Note: these were very noisy, so I recorded the smallest number that kept occurring in the profiler.)

Tonemapper	b89c47b	this PR
linear	4.05	4.1
reinhard	4.6	4.5
filmic	4.8	4.7
ACES	5.25	5.1
AgX	5.7	5.5

Compatibility with Godot 4.4

One of the goals of this PR is to maintain compatibility with existing Godot 4.4 and 4.5 projects. For this reason, it is important that the new curve produces similar visuals to the old AgX approximation. The following comparison demonstrates that visual appearance with the new curve is similar to the old appearance in Godot 4.4:

AgX Blender Curve (Reference)	AgX New Curve (this PR) white = 16.0, contrast = 1.25	AgX Approximation (Godot 4.4.1)
(Not measured)

Other notes

Here’s the Mathematica for the matrix maths and verification of their stability:

LinearSRGBToRec2020 =
  SetPrecision[{{0.627403895934699,0.069097289358232,0.016391438875150},
    {0.329283038377884,0.919540395075458,0.088013307877226},
    {0.043313065687417,0.011362315566309,0.895595253247624}},15];
AgXInset =
  SetPrecision[{{0.856627153315983,0.137318972929847,0.11189821299995},
    {0.0951212405381588,0.761241990602591,0.0767994186031903},
    {0.0482516061458583,0.101439036467562,0.811302368396859}},15];
Print["Combined sRGB to 2020 and AgX inset:"]
combinedInset=SetPrecision[LinearSRGBToRec2020.AgXInset,15]

AgXOutset =
  SetPrecision[{{0.899796955911611,0.11142098895748,0.11142098895748},
    {0.0871996192028351,0.875575586156966,0.0871996192028349},
    {0.013003424885555,0.0130034248855548,0.801379391839686}},15];
Rec2020ToLinearSRGB =
  SetPrecision[{{1.660491002108435,-0.124550474521591,-0.018150763354905},
    {-0.587641138788550,1.132899897125961,-0.100578898008007},
    {-0.072849863319885,-0.008349422604369,1.118729661362913}},16];
Print["Combined AgX outset and 2020 to sRGB:"]
combinedOutset=SetPrecision[Inverse[AgXOutset].Rec2020ToLinearSRGB,15]

Print["Sanity check of combinedInset:"]
SetPrecision[{1,1,1}.combinedInset, 20]

Print["Sanity check of combinedOutset:"]
SetPrecision[{1,1,1}.combinedOutset, 20]

Print["Sanity check of round trip:"]
SetPrecision[({1,1,1}.combinedInset).combinedOutset, 20]

"Combined sRGB to 2020 and AgX inset:"
  {{0.544814746488245,0.140416948464053,0.0888104196149096},
   {0.373787398372697,0.754137554567394,0.178871756420858},
   {0.0813978551390581,0.105445496968552,0.732317823964232}}
"Combined AgX outset and 2020 to sRGB:"
  {{1.96488741169489,-0.299313364904742,-0.164352742528393},
   {-0.855988495690215,1.32639796461980,-0.238183969428088},
   {-0.108898916004672,-0.0270845997150571,1.40253671195648}}
"Sanity check of combinedInset:"
  {1.0000000000000000682,0.9999999999999992744,0.9999999999999992142}
"Sanity check of combinedOutset:"
  {0.9999999999999985100,1.0000000000000003129,1.0000000000000001018}
"Sanity check of round trip:"
  {0.9999999999999993506,0.9999999999999993513,0.9999999999999991612}

Related PRs

• Supersedes Add support for white parameter to AgX tonemapper (Mk. 2) #102425
• Supersedes Improve AgX tonemapping curve. #102435
• Supersedes Add support for white parameter to AgX tonemapper #101559
• Related to [WIP Prototype] Add HDR support to tonemappers #106696

Further explanation

• Add HDR support and configuration parameters to AgX tonemapper godot-proposals#12317
• Explanation of why clipping to output range is required

Thanks

Thanks to Troy Sobotka for the AgX generator, EaryChow for the AgX configuration, and Stephen Hill for helping me through the process of using Mathematica to do the fitting to the AgX Blender tone curve! Also thanks to Timothy Lottes, Erik Reinhard, and John Hable for sharing their work, which was foundational for the new variable dynamic range tonemapping curve.

[Calinou]

Tested locally, it works as expected. Code and documentation look good to me.

Testing project: tonemap_color_correction.zip
Also in a branch: https://github.com/Calinou/godot-demo-projects/tree/tonemap-improve-agx

Before	After

Whitepoint 16.0

Whitepoint 0.1

There seems to be a slight visual difference between 0.1 and 2.0. Should the value be clamped to avoid this? Either way, the whitepoint setting seems to make little difference in AgX compared to other tonemappers. Here, it only affects the bright glowing cube/sphere on the left.

[allenwp]

There seems to be a slight visual difference between 0.1 and 2.0. Should the value be clamped to avoid this?

There should be no difference between a white of 0.1 and 2.0 because it is clipped to 2.0 in environment_storage.cpp:

float awp_high_clip = MAX(p_white, 2.0);

I can't reproduce a difference in result between a white of 0.1 and 2.0. Here's the difference between a 0.1 and 2.0 screenshot, with a Photoshop "Levels" filter applied to add contrast to any different pixels:

And here are the source screenshots:

0.1	2.0

Maybe you had white set to something like 2.04 without realizing it?

[Calinou]

Maybe you had white set to something like 2.04 without realizing it?

Yes, that might be it or just an optical illusion.

[allenwp]

I tested this on a Google Pixel 1 with Android version 10 (Oct 6, 2019) and everything behaved as expected. (No crash issues or unexpected behaviour with the extra bytes in the shader push constants/binary objects.)

Compatibility	Mobile	Forward+ Export (actually Mobile)

I did notice that this phone did not support Forward+, so even when I exported with Forward+ it would still use the Mobile rendering method. This behaviour was not related to this PR because it happened with b89c47b as well.

[Calinou]

I did notice that this phone did not support Forward+, so even when I exported with Forward+ it would still use the Mobile rendering method. This behaviour was not related to this PR because it happened with b89c47b as well.**

Check the rendering method in the project settings (by default, the mobile override ensures Forward+ isn't used on mobile due to poor performance).

[allenwp]

I did notice that this phone did not support Forward+, so even when I exported with Forward+ it would still use the Mobile rendering method. This behaviour was not related to this PR because it happened with b89c47b as well.**

Check the rendering method in the project settings (by default, the mobile override ensures Forward+ isn't used on mobile due to poor performance).

Ah, thanks for sharing this! And this default is for good reason: this phone doesn't support Forward+ independent of this PR. (Forward+ always fails on this phone with ERROR: vkCreateGraphicsPipelines failed with error 5.)

[allenwp]

tldr; non-AgX tonemappers are behaving the same with this PR as they are before this PR.

I did another sanity check across all tonemappers and all three rendering methods on my NVIDIA 980 Ti desktop with a white of 3.0. I found that all non-AgX tonemappers are rendering identically except for ACES. ACES has a barely measurable difference, which I suspect is to do with different rounding error between the CPU calculation vs. the previous shader calculation. The quantization / rounding error difference presents itself as 1 bit of a single R, G, or B nonlinear sRGB encoded channel:

b89c47b	This PR	Difference

So although this is measurable for some specific colour input, it's entirely invisible to a human. As shown in the original PR comment, visible differences are seen in the AgX tonemapper.

[allenwp]

tldr; This PR may introduce a change in behaviour for effects that depend on values above 1.0 being returned from the tonemapper function.

One thing that I did not discuss in this PR is that the tonemapper output with this new curve is clipped to a maximum of 1.0 (or output_max_value for variable / extended dynamic range). I'll try and explain why this clipping is necessary:

Without this clipping, the "allenwp tonemapping curve" has a linear behaviour above the output max value. This is because the shoulder is a Reinhard-like shoulder, and this is the normal and expected behaviour for the Reinhard tonemapper. While this is fine in some cases, it is at odds with the intentional mismatch between inset and outset chromaticity coordinate rotation of the Blender AgX configuration:

Chromaticities working = InsetPrimaries(REC2020_PRI, 0.32965205, 0.28051336, 0.12475368, 2.13976149, -1.22827335, -3.05174246);
Chromaticities destination = InsetPrimaries(REC2020_PRI, 0.32317438, 0.28325605, 0.0374326, 0.,0.,0.);

This cyan strip on very large input values appears because of the mismatch between the rotation in the inset and rotation in the outset matrices (the last three arguments of the InsetPrimaries function. This mismatch is intentional to address the shift towards magenta, as described here.

Interestingly, if I use a tonemapping curve that has non-linear curve above the maximum output value, this cyan strip does not occur:

So I tried for weeks to construct a curve that would maintain the curved shoulder above the output max value and eventually decided that it was not feasible. The main issues that occurred were:

1. Non-piecewise curves would not maintain stable behaviour in the dark-to-mid values when the maximum output value was changed. This was a deal-breaker for variable Extended Dynamic Range (EDR) output, which is a hard requirement. Of course, it's possible there are some maths that I don't fully grasp, but the typical nature of a non-piecewise function is that a change to one part (such as maximum output value) will influence the rest of the curve, even if constraints are applied to it.
2. When I tried to use a true piecewise curve that was 100% stable in the dark-to-mid values with a power function for the shoulder, the mathematical constraints to maintain slope at the crossover point would cause the power function shoulder to breakdown badly with small white values. (Additionally, I'm not sure if the performance cost of the additional pow function call would result in a net gain or loss after factoring in not needing the max function call for clipping.)

So my solution was to simply clip the shoulder values to the maximum output value before applying the mismatching outset matrix. This seemed quite reasonable, because conceptually the purpose of a tonemapper is to place values into the [0, output_max_value] range.

[clayjohn]

To be honest, I'm a little uncomfortable with the huge amount of complexity this adds for little to no noticeable impact. I understand that this is theoretically better than the current state. But I struggle to believe that any user will actually care about the difference.

The comparison photos all look either the same, or extremely similar. And even with the ones that are different, its impossible to say which version is preferable. So I worry that this PR may just be chasing some theoretical ideal without having a strong user-focused reason to be merged.

I suspect the only difference users will notice is the fact that suddenly AgX looks bad because the white point is now 1.0 instead of 16.29.

[Jordyfel]

The original proposal for AgX had a lot of people excited for the "AGX Punchy" version, and I am one of them. This higher quality contrast adjustment would be extremely valuable for cartoony artstyle games. For example

[allenwp]

We had a long discussion about the addition of the white and contrast parameters to the existing AgX tonemapper in the rendering meeting. Here are some of the things that were discussed:

Contrast parameter

I have changed the contrast parameter for AgX to be more clearly only for this tonemapper by renaming it to agx_contrast and separating it from RenderingServer::environment_set_tonemap.

Here are other notes about this decision:

1. clayjohn believes that AgX should not have a contrast parameter because we already have a contrast parameter in Environment. This is an important point regarding ease of use to Godot users and simplicity of the engine, especially to new users:
   • Which should I use when I want to change the contrast? Well, normally just use the contrast paramter. But are you using AgX? Oh, well, in that case use AgX contrast. But then if you might switch away from AgX in the future, then you should just use contrast so it's stable regardless of tonemapper.
2. Because adjustments are applied after tonemapping in Godot 4, the existing contrast adjustment behaves poorly compared to a contrast adjustment that is integrated into the tonemapping curve.
3. All game tonemapping literature that I have read says that allowing the contrast curve of the tonemapper to be adjustable is an important feature.
4. Experimentation confirms that hue shift from channels hard-clipping occurs when using the adjustments contrast control but not with the AgX contrast control. (See image in PR description.)

White parameter

We figured it might be best to introduce an entirely new agx_white parameter instead of using the existing white parameter for these reasons:

1. Introducing a new agx_white will allow us to default AgX white to 16.29 instead of 1.0, which:
   • Matches behaviour in Godot 4.4 and 4.5.
   • Matches behaviour in Blender.
   • Produces better quality visuals with default values.
2. Makes it easier to understand that agx_white behaves a bit differently than white:
   • It is limited to 2.0 instead of 1.0.
   • It is hardcoded to 2.0 on the mobile renderer when using an RGB10A2 scene buffer format.

[ettiSurreal]

Because adjustments are applied after tonemapping in Godot 4, the existing contrast adjustment behaves poorly compared to a contrast adjustment that is integrated into the tonemapping curve.
All game tonemapping literature that I have read says that allowing the contrast curve of the tonemapper to be adjustable is an important feature.

I agree with that. From what I know a simple but effective way to add contrast control is to just take it to the power of something.

For example there could be a simple tonemap_power (similar to exposure) parameter that's shared between all the tonemappers that just takes the pre-tonemapped linear color to the power of x (x being the tonemap_power value).
(or a tonemap_gamma parameter which is the same thing but you take it to the power of 1/x)
(also I'm pretty sure the order of operations is exposure -> power/gamma and not the other way around)

[allenwp]

The AgX contrast parameter does control the power function exponent, similar to what you described -- but importantly, it is anchored on middle grey (18% linear) instead of 1.0 (100% linear).

By applying a power function with the anchor at 1.0, the adjustment will have a significant impact on brightness, which is different than how AgX behaves.

But in the end, a tonemapping curve like the ones in Godot is a contrast curve, so applying another contrast curve before or after the tonemapper is a little weird anyway when performance is a concern.

[allenwp]

Rebased to work with new behaviour in Mobile HDR 2D.

[allenwp]

Rebased to include new mobile/forward+ tonemap shader split. Also improved some comments and the documentation text that describes how agx_white behaves differently when hdr 2d is disabled on the mobile renderer.

[Calinou]

Tested locally, it works as expected on all renderers (including HDR 2D enabled). Code and documentation look good to me.

[clayjohn]

Seems fine to me.

I agree that adding an agx_white property is awkward and makes the interface more bloated. But we can't really avoid it if we want to have a configurable white parameter for AgX. This sort of bloat is the reason I was against merging AgX in the first place. But now that we have AgX merged our choices are limited and we need this for proper HDR support in the future.

Similarly, exposing a new agx_contrast is unfortunate bloat to the interface and causes an awkward naming conflict with the existing contrast parameter. Again, there is no way around it if it really is so important to have, so the best we can do is prefix it with agx_ to make it clear that it is a different form of contrast and hide it from users who aren't using AgX.

Overall, I think this PR strikes the best balance we can achieve given that our goals ultimately require adding some amount of bloat to the interface.

[akien-mga]

Needs rebase, and this should be good to merge before feature freeze.

[Repiteo]

Thanks!