Fix some cases of stuck and ignored shortcuts #19903
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Made some fixes to shortcut handling after a bit of a deep dive. There's always a chance that this "breaks stuff" so it would be good if people actually reported if anything stopped working for them. Maybe even before the end of the next release cycle, so that there's time to investigate, fix and test.
fixes #19888
The issue there is that some windows managers send hardware events, like "wake up" or "lid closed" as key presses to gtk and eventually dt. It may be that all of them do but maybe due to timing issues they don't make it all the way to the application. Maybe wayland buffers them until the application is "ready" to receive them. Whatever the case may be, these events only send key presses, not releases. And since dt keeps track of all the keys that it thinks are currently held down, these keys never clear. There's a break-lock "solution" that a short key press (doesn't have to be escape key) while other keys are "held" releases all of them, but having to use that every time you start dt gets annoying.
The simple solution is to just "ignore all the hardware event keys". But there's no clean way to recognise them. Gtk puts them all in the 0x1008ff00-0x1008ffff range, but also includes "media" (mute, pause) and "shortcut" (mail, calendar, search) there. Some people might want to use those in dt (I do; otherwise I'd lose the whole green top row on my bloomberg keyboard) but otherwise might want to ignore them and let them be handled by the operating system (which may require returning
FALSEin the event handler). The solution I've chosen is to check if the key is used as a shortcut (in any combination with mouse events) and if not to ignore it. Unless you're creating a new shortcut; so don't close your laptop lid while in mapping mode!It would be very good to hear back if this solves the issue people have and if the test fails for some situations that I haven't tested.
This also means that you're no longer receiving the "key not assigned" messages for keys that are not used yet. I haven't removed the code for that message yet, even though I believe it is now unreachable code, because having more messages for unexpected cases can be helpful. See the next section.
@ralfbrown you may find the following an interesting one as it took me a while to figure out.
Testcase: in darkroom, press&release F1-F5 all at the same time. This should toggle all 5 color labels. What instead happens is that sometimes one, sometimes a few, sometimes all get toggled.
The reason is that when processing the first received shortcut,
dt_colorlabels_toggle_label_on_listfirst callsdt_gui_cursor_set_busy. That sets the mouse cursor, processes outstanding gdk events to make sure the changed mouse is immediately visible and puts thegrabon the "progress" area, so that nothing else on the screen can be manipulated with the mouse. When done,dt_gui_cursor_clear_busyswitches the mouse back and processes events again. However, that also leads to further keyboard events being picked up. In this case, other shortcuts to toggle color labels. But before it gets that far, the shortcuts system checks if there's maybe a grab on progress, as an indication that a long running process is going on in the foreground and that all shortcuts should temporarily be ignored. Since the grab hasn't been released yet, it indeed concludes that to be the case and drops (some of) the remaining shortcuts.I solved the problem by moving the call to
dt_gui_process_eventsindt_gui_cursor_clear_busyto the end, aftergtk_grab_remove. But I'm thinking that it could be removed altogether, since after clear_busy the gui should be responsive again and the mouse cursor update would be processed in reasonable time as normal. And if it isn't that's probably because an immediately following event sets it to busy again, so why bother showing the normal mouse cursor in between.While digging into this, I also noticed an issue when you want to use multiple shortcut keys at once, for example to manipulate a section of a curve. Say you've got F1-F9 mapped to the nodes in the tone equaliser and you have fallbacks enabled. You can then hold F3+F4+F5 and use the scroll wheel to move all three of those nodes simultaneously. If you add F6, that will also start moving. If you now release F3, the others should continue responding to the mouse wheel, but they didn't. Because releasing F3 after a long while would set the "long press" flag and that would invalidate all subsequent shortcut activations. Now fixed.