Fix incorrect keycodes when using a non-US keyboard layout. (#755)

* Fix incorrect keycodes when using a non-US keyboard layout.

This commit fixes the issue described in #752, and uses the advised
method to fix it.

* Style fixes

Co-Authored-By: Toqozz <toqoz@hotmail.com>

* Refactoring of macOS `virtualkeycode` fix (#752)

* Applies requested changes as per pull request discussion (#755).

CHANGELOG.md

@@ -37,6 +37,7 @@
     - `LoopDestroyed` is emitted when the `run` or `run_return` method is about to exit.
 - Rename `MonitorId` to `MonitorHandle`.
 - Removed `serde` implementations from `ControlFlow`.
+- On macOS, fix keycodes being incorrect when using a non-US keyboard layout.
 - On Wayland, fix `with_title()` not setting the windows title
 - On Wayland, add `set_wayland_theme()` to control client decoration color theme
 - Added serde serialization to `os::unix::XWindowType`.

src/platform_impl/macos/event_loop.rs

@@ -544,7 +544,67 @@ impl Proxy {
     }
 }
 
-pub fn to_virtual_key_code(code: c_ushort) -> Option<events::VirtualKeyCode> {
+pub fn char_to_keycode(c: char) -> Option<events::VirtualKeyCode> {
+    // We only translate keys that are affected by keyboard layout.
+    //
+    // Note that since keys are translated in a somewhat "dumb" way (reading character)
+    // there is a concern that some combination, i.e. Cmd+char, causes the wrong
+    // letter to be received, and so we receive the wrong key.
+    //
+    // Implementation reference: https://github.com/WebKit/webkit/blob/82bae82cf0f329dbe21059ef0986c4e92fea4ba6/Source/WebCore/platform/cocoa/KeyEventCocoa.mm#L626
+    Some(match c {
+        'a' | 'A' => events::VirtualKeyCode::A,
+        'b' | 'B' => events::VirtualKeyCode::B,
+        'c' | 'C' => events::VirtualKeyCode::C,
+        'd' | 'D' => events::VirtualKeyCode::D,
+        'e' | 'E' => events::VirtualKeyCode::E,
+        'f' | 'F' => events::VirtualKeyCode::F,
+        'g' | 'G' => events::VirtualKeyCode::G,
+        'h' | 'H' => events::VirtualKeyCode::H,
+        'i' | 'I' => events::VirtualKeyCode::I,
+        'j' | 'J' => events::VirtualKeyCode::J,
+        'k' | 'K' => events::VirtualKeyCode::K,
+        'l' | 'L' => events::VirtualKeyCode::L,
+        'm' | 'M' => events::VirtualKeyCode::M,
+        'n' | 'N' => events::VirtualKeyCode::N,
+        'o' | 'O' => events::VirtualKeyCode::O,
+        'p' | 'P' => events::VirtualKeyCode::P,
+        'q' | 'Q' => events::VirtualKeyCode::Q,
+        'r' | 'R' => events::VirtualKeyCode::R,
+        's' | 'S' => events::VirtualKeyCode::S,
+        't' | 'T' => events::VirtualKeyCode::T,
+        'u' | 'U' => events::VirtualKeyCode::U,
+        'v' | 'V' => events::VirtualKeyCode::V,
+        'w' | 'W' => events::VirtualKeyCode::W,
+        'x' | 'X' => events::VirtualKeyCode::X,
+        'y' | 'Y' => events::VirtualKeyCode::Y,
+        'z' | 'Z' => events::VirtualKeyCode::Z,
+        '1' | '!' => events::VirtualKeyCode::Key1,
+        '2' | '@' => events::VirtualKeyCode::Key2,
+        '3' | '#' => events::VirtualKeyCode::Key3,
+        '4' | '$' => events::VirtualKeyCode::Key4,
+        '5' | '%' => events::VirtualKeyCode::Key5,
+        '6' | '^' => events::VirtualKeyCode::Key6,
+        '7' | '&' => events::VirtualKeyCode::Key7,
+        '8' | '*' => events::VirtualKeyCode::Key8,
+        '9' | '(' => events::VirtualKeyCode::Key9,
+        '0' | ')' => events::VirtualKeyCode::Key0,
+        '=' | '+' => events::VirtualKeyCode::Equals,
+        '-' | '_' => events::VirtualKeyCode::Minus,
+        ']' | '}' => events::VirtualKeyCode::RBracket,
+        '[' | '{' => events::VirtualKeyCode::LBracket,
+        '\''| '"' => events::VirtualKeyCode::Apostrophe,
+        ';' | ':' => events::VirtualKeyCode::Semicolon,
+        '\\'| '|' => events::VirtualKeyCode::Backslash,
+        ',' | '<' => events::VirtualKeyCode::Comma,
+        '/' | '?' => events::VirtualKeyCode::Slash,
+        '.' | '>' => events::VirtualKeyCode::Period,
+        '`' | '~' => events::VirtualKeyCode::Grave,
+        _ => return None,
+    })
+}
+
+pub fn scancode_to_keycode(code: c_ushort) -> Option<events::VirtualKeyCode> {
     Some(match code {
         0x00 => events::VirtualKeyCode::A,
         0x01 => events::VirtualKeyCode::S,
@@ -680,20 +740,19 @@ pub fn to_virtual_key_code(code: c_ushort) -> Option<events::VirtualKeyCode> {
     })
 }
 
-pub fn check_additional_virtual_key_codes(
-    s: &Option<String>
+pub fn check_function_keys(
+    s: &String
 ) -> Option<events::VirtualKeyCode> {
-    if let &Some(ref s) = s {
-        if let Some(ch) = s.encode_utf16().next() {
-            return Some(match ch {
-                0xf718 => events::VirtualKeyCode::F21,
-                0xf719 => events::VirtualKeyCode::F22,
-                0xf71a => events::VirtualKeyCode::F23,
-                0xf71b => events::VirtualKeyCode::F24,
-                _ => return None,
-            })
-        }
+    if let Some(ch) = s.encode_utf16().next() {
+        return Some(match ch {
+            0xf718 => events::VirtualKeyCode::F21,
+            0xf719 => events::VirtualKeyCode::F22,
+            0xf71a => events::VirtualKeyCode::F23,
+            0xf71b => events::VirtualKeyCode::F24,
+            _ => return None,
+        })
     }
+
     None
 }
 
@@ -709,6 +768,16 @@ pub fn event_mods(event: cocoa::base::id) -> ModifiersState {
     }
 }
 
+pub fn get_scancode(event: cocoa::base::id) -> c_ushort {
+    // In AppKit, `keyCode` refers to the position (scancode) of a key rather than its character,
+    // and there is no easy way to navtively retrieve the layout-dependent character.
+    // In winit, we use keycode to refer to the key's character, and so this function aligns
+    // AppKit's terminology with ours.
+    unsafe {
+        msg_send![event, keyCode]
+    }
+}
+
 unsafe fn modifier_event(
     ns_event: cocoa::base::id,
     keymask: NSEventModifierFlags,
@@ -721,14 +790,14 @@ unsafe fn modifier_event(
         } else {
             ElementState::Pressed
         };
-        let keycode = NSEvent::keyCode(ns_event);
-        let scancode = keycode as u32;
-        let virtual_keycode = to_virtual_key_code(keycode);
+
+        let scancode = get_scancode(ns_event);
+        let virtual_keycode = scancode_to_keycode(scancode);
         Some(WindowEvent::KeyboardInput {
             device_id: DEVICE_ID,
             input: KeyboardInput {
                 state,
-                scancode,
+                scancode: scancode as u32,
                 virtual_keycode,
                 modifiers: event_mods(ns_event),
             },

src/platform_impl/macos/view.rs

@@ -14,10 +14,11 @@ use objc::declare::ClassDecl;
 use objc::runtime::{Class, Object, Protocol, Sel, BOOL, YES};
 
 use {ElementState, Event, KeyboardInput, MouseButton, WindowEvent, WindowId};
-use platform_impl::platform::event_loop::{DEVICE_ID, event_mods, Shared, to_virtual_key_code, check_additional_virtual_key_codes};
+use platform_impl::platform::event_loop::{DEVICE_ID, event_mods, Shared, to_virtual_key_code, check_additional_virtual_key_codes, get_scancode};
 use platform_impl::platform::util;
 use platform_impl::platform::ffi::*;
 use platform_impl::platform::window::{get_window_id, IdRef};
+use event;
 
 struct ViewState {
     window: id,
@@ -391,36 +392,68 @@ extern fn do_command_by_selector(this: &Object, _sel: Sel, command: Sel) {
     }
 }
 
-fn get_characters(event: id) -> Option<String> {
+fn get_characters(event: id, ignore_modifiers: bool) -> String {
     unsafe {
-        let characters: id = msg_send![event, characters];
+        let characters: id = if ignore_modifiers {
+            msg_send![event, charactersIgnoringModifiers]
+        } else {
+            msg_send![event, characters]
+        };
+
+        assert_ne!(characters, nil);
         let slice = slice::from_raw_parts(
             characters.UTF8String() as *const c_uchar,
             characters.len(),
         );
+
         let string = str::from_utf8_unchecked(slice);
-        Some(string.to_owned())
+
+        string.to_owned()
     }
 }
 
+// Retrieves a layout-independent keycode given an event.
+fn retrieve_keycode(event: id) -> Option<event::VirtualKeyCode> {
+    #[inline]
+    fn get_code(ev: id, raw: bool) -> Option<event::VirtualKeyCode> {
+        let characters = get_characters(ev, raw);
+        characters.chars().next().map_or(None, |c| char_to_keycode(c))
+    }
+
+    // Cmd switches Roman letters for Dvorak-QWERTY layout, so we try modified characters first.
+    // If we don't get a match, then we fall back to unmodified characters.
+    let code = get_code(event, false)
+        .or_else(|| {
+            get_code(event, true)
+        });
+
+    // We've checked all layout related keys, so fall through to scancode.
+    // Reaching this code means that the key is layout-independent (e.g. Backspace, Return).
+    //
+    // We're additionally checking here for F21-F24 keys, since their keycode
+    // can vary, but we know that they are encoded
+    // in characters property.
+    code.or_else(|| {
+        let scancode = get_scancode(event);
+        scancode_to_keycode(scancode)
+            .or_else(|| {
+                check_function_keys(&get_characters(event, true))
+            })
+    })
+}
+
 extern fn key_down(this: &Object, _sel: Sel, event: id) {
     //println!("keyDown");
     unsafe {
         let state_ptr: *mut c_void = *this.get_ivar("winitState");
         let state = &mut *(state_ptr as *mut ViewState);
         let window_id = WindowId(get_window_id(state.window));
+        let characters = get_characters(event, false);
 
-        state.raw_characters = get_characters(event);
+        state.raw_characters = Some(characters.clone());
 
-        let keycode: c_ushort = msg_send![event, keyCode];
-        // We are checking here for F21-F24 keys, since their keycode
-        // can vary, but we know that they are encoded
-        // in characters property.
-        let virtual_keycode = to_virtual_key_code(keycode)
-            .or_else(|| {
-                check_additional_virtual_key_codes(&state.raw_characters)
-            });
-        let scancode = keycode as u32;
+        let scancode = get_scancode(event) as u32;
+        let virtual_keycode = retrieve_keycode(event);
         let is_repeat = msg_send![event, isARepeat];
 
         let window_event = Event::WindowEvent {
@@ -436,17 +469,6 @@ extern fn key_down(this: &Object, _sel: Sel, event: id) {
             },
         };
 
-        let characters: id = msg_send![event, characters];
-        let slice = slice::from_raw_parts(
-            characters.UTF8String() as *const c_uchar,
-            characters.len(),
-            );
-        let string = str::from_utf8_unchecked(slice);
-
-        state.raw_characters = {
-            Some(string.to_owned())
-        };
-
         if let Some(shared) = state.shared.upgrade() {
             shared.pending_events
                 .lock()
@@ -454,7 +476,7 @@ extern fn key_down(this: &Object, _sel: Sel, event: id) {
                 .push_back(window_event);
             // Emit `ReceivedCharacter` for key repeats
             if is_repeat && state.is_key_down{
-                for character in string.chars() {
+                for character in characters.chars() {
                     let window_event = Event::WindowEvent {
                         window_id,
                         event: WindowEvent::ReceivedCharacter(character),
@@ -483,16 +505,9 @@ extern fn key_up(this: &Object, _sel: Sel, event: id) {
 
         state.is_key_down = false;
 
-        // We need characters here to check for additional keys such as
-        // F21-F24.
-        let characters = get_characters(event);
+        let scancode = get_scancode(event) as u32;
+        let virtual_keycode = retrieve_keycode(event);
 
-        let keycode: c_ushort = msg_send![event, keyCode];
-        let virtual_keycode = to_virtual_key_code(keycode)
-            .or_else(|| {
-                check_additional_virtual_key_codes(&characters)
-            });
-        let scancode = keycode as u32;
         let window_event = Event::WindowEvent {
             window_id: WindowId(get_window_id(state.window)),
             event: WindowEvent::KeyboardInput {