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]

File to be removed !

[Christian-Bouhon]

Hello,
Thank you for your feedback!
I have removed src/views/view.c from the PR, it was included by mistake. Sorry about that!
I worked on this project over the weekend to correct all the comments I received, and today I ran tests to check that everything is OK. I will do more tomorrow or later this week. You can see the results on my work branch, BC_CM. I also compiled this branch with the official script adapted to my branch to check that everything works at this level. https://github.com/Christian-Bouhon/darktable/actions/runs/22862606858#artifacts

I have a question before updating the PR. Besides the core files (basecurve.c, basecurve.cl, color_conversion.h), I also modified a few other files to make the scene-referred workflow work properly:

• develop.c and presets.c — to allow basecurve auto-apply presets in scene-referred
• utility.c — to add the new workflow mode to darktable's workflow detection
• modulegroups.c — to show basecurve parameters in the quick access panel
• darktableconfig.xml.in — to add the new entry in the preferences

Would it be better to include everything in this PR, or would you prefer two separate PRs?
I am still learning, so any advice is welcome. Thank you!
Salutations du Lubéron,
Christian

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