search

Smithay / smithay

A smithy for rusty wayland compositors
MIT License
1.9k stars 168 forks source link

keyboard ModifiersState reset for x11 backend upon window cursor enter #646

Open h1771t opened 2 years ago

h1771t commented 2 years ago

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.

i509VCB commented 2 years ago

What does the ICCCM say about this?

h1771t commented 2 years ago

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 commented 2 years ago

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.

Drakulix commented 2 years ago

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?

h1771t commented 2 years ago

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:

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.

Drakulix commented 2 years ago

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

h1771t commented 2 years ago

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

ids1024 commented 2 years ago

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.