From 318deed25b4bad1c0d0cfc29749a50a06a87590b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 7 Jul 2023 12:02:08 +0200 Subject: [PATCH] Skip key down event if preceded by its key equivalent version Previously, we would only track whether the previous key down event was a key equivalent. However, this could cause issues when pressing certain keystrokes in rapid succession, e.g.: - Pressing `shift-right` (to select a character) - Pressing a character (with or without `shift` held down) This would cause GPUI to ignore the second event because it was preceded by a key equivalent event. With this commit, we track the last key equivalent event, and skip the key down event only if it matches the last key equivalent event. --- crates/gpui/src/platform/event.rs | 2 +- crates/gpui/src/platform/mac/window.rs | 59 ++++++++++---------------- 2 files changed, 24 insertions(+), 37 deletions(-) diff --git a/crates/gpui/src/platform/event.rs b/crates/gpui/src/platform/event.rs index c39c76dc34..4456db9a51 100644 --- a/crates/gpui/src/platform/event.rs +++ b/crates/gpui/src/platform/event.rs @@ -4,7 +4,7 @@ use pathfinder_geometry::vector::vec2f; use crate::{geometry::vector::Vector2F, keymap_matcher::Keystroke}; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct KeyDownEvent { pub keystroke: Keystroke, pub is_held: bool, diff --git a/crates/gpui/src/platform/mac/window.rs b/crates/gpui/src/platform/mac/window.rs index 3c82538611..381a4fbaaa 100644 --- a/crates/gpui/src/platform/mac/window.rs +++ b/crates/gpui/src/platform/mac/window.rs @@ -232,10 +232,6 @@ unsafe fn build_window_class(name: &'static str, superclass: &Class) -> *const C sel!(canBecomeKeyWindow), yes as extern "C" fn(&Object, Sel) -> BOOL, ); - decl.add_method( - sel!(sendEvent:), - send_event as extern "C" fn(&Object, Sel, id), - ); decl.add_method( sel!(windowDidResize:), window_did_resize as extern "C" fn(&Object, Sel, id), @@ -299,7 +295,7 @@ struct WindowState { appearance_changed_callback: Option>, input_handler: Option>, pending_key_down: Option<(KeyDownEvent, Option)>, - performed_key_equivalent: bool, + last_key_equivalent: Option, synthetic_drag_counter: usize, executor: Rc, scene_to_render: Option, @@ -521,7 +517,7 @@ impl Window { appearance_changed_callback: None, input_handler: None, pending_key_down: None, - performed_key_equivalent: false, + last_key_equivalent: None, synthetic_drag_counter: 0, executor, scene_to_render: Default::default(), @@ -965,36 +961,34 @@ extern "C" fn handle_key_event(this: &Object, native_event: id, key_equivalent: let window_height = window_state_borrow.content_size().y(); let event = unsafe { Event::from_native(native_event, Some(window_height)) }; - if let Some(event) = event { + if let Some(Event::KeyDown(event)) = event { + // For certain keystrokes, macOS will first dispatch a "key equivalent" event. + // If that event isn't handled, it will then dispatch a "key down" event. GPUI + // makes no distinction between these two types of events, so we need to ignore + // the "key down" event if we've already just processed its "key equivalent" version. if key_equivalent { - window_state_borrow.performed_key_equivalent = true; - } else if window_state_borrow.performed_key_equivalent { + window_state_borrow.last_key_equivalent = Some(event.clone()); + } else if window_state_borrow.last_key_equivalent.take().as_ref() == Some(&event) { return NO; } - let function_is_held; - window_state_borrow.pending_key_down = match event { - Event::KeyDown(event) => { - let keydown = event.keystroke.clone(); - // Ignore events from held-down keys after some of the initially-pressed keys - // were released. - if event.is_held { - if window_state_borrow.last_fresh_keydown.as_ref() != Some(&keydown) { - return YES; - } - } else { - window_state_borrow.last_fresh_keydown = Some(keydown); - } - function_is_held = event.keystroke.function; - Some((event, None)) + let keydown = event.keystroke.clone(); + let fn_modifier = keydown.function; + // Ignore events from held-down keys after some of the initially-pressed keys + // were released. + if event.is_held { + if window_state_borrow.last_fresh_keydown.as_ref() != Some(&keydown) { + return YES; } - - _ => return NO, - }; - + } else { + window_state_borrow.last_fresh_keydown = Some(keydown); + } + window_state_borrow.pending_key_down = Some((event, None)); drop(window_state_borrow); - if !function_is_held { + // Send the event to the input context for IME handling, unless the `fn` modifier is + // being pressed. + if !fn_modifier { unsafe { let input_context: id = msg_send![this, inputContext]; let _: BOOL = msg_send![input_context, handleEvent: native_event]; @@ -1143,13 +1137,6 @@ extern "C" fn cancel_operation(this: &Object, _sel: Sel, _sender: id) { } } -extern "C" fn send_event(this: &Object, _: Sel, native_event: id) { - unsafe { - let _: () = msg_send![super(this, class!(NSWindow)), sendEvent: native_event]; - get_window_state(this).borrow_mut().performed_key_equivalent = false; - } -} - extern "C" fn window_did_resize(this: &Object, _: Sel, _: id) { let window_state = unsafe { get_window_state(this) }; window_state.as_ref().borrow().move_traffic_light();