iop/basecurve: add ACES-like modes, JzAzBz saturation and gamut mapping

[Christian-Bouhon]

This PR modernizes the basecurve module to improve its scene-referred capabilities while ensuring perfect backward compatibility for display-referred workflows.
Key Improvements
1. Scene-Referred Mode (ACES-like/Narkowicz):
◦ New Flow: Implements a perceptual tone mapping pipeline: Signal Analysis -> Matrix-based Looks -> Exposure Normalization -> Perceptual Tone Mapping -> RGB Reconstruction.
◦ Tone Mapping: Added ACES-like and Narkowicz perceptual models for better highlight roll-off and contrast control compared to standard curves.
◦ Looks: Added 3x3 RGB matrix-based "Looks" (Portrait, Sky, etc.) with a mix slider.
2. Color & Gamut Integrity:
◦ Perceptual Saturation (JzAzBz): Improved "UCS saturation balance" to compress highlight saturation while boosting shadows for better visual density.
◦ Hue Correction: New color rotation slider with selective desaturation to eliminate common color drifts (skin tones, blues).
◦ Gamut Securing: Introduced a multi-space gamut engine (sRGB, Adobe RGB, Rec2020) and a specific Anti-Magenta Algorithm (Highlights Protection): Implementation of a selective highlight desaturation logic. When luminance exceeds the threshold (0.8), the compression factor is biased to be 10% stronger on the blue channel compared to red and green.
◦ Goal: This specifically targets the common "magenta shift" in overexposed skies or artificial blue lights, ensuring they desaturate towards white rather than shifting hue.
3. Ergonomics & Compatibility:
◦ Backward Compatibility: Display-referenced processing (classic mode) remains untouched.
◦ UI: Curve graph can now be resized with the mouse.
◦ Introspection: Bumped to version 7.

[Christian-Bouhon]

I just saw that you've been working on the project. Sorry if my last push overwrote some of your maintenance fixes. I've finalised the implementation of the JzAzBz adaptive shoulder extension and CPU/GPU parity. The mathematical logic is now complete on my end.
Thanks for your help,
Christian

[TurboGit]

A first pass on the code.

[Christian-Bouhon]

Thank you for this detailed analysis! I have taken note of all your comments regarding coding style (alignment, one parameter per line, and spaces). I will work on the code this weekend to apply all these corrections and publish the updates.

[TurboGit]

I have finally tested this a bit, some comments:

• I have on the console a bunch of :

34,4786 /home/obry/dev/builds/c-darktable/x86_64-linux-gnu-default/src/src/iop/basecurve.c:3000 gui_init: trying to add widget that already has a parent to box (#1)

• When I change the "color look" it resets to 100% the "look opacity".

• I'm wondering if the target gamut should not be Rec 2020 by default?

[TurboGit]

Also this PR introduces a regression on the CPU vs GPU in the integration tests. The current report is:

Test 0078-basecurve-fusion
      Image mire1.cr2
      CPU & GPU version differ by 9772 pixels

The new report is:

Test 0078-basecurve-fusion
      Image mire1.cr2
      CPU & GPU version differ by 44885 pixels
      CPU & GPU large difference > 10000

So 44885 different pixels in CPU compared to GPU instead of 9772.

[Christian-Bouhon]

So 44885 different pixels in CPU compared to GPU instead of 9772.

I finally found a typo. In color_conversion.h, we have this matrix:

// XYZ to Rec.2020 conversion matrix coefficients
rgb.x = 1.716651f * xyz.x - 0.355671f * xyz.y - 0.253366f * xyz.z;
rgb.y = -0.666684f * xyz.x + 1.616481f * xyz.y + 0.015768f * xyz.z;
rgb.z = 0.017640f * xyz.x - 0.042770f * xyz.y + 0.942103f * xyz.z;

and a small error in the basecurve.c file
" - val[2] = 0.017640f * xyz[0] - 0.042771f * xyz[1] + 0.942103f * xyz[2];"
" + val[2] = 0.017640f * xyz[0] - 0.042770f * xyz[1] + 0.942103f * xyz[2];"

[Christian-Bouhon]

• When I change the "color look" it resets to 100% the "look opacity".

• I'm wondering if the target gamut should not be Rec 2020 by default?

Hello,
For the 100% look, I did it intentionally, so the user can see the effect of the matrix directly and can reduce it if necessary.
Rec 2020 is now the default

Regarding the name without capital letters, I noticed that this was not applied in French. I adapt the fr.po file and create a new pr or link it to this one.
Thank you again for your help.
Christian

[Christian-Bouhon]

After updating my CM_BC work branch from the master branch, as mentioned in the commit, I had to replace these two lines in basecurve.c:
tiling->xalign = 1;
tiling->yalign = 1;
with:
tiling->align = 1;
Do I need to do the same thing on the PR branch?

[TurboGit]

For the 100% look, I did it intentionally, so the user can see the effect of the matrix directly and can reduce it if necessary.

This sounds really wrong to me. We don't do that in any other sliders. If the user needs this it must be done manually, because some other people may be quite annoyed by having to go back to the control for reset to the previously set value.

[TurboGit]

Do I need to do the same thing on the PR branch?

Yes, see 4e6d1b8.

See that only tiling->align exists now:

$ git show 4e6d1b835d0e061b6bc7470e23eb32da760da11f -- src/develop/tiling.h

[wpferguson]

A very quick test with some thoughts

I'd get rid of the kinematics in the names and just use ACES and the other

It would be nice to set a curve in ACES and then switch to the other without the curve resetting so that you can compare the difference.

Do we need a basecurve scene referred workflow? Should that be included in this PR?

Does scene referred basecurve need a tone mapper at the end of the pipe? I think yes because being scene referred the edits can drive the image out of the 0 - 1 range, but maybe not enabled by default?

The following error messages repeat when I open/change image

     0.7723 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2886 gui_init: trying to add widget that already has a parent to box (#1)
     0.7724 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2893 gui_init: trying to add widget that already has a parent to box (#1)
     0.7725 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2908 gui_init: trying to add widget that already has a parent to box (#1)
     0.7726 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2915 gui_init: trying to add widget that already has a parent to box (#1)
     0.7727 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2926 gui_init: trying to add widget that already has a parent to box (#1)
     0.7728 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2938 gui_init: trying to add widget that already has a parent to box (#1)
     0.7729 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2947 gui_init: trying to add widget that already has a parent to box (#1)
     0.7730 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2954 gui_init: trying to add widget that already has a parent to box (#1)
     0.7730 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2965 gui_init: trying to add widget that already has a parent to box (#1)
     0.7731 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2978 gui_init: trying to add widget that already has a parent to box (#1)
     0.7732 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2990 gui_init: trying to add widget that already has a parent to box (#1)
     0.7733 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:2998 gui_init: trying to add widget that already has a parent to box (#1)
     0.7734 /home/bill/src/darktable/test/20320_basecurve/src/iop/basecurve.c:3012 gui_init: trying to add widget that already has a parent to box (#1)

[TurboGit]

You still have a huge amount of error on the console:

    10,9565 /home/obry/dev/builds/c-darktable/x86_64-linux-gnu-default/src/src/iop/basecurve.c:2998 gui_init: trying to add widget that already has a parent to box (#1)
    10,9565 /home/obry/dev/builds/c-darktable/x86_64-linux-gnu-default/src/src/iop/basecurve.c:3012 gui_init: trying to add widget that already has a parent to box (#1)

It looks like you are trying to add a widgets two times. Maybe the box containing the widget and the box itself? (just a wild guess as I haven't looked at the code yet).

And you still are setting "look opacity" to 100% when change "color look". This is not consistent with current UI.

[Christian-Bouhon]

Hello,

It looks like you are trying to add a widgets two times. Maybe the box containing the widget and the box itself? (just a wild guess as I haven't looked at the code yet).

That's right, I found 1 another redundancy.

And you still are setting "look opacity" to 100% when change "color look". This is not consistent with current UI.

The “color looks” are 3x3 matrices that are intended to be used at 100%. I added a mix slider following the same logic as for mask opacity.

Instead of “natural look,” I set neutral (100, 010, 001) as the default, and the opacity slider is grayed out so that the user can easily return to the default value.

Have a nice day.
Christian

[TurboGit]

The “color looks” are 3x3 matrices that are intended to be used at 100%.

I don't see why and your example about blending is exactly the point. When you change the blend mode the opacity is not changed. Again this is not OK and not consistent, I'd prefer to have the "look opacity" to keep it's value and that is the user responsibility to change it if needed.

[Christian-Bouhon]

When you change the blend mode the opacity is not changed. Again this is not OK and not consistent, I'd prefer to have the "look opacity" to keep it's value and that is the user responsibility to change it if needed.

So, if I understand correctly, you want this layout, for example.

1. The default setting is “neutral” with the slider grayed out because it is not used, and if the user moves it, nothing happens.
2. The user chooses a look, e.g., “portrait,” finds the effect too strong, and sets the opacity to 60%.
3. Then, if they decide to change the look, the slider will remain at 60%, and it is up to them to raise it or not.

[TurboGit]

When you change the blend mode the opacity is not changed. Again this is not OK and not consistent, I'd prefer to have the "look opacity" to keep it's value and that is the user responsibility to change it if needed.

So, if I understand correctly, you want this layout, for example.

1. The default setting is “neutral” with the slider grayed out because it is not used, and if the user moves it, nothing happens.

2. The user chooses a look, e.g., “portrait,” finds the effect too strong, and sets the opacity to 60%.

3. Then, if they decide to change the look, the slider will remain at 60%, and it is up to them to raise it or not.

Yes exactly that. Sorry if it was not clear.

[TurboGit]

There is an issue with OpenCL, take the test 0078, it used to give:

Test 0078-basecurve-fusion
      Image mire1.cr2
      CPU & GPU version differ by 9772 pixels
      CPU vs. GPU report :
      ----------------------------------
      Max dE                   : 3.50957
      Avg dE                   : 0.00152
      Std dE                   : 0.02891
      ----------------------------------
      Pixels below avg + 0 std : 99.65 %
      Pixels below avg + 1 std : 99.65 %
      Pixels below avg + 3 std : 99.67 %
      Pixels below avg + 6 std : 99.69 %
      Pixels below avg + 9 std : 99.73 %
      ----------------------------------
      Pixels above tolerance   : 0.00 %
 
      Expected CPU vs. current CPU report :
      ----------------------------------
      Max dE                   : 0.97390
      Avg dE                   : 0.00023
      Std dE                   : 0.00912
      ----------------------------------
      Pixels below avg + 0 std : 99.91 %
      Pixels below avg + 1 std : 99.91 %
      Pixels below avg + 3 std : 99.91 %
      Pixels below avg + 6 std : 99.92 %
      Pixels below avg + 9 std : 99.93 %
      ----------------------------------
      Pixels above tolerance   : 0.00 %
 
  OK

And with this PR we have:

Test 0078-basecurve-fusion
      Image mire1.cr2
      CPU & GPU version differ by 44885 pixels
      CPU & GPU large difference > 10000
      CPU vs. GPU report :
      ----------------------------------
      Max dE                   : 18.79237
      Avg dE                   : 0.01460
      Std dE                   : 0.18322
      ----------------------------------
      Pixels below avg + 0 std : 98.39 %
      Pixels below avg + 1 std : 98.49 %
      Pixels below avg + 3 std : 99.23 %
      Pixels below avg + 6 std : 99.69 %
      Pixels below avg + 9 std : 99.80 %
      ----------------------------------
      Pixels above tolerance   : 0.13 %
 
      Expected CPU vs. current CPU report :
      ----------------------------------
      Max dE                   : 0.97390
      Avg dE                   : 0.00023
      Std dE                   : 0.00912
      ----------------------------------
      Pixels below avg + 0 std : 99.91 %
      Pixels below avg + 1 std : 99.91 %
      Pixels below avg + 3 std : 99.91 %
      Pixels below avg + 6 std : 99.92 %
      Pixels below avg + 9 std : 99.93 %
      ----------------------------------
      Pixels above tolerance   : 0.00 %
 
  OK

As you can see the CPU code gives the same result as previous version but the CPU vs GPU has a larger difference.

Note that I have multiple tests where basecure is used where this happens. So definitely a regression on the OpenCL code.

[Christian-Bouhon]

Hello,,
Thank you all for your feedback. I have taken all requests and comments into consideration for this PR.

I'd get rid of the kinematics in the names and just use ACES and the other

Personally, I would not use the term ACES because I do not use the full curve. Internal comments retain all technical references (Narkowicz 2016, Narkowicz/Filiberto 2021) for greater transparency. I have also implemented an adaptive JzAzBz shoulder curve that acts before it to preserve more detail in highlights and a smoother progression.
I suggest simply using names that describe their function, such as kinematic and dynamic.

Do we need a basecurve scene referred workflow? Should that be included in this PR?

OK, I'll include it in the PR.

Does scene referred basecurve need a tone mapper at the end of the pipe? I think yes because being scene referred the edits can drive the image out of the 0 - 1 range, but maybe not enabled by default?

Thanks for pointing that out. I've corrected the code so that it's always enabled. For your information, it wasn't enabled if the gamut compression function wasn't used.

The following error messages repeat when I open/change image

Fixed bug between the two workflows: legacy and scene.

It would be nice to set a curve in ACES and then switch to the other without the curve resetting so that you can compare the difference.

It is implemented as follows

• The default setting is “neutral” and the slider and opacity slider are hidden
• The user chooses a “color look”; the “look opacity” slider will only be at 100% by default the first time it is used, after which it will retain its last setting

Implemented via last_workflow_mode. Switching between the two curves preserves all slider and curve values for direct comparison. Reset only occurs when switching from display mode to scene-referenced mode.

And you still are setting "look opacity" to 100% when change "color look". This is not consistent with current UI.

It is implemented as follows :
The default setting is “neutral” and the opacity slider are hidden
The user chooses a “color look”; the “look opacity” slider will only be at 100% by default the first time it is used, after which it will retain its last setting.

Here is the latest log without errors.
dt_log_basecurve.txt
If you want to try it without compiling
https://github.com/Christian-Bouhon/darktable/actions/runs/22944579134#artifacts

Greetings from Luberon,
Christian

[TurboGit]

The CPU vs GPU diff is not fixed:

Test 0078-basecurve-fusion
      Image mire1.cr2
      CPU & GPU version differ by 44885 pixels
      CPU & GPU large difference > 10000

Also the "scale for graph" slider is missing.

[Christian-Bouhon]

The CPU vs GPU diff is not fixed:

Test 0078-basecurve-fusion
      Image mire1.cr2
      CPU & GPU version differ by 44885 pixels
      CPU & GPU large difference > 10000

Hello,
The discrepancy of 44,885 pixels appears to be due to a missing line in process_fusion CPU:
sat_adjust *= fminf(L * 4.0f, 1.0f) was present in process_lut and in the OpenCL kernel basecurve_finalize, but absent from process_fusion. This correction has been applied.
In addition, process_fusion and process_lut used different values for alpha (0.5f vs. 0.75f). I unified them to 0.6f to avoid any problems.

Also the "scale for graph" slider is missing.

Indeed, I put it back for the legacy workflow.
Have a nice day,
Christian