IOP Color EQ : Add interactive editing mode for color adjustments in image - #21397
Conversation
Christian-Bouhon
changed the title
|
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 is never reached. So how is
|
|
Hmm, I think you're right. 😉 |
|
Hello,
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. |
|
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,
|
You can press 'a' while zooming/panning to pass the events to the canvas. But you certainly know that. |
|
Hello, |
|
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? |
|
There is some irritating effect.
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. |
|
Hello, A new button has been added to allow direct adjustments to the image when it is activated. If this doesn’t work for you, I can always revert the changes with Friendly regards, |
|
Let me show you an example:
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". |
|
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. |
92ce9de to
80dc653
Compare
|
Hello, You're right, it isn't very stable. I've reverted to the previous version, which is consistent with TE. 2026-06-27.13-34-52.mp4A little fresh air from the Luberon, 🍷 |
|
Please shar your test-image ... |
|
@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, Best regards, |
|
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. There are two possible causes, in order:
I’ll look into it and get back to you. Greetings from the Luberon, |
|
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. |
|
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 |
|
hello, |
|
Indeed, from reading the diffs i spotted it wrongly. |
5f24759 to
dd43408
Compare
dd43408 to
a8d29ad
Compare
|
Hello, |
a55d785 to
af80956
Compare
|
Hello Pascal, I've pushed the colorequal branch (a single commit, af80956add): this consolidates the “under-cursor preview data” code for the tonal and colorimetric equalizers into a shared service at src/develop/preview_data.[ch], with strictly identical rendering. The commit also includes fixes from development. |
TurboGit
left a comment
TurboGit
left a comment
There was a problem hiding this comment.
Some issues:
- Most of the time when opening the module and flying over the image there is no indicator (circle) and nothing works.
- When it works, flying over the color equalizer histogram and scrolling seems to change the last color seen when flying over and not the actual node under the mouse cursor.
- When you scroll up on the TE the curve move upward, for ColorEQ it is the opposite.
- It would be nice on ColorEQ to reuse the same cursor (and shared on the specific module), instead of displaying IL it would display the % of change +/- from neutral value (center of the curve), that's just an idea I'm open to other proposals.
- Hard to reproduce but I have been able to make the ColorEQ on-the-fly circle locked at a certain position, so not following the mouse move. Behavior was always restored when scrolling with the mouse.
|
Hi Pascal I think I've been able to reproduce the bug you encountered. It seems the service doesn't disconnect when you leave the image area, and indeed, if I scroll on the Coloreq graph, the direction reverses, and it uses the last color modified on the image. |
|
Hi Christian, good to read that your reproduce. Thanks for this work, it will be a nice service for dt modules. |
|
Hello,
Greetings from the Luberon, |
5f8690e to
c82c698
Compare
|
Hello, |
|
@Christian-Bouhon : Lot better, three things I have noticed:
|
c82c698 to
bd389fc
Compare
|
Hello,
In addition, the cursor now shows the module input/output colors: the Greetings from the Lubéron, 🌿 |
…w-data service Adds an on-canvas interactive editing mode driven by the hue under the mouse cursor: hovering the image shows a color indicator and lets the scroll wheel apply a Gaussian-weighted adjustment (sigma=35 degrees) to the saturation/hue/brightness nodes of the active channel, Alt+scroll switches channel tabs (on both the image and the graph), and the graph's own scroll handling mirrors the same behavior for the node under the cursor there. The per-pixel hue buffer needed for this is served by a new shared service, dt_preview_data_t (src/develop/preview_data.c/.h): resizing, filling and hashing the preview-pipe buffer under a single GUI lock so readers never observe a resized-but-unfilled buffer, plus freshness checks against the pipe's cumulative hash. gui_focus()/mouse_moved() use it to request a debounced preview reprocess so the indicator works right after opening the module instead of only after an unrelated trigger. The on-canvas cursor itself (crosshair, wedge, circles, text readout) is factored into a shared dt_draw_correction_cursor() helper in src/gui/draw.h and reused by the tone equalizer, so both modules render the same cursor design and future modules can adopt it too.
- don't show a busy/wait cursor while the preview pipe recomputes during hover (toneequal: drop the busy branch in switch_cursors, gate the exposure re-read instead; colorequal: same in _switch_cursors) - draw a white vertical line on the graph at the hue under the mouse cursor, mirroring the tone equalizer's exposure cursor line - open the wedge cursor up to +-90 degrees (clamped at +-1) instead of clamping it to 45 degrees; toneequal pre-scales the correction by 0.5 so the wedge reaches its full span at +-2 EV - store the module input HSB (hue, saturation, brightness) instead of just the hue in the shared preview buffer (3 components per pixel) and show the module input/output colors in the cursor's two circles This brings the branch in line with the review comment on PR darktable-org#21397 and adds the in/out colors feature.
Like the color equalizer's cursor, sample the pixel under the cursor from the preview pipe backbuf and draw the frame lines (wedge, crosshair, outlines) white over dark content and black over bright content. This replaces _match_color_to_background(), which derived grey shades from the estimated output exposure instead of the actual background, so the lines could end up mid-grey. The circles keep their before/after luminance grey shades.
TurboGit
left a comment
TurboGit
left a comment
There was a problem hiding this comment.
Works for me, thanks!
A nice addition and the generic preview_data could be used in some other modules at some point. Nice!
|
Last point, this needs a release note entry. TIA. |
_switch_cursors() and switch_cursors() restored the default cursor with
a raw gdk_window_set_cursor() on the toplevel, while hiding it through
dt_control_change_cursor("none"). Going through the helper for both
halves brings back three things the raw call skips:
- The lock set by dt_control_forbid_change_cursor(). Shortcut-mapping
mode and help mode (src/libs/tools/global_toolbox.c) hold that lock
for as long as the mode is active, apply their own cursor to the same
toplevel, and still forward motion events to the centre canvas -- so
with the tone equalizer focused, moving the pointer over the canvas
replaced their cursor with the plain arrow. dt_gui_cursor_set_busy()
takes the same lock.
- The -d input tracing in dt_control_cursor_debug(), which is called
from the helpers only.
- The GTK3/GTK4 compatibility boundary. gdk_window_set_cursor() has no
GTK4 equivalent; these were its last uses outside src/gui/gtk.c.
Where the lock is not held the two forms are equivalent: on GTK3
dt_control_change_cursor() ends up calling gdk_cursor_new_from_name()
and gdk_window_set_cursor() on the same toplevel window.
For toneequal.c this restores what was there before 8a26f35
(darktable-org#21397); colorequal.c copied the pattern from it in the same commit.
The behaviour that commit intended -- no wait cursor while hovering,
whatever the pipe is doing -- is unchanged.
Built and verified at runtime on X11/GTK3.
Suggested-by: a darktable contributor
@TurboGit Greetings from the Luberon, |
…image (darktable-org#21397) * colorequal: interactive hue-under-cursor editing with a shared preview-data service Adds an on-canvas interactive editing mode driven by the hue under the mouse cursor: hovering the image shows a color indicator and lets the scroll wheel apply a Gaussian-weighted adjustment (sigma=35 degrees) to the saturation/hue/brightness nodes of the active channel, Alt+scroll switches channel tabs (on both the image and the graph), and the graph's own scroll handling mirrors the same behavior for the node under the cursor there. The per-pixel hue buffer needed for this is served by a new shared service, dt_preview_data_t (src/develop/preview_data.c/.h): resizing, filling and hashing the preview-pipe buffer under a single GUI lock so readers never observe a resized-but-unfilled buffer, plus freshness checks against the pipe's cumulative hash. gui_focus()/mouse_moved() use it to request a debounced preview reprocess so the indicator works right after opening the module instead of only after an unrelated trigger. The on-canvas cursor itself (crosshair, wedge, circles, text readout) is factored into a shared dt_draw_correction_cursor() helper in src/gui/draw.h and reused by the tone equalizer, so both modules render the same cursor design and future modules can adopt it too. * colorequal: address review feedback and show in/out colors under cursor - don't show a busy/wait cursor while the preview pipe recomputes during hover (toneequal: drop the busy branch in switch_cursors, gate the exposure re-read instead; colorequal: same in _switch_cursors) - draw a white vertical line on the graph at the hue under the mouse cursor, mirroring the tone equalizer's exposure cursor line - open the wedge cursor up to +-90 degrees (clamped at +-1) instead of clamping it to 45 degrees; toneequal pre-scales the correction by 0.5 so the wedge reaches its full span at +-2 EV - store the module input HSB (hue, saturation, brightness) instead of just the hue in the shared preview buffer (3 components per pixel) and show the module input/output colors in the cursor's two circles This brings the branch in line with the review comment on PR darktable-org#21397 and adds the in/out colors feature. * Restoring the code after a rebase * toneequal: derive cursor frame color from the sampled background Like the color equalizer's cursor, sample the pixel under the cursor from the preview pipe backbuf and draw the frame lines (wedge, crosshair, outlines) white over dark content and black over bright content. This replaces _match_color_to_background(), which derived grey shades from the estimated output exposure instead of the actual background, so the lines could end up mid-grey. The circles keep their before/after luminance grey shades.
3 participants
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 degreesmouse_leave()– invalidates the hue when the mouse leaves the imagegui_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 withreturn 1_area_scrolled_callback()rewritten: dual mode (classic if no picker, Gaussian otherwise)_get_param_ptr()– direct access to parameters viaoffsetof(), used by the Gaussian loopHere is the forum discussion on pixls.us; I haven’t changed the user interface,
Greetings from the Luberon,
Christian