Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Releasing the Shift key does not set virtual_keycode #361

Closed
tomassedovic opened this issue Dec 9, 2017 · 13 comments
Closed

Releasing the Shift key does not set virtual_keycode #361

tomassedovic opened this issue Dec 9, 2017 · 13 comments
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched D - hard Likely harder than most tasks here DS - x11 H - help wanted Someone please save us P - normal Great to have

Comments

@tomassedovic
Copy link

When I press and release the Shift key, the WindowEvent for the release has virtual_keycode None rather than Some(RShift) (and the same is true for the left shift).

The DeviceEvent is fine, so is the key press.

To reproduce:

  1. run cargo run --example window
  2. press and release the shift key

Expected:

The final line should read:

WindowEvent { window_id: WindowId(X(WindowId(127926274))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(3))), input: KeyboardInput { scancode: 54, state: Released, virtual_keycode: Some(RShift), modifiers: ModifiersState { shift: true, ctrl: false, alt: false, logo: false } } } }

Actual:

WindowEvent { window_id: WindowId(X(WindowId(127926274))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(3))), input: KeyboardInput { scancode: 54, state: Released, virtual_keycode: None, modifiers: ModifiersState { shift: true, ctrl: false, alt: false, logo: false } } } }

(look at the virtual_keycode)

I ran git bisect and it ended up with commit 52a78d6

Here's the output that illustrate this on my computer (running Fedora 26 with Wayland I believe as well as on another computer with Fedora 24 + X11).

$ cargo run --example window
    Updating registry `https://github.com/rust-lang/crates.io-index`
   Compiling winit v0.9.0 (file:///home/thomas/code/winit)
    Finished dev [unoptimized + debuginfo] target(s) in 3.32 secs
     Running `target/debug/examples/window`
WindowEvent { window_id: WindowId(X(WindowId(127926274))), event: Resized(1024, 768) }
WindowEvent { window_id: WindowId(X(WindowId(127926274))), event: Moved(10, 35) }
WindowEvent { window_id: WindowId(X(WindowId(127926274))), event: Moved(1518, 161) }
WindowEvent { window_id: WindowId(X(WindowId(127926274))), event: Refresh }
WindowEvent { window_id: WindowId(X(WindowId(127926274))), event: CursorEntered { device_id: DeviceId(X(DeviceId(2))) } }
WindowEvent { window_id: WindowId(X(WindowId(30064771203))), event: CursorMoved { device_id: DeviceId(X(DeviceId(2))), position: (865, 239) } }
WindowEvent { window_id: WindowId(X(WindowId(127926274))), event: Focused(true) }
WindowEvent { window_id: WindowId(X(WindowId(38654705795))), event: CursorMoved { device_id: DeviceId(X(DeviceId(3))), position: (865, 239) } }
DeviceEvent { device_id: DeviceId(X(DeviceId(13))), event: Key(KeyboardInput { scancode: 54, state: Pressed, virtual_keycode: Some(RShift), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } }) }
WindowEvent { window_id: WindowId(X(WindowId(127926274))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(3))), input: KeyboardInput { scancode: 54, state: Pressed, virtual_keycode: Some(RShift), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } } } }
DeviceEvent { device_id: DeviceId(X(DeviceId(13))), event: Key(KeyboardInput { scancode: 54, state: Released, virtual_keycode: Some(RShift), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } }) }
WindowEvent { window_id: WindowId(X(WindowId(127926274))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(3))), input: KeyboardInput { scancode: 54, state: Released, virtual_keycode: None, modifiers: ModifiersState { shift: true, ctrl: false, alt: false, logo: false } } } }
@francesca64
Copy link
Member

I'm personally not able to reproduce this on X11, but is it resolved if you try this branch? It changes all the relevant code on X11, though Wayland is outside of my purview.

@tomassedovic
Copy link
Author

Thanks for looking into this!

I had to install libxkbcommon-x11-devel. Without it, it was crashing with:

thread 'main' panicked at 'Library libxkbcommon-x11.so could not be loaded.', /checkout/src/libcore/option.rs:839:4

After that, running cargo run --example window didn't fix it but the behaviour got a little weirder:

If I only pressed and released the Shift key after running it, it would report virtual_keycode: None for every event. If I press a few alphanumeric keys and only then press shift, I would get the "old" behaviour back: virtual_keycode being set for press but not release of shift.

@francesca64
Copy link
Member

I've updated the branch to update state much more correctly. I'm hoping that fixes things, but if not, I have some thoughts.

You're getting the correct scancode, so that means the problem must be in the keysym you're getting. Keysyms are based on both the scancode and your keyboard mapping, and they come from external functions (which are both official and beyond our control). I've added a line to log which keysym you're getting, since if we're lucky that will shed some light on things.

So here's some info that could help:

  • Which keysym you're getting (the expected value for RShift is 65506)
  • If the DeviceId listed for Pressed and Released are the same (keymaps are handled per-device; while they were all the same in the log you posted initially, I've changed how DeviceIds are determined)
  • setxkbmap -query

@tomassedovic
Copy link
Author

Good catch on the setxkbmap -query. I didn't mention this because it's not something I ever actually think about, but I've written my own XKB keyboard layout and the issue is only present when it's active. I've tried a bunch of other layouts and they all worked fine.

So this is something on my end. If you want to close the issue, be my guest. I'm a bit confused because this did use to work before, so it might be something we want to track down nonetheless.

In case it helps, here's the info you asked for:

$ setxkbmap -query
rules:      evdev
model:      pc105
layout:     gb,us,us
variant:    shadowkl_noswap,,

And here's the log. I run the example, then press & release left shift, press & release right shift and then close it with Alt+F4.

$ cargo run --example window 
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `target/debug/examples/window`
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: Resized(1024, 704) }
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: Moved(10, 45) }
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: Moved(50, 64) }
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: Refresh }
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: CursorEntered { device_id: DeviceId(X(DeviceId(2))) } }
WindowEvent { window_id: WindowId(X(WindowId(30064771203))), event: CursorMoved { device_id: DeviceId(X(DeviceId(2))), position: (922, 273) } }
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: Focused(true) }
WindowEvent { window_id: WindowId(X(WindowId(38654705795))), event: CursorMoved { device_id: DeviceId(X(DeviceId(3))), position: (922, 273) } }
KeySym 65509
DeviceEvent { device_id: DeviceId(X(DeviceId(10))), event: Key(KeyboardInput { scancode: 42, state: Pressed, virtual_keycode: None, modifiers: ModifiersState { shift: true, ctrl: false, alt: false, logo: false } }) }
KeySym 65509
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(10))), input: KeyboardInput { scancode: 42, state: Pressed, virtual_keycode: None, modifiers: ModifiersState { shift: true, ctrl: false, alt: false, logo: false } } } }
KeySym 65509
DeviceEvent { device_id: DeviceId(X(DeviceId(10))), event: Key(KeyboardInput { scancode: 42, state: Released, virtual_keycode: None, modifiers: ModifiersState { shift: true, ctrl: false, alt: false, logo: false } }) }
KeySym 65505
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(10))), input: KeyboardInput { scancode: 42, state: Released, virtual_keycode: Some(LShift), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } } } }
KeySym 65506
DeviceEvent { device_id: DeviceId(X(DeviceId(10))), event: Key(KeyboardInput { scancode: 54, state: Pressed, virtual_keycode: Some(RShift), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } }) }
KeySym 65509
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(10))), input: KeyboardInput { scancode: 54, state: Pressed, virtual_keycode: None, modifiers: ModifiersState { shift: true, ctrl: false, alt: false, logo: false } } } }
KeySym 65509
DeviceEvent { device_id: DeviceId(X(DeviceId(10))), event: Key(KeyboardInput { scancode: 54, state: Released, virtual_keycode: None, modifiers: ModifiersState { shift: true, ctrl: false, alt: false, logo: false } }) }
KeySym 65506
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(10))), input: KeyboardInput { scancode: 54, state: Released, virtual_keycode: Some(RShift), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } } } }
KeySym 65513
DeviceEvent { device_id: DeviceId(X(DeviceId(10))), event: Key(KeyboardInput { scancode: 56, state: Pressed, virtual_keycode: Some(LAlt), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } }) }
KeySym 65513
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(10))), input: KeyboardInput { scancode: 56, state: Pressed, virtual_keycode: Some(LAlt), modifiers: ModifiersState { shift: false, ctrl: false, alt: true, logo: false } } } }
KeySym 65473
DeviceEvent { device_id: DeviceId(X(DeviceId(10))), event: Key(KeyboardInput { scancode: 62, state: Pressed, virtual_keycode: Some(F4), modifiers: ModifiersState { shift: false, ctrl: false, alt: true, logo: false } }) }
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: Focused(false) }
WindowEvent { window_id: WindowId(X(WindowId(31457282))), event: Closed }

@francesca64
Copy link
Member

65509 is the keysym for caps lock.

In regards to it working in the past... my research shows that XKeycodeToKeysym (which was used prior to the commit you bisected to) is known for being broken, especially in regards to layouts, so it might've only worked accidentally.

I've made a GLFW program to test if GLFW gives you the expected events. If it does, then you can give me a copy of your layout and I can try to get to the bottom of why things are working differently here.

@sebmiq
Copy link

sebmiq commented Apr 19, 2018

Hey, same issue here. Namely, pressing and releasing LShift in cargo run --example window gives me

DeviceEvent { device_id: DeviceId(X(DeviceId(10))), event: Key(KeyboardInput { scancode: 42, state: Pressed, virtual_keycode: Some(LShift), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } }) }
WindowEvent { window_id: WindowId(X(WindowId(81788930))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(3))), input: KeyboardInput { scancode: 42, state: Pressed, virtual_keycode: Some(LShift), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } } } }
DeviceEvent { device_id: DeviceId(X(DeviceId(10))), event: Key(KeyboardInput { scancode: 42, state: Released, virtual_keycode: Some(LShift), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } }) }
WindowEvent { window_id: WindowId(X(WindowId(81788930))), event: KeyboardInput { device_id: DeviceId(X(DeviceId(3))), input: KeyboardInput { scancode: 42, state: Released, virtual_keycode: None, modifiers: ModifiersState { shift: true, ctrl: false, alt: false, logo: false } } } }

I've run your GLFW program, which ignores my layout and gives me

key: LeftShift, scancode: 50, state: Press, modifiers: (empty)
key: LeftShift, scancode: 50, state: Release, modifiers: Shift

My layout is set using the Xfce utility, and the issue remains w/e I change it to.

If there's anything I can do to help.

@francesca64
Copy link
Member

@JadeSnail I may have taken for granted that GLFW actually has good layout support. Anyway, the fact that the keycode is correct on the DeviceEvent but not the WindowEvent gives me some ideas. Can you give me some more information on your setup, or whatever else I'd need to reproduce this?

(Also, the branch I linked to earlier here is no longer relevant, so be sure to run against master.)

@sebmiq
Copy link

sebmiq commented Apr 19, 2018

I was running the latest version of winit.

What kind of information are you looking for ?

The issue occurs on both my desktop and laptop, which are running arch linux, X, and Xfce. It seems to occur regardless of the layout chosen in Xfce (I pick any layout, remove other layouts, even reboot).

Oh here's setxkbmap -query :

rules:      evdev
model:      pc105
layout:     fr,fr
variant:    bepo,azerty
options:    ctrl:nocaps,grp:shifts_toggle

@sebmiq
Copy link

sebmiq commented Apr 19, 2018

The grp:shifts_toggle option seems to be the culprit, if I remove it, the issue goes away.

@francesca64
Copy link
Member

Ah, then I should be able to look into how to fix this. What are some examples of programs it works correctly in?

@sebmiq
Copy link

sebmiq commented Apr 19, 2018

I don't use shift for anything other than capitilizing letters, so I really can't say.

I'd be happy to test things if you've got candidates with an easy way to know if it works or not.

The way Xfce deals with keyboard shortuts may «work correctly»: if I set a shortcut for Shift+A, and keep both pressed, the shortcut is repeatedly called, and when I release the Shift key, it isn't anymore.

@tomassedovic
Copy link
Author

Sorry for not answering earlier. On my setup, it's also related to having a behaviour for pressing both Shift keys at once. In my case this acts as a caps lock, not a layout toggle.

SDL2 handles this fine for me. I've adapted the keyboard-state example:

https://github.com/Rust-SDL2/rust-sdl2/blob/master/examples/keyboard-state.rs

extern crate sdl2;

use sdl2::event::Event;
use std::time::Duration;

pub fn main() {
    let sdl_context = sdl2::init().unwrap();
    let video_subsystem = sdl_context.video().unwrap();

    let _window = video_subsystem.window("Keyboard", 800, 600)
        .position_centered()
        .build()
        .unwrap();

    let mut events = sdl_context.event_pump().unwrap();

    'running: loop {
        for event in events.poll_iter() {
            println!("{:?}", event);
            if let Event::Quit {..} = event {
                break 'running;
            };
        }

        std::thread::sleep(Duration::from_millis(100));
    }
}

And running it, pressing and releasing left shift and then right shift, I get the following output:

$ cargo run --example keyboard-state
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `target/debug/examples/keyboard-state`
Window { timestamp: 69, window_id: 2, win_event: Shown }
Window { timestamp: 90, window_id: 2, win_event: Moved(140, 694) }
Unknown { timestamp: 91, type_: 772 }
Window { timestamp: 91, window_id: 2, win_event: Exposed }
Window { timestamp: 91, window_id: 2, win_event: FocusGained }
Window { timestamp: 92, window_id: 2, win_event: TakeFocus }
KeyDown { timestamp: 1896, window_id: 2, keycode: Some(LShift), scancode: Some(LShift), keymod: LSHIFTMOD | NUMMOD, repeat: false }
KeyUp { timestamp: 2095, window_id: 2, keycode: Some(LShift), scancode: Some(LShift), keymod: NUMMOD, repeat: false }
KeyDown { timestamp: 3097, window_id: 2, keycode: Some(RShift), scancode: Some(RShift), keymod: RSHIFTMOD | NUMMOD, repeat: false }
KeyUp { timestamp: 3298, window_id: 2, keycode: Some(RShift), scancode: Some(RShift), keymod: NUMMOD, repeat: false }

@francesca64 francesca64 added B - bug Dang, that shouldn't have happened H - help wanted Someone please save us DS - x11 C - needs investigation Issue must be confirmed and researched D - hard Likely harder than most tasks here P - normal Great to have labels May 6, 2018
@cymno
Copy link

cymno commented Apr 3, 2020

I can confirm this bug on Kubuntu 18.04, X11 with KDE Plasma 5.12.9.
In this demo using winit::events for key input, the Shift key release (shift is used for downwards movement) is not captured.

The bug only occurs when I set special behaviour of the shift key in my systems settings, changing it back even while the demo program is running eliminates the bug immediately. The bug is present whenever setxkbmap -query shows at least one of
shift:both_capslock_cancel, shift:both_capslock, or shift:both_shiftlock
in the options field

@tomassedovic's test using SDL2 shows the correct behaviour in both cases

kchibisov added a commit that referenced this issue May 28, 2023
Overhaul the keyboard API in winit to mimic the W3C specification
to achieve better crossplatform parity. The `KeyboardInput` event
is now uses `KeyEvent` which consists of:

  - `physical_key` - a cross platform way to refer to scancodes;
  - `logical_key`  - keysym value, which shows your key respecting the
                     layout;
  - `text`         - the text produced by this keypress;
  - `location`     - the location of the key on the keyboard;
  - `repeat`       - whether the key was produced by the repeat.

And also a `platform_specific` field which encapsulates extra
information on desktop platforms, like key without modifiers
and text with all modifiers.

The `Modifiers` were also slightly reworked as in, the information
whether the left or right modifier is pressed is now also exposed
on platforms where it could be queried reliably. The support was
also added for the web and orbital platforms finishing the API
change.

This change made the `OptionAsAlt` API on macOS redundant thus it
was removed all together.

Co-Authored-By: Artúr Kovács <kovacs.artur.barnabas@gmail.com>
Co-Authored-By: Kirill Chibisov <contact@kchibisov.com>
Co-Authored-By: daxpedda <daxpedda@gmail.com>
Fixes: #2631.
Fixes: #2055.
Fixes: #2032.
Fixes: #1904.
Fixes: #1810.
Fixes: #1700.
Fixes: #1443.
Fixes: #1343.
Fixes: #1208.
Fixes: #1151.
Fixes: #812.
Fixes: #600.
Fixes: #361.
Fixes: #343.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched D - hard Likely harder than most tasks here DS - x11 H - help wanted Someone please save us P - normal Great to have
Development

No branches or pull requests

5 participants