From 83f80d1362d5ddffd2d36e2591bbe9c462b063fd Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 5 Jan 2024 09:30:12 +0100 Subject: [PATCH 1/6] Report correct logical key on Mac when Ctrl or Cmd is pressed --- CHANGELOG.md | 1 + src/platform_impl/macos/event.rs | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df9fe00acd..e573465831 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Unreleased` header. - **Breaking:** Rename `VideoMode` to `VideoModeHandle` to represent that it doesn't hold static data. - **Breaking:** No longer export `platform::x11::XNotSupported`. - **Breaking:** Renamed `platform::x11::XWindowType` to `platform::x11::WindowType`. +- On macOS, report correct logical key when Ctrl or Cmd is pressed. # 0.29.8 diff --git a/src/platform_impl/macos/event.rs b/src/platform_impl/macos/event.rs index b4124ee05d..ac14897ca9 100644 --- a/src/platform_impl/macos/event.rs +++ b/src/platform_impl/macos/event.rs @@ -116,7 +116,16 @@ pub(crate) fn create_key_event( let text_with_all_modifiers: Option = if key_override.is_some() { None } else { - let characters = unsafe { ns_event.characters() } + // The logical key should always be the key resulting from applying all modifiers. + // Consider these cases: + // 1) Pressing the A key: logical key should be "a" + // 2) Pressing SHIFT A: logical key should be "A" + // 3) Pressing CTRL SHIFT A: logical key should also be "A" + // `ns_event.characters()` gets 3) wrong, returning "a" instead of "A". + // In fact it gets it wrong anytime CTRL or CMD is pressed. + // `ns_event.charactersIgnoringModifiers()` gets everything right. + // See https://github.com/rust-windowing/winit/issues/3078 + let characters = unsafe { ns_event.charactersIgnoringModifiers() } .map(|s| s.to_string()) .unwrap_or_default(); if characters.is_empty() { From efe00c80520984eeedce689747d0cbd7ea33a3b8 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 5 Jan 2024 09:51:09 +0100 Subject: [PATCH 2/6] Improve comment --- src/platform_impl/macos/event.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/platform_impl/macos/event.rs b/src/platform_impl/macos/event.rs index ac14897ca9..3fc5488cc6 100644 --- a/src/platform_impl/macos/event.rs +++ b/src/platform_impl/macos/event.rs @@ -123,8 +123,10 @@ pub(crate) fn create_key_event( // 3) Pressing CTRL SHIFT A: logical key should also be "A" // `ns_event.characters()` gets 3) wrong, returning "a" instead of "A". // In fact it gets it wrong anytime CTRL or CMD is pressed. - // `ns_event.charactersIgnoringModifiers()` gets everything right. - // See https://github.com/rust-windowing/winit/issues/3078 + // `ns_event.charactersIgnoringModifiers()` gets everything right, + // because it ignores all modifiers except for shift. + // https://developer.apple.com/documentation/appkit/nsevent/1524605-charactersignoringmodifiers + // https://github.com/rust-windowing/winit/issues/3078 let characters = unsafe { ns_event.charactersIgnoringModifiers() } .map(|s| s.to_string()) .unwrap_or_default(); From d6f6f349f3b6ed4549f13e5acb7342e25d4d3d51 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 5 Jan 2024 10:03:56 +0100 Subject: [PATCH 3/6] Revert first fix, do a different one --- src/platform_impl/macos/event.rs | 35 +++++++++++++++++++------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/platform_impl/macos/event.rs b/src/platform_impl/macos/event.rs index 3fc5488cc6..ef543ef893 100644 --- a/src/platform_impl/macos/event.rs +++ b/src/platform_impl/macos/event.rs @@ -86,6 +86,7 @@ pub fn get_modifierless_char(scancode: u16) -> Key { Key::Character(SmolStr::new(chars)) } +// Ignores all modifiers except for shift fn get_logical_key_char(ns_event: &NSEvent, modifierless_chars: &str) -> Key { let string = unsafe { ns_event.charactersIgnoringModifiers() } .map(|s| s.to_string()) @@ -113,21 +114,24 @@ pub(crate) fn create_key_event( let scancode = unsafe { ns_event.keyCode() }; let mut physical_key = key_override.unwrap_or_else(|| scancode_to_physicalkey(scancode as u32)); + // NOTE: + // The logical key should be the key resulting from applying all modifiers. + // Consider these cases: + // 1) Pressing the A key: logical key should be "a" + // 2) Pressing SHIFT A: logical key should be "A" + // 3) Pressing CTRL SHIFT A: logical key should also be "A" + // `ns_event.characters()` gets 3) wrong, returning "a" instead of "A". + // In fact it gets it wrong anytime CTRL or CMD is pressed. + // `ns_event.charactersIgnoringModifiers()` gets everything right, + // because it ignores all modifiers except for shift. + // https://developer.apple.com/documentation/appkit/nsevent/1524605-charactersignoringmodifiers + // https://github.com/rust-windowing/winit/issues/3078 + + // `text_with_all_modifiers` is unreliable when ctrl or cmd is pressed let text_with_all_modifiers: Option = if key_override.is_some() { None } else { - // The logical key should always be the key resulting from applying all modifiers. - // Consider these cases: - // 1) Pressing the A key: logical key should be "a" - // 2) Pressing SHIFT A: logical key should be "A" - // 3) Pressing CTRL SHIFT A: logical key should also be "A" - // `ns_event.characters()` gets 3) wrong, returning "a" instead of "A". - // In fact it gets it wrong anytime CTRL or CMD is pressed. - // `ns_event.charactersIgnoringModifiers()` gets everything right, - // because it ignores all modifiers except for shift. - // https://developer.apple.com/documentation/appkit/nsevent/1524605-charactersignoringmodifiers - // https://github.com/rust-windowing/winit/issues/3078 - let characters = unsafe { ns_event.charactersIgnoringModifiers() } + let characters = unsafe { ns_event.characters() } .map(|s| s.to_string()) .unwrap_or_default(); if characters.is_empty() { @@ -143,18 +147,21 @@ pub(crate) fn create_key_event( let key_from_code = code_to_key(physical_key, scancode); let (logical_key, key_without_modifiers) = if matches!(key_from_code, Key::Unidentified(_)) { + // `key_without_modifiers` ignores all modifiers except for shift. let key_without_modifiers = get_modifierless_char(scancode); let modifiers = unsafe { ns_event.modifierFlags() }; let has_ctrl = flags_contains(modifiers, NSEventModifierFlagControl); + let has_cmd = flags_contains(modifiers, NSEventModifierFlagCommand); let logical_key = match text_with_all_modifiers.as_ref() { - // Only checking for ctrl here, not checking for alt because we DO want to + // Only checking for ctrl and cmd here, not checking for alt because we DO want to // include its effect in the key. For example if -on the Germay layout- one // presses alt+8, the logical key should be "{" // Also not checking if this is a release event because then this issue would // still affect the key release. - Some(text) if !has_ctrl => Key::Character(text.clone()), + Some(text) if !has_ctrl && !has_cmd => Key::Character(text.clone()), + _ => match key_without_modifiers.as_ref() { Key::Character(ch) => get_logical_key_char(ns_event, ch), // Don't try to get text for events which likely don't have it. From 26ffd175722e50d23e71dfda48d3454bda5cd71b Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 5 Jan 2024 10:47:08 +0100 Subject: [PATCH 4/6] Improve comments --- src/platform_impl/macos/event.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/platform_impl/macos/event.rs b/src/platform_impl/macos/event.rs index ef543ef893..4fa9b5cdac 100644 --- a/src/platform_impl/macos/event.rs +++ b/src/platform_impl/macos/event.rs @@ -28,6 +28,7 @@ pub struct KeyEventExtra { pub key_without_modifiers: Key, } +/// Ignores ALL modifiers. pub fn get_modifierless_char(scancode: u16) -> Key { let mut string = [0; 16]; let input_source; @@ -86,7 +87,7 @@ pub fn get_modifierless_char(scancode: u16) -> Key { Key::Character(SmolStr::new(chars)) } -// Ignores all modifiers except for shift +// Ignores all modifiers except for SHIFT (yes, even ALT is ignored). fn get_logical_key_char(ns_event: &NSEvent, modifierless_chars: &str) -> Key { let string = unsafe { ns_event.charactersIgnoringModifiers() } .map(|s| s.to_string()) @@ -147,7 +148,7 @@ pub(crate) fn create_key_event( let key_from_code = code_to_key(physical_key, scancode); let (logical_key, key_without_modifiers) = if matches!(key_from_code, Key::Unidentified(_)) { - // `key_without_modifiers` ignores all modifiers except for shift. + // `get_modifierless_char/key_without_modifiers` ignores ALL modifiers. let key_without_modifiers = get_modifierless_char(scancode); let modifiers = unsafe { ns_event.modifierFlags() }; @@ -160,11 +161,16 @@ pub(crate) fn create_key_event( // presses alt+8, the logical key should be "{" // Also not checking if this is a release event because then this issue would // still affect the key release. - Some(text) if !has_ctrl && !has_cmd => Key::Character(text.clone()), + Some(text) if !has_ctrl && !has_cmd => { + // Character heeding both SHIFT and ALT. + Key::Character(text.clone()) + } _ => match key_without_modifiers.as_ref() { + // Character heeding just SHIFT, ignoring ALT. Key::Character(ch) => get_logical_key_char(ns_event, ch), - // Don't try to get text for events which likely don't have it. + + // Character ignoring ALL modifiers. _ => key_without_modifiers.clone(), }, }; From a634fec6bf0044aeb27bdb840c4db8f0509dc81e Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 5 Jan 2024 10:49:10 +0100 Subject: [PATCH 5/6] improve comment --- src/platform_impl/macos/event.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/platform_impl/macos/event.rs b/src/platform_impl/macos/event.rs index 4fa9b5cdac..18e090d2ae 100644 --- a/src/platform_impl/macos/event.rs +++ b/src/platform_impl/macos/event.rs @@ -115,18 +115,12 @@ pub(crate) fn create_key_event( let scancode = unsafe { ns_event.keyCode() }; let mut physical_key = key_override.unwrap_or_else(|| scancode_to_physicalkey(scancode as u32)); - // NOTE: - // The logical key should be the key resulting from applying all modifiers. - // Consider these cases: - // 1) Pressing the A key: logical key should be "a" - // 2) Pressing SHIFT A: logical key should be "A" - // 3) Pressing CTRL SHIFT A: logical key should also be "A" - // `ns_event.characters()` gets 3) wrong, returning "a" instead of "A". - // In fact it gets it wrong anytime CTRL or CMD is pressed. - // `ns_event.charactersIgnoringModifiers()` gets everything right, - // because it ignores all modifiers except for shift. - // https://developer.apple.com/documentation/appkit/nsevent/1524605-charactersignoringmodifiers - // https://github.com/rust-windowing/winit/issues/3078 + // NOTE: The logical key should heed both SHIFT and ALT if possible. + // For instance: + // * Pressing the A key: logical key should be "a" + // * Pressing SHIFT A: logical key should be "A" + // * Pressing CTRL SHIFT A: logical key should also be "A" + // This is not easy to tease out of `NSEvent`, but we do our best. // `text_with_all_modifiers` is unreliable when ctrl or cmd is pressed let text_with_all_modifiers: Option = if key_override.is_some() { From 561bbc47c75fbb983b0acf450469ab79c92dcc99 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 5 Jan 2024 11:32:36 +0100 Subject: [PATCH 6/6] Remove useful comment --- src/platform_impl/macos/event.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/platform_impl/macos/event.rs b/src/platform_impl/macos/event.rs index 18e090d2ae..ff863f38fa 100644 --- a/src/platform_impl/macos/event.rs +++ b/src/platform_impl/macos/event.rs @@ -122,7 +122,6 @@ pub(crate) fn create_key_event( // * Pressing CTRL SHIFT A: logical key should also be "A" // This is not easy to tease out of `NSEvent`, but we do our best. - // `text_with_all_modifiers` is unreliable when ctrl or cmd is pressed let text_with_all_modifiers: Option = if key_override.is_some() { None } else {