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

On Wayland, add missing virtual key codes mappings #1534

Closed
wants to merge 1 commit into from

Conversation

kchibisov
Copy link
Member

  • 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

cc @vberger

@elinorbgr
Copy link
Contributor

I'm ok with these changes, however I notice there is some inconsistency in the handling of keys of the first row in winit.

Historically these keys were always matched to the numbers, irrespective of the actual keymap. In a game-oriented fashion.

Now, for some platforms winit no longer does this and just follow the keymap, for others it still does. Like macos for example:

'1' | '!' => VirtualKeyCode::Key1,
'2' | '@' => VirtualKeyCode::Key2,
'3' | '#' => VirtualKeyCode::Key3,
'4' | '$' => VirtualKeyCode::Key4,
'5' | '%' => VirtualKeyCode::Key5,
'6' | '^' => VirtualKeyCode::Key6,
'7' | '&' => VirtualKeyCode::Key7,
'8' | '*' => VirtualKeyCode::Key8,
'9' | '(' => VirtualKeyCode::Key9,
'0' | ')' => VirtualKeyCode::Key0,

This PR also removes this behavior for Wayland, which currently still has it. Is there something decided about that ? @Osspial @goddessfreya

@kchibisov
Copy link
Member Author

Now, for some platforms winit no longer does this and just follow the keymap, for others it still does. Like macos for example:

I wouldn't count macOS here, since its events are completely broken most of the time and for example alacritty has all the bugs due to that, note, you've linked a bit wrong code here, since the thing Wayland was doing is the same as Scancode to VirtualKeyCode mapping, which is here

0x12 => VirtualKeyCode::Key1,
0x13 => VirtualKeyCode::Key2,
0x14 => VirtualKeyCode::Key3,
0x15 => VirtualKeyCode::Key4,
0x16 => VirtualKeyCode::Key6,
0x17 => VirtualKeyCode::Key5,
0x18 => VirtualKeyCode::Equals,
0x19 => VirtualKeyCode::Key9,
0x1a => VirtualKeyCode::Key7,
0x1b => VirtualKeyCode::Minus,
0x1c => VirtualKeyCode::Key8,
0x1d => VirtualKeyCode::Key0,
0x1e => VirtualKeyCode::RBracket,

But it also doesn't get executed in some cases, and the code you've linked is the exact this case, when it decides to do something differently, it maps character it got from macOS to a VirtualKeyCode, so there's no guarantee that "1" was physically 1 under the hood.

Anyway, I'd like to wait here for maintainers as well.

@lovesegfault
Copy link

I applied this diff to glutin:

diff --git a/glutin/Cargo.toml b/glutin/Cargo.toml
index 10d580b..5593f28 100644
--- a/glutin/Cargo.toml
+++ b/glutin/Cargo.toml
@@ -18,7 +18,7 @@ serde = ["winit/serde"]
 
 [dependencies]
 lazy_static = "1.3"
-winit = "0.22.0"
+winit = { git = "https://github.com/kchibisov/winit", branch = "wayland-keysyms" }
 
 [target.'cfg(target_os = "android")'.dependencies]
 android_glue = "0.2"
diff --git a/glutin_examples/Cargo.toml b/glutin_examples/Cargo.toml
index 49faae0..3d35945 100644
--- a/glutin_examples/Cargo.toml
+++ b/glutin_examples/Cargo.toml
@@ -12,7 +12,7 @@ publish = false
 
 [dependencies]
 glutin = { path = "../glutin" }
-winit = "0.20.0"
+winit = { git = "https://github.com/kchibisov/winit", branch = "wayland-keysyms" }
 takeable-option = "0.4"
 image = "0.21"

And proceeded to try the window example, but the results were the same as reported in #1533, no virtual keycodes show up. Logs:

NewEvents(WaitCancelled { start: Instant { tv_sec: 48810, tv_nsec: 113933706 }, requested_resume: None })
WindowEvent { window_id: WindowId(Wayland(WindowId(94879317467360))), event: KeyboardInput { device_id: DeviceId(Wayland(DeviceId)), input: KeyboardInput { scancode: 30, state: Released, virtual_keycode: None, modifiers: (empty) }, is_synthetic: false } }
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48810, tv_nsec: 114025276 }, requested_resume: None })
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48810, tv_nsec: 457933180 }, requested_resume: None })
WindowEvent { window_id: WindowId(Wayland(WindowId(94879317467360))), event: KeyboardInput { device_id: DeviceId(Wayland(DeviceId)), input: KeyboardInput { scancode: 31, state: Pressed, virtual_keycode: None, modifiers: (empty) }, is_synthetic: false } }
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48810, tv_nsec: 458021348 }, requested_resume: None })
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48810, tv_nsec: 597929953 }, requested_resume: None })
WindowEvent { window_id: WindowId(Wayland(WindowId(94879317467360))), event: KeyboardInput { device_id: DeviceId(Wayland(DeviceId)), input: KeyboardInput { scancode: 31, state: Released, virtual_keycode: None, modifiers: (empty) }, is_synthetic: false } }
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48810, tv_nsec: 598019035 }, requested_resume: None })
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48811, tv_nsec: 219856410 }, requested_resume: None })
WindowEvent { window_id: WindowId(Wayland(WindowId(94879317467360))), event: KeyboardInput { device_id: DeviceId(Wayland(DeviceId)), input: KeyboardInput { scancode: 32, state: Pressed, virtual_keycode: None, modifiers: (empty) }, is_synthetic: false } }
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48811, tv_nsec: 219947645 }, requested_resume: None })
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48811, tv_nsec: 371926522 }, requested_resume: None })
WindowEvent { window_id: WindowId(Wayland(WindowId(94879317467360))), event: KeyboardInput { device_id: DeviceId(Wayland(DeviceId)), input: KeyboardInput { scancode: 32, state: Released, virtual_keycode: None, modifiers: (empty) }, is_synthetic: false } }
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48811, tv_nsec: 372014896 }, requested_resume: None })
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48812, tv_nsec: 547937773 }, requested_resume: None })
WindowEvent { window_id: WindowId(Wayland(WindowId(94879317467360))), event: KeyboardInput { device_id: DeviceId(Wayland(DeviceId)), input: KeyboardInput { scancode: 126, state: Pressed, virtual_keycode: None, modifiers: (empty) }, is_synthetic: false } }
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48812, tv_nsec: 548027877 }, requested_resume: None })
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48812, tv_nsec: 855930938 }, requested_resume: None })
WindowEvent { window_id: WindowId(Wayland(WindowId(94879317467360))), event: KeyboardInput { device_id: DeviceId(Wayland(DeviceId)), input: KeyboardInput { scancode: 42, state: Pressed, virtual_keycode: None, modifiers: (empty) }, is_synthetic: false } }
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48812, tv_nsec: 856020114 }, requested_resume: None })
MainEventsCleared
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { tv_sec: 48813, tv_nsec: 683847196 }, requested_resume: None })
WindowEvent { window_id: WindowId(Wayland(WindowId(94879317467360))), event: CloseRequested }
MainEventsCleared
RedrawEventsCleared
LoopDestroyed

@kchibisov
Copy link
Member Author

We've already figured out with @lovesegfault on IRC, that his issue was due to NixOS and rpath.

@jbeich
Copy link

jbeich commented Jun 10, 2020

This fixes Ctrl+] on us(dvp) layout for me.

@JMS55
Copy link

JMS55 commented Jun 26, 2020

Is this PR waiting on anything? This would be useful to me, as for my game, I use the period . key, but it doesn't work on Wayland currently.

@carlsmedstad
Copy link

Also wondering if this is waiting on something.

I rebased this change on master with no conflicts, built Alacritty with it and it does indeed solve alacritty/alacritty#3998.

@kchibisov
Copy link
Member Author

Since there was no concerns being made this PR will be a part of #1653.

@kchibisov kchibisov closed this Aug 28, 2020
@kchibisov kchibisov deleted the wayland-keysyms branch August 28, 2020 06:46
@kchibisov kchibisov restored the wayland-keysyms branch August 28, 2020 06:46
@kchibisov kchibisov deleted the wayland-keysyms branch August 28, 2020 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants