Merge scene-referred contrast module

[jandren]

I volunteered over at pixls to make a new PR from this closed PR:
#20447

But I would like to discuss a plan for how to do it before putting any time into coding.
First some references to what has happened.
Initial simple module with one frequency scale by @wilecoyote2015: https://discuss.pixls.us/t/experiments-with-a-scene-referred-local-contrast-module-proof-of-concept/55402
Discussion quickly turned into a lets-do-more situation which eventually led to what Christian proposed for his PR and documented in https://discuss.pixls.us/t/contrast-management-rgb-a-new-scene-referred-approach-poc/56004

TLDR
First we had a module that increased the local contrast (high pass) defined around a edge aware low pass filter (EIGF)
Then the module evolved into 5 copies of this filter, plus noise rejection, a luma dependent effect, and a chroma dependent effect.

Proposal for how to merge this

1. Begin with a basic version of the module, one eigf filter and a noise suppression method.
2. Add OpenCL support
3. Add more controls such as luma and chroma dependence
4. After more user input and testing, evaluate how to potentially include more filter instances inside the module which would add enough value to users workflow compared to just instancing the module a couple of times.

How does this sound to the maintainer team?
Adding everything at once means a large review effort and also makes it harder for me to take responsibility for the merged work.

[TurboGit]

First of all a big thanks to you @jandren for stepped-in !

As I have not followed the whole PX discussion can you answer some questions to be sure I have the big picture about this module.

1. How does it compare to the current "contrast equalizer"
   • One is based on wavelet the new module use EIGF and so better avoid halos?
   • more to this?
2. The current "contrast equalizer" is based on Lab, the new module will be RGB and so avoid some color-space conversions, right?
3. In the original PR there was multiple way to handling contrast one what a pyramidal UX/UI, is that part of your first point above? If not can you clarify?
4. Anything else we should know about the module internals/limitations?

(I think all important data should be here in GitHub)

I'm ok with the steps above, but still want to better understand where is our ultimate goal.

[MStraeten]

I'd suggest to also take into account the color channel wise option you described in https://discuss.pixls.us/t/experiments-with-a-scene-referred-local-contrast-module-proof-of-concept/55402/270
That adds a feature not available yet in other modules (e.g. contrast equalizer)

[Christian-Bouhon]

Hello,

Thank you for your help.
If I understand you correctly, you’d like to revert to a module that only handles local contrast and not integrate the module as I designed it? Personally, I don’t think one precludes the other.

Regarding the EIGF, the “tone equalizer” module does not use OpenCL. I think we should port the eigf.c file to OpenCL so that both modules can benefit from it.

Would you like me to continue working on the module or not ? → array

Greetings from the Luberon,
Christian

[jandren]

No worries Christian!

Not for reverting it! I just want to follow software development best practices and split the merging into smaller PRs, this makes it a lot easier to review the code for the maintainers. That is at least my experience from working as a software developer. See for example this article if you want to learn more! https://dev.to/budiwidhiyanto/the-art-of-pull-requests-a-developers-guide-to-smooth-code-reviews-38bk (Point 3. Keep the PR Focused and Challenge 1: Pull Requests with Too Many Changes) This is even more important for code which has been developed using AI coding assistants as the amount of code can become overwhelming to review.

@MStraeten yes that should be considered, probably as part of step 3, which in reality might become multiple PRs.

@TurboGit I will give some quick answers now and come back with a more detailed one towards next weekend.

1. Its probably most similar to the local contrast module when using bilateral blur. Difference is that is acts on scene referred rgb ratio scaling instead of LAB and that it uses a different low pass filter to create the unsharp mask, EIGF. Its not perfect at avoiding halos but a lot of users has been quite satisfied with the initial results.
2. Applying contrast like exposure by scaling the rgb ratio (vector magnitude) is good at not creating any artifacts or color problems. Works well together with the tone mappers that come afterwards.
3. I want to wait with the pyramidal part as it adds a lot of both code and UI complexity. There is no backwards compatibility problems with only adding the "local contrast" slider first and then extending with more levels as needed (they are applied in parallel and makes no assumption about the other levels and can be added without conflict).
4. TBD with a figure or two.

[Christian-Bouhon]

Hello,

This week, I used the module on about a hundred photos, and I can tell you that I didn’t encounter any issues, it works very well. It’s sometimes a bit slow during export, which is due to the lack of OpenCL.

I remain convinced that it’s possible to migrate the module as-is. Splitting it into multiple PRs would require introspections and make the code heavier. That said, as I’ve already mentioned, I’m not a professional in this field; I’m working with my retired-person’s logic.

The request for the 5 scales came up during discussions on the forum.
https://discuss.pixls.us/t/experiments-with-a-scene-referred-local-contrast-module-proof-of-concept/55402/60
It was then approved by Boris, an experienced user.
https://discuss.pixls.us/t/experiments-with-a-scene-referred-local-contrast-module-proof-of-concept/55402/162

I also think there is room for two modules; see this post,
https://discuss.pixls.us/t/experiments-with-a-scene-referred-local-contrast-module-proof-of-concept/55402/382

Since I’ve addressed all the feedback received on the first PR, I propose resubmitting a pull request and letting the darktable team decide whether to accept it or not.
Just to clarify, one PR at a time, I want to avoid the same issues.
What do you think? It’s up to you.

Greetings from the Luberon 🌿
Christian

[jandren]

For reference @lefth collected the relevant commits for the proposed module:
https://github.com/lefth/darktable/tree/ContrastManager
#20447 (comment)

There is a lot going on in this module and I think it deserves good feedback also from the devs when being integrated. I hope I can help with that. Still working on a figure or two to connect it all and base the way forward on.

[lefth]

Thanks, @jandren. Btw, in that tree I also did some refactoring that the devs asked for in the code review. The array commit might need to be reverted if you want to add one detail scale before the others.

[jandren]

@Christian-Bouhon I'm trying to make sense of the CSF adaption, could you help me out?
I tried plotting it as you only apply it on a per pixel basis but didn't gave me much.
Settings CSF to 0% inactivates the global contrast term btw.

I'm working on some flowcharts so that we can come forward with the next steps.

[Christian-Bouhon]

Hello,
First of all, I’d like to thank you all for your help with this project,

You're right: the CSF slider acts as a “switch” or a power control (master mix) for the overall contrast.
The two sliders are linked to each other

const float global_term = (gain_global - 1.0f) * d->csf_adaptation * csf_weight * log_lum * w_global;

As you can see, d->csf_adaptation (the CSF slider) is used as a direct multiplier for the entire calculation.

Here is the current CSF logic:

1. Global contrast weighting

multiplier for the global contrast (global_scale). The code first calculates a Gaussian weight (csf_weight) centered on the mid-gray (0.1845 on a linear scale) :

const float log_lum = log2f(lum_pixel / 0.1845f); const float csf_weight = expf(-powf(log_lum, 2.0f) / 12.5f);

This weighting is greatest for midtones and gradually decreases toward deep shadows and extreme highlights (with a standard deviation of approximately 2.5 EV).

const float global_term = (gain_global - 1.0f) * d->csf_adaptation * csf_weight * log_lum * w_global;

The more you move this slider to the right, the more the overall contrast effect will be concentrated on the midtones.

2. Saturation Boost (secondary effect)

The slider has a dual function. When it exceeds 50% (0.5f), it triggers a gradual increase in saturation, which is also weighted by the sensitivity curve (csf_weight):

if(d->csf_adaptation > 0.5f)
{
    const float saturation_boost = 1.0f + (d->csf_adaptation - 0.5f) * csf_weight * 0.1f;
    for_each_channel(c)
        out[4 * k + c] *= saturation_boost;
}

This helps restore vibrancy to colors in areas where contrast is increased, thereby preventing a washed-out or grayish appearance during significant adjustments.

3. Influence on color contrast

Finally, if you use the “colorful contrast” setting (colorful_contrast), the CSF weight is also used to modulate the separation between the red and blue channels. This ensures that color accentuation (warm tones vs. cool tones) follows the same logic of visual perception as brightness.

4. Proposal
   Now, following your request, you’ve given me a new idea, and I thank you for it.
   I propose another approach to the master mix using interpolation (or transparency) rather than direct multiplication; we transform the slider’s behavior: instead of being an “all-or-nothing” switch for overall contrast, it becomes a perceptual adjustment.
   At 0%, the overall contrast is applied mathematically in a linear manner. At 100%, it strictly follows the visual sensitivity curve (CSF)

Edit : FYI, I tried the suggestion above and it works great. Thanks for your help.

Christian-Bouhon/libre-dt-lab@b2d29e3

Greetings from the Luberon 🌿
Christian

[jandren]

Ok! So its a per-pixel effect then, good to know. Your update might be closer to what I expect of a modifier slider (0% - normal contrast, 100% - adjusted contrast).

Anyway, finished what I hope is a accurate representation of the module. I have color coded my proposed steps for merging as:

1. Green, get the core functionality in, one filter with good behavior in the presence of noise.
2. Yellow, modifiers, I think this step might need some more work and tweaks from playing around with the module and reading the code, but easier as a separate step.
3. Blue, additional filter instances.

Some notes:

• The luminance is currently calculated for every filter. Would be enough with once and then copy that to all buffers.
• For knowledge to the maintainers (@TurboGit et al), the filters are parallel instances of a eigf high pass filter with filter size progression similar to a pyramid filter, but it is not a pyramid filter as they have no band pass behavior like what is used in fex the contrast equalizer. This is a advantage for the merging process as we can add a arbitrary number of filters down the line. A question from me is how this plays with core darktable design principles as it could also be done with multiple module instances.

[Donatzsky]

A question from me is how this plays with core darktable design principles as it could also be done with multiple module instances.

Requiring multiple instances would be a usability nightmare. Defaults need to be sensible and I just don't see how that could be the case when each instance only concerns itself with a single details scale. They would all start with the exact same parameters and then the user would have to fiddle around to get the scale dialed in, including the overlap that seems to be an important part of @Christian-Bouhon's design. No thanks.

A much better solution would to let the user set the number of scales, along with high and low anchors/limits. The default would be as originally envisioned, but with added flexibility when wanted. The UI should probably use the EQ style, in that case.