smithay icon indicating copy to clipboard operation
smithay copied to clipboard

keyboard ModifiersState reset for x11 backend upon window cursor enter

Open h1771t opened this issue 3 years ago • 8 comments

In experimenting with making smithay x11 backend grab keyboard to allow super key shortcuts to work (when the smithay x11 backend window is not the main x11 shell), I have these edits in src/backend/x11/window_inner.rs

impl WindowInner {
    pub fn cursor_enter(&self) {
                ...
                connection.grab_keyboard(false, self.id, x11rb::CURRENT_TIME, x11::GrabMode::ASYNC, x11::GrabMode::ASYNC).unwrap();
..
    pub fn cursor_leave(&self) {
                ...
                connection.ungrab_keyboard(x11rb::CURRENT_TIME).unwrap();

This does work, with one major issue. When the x11 backend is not the main x11 shell but rather just another x11 window being run, and the user moves the mouse off the window and presses ALT+TAB, the window/seat sees the alt:true keyboard ModifiersState up until the TAB key is hit. At that point the x11 main shell switches app to another window. The window/seat does not end up getting the alt:false. If the user then hits ALT+TAB again to move back to the smithay x11 window, the ModifiersState stays stuck at alt:true even though the ALT key is not pressed. At this point pressing ALT and releasing makes no difference, the ModifiersState seems permanently stuck at alt:true

Issue also described at https://github.com/pop-os/cosmic-comp/issues/9#issuecomment-1163218515

As per this suggestion https://github.com/pop-os/cosmic-comp/issues/9#issuecomment-1163258734 I have opened a ticket here to get feedback. The suggestion made was to possibly have the ModifiersState reset when the x11 window comes back into focus.

h1771t avatar Jun 23 '22 12:06 h1771t

What does the ICCCM say about this?

i509VCB avatar Jun 23 '22 19:06 i509VCB

I cannot see anything specific. I looked in https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html#the_keyboard_mapping

However smithay uses https://docs.rs/xkbcommon/0.4.0/xkbcommon/xkb/struct.State.html#method.update_key which has this description which looks related:

A series of calls to this function should be consistent; that is, a call with xkb::KEY_DOWN for a key should be matched by an xkb::KEY_UP; if a key is pressed twice, it should be released twice; etc. Otherwise (e.g. due to missed input events), situations like "stuck modifiers" may occur

I suspect smithay x11 backend gets alt-pressed event upon alt+tab to move away. It does not get the alt-depressed due to main shell moving to another app. Then upon another alt+tab back to smithay x11, the main shell doesn't send a alt-depressed perhaps? So from then on, xkb State has alt pressed, any further pressing only increments to 2 and depressing decrements back to 1, so it never sees 0 again.

I am going to attempt to using https://docs.rs/xkbcommon/0.4.0/xkbcommon/xkb/struct.State.html#method.update_mask to reset the mask upon window leave event, and see if it fixes the issue for me. I will post the results I find

h1771t avatar Jun 27 '22 11:06 h1771t

Indeed if I add a function in src/wayland/seat/keyboard.rs

+    fn reset_modifiers(&mut self) {
+        self.state.update_mask(0, 0, 0, 0, 0, 0);
+    }

And have it called from compositor when a window enter-event occurs, then the issues seems to resolve for me. Super key shortcuts work for me in x11 backend window when it isn't main shell, even after alt+tab switching to another app and then back to x11 backend window again.

h1771t avatar Jun 27 '22 13:06 h1771t

This does work, with one major issue. When the x11 backend is not the main x11 shell but rather just another x11 window being run, and the user moves the mouse off the window and presses ALT+TAB, the window/seat sees the alt:true keyboard ModifiersState up until the TAB key is hit. At that point the x11 main shell switches app to another window. The window/seat does not end up getting the alt:false. If the user then hits ALT+TAB again to move back to the smithay x11 window, the ModifiersState stays stuck at alt:true even though the ALT key is not pressed. At this point pressing ALT and releasing makes no difference, the ModifiersState seems permanently stuck at alt:true

In which desktop environment does this happen? On Wayland (with XWayland) or inside an X11 session? Also does the connection.grab_keyboard request return a XCB_GRAB_STATUS_SUCCESS (or whatever the equivalent is in x11rb), could you log that?

Because this seems like a bug in the DEs application switcher, normally if the window has successfully obtained a grab, it should receive all keyboard events (including the alt+tab release event).

We need to figure out, who is not behaving correctly here.

@i509VCB The ICCCM indeed has no conventions for this edge case, that would help us here. At least I was not able to find some. What is your opinion on this? Resetting the modifiers map seems reasonable to me, but I am not sure, if we mess up the state, if a modifier is already pressen when re-entering the window. Does Xorg send the modifiers again on focus?

Drakulix avatar Jun 28 '22 10:06 Drakulix

In which desktop environment does this happen? On Wayland (with XWayland) or inside an X11 session?

Inside X11. Here are the steps I'm using to reproduce:

  • Install Pop_OS! 22.04 in qemu vm, login on default gui session as normal user
  • clone cosmic-comp git repo
  • build and then run Cosmic comp then opens in a x11 window (not main shell) within the default Pop_OS! ui session which is xorg for me inside qemu. Looking at logs from cosmic-comp, I can see INFO Connecting to the X server, smithay_module: backend_x11 so I know it is using smithay x11 backend
  • If you move the mouse into cosmic-comp window and click to give it focus, you can notice the windows super key shortcuts for cosmic-comp won't work (instead it is invoking Pop!_OS main shell super actions). The initial keyboard grab PR was to get the super keys working.
  • Apply patches to get keyboard grab/ungrab working (these are the grab_keyboard and ungrab_keyboard bits from https://github.com/pop-os/smithay/compare/main...h1771t:smithay-1:main#) in order to produce issue 2 which this github issue was opened for.
  • After grab/ungrab patches, build and run cosmic-comp, the super key actions in cosmic-comp now works
  • Now to reproduce issue 2, launch cosmic-comp and see that super key actions are working
  • Move the mouse outside cosmic-comp window (causes a keyboard ungrab)
  • press ALT+TAB causing Pop_OS! default shell to move to another app, eg a running terminal
  • press ALT+TAB to move back to cosmic-comp window, move mouse inside cosmic-comp window (causes a keyboard grab)
  • Try do super key actions again, now it does not work anymore due to ModifiersState thinking ALT is still pressed

If I apply the full patch from https://github.com/pop-os/smithay/compare/main...h1771t:smithay-1:main# for smithay and also this one for cosmic-comp https://github.com/pop-os/cosmic-comp/pull/20/files which is resetting the keyboard modifiers upon mouse-enter, then issue 2 is resolved for me, ie. super key actions of cosmic-comp work for me even if I have ALT+TAB away from cosmic-comp window and then back again

I am not sure thought that resetting modifiers on mouse-enter is the right way to fix this.

Also does the connection.grab_keyboard request return a XCB_GRAB_STATUS_SUCCESS (or whatever the equivalent is in x11rb), could you log that?

This is the exact keyboard grab function being used, but it is unrelated to issue 2 because the grab and ungrab does work fine: https://docs.rs/x11rb/0.9.0/x11rb/protocol/xproto/trait.ConnectionExt.html#method.grab_keyboard

Because this seems like a bug in the DEs application switcher, normally if the window has successfully obtained a grab, it should receive all keyboard events (including the alt+tab release event).

Sorry I was not being clear around this. With the mouse inside cosmic-comp window (grabbed), all keyboard is indeed handled by cosmic-comp, including ALT+TAB This behaves correctly. Only when I move the mouse out of cosmic-comp window (ungrabbed) and ALT+TAB, do I cause issue 2.

Seems even with the mouse outside cosmic-comp window (ungrabbed), cosmic-comp still has focus so when doing ALT+TAB, cosmic-comp/smithay detects a ALT-pressed, but as soon as TAB is hit, the main Pop_OS! shell switches apps and ALT-pressed stays stuck in cosmic-comp/smithay

We need to figure out, who is not behaving correctly here.

I suspect the main shell/DE app switcher in Pop_OS! is behaving fine, and most x11 apps won't track modifierState the way cosmic-comp/smithay needs to, so they won't experience the issue? I'm not totally sure.

h1771t avatar Jun 28 '22 11:06 h1771t

Also does the connection.grab_keyboard request return a XCB_GRAB_STATUS_SUCCESS (or whatever the equivalent is in x11rb), could you log that?

This is the exact keyboard grab function being used, but it is unrelated to issue 2 because the grab and ungrab does work fine: https://docs.rs/x11rb/0.9.0/x11rb/protocol/xproto/trait.ConnectionExt.html#method.grab_keyboard

Yes, and that function returns a Cookie with a GrabKeyboardReply, it would be interesting to log, if it succeeds, although this is not the issue at hand, because...

Because this seems like a bug in the DEs application switcher, normally if the window has successfully obtained a grab, it should receive all keyboard events (including the alt+tab release event).

Sorry I was not being clear around this. With the mouse inside cosmic-comp window (grabbed), all keyboard is indeed handled by cosmic-comp, including ALT+TAB This behaves correctly. Only when I move the mouse out of cosmic-comp window (ungrabbed) and ALT+TAB, do I cause issue 2.

Seems even with the mouse outside cosmic-comp window (ungrabbed), cosmic-comp still has focus so when doing ALT+TAB, cosmic-comp/smithay detects a ALT-pressed, but as soon as TAB is hit, the main Pop_OS! shell switches apps and ALT-pressed stays stuck in cosmic-comp/smithay

This indicates, that grabbing is not related to this issue at all. Just moving the mouse outside the window and loosing focus while a modifier is pressed, would cause said modifier to get stuck.

It seems winit has applied a similar fix to a very similar problem: https://github.com/rust-windowing/winit/pull/1279

The correct solution here is not resetting the modifiers (which might report modifiers as unset, when they are not), but to query the modifiers from X11 after gaining focus again.

The functions they use to get that state seems little annoying, but should map to similar calls from x11rb: https://github.com/murarth/winit/blob/master/src/platform_impl/linux/x11/util/modifiers.rs#L50-L95

Drakulix avatar Jun 28 '22 12:06 Drakulix

Great thanks, makes sense. I will attempt to make a PR for the correct fix then.

h1771t avatar Jun 28 '22 14:06 h1771t

X has a XKeymapEvent that's produced when the window gains keyboard focus, which it should be possible to use to determine what keys (including modifiers) are held when the window is focused. But interestingly GTK doesn't seem to use and, and I guess the winit code linked above doesn't either.

ids1024 avatar Sep 15 '22 20:09 ids1024