From 74c8227eef6550e0549b7ebb6796d3d51c463f3b Mon Sep 17 00:00:00 2001 From: Kofa Date: Sat, 24 Jan 2026 14:52:16 +0100 Subject: [PATCH 1/3] agx: don't activate the pivot pickers until the user starts dragging the mouse, to avoid first picking a pivot based on the whole image --- src/gui/color_picker_proxy.c | 20 +++++++++++++++++++- src/gui/color_picker_proxy.h | 6 +++++- src/iop/agx.c | 4 ++-- src/views/darkroom.c | 4 ++++ 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/gui/color_picker_proxy.c b/src/gui/color_picker_proxy.c index e131a8fed818..56a1062c0ab1 100644 --- a/src/gui/color_picker_proxy.c +++ b/src/gui/color_picker_proxy.c @@ -73,6 +73,11 @@ gboolean dt_iop_color_picker_is_visible(const dt_develop_t *dev) return module_picker || primary_picker; } +gboolean dt_iop_color_picker_is_area_empty(const dt_pickerbox_t box) +{ + return (fabsf(box[2] - box[0]) < 1e-5f || fabsf(box[3] - box[1]) < 1e-5f); +} + static gboolean _record_point_area(dt_iop_color_picker_t *self) { const dt_colorpicker_sample_t *const sample = @@ -201,7 +206,13 @@ static gboolean _color_picker_callback_button_press(GtkWidget *button, // pull picker's last recorded positions if(kind & DT_COLOR_PICKER_AREA) { - if( self->pick_box[0] == 0.0f && self->pick_box[1] == 0.0f + if(flags & DT_COLOR_PICKER_NO_AUTO) + { + // reset coordinates to 0.0f to create a zero-area box, + // requiring the user to drag-select manually + memset(self->pick_box, 0, sizeof(self->pick_box)); + } + else if( self->pick_box[0] == 0.0f && self->pick_box[1] == 0.0f && self->pick_box[2] == 1.0f && self->pick_box[3] == 1.0f) { dt_boundingbox_t reset = { 0.02f, 0.02f, 0.98f, 0.98f }; @@ -317,6 +328,13 @@ static void _iop_color_picker_pickerdata_ready_callback(gpointer instance, // iops only need new picker data if the pointer has moved if(_record_point_area(picker)) { + // in Area mode, ignore updates where the area is empty (e.g. when using DT_COLOR_PICKER_NO_AUTO + // and the user has not started dragging yet) + if((picker->flags & DT_COLOR_PICKER_AREA) && dt_iop_color_picker_is_area_empty(picker->pick_box)) + { + return; + } + if(!module->blend_data || !blend_color_picker_apply(module, picker->colorpick, pipe)) { if(module->color_picker_apply) diff --git a/src/gui/color_picker_proxy.h b/src/gui/color_picker_proxy.h index 6b58e685daed..7cc0f0454a33 100644 --- a/src/gui/color_picker_proxy.h +++ b/src/gui/color_picker_proxy.h @@ -40,7 +40,9 @@ typedef enum _iop_color_picker_flags_t // only works with 4-channel images DT_COLOR_PICKER_DENOISE = 1 << 2, // all pickers sample input, only ones with this flag set sample output - DT_COLOR_PICKER_IO = 1 << 3 + DT_COLOR_PICKER_IO = 1 << 3, + // don't auto-expand the area, wait for user selection via dragging + DT_COLOR_PICKER_NO_AUTO = 1 << 4 } dt_iop_color_picker_flags_t; typedef struct dt_iop_color_picker_t @@ -71,6 +73,8 @@ typedef struct dt_iop_color_picker_t gboolean dt_iop_color_picker_is_visible(const dt_develop_t *dev); +gboolean dt_iop_color_picker_is_area_empty(const dt_pickerbox_t box); + //* reset current color picker if not keep-active or not keep */ void dt_iop_color_picker_reset(dt_iop_module_t *module, const gboolean keep); diff --git a/src/iop/agx.c b/src/iop/agx.c index 1c1229b7011b..c2116b980db1 100644 --- a/src/iop/agx.c +++ b/src/iop/agx.c @@ -1815,7 +1815,7 @@ static GtkWidget* _create_basic_curve_controls_box(dt_iop_module_t *self, dt_iop_basic_curve_controls_t *controls = &g->basic_curve_controls; // curve_pivot_x_relative_ev with picker - slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE, dt_bauhaus_slider_from_params(section, "curve_pivot_x")); + slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE | DT_COLOR_PICKER_NO_AUTO, dt_bauhaus_slider_from_params(section, "curve_pivot_x")); controls->curve_pivot_x = slider; dt_bauhaus_slider_set_format(slider, _(" EV")); dt_bauhaus_slider_set_digits(slider, 2); @@ -1824,7 +1824,7 @@ static GtkWidget* _create_basic_curve_controls_box(dt_iop_module_t *self, "used to set the pivot relative to mid-gray")); // curve_pivot_y_linear - slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE, dt_bauhaus_slider_from_params(section, "curve_pivot_y_linear_output")); + slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE | DT_COLOR_PICKER_NO_AUTO, dt_bauhaus_slider_from_params(section, "curve_pivot_y_linear_output")); controls->curve_pivot_y_linear = slider; dt_bauhaus_slider_set_format(slider, "%"); dt_bauhaus_slider_set_digits(slider, 2); diff --git a/src/views/darkroom.c b/src/views/darkroom.c index 79024b965534..0831fe53ece2 100644 --- a/src/views/darkroom.c +++ b/src/views/darkroom.c @@ -310,6 +310,10 @@ static void _darkroom_pickers_draw(dt_view_t *self, // overlays are aligned with pixels for a clean look if(sample->size == DT_LIB_COLORPICKER_SIZE_BOX) { + // do not draw if the area is empty + if(dt_iop_color_picker_is_area_empty(sample->box)) + continue; + dt_boundingbox_t fbox; dt_color_picker_transform_box(dev, 2, sample->box, fbox, FALSE); x = fbox[0]; From 0b7292e7418c62c5a97960f258701c3d3b4544fa Mon Sep 17 00:00:00 2001 From: Kofa Date: Sun, 25 Jan 2026 15:12:09 +0100 Subject: [PATCH 2/3] agx: new picker flag: DT_COLOR_PICKER_CALLBACK_ONLY_WHEN_DONE; no box around initial point in NO_AUTO mode --- src/gui/color_picker_proxy.c | 7 ++++++- src/gui/color_picker_proxy.h | 4 +++- src/iop/agx.c | 4 ++-- src/views/darkroom.c | 25 ++++++++++++++++++------- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/gui/color_picker_proxy.c b/src/gui/color_picker_proxy.c index 56a1062c0ab1..216d2fa32d34 100644 --- a/src/gui/color_picker_proxy.c +++ b/src/gui/color_picker_proxy.c @@ -75,7 +75,7 @@ gboolean dt_iop_color_picker_is_visible(const dt_develop_t *dev) gboolean dt_iop_color_picker_is_area_empty(const dt_pickerbox_t box) { - return (fabsf(box[2] - box[0]) < 1e-5f || fabsf(box[3] - box[1]) < 1e-5f); + return (fabsf(box[2] - box[0]) < 1e-3f || fabsf(box[3] - box[1]) < 1e-3f); } static gboolean _record_point_area(dt_iop_color_picker_t *self) @@ -325,6 +325,11 @@ static void _iop_color_picker_pickerdata_ready_callback(gpointer instance, pipe->changed |= DT_DEV_PIPE_REMOVE; pipe->cache_obsolete = TRUE; + if((picker->flags & DT_COLOR_PICKER_CALLBACK_ONLY_WHEN_DONE) && darktable.control->button_down) + { + return; + } + // iops only need new picker data if the pointer has moved if(_record_point_area(picker)) { diff --git a/src/gui/color_picker_proxy.h b/src/gui/color_picker_proxy.h index 7cc0f0454a33..0ea39c8ac27b 100644 --- a/src/gui/color_picker_proxy.h +++ b/src/gui/color_picker_proxy.h @@ -42,7 +42,9 @@ typedef enum _iop_color_picker_flags_t // all pickers sample input, only ones with this flag set sample output DT_COLOR_PICKER_IO = 1 << 3, // don't auto-expand the area, wait for user selection via dragging - DT_COLOR_PICKER_NO_AUTO = 1 << 4 + DT_COLOR_PICKER_NO_AUTO = 1 << 4, + // only invoke the callback when the user releases the button + DT_COLOR_PICKER_CALLBACK_ONLY_WHEN_DONE = 1 << 5 } dt_iop_color_picker_flags_t; typedef struct dt_iop_color_picker_t diff --git a/src/iop/agx.c b/src/iop/agx.c index c2116b980db1..3e85985650bc 100644 --- a/src/iop/agx.c +++ b/src/iop/agx.c @@ -1815,7 +1815,7 @@ static GtkWidget* _create_basic_curve_controls_box(dt_iop_module_t *self, dt_iop_basic_curve_controls_t *controls = &g->basic_curve_controls; // curve_pivot_x_relative_ev with picker - slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE | DT_COLOR_PICKER_NO_AUTO, dt_bauhaus_slider_from_params(section, "curve_pivot_x")); + slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE | DT_COLOR_PICKER_NO_AUTO | DT_COLOR_PICKER_CALLBACK_ONLY_WHEN_DONE, dt_bauhaus_slider_from_params(section, "curve_pivot_x")); controls->curve_pivot_x = slider; dt_bauhaus_slider_set_format(slider, _(" EV")); dt_bauhaus_slider_set_digits(slider, 2); @@ -1824,7 +1824,7 @@ static GtkWidget* _create_basic_curve_controls_box(dt_iop_module_t *self, "used to set the pivot relative to mid-gray")); // curve_pivot_y_linear - slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE | DT_COLOR_PICKER_NO_AUTO, dt_bauhaus_slider_from_params(section, "curve_pivot_y_linear_output")); + slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE | DT_COLOR_PICKER_NO_AUTO | DT_COLOR_PICKER_CALLBACK_ONLY_WHEN_DONE, dt_bauhaus_slider_from_params(section, "curve_pivot_y_linear_output")); controls->curve_pivot_y_linear = slider; dt_bauhaus_slider_set_format(slider, "%"); dt_bauhaus_slider_set_digits(slider, 2); diff --git a/src/views/darkroom.c b/src/views/darkroom.c index 0831fe53ece2..3a8536aecff2 100644 --- a/src/views/darkroom.c +++ b/src/views/darkroom.c @@ -3550,13 +3550,24 @@ int button_pressed(dt_view_t *self, } else { - const float dx = 0.02f; - const float dy = dx * (float)dev->full.pipe->processed_width / (float)dev->full.pipe->processed_height; - const dt_boundingbox_t fbox = { zoom_x - dx, - zoom_y - dy, - zoom_x + dx, - zoom_y + dy }; - dt_color_picker_backtransform_box(dev, 2, fbox, sample->box); + dt_iop_color_picker_t *picker = darktable.lib->proxy.colorpicker.picker_proxy; + + if(picker && (picker->flags & DT_COLOR_PICKER_NO_AUTO)) + { + // don't create a box around the starting point; the user has to explicitly select an area by dragging + const dt_boundingbox_t fbox = { zoom_x, zoom_y, zoom_x, zoom_y }; + dt_color_picker_backtransform_box(dev, 2, fbox, sample->box); + } + else + { + const float dx = 0.02f; + const float dy = dx * (float)dev->full.pipe->processed_width / (float)dev->full.pipe->processed_height; + const dt_boundingbox_t fbox = { zoom_x - dx, + zoom_y - dy, + zoom_x + dx, + zoom_y + dy }; + dt_color_picker_backtransform_box(dev, 2, fbox, sample->box); + } } dt_control_change_cursor(GDK_FLEUR); } From fbad3408df07828b6ff209a63b717a1e49ce61bf Mon Sep 17 00:00:00 2001 From: Kofa Date: Wed, 28 Jan 2026 13:49:25 +0100 Subject: [PATCH 3/3] agx: less intrusive version of DT_COLOR_PICKER_NO_AUTO, and handling picker only when button is released --- src/gui/color_picker_proxy.c | 25 +++++++++++-------------- src/gui/color_picker_proxy.h | 4 +--- src/iop/agx.c | 10 ++++++++-- src/views/darkroom.c | 29 +++++++++++------------------ 4 files changed, 31 insertions(+), 37 deletions(-) diff --git a/src/gui/color_picker_proxy.c b/src/gui/color_picker_proxy.c index 216d2fa32d34..0b1d343035fb 100644 --- a/src/gui/color_picker_proxy.c +++ b/src/gui/color_picker_proxy.c @@ -206,17 +206,19 @@ static gboolean _color_picker_callback_button_press(GtkWidget *button, // pull picker's last recorded positions if(kind & DT_COLOR_PICKER_AREA) { - if(flags & DT_COLOR_PICKER_NO_AUTO) - { - // reset coordinates to 0.0f to create a zero-area box, - // requiring the user to drag-select manually - memset(self->pick_box, 0, sizeof(self->pick_box)); - } - else if( self->pick_box[0] == 0.0f && self->pick_box[1] == 0.0f + if( self->pick_box[0] == 0.0f && self->pick_box[1] == 0.0f && self->pick_box[2] == 1.0f && self->pick_box[3] == 1.0f) { - dt_boundingbox_t reset = { 0.02f, 0.02f, 0.98f, 0.98f }; - dt_color_picker_backtransform_box(darktable.develop, 2, reset, self->pick_box); + if(flags & DT_COLOR_PICKER_NO_AUTO) + { + // reset coordinates to 0.0f to create a zero-area box, + // requiring the user to drag-select manually + memset(self->pick_box, 0, sizeof(self->pick_box)); + } + else { + dt_boundingbox_t reset = { 0.02f, 0.02f, 0.98f, 0.98f }; + dt_color_picker_backtransform_box(darktable.develop, 2, reset, self->pick_box); + } } dt_lib_colorpicker_set_box_area(darktable.lib, self->pick_box); } @@ -325,11 +327,6 @@ static void _iop_color_picker_pickerdata_ready_callback(gpointer instance, pipe->changed |= DT_DEV_PIPE_REMOVE; pipe->cache_obsolete = TRUE; - if((picker->flags & DT_COLOR_PICKER_CALLBACK_ONLY_WHEN_DONE) && darktable.control->button_down) - { - return; - } - // iops only need new picker data if the pointer has moved if(_record_point_area(picker)) { diff --git a/src/gui/color_picker_proxy.h b/src/gui/color_picker_proxy.h index 0ea39c8ac27b..7cc0f0454a33 100644 --- a/src/gui/color_picker_proxy.h +++ b/src/gui/color_picker_proxy.h @@ -42,9 +42,7 @@ typedef enum _iop_color_picker_flags_t // all pickers sample input, only ones with this flag set sample output DT_COLOR_PICKER_IO = 1 << 3, // don't auto-expand the area, wait for user selection via dragging - DT_COLOR_PICKER_NO_AUTO = 1 << 4, - // only invoke the callback when the user releases the button - DT_COLOR_PICKER_CALLBACK_ONLY_WHEN_DONE = 1 << 5 + DT_COLOR_PICKER_NO_AUTO = 1 << 4 } dt_iop_color_picker_flags_t; typedef struct dt_iop_color_picker_t diff --git a/src/iop/agx.c b/src/iop/agx.c index 3e85985650bc..0676dd7be840 100644 --- a/src/iop/agx.c +++ b/src/iop/agx.c @@ -1170,6 +1170,12 @@ static void _read_exposure_params_callback(GtkWidget *widget, // move only the pivot's relative (input) exposure, and recalculate its output based on mid-gray static void _apply_auto_pivot_xy(dt_iop_module_t *self, const dt_iop_order_iccprofile_info_t *profile) { + printf("_apply_auto_pivot_xy, %d\n", darktable.control->button_down); + // don't update while the user is dragging the mouse + if (darktable.control->button_down) { + return; + } + dt_iop_agx_params_t *p = self->params; const dt_iop_agx_gui_data_t *g = self->gui_data; @@ -1815,7 +1821,7 @@ static GtkWidget* _create_basic_curve_controls_box(dt_iop_module_t *self, dt_iop_basic_curve_controls_t *controls = &g->basic_curve_controls; // curve_pivot_x_relative_ev with picker - slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE | DT_COLOR_PICKER_NO_AUTO | DT_COLOR_PICKER_CALLBACK_ONLY_WHEN_DONE, dt_bauhaus_slider_from_params(section, "curve_pivot_x")); + slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE, dt_bauhaus_slider_from_params(section, "curve_pivot_x")); controls->curve_pivot_x = slider; dt_bauhaus_slider_set_format(slider, _(" EV")); dt_bauhaus_slider_set_digits(slider, 2); @@ -1824,7 +1830,7 @@ static GtkWidget* _create_basic_curve_controls_box(dt_iop_module_t *self, "used to set the pivot relative to mid-gray")); // curve_pivot_y_linear - slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE | DT_COLOR_PICKER_NO_AUTO | DT_COLOR_PICKER_CALLBACK_ONLY_WHEN_DONE, dt_bauhaus_slider_from_params(section, "curve_pivot_y_linear_output")); + slider = dt_color_picker_new(self, DT_COLOR_PICKER_AREA | DT_COLOR_PICKER_DENOISE | DT_COLOR_PICKER_NO_AUTO, dt_bauhaus_slider_from_params(section, "curve_pivot_y_linear_output")); controls->curve_pivot_y_linear = slider; dt_bauhaus_slider_set_format(slider, "%"); dt_bauhaus_slider_set_digits(slider, 2); diff --git a/src/views/darkroom.c b/src/views/darkroom.c index 3a8536aecff2..21d487cdb234 100644 --- a/src/views/darkroom.c +++ b/src/views/darkroom.c @@ -3446,6 +3446,10 @@ int button_released(dt_view_t *self, int handled = 0; if(dt_iop_color_picker_is_visible(dev) && which == GDK_BUTTON_PRIMARY) { + // force an update on release so modules can detect the finish event + if(darktable.lib->proxy.colorpicker.picker_proxy) + darktable.lib->proxy.colorpicker.picker_proxy->changed = TRUE; + // only sample box picker at end, for speed if(darktable.lib->proxy.colorpicker.primary_sample->size == DT_LIB_COLORPICKER_SIZE_BOX) { @@ -3550,24 +3554,13 @@ int button_pressed(dt_view_t *self, } else { - dt_iop_color_picker_t *picker = darktable.lib->proxy.colorpicker.picker_proxy; - - if(picker && (picker->flags & DT_COLOR_PICKER_NO_AUTO)) - { - // don't create a box around the starting point; the user has to explicitly select an area by dragging - const dt_boundingbox_t fbox = { zoom_x, zoom_y, zoom_x, zoom_y }; - dt_color_picker_backtransform_box(dev, 2, fbox, sample->box); - } - else - { - const float dx = 0.02f; - const float dy = dx * (float)dev->full.pipe->processed_width / (float)dev->full.pipe->processed_height; - const dt_boundingbox_t fbox = { zoom_x - dx, - zoom_y - dy, - zoom_x + dx, - zoom_y + dy }; - dt_color_picker_backtransform_box(dev, 2, fbox, sample->box); - } + const float dx = 0.02f; + const float dy = dx * (float)dev->full.pipe->processed_width / (float)dev->full.pipe->processed_height; + const dt_boundingbox_t fbox = { zoom_x - dx, + zoom_y - dy, + zoom_x + dx, + zoom_y + dy }; + dt_color_picker_backtransform_box(dev, 2, fbox, sample->box); } dt_control_change_cursor(GDK_FLEUR); }