Skip to content

gtk4-prep: fix color picker shortcuts and double-press on hold keys - #21733

Closed
Arecsu wants to merge 3 commits into
darktable-org:masterfrom
Arecsu:fix/picker-shortcuts
Closed

gtk4-prep: fix color picker shortcuts and double-press on hold keys#21733
Arecsu wants to merge 3 commits into
darktable-org:masterfrom
Arecsu:fix/picker-shortcuts

Conversation

@Arecsu

@Arecsu Arecsu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

1. Picker shortcuts

The gtk4-prep migration broke shortcuts for standalone picker buttons (like AgX's auto tune levels): click handling moved to a capture-phase gesture, and shortcuts (which synthesized fake button-press-event signals) can't reach a capture-phase gesture. So shortcuts silently stopped triggering pickers while clicks kept working.

Fix: standalone pickers now use their own action definition, like bauhaus widgets already did, calling the same code real clicks use. All twelve pickers share it, and the shared callback takes explicit ctrl/right-click flags instead of fabricating GdkEvents.

2. Double-press vs. hold on the same key

A key with a hold shortcut (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, so the double was never detected.

Fix: the hold press is now recorded, and a fast consecutive press of the same key (when a double/triple shortcut exists for it) is detected as a double/triple press instead of re-engaging the hold. Both work now: hold a to pan/zoom/rotate, tap a twice for a;double.

Testing

  • Shortcuts, clicks and right-clicks across all twelve standalone pickers
  • Double-press shortcuts, incl. b;double (AgX auto tune) and a custom a;double
  • Hold a (pan/zoom/rotate) and w (full preview) still behave normally
  • Masks, curve editing and snapshots unaffected

Related: #15920 #20433
Fixes #21730
Fixes #21736

@Arecsu Arecsu changed the title fix color picker shortcuts and remove the dead "force pan/zoom/rotate with mouse" hold action gtk4-prep: fix color picker shortcuts, remove the dead "force pan/zoom/rotate with mouse" hold action Aug 6, 2026
… 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.
@Arecsu
Arecsu force-pushed the fix/picker-shortcuts branch from cd8826b to 0618c15 Compare August 6, 2026 02:24
@TurboGit TurboGit added this to the 5.8 milestone Aug 6, 2026
@TurboGit TurboGit added bugfix pull request fixing a bug gtk4 labels Aug 6, 2026
@TurboGit

TurboGit commented Aug 6, 2026

Copy link
Copy Markdown
Member

The default a hold shortcut sets a darkroom_skip_mouse_events flag so the mouse pans/zooms/rotates the image instead of interacting with module overlays. Except it doesn't actually work: the flag gets set while a is held, but button presses never reach the center view's handlers, so panning never engages. I confirmed this on a pre-gtk4-prep checkout — it's been broken for a while. At least, I couldn't find a way to trigger this myself.

This is working on my side and it is broken with this PR. Or maybe we have different way to test this?

@Arecsu

Arecsu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

The default a hold shortcut sets a darkroom_skip_mouse_events flag so the mouse pans/zooms/rotates the image instead of interacting with module overlays. Except it doesn't actually work: the flag gets set while a is held, but button presses never reach the center view's handlers, so panning never engages. I confirmed this on a pre-gtk4-prep checkout — it's been broken for a while. At least, I couldn't find a way to trigger this myself.

This is working on my side and it is broken with this PR. Or maybe we have different way to test this?

This is interesting. I'm unable to remember if I tried this on macOS or Linux. Where did you test this? There was no way for me to get it working at all.

Still, the "double A" key press would be completely eaten by it, as stated by one of the issue reporters. I tried my best to find a code that would make both scenarios to work but it looked really, really really bad honestly. Maybe it's incompetence on my side

@TurboGit

TurboGit commented Aug 6, 2026

Copy link
Copy Markdown
Member

To test on my side without this PR:

  • Activate Agx
  • Click on auto-tune picker
  • Zoom on image
  • Keep pressed a and pan the image around

This last step works on current master and fails with this PR.

@TurboGit

TurboGit commented Aug 6, 2026

Copy link
Copy Markdown
Member

This is interesting. I'm unable to remember if I tried this on macOS or Linux. Where did you test this? There was no way for me to get it working at all.

Testing on GNU/Linux. Maybe your a shortcut have been removed?

@TurboGit

TurboGit commented Aug 6, 2026

Copy link
Copy Markdown
Member

Still, the "double A" key press would be completely eaten by it

Just to clarify, you need to press a and keep it pressed while panning/zooming...

@Arecsu

Arecsu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Ahhhhh I didn't know it worked that way, I was trying holding A in a 100% zoomed image and thinking it will move when my cursor was over the sidebar or something. I couldn't understand the purpose of it hahahaha I feel ashamed. Well that's really useful I would say. So, press A while using a color-picker tool for instance, and just moving the cursor or scroll wheel will do the trick right?

@TurboGit

TurboGit commented Aug 6, 2026

Copy link
Copy Markdown
Member

Well that's really useful I would say.

Sure, it is also a needed feature when using the calibrating feature of "color calibration" module. In fact all modules displaying an overlay need this to be able to pan/zoom instead of interacting with the overlay.

So, press A while using a color-picker tool for instance, and just moving the cursor or scroll wheel will do the trick right?

Yes.

@Arecsu

Arecsu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Sorry about misunderstanding this one. Will check it tomorrow if possible, It's pretty late here. I will revert that commit but the mechanism to make "double A" shortcut to work will be sort of a mystery. If you have any clues or directions, it would be super useful!

@TurboGit

TurboGit commented Aug 6, 2026

Copy link
Copy Markdown
Member

@Arecsu : Nothing urgent, and thanks for being so responsive.

Arecsu added 2 commits August 6, 2026 12:58
…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.
@Arecsu
Arecsu force-pushed the fix/picker-shortcuts branch from 0618c15 to a426666 Compare August 6, 2026 16:27
@Arecsu Arecsu changed the title gtk4-prep: fix color picker shortcuts, remove the dead "force pan/zoom/rotate with mouse" hold action gtk4-prep: fix color picker shortcuts and double-press on hold keys Aug 6, 2026
@Arecsu

Arecsu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@TurboGit should be done by now, please test if possible with double a and triple a custom shortcuts, hold X and double X onto something, color pickers should not crash and should activate, be it with standard clicks or shortcuts, and hold key shortcuts should still let other normal, double or triple presses to be triggered

@TurboGit

TurboGit commented Aug 6, 2026

Copy link
Copy Markdown
Member

See https://www.githubstatus.com/ there is issues with GitHub runner at the moment.

@Arecsu

Arecsu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

See https://www.githubstatus.com/ there is issues with GitHub runner at the moment.

Yep, maybe they are migrating to gtk4 as well 🙂‍↕️

@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

bugfix pull request fixing a bug gtk4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash when trying to set white balance from selected area Calling AgX auto-tune-levels via shortcut broken

2 participants