Skip to content

Conversation

@rti
Copy link

@rti rti commented Sep 9, 2021

This patch addresses an issue in the modifier key handling appearing on
gnome/x11 while focussing a winit window using a meta/super/logo key
shortcut.

As soon as the window gained focus via a shortcut using a meta key,
gnome/x11 reports a meta modifier as pressed in the focus event, even
though it was just released. No key release events were sent afterwards,
so the meta modifier remained "stuck" in the winit state.

This patch refactors the modifier key handling following this goals:

  • Only rely on winit's own modifier state tracking for key and focus
    events, do not mix it with x11's modifier reporting
  • Track modifiers only via non-raw key events, so winit does not track
    modifier changes when not focussed
  • Integrate modifier handling with the synthetic key events mechanism
    in order to report release of all currently held modifiers when
    unfocussed and report set for all currently held modifiers when
    focussed.

If you wonder why somebody is creating a pull request for a branch currently being in a pull request for the upstream repo... For https://github.com/neovide/neovide @Kethku created a winit branch https://github.com/kethku/winit/tree/new-keyboard-all pulling in this very WIP branch already for the current neovide main branch. I am running neovide main and experienced the before mentioned problem on my platform. So I tried to fix it.

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

This patch addresses an issue in the modifier key handling appearing on
gnome/x11 while focussing a winit window using a meta/super/logo key
shortcut.

As soon as the window gained focus via a shortcut using a meta key,
gnome/x11 reports a meta modifier as pressed in the focus event, even
though it was just released. No key release events were sent afterwards,
so the meta modifier remained "stuck" in the winit state.

This patch refactors the modifier key handling following this goals:
 - Only rely on winit's own modifier state tracking for key and focus
   events, do not mix it with x11's modifier reporting
 - Track modifiers only via non-raw key events, so winit does not track
   modifier changes when not focussed
 - Integrate modifier handling with the synthetic key events mechanism
   in order to report release of all currently held modifiers when
   unfocussed and report set for all currently held modifiers when
   focussed.
@rti rti force-pushed the fix-gnome-x11-meta-mod-focus-maroider branch from 536f5b5 to 9fea819 Compare September 9, 2021 19:36
@maroider
Copy link
Owner

maroider commented Sep 10, 2021

I know I said I'd look at this today, and I didn't completely forget, but I ended up just getting distracted by other things.
I'll get around to this tomorrow (I hope).

@maroider
Copy link
Owner

maroider commented Sep 12, 2021

I've encountered what looks like a bug. I use Sway (and I'm running the test program under XWayland), so I am able to change the focused simply by moving the mouse. So I tried holding down the Shift key and flicking the mouse in and out of my test program's window. The problem is that I sporadically get ModifiersChanged(ModifiersState::SHIFT) and ModifiersChanged(0), even though I never let go of the Shift key. In contrast to this behaviour, the current tip of the new-keyboard-linux branch seems to reliably emit ModifiersChanged(ModifiersState::SHIFT) and ModifiersChanged(0) when focus is gained and lost, respectively.

@rti
Copy link
Author

rti commented Sep 12, 2021

Thanks a lot @maroider for looking into this.

I tried to reproduce the issue you described and was not able to encounter it on my machine. Also, I think the issue that motivated me to submit this pull request is slightly different from the one you are describing.

Let me summarize what I did and see on my platform (Gnome 40.4.0, X11, Arch Linux).

Test program

I am using the following test program (slightly modified version of winit/examples/window.rs:

use simple_logger::SimpleLogger;
use winit::{
    event::{Event, WindowEvent},
    event_loop::{ControlFlow, EventLoop},
    window::WindowBuilder,
};

fn main() {
    SimpleLogger::new().init().unwrap();
    let event_loop = EventLoop::new();

    let window = WindowBuilder::new()
        .with_title("A fantastic window!")
        .with_inner_size(winit::dpi::LogicalSize::new(512.0, 512.0))
        .build(&event_loop)
        .unwrap();

    event_loop.run(move |event, _, control_flow| {
        *control_flow = ControlFlow::Wait;

        match event {
            Event::WindowEvent {
                event: WindowEvent::CloseRequested,
                window_id,
            } if window_id == window.id() => *control_flow = ControlFlow::Exit,
            Event::MainEventsCleared => {
                window.request_redraw();
            }
            Event::WindowEvent {
                event: WindowEvent::KeyboardInput { device_id, input, is_synthetic },
                // or: event: WindowEvent::KeyboardInput { device_id, event, is_synthetic },
                ..
            } => {
                if is_synthetic {
                    println!("APP: Key {:?} {:?} syn", input.virtual_keycode, input.state);
                    // or: println!("APP: Key {:?} {:?} syn", event.logical_key, event.state);
                }
                else {
                    println!("APP: Key {:?} {:?}", input.virtual_keycode, input.state);
                    // or: println!("APP: Key {:?} {:?}", event.logical_key, event.state);
                }
            }
            Event::WindowEvent {
                event: WindowEvent::ModifiersChanged(modifiers),
                ..
            } => {
                println!("APP: Mod {:?}", modifiers);
            }
            _ => (),
        }
    });
}

Issue: missed modifiers using "mouse focus"

This tests for the issue you described - as I understand it.

  • Set Gnome / Mutter to "Focus on hover" via Gnome Tweaks
  • Start the test program
  • Focus the window with focus mouse follow
  • Press Shift
  • Un-focus the window with focus mouse follow
  • Re-focus the windows with focus mouse follow
  • Release Shift

rust-windowing/winit master

rust-windowing@1b3b82a

Output (lines starting with // are comments added by me afterwards)

2021-09-12 21:34:27,721 TRACE [mio::poll] registering event source with poller: token=Token(0), interests=READABLE
2021-09-12 21:34:27,727 INFO [winit::platform_impl::platform::x11::window] Guessed window scale factor: 1
2021-09-12 21:34:27,727 DEBUG [winit::platform_impl::platform::x11::window] Calculated physical dimensions: 512x512
// now I press and keep holding the shift key
APP: Mod SHIFT
APP: Key Some(LShift) Pressed
// now I leave the window via a mouse move
APP: Key Some(LShift) Released syn
APP: Mod (empty)
// now I enter the window via a mouse move
APP: Mod SHIFT
APP: Key Some(LShift) Pressed syn
// now I release the shift key
APP: Mod (empty)
APP: Key Some(LShift) Released

This does not change if I repeatedly leave and enter the window with the mouse.
This is the behavior I expect.

maroider/winit new-keyboard-linux

3ae1593

Output (lines starting with // are comments added by me afterwards)

2021-09-12 21:46:41,483 TRACE [mio::poll] registering event source with poller: token=Token(0), interests=READABLE
2021-09-12 21:46:41,495 INFO [winit::platform_impl::platform::x11::window] Guessed window scale factor: 1
2021-09-12 21:46:41,495 DEBUG [winit::platform_impl::platform::x11::window] Calculated physical dimensions: 512x512
// now I press and keep holding the shift key
APP: Mod SHIFT
APP: Key Shift Pressed
// now I leave the window via a mouse move
APP: Key Shift Released syn
APP: Mod (empty)
// now I enter the window via a mouse move
APP: Mod SHIFT
APP: Key Shift Pressed syn
// now I release the shift key
APP: Mod (empty)
APP: Key Shift Released

This does not change if I repeatedly leave and enter the window with the mouse.
This is also the behavior I expect.

Conclusion

Unfortunately I cannot reproduce the issue you describe, neither on the current master nor on the new-keyboard-linux branch.

Issue: Hanging meta key on focus change via meta shortcut on Gnome/X11

This is the issue I was initially encountering which also motivated me to open this pull request. I think this issue is actually really tied to Gnome on X11 and the meta key. So it is quite specific. I was not able to reproduce it on Gnome/Wayland nor on i3 nor XFCE.

Here you can find some more information on the issue as I documented it over at neovide neovide/neovide#931 (comment). Another neovide user reported a similar issue. My patch seems to solve the problem for him as well (neovide/neovide#942). Actually, to me it looks a bit like gnome is misbehaving here. Still I wanted to fix the issue on the neovide/winit level, because others seem to be able to handle Gnome's weird behavior as well (e.g. SDL2 does not have the problem on Gnome/X11, I verified this with another test program).

  • Set the left meta key to be the "Overview shortcut" in Gnome Tweaks App, Keyboard & Mouse (this is the Gnome default setting)
  • Set a key combination using the left meta key (e.g. meta-up) to maximize the window via Gnome Settings, Keyboard, Customize Shortcuts (this is the Gnome default setting)
  • Start the test program and focus the window with the mouse (should be actually focused automatically)
  • Use meta-up to maximize the window, release up and meta keys
  • Press another modifier key such as shift and check the current modifier status reported by the test program

rust-windowing/winit master

rust-windowing@1b3b82a

Output (lines starting with // are comments added by me afterwards, meta is called LOGO here)

2021-09-12 22:45:20,567 TRACE [mio::poll] registering event source with poller: token=Token(0), interests=READABLE
2021-09-12 22:45:20,576 INFO [winit::platform_impl::platform::x11::window] Guessed window scale factor: 1
2021-09-12 22:45:20,576 DEBUG [winit::platform_impl::platform::x11::window] Calculated physical dimensions: 512x512
// here I press the left meta key, the window looses focus
APP: Mod LOGO
APP: Key None Released syn
APP: Mod (empty)
// here I press and release the up key and the window maximizes and regains focus
APP: Mod LOGO
// here I release the meta key
// here I press the shift key
APP: Mod SHIFT | LOGO
APP: Mod SHIFT
APP: Key Some(RShift) Pressed

As you can see in the output, the meta modifier is reported together with the shift modifier after the window got maximized, even though only the shift key is pressed. Another modifier change event is fired directly afterwards fixing the modifier state report, though, no key release event is fired to justify the modifier state change.

This is not the behavior I'd expect as the meta key is reported while not pressed and the modifier state is not in sync with key press and release events.

maroider/winit new-keyboard-linux

3ae1593

Output (lines starting with // are comments added by me afterwards, meta is called SUPER here)

2021-09-12 22:57:20,753 TRACE [mio::poll] registering event source with poller: token=Token(0), interests=READABLE
2021-09-12 22:57:20,764 INFO [winit::platform_impl::platform::x11::window] Guessed window scale factor: 1
2021-09-12 22:57:20,764 DEBUG [winit::platform_impl::platform::x11::window] Calculated physical dimensions: 512x512
// here I press the left meta key, the window looses focus
APP: Mod SUPER
APP: Key Super Pressed
APP: Key Super Released syn
APP: Mod (empty)
// here I press and release the up key and the window maximizes and regains focus
APP: Mod SUPER
// here I release the meta key
// here I press the shift key
APP: Mod SHIFT | SUPER
APP: Key Shift Pressed
// here I release the shift key
APP: Mod SUPER
APP: Key Shift Released

As you can see in the output, the meta modifier is reported together with the shift modifier after the window got maximized, even though only the shift key is pressed. After releasing the shift key, still, the meta key is reported, even though it was released long before. The meta key hangs. This is the actual issue I am trying to fix. Comparing this behavior to the master branch, I'd even say this is a regression. As the master branch recovers from this state on it's own. This branch needs another re-focus or mouse move to recover.

This is not the behavior I'd expect as the meta key is reported while not pressed and the modifier state is not in sync with key press and release events.

rti/winit fix-gnome-x11-meta-mod-focus-maroider

rti@9fea819

Output (lines starting with // are comments added by me afterwards, meta is called SUPER here)

2021-09-12 23:06:19,938 TRACE [mio::poll] registering event source with poller: token=Token(0), interests=READABLE
2021-09-12 23:06:19,950 INFO [winit::platform_impl::platform::x11::window] Guessed window scale factor: 1
2021-09-12 23:06:19,950 DEBUG [winit::platform_impl::platform::x11::window] Calculated physical dimensions: 512x512
// here I press the left meta key, the window looses focus
APP: Mod SUPER
APP: Key Super Pressed
APP: Mod (empty)
APP: Key Super Released syn
// here I press and release the up key and the window maximizes and regains focus
// here I release the meta key
// here I press the shift key
APP: Mod SHIFT
APP: Key Shift Pressed
// here I release the shift key
APP: Mod (empty)
APP: Key Shift Released

As you can see in the output, the key and modifier state is consistent. This is the behavior I'd expect.

Please excuse the way too long comment. I hope it solved the purpose to share my thoughts and motivation with you.

@rti
Copy link
Author

rti commented Sep 20, 2021

Please let me know if I can be of any further help on this topic.

@maroider
Copy link
Owner

maroider commented Oct 6, 2021

Unfortunately I cannot reproduce the issue you describe, neither on the current master nor on the new-keyboard-linux branch.

I've reproduced the bug on both branches, so it might just be a Sway bug.

I've discovered another thing, though: I seem to have forgotten entirely about synthetic events when implementing this, as I can't seem to get any at all with the new-keyboard-linux branch. This PR seems to at least add some synthetic events, but they're not consistently emitted.

@maroider
Copy link
Owner

maroider commented Nov 17, 2021

I'm apparently dumb. Synthetic events work fine for me on X11, and this PR has nothing to do with

Furthermore, this PR appears to work fine for me with i3.

@maroider maroider merged commit c48a68e into maroider:new-keyboard-linux Nov 17, 2021
@rti
Copy link
Author

rti commented Nov 22, 2021

Thanks a lot for merging this!

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