Skip to content

Commit 9017e89

Browse files
committed
appkit: route IME key events through NSTextInputContext::handleEvent.
1 parent 1785b7b commit 9017e89

File tree

2 files changed

+202
-21
lines changed

2 files changed

+202
-21
lines changed

winit-appkit/src/view.rs

Lines changed: 199 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
use std::cell::{Cell, RefCell};
33
use std::collections::{HashMap, VecDeque};
44
use std::ptr;
5-
use std::rc::Rc;
5+
use std::rc::{Rc, Weak};
66

77
use dpi::{LogicalPosition, LogicalSize};
88
use objc2::rc::Retained;
@@ -17,6 +17,7 @@ use objc2_foundation::{
1717
NSArray, NSAttributedString, NSAttributedStringKey, NSCopying, NSMutableAttributedString,
1818
NSNotFound, NSObject, NSPoint, NSRange, NSRect, NSSize, NSString, NSUInteger,
1919
};
20+
use smol_str::SmolStr;
2021
use winit_core::event::{
2122
DeviceEvent, ElementState, Ime, KeyEvent, Modifiers, MouseButton, MouseScrollDelta,
2223
PointerKind, PointerSource, TouchPhase, WindowEvent,
@@ -45,6 +46,37 @@ impl Default for CursorState {
4546
}
4647
}
4748

49+
/// A per-queued raw-character gate used to drop stale raw KeyboardInput events.
50+
/// Each queued raw character captures an Rc<EventFilterToken> inside the runloop-dispatched closure;
51+
/// when an IME Commit for the same key event arrives, we flip `deliver = false` so the closure becomes a no-op.
52+
#[derive(Debug)]
53+
struct EventFilterToken {
54+
/// Whether this queued raw KeyboardInput should be delivered to the app.
55+
/// Set to `false` by `drop_conflicting_raw_characters` when an IME commit supersedes it.
56+
deliver: Cell<bool>,
57+
}
58+
59+
impl EventFilterToken {
60+
fn new() -> Self {
61+
Self { deliver: Cell::new(true) }
62+
}
63+
}
64+
65+
/// Bookkeeping for a raw-character KeyboardInput that was scheduled for dispatch.
66+
/// - `serial`: monotonically increasing key-event serial so we can match it against an IME-handled NSEvent
67+
/// in the same runloop tick.
68+
/// - `text`: the raw character payload (e.g. ".") so we can compare with a subsequent IME commit (e.g. "。").
69+
/// - `token`: Weak reference allowing the IME path to cancel delivery without keeping the event alive.
70+
#[derive(Debug)]
71+
struct PendingRawCharacter {
72+
/// Serial of the key event that produced this raw character.
73+
serial: u64,
74+
/// Raw character text captured from `KeyEvent.text`.
75+
text: SmolStr,
76+
/// Weak handle to the gate used by the dispatch closure.
77+
token: Weak<EventFilterToken>,
78+
}
79+
4880
#[derive(Debug, Eq, PartialEq, Clone, Copy, Default)]
4981
enum ImeState {
5082
#[default]
@@ -135,6 +167,13 @@ pub struct ViewState {
135167
marked_text: RefCell<Retained<NSMutableAttributedString>>,
136168
accepts_first_mouse: bool,
137169

170+
/// Monotonic counter incremented per keyDown/keyUp; groups raw text and IME handling within the same runloop turn.
171+
current_event_serial: Cell<u64>,
172+
/// Serial of the last `NSEvent` that was handed to `NSTextInputContext::handleEvent`.
173+
last_handled_event_serial: Cell<u64>,
174+
/// Raw-character events queued for delivery; used to drop them if an IME Commit disagrees.
175+
pending_raw_characters: RefCell<Vec<PendingRawCharacter>>,
176+
138177
/// The state of the `Option` as `Alt`.
139178
option_as_alt: Cell<OptionAsAlt>,
140179
}
@@ -395,6 +434,8 @@ define_class!(
395434

396435
// Commit only if we have marked text.
397436
if self.hasMarkedText() && self.is_ime_enabled() && !is_control {
437+
// Safety net: if a raw ReceivedCharacter from this tick exists and differs (e.g. '.' vs '。'), drop it.
438+
self.drop_conflicting_raw_characters(&string);
398439
self.queue_event(WindowEvent::Ime(Ime::Preedit(String::new(), None)));
399440
self.queue_event(WindowEvent::Ime(Ime::Commit(string)));
400441
self.ivars().ime_state.set(ImeState::Committed);
@@ -445,6 +486,8 @@ define_class!(
445486
#[unsafe(method(keyDown:))]
446487
fn key_down(&self, event: &NSEvent) {
447488
trace_scope!("keyDown:");
489+
self.begin_key_event();
490+
let mut ime_consumed_event = false;
448491
{
449492
let mut prev_input_source = self.ivars().input_source.borrow_mut();
450493
let current_input_source = self.current_input_source();
@@ -468,8 +511,13 @@ define_class!(
468511
// `doCommandBySelector`. (doCommandBySelector means that the keyboard input
469512
// is not handled by IME and should be handled by the application)
470513
if self.ivars().ime_capabilities.get().is_some() {
471-
let events_for_nsview = NSArray::from_slice(&[&*event]);
472-
self.interpretKeyEvents(&events_for_nsview);
514+
// Route the event through `NSTextInputContext` first; this calls into `IMKInputController`.
515+
ime_consumed_event = self.handle_text_input_event(&event);
516+
if !ime_consumed_event {
517+
// If the IME didn't take it, fall back to Cocoa's `interpretKeyEvents` to produce `insertText:`/`doCommandBySelector:`.
518+
let events_for_nsview = NSArray::from_slice(&[&*event]);
519+
self.interpretKeyEvents(&events_for_nsview);
520+
}
473521

474522
// If the text was committed we must treat the next keyboard event as IME related.
475523
if self.ivars().ime_state.get() == ImeState::Committed {
@@ -491,30 +539,34 @@ define_class!(
491539
_ => old_ime_state != self.ivars().ime_state.get(),
492540
};
493541

494-
if !had_ime_input || self.ivars().forward_key_to_app.get() {
542+
// Only send a raw KeyboardInput if IME did not produce preedit/commit and did not consume the event.
543+
if self.ivars().forward_key_to_app.get() || (!had_ime_input && !ime_consumed_event) {
495544
let key_event = create_key_event(&event, true, event.isARepeat());
496-
self.queue_event(WindowEvent::KeyboardInput {
497-
device_id: None,
498-
event: key_event,
499-
is_synthetic: false,
500-
});
545+
self.queue_keyboard_input_event(key_event, false);
501546
}
502547
}
503548

504549
#[unsafe(method(keyUp:))]
505550
fn key_up(&self, event: &NSEvent) {
506551
trace_scope!("keyUp:");
552+
self.begin_key_event();
553+
let mut ime_consumed_event = false;
507554

508555
let event = replace_event(event, self.option_as_alt());
509556
self.update_modifiers(&event, false);
510557

558+
if self.ivars().ime_capabilities.get().is_some() {
559+
// Let IME observe keyUp too; some IMEs compare keyDown/keyUp (e.g. Shift single-tap detection).
560+
ime_consumed_event = self.handle_text_input_event(&event);
561+
}
562+
511563
// We want to send keyboard input when we are currently in the ground state.
512-
if matches!(self.ivars().ime_state.get(), ImeState::Ground | ImeState::Disabled) {
513-
self.queue_event(WindowEvent::KeyboardInput {
514-
device_id: None,
515-
event: create_key_event(&event, false, false),
516-
is_synthetic: false,
517-
});
564+
// If IME is idle/disabled and didn't consume keyUp, forward it to the app.
565+
if matches!(self.ivars().ime_state.get(), ImeState::Ground | ImeState::Disabled)
566+
&& !ime_consumed_event
567+
{
568+
let key_event = create_key_event(&event, false, false);
569+
self.queue_keyboard_input_event(key_event, false);
518570
}
519571
}
520572

@@ -561,11 +613,7 @@ define_class!(
561613
self.update_modifiers(&event, false);
562614
let event = create_key_event(&event, true, event.isARepeat());
563615

564-
self.queue_event(WindowEvent::KeyboardInput {
565-
device_id: None,
566-
event,
567-
is_synthetic: false,
568-
});
616+
self.queue_keyboard_input_event(event, false);
569617
}
570618

571619
// In the past (?), `mouseMoved:` events were not generated when the
@@ -812,6 +860,9 @@ impl WinitView {
812860
forward_key_to_app: Default::default(),
813861
marked_text: Default::default(),
814862
accepts_first_mouse,
863+
current_event_serial: Cell::new(0),
864+
last_handled_event_serial: Cell::new(0),
865+
pending_raw_characters: RefCell::new(Vec::new()),
815866
option_as_alt: Cell::new(option_as_alt),
816867
});
817868
let this: Retained<Self> = unsafe { msg_send![super(this), init] };
@@ -832,6 +883,127 @@ impl WinitView {
832883
});
833884
}
834885

886+
/// Queue a KeyboardInput for delivery, with an option to drop it later if an IME commit supersedes it.
887+
///
888+
/// Rationale: when an IME is active, macOS can generate both a raw character (e.g. '.') and an IME commit
889+
/// (e.g. '。') for the same physical key press. We tentatively enqueue the raw event, but guard it with a
890+
/// token so `drop_conflicting_raw_characters` can cancel delivery in the same runloop turn.
891+
fn queue_keyboard_input_event(&self, key_event: KeyEvent, is_synthetic: bool) {
892+
// Trim any stale entries whose tokens have already been dropped by dispatched closures.
893+
self.cleanup_pending_raw_characters();
894+
895+
// Associate this event with the current key event serial.
896+
let serial = self.ivars().current_event_serial.get();
897+
// Only character-bearing events participate in the safety net; non-text key events bypass the filter.
898+
let token = key_event.text.as_ref().map(|text| {
899+
let token = Rc::new(EventFilterToken::new());
900+
self.ivars().pending_raw_characters.borrow_mut().push(PendingRawCharacter {
901+
serial,
902+
text: text.clone(),
903+
token: Rc::downgrade(&token),
904+
});
905+
token
906+
});
907+
908+
let window_event =
909+
WindowEvent::KeyboardInput { device_id: None, event: key_event, is_synthetic };
910+
let window_id = window_id(&self.window());
911+
912+
if let Some(token) = token {
913+
// Defer dispatch and drop the event if IME said to supersede it.
914+
let event_to_dispatch = window_event.clone();
915+
self.ivars().app_state.maybe_queue_with_handler(move |app, event_loop| {
916+
// The IME path may have flipped `deliver` to false.
917+
if !token.deliver.get() {
918+
return;
919+
}
920+
app.window_event(event_loop, window_id, event_to_dispatch.clone());
921+
});
922+
} else {
923+
// No text payload: dispatch as-is.
924+
let event_to_dispatch = window_event;
925+
self.ivars().app_state.maybe_queue_with_handler(move |app, event_loop| {
926+
app.window_event(event_loop, window_id, event_to_dispatch.clone());
927+
});
928+
}
929+
}
930+
931+
/// Start a new serial for the current keyDown/keyUp pair.
932+
///
933+
/// We use this to correlate raw-character events with IME handling within the same runloop tick.
934+
fn begin_key_event(&self) {
935+
let next = self.ivars().current_event_serial.get().wrapping_add(1);
936+
self.ivars().current_event_serial.set(next);
937+
}
938+
939+
/// Let IME observe the native NSEvent via `NSTextInputContext::handleEvent` and record the serial.
940+
///
941+
/// Returns true when the IME consumed the event; in that case we should suppress raw character delivery.
942+
fn handle_text_input_event(&self, event: &NSEvent) -> bool {
943+
let Some(input_context) = self.inputContext() else {
944+
return false;
945+
};
946+
947+
// Record which serial was seen by the IME so `drop_conflicting_raw_characters` knows what to cancel.
948+
let serial = self.ivars().current_event_serial.get();
949+
self.ivars().last_handled_event_serial.set(serial);
950+
951+
input_context.handleEvent(event)
952+
}
953+
954+
/// Drop the most relevant queued raw-character event if its text disagrees with the IME commit.
955+
///
956+
/// Strategy:
957+
/// - Prefer the raw character queued in the same serial (same key event) as the IME-handled NSEvent.
958+
/// - If none is found (ordering nuances), fall back to the newest still-alive raw character.
959+
/// - If its text != `commit`, flip its token to prevent delivery.
960+
fn drop_conflicting_raw_characters(&self, commit: &str) {
961+
let serial = self.ivars().last_handled_event_serial.get();
962+
let mut pending = self.ivars().pending_raw_characters.borrow_mut();
963+
964+
let mut target: Option<(Weak<EventFilterToken>, SmolStr)> = None;
965+
966+
// Search from newest to oldest to find a match in the same serial.
967+
for entry in pending.iter().rev() {
968+
if entry.token.upgrade().is_none() {
969+
continue;
970+
}
971+
972+
if entry.serial == serial {
973+
target = Some((entry.token.clone(), entry.text.clone()));
974+
break;
975+
}
976+
}
977+
978+
// If we didn't find one in the same serial, take the newest alive entry.
979+
if target.is_none() {
980+
if let Some(entry) = pending.iter().rev().find(|entry| entry.token.upgrade().is_some())
981+
{
982+
target = Some((entry.token.clone(), entry.text.clone()));
983+
}
984+
}
985+
986+
if let Some((token, text)) = target {
987+
if text.as_str() != commit {
988+
if let Some(token) = token.upgrade() {
989+
// Cancel delivery of the stale raw character.
990+
token.deliver.set(false);
991+
}
992+
}
993+
}
994+
995+
// GC: keep only entries whose tokens are still alive.
996+
pending.retain(|entry| entry.token.upgrade().is_some());
997+
}
998+
999+
/// Remove bookkeeping entries whose dispatch tokens have already been dropped.
1000+
fn cleanup_pending_raw_characters(&self) {
1001+
self.ivars()
1002+
.pending_raw_characters
1003+
.borrow_mut()
1004+
.retain(|entry| entry.token.upgrade().is_some());
1005+
}
1006+
8351007
fn scale_factor(&self) -> f64 {
8361008
self.window().backingScaleFactor() as f64
8371009
}
@@ -1030,7 +1202,13 @@ impl WinitView {
10301202
drop(phys_mod_state);
10311203

10321204
for event in events {
1033-
self.queue_event(event);
1205+
match event {
1206+
// Route synthesized modifier presses through the same filtering path to honor IME safety net.
1207+
WindowEvent::KeyboardInput { event: key_event, is_synthetic, .. } => {
1208+
self.queue_keyboard_input_event(key_event, is_synthetic);
1209+
},
1210+
other => self.queue_event(other),
1211+
}
10341212
}
10351213
}
10361214
}

winit/src/changelog/unreleased.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,9 @@ changelog entry.
276276
- On macOS, fixed redundant `SurfaceResized` event at window creation.
277277
- On macOS, don't panic on monitors with unknown bit-depths.
278278
- On macOS, fixed crash when closing the window on macOS 26+.
279+
- On macOS, route IME key events through `NSTextInputContext::handleEvent` to avoid duplicate
280+
punctuation and dead key commits produced by third-party input methods, aligning character
281+
delivery with Cocoa.
279282
- On macOS, make AppKit monitor handle tests derive their reference display IDs from
280283
`CGMainDisplayID`, preventing failures on systems where the primary monitor is not display `1`.
281284
- On Windows, account for mouse wheel lines per scroll setting for `WindowEvent::MouseWheel`.

0 commit comments

Comments
 (0)