gtk4-prep: event controller & shortcut fixes - #21745
Conversation
… buttons The gtk4-prep migration replaced the picker buttons' GObject button-press-event handler with a CAPTURE-phase GtkGestureMultiPress that claims the event sequence (the button's own internal bubble-phase gesture would otherwise toggle it a second time). Shortcut activation however still goes through dt_action_def_toggle, whose generic processor (_action_process_toggle) synthesizes GObject button-press-event signals: those only reach the widget class handler (bubble-phase controllers) and carry no device, so they can never trigger the CAPTURE-phase gesture -- picker shortcuts bound to standalone toggle buttons (e.g. AgX 'auto tune levels') silently stopped working. Give standalone picker buttons their own action definition (like bauhaus widgets have dt_action_def_bauhaus) whose process calls the same shared entry as real clicks, so shortcuts reach the picker directly without event synthesis. Update the five modules that define standalone picker buttons (AgX, channel mixer RGB, negadoctor, tone curve) to use it. Real clicks still go through the CAPTURE gesture, which now also passes the clicked button through so a secondary click still switches to area mode.
…efinition The earlier fix updated only the single-line dt_action_define_iop calls; multi-line call sites had the same latent shortcut bug. Borders (border and frame line color), watermark, the RGB levels pickers (macro) and the primary colorpicker lib button all used dt_action_def_toggle with a standalone toggle button, so picker shortcuts could not reach their CAPTURE-phase gesture either. Switch them all to dt_action_def_color_picker and drop the synthetic GdkEventButton construction in the shared callback: the picker logic only cares about ctrl/right-click, so pass those as explicit flags from the three activation paths (bauhaus quad, gesture, shortcut).
A key with a hold shortcut (e.g. the default 'a' = force pan/zoom/rotate with mouse) could never trigger a double/triple-press shortcut bound to the same key: every press re-engaged the hold and returned early, so the double/triple press detection never ran. Now the hold press is recorded in the shortcut state, and a fast consecutive press of the same key - while a double/triple press shortcut exists for it (checked via _shortcut_has_double_triple_press()) - falls through to the normal handling and is detected as a double/triple press instead of re-engaging the hold.
…lick The gtk4-prep event-controller conversion dropped the early return that terminated the secondary-button endpoint-reset branch in the four curve editors (tone curve, rgb curves, base curve, color zones). Control fell through into the node-removal loop, so right-clicking an endpoint reset it AND silently decremented the persisted node count. The branch became reachable once dt_gui_connect_click() started listening to all buttons (right-click no longer filtered out), so this is a live data-corruption bug, not a latent one.
The gesture callback _gui_reset_clicked called dt_dev_add_history_item() twice in a row, so one reset click produced two undo records and ran the pipeline TOP_CHANGED invalidation / history autosave path twice. Same fix as PR darktable-org#21727.
The dt_gui_get_scroll_unit_delta(s)_fallback() helpers re-read the raw GdkEventScroll through dt_gui_get_scroll_unit_deltas(), whose static accumulator feeds off the same smooth-scroll stream the DISCRETE scroll proxy already accumulated. The two accumulators phase-drift, so the fallback returned FALSE with delta 0 on real proxy steps, and: - the bauhaus popup slider called _slider_zoom_range(w, 0) unconditionally, triggering the middle-click 'reset zoom range' (soft<->hard bounds toggle) on every sub-threshold trackpad scroll; - the module presets scroller called dt_gui_presets_apply_adjacent_preset(module, 0), which matched no row (LIMIT 0) but still toasted a misleading "preset 'x' (last)". The DISCRETE proxy already delivers integer unit steps, so both handlers now use the proxy dx/dy directly with a delta != 0 guard (sign conventions match the pre-migration dt_gui_get_scroll_unit_delta path), and the fallback helpers are deleted - they were dead code on GTK3, where the current event is always present inside controller callbacks.
GtkGestureMultiPress on GTK3 counts n_press by itself: GDK delivers a
second GDK_BUTTON_PRESS before GDK_2BUTTON_PRESS for the second click of a
double-click (gdk_synthesize_click appends a copy, gdkevents.c), and
gtk_gesture_multi_press_begin() emits pressed(n_press=2) for it
(gtkgesturemultipress.c). The in-tree comment claiming GDK never delivers
a second GDK_BUTTON_PRESS was wrong.
The dt_gui_connect_double_click() bridge therefore fired the press callback
a second time on every double/triple click: the gesture's own n_press=2 call
plus the bridge's GDK_2BUTTON_PRESS forward. In the thumbtable and culling
handlers this ran dt_view_manager_switch("darkroom") twice (a same-view
switch still tears down and rebuilds every expandable lib plugin) and the
filmstrip deselect/select/ACTIVATE twice.
Deleted the whole bridge (DblClkData, _dbl_clk_free, _dbl_clk_handler,
dt_gui_connect_double_click/disconnect, macro, both call sites) and
simplified the callbacks' gesture?get_current_button:GDK_BUTTON_PRIMARY
ternaries to a direct gtk_gesture_single_get_current_button() call.
Also makes dt_gui_connect_click_all() a plain alias of
dt_gui_connect_click(): the base function already sets button=0, so the
redundant second gtk_gesture_single_set_button() is gone.
gtk_get_current_event() returns an owned copy on GTK3 (transfer full); every copy must be released with gdk_event_free(). The event-controller migration introduced copies in callbacks that never freed them, leaking one GdkEvent per event: - culling button-press and motion-notify callbacks (root coordinates) - thumbtable motion-notify callback (root coordinates) - darkroom second-window scroll callback (all return paths) - colorlabels secondary-click branch - shortcuts-dialog key handler (accelerators) - metadata textview key handler - collection treeview click handler - all four return paths (this one was missed by the panel review) - gtk_menu_popup_at_pointer(menu, gtk_get_current_event()) in collect and masks New helper dt_gui_get_current_root_coords() (gtk.h/gtk.c) centralizes the get/use/free dance for the four root-coordinate sites; it carries a GTK4 migration note (gtk_get_current_event() disappears, use the borrowed gesture/controller events).
…ease handlers The pre-migration button-release handlers required GDK_BUTTON_PRIMARY; the event-controller conversion dropped the test while connecting the handlers through dt_gui_connect_click_all() (button=0), so middle- and right-button releases started changing the image selection: - thumbtable: right/middle release over a thumbnail ran dt_selection_toggle/select_range/select_single; - culling: middle-click zoom also flipped table->selection, changing which image later act-on operations apply to. Both release handlers now check gtk_gesture_single_get_current_button(gesture) == GDK_BUTTON_PRIMARY again.
The shortcut dispatcher (accelerators.c) activates gesture-driven widgets (DT_ACTION_GESTURE_KEY, e.g. every dt_iop_togglebutton_new widget, the masks-lib shape buttons, the ashift fit/structure buttons) by emitting a bare "pressed" signal on the stored GtkGestureMultiPress. The action effect was never translated into that synthetic press, so the ctrl-, right-button and long-press effect variants (dt_action_effect_toggle's TOGGLE_CTRL/ON_CTRL/TOGGLE_RIGHT/ON_RIGHT, dt_action_effect_activate's ACTIVATE_CTRL/ACTIVATE_RIGHT) collapsed into a plain primary click. Effects: - ashift fit_v/fit_h/fit_both: the callback read dt_key_modifier_state() (live keyboard state), so a shortcut bound to ctrl+<key> with the plain activate effect ran the ctrl fit variant (rotation-only) and the plain fit became unreachable from that shortcut; pre-migration the synthetic event carried the effect-encoded state. - blend_gui _blendop_masks_add_shape / _blendop_blendif_showmask_clicked / _blendop_masks_show_and_edit and masks _bt_add_shape_cb read gtk_get_current_event_state(), which during shortcut dispatch is the key event, so ctrl-toggle/ctrl-on effects behaved like plain toggle. - retouch rt_add_shape_callback (continuous creation), retouch rt_select_algorithm_callback (ctrl restricts algorithm switch) and the spots shape-add callback had the same pattern (missed by the review). Fix: the dispatcher encodes the effect as (state << 8) | button into object data on the gesture (DT_ACTION_GESTURE_SYNTH_KEY) around the synthetic "pressed" emit; dt_gui_current_button() (gtk.h) consults it first (right effects present as GDK_BUTTON_SECONDARY) and the new dt_gui_current_state() helper returns the effect-determined state, falling back to the real event state for genuine clicks. All affected callbacks migrated to the helpers. Also fixes the retouch editing-toggle regression: rt_edit_masks_callback read gtk_get_current_event() (the key event during dispatch, button 0), so a keyboard shortcut on retouch > editing only cleared the shape toggles and never entered/left mask-edit mode; the helpers return a primary button for shortcut presses. That callback also leaked its event copy - removed.
…oundaries A continuous trackpad scroll is a series of swipes, and each swipe ends with a short opposite-sign stream (fingers lifting/rebounding) that the DISCRETE scroll proxy accumulates into a step back. At the first/last preset this oscillated between the boundary preset and its neighbour (e.g. 1<->2, 9<->10) and spammed the "(first)"/"(last)" toast. Ignore a direction reversal that arrives within 250 ms of the previous step: a real direction change comes from a new gesture and is slower.
…urve node The curve editors' press handlers return no value, so a right-click on a curve node bubbled up to the module body's right-click handler, which opens the presets popup - while the node was being deleted/reset at the same time. The pre-migration button-press handlers returned TRUE for node actions, stopping GTK signal propagation. The curve gestures now claim the event sequence when handling a node right-click, consuming the event so the body handler never runs. Clicks on empty curve area still propagate (matching the pre-migration FALSE return, which also opened the presets menu there).
Arecsu
changed the title
Arecsu
changed the title
Arecsu
changed the title
Important (accepted, medium)
Minor (accepted, low)Both entries below concern the same new preset-scroll dampener in
Contested (reviewers held positions — you decide)
|
… gesture presses The effect values DT_ACTION_EFFECT_ON/OFF alias ACTIVATE_CTRL/ACTIVATE_RIGHT (action.h), so the shared _action_effect_button_state() encoded a toggle's plain on/off shortcut as ctrl+click/right-click on gesture-backed toggle buttons. A scroll- or move-bound shortcut then ran the wrong variant or nothing at all (masks shape-add dropped the off variant via its primary- button guard; ashift structure-auto behaved identically). The caller's action definition now selects the mapping: toggles deliver plain on/off as a plain primary click (matching the retained synthetic branch), buttons keep the ctrl/right encoding for ACTIVATE_CTRL/ACTIVATE_RIGHT.
… double-press window Engaging a hold shortcut stores the key in _sc (needed for the fast second-press double/triple detection and for move routing while held), but dt_shortcut_key_release() returned before _delay_for_double_triple(), so _key_release_delayed() - the only normal-path _sc reset - was never scheduled for a hold key. With stock defaults (a/w are hold-bound) one hold followed by a wheel scroll resolved the scroll as "that key + move" and toggled darkroom_skip_mouse_events instead of zooming. The hold release now schedules the same delayed cleanup as a normal key release: _sc.key stays valid for the double/triple-press window and is cleared once it elapses. A _hold_release_pending flag keeps the delayed pass from re-dispatching the shortcut (the hold action already ran ON/OFF), and _interrupt_delayed_release() drops the stale key when the pending release is cancelled by an unrelated event.
… own action definition The color-picker conversion covered 12 sites but left 8 standalone pickers on the generic toggle action, whose synthetic-event path cannot reach the CAPTURE-phase gesture: the shortcut flipped the GtkToggleButton state via gtk_button_clicked() while _color_picker_callback_button_press() never ran, leaving the button armed with no picker instance. The pickers now use dt_action_def_color_picker, routing shortcuts through the same entry point as a real click. Same element name and fallbacks, so existing bindings are unaffected.
btn_make_radio_callback() read gtk_get_current_event() (the key event during shortcut dispatch) instead of the gesture's synthetic state, so a declared ctrl variant was lost and creation_continuous was set from live modifier state. Now uses dt_gui_current_state(gesture) - the declared effect for shortcut presses, live state for real clicks - and drops the leaked event copy.
…gn comparison The direction-reversal dampener used file-scope statics shared by every module's presets button (a reversal on module A dropped a step on module B), applied to discrete wheel clicks too (a deliberate wheel reversal within 250 ms lost a step), and compared the full delta against a stored sign, discarding same-direction multi-step deltas the discrete scroll proxy legitimately emits. The state now lives per presets button, the hysteresis only applies to smooth (trackpad) scrolls, and the reversal test compares signs.
… gesture The collections treeview gesture ran in the default BUBBLE phase without claiming, so every press was also processed by the treeview's internal bubble-phase GtkGestureMultiPress: the callback's unconditional unselect_all+select_path ran first, then the native ctrl-toggle / shift handling mutated the selection again, breaking ctrl-click and shift-range selection in the day/time multiple-selection rules. Use the documented CAPTURE+claim pattern (as in gui/accelerators.c) to replicate the event consumption of the pre-migration button-press-event handler.
New fixesToggle Hold-key shortcuts leaving a stale key in the shortcut state. The hold-engage branch records the key in The eight standalone color pickers that were left out. Eight pickers (blend show-color/set-range, colorzones and rgbcurve show-color/create-curve, retouch fill-color, temperature area picker) were still on the generic toggle action: their shortcuts flipped the button via Liquify tool shortcuts. Preset-scroll dampener (both minor items). The dampener used process-global statics (a reversal on module A swallowed a step on module B), applied to discrete wheel clicks too, and compared the full delta against a stored sign, dropping same-direction |delta| ≥ 2. State is now per presets button, the hysteresis only applies to smooth (trackpad) scrolls, and the reversal test compares signs. Collections treeview — the contested one. Starvation is ruled out: On the contested severities: I'd agree with medium on the hold-key item (stock defaults reach it), and with the low reading on the collections item for this PR, since it's pre-existing. Both are moot now that they're fixed. TestsManual tests already conducted with these fixesAll of the following were manually tested on this branch and verified working (where a behavior was broken before, the old broken behavior is noted as "(before: ...)"). Darkroom — shortcut effects on gesture-backed toggles
Darkroom — hold-key shortcuts
Darkroom — standalone color pickers (the eight)
Darkroom — liquify tool shortcuts
Darkroom — preset-scroll dampener
Collections — day/time rules (the contested fix)
General sanity
|
|
Might be my impression and pure placebo, but running darktable with all these refactors and fixes feels cleaner |
Important (accepted)
Minor (accepted)
ContestedNone — every issue reached unanimity among all three seats. UnresolvedNone. Rejected (raised then dropped)
Merged
|
|
@kofa73 I appreciate you taking your time to do that. However, it feels like it's going to be a constant loop of going through it, me revisiting these things, the tool you're using to review it again, and back and forth. What if the same tooling you're using is also capable of doing those fixed and submitting it as commits, patches or something? My time is getting severely limited and although I'm progressing with this, these write ups are just too much to handle. To me it makes sense that given the time it took them to revisit the code, the source code of gtk to fact check, etc, a patch is like a small ask for them at the end |
|
The tool that runs the reviews is Claude Code, so sure, I could have it fix issues. However, I'm really uncomfortable with submitting code "fixing" issues in code I'm not familiar with, especially if I'm also unfamiliar with the technology (UI in general). I would only be able to "review" the fixes by running the agents again, and looping locally, and then still submitting code I personally understand very little about. So, please forgive me that I will not do that. I'm happy to run the reviews for you, if you wish, and sharing the results; you can also run the reviews yourself, the tool is at https://github.com/kofa73/panel-review - reusability was not really a concern, so you would probably have to fight a bit to get it running. Claude Code is a must, and at least one of Codex CLI and Antigravity CLI are required (+ subscriptions or API access, the latter of which would be prohibitively expensive for us mere mortals). Alternatively, I can send you a review skill for any coding agent that has been sharpened on darktable commits from people fixing bugs, you can run that with any coding agent and model of your choice. Mention me in a commit if you would like me to run a review, at any time. I'll try to do it as soon as I can. |
|
I've been experimenting with similar tools for the last year or so. They are getting better indeed (both tooling and the intelligence of the LLMs), but they still tend to hallucinate and confabulate stuff, no matter if they are discussing between them and so on. I'm doing this with a combination of my own workflow, skills and intuition I gained with using them, specifically Deepseek Flash v4 and Pi. Darktable's codebase, from what I've been experiencing, it's pretty fragile. We got modules that use different kinds of picker tools for instance, with different behaviors, and different bugs will trigger one and the other. The mask ellipse tools have some bugs different when applied from the mask manager module at the left vs inside a module drawn mask directly. The scrolling issues that sometimes are OS specific issues due to GTK3 incompleteness or deprecated system APIs. To just name a few. When you got a complex codebase like this one, and scenarios like these, the ideal move is just one LLM vs deterministic tests they can loop on. This is the idea (link to comment) I proposed at #20433. Well, this works wonders whether is a complex or simple codebase. But still, it is even more true in complex codebases when compared to just looping agents through the code. Not even a simple human would be able to do this in a reasonable amount of time (considering only spare time) IMO, while new features keep getting incorporated and more code dump into all this code debt. My idea, and please maybe I'm wrong, is the following:
|
|
I agree with you on tests. For processing modules (pure algorithms), it's much easier to do (that's why I'm in backend development ;-) ). For the UI, not so. |
|
Did some testing with a compiled AppImage from your branch with native wayland: AgX Pickers fixed (#21730), didn't find any new issues in the darkroom UI. Only one minor nitpick is the styles buttons "hover state" still being "sticky". Although they behave suboptimally in 5.6 (and probably before that), too. This PR: 5.6: |
- dt_shortcut_key_press() now flushes a still-armed delayed release before the hold takes over _sc: the pending pass requires c->key == s->key, so left armed it would resolve against the hold key and re-run the hold action, silently disengaging the hold while the key is still held. A same-key re-engage inside the window is that pending pass's own double-press check and is only cancelled. - dt_shortcut_key_release() cancels a still-armed delayed release before scheduling the hold cleanup, mirroring the ordinary key release, so g_timeout_add() cannot orphan a live source handle. - _delay_for_double_triple()'s stuck-key early return drops the _hold_release_pending flag and the stale hold key, or the next ordinary key release would silently skip its own dispatch. - _key_release_delayed() restores the identity of a still-engaged hold key when _pressed_keys is empty but _hold_keys is not, keeping move/scroll routing alive for the held key.
- the CAPTURE-phase gesture claims selectively (modifier presses, single-click mode, MONTH rule, folder/filmroll right-click) instead of unconditionally, so a plain click reaches the treeview's internal gesture again: single-click expander toggles, focus grab and cursor placement work as before the migration. - a plain primary double-click on the expander arrow claims only the second press and skips the module's own toggle: the internal gesture toggles on the first release, giving exactly one toggle with no expand/collapse flicker, while longer click bursts keep responding. - shift+ctrl+click on a folder/filmroll row activates again, making the folders<->filmrolls view switch reachable.
…close GTK3 synthesizes a fake enter crossing (GDK_CROSSING_GTK_UNGRAB) on every widget a grab was shadowing when the grab ends -- menus, popovers, modal dialogs -- so the button's internal enter handler marks the pointer as inside and the button keeps its hover state even after the pointer has left, only clearing on the next genuine crossing. Connect a grab-notify handler on dtgtk buttons that drops the stale hover/pressed flags when the grab ends and the pointer is not over the button. The lib module reset button additionally ran its action from a custom gesture's press, so the confirm dialog's grab split the button's press from its release (the press was processed after the dialog closed, the release was consumed by the dialog's grab), leaving the button stuck in its pressed state; wiring the action to the button's own clicked signal keeps the press/release cycle intact, and also avoids the cancel forwarding in dt_gui_connect_click() firing the reset twice.
|
@anoderay should be fixed with the last commit. The history reset button from the module in the left sidebar at darkroom was also getting stuck, and sometimes won't even show the confirmation dialog. This is sort of a hack to make it work in GTK3 properly. Maybe it even gets rid of some other bugs as you said that it was behaving sub-optimally. Check it if you can please! GTK4 is just much better at handling these things without surprises nor hacks. We have to live with this code (I commented around it that it needs to be removed) in the meantime while we still support gtk3. |
|
@TurboGit I believe this pr is ready to be merged, I exhausted my research for further fixes and improvements, at least for now, along with the suggestions from Kofa, and so far so good. At least the master branch will benefit of so many fixes going along with it at its current state. To me, revisiting the code over and over again, can't spot anything out of the ordinary. If you could review it, would be neat |
|
@Arecsu Sticky buttons are gone. But I did now notice one more thing: In 5.6 and before when selecting multiple image and hovering over one of them, then e.g. rejecting, deleting, moving etc. will act on the whole selection. This makes sense IMHO. Currently even when selecting multiple images only the hovered image is acted upon when rating/rejecting and moving (moving when triggered by shortcut); for color lables the behaviour is still as before. |
|
I think you may have also fixed #20018 . In 5.7+135 this happens very often (but inconsistently) for me, but I cannot provoke it yet in the current state. |
|
Ahhhh the issue with the hover selecting images that's on me, I was playing with that part of the code two days ago to make proper keyboard priority selection to work (#21385) and a whole can of worms spawned with it. Your videos and details are incredibly important in debugging that one, will check! And about the other issue fixed, glad you found out about it. Even more so, there might be other reports that might have been solved by now because I've been encountering stuff that suddenly started to work better for some reason along the way, although I don't recall exactly what. It's been such a monumental task that I'm forgetting plenty of things at this point |
The shortcut machinery grabs the pointer on every key press (gdk_seat_grab()); the synthetic crossings that the grab/ungrab dance generates reach the thumbtable and culling leave handlers, which cleared mouse_inside unconditionally -- the darktable-org#21734 hovered-image fix (2f24f86) only guarded the mouse_over_id clear. With mouse_inside FALSE, _cache_update_hover() skips the "hovered image is inside the selection" check (column 1) and acts on the hovered image only, so rating, rejecting, moving and color labels hit just the hovered thumbnail instead of the whole selection for every shortcut press until the pointer moves. Clear mouse_inside only on real leaves (same guard as mouse_over_id: not an inferior/grab crossing, and not while the pointer is grabbed), and restore it in the enter handler so the state self-heals.
|
@anoderay Yannic, you can test the lighttable and film strip at the bottom once again, it should be working as intended, be it with hover priority setting or without it! |
|
@Arecsu looks good now, thanks! |
4 participants
All the changes from #21733, #21741 and #21742 are included in this PR. Their commits are part of this branch. I closed those PRs because they shared most of their code paths with the review fixes below; a single PR is easier to review than three overlapping ones.
From the previous PRs, incorporated here:
Color picker shortcuts + double-press on hold keys (#21733). The event-controller conversion broke shortcuts on standalone picker buttons (e.g. AgX auto-tune): shortcuts synthesize fake button-press events that capture-phase gestures never receive. Pickers now use their own action definition and the shared callback takes explicit ctrl/right-click flags instead of fabricating events. Also, a key with a hold shortcut (default
a) could never trigger a double/triple-press shortcut on the same key; the hold press is now recorded and a fast second press is detected as a double press.Lighttable scrolling (#21741). Scrolling stopped as soon as the pointer was over an always-visible thumbnail overlay icon (e.g. the star ratings): the converted scroll handlers were target-only instead of bubbling. Bubble phase restored, and the raw smooth scroll deltas in thumbtable/culling were restored (touchpad scrolling had been chopped into fixed 50 px steps).
Gesture-driven toggle button shortcuts (#21742). Widgets whose action lives in a gesture (mask shape buttons, retouch/spots, filmicrgb, temperature, liquify, ashift fit/structure, the Masks module buttons, ...) now register the gesture (
DT_ACTION_GESTURE_KEY), and the shortcut layer fires the gesture's press signal — the same path a real click takes.New fixes
Help provided by the technical writeups of @kofa73 here and here.
GtkGestureMultiPresscountsn_pressnatively, and the bridge fired every double-click twice (double view switch in lighttable/culling, duplicate filmstrip select+ACTIVATE).GdkEventleaks fromgtk_get_current_event()in controller callbacks (culling, thumbtable, darkroom second window, colorlabels, shortcuts dialog, metadata, collection treeview), plus adt_gui_get_current_root_coords()helper.*_fallbackhelpers that double-accumulated after the DISCRETE proxy (bauhaus popup zoom-range reset, misleadingpreset '(last)'toast); preset scrolling now uses the proxy steps directly, with a reversal dampener at the preset list boundaries.dt_dev_add_history_item).ctrl+vshortcut ran the ctrl variant of a plain-activate binding, and the plain fit was unreachable from that shortcut).Linked issues
Fixes #21730
Fixes #21736
Fixes #21735
Fixes #21578
And, besides those previously quoted fixes, plenty of other things were tested working properly along the way.
Tests
Manual tests already conducted
All of the following were manually tested on this branch and verified working (where a behavior was broken before this PR, the old broken behavior is noted as "(before: ...)").
Darkroom — module header
Right-click on a module body (e.g. exposure, tone curve): the presets popup must open (this was dead before the button=0 fix upstream; verify it still works).
Module reset button: single click → module resets and the undo history gains exactly one entry (undo once, then redo — before, one click created two undo steps).
Presets button scroll: hover the module presets arrow and scroll the wheel → presets cycle (previous/next) with the toast; a smooth trackpad scroll must not show a bogus
preset 'x' (last)toast when nothing was applied.Ctrl+click the reset button → re-applies auto-presets (once).
Darkroom — curve editors (tone curve, RGB curves, base curve, color zones)
Right-click a curve endpoint (first or last node): the endpoint resets to 0/1 and no node is deleted (before this fix the node count silently shrank). Right-click a middle node → deletes the node as before. Check the persisted params: draw a curve, right-click an endpoint, then expand/collapse the module — node count must be unchanged. Right-click on empty curve area → presets menu still opens
Double-click the curve area → resets the curve (unchanged). Result: Works correctly.
Lighttable — thumbtable / culling
Double-click a thumbnail in filemanager/zoom mode → switches to darkroom once (before, the bridge+gesture double-fired; on slow machines you could see the modules rebuild twice / a flash).
Right-click a thumbnail and release → selection must not change. Middle-click release → selection must not change (before, any-button release selected the hovered image).
Culling mode: middle-click → zoom toggle works and does not flip the culling selection (act-on set). Right-click release → no selection change.
Memory: wiggle the mouse over the lighttable/culling grid and scroll a lot (e.g. 30 s), then check RSS with
top/Activity Monitor — should stay flat (before, culling/thumbtable motion leaked one GdkEvent per event).Bauhaus slider popup
Second darkroom window (preview2)
Shortcut-activated toggle buttons (PR #21742 fixes)
Retouch "editing" toggle via keyboard: open Retouch, bind a shortcut to Retouch → Editing (or use the existing default), press it → mask edit mode must toggle on/off (before: only the shape buttons cleared and edit mode never changed).
effect variants
ashift→ find "fit" → "vertical". Select it.v→ a new row appears in the right shortcuts list.v→ it must run the ctrl variant (fit rotation/lens-shift only, not the full vertical fit). Before this fix it ran the plain fit.ctrl+vwith the plain "activate" effect → pressingctrl+vmust do the plain fit (before: the live ctrl state hijacked it into the ctrl variant).Retouch shape buttons (path/circle/ellipse/brush) via shortcut: ctrl-effect shortcut → continuous (multi) shape creation starts.
Color labels: The "colorlabels" module is in the lighttable, bottom-left panel (the section with the color squares under the thumbnails area). Right-click one of the color squares → the description-edit popup should open at the pointer.
Collections / masks / metadata (leak + popup)
Masks tree (Blend → Drawn Mask → right-click row) — context menu opens, no leak.
Shortcuts dialog: type in the search box — memory stays flat.
Folders & filmrolls are single-select by design. Click a folder row (e.g.
photos/2024/) → only that row is selected and the collection filters to it. Shift-click or ctrl/cmd-click a second row must not extend the selection. Right-click a folder or filmroll row → the context menu opens at the pointer ("update path to files...", "remove...").Shift-range selection only exists for the time/exif views (day, month, time, aperture, focal length, ISO, exposure, aspect ratio, rating): click one row, shift-click another → the rows in between are selected and the rule becomes a range
[a;b](order follows the sort direction). Ctrl+click adds nothing here — it is single-select on those views too.Double-click toggles expand/collapse on tree parents; a single shift- or ctrl-click on a folder/filmroll row activates it directly (same as double-click); shift+ctrl+click on a folder row switches to the matching filmroll view and vice versa.
Related: #15920 #20433