Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix GPUI keyup events not firing on Windows and macOS (#27290)

Browse source 
While building my own application using GPUI, I found that the `key_up`
event doesn't fire on Windows or macOS, with each platform failing for
different reasons. These events aren't used anywhere in Zed yet, so it
makes sense that the issue hasn't already been caught.

I don't have a Linux machine set up right now, so I don't know if these
events fire correctly on Linux or not.

---

Without this fix, a simple layout like the following:

```rust
div()
    .on_key_down(cx.listener(|_, event, _, _| println!("Key down: {:?}", event)))
    .on_key_up(cx.listener(|_, event, _, _| println!("Key up: {:?}", event)));
```

...would result in the following logs if the 'a' key was pressed:

```text
Key down: KeyDownEvent { keystroke: Keystroke { modifiers: Modifiers { control: false, alt: false, shift: false, platform: false, function: false }, key: "a", key_char: Some("a") }, is_held: false }
<eof>
```

With this fix, the `key_up` event fires correctly, resulting in the
following logs:

```text
Key down: KeyDownEvent { keystroke: Keystroke { modifiers: Modifiers { control: false, alt: false, shift: false, platform: false, function: false }, key: "a", key_char: Some("a") }, is_held: false }
Key up: KeyUpEvent { keystroke: Keystroke { modifiers: Modifiers { control: false, alt: false, shift: false, platform: false, function: false }, key: "a", key_char: None } }
<eof>
```

---

I've made the assumption that the `key_char` field shouldn't be set on
the `key_up` event since, unlike the `key_down` event, it's not an event
that may produce a character.

Happy to make any changes to this PR as required. If it would be
preferable to test this on Linux as well before it's merged, let me know
and I'll sort something out.

Hopefully this makes the experience of building new applications on GPUI
smoother, and potentially saves the Zed team some time if this event is
ever used in the future.

Release Notes:

- N/A
This commit is contained in:
Felix Packard 2025-03-28 21:39:15 +00:00 • committed by GitHub
parent 01b400ea29
commit bbd1e628f0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 101 additions and 81 deletions
Download patch file Download diff file Expand all files Collapse all files

174
crates/gpui/src/platform/mac/window.rs
View file

@ -108,6 +108,10 @@ unsafe fn build_classes() {
sel!(keyDown:),
handle_key_down as extern "C" fn(&Object, Sel, id),
);
decl.add_method(
sel!(keyUp:),
handle_key_up as extern "C" fn(&Object, Sel, id),
);
decl.add_method(
sel!(mouseDown:),
handle_view_event as extern "C" fn(&Object, Sel, id),
@ -1219,6 +1223,10 @@ extern "C" fn handle_key_down(this: &Object, _: Sel, native_event: id) {
handle_key_event(this, native_event, false);
}
extern "C" fn handle_key_up(this: &Object, _: Sel, native_event: id) {
handle_key_event(this, native_event, false);
}
// Things to test if you're modifying this method:
// U.S. layout:
// - The IME consumes characters like 'j' and 'k', which makes paging through `less` in
@ -1251,101 +1259,113 @@ extern "C" fn handle_key_event(this: &Object, native_event: id, key_equivalent:
let window_height = lock.content_size().height;
let event = unsafe { PlatformInput::from_native(native_event, Some(window_height)) };
let Some(PlatformInput::KeyDown(mut event)) = event else {
let Some(event) = event else {
return NO;
};
// 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 {
lock.last_key_equivalent = Some(event.clone());
} else if lock.last_key_equivalent.take().as_ref() == Some(&event) {
return NO;
}
drop(lock);
let is_composing = with_input_handler(this, |input_handler| input_handler.marked_text_range())
.flatten()
.is_some();
// If we're composing, send the key to the input handler first;
// otherwise we only send to the input handler if we don't have a matching binding.
// The input handler may call `do_command_by_selector` if it doesn't know how to handle
// a key. If it does so, it will return YES so we won't send the key twice.
// We also do this for non-printing keys (like arrow keys and escape) as the IME menu
// may need them even if there is no marked text;
// however we skip keys with control or the input handler adds control-characters to the buffer.
// and keys with function, as the input handler swallows them.
if is_composing
|| (event.keystroke.key_char.is_none()
&& !event.keystroke.modifiers.control
&& !event.keystroke.modifiers.function)
{
{
let mut lock = window_state.as_ref().lock();
lock.keystroke_for_do_command = Some(event.keystroke.clone());
lock.do_command_handled.take();
drop(lock);
}
let handled: BOOL = unsafe {
let input_context: id = msg_send![this, inputContext];
msg_send![input_context, handleEvent: native_event]
};
window_state.as_ref().lock().keystroke_for_do_command.take();
if let Some(handled) = window_state.as_ref().lock().do_command_handled.take() {
return handled as BOOL;
} else if handled == YES {
return YES;
}
let run_callback = |event: PlatformInput| -> BOOL {
let mut callback = window_state.as_ref().lock().event_callback.take();
let handled: BOOL = if let Some(callback) = callback.as_mut() {
!callback(PlatformInput::KeyDown(event)).propagate as BOOL
!callback(event).propagate as BOOL
} else {
NO
};
window_state.as_ref().lock().event_callback = callback;
return handled as BOOL;
}
let mut callback = window_state.as_ref().lock().event_callback.take();
let handled = if let Some(callback) = callback.as_mut() {
!callback(PlatformInput::KeyDown(event.clone())).propagate as BOOL
} else {
NO
handled
};
window_state.as_ref().lock().event_callback = callback;
if handled == YES {
return YES;
}
if event.is_held {
if let Some(key_char) = event.keystroke.key_char.as_ref() {
let handled = with_input_handler(&this, |input_handler| {
if !input_handler.apple_press_and_hold_enabled() {
input_handler.replace_text_in_range(None, &key_char);
match event {
PlatformInput::KeyDown(mut key_down_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 {
lock.last_key_equivalent = Some(key_down_event.clone());
} else if lock.last_key_equivalent.take().as_ref() == Some(&key_down_event) {
return NO;
}
drop(lock);
let is_composing =
with_input_handler(this, |input_handler| input_handler.marked_text_range())
.flatten()
.is_some();
// If we're composing, send the key to the input handler first;
// otherwise we only send to the input handler if we don't have a matching binding.
// The input handler may call `do_command_by_selector` if it doesn't know how to handle
// a key. If it does so, it will return YES so we won't send the key twice.
// We also do this for non-printing keys (like arrow keys and escape) as the IME menu
// may need them even if there is no marked text;
// however we skip keys with control or the input handler adds control-characters to the buffer.
// and keys with function, as the input handler swallows them.
if is_composing
|| (key_down_event.keystroke.key_char.is_none()
&& !key_down_event.keystroke.modifiers.control
&& !key_down_event.keystroke.modifiers.function)
{
{
let mut lock = window_state.as_ref().lock();
lock.keystroke_for_do_command = Some(key_down_event.keystroke.clone());
lock.do_command_handled.take();
drop(lock);
}
let handled: BOOL = unsafe {
let input_context: id = msg_send![this, inputContext];
msg_send![input_context, handleEvent: native_event]
};
window_state.as_ref().lock().keystroke_for_do_command.take();
if let Some(handled) = window_state.as_ref().lock().do_command_handled.take() {
return handled as BOOL;
} else if handled == YES {
return YES;
}
NO
});
if handled == Some(YES) {
let handled = run_callback(PlatformInput::KeyDown(key_down_event));
return handled;
}
let handled = run_callback(PlatformInput::KeyDown(key_down_event.clone()));
if handled == YES {
return YES;
}
if key_down_event.is_held {
if let Some(key_char) = key_down_event.keystroke.key_char.as_ref() {
let handled = with_input_handler(&this, |input_handler| {
if !input_handler.apple_press_and_hold_enabled() {
input_handler.replace_text_in_range(None, &key_char);
return YES;
}
NO
});
if handled == Some(YES) {
return YES;
}
}
}
// Don't send key equivalents to the input handler,
// or macOS shortcuts like cmd-` will stop working.
if key_equivalent {
return NO;
}
unsafe {
let input_context: id = msg_send![this, inputContext];
msg_send![input_context, handleEvent: native_event]
}
}
}
// Don't send key equivalents to the input handler,
// or macOS shortcuts like cmd-` will stop working.
if key_equivalent {
return NO;
}
PlatformInput::KeyUp(_) => {
drop(lock);
run_callback(event)
}
unsafe {
let input_context: id = msg_send![this, inputContext];
msg_send![input_context, handleEvent: native_event]
_ => NO,
}
}

8
crates/gpui/src/platform/windows/events.rs
View file

@ -361,7 +361,7 @@ fn handle_keydown_msg(
lparam: LPARAM,
state_ptr: Rc<WindowsWindowStatePtr>,
) -> Option<isize> {
let Some(keystroke_or_modifier) = parse_keydown_msg_keystroke(wparam) else {
let Some(keystroke_or_modifier) = parse_keystroke_from_vkey(wparam, false) else {
return Some(1);
};
let mut lock = state_ptr.state.borrow_mut();
@ -391,7 +391,7 @@ fn handle_keydown_msg(
}
fn handle_keyup_msg(wparam: WPARAM, state_ptr: Rc<WindowsWindowStatePtr>) -> Option<isize> {
let Some(keystroke_or_modifier) = parse_keydown_msg_keystroke(wparam) else {
let Some(keystroke_or_modifier) = parse_keystroke_from_vkey(wparam, true) else {
return Some(1);
};
let mut lock = state_ptr.state.borrow_mut();
@ -1288,7 +1288,7 @@ enum KeystrokeOrModifier {
Modifier(Modifiers),
}
fn parse_keydown_msg_keystroke(wparam: WPARAM) -> Option<KeystrokeOrModifier> {
fn parse_keystroke_from_vkey(wparam: WPARAM, is_keyup: bool) -> Option<KeystrokeOrModifier> {
let vk_code = wparam.loword();
let modifiers = current_modifiers();
@ -1316,7 +1316,7 @@ fn parse_keydown_msg_keystroke(wparam: WPARAM) -> Option<KeystrokeOrModifier> {
return Some(KeystrokeOrModifier::Modifier(modifiers));
}
if modifiers.control || modifiers.alt {
if modifiers.control || modifiers.alt || is_keyup {
let basic_key = basic_vkcode_to_string(vk_code, modifiers);
if let Some(basic_key) = basic_key {
return Some(KeystrokeOrModifier::Keystroke(basic_key));