From 31ad20bd8c11aef628de8313cde6d4da3589c7f1 Mon Sep 17 00:00:00 2001 From: stnKrisna Date: Fri, 15 May 2026 17:05:14 +1000 Subject: [PATCH 1/5] Allow crop by scrolling --- src/iop/crop.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/iop/crop.c b/src/iop/crop.c index 8943775d7efc..d19ef6648cfb 100644 --- a/src/iop/crop.c +++ b/src/iop/crop.c @@ -107,6 +107,8 @@ typedef struct dt_iop_crop_gui_data_t gboolean preview_ready; gint64 focus_time; dt_gui_collapsible_section_t cs; + + float scroll_origin_x, scroll_origin_y; } dt_iop_crop_gui_data_t; typedef struct dt_iop_crop_data_t @@ -1263,6 +1265,8 @@ void gui_init(dt_iop_module_t *self) g->shift_hold = FALSE; g->ctrl_hold = FALSE; g->preview_ready = FALSE; + g->scroll_origin_x = -1.0f; + g->scroll_origin_y = -1.0f; dt_iop_crop_aspect_t aspects[] = { { _("freehand"), 0, 0 }, @@ -1663,6 +1667,62 @@ void gui_post_expose(dt_iop_module_t *self, cairo_stroke(cr); } +int scrolled(dt_iop_module_t *self, + const float pzx, + const float pzy, + const int up, + const uint32_t state) +{ + dt_iop_crop_gui_data_t *g = self->gui_data; + + if(!g->preview_ready || self->dev->preview_pipe->loading) return 0; + + // handle resize only when pointer is inside the crop rectangle + if(pzx < g->clip_x || pzx > g->clip_x + g->clip_w + || pzy < g->clip_y || pzy > g->clip_y + g->clip_h) + return 0; + + _set_max_clip(self); + + // clamp min size so we never lose the crop rect entirely + const float min_cw = MAX(MIN_CROP_SIZE, g->clip_max_w / 100.0f); + const float min_ch = MAX(MIN_CROP_SIZE, g->clip_max_h / 100.0f); + + // compute scale factor (same 0.97 exponential scaling that masks use) + const float factor = dt_mask_scroll_increases(up) ? 1.0f / 0.97f : 0.97f; + + float new_cw = g->clip_w * factor; + float new_ch = g->clip_h * factor; + + // clamp size to limits + new_cw = CLAMP(new_cw, min_cw, g->clip_max_w); + new_ch = CLAMP(new_ch, min_ch, g->clip_max_h); + + // scale around pointer position so that point under cursor stays fixed + float new_cx = pzx + (g->clip_x - pzx) * (new_cw / g->clip_w); + float new_cy = pzy + (g->clip_y - pzy) * (new_ch / g->clip_h); + + // clamp against max clip bounds + new_cx = CLAMP(new_cx, g->clip_max_x, g->clip_max_x + g->clip_max_w - new_cw); + new_cy = CLAMP(new_cy, g->clip_max_y, g->clip_max_y + g->clip_max_h - new_ch); + + g->clip_x = new_cx; + g->clip_y = new_cy; + g->clip_w = new_cw; + g->clip_h = new_ch; + + // enforce aspect ratio + _aspect_apply(self, GRAB_HORIZONTAL | GRAB_VERTICAL); + + // update sliders without triggering pixelpipe re-run + ++darktable.gui->reset; + _update_sliders_and_limit(g); + --darktable.gui->reset; + + dt_control_queue_redraw_center(); + return 1; +} + int mouse_moved(dt_iop_module_t *self, const float pzx, const float pzy, @@ -1945,6 +2005,8 @@ GSList *mouse_actions(dt_iop_module_t *self) _("[%s on borders] crop"), self->name()); lm = dt_mouse_action_create_format(lm, DT_MOUSE_ACTION_LEFT_DRAG, GDK_SHIFT_MASK, _("[%s on borders] crop keeping ratio"), self->name()); + lm = dt_mouse_action_create_format(lm, DT_MOUSE_ACTION_SCROLL, 0, + _("[%s] resize on hover"), self->name()); return lm; } From f1cb246ee3b3cb6a630efa38ff7d0f1195fb6012 Mon Sep 17 00:00:00 2001 From: stnKrisna Date: Fri, 15 May 2026 17:12:29 +1000 Subject: [PATCH 2/5] Allow crop by drawing rectangle --- src/iop/crop.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/src/iop/crop.c b/src/iop/crop.c index d19ef6648cfb..7a0c13386a67 100644 --- a/src/iop/crop.c +++ b/src/iop/crop.c @@ -109,6 +109,9 @@ typedef struct dt_iop_crop_gui_data_t dt_gui_collapsible_section_t cs; float scroll_origin_x, scroll_origin_y; + + gboolean new_crop_active; + float new_crop_x0, new_crop_y0; } dt_iop_crop_gui_data_t; typedef struct dt_iop_crop_data_t @@ -1267,6 +1270,7 @@ void gui_init(dt_iop_module_t *self) g->preview_ready = FALSE; g->scroll_origin_x = -1.0f; g->scroll_origin_y = -1.0f; + g->new_crop_active = FALSE; dt_iop_crop_aspect_t aspects[] = { { _("freehand"), 0, 0 }, @@ -1745,6 +1749,68 @@ int mouse_moved(dt_iop_module_t *self, if(darktable.control->button_down && darktable.control->button_down_which == GDK_BUTTON_PRIMARY) { + // Alt+left-drag: draw a new rectangle constrained to current aspect ratio + if(g->new_crop_active) + { + const float dx = pzx - g->new_crop_x0; + const float dy = pzy - g->new_crop_y0; + const float adx = fabsf(dx); + const float ady = fabsf(dy); + + const float aspect = _aspect_ratio_get(self, g->aspect_presets); + + float new_cx, new_cy, new_cw, new_ch; + + if(aspect > 0.0f) + { + // constrained to current aspect ratio + if(adx * aspect >= ady) + { + // width-driven: width follows mouse, height derived from ratio + new_cw = adx; + new_ch = adx / aspect; + } + else + { + // height-driven: height follows mouse, width derived from ratio + new_ch = ady; + new_cw = ady * aspect; + } + new_cx = dx > 0.0f ? g->new_crop_x0 : g->new_crop_x0 - new_cw; + new_cy = dy > 0.0f ? g->new_crop_y0 : g->new_crop_y0 - new_ch; + } + else + { + // freehand — no aspect constraint + new_cx = fminf(g->new_crop_x0, pzx); + new_cy = fminf(g->new_crop_y0, pzy); + new_cw = adx; + new_ch = ady; + } + + // clamp against max clip bounds + new_cx = CLAMP(new_cx, g->clip_max_x, + g->clip_max_x + g->clip_max_w - MIN_CROP_SIZE); + new_cy = CLAMP(new_cy, g->clip_max_y, + g->clip_max_y + g->clip_max_h - MIN_CROP_SIZE); + new_cw = CLAMP(new_cw, MIN_CROP_SIZE, + g->clip_max_x + g->clip_max_w - new_cx); + new_ch = CLAMP(new_ch, MIN_CROP_SIZE, + g->clip_max_y + g->clip_max_h - new_cy); + + g->clip_x = new_cx; + g->clip_y = new_cy; + g->clip_w = new_cw; + g->clip_h = new_ch; + + ++darktable.gui->reset; + _update_sliders_and_limit(g); + --darktable.gui->reset; + + dt_control_queue_redraw_center(); + return 1; + } + // draw a light gray frame, to show it's not stored yet: // first mouse button, adjust cropping frame, but what do we do? const float bzx = g->button_down_zoom_x; @@ -1911,6 +1977,13 @@ int button_released(dt_iop_module_t *self, // we don't do anything if the image is not ready if(!g->preview_ready) return 0; + if(g->new_crop_active) + { + g->new_crop_active = FALSE; + _commit_box(self, g, p, FALSE); + return 1; + } + /* reset internal ui states*/ g->shift_hold = FALSE; g->ctrl_hold = FALSE; @@ -1943,6 +2016,15 @@ int button_pressed(dt_iop_module_t *self, if(which == GDK_BUTTON_PRIMARY) { + // Alt+left-click+drag draws a new rectangle constrained to aspect ratio + if(dt_modifiers_include(state, GDK_MOD1_MASK)) + { + g->new_crop_active = TRUE; + g->new_crop_x0 = bzx; + g->new_crop_y0 = bzy; + return 1; + } + float wd, ht; dt_dev_get_preview_size(self->dev, &wd, &ht); @@ -2007,6 +2089,8 @@ GSList *mouse_actions(dt_iop_module_t *self) _("[%s on borders] crop keeping ratio"), self->name()); lm = dt_mouse_action_create_format(lm, DT_MOUSE_ACTION_SCROLL, 0, _("[%s] resize on hover"), self->name()); + lm = dt_mouse_action_create_format(lm, DT_MOUSE_ACTION_LEFT_DRAG, GDK_MOD1_MASK, + _("[%s] new crop rectangle"), self->name()); return lm; } From 43a374022462eb06127e4b1337d2b2754d7c97f4 Mon Sep 17 00:00:00 2001 From: stnKrisna Date: Sat, 16 May 2026 14:45:26 +1000 Subject: [PATCH 3/5] Assign shortcut x to flip aspect ratio --- src/iop/crop.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/iop/crop.c b/src/iop/crop.c index 7a0c13386a67..977feaf63d59 100644 --- a/src/iop/crop.c +++ b/src/iop/crop.c @@ -1204,6 +1204,16 @@ static void _event_aspect_flip(GtkWidget *button, dt_iop_module_t *self) _commit_box(self, g, p, TRUE); } +static void _crop_flip_aspect_shortcut(dt_action_t *action) +{ + dt_action_t *owner = action->owner; + if(!owner || owner->type != DT_ACTION_TYPE_IOP) return; + dt_iop_module_t *self = dt_iop_get_module_preferred_instance((dt_iop_module_so_t *)owner); + if(!self) return; + _event_key_swap(self); + _commit_box(self, self->gui_data, self->params, TRUE); +} + static gint _aspect_ratio_cmp(const dt_iop_crop_aspect_t *a, const dt_iop_crop_aspect_t *b) { @@ -1450,6 +1460,8 @@ void gui_init(dt_iop_module_t *self) darktable.develop->cropping.flip_callback = _crop_handle_flip; dt_shortcut_register(DT_ACTION(self->so), 0, 0, GDK_KEY_c, 0); + dt_action_register(DT_ACTION(self->so), N_("flip aspect ratio"), + _crop_flip_aspect_shortcut, GDK_KEY_x, 0); } static void _aspect_free(gpointer data) From 823fecbeda7f70787bb7a2e37a9d185db477fff3 Mon Sep 17 00:00:00 2001 From: stnKrisna Date: Mon, 18 May 2026 20:05:04 +1000 Subject: [PATCH 4/5] Reset crop on mouse up --- src/iop/crop.c | 38 +++++++++++++++++++++++++++++++------- src/views/darkroom.c | 19 ++++++++++++++++--- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/iop/crop.c b/src/iop/crop.c index 977feaf63d59..345237484156 100644 --- a/src/iop/crop.c +++ b/src/iop/crop.c @@ -1751,6 +1751,10 @@ int mouse_moved(dt_iop_module_t *self, // we don't do anything if the image is not ready if(!g->preview_ready || self->dev->preview_pipe->loading) return 0; + // claim right-click drag to prevent proxy.rotate (straighten horizon) from interfering + if(darktable.control->button_down && darktable.control->button_down_which == GDK_BUTTON_SECONDARY) + return 1; + float wd, ht; dt_dev_get_preview_size(self->dev, &wd, &ht); @@ -1989,6 +1993,18 @@ int button_released(dt_iop_module_t *self, // we don't do anything if the image is not ready if(!g->preview_ready) return 0; + if(which == GDK_BUTTON_SECONDARY) + { + // we reset cropping on RMB release + g->clip_x = 0.0f; + g->clip_y = 0.0f; + g->clip_w = 1.0f; + g->clip_h = 1.0f; + _aspect_apply(self, GRAB_BOTTOM_RIGHT); + gui_changed(self, NULL, NULL); + return 1; + } + if(g->new_crop_active) { g->new_crop_active = FALSE; @@ -2079,13 +2095,21 @@ int button_pressed(dt_iop_module_t *self, } else if(which == GDK_BUTTON_SECONDARY) { - // we reset cropping - g->clip_x = 0.0f; - g->clip_y = 0.0f; - g->clip_w = 1.0f; - g->clip_h = 1.0f; - _aspect_apply(self, GRAB_BOTTOM_RIGHT); - gui_changed(self, NULL, NULL); + float wd, ht; + dt_dev_get_preview_size(self->dev, &wd, &ht); + + // switch module on already, other code depends in this: + if(!self->enabled) + dt_dev_add_history_item(darktable.develop, self, TRUE); + + g->button_down_zoom_x = bzx; + g->button_down_zoom_y = bzy; + + /* update prev clip box with current */ + g->prev_clip_x = g->clip_x; + g->prev_clip_y = g->clip_y; + g->prev_clip_w = g->clip_w; + g->prev_clip_h = g->clip_h; return 1; } else diff --git a/src/views/darkroom.c b/src/views/darkroom.c index 679ebce87905..3573c77afd82 100644 --- a/src/views/darkroom.c +++ b/src/views/darkroom.c @@ -513,6 +513,21 @@ static void _module_gui_post_expose(dt_iop_module_t *module, cairo_restore(cri); } +// returns TRUE when we should skip all module overlays and only draw the rotation line +static gboolean _should_skip_overlays_for_rotation(const dt_iop_module_t *const dmod, + const dt_develop_t *const dev) +{ + if(dmod && !strcmp(dmod->op, "crop")) + { + return FALSE; + } + if(!dev->proxy.rotate) return FALSE; + if(dmod == dev->proxy.rotate) return TRUE; + if(darktable.control->button_down_which == GDK_BUTTON_SECONDARY) + return TRUE; + return FALSE; +} + static void _view_paint_surface(cairo_t *cr, const size_t width, const size_t height, @@ -856,9 +871,7 @@ void expose(dt_view_t *self, } // if dragging the rotation line, do it and nothing else - if(dev->proxy.rotate - && (darktable.control->button_down_which == GDK_BUTTON_SECONDARY - || dmod == dev->proxy.rotate)) + if(_should_skip_overlays_for_rotation(dmod, dev)) { // reminder, we want this to be exposed always for guidings if(dev->proxy.rotate && dev->proxy.rotate->gui_post_expose) From 23b078accaf354537ea2a7a384421f19e7e432a4 Mon Sep 17 00:00:00 2001 From: stnKrisna Date: Mon, 18 May 2026 22:56:59 +1000 Subject: [PATCH 5/5] Implement RMB+drag to create crop rect --- src/iop/crop.c | 164 +++++++++++++++++++++++-------------------------- 1 file changed, 77 insertions(+), 87 deletions(-) diff --git a/src/iop/crop.c b/src/iop/crop.c index 345237484156..311e4acd5bcd 100644 --- a/src/iop/crop.c +++ b/src/iop/crop.c @@ -1753,7 +1753,18 @@ int mouse_moved(dt_iop_module_t *self, // claim right-click drag to prevent proxy.rotate (straighten horizon) from interfering if(darktable.control->button_down && darktable.control->button_down_which == GDK_BUTTON_SECONDARY) - return 1; + { + if(!g->new_crop_active) + { + // only activate when the mouse actually moves (click without drag = reset) + const float dx = pzx - g->new_crop_x0; + const float dy = pzy - g->new_crop_y0; + if(dx * dx + dy * dy > 0.0f) + g->new_crop_active = TRUE; + else + return 1; + } + } float wd, ht; dt_dev_get_preview_size(self->dev, &wd, &ht); @@ -1763,70 +1774,69 @@ int mouse_moved(dt_iop_module_t *self, _set_max_clip(self); - if(darktable.control->button_down && darktable.control->button_down_which == GDK_BUTTON_PRIMARY) + if(g->new_crop_active) { - // Alt+left-drag: draw a new rectangle constrained to current aspect ratio - if(g->new_crop_active) - { - const float dx = pzx - g->new_crop_x0; - const float dy = pzy - g->new_crop_y0; - const float adx = fabsf(dx); - const float ady = fabsf(dy); + const float dx = pzx - g->new_crop_x0; + const float dy = pzy - g->new_crop_y0; + const float adx = fabsf(dx); + const float ady = fabsf(dy); - const float aspect = _aspect_ratio_get(self, g->aspect_presets); + const float aspect = _aspect_ratio_get(self, g->aspect_presets); - float new_cx, new_cy, new_cw, new_ch; + float new_cx, new_cy, new_cw, new_ch; - if(aspect > 0.0f) + if(aspect > 0.0f) + { + // constrained to current aspect ratio + if(adx * aspect >= ady) { - // constrained to current aspect ratio - if(adx * aspect >= ady) - { - // width-driven: width follows mouse, height derived from ratio - new_cw = adx; - new_ch = adx / aspect; - } - else - { - // height-driven: height follows mouse, width derived from ratio - new_ch = ady; - new_cw = ady * aspect; - } - new_cx = dx > 0.0f ? g->new_crop_x0 : g->new_crop_x0 - new_cw; - new_cy = dy > 0.0f ? g->new_crop_y0 : g->new_crop_y0 - new_ch; + // width-driven: width follows mouse, height derived from ratio + new_cw = adx; + new_ch = adx / aspect; } else { - // freehand — no aspect constraint - new_cx = fminf(g->new_crop_x0, pzx); - new_cy = fminf(g->new_crop_y0, pzy); - new_cw = adx; + // height-driven: height follows mouse, width derived from ratio new_ch = ady; + new_cw = ady * aspect; } - - // clamp against max clip bounds - new_cx = CLAMP(new_cx, g->clip_max_x, - g->clip_max_x + g->clip_max_w - MIN_CROP_SIZE); - new_cy = CLAMP(new_cy, g->clip_max_y, - g->clip_max_y + g->clip_max_h - MIN_CROP_SIZE); - new_cw = CLAMP(new_cw, MIN_CROP_SIZE, - g->clip_max_x + g->clip_max_w - new_cx); - new_ch = CLAMP(new_ch, MIN_CROP_SIZE, - g->clip_max_y + g->clip_max_h - new_cy); - - g->clip_x = new_cx; - g->clip_y = new_cy; - g->clip_w = new_cw; - g->clip_h = new_ch; - - ++darktable.gui->reset; - _update_sliders_and_limit(g); - --darktable.gui->reset; - - dt_control_queue_redraw_center(); - return 1; + new_cx = dx > 0.0f ? g->new_crop_x0 : g->new_crop_x0 - new_cw; + new_cy = dy > 0.0f ? g->new_crop_y0 : g->new_crop_y0 - new_ch; } + else + { + // freehand — no aspect constraint + new_cx = fminf(g->new_crop_x0, pzx); + new_cy = fminf(g->new_crop_y0, pzy); + new_cw = adx; + new_ch = ady; + } + + // clamp against max clip bounds + new_cx = CLAMP(new_cx, g->clip_max_x, + g->clip_max_x + g->clip_max_w - MIN_CROP_SIZE); + new_cy = CLAMP(new_cy, g->clip_max_y, + g->clip_max_y + g->clip_max_h - MIN_CROP_SIZE); + new_cw = CLAMP(new_cw, MIN_CROP_SIZE, + g->clip_max_x + g->clip_max_w - new_cx); + new_ch = CLAMP(new_ch, MIN_CROP_SIZE, + g->clip_max_y + g->clip_max_h - new_cy); + + g->clip_x = new_cx; + g->clip_y = new_cy; + g->clip_w = new_cw; + g->clip_h = new_ch; + + ++darktable.gui->reset; + _update_sliders_and_limit(g); + --darktable.gui->reset; + dt_control_queue_redraw_center(); + return 1; + } + + if(darktable.control->button_down && darktable.control->button_down_which == GDK_BUTTON_PRIMARY) + { // draw a light gray frame, to show it's not stored yet: // first mouse button, adjust cropping frame, but what do we do? const float bzx = g->button_down_zoom_x; @@ -1993,9 +2003,16 @@ int button_released(dt_iop_module_t *self, // we don't do anything if the image is not ready if(!g->preview_ready) return 0; + if(g->new_crop_active) + { + g->new_crop_active = FALSE; + _commit_box(self, g, p, FALSE); + return 1; + } + if(which == GDK_BUTTON_SECONDARY) { - // we reset cropping on RMB release + // we reset cropping on RMB click (release without drag) g->clip_x = 0.0f; g->clip_y = 0.0f; g->clip_w = 1.0f; @@ -2005,13 +2022,6 @@ int button_released(dt_iop_module_t *self, return 1; } - if(g->new_crop_active) - { - g->new_crop_active = FALSE; - _commit_box(self, g, p, FALSE); - return 1; - } - /* reset internal ui states*/ g->shift_hold = FALSE; g->ctrl_hold = FALSE; @@ -2044,15 +2054,6 @@ int button_pressed(dt_iop_module_t *self, if(which == GDK_BUTTON_PRIMARY) { - // Alt+left-click+drag draws a new rectangle constrained to aspect ratio - if(dt_modifiers_include(state, GDK_MOD1_MASK)) - { - g->new_crop_active = TRUE; - g->new_crop_x0 = bzx; - g->new_crop_y0 = bzy; - return 1; - } - float wd, ht; dt_dev_get_preview_size(self->dev, &wd, &ht); @@ -2095,21 +2096,10 @@ int button_pressed(dt_iop_module_t *self, } else if(which == GDK_BUTTON_SECONDARY) { - float wd, ht; - dt_dev_get_preview_size(self->dev, &wd, &ht); - - // switch module on already, other code depends in this: - if(!self->enabled) - dt_dev_add_history_item(darktable.develop, self, TRUE); - - g->button_down_zoom_x = bzx; - g->button_down_zoom_y = bzy; - - /* update prev clip box with current */ - g->prev_clip_x = g->clip_x; - g->prev_clip_y = g->clip_y; - g->prev_clip_w = g->clip_w; - g->prev_clip_h = g->clip_h; + g->new_crop_x0 = bzx; + g->new_crop_y0 = bzy; + if(!self->enabled) + dt_dev_add_history_item(darktable.develop, self, TRUE); return 1; } else @@ -2125,8 +2115,8 @@ GSList *mouse_actions(dt_iop_module_t *self) _("[%s on borders] crop keeping ratio"), self->name()); lm = dt_mouse_action_create_format(lm, DT_MOUSE_ACTION_SCROLL, 0, _("[%s] resize on hover"), self->name()); - lm = dt_mouse_action_create_format(lm, DT_MOUSE_ACTION_LEFT_DRAG, GDK_MOD1_MASK, - _("[%s] new crop rectangle"), self->name()); + lm = dt_mouse_action_create_format(lm, DT_MOUSE_ACTION_RIGHT_DRAG, 0, + _("[%s] draw new crop rectangle"), self->name()); return lm; }