Skip to content

gtk4-prep: fix shortcut activation for gesture-driven toggle buttons - #21742

Closed
Arecsu wants to merge 2 commits into
darktable-org:masterfrom
Arecsu:gtk4/iop-togglebutton-shortcuts
Closed

gtk4-prep: fix shortcut activation for gesture-driven toggle buttons#21742
Arecsu wants to merge 2 commits into
darktable-org:masterfrom
Arecsu:gtk4/iop-togglebutton-shortcuts

Conversation

@Arecsu

@Arecsu Arecsu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Since #21659 moved these buttons' actions onto gesture controllers, shortcuts stopped working for them: the icon would light up, but the tool never activated — e.g. a custom "add gradient to mask" binding highlighted the gradient button without actually entering gradient mode.

The reason: the shortcut layer activates widgets by synthesizing fake mouse-press events, and gesture controllers never receive synthetic events. So the shortcut only reached the button's visual toggle, never the code that does the work.

This makes widgets opt in: a widget whose action lives in a gesture registers that gesture (DT_ACTION_GESTURE_KEY), and the shortcut layer then fires the gesture's press signal — the exact same path a real click takes. Everything else is untouched.

One change fixes all of them: the mask shape buttons and "edit mask elements", filmicrgb, temperature, liquify, spots, retouch, the Masks module sidebar buttons, and the rotate-and-perspective fit/structure buttons.

Related: #15920 #20433

CC: @masterpiga

@kofa73

kofa73 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Important

  • src/gui/accelerators.c:222 — The gesture-activation branch in _action_process_toggle /
    _action_process_button never translates the action effect into the synthetic gesture, so ctrl-,
    right-button and long-press shortcut effect variants collapse into a plain unmodified primary click.
    — ⚠ detail contested: severity (raised as medium by two seats and low by one at the blind pass;
    never re-debated because the issue was accepted at birth)
    • Evidence:
      • src/gui/accelerators.c:222 — The gesture branch emits a bare
        g_signal_emit_by_name(gesture, "pressed", 1, 0.0, 0.0) and does not use effect; only the
        retained else branch encodes DT_ACTION_EFFECT_TOGGLE_CTRL/ON_CTRL into event->button.state
        and TOGGLE_RIGHT/ON_RIGHT into event->button.button.
        When: target widget carries DT_ACTION_GESTURE_KEY (every dt_iop_togglebutton_new widget,
        the six masks-lib shape buttons, the six ashift buttons).
        Impact: the four modifier effects offered by dt_action_effect_toggle
        (accelerators.c:151-159) become indistinguishable from plain "toggle"/"on".
      • src/gui/accelerators.c:276 — Same omission in _action_process_button:
        DT_ACTION_EFFECT_ACTIVATE_CTRL/ACTIVATE_RIGHT (dt_action_effect_activate,
        accelerators.c:168-172) are only honoured in the else branch, so ashift's
        fit_v/fit_h/fit_both (registered with dt_action_def_button at ashift.c:6134-6139) can no
        longer be shortcut-activated with a ctrl or right variant.
        Impact: right-click and long-press fallback shortcuts invoke registered gestures as primary
        clicks.
      • src/gui/accelerators.c:389 — The button action fallback explicitly maps right-button and
        long-press shortcuts to DT_ACTION_EFFECT_ACTIVATE_RIGHT, so the effect value that the gesture
        branch discards is genuinely produced upstream.
      • src/gui/gtk.h:653dt_gui_current_button maps a gesture whose current button is zero to
        GDK_BUTTON_PRIMARY, irreversibly converting synthetic secondary clicks into primary clicks.
      • src/iop/ashift.c:6120 — The fit_v control stores its dt_gui_connect_click gesture under
        DT_ACTION_GESTURE_KEY, so it takes the new gesture branch.
      • src/iop/ashift.c:6135 — The fit_v control is registered with the button action definition
        that has the right-click fallback.
      • src/iop/ashift.c:5329 — The fit callback enters its fit-and-history path when
        dt_gui_current_button returns GDK_BUTTON_PRIMARY, so an ACTIVATE_RIGHT shortcut performs
        the fit instead of following the secondary-click path.
      • src/iop/ashift.c:5334_event_fit_v_button_clicked derives its ctrl/shift branches from
        dt_key_modifier_state(), which polls the live pointer/keyboard state (src/gui/gtk.c:3896-3903),
        not from the action effect.
        When: user binds "fit vertical" to a shortcut containing ctrl or shift, e.g. ctrl+v.
        Impact: pressing that shortcut runs the ctrl variant (ASHIFT_FIT_ROTATION_VERTICAL_LINES
        only) and the resulting params are committed with dt_dev_add_history_item, while the plain fit
        becomes unreachable from that shortcut; conversely the "ctrl-activate" effect bound to an
        unmodified key performs the unmodified fit.
      • src/develop/blend_gui.c:1641_blendop_masks_add_shape sets continuous from
        gtk_get_current_event_state(), so the "add multiple <shape>" behaviour advertised in the
        tooltip built by dt_iop_togglebutton_new (imageop_gui.c:282) can only be requested by a
        modifier in the current event, which the gesture branch never supplies.
        Impact: the ctrl-toggle/ctrl-on effects on the six blend shape buttons behave exactly like
        plain toggle; the same pattern applies to _blendop_blendif_showmask_clicked
        (blend_gui.c:1376) and _bt_add_shape_cb (libs/masks.c:682).

Minor

  • src/gui/accelerators.c:229 — The stored-gesture shortcut path supplies no button-bearing
    GdkEvent, so rt_edit_masks_callback derives button 0 from gtk_get_current_event() and skips
    the edit-mode transition: a keyboard shortcut on retouch's "editing" toggle only clears the shape
    toggles and any in-progress shape creation instead of entering or leaving mask-edit mode.
    — ⚠ detail contested: location (one seat proposed re-anchoring the issue to the retouch call site
    src/iop/retouch.c:1821 rather than the dispatcher; the other two left the dispatcher anchor, so
    the detail never converged before the per-issue round limit)
    • Evidence:
      • src/gui/accelerators.c:229 — The stored-gesture toggle branch manually emits pressed
        instead of supplying the callback with a GdkEvent.
      • src/gui/accelerators.c:4802 — The shortcut dispatcher starts release processing from a
        GDK_KEY_RELEASE event rather than a GDK_BUTTON_PRESS event.
      • src/common/gdk_event_utils.h:48 — The button accessor initializes its result to zero and only
        changes it when the supplied event has a button value.
      • src/iop/retouch.c:1843 — The retouch callback performs the edit-mode transition only when the
        derived button is GDK_BUTTON_PRIMARY.
      • src/iop/retouch.c:2449 — The retouch editing control is created with
        dt_iop_togglebutton_new.
      • src/develop/imageop_gui.c:275dt_iop_togglebutton_new stores its gesture under
        DT_ACTION_GESTURE_KEY for direct shortcut activation.
        Impact: a keyboard shortcut assigned to retouch editing clears the auxiliary toggle state but
        does not enter or leave edit mode.
      • src/iop/retouch.c:1821 — retouch's callback still reads the button via
        gtk_get_current_event()/dt_gdk_event_get_button(), while the same change introduced
        dt_gui_current_button() (src/gui/gtk.h:650, maps gesture button 0 to GDK_BUTTON_PRIMARY) and
        applied it to the ashift, blend_gui and masks-lib gesture callbacks; spots.c:361 _edit_masks
        has no button gate at all, so bt_edit_masks in retouch is the only gesture-activated masks
        control that fails.
        Impact: retouch.c:1828-1841 still cancel shape creation and clear
        bt_path/bt_circle/bt_ellipse/bt_brush, so the shortcut performs a partial action while
        bd->masks_shown and bt_edit_masks stay unchanged.
      • src/gui/gtk.h:646dt_gui_current_button() maps the stored shortcut gesture's button 0 to
        GDK_BUTTON_PRIMARY.
      • src/iop/retouch.c:1820rt_edit_masks_callback fetches the current event using
        gtk_get_current_event(), which returns the keyboard event when triggered by a shortcut,
        leading dt_gdk_event_get_button to return 0.
      • src/iop/retouch.c:1816 — On master rt_edit_masks_callback is a button-press-event handler
        taking GdkEventButton*, so the pre-gesture shortcut path (synthetic GDK_BUTTON_PRESS with
        button=GDK_BUTTON_PRIMARY, accelerators.c:233-249) delivered a primary button and the
        edit-mode transition ran.
        When: comparing branch HEAD against master for the same shortcut.
        Impact: the retouch editing shortcut worked before the gesture migration, so the missing
        dt_gui_current_button conversion in retouch is a functional regression, not a pre-existing gap.
      • src/develop/imageop_gui.c:290 — The togglebutton helper registers the widget with the toggle
        action system.
        When: a shortcut is assigned to the retouch editing action.
    • Counter-analysis raised during debate (all of it ultimately supporting the issue, arguing only
      for the lower severity):
      • Verified: accelerators.c:229 emits the gesture "pressed" signal directly, so no GdkEvent is
        pushed; during shortcut dispatch gtk_get_current_event() yields the key event
        (accelerators.c:4788/4802) or NULL, and gdk_event_get_button() leaves 0 for both, so
        retouch.c:1821 gives button 0 and the primary-gated block at retouch.c:1843 is skipped while
        retouch.c:1828-1841 still runs. Impact is UI-only (mask overlay edit mode and toggle visuals);
        no wrong pixels, history, or persisted data, so severity fits the low band.
      • Chain verified: retouch.c:2449 builds bt_edit_masks via dt_iop_togglebutton_new, which now
        stores the GtkGestureMultiPress under DT_ACTION_GESTURE_KEY (imageop_gui.c:275);
        accelerators.c:229 emits "pressed" with no GdkEvent; the dispatcher runs from the main window
        "event" signal (darktable.c:2046), so gtk_get_current_event() at retouch.c:1820 yields the key
        event (or NULL for timer-driven activation) and dt_gdk_event_get_button leaves 0
        (gdk_event_utils.h:47-49); retouch.c:1843 therefore skips the whole
        DT_ENTER_GUI_UPDATE/masks_shown/dt_masks_set_edit_mode block while 1828-1841 already
        cancelled shape creation and cleared the shape toggles.

Merged

No separate issue was folded into another. Three independently raised Round-0 findings
(one per seat) described the same mechanism at the same code path and were clustered into i1
before debate; their two distinct call sites (_action_process_toggle at accelerators.c:222 and
_action_process_button at accelerators.c:276) are preserved as separate evidence points inside
that issue because the branch and the fix are identical.

i2 was deliberately not merged into i1 despite sharing the same source branch. i1 is the
loss of the ctrl/right effect variants — plain activation still works there, because
dt_gui_current_button maps gesture button 0 to GDK_BUTTON_PRIMARY. i2 is the absence of any
GdkEvent, which makes the gdk_event_utils button accessor return 0 (not PRIMARY), so even an
unmodified shortcut is a no-op for callbacks that read the event. Different mechanism, different
fix.

@Arecsu

Arecsu commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #21745

@Arecsu Arecsu closed this Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants