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

Add prefix byte for extended scancodes on Windows #1679

Merged
merged 7 commits into from
Oct 19, 2020

Conversation

vickles
Copy link
Contributor

@vickles vickles commented Aug 25, 2020

Currently on Windows only the low byte of scancodes is passed back through the KeyboardInput{ scancode } field. This causes scancode collisions between different keys.

Some examples that I've run up against in my game project:

  • Both HOME and NumPad 7 map to 0x47. HOME should be 0xE0 47, and NumPad 7 should be 0x47.
  • Both UP and NumPad 8 map to 0x48. UP should be 0xE0 48, and NumPad 8 should be 0x48.

I've been referencing this Microsoft doc: Keyboard Scan Code Specification. It seems to contains the full breakdown of how scancodes are defined on windows.

This change prepends the prefix byte to scancodes which have the extended bit set. It allows me to distinguish between keys properly by scancode, and aligns them with the codes in that spec doc.

@vickles vickles force-pushed the windows-multibyte-scancodes branch 3 times, most recently from a839b6d to bc3c111 Compare August 29, 2020 15:56
@vickles vickles force-pushed the windows-multibyte-scancodes branch from bc3c111 to 5ab8642 Compare September 6, 2020 19:39
@kchibisov
Copy link
Member

@vickles just fyi, since you rebase that patch from time to time, we haven't forgot about it, but since neither @chrisduerr nor me maintaining Windows, we're not that fast to take actions when it comes to it. However if it'll take too long for Windows maintainer(s) to appear we'll likely go back to it ourselves, even though we don't use Windows in particular.

@vickles
Copy link
Contributor Author

vickles commented Sep 11, 2020

No problem. I'm updating my branch both to make sure it stays mergeable here, and also because I'm using that branch on my fork as the source of winit in my project

@vickles vickles force-pushed the windows-multibyte-scancodes branch from c399c0a to acc0246 Compare October 3, 2020 20:28
@@ -343,6 +343,7 @@ pub fn handle_extended_keys(
extended: bool,
) -> Option<(c_int, UINT)> {
// Welcome to hell https://blog.molecular-matters.com/2011/09/05/properly-handling-keyboard-input/
scancode = if extended { 0xE000 } else { 0x0000 } | scancode;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This affects the PAUSE/SCROLL logic below as it doesn't filter the additional event caused by RAWINPUT:

PR:

DeviceEvent { device_id: DeviceId(DeviceId(65599)), event: Key(KeyboardInput { scancode: 57373, state: Released, virtual_keycode: Some(Pause), modifiers: (empty) }) }
WindowEvent { window_id: WindowId(WindowId(0x75065e)), event: KeyboardInput { device_id: DeviceId(DeviceId(0)), input: KeyboardInput { scancode: 69, state: Released, virtual_keycode: Some(Pause), modifiers: (empty) }, is_synthetic: false } }
DeviceEvent { device_id: DeviceId(DeviceId(65599)), event: Key(KeyboardInput { scancode: 69, state: Released, virtual_keycode: Some(Pause), modifiers: (empty) }) }

master branch:

WindowEvent { window_id: WindowId(WindowId(0xd06cc)), event: KeyboardInput { device_id:  DeviceId(DeviceId(0)), input: KeyboardInput { scancode: 69, state: Released, virtual_keycode: Some(Pause), modifiers: (empty) }, is_synthetic: false } }
DeviceEvent { device_id: DeviceId(DeviceId(65599)), event: Key(KeyboardInput { scancode: 69, state: Released, virtual_keycode: Some(Pause), modifiers: (empty) }) }

Copy link
Contributor Author

@vickles vickles Oct 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated this logic. I tested master and found it was reporting the same scancode and virtual key for pause and numlock. The updated version should handle them both as separate entities better. I also tested scroll lock, pause and numlock with a variety of modifiers and it seems to be looking happy now.

@vickles vickles requested a review from msiglreith October 19, 2020 07:52
@msiglreith msiglreith merged commit fbd3918 into rust-windowing:master Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants