From b5bc93a36c9d3e104bf69e8d1e52e34888d51d30 Mon Sep 17 00:00:00 2001 From: Christian Bouhon Date: Sun, 16 Aug 2026 13:20:17 +0200 Subject: [PATCH 1/4] 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. --- src/CMakeLists.txt | 1 + src/develop/preview_data.c | 232 +++++++++++ src/develop/preview_data.h | 174 ++++++++ src/gui/draw.h | 167 ++++++++ src/iop/colorequal.c | 788 +++++++++++++++++++++++++++++++++++-- src/iop/toneequal.c | 235 +++-------- 6 files changed, 1381 insertions(+), 216 deletions(-) create mode 100644 src/develop/preview_data.c create mode 100644 src/develop/preview_data.h mode change 100644 => 100755 src/iop/colorequal.c diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 149d45ba42b..3ab4f909291 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -130,6 +130,7 @@ FILE(GLOB SOURCE_FILES "develop/masks/masks.c" "develop/masks/path.c" "develop/pixelpipe.c" + "develop/preview_data.c" "develop/tiling.c" "dtgtk/button.c" "dtgtk/culling.c" diff --git a/src/develop/preview_data.c b/src/develop/preview_data.c new file mode 100644 index 00000000000..4bed853ca9f --- /dev/null +++ b/src/develop/preview_data.c @@ -0,0 +1,232 @@ +/* + This file is part of darktable, + Copyright (C) 2026 darktable developers. + + darktable is free software: you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + darktable is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with darktable. If not, see . +*/ + +#include "develop/preview_data.h" + +#include "common/darktable.h" +#include "develop/develop.h" +#include "develop/imageop.h" +#include "develop/pixelpipe_hb.h" + +void dt_preview_data_alloc(dt_preview_data_t *pd, + const dt_iop_module_t *module) +{ + pd->buf = NULL; + pd->width = 0; + pd->height = 0; + pd->hash = DT_INVALID_HASH; + pd->module = module; +} + +void dt_preview_data_free(dt_preview_data_t *pd) +{ + if(pd->buf) + { + dt_free_align(pd->buf); + pd->buf = NULL; + } + pd->width = 0; + pd->height = 0; + pd->hash = DT_INVALID_HASH; + pd->module = NULL; +} + +void dt_preview_data_store(dt_preview_data_t *pd, + const size_t width, + const size_t height, + const dt_dev_pixelpipe_iop_t *piece, + dt_preview_data_fill_t fill, + void *const user_data) +{ + if(!pd || !pd->module || !fill || !piece) return; + + // resize, fill and hash commit all happen under the same GUI lock so + // that the GUI thread can never observe a resized but not-yet-filled + // buffer (a window the earlier ensure()/fill/set_hash split exposed). + dt_iop_gui_enter_critical_section((dt_iop_module_t *)pd->module); + + gboolean can_fill = TRUE; + if(pd->width != width || pd->height != height) + { + float *const new_buf = dt_alloc_align_float(width * height); + if(new_buf) + { + dt_free_align(pd->buf); + pd->buf = new_buf; + pd->width = width; + pd->height = height; + } + else + { + pd->hash = DT_INVALID_HASH; + can_fill = FALSE; + } + } + + if(can_fill && pd->buf) + { + fill(user_data, pd->buf, (size_t)width * height); + pd->hash = dt_dev_pixelpipe_piece_hash((dt_dev_pixelpipe_iop_t *)piece, + &piece->processed_roi_out, TRUE); + } + + dt_iop_gui_leave_critical_section((dt_iop_module_t *)pd->module); +} + +float *dt_preview_data_resize(dt_preview_data_t *pd, + const size_t width, + const size_t height, + dt_preview_data_resize_cb_t resize_cb, + void *const user_data) +{ + if(!pd || !pd->module) return NULL; + + // The resize and the caller's invalidation callback run under the same + // GUI lock so that the module can atomically mark its dependent state + // stale together with the buffer reallocation. + dt_iop_gui_enter_critical_section((dt_iop_module_t *)pd->module); + + gboolean ok = TRUE; + if(pd->width != width || pd->height != height) + { + float *const new_buf = dt_alloc_align_float(width * height); + if(new_buf) + { + dt_free_align(pd->buf); + pd->buf = new_buf; + pd->width = width; + pd->height = height; + } + else + { + pd->hash = DT_INVALID_HASH; + ok = FALSE; + } + if(resize_cb) resize_cb(user_data); + } + + float *const buf = ok ? pd->buf : NULL; + dt_iop_gui_leave_critical_section((dt_iop_module_t *)pd->module); + + return buf; +} + +void dt_preview_data_set_hash(dt_preview_data_t *pd, + const dt_dev_pixelpipe_iop_t *piece) +{ + if(!pd || !pd->module) return; + + dt_iop_gui_enter_critical_section((dt_iop_module_t *)pd->module); + pd->hash = dt_dev_pixelpipe_piece_hash((dt_dev_pixelpipe_iop_t *)piece, + &piece->processed_roi_out, TRUE); + dt_iop_gui_leave_critical_section((dt_iop_module_t *)pd->module); +} + +gboolean dt_preview_data_get(dt_preview_data_t *pd, + const size_t x, + const size_t y, + float *value) +{ + if(!pd || !pd->module || !value) return FALSE; + + dt_iop_gui_enter_critical_section((dt_iop_module_t *)pd->module); + + gboolean ok = FALSE; + float v = 0.f; + // The bounds check and the buffer read must both happen under the + // GUI lock: the pipe thread may resize pd->buf between the two. + if(pd->buf && x < pd->width && y < pd->height) + { + const size_t idx = (size_t)y * pd->width + (size_t)x; + v = pd->buf[idx]; + ok = TRUE; + } + + dt_iop_gui_leave_critical_section((dt_iop_module_t *)pd->module); + + *value = v; + return ok; +} + +gboolean dt_preview_data_is_fresh(dt_preview_data_t *pd) +{ + if(!pd || !pd->module || !pd->buf) return FALSE; + + const dt_iop_module_t *const module = pd->module; + dt_iop_gui_enter_critical_section((dt_iop_module_t *)module); + + // No value stored yet, or the stored data has been invalidated. + const dt_hash_t stored_hash = pd->hash; + gboolean fresh = (stored_hash != DT_INVALID_HASH); + + if(fresh) + { + const dt_develop_t *const dev = module->dev; + if(!dev || !dev->preview_pipe) + fresh = FALSE; + else + { + dt_dev_pixelpipe_iop_t *piece = NULL; + for(GList *iter = dev->preview_pipe->nodes; iter; iter = g_list_next(iter)) + { + dt_dev_pixelpipe_iop_t *const p = (dt_dev_pixelpipe_iop_t *)iter->data; + if(p->module == module) + { + piece = p; + break; + } + } + if(!piece) + fresh = FALSE; + else + { + const dt_hash_t cur_hash = dt_dev_pixelpipe_piece_hash(piece, &piece->processed_roi_out, TRUE); + fresh = (cur_hash == stored_hash); + } + } + } + + dt_iop_gui_leave_critical_section((dt_iop_module_t *)module); + return fresh; +} + +dt_hash_t dt_preview_data_get_hash(dt_preview_data_t *pd) +{ + if(!pd || !pd->module) return DT_INVALID_HASH; + + dt_iop_gui_enter_critical_section((dt_iop_module_t *)pd->module); + const dt_hash_t hash = pd->hash; + dt_iop_gui_leave_critical_section((dt_iop_module_t *)pd->module); + + return hash; +} + +void dt_preview_data_invalidate(dt_preview_data_t *pd) +{ + if(!pd || !pd->module) return; + + dt_iop_gui_enter_critical_section((dt_iop_module_t *)pd->module); + pd->hash = DT_INVALID_HASH; + dt_iop_gui_leave_critical_section((dt_iop_module_t *)pd->module); +} + +// clang-format off +// modelines: These editor modelines have been set for all relevant files by tools/update_modelines.py +// vim: shiftwidth=2 expandtab tabstop=2 cindent +// kate: tab-indents: off; indent-width 2; replace-tabs on; indent-mode cstyle; remove-trailing-spaces modified; +// clang-format on \ No newline at end of file diff --git a/src/develop/preview_data.h b/src/develop/preview_data.h new file mode 100644 index 00000000000..36c273384ec --- /dev/null +++ b/src/develop/preview_data.h @@ -0,0 +1,174 @@ +/* + This file is part of darktable, + Copyright (C) 2026 darktable developers. + + darktable is free software: you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + darktable is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with darktable. If not, see . +*/ + +/* + Generic support for reading a per-pixel scalar value produced on the + preview pipe while flying over the image in the darkroom. + + iop modules (tone equalizer, color equalizer, ...) that want to + display a value under the mouse cursor (exposure, hue, ...) previously + had to duplicate buffer management, hashing and locking in each module. + This service factorizes that common plumbing: + + - allocation/resizing of the per-pixel scalar buffer, + - recording and checking the cumulative pipe hash, + - thread-safe write (pipe thread) and read (GUI thread), + - invalidation helpers. + + The service only deals with *data*. How the per-pixel value is + computed (the module's own process(), CPU or OpenCL) and how it is + rendered on screen (the module's own gui_post_expose() and cursor + handling) stay fully module specific. The mapping between normalized + cursor coordinates and buffer pixels is also left to the module: + it depends on the module position in the pipe (geometry transforms + located after the module, such as crop, must be inverted back to reach + the module buffer space) and is therefore not part of this service. +*/ + +#pragma once + +#ifdef __cplusplus +extern "C" +{ +#endif + +#include "common/darktable.h" +#include "develop/develop.h" +#include "develop/pixelpipe_hb.h" + +typedef struct dt_preview_data_t +{ + float *buf; // one float per pixel of the preview pipe + size_t width; // buffer width in pixels + size_t height; // buffer height in pixels + dt_hash_t hash; // cumulative pipe hash when the buffer was last filled + const dt_iop_module_t *module; // owning module +} dt_preview_data_t; + +/** create/allocate the structure. */ +void dt_preview_data_alloc(dt_preview_data_t *pd, + const dt_iop_module_t *module); + +/** free allocated data. */ +void dt_preview_data_free(dt_preview_data_t *pd); + +/** fill the buffer with the per-pixel values; called under the module GUI lock. */ +typedef void (*dt_preview_data_fill_t)(void *const user_data, + float *const buf, + const size_t npixels); + +/** called under the module GUI lock after the buffer has been resized. */ +typedef void (*dt_preview_data_resize_cb_t)(void *const user_data); + +/** + * Make sure the buffer has the given dimensions, call fill() while + * holding the module GUI lock, then commit the cumulative pipe hash of + * piece. resize, fill and hash commit all happen atomically under a + * single critical section, so that the GUI can never observe a resized + * but not-yet-filled buffer. Thread-safe. + * + * On allocation failure the previously stored data is invalidated and + * the old buffer is kept (no fill, no new hash). + * + * Intended for cheap per-pixel copies (e.g. a hue buffer filled from + * process() output). Expensive fills that must not hold the GUI lock + * should use dt_preview_data_resize() and commit afterwards with + * dt_preview_data_set_hash(). + */ +void dt_preview_data_store(dt_preview_data_t *pd, + const size_t width, + const size_t height, + const dt_dev_pixelpipe_iop_t *piece, + dt_preview_data_fill_t fill, + void *const user_data); + +/** + * Make sure the buffer has the given dimensions and return a pointer to + * it (owned by the service, valid until the next resize()/free() call). + * Returns NULL on allocation failure (the stored data is invalidated). + * + * If the buffer had to be resized, resize_cb() is called while still + * holding the module GUI lock so that the module can atomically + * invalidate its own dependent state (e.g. its "buffer valid" flag) + * together with the resize. Thread-safe. + */ +float *dt_preview_data_resize(dt_preview_data_t *pd, + const size_t width, + const size_t height, + dt_preview_data_resize_cb_t resize_cb, + void *const user_data); + +/** + * Record the cumulative hash of the module's piece in the preview + * pipe. Must be called after the buffer has been (re)filled in memory + * returned by dt_preview_data_resize() so that is_fresh() can later + * validate the data. Thread-safe. + */ +void dt_preview_data_set_hash(dt_preview_data_t *pd, + const dt_dev_pixelpipe_iop_t *piece); + +/** + * Read the scalar value at buffer pixel (x, y). + * + * Thread-safe: takes the module GUI lock around the read. + * Returns TRUE and fills *value on success, FALSE if there is no data + * stored yet or if (x, y) is outside the buffer. + * + * @param x, y: coordinates in buffer pixels. The mapping from the + * cursor position to buffer pixels is module specific and + * must be done by the caller (see comment at top of file). + */ +gboolean dt_preview_data_get(dt_preview_data_t *pd, + const size_t x, + const size_t y, + float *value); + +/** + * Check whether the stored data is still fresh with respect to the + * current preview pipe: TRUE when a value is stored and the cumulative + * hash of the module's piece in the preview pipe still matches the hash + * recorded at store time. FALSE when nothing is stored yet or when the + * upstream pipe state has changed (the module needs a reprocess). + * + * Thread-safe. + */ +gboolean dt_preview_data_is_fresh(dt_preview_data_t *pd); + +/** + * Return the cumulative pipe hash recorded at the last store, so the + * module can decide whether its per-pixel value needs to be recomputed + * (compared against the hash of the current pipe state). Thread-safe. + */ +dt_hash_t dt_preview_data_get_hash(dt_preview_data_t *pd); + +/** + * Invalidate the stored data (mark the recorded hash as stale). + * The buffer itself is kept and may be read but is_fresh() will return + * FALSE until a new set_hash() happens. Thread-safe. + */ +void dt_preview_data_invalidate(dt_preview_data_t *pd); + +#ifdef __cplusplus +} +#endif + +// clang-format off +// modelines: These editor modelines have been set for all relevant files by tools/update_modelines.py +// vim: shiftwidth=2 expandtab tabstop=2 cindent +// kate: tab-indents: off; indent-width 2; replace-tabs on; indent-mode cstyle; remove-trailing-spaces modified; +// clang-format on \ No newline at end of file diff --git a/src/gui/draw.h b/src/gui/draw.h index 23a691288e5..24c1de1fb4c 100644 --- a/src/gui/draw.h +++ b/src/gui/draw.h @@ -95,6 +95,173 @@ static inline void dt_draw_line(cairo_t *cr, cairo_line_to(cr, right, bottom); } +/** fills the current clip region with evenly spaced diagonal hatch lines. + * center is the (x, y) coordinates of the region to draw; span is the + * distance of the region's bounds to the center, over (x, y) axes. + */ +static inline void dt_draw_hatches(cairo_t *cr, + const double center[2], + const double span[2], + const int instances, + const double line_width, + const double shade) +{ + const double C0[2] = { center[0] - span[0], center[1] - span[1] }; + const double C2[2] = { center[0] + span[0], center[1] + span[1] }; + + const double delta[2] = { 2.0 * span[0] / (double)instances, + 2.0 * span[1] / (double)instances }; + + cairo_set_line_width(cr, line_width); + cairo_set_source_rgb(cr, shade, shade, shade); + + for(int i = -instances / 2 - 1; i <= instances / 2 + 1; i++) + { + cairo_move_to(cr, C0[0] + (double)i * delta[0], C0[1]); + cairo_line_to(cr, C2[0] + (double)i * delta[0], C2[1]); + cairo_stroke(cr); + } +} + +/** one filled, optionally hatched circle of dt_draw_correction_cursor(). */ +static inline void _dt_draw_cursor_circle(cairo_t *cr, + const double x, + const double y, + const double radius, + const float color[3], + const float alpha, + const float zoom_scale, + const gboolean hatch, + const float frame_color[3]) +{ + const double radius_z = radius / zoom_scale; + + cairo_set_source_rgba(cr, color[0], color[1], color[2], alpha); + cairo_arc(cr, x, y, radius_z, 0, 2 * M_PI); + cairo_fill_preserve(cr); + cairo_save(cr); + cairo_clip(cr); + + if(hatch) + { + const double pointer_coord[2] = { x, y }; + const double span[2] = { radius_z, radius_z }; + dt_draw_hatches(cr, pointer_coord, span, 6, DT_PIXEL_APPLY_DPI(1.0 / zoom_scale), 0.3); + } + cairo_restore(cr); + + // outline the circle in the same color as the crosshair/wedge, so it + // stays legible against any background + cairo_set_source_rgb(cr, frame_color[0], frame_color[1], frame_color[2]); + cairo_set_line_width(cr, DT_PIXEL_APPLY_DPI(1.0 / zoom_scale)); + cairo_arc(cr, x, y, radius_z, 0, 2 * M_PI); + cairo_stroke(cr); +} + +/** + * dt_draw_correction_cursor — the on-canvas cursor shown by iop modules + * that let the user adjust a per-pixel correction by hovering/scrolling + * over the image (tone equalizer, color equalizer, ...): a crosshair + * frame, a pie-wedge on the left showing the magnitude/direction of the + * correction, one or two concentric filled circles, and a text label to + * the right. Factored out so every such module shares the same design. + * + * pointerx, pointery: cursor position, in the same coordinate space as + * the rest of the module's gui_post_expose() drawing. + * correction_norm: signed magnitude of the correction, roughly in + * [-1 ; 1] (not clamped here — pre-scale if your natural range is + * larger); drives the wedge's angular span, up to ±45°. + * frame_color: color of the wedge outline, the crosshair/ground-level + * lines, and the outline stroked around both circles. + * outer_color, inner_color: fill colors of the outer (radius 16) and + * inner (radius 8) circles — pass the same color for both for a plain + * single-color dot, or two different colors to show e.g. a value + * before/after the correction. + * outer_hatch, inner_hatch: overlay diagonal hatching on that circle + * (e.g. to flag an out-of-range value); pass FALSE to disable. + * text: label drawn in a background pill to the right of the circle + * (e.g. "+1.2 EV", "+12%", "-4.2°"); pass NULL or "" to omit it. + */ +static inline void dt_draw_correction_cursor(cairo_t *cr, + const double pointerx, + const double pointery, + const float zoom_scale, + const float correction_norm, + const float frame_color[3], + const float outer_color[3], + const gboolean outer_hatch, + const float inner_color[3], + const gboolean inner_hatch, + const char *text) +{ + const double outer_radius = 16.0; + const double inner_radius = outer_radius / 2.0; + const double padding = 4.0; // matches the bauhaus quad padding (kept in sync manually) + const double setting_offset_x = (outer_radius + 4.0 * padding) / zoom_scale; + const double fill_width = DT_PIXEL_APPLY_DPI(4.0 / zoom_scale); + + // wedge showing the magnitude/direction of the correction + cairo_set_source_rgb(cr, frame_color[0], frame_color[1], frame_color[2]); + cairo_set_line_width(cr, 2.0 * fill_width); + cairo_move_to(cr, pointerx - setting_offset_x, pointery); + if(correction_norm > 0.0f) + cairo_arc(cr, pointerx, pointery, setting_offset_x, + M_PI, M_PI + correction_norm * M_PI_4); + else + cairo_arc_negative(cr, pointerx, pointery, setting_offset_x, + M_PI, M_PI + correction_norm * M_PI_4); + cairo_stroke(cr); + + // ground-level reference bars + cairo_set_line_width(cr, DT_PIXEL_APPLY_DPI(1.5 / zoom_scale)); + cairo_move_to(cr, pointerx + (outer_radius + 2.0 * padding) / zoom_scale, pointery); + cairo_line_to(cr, pointerx + outer_radius / zoom_scale, pointery); + cairo_move_to(cr, pointerx - outer_radius / zoom_scale, pointery); + cairo_line_to(cr, pointerx - setting_offset_x - 4.0 * padding / zoom_scale, pointery); + cairo_stroke(cr); + + // crosshair + cairo_move_to(cr, pointerx, pointery + setting_offset_x + fill_width); + cairo_line_to(cr, pointerx, pointery + outer_radius / zoom_scale); + cairo_move_to(cr, pointerx, pointery - outer_radius / zoom_scale); + cairo_line_to(cr, pointerx, pointery - setting_offset_x - fill_width); + cairo_stroke(cr); + + _dt_draw_cursor_circle(cr, pointerx, pointery, outer_radius, outer_color, 0.9f, zoom_scale, outer_hatch, frame_color); + _dt_draw_cursor_circle(cr, pointerx, pointery, inner_radius, inner_color, 0.9f, zoom_scale, inner_hatch, frame_color); + + if(!text || !*text) return; + + PangoFontDescription *desc = dt_gui_get_font(); + const int old_size = pango_font_description_get_size(desc); + pango_font_description_set_size(desc, (int)(old_size / zoom_scale)); + + PangoLayout *layout = pango_cairo_create_layout(cr); + pango_layout_set_font_description(layout, desc); + pango_cairo_context_set_resolution(pango_layout_get_context(layout), darktable.gui->dpi); + pango_layout_set_text(layout, text, -1); + + PangoRectangle ink; + pango_layout_get_pixel_extents(layout, &ink, NULL); + + const double pad = padding / zoom_scale; + const double tx = pointerx + (outer_radius + 2.0 * padding) / zoom_scale; + const double ty = pointery - ink.y - ink.height / 2.0 - pad; + + cairo_rectangle(cr, tx, ty, + ink.width + 2.0 * ink.x + 2.0 * pad, ink.height + 2.0 * ink.y + 2.0 * pad); + cairo_set_source_rgba(cr, 0.0, 0.0, 0.0, 0.8); + cairo_fill(cr); + + cairo_move_to(cr, tx + pad, pointery - ink.y - ink.height / 2.0); + cairo_set_source_rgba(cr, 1.0, 1.0, 1.0, 1.0); + pango_cairo_show_layout(cr, layout); + cairo_stroke(cr); + + pango_font_description_free(desc); + g_object_unref(layout); +} + static inline void dt_draw_grid(cairo_t *cr, const int num, const int left, diff --git a/src/iop/colorequal.c b/src/iop/colorequal.c old mode 100644 new mode 100755 index 0fb536cf32e..39a1486b3e5 --- a/src/iop/colorequal.c +++ b/src/iop/colorequal.c @@ -71,6 +71,7 @@ None;midi:CC24=iop/colorequal/brightness/magenta #include "develop/imageop.h" #include "develop/imageop_math.h" #include "develop/imageop_gui.h" +#include "develop/preview_data.h" #include "develop/tiling.h" #include "dtgtk/drawingarea.h" #include "dtgtk/expander.h" @@ -282,6 +283,35 @@ typedef struct dt_iop_colorequal_gui_data_t gboolean on_node; int selected; float points[NODES+1][2]; + + // Hue read under mouse cursor (degrees, GUI space 0..360) + float cursor_hue; + // TRUE if the last hue reading is usable (picker active, sufficient chroma) + gboolean cursor_valid; + + // Cursor position in preview image coordinates (for gui_post_expose) + float cursor_pos_x; + float cursor_pos_y; + + // TRUE once a preview reprocess has been requested to (re)fill pd and + // no fresh data has been observed since. Prevents flooding the pipeline + // with redundant reprocess requests while hovering with a stale/missing + // buffer (see mouse_moved()/gui_focus()). + gboolean reprocess_pending; + + // Last cursor position within the graph/histogram widget, in widget + // pixel coordinates, and whether it is currently valid. Independent of + // cursor_pos_x/y (which track the mouse over the main image) so that + // scrolling on the graph acts on the node actually under the cursor + // there, not on the last hue seen while hovering the image. + float graph_cursor_x; + gboolean graph_cursor_valid; + + // Shared preview pipe under-cursor data (buffer + freshness hash), + // filled by process() for the preview pipe (CPU and OpenCL paths). + // Enables direct reading of the UCS hue (radians) under the cursor + // without depending on the GTK color picker (asynchronous). + dt_preview_data_t pd; } dt_iop_colorequal_gui_data_t; void init_global(dt_iop_module_so_t *self) @@ -950,6 +980,17 @@ static void _prepare_process(const float roi_scale, _init_satweights(d->contrast); } +static void _copy_hue_cb(void *const user_data, + float *const buf, + const size_t npixels) +{ + // pix_out[0] = HSB hue (radians UCS) + const float *const src = (const float *)user_data; + DT_OMP_FOR() + for(size_t k = 0; k < npixels; k++) + buf[k] = src[k * 4]; +} + void process(dt_iop_module_t *self, dt_dev_pixelpipe_iop_t *piece, const void *const i, @@ -1088,6 +1129,15 @@ void process(dt_iop_module_t *self, } } + // Cache the UCS hue (radians) in the preview buffer for mouse_moved/scrolled. + // The service resizes, fills and commits the freshness hash under one GUI + // lock so the GUI thread can never observe a resized but not-yet-filled buffer. + if(g && (piece->pipe->type & DT_DEV_PIXELPIPE_PREVIEW)) + { + dt_iop_colorequal_gui_data_t *gui = self->gui_data; // non-const for writing + dt_preview_data_store(&gui->pd, width, height, piece, _copy_hue_cb, (void *)out); + } + if(d->use_filter && !run_fast) { // blur the saturation gradients @@ -1586,6 +1636,25 @@ int process_cl(dt_iop_module_t *self, CLARG(width), CLARG(height)); if(err != CL_SUCCESS) goto error; + // On the preview pipe, read the original (uncorrected) hue back from the + // GPU pixout buffer to populate the shared preview buffer for + // mouse_moved/scrolled. pixout[k].x contains the raw HSB hue (same as + // the CPU process() path). + if(self->gui_data && (piece->pipe->type & DT_DEV_PIXELPIPE_PREVIEW)) + { + dt_iop_colorequal_gui_data_t *gui = (dt_iop_colorequal_gui_data_t *)self->gui_data; + const size_t npixels = (size_t)width * height; + const size_t px_sz = 4 * npixels * sizeof(float); + float *host_pixout = dt_alloc_align_float(4 * npixels); + if(host_pixout) + { + err = dt_opencl_read_buffer_from_device(devid, host_pixout, pixout, 0, px_sz, TRUE); + if(err == CL_SUCCESS) + dt_preview_data_store(&gui->pd, width, height, piece, _copy_hue_cb, (void *)host_pixout); + dt_free_align(host_pixout); + } + } + if(guiding && !run_fast) { err = dt_gaussian_mean_blur_cl(devid, Lscharr, width, height, 1, scharr_sigma); @@ -2232,10 +2301,71 @@ void init_presets(dt_iop_module_so_t *self) TRUE, DEVELOP_BLEND_CS_RGB_SCENE); } +/* _switch_cursors — mirrors the tone equalizer's on-canvas cursor + * handling: hide the native GTK cursor so only our own indicator + * (gui_post_expose) is visible while a valid reading is available and the + * preview pipe is idle; show a "wait" cursor while it is (re)computing; + * fall back to the default cursor otherwise (mask editing, module not + * focused, no valid reading yet). + */ +static void _switch_cursors(dt_iop_module_t *self) +{ + dt_iop_colorequal_gui_data_t *g = self->gui_data; + if(!g || !self->dev->gui_attached) return; + + GtkWidget *widget = dt_ui_main_window(darktable.gui->ui); + + // Editing a mask (brush/path/etc.) or canvas otherwise not interactive: + // leave the default cursor alone. + if((self->dev->form_gui && self->dev->form_gui->creation) + || dt_iop_canvas_not_sensitive(self->dev)) + { + GdkCursor *const cursor = gdk_cursor_new_from_name(gdk_display_get_default(), "default"); + gdk_window_set_cursor(gtk_widget_get_window(widget), cursor); + g_object_unref(cursor); + return; + } + + if(!self->expanded) + return; // module not focused: let the app decide + + if(g->cursor_valid && dt_pipe_processing(self->dev->preview_pipe)) + { + GdkCursor *const cursor = gdk_cursor_new_from_name(gdk_display_get_default(), "wait"); + gdk_window_set_cursor(gtk_widget_get_window(widget), cursor); + g_object_unref(cursor); + } + else if(g->cursor_valid) + { + // pipe idle with a valid reading: hide the native cursor + dt_control_change_cursor("none"); + } + else + { + GdkCursor *const cursor = gdk_cursor_new_from_name(gdk_display_get_default(), "default"); + gdk_window_set_cursor(gtk_widget_get_window(widget), cursor); + g_object_unref(cursor); + } +} + void gui_focus(dt_iop_module_t *self, gboolean in) { dt_iop_colorequal_gui_data_t *g = self->gui_data; - if(!in) + if(in) + { + // Opening/focusing the module does not by itself dirty the pipe, so the + // shared under-cursor buffer can still be NULL/stale (gui_init() always + // resets it). Kick a preview-only reprocess so hovering works right + // away instead of only after some unrelated trigger happens to also + // reprocess the preview pipe. + if(!dt_preview_data_is_fresh(&g->pd) && !g->reprocess_pending) + { + g->reprocess_pending = TRUE; + dt_dev_reprocess_preview(self->dev, self->iop_order); + } + _switch_cursors(self); + } + else { dt_iop_color_picker_reset(self, FALSE); const gboolean buttons = g->mask_mode != 0; @@ -2243,7 +2373,12 @@ void gui_focus(dt_iop_module_t *self, gboolean in) dt_bauhaus_widget_set_quad_active(g->threshold, FALSE); dt_bauhaus_widget_set_quad_active(g->hue_shift, FALSE); g->mask_mode = 0; + g->cursor_valid = FALSE; // disables Gaussian mode when module loses focus + g->reprocess_pending = FALSE; + dt_preview_data_invalidate(&g->pd); if(buttons) dt_dev_reprocess_center(self->dev, self->iop_order); + _switch_cursors(self); + dt_control_queue_redraw_center(); } } @@ -2537,6 +2672,480 @@ static void _pipe_RGB_to_Ych(dt_iop_module_t *self, Ych[2] = DT_2PI_F + Ych[2]; } +/* mouse_moved — updates the hue read under the mouse cursor. + * + * Reads directly from the preview buffer filled by process() to get + * the UCS hue (radians) at the point under the cursor. No dependency + * on the GTK color picker (asynchronous and unpredictable). + * + * The hue stored in the preview buffer (g->pd.buf) is in UCS radians [-π ; π] + * (from atan2f(V, U) in dt_UCS_LUV_to_JCH). + * + * Conversion to GUI degrees: + * ucs_rad = deg2rad(gui_deg + ANGLE_SHIFT) + * → gui_deg = rad2deg(ucs_rad) − ANGLE_SHIFT + * + * Returns 0 to let darktable propagate the event normally. + */ +int mouse_moved(dt_iop_module_t *self, + const float pzx, + const float pzy, + const double pressure, + const int which, + const float zoom_scale) +{ + dt_iop_colorequal_gui_data_t *g = self->gui_data; + if(!g) return 0; + + // Disable cursor tracking when drawing a mask (brush/path/etc.) + if(self->dev->form_gui && self->dev->form_gui->creation) + { + g->cursor_valid = FALSE; + _switch_cursors(self); + return 0; + } + + // Read hue from the preview buffer + float hue_rad = 0.f; + gboolean have_hue = FALSE; + dt_iop_gui_enter_critical_section(self); + const float *buf = g->pd.buf; + const int bwidth = g->pd.width; + const int bheight = g->pd.height; + if(buf != NULL && bwidth > 0 && bheight > 0) + { + const int cx = CLAMP((int)(pzx * bwidth), 0, bwidth - 1); + const int cy = CLAMP((int)(pzy * bheight), 0, bheight - 1); + hue_rad = buf[(size_t)cy * bwidth + cx]; + have_hue = TRUE; + } + dt_iop_gui_leave_critical_section(self); + + if(!have_hue) + { + g->cursor_valid = FALSE; + // The buffer is missing entirely (e.g. gui_init() just reset it, or the + // module was never reprocessed on the preview pipe yet). Nothing else + // will refill it on its own — ask for a preview reprocess, debounced so + // we don't flood the pipeline while hovering with no data available. + if(!g->reprocess_pending) + { + g->reprocess_pending = TRUE; + dt_dev_reprocess_preview(self->dev, self->iop_order); + } + _switch_cursors(self); + return 0; + } + + // UCS hue in radians (may be in [-π ; π]) + if(hue_rad < 0.f) hue_rad += DT_2PI_F; + + // Convert to GUI degrees: inverse of _conventional_hue_deg_to_ucs_rad() + g->cursor_hue = hue_rad * (180.f / M_PI_F) - ANGLE_SHIFT; + + // Wrap into [0 ; 360[ + if(g->cursor_hue < 0.f) g->cursor_hue += 360.f; + if(g->cursor_hue >= 360.f) g->cursor_hue -= 360.f; + + // Store normalized cursor position [0..1] for gui_post_expose + g->cursor_pos_x = pzx; + g->cursor_pos_y = pzy; + + // Validate buffer freshness against the cumulative pipe hash. + // cursor_valid is set TRUE only when the hash matches, so the GUI + // indicator (gui_post_expose) and the graph Gaussian mode + // (_area_scrolled_callback) never see stale pipeline data. + g->cursor_valid = dt_preview_data_is_fresh(&g->pd); + if(g->cursor_valid) + { + g->reprocess_pending = FALSE; + dt_control_queue_redraw_center(); + } + else if(!g->reprocess_pending) + { + // Buffer exists but is stale (params changed since it was filled) — + // same debounced reprocess request as above so tracking self-heals + // instead of staying frozen until an unrelated trigger (e.g. scroll) + // happens to kick a reprocess. + g->reprocess_pending = TRUE; + dt_dev_reprocess_preview(self->dev, self->iop_order); + } + _switch_cursors(self); + return 0; +} + +/* mouse_leave — invalidates hue tracking when the mouse leaves the image. + * + * Without this, cursor_valid would remain TRUE with stale hue data, + * and the scroll wheel would continue affecting sliders even outside the image. + */ +int mouse_leave(dt_iop_module_t *self) +{ + dt_iop_colorequal_gui_data_t *g = self->gui_data; + if(!g) return 0; + + g->cursor_valid = FALSE; + _switch_cursors(self); + gtk_widget_queue_draw(GTK_WIDGET(g->area)); + dt_control_queue_redraw_center(); + + return 1; +} + +// Forward declarations: defined further down in the file, alongside the +// rest of the node/Gaussian-weighting helpers they belong with, but also +// needed here to draw the "% from neutral" readout in gui_post_expose(). +static float *_get_param_ptr(dt_iop_colorequal_params_t *p, + const dt_iop_colorequal_channel_t channel, + const int k, + float *out_min, + float *out_max); +static float _gaussian_interp_value(const dt_iop_colorequal_params_t *p, + const dt_iop_colorequal_gui_data_t *g, + const float ref_hue_deg); + +/* gui_post_expose — draws a color indicator circle over the image + * showing the color under the cursor, plus a "% from neutral" (or, for + * the hue channel, "° from neutral") readout of the active channel's + * current correction at that hue. + */ +void gui_post_expose(dt_iop_module_t *self, + cairo_t *cr, + const float width, + const float height, + const float pointerx, + const float pointery, + const float zoom_scale) +{ + dt_iop_colorequal_gui_data_t *g = self->gui_data; + if(!g || !g->cursor_valid) return; + + // Hide cursor indicator when drawing a mask (brush/path/etc.) + if(self->dev->form_gui && self->dev->form_gui->creation) return; + + // Read the color from the preview pipe backbuf + dt_develop_t *dev = self->dev; + dt_pthread_mutex_t *mutex = &dev->preview_pipe->backbuf_mutex; + uint8_t *backbuf = dev->preview_pipe->backbuf; + const int buf_w = dev->preview_pipe->backbuf_width; + const int buf_h = dev->preview_pipe->backbuf_height; + + float cr_f = 0.5f, cg_f = 0.5f, cb_f = 0.5f; // fallback grey + + if(backbuf && buf_w > 0 && buf_h > 0) + { + const int px = CLAMP((int)(g->cursor_pos_x * buf_w), 0, buf_w - 1); + const int py = CLAMP((int)(g->cursor_pos_y * buf_h), 0, buf_h - 1); + + dt_pthread_mutex_lock(mutex); + const size_t idx = (size_t)py * buf_w * 4 + px * 4; + // backbuf is CAIRO_FORMAT_ARGB32: B, G, R, A byte order on little-endian + cb_f = backbuf[idx + 0] / 255.0f; + cg_f = backbuf[idx + 1] / 255.0f; + cr_f = backbuf[idx + 2] / 255.0f; + dt_pthread_mutex_unlock(mutex); + } + + // Position in full image coordinates + const float cx = g->cursor_pos_x * width; + const float cy = g->cursor_pos_y * height; + + // "% from neutral" (hue: "° from neutral") readout for the active + // channel, using the same Gaussian blend scrolling would apply here. + const dt_iop_colorequal_params_t *p = self->params; + const float value = _gaussian_interp_value(p, g, g->cursor_hue); + + char text[64]; + // Wedge magnitude/direction, scaled to the shared cursor's ±45° range: + // hue is an offset in [-180° ; 180°], sat/bright a gain in [0 ; 2] with + // 1.0 = neutral (so value - 1.0 is already in [-1 ; 1]). + float correction_norm; + if(g->channel == HUE) + { + snprintf(text, sizeof(text), "%+.1f°", value); // value is already an offset from neutral (0°) + correction_norm = value / 180.0f; + } + else + { + snprintf(text, sizeof(text), "%+.1f%%", (value - 1.0f) * 100.0f); // 1.0 = neutral gain + correction_norm = value - 1.0f; + } + + const float sampled_color[3] = { cr_f, cg_f, cb_f }; + + // Crosshair/wedge/outline color adapts to the sampled background, same + // spirit as the tone equalizer's cursor: white over dark content, black + // over light content, so it stays legible everywhere. + const float bg_luma = 0.3f * cr_f + 0.59f * cg_f + 0.11f * cb_f; + const float frame_shade = (bg_luma > 0.5f) ? 0.0f : 1.0f; + const float frame_color[3] = { frame_shade, frame_shade, frame_shade }; + + dt_draw_correction_cursor(cr, cx, cy, zoom_scale, correction_norm, + frame_color, + sampled_color, FALSE, + sampled_color, FALSE, + text); +} + +/* _get_param_ptr — returns a direct pointer to the parameter value + * of node k for the active channel, along with its min/max bounds. + * + * Uses offsetof() for copy-free access to struct fields, + * consistent with _pack_saturation / _pack_hue / _pack_brightness. + * + * Bounds per channel: + * HUE : [-180° ; +180°] + * SATURATION : [ 0.0 ; 2.0] (multiplier, 1.0 = neutral) + * BRIGHTNESS : [ 0.0 ; 2.0] (multiplier, 1.0 = neutral) + */ +static float *_get_param_ptr(dt_iop_colorequal_params_t *p, + const dt_iop_colorequal_channel_t channel, + const int k, + float *out_min, + float *out_max) +{ + // Offsets in the struct — same order as the _pack_*() functions + static const size_t sat_off[NODES] = { + offsetof(dt_iop_colorequal_params_t, sat_red), + offsetof(dt_iop_colorequal_params_t, sat_orange), + offsetof(dt_iop_colorequal_params_t, sat_yellow), + offsetof(dt_iop_colorequal_params_t, sat_green), + offsetof(dt_iop_colorequal_params_t, sat_cyan), + offsetof(dt_iop_colorequal_params_t, sat_blue), + offsetof(dt_iop_colorequal_params_t, sat_lavender), + offsetof(dt_iop_colorequal_params_t, sat_magenta) }; + + static const size_t hue_off[NODES] = { + offsetof(dt_iop_colorequal_params_t, hue_red), + offsetof(dt_iop_colorequal_params_t, hue_orange), + offsetof(dt_iop_colorequal_params_t, hue_yellow), + offsetof(dt_iop_colorequal_params_t, hue_green), + offsetof(dt_iop_colorequal_params_t, hue_cyan), + offsetof(dt_iop_colorequal_params_t, hue_blue), + offsetof(dt_iop_colorequal_params_t, hue_lavender), + offsetof(dt_iop_colorequal_params_t, hue_magenta) }; + + static const size_t bright_off[NODES] = { + offsetof(dt_iop_colorequal_params_t, bright_red), + offsetof(dt_iop_colorequal_params_t, bright_orange), + offsetof(dt_iop_colorequal_params_t, bright_yellow), + offsetof(dt_iop_colorequal_params_t, bright_green), + offsetof(dt_iop_colorequal_params_t, bright_cyan), + offsetof(dt_iop_colorequal_params_t, bright_blue), + offsetof(dt_iop_colorequal_params_t, bright_lavender), + offsetof(dt_iop_colorequal_params_t, bright_magenta) }; + + char *base = (char *)p; + switch(channel) + { + case HUE: + *out_min = -180.f; *out_max = 180.f; + return (float *)(base + hue_off[k]); + case SATURATION: + *out_min = 0.f; *out_max = 2.f; + return (float *)(base + sat_off[k]); + case BRIGHTNESS: + default: + *out_min = 0.f; *out_max = 2.f; + return (float *)(base + bright_off[k]); + } +} + +static GtkWidget *_get_slider(const dt_iop_colorequal_gui_data_t *g, const int selected) +{ + GtkWidget *w = NULL; + + switch(g->channel) + { + case(SATURATION): + w = g->sat_sliders[selected]; + break; + case(HUE): + w = g->hue_sliders[selected]; + break; + case(BRIGHTNESS): + default: + w = g->bright_sliders[selected]; + break; + } + + return w; +} + +// Sigma (degrees) of the Gaussian weighting used both when adjusting +// sliders around the cursor hue and when reading back the interpolated +// value under the cursor for display. +#define GAUSSIAN_SIGMA_DEG 35.0f + +/* Angular position of node k in GUI degrees [0 ; 360[, accounting for hue_shift. */ +static inline float _node_hue_deg(const int k, const float hue_shift) +{ + const float node_ucs_rad = _get_hue_node(k, hue_shift); + float node_deg = node_ucs_rad * (180.f / M_PI_F) - ANGLE_SHIFT; + if(node_deg < 0.f) node_deg += 360.f; + if(node_deg >= 360.f) node_deg -= 360.f; + return node_deg; +} + +/* Minimum circular distance between two hues in degrees, in [0 ; 180°]. */ +static inline float _hue_circular_dist_deg(const float a, const float b) +{ + float dist = fabsf(a - b); + if(dist > 180.f) dist = 360.f - dist; + return dist; +} + +/* Gaussian weight for a given circular hue distance: 1.0 at center, + * decays to 0 at large distance. Shared by the slider-adjustment path + * and the under-cursor value readout so they can't drift apart. */ +static inline float _gaussian_weight(const float dist_deg) +{ + const float inv2s2 = 1.0f / (2.0f * GAUSSIAN_SIGMA_DEG * GAUSSIAN_SIGMA_DEG); + return expf(-(dist_deg * dist_deg) * inv2s2); +} + +/* Gaussian-weighted blend of the active channel's current per-node values + * around ref_hue_deg — the same weighting scrolling would apply, read + * back rather than written, for the on-canvas "value at cursor" readout. + */ +static float _gaussian_interp_value(const dt_iop_colorequal_params_t *p, + const dt_iop_colorequal_gui_data_t *g, + const float ref_hue_deg) +{ + float wsum = 0.f, vsum = 0.f; + + for(int k = 0; k < NODES; k++) + { + const float node_deg = _node_hue_deg(k, p->hue_shift); + const float dist = _hue_circular_dist_deg(ref_hue_deg, node_deg); + const float weight = _gaussian_weight(dist); + + float vmin, vmax; + const float *val = _get_param_ptr((dt_iop_colorequal_params_t *)p, g->channel, k, &vmin, &vmax); + wsum += weight; + vsum += weight * (*val); + } + + return (wsum > 1e-6f) ? (vsum / wsum) : 0.f; +} + +/* Apply a Gaussian-weighted adjustment to all sliders of the active + * channel, centered on ref_hue_deg. + * Nodes farther than sigma (35°) receive diminishing influence; + * contributions below 1% are skipped. + * Returns TRUE if any slider value changed. + */ +static gboolean _adjust_params_gaussian(dt_iop_module_t *self, + dt_iop_colorequal_params_t *p, + dt_iop_colorequal_gui_data_t *g, + const float move, + const float ref_hue_deg) +{ + gboolean changed = FALSE; + + for(int k = 0; k < NODES; k++) + { + const float node_deg = _node_hue_deg(k, p->hue_shift); + const float dist = _hue_circular_dist_deg(ref_hue_deg, node_deg); + + const float weight = _gaussian_weight(dist); + if(weight < 0.01f) continue; // negligible contribution + + float vmin, vmax; + float *val = _get_param_ptr(p, g->channel, k, &vmin, &vmax); + *val = CLAMP(*val + move * weight, vmin, vmax); + + // Update the slider — let the callback fire for redraw + GtkWidget *w = _get_slider(g, k); + if(w) dt_bauhaus_slider_set(w, *val); + + changed = TRUE; + } + + if(changed) + { + dt_dev_add_history_item(self->dev, self, TRUE); + gtk_widget_queue_draw(GTK_WIDGET(g->area)); + } + + return changed; +} + +/* scrolled — IOP hook called by darktable when the scroll wheel is used + * WHILE THE MOUSE IS OVER THE IMAGE in the darkroom (not over the GUI panel). + * + * This function — not _area_scrolled_callback — intercepts the event + * before darktable sends it to the zoom handler. + * Returning 1 consumes the event and BLOCKS image zoom. + * Returning 0 lets darktable zoom normally. + * + * Logic: + * - Reads the hue directly from the cached preview buffer + * (not via mouse_moved, to avoid gating on pipeline hash) + * - If a valid hue is available under the cursor + * → applies Gaussian weighting to the active channel's sliders + * → returns 1 to block zoom + * - Otherwise → returns 0, normal zoom + */ +int scrolled(dt_iop_module_t *self, + const float x, + const float y, + const int up, + const uint32_t state) +{ + dt_iop_colorequal_gui_data_t *g = self->gui_data; + + if(!g) return 0; + + // Alt+scroll: switch channel tab, matching the graph's Alt+scroll + // behavior (_area_scrolled_callback). Handled before the hue lookup + // below since switching tabs doesn't need a hue reading. + if(dt_modifier_is(state, GDK_MOD1_MASK)) + { + const int pages = gtk_notebook_get_n_pages(g->notebook); + const int current = gtk_notebook_get_current_page(g->notebook); + const int next = (current + (up ? 1 : -1) + pages) % pages; + gtk_notebook_set_current_page(g->notebook, next); + return 1; // consumes the event → blocks image zoom, same as the normal path + } + + // Read the hue directly from the cached preview buffer (race-safe). + // We do NOT call mouse_moved() here because that would gate on the + // pipeline hash — scroll-based adjustment should work even with + // slightly stale data rather than falling through to image zoom. + float hue_rad = 0.f; + gboolean have_hue = FALSE; + if(g->pd.buf && g->pd.width > 0 && g->pd.height > 0) + { + const int cx = CLAMP((int)(x * g->pd.width), 0, (int)g->pd.width - 1); + const int cy = CLAMP((int)(y * g->pd.height), 0, (int)g->pd.height - 1); + have_hue = dt_preview_data_get(&g->pd, cx, cy, &hue_rad); + } + if(!have_hue) return 0; + + // Convert UCS hue → GUI degrees + if(hue_rad < 0.f) hue_rad += DT_2PI_F; + float hue_deg = hue_rad * (180.f / M_PI_F) - ANGLE_SHIFT; + if(hue_deg < 0.f) hue_deg += 360.f; + if(hue_deg >= 360.f) hue_deg -= 360.f; + g->cursor_hue = hue_deg; + g->cursor_pos_x = x; + g->cursor_pos_y = y; + g->cursor_valid = TRUE; + + // Step: 1.0 for hue (°), 0.01 for sat/bright (%) + // Ctrl for fine precision (÷10) + const float base_step = (g->channel == HUE) ? 1.0f : 0.01f; + const float step = dt_modifier_is(state, GDK_CONTROL_MASK) ? base_step * 0.1f : base_step; + // up=1 → scroll up → increase value + const float move = up ? +step : -step; + + _adjust_params_gaussian(self, self->params, g, move, g->cursor_hue); + _switch_cursors(self); + + return 1; // consumes the event → BLOCKS image zoom +} + void color_picker_apply(dt_iop_module_t *self, GtkWidget *picker, dt_dev_pixelpipe_t *pipe) @@ -2611,28 +3220,6 @@ static void _channel_tabs_switch_callback(GtkNotebook *notebook, gtk_widget_queue_draw(GTK_WIDGET(g->area)); } -static GtkWidget *_get_slider(const dt_iop_colorequal_gui_data_t *g, const int selected) -{ - GtkWidget *w = NULL; - - switch(g->channel) - { - case(SATURATION): - w = g->sat_sliders[selected]; - break; - case(HUE): - w = g->hue_sliders[selected]; - break; - case(BRIGHTNESS): - default: - w = g->bright_sliders[selected]; - break; - } - - gtk_widget_realize(w); - return w; -} - static void _area_set_value(const dt_iop_colorequal_gui_data_t *g, const float graph_height, const float pos) @@ -2695,18 +3282,130 @@ static void _area_reset_nodes(dt_iop_colorequal_gui_data_t *g) } } +/* _graph_x_to_hue_deg — continuous hue (GUI degrees) under a given x + * position within the graph widget. Nodes are laid out at evenly spaced x + * positions independent of hue_shift (see the drawing loop populating + * g->points[]), while their hue value is offset by hue_shift, so we + * interpolate the fractional node index from x and apply the same + * per-node hue mapping as _node_hue_deg() to it. Used so that scrolling + * on the graph can weight around the hue actually under the cursor there, + * rather than the last hue seen while hovering the main image. + */ +static float _graph_x_to_hue_deg(const dt_iop_colorequal_gui_data_t *g, + const dt_iop_colorequal_params_t *p, + const float x) +{ + const float span = g->points[1][0] - g->points[0][0]; + if(fabsf(span) < 1e-6f) return g->cursor_hue; // graph not laid out yet + + const float frac_k = (x - g->points[0][0]) / span; + float hue_deg = _node_hue_deg(0, p->hue_shift) + frac_k * (360.f / (float)NODES); + hue_deg = fmodf(hue_deg, 360.f); + if(hue_deg < 0.f) hue_deg += 360.f; + return hue_deg; +} + +/* _area_scrolled_callback — scroll wheel handling on the graph. + * + * Behavior depending on preview buffer state: + * + * A) Buffer exists (g->pd.buf != NULL) → Gaussian mode + * The scroll wheel modifies all sliders of the active channel based on + * their angular distance to the hue under the cursor on the graph + * itself. Weight follows a Gaussian with sigma=35°: the closest node + * receives maximum movement, neighbors receive a decreasing fraction. + * This ensures smooth transitions with no dead zones. + * + * B) No buffer yet → classic single-node behavior + * The scroll wheel is forwarded to the slider of the selected node in the graph. + * + * C) Alt+scroll: switch page (original behavior unchanged). + * + * Modifiers: + * Ctrl → fine step (0.001 instead of 0.01) + */ static void _area_scrolled_callback(GtkEventControllerScroll *controller, - gdouble dx, - gdouble dy, - dt_iop_module_t *self) + gdouble dx, + gdouble dy, + dt_iop_module_t *self) { - const dt_iop_colorequal_gui_data_t *g = self->gui_data; + GtkWidget *const widget = dt_gui_get_widget(controller); + dt_iop_colorequal_gui_data_t *g = self->gui_data; + dt_iop_colorequal_params_t *p = self->params; + + const GdkModifierType state = dt_key_modifier_state(); + + // Alt+scroll: switch page (original behavior unchanged) + if(dt_modifier_is(state, GDK_MOD1_MASK)) + { + // previously the scroll event was forwarded to the notebook; + // event controllers cannot forward, so switch the page directly + const int pages = gtk_notebook_get_n_pages(g->notebook); + const int current = gtk_notebook_get_current_page(g->notebook); + const int next = (current + (dy > 0.0 ? -1 : 1) + pages) % pages; + gtk_notebook_set_current_page(g->notebook, next); + return; + } + + // The Gaussian mode below weights around the hue under the cursor on the + // graph itself (g->graph_cursor_x, tracked by _area_motion_notify_callback), + // falling back to the last hue seen on the main image (g->cursor_hue) only + // if the graph has not seen a motion event yet. + + // If no preview buffer has been allocated yet, fall back to classic + // single-node adjustment. Otherwise use Gaussian weighting — we + // check buffer existence rather than cursor_valid (which gates on + // pipe hash) so that the graph remains usable with slightly stale data. + if(g->pd.buf == NULL) + { + const float base_step = (g->channel == HUE) ? 1.0f : 0.01f; + const float step = dt_modifier_is(state, GDK_CONTROL_MASK) + ? base_step * 0.1f : base_step; + // dy < 0 on scroll-up (darktable's canonical convention, see + // src/gui/gtk.c) so negate it to make scroll-up increase, consistent + // with scrolled() and with the tone equalizer. + const float move = (float)(-dy) * step; + + float vmin, vmax; + float *val = _get_param_ptr(p, g->channel, g->selected, &vmin, &vmax); + const float old_val = *val; + float new_val = *val + move; + if(g->channel == HUE) + { + if(new_val > 180.f) new_val -= 360.f; + else if(new_val < -180.f) new_val += 360.f; + } + else + new_val = CLAMP(new_val, vmin, vmax); + *val = new_val; - const GdkModifierType state = dt_gui_get_current_event_state(GTK_EVENT_CONTROLLER(controller)); - dt_gui_forward_scroll(controller, - dt_modifier_is(state, GDK_MOD1_MASK) - ? GTK_WIDGET(g->notebook) - : _get_slider(g, g->selected)); + GtkWidget *w = _get_slider(g, g->selected); + if(w) dt_bauhaus_slider_set(w, *val); + + if(*val != old_val) + dt_dev_add_history_item(self->dev, self, TRUE); + gtk_widget_queue_draw(widget); + return; + } + + // --- Gaussian mode ------------------------------------------------------- + + const float base_step = (g->channel == HUE) ? 1.0f : 0.01f; + const float step = dt_modifier_is(state, GDK_CONTROL_MASK) + ? base_step * 0.1f + : base_step; + // dy < 0 on scroll-up (darktable's canonical convention, see + // src/gui/gtk.c) so negate it to make scroll-up increase, consistent + // with scrolled() and with the tone equalizer. + const float move = (float)(-dy) * step; + + const float ref_hue_deg = g->graph_cursor_valid + ? _graph_x_to_hue_deg(g, p, g->graph_cursor_x) + : g->cursor_hue; + + _adjust_params_gaussian(self, p, g, move, ref_hue_deg); + + gtk_widget_queue_draw(widget); } static void _area_motion_notify_callback(GtkEventControllerMotion *controller, @@ -2716,6 +3415,9 @@ static void _area_motion_notify_callback(GtkEventControllerMotion *controller, { dt_iop_colorequal_gui_data_t *g = self->gui_data; + g->graph_cursor_x = (float)x; + g->graph_cursor_valid = TRUE; + if(g->dragging && g->on_node) _area_set_pos(g, y); else @@ -2724,8 +3426,8 @@ static void _area_motion_notify_callback(GtkEventControllerMotion *controller, const float epsilon = DT_PIXEL_APPLY_DPI(10.0); const int oldsel = g->selected; const int oldon = g->on_node; - g->selected = (int)(((float)x - g->points[0][0]) - / (g->points[1][0] - g->points[0][0]) + 0.5f) % NODES; + g->selected = (((int)(((float)x - g->points[0][0]) + / (g->points[1][0] - g->points[0][0]) + 0.5f) % NODES) + NODES) % NODES; g->on_node = fabsf(g->points[g->selected][1] - (float)y) < epsilon; darktable.control->element = g->selected; if(oldsel != g->selected || oldon != g->on_node) @@ -2733,6 +3435,13 @@ static void _area_motion_notify_callback(GtkEventControllerMotion *controller, } } +static void _area_leave_callback(GtkEventControllerMotion *controller, + dt_iop_module_t *self) +{ + dt_iop_colorequal_gui_data_t *g = self->gui_data; + g->graph_cursor_valid = FALSE; +} + static void _area_button_press_callback(GtkGestureSingle *gesture, gint n_press, gdouble x, @@ -2854,6 +3563,7 @@ void gui_cleanup(dt_iop_module_t *self) } dt_free_align(g->gamut_LUT); + dt_preview_data_free(&g->pd); // Destroy the background cache for(dt_iop_colorequal_channel_t chan = 0; chan < NUM_CHANNELS; chan++) @@ -2951,6 +3661,14 @@ void gui_init(dt_iop_module_t *self) g->work_profile = work_profile; g->gradients_cached = FALSE; g->on_node = FALSE; + g->cursor_hue = 0.f; + g->cursor_valid = FALSE; + g->cursor_pos_x = 0.f; + g->cursor_pos_y = 0.f; + g->reprocess_pending = FALSE; + g->graph_cursor_x = 0.f; + g->graph_cursor_valid = FALSE; + dt_preview_data_alloc(&g->pd, self); for(dt_iop_colorequal_channel_t chan = 0; chan < NUM_CHANNELS; chan++) { g->b_data[chan] = NULL; @@ -2990,7 +3708,7 @@ void gui_init(dt_iop_module_t *self) | GDK_BUTTON_RELEASE_MASK | darktable.gui->scroll_mask); dt_gui_connect_click_all(g->area, _area_button_press_callback, _area_button_release_callback, self); - dt_gui_connect_motion(g->area, _area_motion_notify_callback, NULL, NULL, self); + dt_gui_connect_motion(g->area, _area_motion_notify_callback, NULL, _area_leave_callback, self); dt_gui_connect_scroll(g->area, GTK_EVENT_CONTROLLER_SCROLL_BOTH_AXES | GTK_EVENT_CONTROLLER_SCROLL_DISCRETE, _area_scrolled_callback, self); diff --git a/src/iop/toneequal.c b/src/iop/toneequal.c index 788435cdf55..6e87d6eac79 100644 --- a/src/iop/toneequal.c +++ b/src/iop/toneequal.c @@ -108,6 +108,7 @@ #include "develop/imageop.h" #include "develop/imageop_math.h" #include "develop/imageop_gui.h" +#include "develop/preview_data.h" #include "dtgtk/drawingarea.h" #include "dtgtk/expander.h" #include "gui/accelerators.h" @@ -231,9 +232,10 @@ typedef struct dt_iop_toneequalizer_gui_data_t // 6 uint64 to pack - contiguous-ish memory dt_hash_t ui_preview_hash; - dt_hash_t thumb_preview_hash; size_t full_preview_buf_width, full_preview_buf_height; - size_t thumb_preview_buf_width, thumb_preview_buf_height; + + // shared preview pipe under-cursor data (buffer + freshness hash) + dt_preview_data_t pd; // Misc stuff, contiguity, length and alignment unknown float scale; @@ -243,7 +245,6 @@ typedef struct dt_iop_toneequalizer_gui_data_t float histogram_last_decile; // Heap arrays, 64 bits-aligned, unknown length - float *thumb_preview_buf; float *full_preview_buf; // GTK garbage, nobody cares, no SIMD here @@ -630,12 +631,21 @@ static void invalidate_luminance_cache(dt_iop_module_t *const self) g->max_histogram = 1; g->luminance_valid = FALSE; g->histogram_valid = FALSE; - g->thumb_preview_hash = DT_INVALID_HASH; g->ui_preview_hash = DT_INVALID_HASH; dt_iop_gui_leave_critical_section(self); + dt_preview_data_invalidate(&g->pd); dt_iop_refresh_all(self); } +static void _toneeq_preview_resized(void *const user_data) +{ + // Called under the module GUI lock when the preview buffer has been + // reallocated: don't let the GUI read it before it has been recomputed. + dt_iop_module_t *const self = (dt_iop_module_t *)user_data; + dt_iop_toneequalizer_gui_data_t *const g = self->gui_data; + if(g) g->luminance_valid = FALSE; +} + // gaussian-ish kernel - sum is == 1.0f so we don't care much about actual coeffs static const dt_colormatrix_t gauss_kernel = { { 0.076555024f, 0.124401914f, 0.076555024f }, @@ -730,9 +740,9 @@ static float _luminance_from_module_buffer(const dt_iop_module_t *self) _get_point(self, c_x, c_y, &b_x, &b_y); - return get_luminance_from_buffer(g->thumb_preview_buf, - g->thumb_preview_buf_width, - g->thumb_preview_buf_height, + return get_luminance_from_buffer(g->pd.buf, + g->pd.width, + g->pd.height, b_x, b_y); } @@ -1027,11 +1037,11 @@ void toneeq_process(dt_iop_module_t *self, { dt_iop_gui_enter_critical_section(self); g->ui_preview_hash = DT_INVALID_HASH; - g->thumb_preview_hash = DT_INVALID_HASH; g->pipe_order = piece->module->iop_order; g->luminance_valid = FALSE; g->histogram_valid = FALSE; dt_iop_gui_leave_critical_section(self); + dt_preview_data_invalidate(&g->pd); } if(dt_pipe_is_full(piece->pipe)) @@ -1056,23 +1066,11 @@ void toneeq_process(dt_iop_module_t *self, { // For preview pipe we need to cache it too because we have to // compute the full image stats upon user request in GUI threads. - // Locks are required since GUI reads and writes on that buffer. - - // Re-allocate a new buffer if the thumb preview size has changed - dt_iop_gui_enter_critical_section(self); - if(g->thumb_preview_buf_width != width || g->thumb_preview_buf_height != height) - { - dt_free_align(g->thumb_preview_buf); - g->thumb_preview_buf = dt_alloc_align_float(num_elem); - g->thumb_preview_buf_width = width; - g->thumb_preview_buf_height = height; - g->luminance_valid = FALSE; - } - - luminance = g->thumb_preview_buf; + // The shared under-cursor service owns the buffer and its locks. + // The resize and the luminance_valid invalidation happen under one + // GUI lock so the GUI never reads a resized, not-yet-recomputed buffer. + luminance = dt_preview_data_resize(&g->pd, width, height, _toneeq_preview_resized, self); cached = TRUE; - - dt_iop_gui_leave_critical_section(self); } else // just to please GCC { @@ -1116,8 +1114,7 @@ void toneeq_process(dt_iop_module_t *self, } else if(dt_pipe_is_preview(piece->pipe)) { - dt_hash_t saved_hash; - hash_set_get(&g->thumb_preview_hash, &saved_hash, &self->gui_lock); + const dt_hash_t saved_hash = dt_preview_data_get_hash(&g->pd); dt_iop_gui_enter_critical_section(self); const gboolean luminance_valid = g->luminance_valid; @@ -1126,10 +1123,18 @@ void toneeq_process(dt_iop_module_t *self, if(saved_hash != hash || !luminance_valid) { /* compute only if upstream pipe state has changed */ + // Flag the cache as being recomputed so the GUI threads never + // read a partially filled buffer, then commit hash + validity + // once the data is ready. dt_iop_gui_enter_critical_section(self); - g->thumb_preview_hash = hash; g->histogram_valid = FALSE; + g->luminance_valid = FALSE; + dt_iop_gui_leave_critical_section(self); + compute_luminance_mask(in, luminance, width, height, d); + dt_preview_data_set_hash(&g->pd, piece); + + dt_iop_gui_enter_critical_section(self); g->luminance_valid = TRUE; dt_iop_gui_leave_critical_section(self); dt_dev_pixelpipe_cache_invalidate_later(piece->pipe, self->iop_order, "toneequal: "); @@ -1337,7 +1342,7 @@ static void gui_cache_init(dt_iop_module_t *self) dt_iop_gui_enter_critical_section(self); g->ui_preview_hash = DT_INVALID_HASH; - g->thumb_preview_hash = DT_INVALID_HASH; + dt_preview_data_alloc(&g->pd, self); g->max_histogram = 1; g->scale = 1.0f; g->sigma = M_SQRT2_F; @@ -1362,10 +1367,6 @@ static void gui_cache_init(dt_iop_module_t *self) g->full_preview_buf_width = 0; g->full_preview_buf_height = 0; - g->thumb_preview_buf = NULL; - g->thumb_preview_buf_width = 0; - g->thumb_preview_buf_height = 0; - g->desc = NULL; g->layout = NULL; g->cr = NULL; @@ -1477,8 +1478,8 @@ static inline void update_histogram(dt_iop_module_t *const self) dt_iop_gui_enter_critical_section(self); if(!g->histogram_valid && g->luminance_valid) { - const size_t num_elem = g->thumb_preview_buf_height * g->thumb_preview_buf_width; - compute_log_histogram_and_stats(g->thumb_preview_buf, g->histogram, num_elem, + const size_t num_elem = g->pd.height * g->pd.width; + compute_log_histogram_and_stats(g->pd.buf, g->histogram, num_elem, &g->max_histogram, &g->histogram_first_decile, &g->histogram_last_decile); g->histogram_average = (g->histogram_first_decile + g->histogram_last_decile) / 2.0f; @@ -2263,80 +2264,18 @@ static inline gboolean _init_drawing(dt_iop_module_t *const restrict self, dt_iop_toneequalizer_gui_data_t *const restrict g); -void cairo_draw_hatches(cairo_t *cr, - double center[2], - double span[2], - const int instances, - const double line_width, - const double shade) -{ - // center is the (x, y) coordinates of the region to draw - // span is the distance of the region's bounds to the center, over (x, y) axes - - // Get the coordinates of the corners of the bounding box of the region - const double C0[2] = { center[0] - span[0], center[1] - span[1] }; - const double C2[2] = { center[0] + span[0], center[1] + span[1] }; - - const double delta[2] = { 2.0 * span[0] / (double)instances, - 2.0 * span[1] / (double)instances }; - - cairo_set_line_width(cr, line_width); - cairo_set_source_rgb(cr, shade, shade, shade); +// The on-canvas correction cursor itself (crosshair, wedge, circles, text +// label) is shared with other modules via dt_draw_correction_cursor() in +// gui/draw.h; only the exposure-specific grey shades fed into it stay here. - for(int i = -instances / 2 - 1; i <= instances / 2 + 1; i++) - { - cairo_move_to(cr, C0[0] + (double)i * delta[0], C0[1]); - cairo_line_to(cr, C2[0] + (double)i * delta[0], C2[1]); - cairo_stroke(cr); - } -} - -static void get_shade_from_luminance(cairo_t *cr, - const float luminance, - const float alpha) +static float _shade_from_luminance(const float luminance) { // TODO: fetch screen gamma from ICC display profile const float gamma = 1.0f / 2.2f; - const float shade = powf(luminance, gamma); - cairo_set_source_rgba(cr, shade, shade, shade, alpha); + return powf(luminance, gamma); } - -static void draw_exposure_cursor(cairo_t *cr, - const double pointerx, - const double pointery, - const double radius, - const float luminance, - const float zoom_scale, - const int instances, - const float alpha) -{ - // Draw a circle cursor filled with a grey shade corresponding to a luminance value - // or hatches if the value is above the overexposed threshold - - const double radius_z = radius / zoom_scale; - - get_shade_from_luminance(cr, luminance, alpha); - cairo_arc(cr, pointerx, pointery, radius_z, 0, 2 * M_PI); - cairo_fill_preserve(cr); - cairo_save(cr); - cairo_clip(cr); - - if(log2f(luminance) > 0.0f) - { - // if overexposed, draw hatches - double pointer_coord[2] = { pointerx, pointery }; - double span[2] = { radius_z, radius_z }; - cairo_draw_hatches(cr, pointer_coord, span, instances, - DT_PIXEL_APPLY_DPI(1. / zoom_scale), 0.3); - } - cairo_restore(cr); -} - - -static void match_color_to_background(cairo_t *cr, - const float exposure, - const float alpha) +static void _match_color_to_background(float rgb[3], const float exposure) { float shade = 0.0f; // TODO: put that as a preference in darktablerc @@ -2347,7 +2286,7 @@ static void match_color_to_background(cairo_t *cr, else shade = (fmaxf(exposure / contrast, -5.0f) + 2.5f); - get_shade_from_luminance(cr, exp2f(shade), alpha); + rgb[0] = rgb[1] = rgb[2] = _shade_from_luminance(exp2f(shade)); } @@ -2420,90 +2359,24 @@ void gui_post_expose(dt_iop_module_t *self, if(dt_isnan(exposure_in)) return; // something went wrong - // set custom cursor dimensions - const double outer_radius = 16.; - const double inner_radius = outer_radius / 2.0; - const double setting_offset_x = (outer_radius + 4. * g->inner_padding) / zoom_scale; - const double fill_width = DT_PIXEL_APPLY_DPI(4. / zoom_scale); - - // setting fill bars - match_color_to_background(cr, exposure_out, 1.0); - cairo_set_line_width(cr, 2.0 * fill_width); - cairo_move_to(cr, x_pointer - setting_offset_x, y_pointer); - - if(correction > 0.0f) - cairo_arc(cr, x_pointer, y_pointer, setting_offset_x, - M_PI, M_PI + correction * M_PI_4); - else - cairo_arc_negative(cr, x_pointer, y_pointer, setting_offset_x, - M_PI, M_PI + correction * M_PI_4); - - cairo_stroke(cr); - - // setting ground level - cairo_set_line_width(cr, DT_PIXEL_APPLY_DPI(1.5 / zoom_scale)); - cairo_move_to(cr, x_pointer + (outer_radius + 2. * g->inner_padding) / zoom_scale, - y_pointer); - cairo_line_to(cr, x_pointer + outer_radius / zoom_scale, y_pointer); - cairo_move_to(cr, x_pointer - outer_radius / zoom_scale, y_pointer); - cairo_line_to(cr, x_pointer - setting_offset_x - 4.0 * g->inner_padding / zoom_scale, - y_pointer); - cairo_stroke(cr); - - // setting cursor cross hair - cairo_set_line_width(cr, DT_PIXEL_APPLY_DPI(1.5 / zoom_scale)); - cairo_move_to(cr, x_pointer, y_pointer + setting_offset_x + fill_width); - cairo_line_to(cr, x_pointer, y_pointer + outer_radius / zoom_scale); - cairo_move_to(cr, x_pointer, y_pointer - outer_radius / zoom_scale); - cairo_line_to(cr, x_pointer, y_pointer - setting_offset_x - fill_width); - cairo_stroke(cr); - - // draw exposure cursor - draw_exposure_cursor(cr, x_pointer, y_pointer, outer_radius, - luminance_in, zoom_scale, 6, .9); - draw_exposure_cursor(cr, x_pointer, y_pointer, inner_radius, - luminance_out, zoom_scale, 3, .9); - - // Create Pango objects : texts char text[256]; - PangoLayout *layout; - PangoRectangle ink; - PangoFontDescription *desc = dt_gui_get_font(); - - // Avoid text resizing based on zoom level - const int old_size = pango_font_description_get_size(desc); - pango_font_description_set_size (desc, (int)(old_size / zoom_scale)); - layout = pango_cairo_create_layout(cr); - pango_layout_set_font_description(layout, desc); - pango_cairo_context_set_resolution(pango_layout_get_context(layout), darktable.gui->dpi); - - // Build text object if(g->luminance_valid && self->enabled) snprintf(text, sizeof(text), _("%+.1f EV"), exposure_in); else snprintf(text, sizeof(text), "? EV"); - pango_layout_set_text(layout, text, -1); - pango_layout_get_pixel_extents(layout, &ink, NULL); - - // Draw the text plain blackground - get_shade_from_luminance(cr, luminance_out, 0.75); - cairo_rectangle(cr, - x_pointer + (outer_radius + 2. * g->inner_padding) / zoom_scale, - y_pointer - ink.y - ink.height / 2.0 - g->inner_padding / zoom_scale, - ink.width + 2.0 * ink.x + 4. * g->inner_padding / zoom_scale, - ink.height + 2.0 * ink.y + 2. * g->inner_padding / zoom_scale); - cairo_fill(cr); - - // Display the EV reading - match_color_to_background(cr, exposure_out, 1.0); - cairo_move_to(cr, x_pointer + (outer_radius + 4. * g->inner_padding) / zoom_scale, - y_pointer - ink.y - ink.height / 2.); - pango_cairo_show_layout(cr, layout); - cairo_stroke(cr); + float frame_color[3]; + _match_color_to_background(frame_color, exposure_out); + const float outer_shade = _shade_from_luminance(luminance_in); + const float inner_shade = _shade_from_luminance(luminance_out); + const float outer_color[3] = { outer_shade, outer_shade, outer_shade }; + const float inner_color[3] = { inner_shade, inner_shade, inner_shade }; - pango_font_description_free(desc); - g_object_unref(layout); + dt_draw_correction_cursor(cr, x_pointer, y_pointer, zoom_scale, correction, + frame_color, + outer_color, log2f(luminance_in) > 0.0f, + inner_color, log2f(luminance_out) > 0.0f, + text); if(g->luminance_valid && self->enabled) { @@ -3435,7 +3308,7 @@ void gui_cleanup(dt_iop_module_t *self) dt_conf_set_int("plugins/darkroom/toneequal/gui_page", gtk_notebook_get_current_page (g->notebook)); - dt_free_align(g->thumb_preview_buf); + dt_preview_data_free((dt_preview_data_t *)&g->pd); dt_free_align(g->full_preview_buf); if(g->desc) pango_font_description_free(g->desc); From 646e7638b2109422aae80d843efda7e9fd283a10 Mon Sep 17 00:00:00 2001 From: Christian Bouhon Date: Thu, 20 Aug 2026 13:55:43 +0200 Subject: [PATCH 2/4] 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 #21397 and adds the in/out colors feature. --- src/develop/preview_data.c | 13 +-- src/develop/preview_data.h | 10 ++- src/gui/draw.h | 17 ++-- src/iop/colorequal.c | 158 ++++++++++++++++++++++++++++++------- 4 files changed, 152 insertions(+), 46 deletions(-) diff --git a/src/develop/preview_data.c b/src/develop/preview_data.c index 4bed853ca9f..66ed7e76dde 100644 --- a/src/develop/preview_data.c +++ b/src/develop/preview_data.c @@ -29,6 +29,7 @@ void dt_preview_data_alloc(dt_preview_data_t *pd, pd->buf = NULL; pd->width = 0; pd->height = 0; + pd->components = 1; pd->hash = DT_INVALID_HASH; pd->module = module; } @@ -60,10 +61,11 @@ void dt_preview_data_store(dt_preview_data_t *pd, // buffer (a window the earlier ensure()/fill/set_hash split exposed). dt_iop_gui_enter_critical_section((dt_iop_module_t *)pd->module); + const size_t nelems = width * height * pd->components; gboolean can_fill = TRUE; if(pd->width != width || pd->height != height) { - float *const new_buf = dt_alloc_align_float(width * height); + float *const new_buf = dt_alloc_align_float(nelems); if(new_buf) { dt_free_align(pd->buf); @@ -80,7 +82,7 @@ void dt_preview_data_store(dt_preview_data_t *pd, if(can_fill && pd->buf) { - fill(user_data, pd->buf, (size_t)width * height); + fill(user_data, pd->buf, nelems); pd->hash = dt_dev_pixelpipe_piece_hash((dt_dev_pixelpipe_iop_t *)piece, &piece->processed_roi_out, TRUE); } @@ -104,7 +106,7 @@ float *dt_preview_data_resize(dt_preview_data_t *pd, gboolean ok = TRUE; if(pd->width != width || pd->height != height) { - float *const new_buf = dt_alloc_align_float(width * height); + float *const new_buf = dt_alloc_align_float(width * height * pd->components); if(new_buf) { dt_free_align(pd->buf); @@ -140,6 +142,7 @@ void dt_preview_data_set_hash(dt_preview_data_t *pd, gboolean dt_preview_data_get(dt_preview_data_t *pd, const size_t x, const size_t y, + const size_t comp, float *value) { if(!pd || !pd->module || !value) return FALSE; @@ -150,9 +153,9 @@ gboolean dt_preview_data_get(dt_preview_data_t *pd, float v = 0.f; // The bounds check and the buffer read must both happen under the // GUI lock: the pipe thread may resize pd->buf between the two. - if(pd->buf && x < pd->width && y < pd->height) + if(pd->buf && x < pd->width && y < pd->height && comp < pd->components) { - const size_t idx = (size_t)y * pd->width + (size_t)x; + const size_t idx = (((size_t)y * pd->width + (size_t)x) * pd->components) + comp; v = pd->buf[idx]; ok = TRUE; } diff --git a/src/develop/preview_data.h b/src/develop/preview_data.h index 36c273384ec..65c12db9f81 100644 --- a/src/develop/preview_data.h +++ b/src/develop/preview_data.h @@ -53,9 +53,10 @@ extern "C" typedef struct dt_preview_data_t { - float *buf; // one float per pixel of the preview pipe + float *buf; // components floats per pixel of the preview pipe size_t width; // buffer width in pixels size_t height; // buffer height in pixels + size_t components; // floats per pixel (1 = scalar, default; e.g. 3 = HSB) dt_hash_t hash; // cumulative pipe hash when the buffer was last filled const dt_iop_module_t *module; // owning module } dt_preview_data_t; @@ -123,19 +124,22 @@ void dt_preview_data_set_hash(dt_preview_data_t *pd, const dt_dev_pixelpipe_iop_t *piece); /** - * Read the scalar value at buffer pixel (x, y). + * Read a component of the per-pixel value at buffer pixel (x, y). * * Thread-safe: takes the module GUI lock around the read. * Returns TRUE and fills *value on success, FALSE if there is no data - * stored yet or if (x, y) is outside the buffer. + * stored yet, if (x, y) is outside the buffer or if comp is out of the + * [0, components) range. * * @param x, y: coordinates in buffer pixels. The mapping from the * cursor position to buffer pixels is module specific and * must be done by the caller (see comment at top of file). + * @param comp: component index, in [0, components). */ gboolean dt_preview_data_get(dt_preview_data_t *pd, const size_t x, const size_t y, + const size_t comp, float *value); /** diff --git a/src/gui/draw.h b/src/gui/draw.h index 24c1de1fb4c..0d4a87dd3dd 100644 --- a/src/gui/draw.h +++ b/src/gui/draw.h @@ -169,8 +169,8 @@ static inline void _dt_draw_cursor_circle(cairo_t *cr, * pointerx, pointery: cursor position, in the same coordinate space as * the rest of the module's gui_post_expose() drawing. * correction_norm: signed magnitude of the correction, roughly in - * [-1 ; 1] (not clamped here — pre-scale if your natural range is - * larger); drives the wedge's angular span, up to ±45°. + * [-1 ; 1] (values are clamped to that range here — pre-scale if your + * natural range is larger); drives the wedge's angular span, up to ±90°. * frame_color: color of the wedge outline, the crosshair/ground-level * lines, and the outline stroked around both circles. * outer_color, inner_color: fill colors of the outer (radius 16) and @@ -200,16 +200,17 @@ static inline void dt_draw_correction_cursor(cairo_t *cr, const double setting_offset_x = (outer_radius + 4.0 * padding) / zoom_scale; const double fill_width = DT_PIXEL_APPLY_DPI(4.0 / zoom_scale); - // wedge showing the magnitude/direction of the correction + // wedge showing the magnitude/direction of the correction. + // Opens up to ±90° (M_PI_2) at full magnitude (|correction_norm| = 1), + // whereas a neutral (0) correction leaves a bare horizontal bar. + const double wedge_end = M_PI + CLAMP(correction_norm, -1.0, 1.0) * M_PI_2; cairo_set_source_rgb(cr, frame_color[0], frame_color[1], frame_color[2]); cairo_set_line_width(cr, 2.0 * fill_width); cairo_move_to(cr, pointerx - setting_offset_x, pointery); - if(correction_norm > 0.0f) - cairo_arc(cr, pointerx, pointery, setting_offset_x, - M_PI, M_PI + correction_norm * M_PI_4); + if(correction_norm >= 0.0f) + cairo_arc(cr, pointerx, pointery, setting_offset_x, M_PI, wedge_end); else - cairo_arc_negative(cr, pointerx, pointery, setting_offset_x, - M_PI, M_PI + correction_norm * M_PI_4); + cairo_arc_negative(cr, pointerx, pointery, setting_offset_x, M_PI, wedge_end); cairo_stroke(cr); // ground-level reference bars diff --git a/src/iop/colorequal.c b/src/iop/colorequal.c index 39a1486b3e5..47f36d61b2f 100755 --- a/src/iop/colorequal.c +++ b/src/iop/colorequal.c @@ -980,15 +980,21 @@ static void _prepare_process(const float roi_scale, _init_satweights(d->contrast); } -static void _copy_hue_cb(void *const user_data, +static void _copy_HSB_cb(void *const user_data, float *const buf, const size_t npixels) { - // pix_out[0] = HSB hue (radians UCS) + // pix_out[0..2] = HSB of the module input pixel: hue (radians UCS), + // saturation, brightness — computed in process() before any correction. + // npixels covers components floats per pixel (3·width·height). const float *const src = (const float *)user_data; DT_OMP_FOR() - for(size_t k = 0; k < npixels; k++) - buf[k] = src[k * 4]; + for(size_t k = 0; k < npixels / 3; k++) + { + buf[k * 3 + 0] = src[k * 4 + 0]; + buf[k * 3 + 1] = src[k * 4 + 1]; + buf[k * 3 + 2] = src[k * 4 + 2]; + } } void process(dt_iop_module_t *self, @@ -1129,13 +1135,15 @@ void process(dt_iop_module_t *self, } } - // Cache the UCS hue (radians) in the preview buffer for mouse_moved/scrolled. - // The service resizes, fills and commits the freshness hash under one GUI - // lock so the GUI thread can never observe a resized but not-yet-filled buffer. + // Cache the HSB (hue in radians, saturation, brightness) of the module + // *input* pixel in the preview buffer for mouse_moved/scrolled and the + // cursor in/out colors. The service resizes, fills and commits the + // freshness hash under one GUI lock so the GUI thread can never observe + // a resized but not-yet-filled buffer. if(g && (piece->pipe->type & DT_DEV_PIXELPIPE_PREVIEW)) { dt_iop_colorequal_gui_data_t *gui = self->gui_data; // non-const for writing - dt_preview_data_store(&gui->pd, width, height, piece, _copy_hue_cb, (void *)out); + dt_preview_data_store(&gui->pd, width, height, piece, _copy_HSB_cb, (void *)out); } if(d->use_filter && !run_fast) @@ -1636,10 +1644,10 @@ int process_cl(dt_iop_module_t *self, CLARG(width), CLARG(height)); if(err != CL_SUCCESS) goto error; - // On the preview pipe, read the original (uncorrected) hue back from the + // On the preview pipe, read the original (uncorrected) HSB back from the // GPU pixout buffer to populate the shared preview buffer for - // mouse_moved/scrolled. pixout[k].x contains the raw HSB hue (same as - // the CPU process() path). + // mouse_moved/scrolled. pixout[k].xyz contains the raw HSB of the + // module input (same as the CPU process() path). if(self->gui_data && (piece->pipe->type & DT_DEV_PIXELPIPE_PREVIEW)) { dt_iop_colorequal_gui_data_t *gui = (dt_iop_colorequal_gui_data_t *)self->gui_data; @@ -1650,7 +1658,7 @@ int process_cl(dt_iop_module_t *self, { err = dt_opencl_read_buffer_from_device(devid, host_pixout, pixout, 0, px_sz, TRUE); if(err == CL_SUCCESS) - dt_preview_data_store(&gui->pd, width, height, piece, _copy_hue_cb, (void *)host_pixout); + dt_preview_data_store(&gui->pd, width, height, piece, _copy_HSB_cb, (void *)host_pixout); dt_free_align(host_pixout); } } @@ -2303,10 +2311,10 @@ void init_presets(dt_iop_module_so_t *self) /* _switch_cursors — mirrors the tone equalizer's on-canvas cursor * handling: hide the native GTK cursor so only our own indicator - * (gui_post_expose) is visible while a valid reading is available and the - * preview pipe is idle; show a "wait" cursor while it is (re)computing; - * fall back to the default cursor otherwise (mask editing, module not - * focused, no valid reading yet). + * (gui_post_expose) is visible while a valid reading is available, + * whether or not the preview pipe is still (re)computing. No busy/wait + * animation is shown while hovering. Falls back to the default cursor + * otherwise (mask editing, module not focused, no valid reading yet). */ static void _switch_cursors(dt_iop_module_t *self) { @@ -2329,15 +2337,11 @@ static void _switch_cursors(dt_iop_module_t *self) if(!self->expanded) return; // module not focused: let the app decide - if(g->cursor_valid && dt_pipe_processing(self->dev->preview_pipe)) - { - GdkCursor *const cursor = gdk_cursor_new_from_name(gdk_display_get_default(), "wait"); - gdk_window_set_cursor(gtk_widget_get_window(widget), cursor); - g_object_unref(cursor); - } - else if(g->cursor_valid) + if(g->cursor_valid) { - // pipe idle with a valid reading: hide the native cursor + // valid reading: hide the native cursor and rely on the custom + // indicator drawn by gui_post_expose, whatever the pipe processing + // state (no busy animation while hovering) dt_control_change_cursor("none"); } else @@ -2629,6 +2633,23 @@ static gboolean _iop_colorequalizer_draw(GtkWidget *widget, cairo_fill(cr); } + // Draw a white vertical line showing the hue currently under the mouse + // cursor on the main image (mirroring the tone equalizer's exposure + // cursor line). The graph x-axis is linear in conventional GUI degrees, + // shifted by hue_shift, so the hue is mapped directly to x. + if(self->enabled && g->cursor_valid) + { + float x_cursor = (g->cursor_hue / 360.0f + dx) * graph_width; + x_cursor = fmodf(x_cursor, graph_width); // hue is periodic + if(x_cursor < 0.0f) x_cursor += graph_width; + + cairo_set_line_width(cr, DT_PIXEL_APPLY_DPI(1.5)); + set_color(cr, darktable.bauhaus->graph_fg); + cairo_move_to(cr, x_cursor, 0.0); + cairo_line_to(cr, x_cursor, graph_height); + cairo_stroke(cr); + } + dt_free_align(g->LUT); if(self->enabled && self->request_color_pick == DT_REQUEST_COLORPICK_MODULE) @@ -2705,7 +2726,7 @@ int mouse_moved(dt_iop_module_t *self, return 0; } - // Read hue from the preview buffer + // Read hue (component 0 of the stored HSB) from the preview buffer float hue_rad = 0.f; gboolean have_hue = FALSE; dt_iop_gui_enter_critical_section(self); @@ -2716,7 +2737,7 @@ int mouse_moved(dt_iop_module_t *self, { const int cx = CLAMP((int)(pzx * bwidth), 0, bwidth - 1); const int cy = CLAMP((int)(pzy * bheight), 0, bheight - 1); - hue_rad = buf[(size_t)cy * bwidth + cx]; + hue_rad = buf[3 * ((size_t)cy * bwidth + cx)]; have_hue = TRUE; } dt_iop_gui_leave_critical_section(self); @@ -2871,7 +2892,80 @@ void gui_post_expose(dt_iop_module_t *self, correction_norm = value - 1.0f; } - const float sampled_color[3] = { cr_f, cg_f, cb_f }; + // Module input/output colors at the cursor, read from the shared + // preview buffer (same HSB of the module *input* pixel that mouse_moved + // and scrolled use, stored by process() from the pre-correction values). + // The "out" color replays the exact process() correction math — the three + // RBF LUTs from the current params combined as in STEP 4/5 of process(). + float in_color[3] = { cr_f, cg_f, cb_f }; + float out_color[3] = { cr_f, cg_f, cb_f }; + + if(g->pd.buf && g->pd.width > 0 && g->pd.height > 0 && g->gamut_LUT) + { + const int p_cx = CLAMP((int)(g->cursor_pos_x * g->pd.width), 0, (int)g->pd.width - 1); + const int p_cy = CLAMP((int)(g->cursor_pos_y * g->pd.height), 0, (int)g->pd.height - 1); + + // Read the 3 HSB components under one lock, like mouse_moved does. + float hue_in = 0.f, sat_in = 0.f, bright_in = 0.f; + gboolean have_hsb = FALSE; + dt_iop_gui_enter_critical_section(self); + const float *buf = g->pd.buf; + if(buf) + { + const size_t idx = (size_t)p_cy * g->pd.width + p_cx; + hue_in = buf[3 * idx + 0]; + sat_in = buf[3 * idx + 1]; + bright_in = buf[3 * idx + 2]; + have_hsb = TRUE; + } + dt_iop_gui_leave_critical_section(self); + + if(have_hsb) + { + // Rebuild the three RBF LUTs from the current params, exactly as + // commit_params() does for the pipe data. + float DT_ALIGNED_ARRAY sat_values[NODES]; + float DT_ALIGNED_ARRAY hue_values[NODES]; + float DT_ALIGNED_ARRAY bright_values[NODES]; + float DT_ALIGNED_ARRAY LUT_hue[LUT_ELEM]; + float DT_ALIGNED_ARRAY LUT_sat[LUT_ELEM]; + float DT_ALIGNED_ARRAY LUT_bright[LUT_ELEM]; + + _pack_saturation(p, sat_values); + _periodic_RBF_interpolate(sat_values, M_PI_F, LUT_sat, p->hue_shift, TRUE); + _pack_hue(p, hue_values); + _periodic_RBF_interpolate(hue_values, 1.f / p->smoothing_hue * M_PI_F, + LUT_hue, p->hue_shift, FALSE); + _pack_brightness(p, bright_values); + _periodic_RBF_interpolate(bright_values, M_PI_F, LUT_bright, p->hue_shift, TRUE); + + // Corrections as in process() STEP 3/4 (hue is an offset, sat a gain, + // brightness a gain applied through b_corrections). + const float corr_hue = lookup_gamut(LUT_hue, hue_in); + const float corr_sat = lookup_gamut(LUT_sat, hue_in); + const float b_corr = sat_in * (lookup_gamut(LUT_bright, hue_in) - 1.0f); + + const float hue_out = hue_in + corr_hue; + const float sat_out = MAX(0.f, sat_in * (1.f + SAT_EFFECT * (corr_sat - 1.f))); + const float bright_out = MAX(0.f, bright_in * (1.f + BRIGHT_EFFECT * b_corr)); + + // gamut-map + convert to display RGB, same path as the module's + // sliders/graphs (g->white_adapted_profile may be NULL → sRGB fallback + // inside _build_dt_UCS_HSB_gradients). + dt_aligned_pixel_t RGB = { 1.f }; + _build_dt_UCS_HSB_gradients((dt_aligned_pixel_t){ hue_in, sat_in, bright_in, 1.0f }, + RGB, g->white_adapted_profile, g->gamut_LUT); + in_color[0] = RGB[0]; + in_color[1] = RGB[1]; + in_color[2] = RGB[2]; + + _build_dt_UCS_HSB_gradients((dt_aligned_pixel_t){ hue_out, sat_out, bright_out, 1.0f }, + RGB, g->white_adapted_profile, g->gamut_LUT); + out_color[0] = RGB[0]; + out_color[1] = RGB[1]; + out_color[2] = RGB[2]; + } + } // Crosshair/wedge/outline color adapts to the sampled background, same // spirit as the tone equalizer's cursor: white over dark content, black @@ -2882,9 +2976,12 @@ void gui_post_expose(dt_iop_module_t *self, dt_draw_correction_cursor(cr, cx, cy, zoom_scale, correction_norm, frame_color, - sampled_color, FALSE, - sampled_color, FALSE, + in_color, FALSE, // outer: module input color + out_color, FALSE, // inner: module output color text); + + // keep the graph's cursor indicator (white vertical line) in sync + gtk_widget_queue_draw(GTK_WIDGET(g->area)); } /* _get_param_ptr — returns a direct pointer to the parameter value @@ -3119,7 +3216,7 @@ int scrolled(dt_iop_module_t *self, { const int cx = CLAMP((int)(x * g->pd.width), 0, (int)g->pd.width - 1); const int cy = CLAMP((int)(y * g->pd.height), 0, (int)g->pd.height - 1); - have_hue = dt_preview_data_get(&g->pd, cx, cy, &hue_rad); + have_hue = dt_preview_data_get(&g->pd, cx, cy, 0, &hue_rad); } if(!have_hue) return 0; @@ -3669,6 +3766,7 @@ void gui_init(dt_iop_module_t *self) g->graph_cursor_x = 0.f; g->graph_cursor_valid = FALSE; dt_preview_data_alloc(&g->pd, self); + g->pd.components = 3; // store the HSB of the module input pixel per sample for(dt_iop_colorequal_channel_t chan = 0; chan < NUM_CHANNELS; chan++) { g->b_data[chan] = NULL; From bd389fcd1ae0b1064107981431b9ce1d3b7f2bc3 Mon Sep 17 00:00:00 2001 From: Christian Bouhon Date: Fri, 21 Aug 2026 18:32:46 +0200 Subject: [PATCH 3/4] Restoring the code after a rebase --- src/iop/toneequal.c | 112 +++++++++++++++++--------------------------- 1 file changed, 43 insertions(+), 69 deletions(-) diff --git a/src/iop/toneequal.c b/src/iop/toneequal.c index 6e87d6eac79..753e8ffba4f 100644 --- a/src/iop/toneequal.c +++ b/src/iop/toneequal.c @@ -1354,7 +1354,7 @@ static void gui_cache_init(dt_iop_module_t *self) g->lut_valid = FALSE; // TRUE if the gui_lut is ready g->graph_valid = FALSE; // TRUE if the UI graph view is ready g->user_param_valid = FALSE; // TRUE if users params set in interactive view are in bounds - g->factors_valid = FALSE; // TRUE once radial-basis coeffs have been successfully solved + g->factors_valid = TRUE; // TRUE if radial-basis coeffs are ready g->valid_nodes_x = FALSE; // TRUE if x coordinates of graph nodes have been inited g->valid_nodes_y = FALSE; // TRUE if y coordinates of graph nodes have been inited @@ -1500,13 +1500,6 @@ static inline void compute_lut_correction(dt_iop_toneequalizer_gui_data_t *g, if(g == NULL) return; float *const restrict LUT = g->gui_lut; - - if(!g->factors_valid) - { - for(size_t i = 0; i < UI_SAMPLES; i++) LUT[i] = offset; - return; - } - const float *const restrict factors = g->factors; const float sigma = g->sigma; @@ -1520,15 +1513,7 @@ static inline void compute_lut_correction(dt_iop_toneequalizer_gui_data_t *g, } } -// Mark g->interpolation_matrix as invalid to force a recompute, and update g->sigma -// which the matrix computation in update_curve_lut uses. -// Important: the caller must hold the GUI critical section. -static inline void _invalidate_interpolation_matrix_on_sigma_change(dt_iop_toneequalizer_gui_data_t *g, - const float smoothing) -{ - if(g->sigma != smoothing) g->interpolation_valid = FALSE; - g->sigma = smoothing; -} + static inline gboolean update_curve_lut(dt_iop_module_t *self) { @@ -1625,17 +1610,16 @@ void commit_params(dt_iop_module_t *self, /* * Perform a radial-based interpolation using a series gaussian functions */ - - gboolean curve_valid; - if(self->dev->gui_attached && g) { dt_iop_gui_enter_critical_section(self); - _invalidate_interpolation_matrix_on_sigma_change(g, p->smoothing); + if(g->sigma != p->smoothing) + g->interpolation_valid = FALSE; + g->sigma = p->smoothing; g->user_param_valid = FALSE; // force updating channels factors dt_iop_gui_leave_critical_section(self); - curve_valid = update_curve_lut(self); + update_curve_lut(self); dt_iop_gui_enter_critical_section(self); dt_simd_memcpy(g->factors, d->factors, PIXEL_CHAN); @@ -1649,23 +1633,14 @@ void commit_params(dt_iop_module_t *self, float A[CHANNELS * PIXEL_CHAN] DT_ALIGNED_ARRAY; build_interpolation_matrix(A, p->smoothing); - curve_valid = pseudo_solve(A, factors, CHANNELS, PIXEL_CHAN, TRUE); + pseudo_solve(A, factors, CHANNELS, PIXEL_CHAN, TRUE); dt_simd_memcpy(factors, d->factors, PIXEL_CHAN); } // compute the correction LUT here to spare some time in process // when computing several times toneequalizer with same parameters - if(curve_valid) - { - compute_correction_lut(d->correction_lut, d->smoothing, d->factors); - } - else - { - // solver failed; make sure the operation is a no-op with/without darkroom GUI - for(size_t i = 0; i < LUT_RESOLUTION * PIXEL_CHAN + 1; i++) - d->correction_lut[i] = 1.0f; - } + compute_correction_lut(d->correction_lut, d->smoothing, d->factors); } @@ -1787,14 +1762,12 @@ static void smoothing_callback(GtkWidget *slider, dt_iop_module_t *self) { DT_GUARD_GUI_UPDATE(); dt_iop_toneequalizer_params_t *p = self->params; - dt_iop_toneequalizer_gui_data_t *g = self->gui_data; + const dt_iop_toneequalizer_gui_data_t *g = self->gui_data; p->smoothing= powf(M_SQRT2_F, 1.0f + dt_bauhaus_slider_get(slider)); - // avoid stale matrix; commit_params(), which also performs the invalidation, has not run yet - dt_iop_gui_enter_critical_section(self); - _invalidate_interpolation_matrix_on_sigma_change(g, p->smoothing); - dt_iop_gui_leave_critical_section(self); + float factors[CHANNELS] DT_ALIGNED_ARRAY; + get_channels_factors(factors, p); // Solve the interpolation by least-squares to check the validity of the smoothing param if(!update_curve_lut(self)) @@ -1998,12 +1971,17 @@ static void switch_cursors(dt_iop_module_t *self) if(!g || !self->dev->gui_attached) return; + GtkWidget *widget = dt_ui_main_window(darktable.gui->ui); + // if we are editing masks or using colour-pickers, do not display controls if(in_mask_editing(self) || dt_iop_canvas_not_sensitive(self->dev)) { // display default cursor - dt_control_change_cursor("default"); + GdkCursor *const cursor = + gdk_cursor_new_from_name(gdk_display_get_default(), "default"); + gdk_window_set_cursor(gtk_widget_get_window(widget), cursor); + g_object_unref(cursor); return; } @@ -2019,21 +1997,11 @@ static void switch_cursors(dt_iop_module_t *self) // do nothing and let the app decide return; } - else if((dt_pipe_processing(self->dev->full.pipe) - || self->dev->full.pipe->status == DT_DEV_PIXELPIPE_DIRTY - || self->dev->preview_pipe->status == DT_DEV_PIXELPIPE_DIRTY) - && g->cursor_valid) - { - // if pipe is busy or dirty but cursor is on preview, - // display waiting cursor while pipe reprocesses - dt_control_change_cursor("wait"); - - dt_control_queue_redraw_center(); - } - else if(g->cursor_valid && !dt_pipe_processing(self->dev->full.pipe)) + else if(g->cursor_valid) { - // if pipe is clean and idle and cursor is on preview, - // hide GTK cursor because we display our custom one + // if cursor is on the preview, hide GTK cursor because we display + // our custom one. We do this whether or not the pipe is still + // (re)computing, so no busy animation appears while hovering. dt_control_change_cursor("none"); dt_control_hinter_message(_("scroll over image to change tone exposure\n" "shift+scroll for large steps; " @@ -2045,7 +2013,10 @@ static void switch_cursors(dt_iop_module_t *self) { // if module is active and opened but cursor is out of the preview, // display default cursor - dt_control_change_cursor("default"); + GdkCursor *const cursor = + gdk_cursor_new_from_name(gdk_display_get_default(), "default"); + gdk_window_set_cursor(gtk_widget_get_window(widget), cursor); + g_object_unref(cursor); dt_control_queue_redraw_center(); } @@ -2053,7 +2024,10 @@ static void switch_cursors(dt_iop_module_t *self) { // in any other situation where module has focus, // reset the cursor but don't launch a redraw - dt_control_change_cursor("default"); + GdkCursor *const cursor = + gdk_cursor_new_from_name(gdk_display_get_default(), "default"); + gdk_window_set_cursor(gtk_widget_get_window(widget), cursor); + g_object_unref(cursor); } } @@ -2117,7 +2091,10 @@ int mouse_leave(dt_iop_module_t *self) dt_iop_gui_leave_critical_section(self); // display default cursor - dt_control_change_cursor("default"); + GtkWidget *widget = dt_ui_main_window(darktable.gui->ui); + GdkCursor *cursor = gdk_cursor_new_from_name(gdk_display_get_default(), "default"); + gdk_window_set_cursor(gtk_widget_get_window(widget), cursor); + g_object_unref(cursor); dt_control_queue_redraw_center(); gtk_widget_queue_draw(GTK_WIDGET(g->area)); @@ -2310,7 +2287,6 @@ void gui_post_expose(dt_iop_module_t *self, const gboolean fail = !g->cursor_valid || !g->interpolation_valid - || dt_pipe_processing(dev->full.pipe) || !g->has_focus; dt_iop_gui_leave_critical_section(self); @@ -2321,8 +2297,10 @@ void gui_post_expose(dt_iop_module_t *self, if(!_init_drawing(self, self->widget, g)) return; - // re-read the exposure in case it has changed - if(g->luminance_valid && self->enabled) + // Re-read the exposure in case it has changed. While the pipe is busy + // the module buffer may be mid-recompute, so keep the last value and + // stay drawing the indicator (no blinking cursor during reprocess). + if(g->luminance_valid && self->enabled && !dt_pipe_processing(dev->full.pipe)) g->cursor_exposure = log2f(_luminance_from_module_buffer(self)); dt_iop_gui_enter_critical_section(self); @@ -2342,15 +2320,8 @@ void gui_post_expose(dt_iop_module_t *self, exposure_in = g->cursor_exposure; luminance_in = exp2f(exposure_in); - // avoid stale g->factors: only set correction if factors were successfully solved; - // otherwise, leave correction = 0 EV, which is what the pixels get — commit_params() fills - // correction_lut with 1.0 when there is no valid solution. - if(g->factors_valid) - { - // Get the corresponding correction and compute resulting exposure - correction = log2f(pixel_correction(exposure_in, g->factors, g->sigma)); - } - + // Get the corresponding correction and compute resulting exposure + correction = log2f(pixel_correction(exposure_in, g->factors, g->sigma)); exposure_out = exposure_in + correction; luminance_out = exp2f(exposure_out); } @@ -2372,7 +2343,10 @@ void gui_post_expose(dt_iop_module_t *self, const float outer_color[3] = { outer_shade, outer_shade, outer_shade }; const float inner_color[3] = { inner_shade, inner_shade, inner_shade }; - dt_draw_correction_cursor(cr, x_pointer, y_pointer, zoom_scale, correction, + // The wedge normalizes the correction to ±1 (full ±90°); tone equalizer + // corrections are expressed in EV and regularly exceed ±1 EV, so halve + // the value here: the wedge then reaches its full ±90° at ±2 EV. + dt_draw_correction_cursor(cr, x_pointer, y_pointer, zoom_scale, 0.5f * correction, frame_color, outer_color, log2f(luminance_in) > 0.0f, inner_color, log2f(luminance_out) > 0.0f, From dcb6bc427048f60e153c86b0a6bd5aff38acce89 Mon Sep 17 00:00:00 2001 From: Christian Bouhon Date: Fri, 21 Aug 2026 19:18:51 +0200 Subject: [PATCH 4/4] 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. --- src/iop/toneequal.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/iop/toneequal.c b/src/iop/toneequal.c index 753e8ffba4f..19bce1be573 100644 --- a/src/iop/toneequal.c +++ b/src/iop/toneequal.c @@ -2252,20 +2252,6 @@ static float _shade_from_luminance(const float luminance) return powf(luminance, gamma); } -static void _match_color_to_background(float rgb[3], const float exposure) -{ - float shade = 0.0f; - // TODO: put that as a preference in darktablerc - const float contrast = 1.0f; - - if(exposure > -2.5f) - shade = (fminf(exposure * contrast, 0.0f) - 2.5f); - else - shade = (fmaxf(exposure / contrast, -5.0f) + 2.5f); - - rgb[0] = rgb[1] = rgb[2] = _shade_from_luminance(exp2f(shade)); -} - void gui_post_expose(dt_iop_module_t *self, cairo_t *cr, @@ -2336,8 +2322,29 @@ void gui_post_expose(dt_iop_module_t *self, else snprintf(text, sizeof(text), "? EV"); - float frame_color[3]; - _match_color_to_background(frame_color, exposure_out); + // Sample the pixel under the cursor from the preview pipe backbuf: + // white frame lines over dark content, black over bright content, + // like the color equalizer's cursor. The circles keep the + // exposure-specific shades below to convey the before/after luminance. + uint8_t *backbuf = dev->preview_pipe->backbuf; + const int buf_w = dev->preview_pipe->backbuf_width; + const int buf_h = dev->preview_pipe->backbuf_height; + float cr_f = 0.5f, cg_f = 0.5f, cb_f = 0.5f; // fallback mid-grey + if(backbuf && buf_w > 0 && buf_h > 0) + { + const int px = CLAMP((int)x_pointer, 0, buf_w - 1); + const int py = CLAMP((int)y_pointer, 0, buf_h - 1); + dt_pthread_mutex_lock(&dev->preview_pipe->backbuf_mutex); + const size_t idx = (size_t)py * buf_w * 4 + px * 4; + // backbuf is CAIRO_FORMAT_ARGB32: B, G, R, A byte order on little-endian + cb_f = backbuf[idx + 0] / 255.0f; + cg_f = backbuf[idx + 1] / 255.0f; + cr_f = backbuf[idx + 2] / 255.0f; + dt_pthread_mutex_unlock(&dev->preview_pipe->backbuf_mutex); + } + const float bg_luma = 0.3f * cr_f + 0.59f * cg_f + 0.11f * cb_f; + const float frame_shade = (bg_luma > 0.5f) ? 0.0f : 1.0f; + const float frame_color[3] = { frame_shade, frame_shade, frame_shade }; const float outer_shade = _shade_from_luminance(luminance_in); const float inner_shade = _shade_from_luminance(luminance_out); const float outer_color[3] = { outer_shade, outer_shade, outer_shade };