IOP Color EQ : Add interactive editing mode for color adjustments in image

[Christian-Bouhon]

Hello,

Here’s an old idea that’s resurfaced in my mind: an adaptation of the color equalizer module inspired by the tone equalizer module.

I’ve implemented the ability to correct a color by selecting it directly on the photo, on the fly. Needless to say, the AI has been a big help.

New Features

• mouse_moved() – reads the UCS hue from the preview pipe and converts it to GUI degrees
• mouse_leave() – invalidates the hue when the mouse leaves the image
• gui_post_expose() – draws a color circle at the cursor (black + RGB fill)
• scrolled() – scroll wheel on the image → applies a Gaussian weighting (σ=35°) to the nodes of the active channel, locks zooming with return 1
• _area_scrolled_callback() rewritten: dual mode (classic if no picker, Gaussian otherwise)
• _get_param_ptr() – direct access to parameters via offsetof(), used by the Gaussian loop

Here is the forum discussion on pixls.us; I haven’t changed the user interface,

Greetings from the Luberon,
Christian

[jenshannoschwalm]

Hmm, just did a first test.

This does work only when used with "default scheduling profile" (this is very likely just one trigger for the issue), otherwise code after

  if(!g->cursor_valid) return 0; // no valid hue: normal zoom

is never reached.

So how is g->cursor_valid is set to TRUE?

1. You test for preview dimensions, this seems to be not correct to me as we can easily modify data effecting hue by any shortcut means in other modules before CE. I think you have to protect data via the color equalizer piece hash. Or the valid gboolean could be the hash and tested for identity.
2. So there remains the problems of preview pipe data not available - current dt pixelpipe code does not ensure that. Not sure how to treat this "best way".

[Christian-Bouhon]

Hmm, I think you're right. 😉
Thanks for your analysis.
I think I have a solution to the problem you brought up; I'll test it and get back to you.

[Christian-Bouhon]

Hello,
Thank you very much for your careful review, your remarks were spot on and led to real improvements.
Here is a summary of what was addressed:

1. preview_pipe_hash is stored in process() via dt_dev_pixelpipe_piece_hash(piece, roi, TRUE) right after filling the hue buffer (same pattern as toneequal.c).
2. mouse_moved() now sets cursor_valid = TRUE only when the hash matches — exactly as you suggested. The GUI indicator (gui_post_expose) and the graph's Gaussian mode (_area_scrolled_callback) gate on cursor_valid, so they never see stale data.
3. scrolled() reads the hue directly from the cached buffer (under the critical section) instead of calling mouse_moved(), to avoid falling through to image zoom when the hash doesn't match. Scroll-based adjustment tolerates slightly stale data gracefully.
4. _area_scrolled_callback() checks preview_hue_buf != NULL (buffer existence) rather than cursor_valid for the Gaussian/single-node decision, so the graph remains usable even when the hash is stale.
5. Hash is reset to DT_INVALID_HASH in gui_init(), gui_focus() (on focus loss), and the critical section protects the buffer read against the use-after-free race.

Bottom line: non-default scheduling profiles are now safe, image zoom on scroll is never triggered, and the graph keeps its node-weighted behavior.

Thanks again for taking the time, this is a much more solid implementation because of your feedback.
Greetings from the Luberon,
Christian

[jenshannoschwalm]

I did some fresh testing, i think it works now as you intended! The way you change the nodes seems good to me too.

Don't take me wrong but personally i don't think the strategy "if mouse is in main canvas use scrolling for CE changes" is a good one (knowing hat TE also does it like this ...) . One simple reason is, i miss zooming in/out using the scroll wheel a lot.

I would prefer it to be bound to the picker. That would also mean,

1. the "position-for-color-change" would follow any drag, rotate, zoom ...
2. would correspond to positioning of the hue hint
3. just take hue data from picker data instead of the hue buffer.

[TurboGit]

One simple reason is, i miss zooming in/out using the scroll wheel a lot.

You can press 'a' while zooming/panning to pass the events to the canvas. But you certainly know that.

[Christian-Bouhon]

Hello,
Thank you for your feedback. It is certainly possible to add a second button to enable or disable the mouse's interactive mode.
Personally, I prefer to keep the same mode as in TE, but if you'd like, I can add this feature.

Greetings from the Luberon, 🌿
Christian

[jenshannoschwalm]

I dont have a strong opinion. I thought, if the picker is active, that could be checked and your modify-nodes from hue could step in. No?

[jenshannoschwalm]

There is some irritating effect.

1. Use an image with a deep-red part and drastically change hue for visibility until it gets greenish.
2. Then use CE at the part origin-red-now-greenish.
   a) Your algo finds red and corrects the nodes accordingly but the color circle is wrong/misleading (greenish)
   b) when using the picker the little dot is also wrongly colored but at least the "marker" in the CE module part is right.

I think we need some marker as it would be very surprising if you hover over green but change reds. Another argument to go via the picker.

The TE has a much easier job here going for blurred luminance.

[Christian-Bouhon]

Hello,

A new button has been added to allow direct adjustments to the image when it is activated.
The module’s two color pickers automatically exclude each other: if one is activated, the other is immediately deactivated, thanks to a complete reset of the color picker’s internal state (dt_iop_color_picker_reset) and a delay (g_idle_add) in the interactive button’s state change to prevent re-entry into the GTK signal chain.

If this doesn’t work for you, I can always revert the changes with git reset --hard HEAD~1. 😉

Friendly regards,
Christian

[jenshannoschwalm]

Let me show you an example:

1. Base image

2. Modify by using color equalizer (but you could choose any other module changing hue for this ...)

3. Now you see two greenish parts of the image and you want the lower part to shift to yellow. The old picker at least told you

we are processing "red" at this location. (Yes the little color dot is wrong too :-)

Maybe it's just too hot today and i should stop thinking here but it seems to me we need some sort of information about the "color-of-CE-input".
(There are some minor issues with picker inter-dependencies btw)

[TurboGit]

This does not work on my side. I have clicked on the picker with the +:

When I hover over the image and use the mouse wheel it just zoom as if the picker was not selected.

BTW, we need to work on consistency here. The Tone Equalizer does not require a picker. Having different way of interacting with a module for the same kind of action is not good to me.

[Christian-Bouhon]

Hello,

You're right, it isn't very stable. I've reverted to the previous version, which is consistent with TE.
For this kind of test, I use a test pattern I created in Inkscape; each square has a solid color because, with a gradient, it's difficult to match the exact same color.
Here's a short video.

2026-06-27.13-34-52.mp4

A little fresh air from the Luberon, 🍷
Christian

[jenshannoschwalm]

Please shar your test-image ...

[jenshannoschwalm]

@Christian-Bouhon after commenting in #20626 about dt caching i thought again about your code here (and why i don't like the way of caching at all).

It simply should not be necessary in the vast majority of use cases as a) all you do here only happens if the CE moduly is the active module and b) we already make sure the input data of the active module is available in the pipe cache for CPU and OpenCL code.

So we just need some extra pixelpipe stuff to get access to preview pipe input (and possibly output) data of the active module. that might help in a lot of situations, here for your new feature but also for in-module histograms or stats... I would implement that.

[Christian-Bouhon]

Please shar your test-image ...
Hello,
You can find them here: https://github.com/Christian-Bouhon/test_chart.
If you prefer TIFF files, I can upload them to Google Drive.
Greetings from the Luberon 🇫🇷
Christian

[Christian-Bouhon]

@Christian-Bouhon after commenting in #20626 about dt caching i thought again about your code here (and why i don't like the way of caching at all).

It simply should not be necessary in the vast majority of use cases as a) all you do here only happens if the CE moduly is the active module and b) we already make sure the input data of the active module is available in the pipe cache for CPU and OpenCL code.

So we just need some extra pixelpipe stuff to get access to preview pipe input (and possibly output) data of the active module. that might help in a lot of situations, here for your new feature but also for in-module histograms or stats... I would implement that.

Hi,
Thank you very much for your suggestion; I really appreciate that you took the time to look into this. This approach makes perfect sense, but refactoring the Pixelpipe infrastructure to expose the cached input data is far beyond my current skills and my understanding of how darktable works internally. For now, I’d prefer to stick with the current approach, which is functional and, above all, simpler for me.
If you can take care of this, I can only thank you for your help.

Best regards,
Christian

[Christian-Bouhon]

Hello,

I saved this as a draft because I’ve received feedback from users of my experimental fork saying that the module is no longer working as it did before.
https://discuss.pixls.us/t/poc-introduction-to-a-new-experimental-module-3d-colorimetric-film-3dcf/58009/92

There are two possible causes, in order:

1. Code cleanup for this PR
2. This latest change.

I’ll look into it and get back to you.

Greetings from the Luberon,
Christian

[jenshannoschwalm]

How does the preview pipe write the cache if processed on GPU?

[Christian-Bouhon]

How does the preview pipe write the cache if processed on GPU?

Hello,

You're right, it wasn't writing it, that was the bug. With the latest commit, the module is working.
When the preview pipe runs on GPU (process_cl), we read pixout (which holds the original uncorrected HSB) back from device memory to host after process_data, extract the hue channel, and populate preview_hue_buf, same result as the CPU path, just with a synchronous GPU > CPU readback
Greetings from the Luberon,
Christian

[jenshannoschwalm]

Just being curious; is there a good reason to use data after processing the module? Changing the hue at a certain location results in slightly different access to nodes

[Christian-Bouhon]

hello,
If I understand the question correctly, and unless I'm mistaken, the original color value is already stored in pixout[k].x before any corrections are applied; the kernel first fills pixout with the raw HSB data, then writes the corrections to a separate buffer. We read pixout immediately after the process_data function; this is the same original hue as the one captured by the CPU path between steps 3 and 5. The post-correction hue is never read.
Have a nice day,
Christian

[jenshannoschwalm]

Indeed, from reading the diffs i spotted it wrongly.