diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index 234a9989b..90f0311d5 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -9,6 +9,9 @@ #![expect(clippy::manual_range_contains)] +#[cfg(target_os = "windows")] +use std::collections::HashSet; + #[cfg(feature = "accesskit")] pub use accesskit_winit; pub use egui; @@ -106,6 +109,12 @@ pub struct State { allow_ime: bool, ime_rect_px: Option, + + /// Used by [`State::try_on_ime_processed_keyboard_input`] to track key + /// release events that should be filtered out. See comments in that method + /// for details. + #[cfg(target_os = "windows")] + pressed_processed_physical_keys: HashSet, } impl State { @@ -148,6 +157,8 @@ impl State { allow_ime: false, ime_rect_px: None, + #[cfg(target_os = "windows")] + pressed_processed_physical_keys: HashSet::new(), }; slf.egui_input @@ -364,25 +375,33 @@ impl State { is_synthetic, .. } => { - // Winit generates fake "synthetic" KeyboardInput events when the focus - // is changed to the window, or away from it. Synthetic key presses - // represent no real key presses and should be ignored. - // See https://github.com/rust-windowing/winit/issues/3543 if *is_synthetic && event.state == ElementState::Pressed { + // Winit generates fake "synthetic" KeyboardInput events when the focus + // is changed to the window, or away from it. Synthetic key presses + // represent no real key presses and should be ignored. + // See https://github.com/rust-windowing/winit/issues/3543 EventResponse { repaint: true, consumed: false, } } else { - self.on_keyboard_input(event); + let egui_wants_keyboard_input = self.egui_ctx.egui_wants_keyboard_input(); - // When pressing the Tab key, egui focuses the first focusable element, hence Tab always consumes. - let consumed = self.egui_ctx.egui_wants_keyboard_input() - || event.logical_key - == winit::keyboard::Key::Named(winit::keyboard::NamedKey::Tab); - EventResponse { - repaint: true, - consumed, + if let Some(response) = + self.try_on_ime_processed_keyboard_input(event, egui_wants_keyboard_input) + { + response + } else { + self.on_keyboard_input(event); + + // When pressing the Tab key, egui focuses the first focusable element, hence Tab always consumes. + let consumed = egui_wants_keyboard_input + || event.logical_key + == winit::keyboard::Key::Named(winit::keyboard::NamedKey::Tab); + EventResponse { + repaint: true, + consumed, + } } } } @@ -526,6 +545,91 @@ impl State { } } + #[cfg(not(target_os = "windows"))] + #[expect(clippy::unused_self, clippy::needless_pass_by_ref_mut)] + #[inline(always)] + fn try_on_ime_processed_keyboard_input( + &mut self, + _event: &winit::event::KeyEvent, + _egui_wants_keyboard_input: bool, + ) -> Option { + // `KeyboardInput` events processed by the IME are not emitted by + // `winit` on non-Windows platforms, so we don't need to do anything + // here. + + None + } + + #[cfg(target_os = "windows")] + #[inline(always)] + fn try_on_ime_processed_keyboard_input( + &mut self, + event: &winit::event::KeyEvent, + egui_wants_keyboard_input: bool, + ) -> Option { + if !self.allow_ime { + None + } else if event.logical_key == winit::keyboard::NamedKey::Process { + // On Windows, the current version of `winit` (0.30.12) has a bug + // where `KeyboardInput` events processed by the IME are still + // emitted. [^1] + // + // As a workaround, we detect these events by checking whether their + // `logical_key` is `winit::keyboard::NamedKey::Process`, and filter + // them out to keep behavior consistent with other platforms. + // + // `winit::keyboard::NamedKey::Process` is not documented in + // `winit`. Reading through its source code, we find that it is + // mapped from `VK_PROCESSKEY` on Windows [^2]. (On an unrelated + // note, Web is the only other platform that also uses it [^3].) + // According to Microsoft, “the IME sets the virtual key value + // to `VK_PROCESSKEY` after processing a key input message” [^4]. + // See also [^5]. + // (I can't find a documentation page dedicated to this value.) + // + // TODO(umajho): Remove this workaround once the `winit` bug is fixed + // and we've updated to a version that includes the fix. NOTE: Don't + // forget to also remove the `pressed_processed_physical_keys` field + // and its related code. + // + // [^1]: https://github.com/rust-windowing/winit/issues/4508 + // [^2]: https://github.com/rust-windowing/winit/blob/e9809ef54b18499bb4f2cac945719ecc2a61061b/src/platform_impl/windows/keyboard_layout.rs#L946 + // [^3]: https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_key_values + // [^4]: https://learn.microsoft.com/en-us/windows/win32/api/imm/nf-imm-immgetvirtualkey#remarks + // [^5]: https://learn.microsoft.com/en-us/windows/win32/learnwin32/keyboard-input#character-messages + + self.pressed_processed_physical_keys + .insert(event.physical_key); + + Some(EventResponse { + repaint: false, + consumed: egui_wants_keyboard_input, + }) + } else if event.state == ElementState::Released + && self + .pressed_processed_physical_keys + .remove(&event.physical_key) + { + // Unlike key-presses, we can not tell whether a key-release event + // is processed by the IME or not by looking at its `logical_key`, + // because their `logical_key` is the original value (e.g. + // `winit::keyboard::Key::Character(…)`) rather than + // `winit::keyboard::Key::Named(winit::keyboard::NamedKey::Process)`. + // (See the screencast for Windows in [^1].) + // So we track the physical keys of processed key-presses and + // filter out the corresponding key-releases. + // + // [^1]: https://github.com/rust-windowing/winit/issues/4508 + + Some(EventResponse { + repaint: false, + consumed: egui_wants_keyboard_input, + }) + } else { + None + } + } + /// ## NOTE /// /// on Mac even Cmd-C is pressed during ime, a `c` is pushed to Preedit. @@ -1001,6 +1105,16 @@ impl State { let allow_ime = ime.is_some(); if self.allow_ime != allow_ime { self.allow_ime = allow_ime; + #[cfg(target_os = "windows")] + if !self.allow_ime { + // Defensively clear the set to avoid unexpected behavior. + // + // We don't do the same in `ime_event_disable` because the key + // release events for IME confirmation keys arrive after + // `winit::event::Ime::Disabled`. + self.pressed_processed_physical_keys.clear(); + } + profiling::scope!("set_ime_allowed"); window.set_ime_allowed(allow_ime); } diff --git a/crates/egui/src/data/input.rs b/crates/egui/src/data/input.rs index a52d40233..5e1680334 100644 --- a/crates/egui/src/data/input.rs +++ b/crates/egui/src/data/input.rs @@ -441,6 +441,10 @@ pub enum Event { Text(String), /// A key was pressed or released. + /// + /// ## Note for integration authors + /// + /// Key events that has been processed by IMEs should not be sent to `egui`. Key { /// Most of the time, it's the logical key, heeding the active keymap -- for instance, if the user has Dvorak /// keyboard layout, it will be taken into account. diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index a6c41e71c..1f103d2f8 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -993,13 +993,7 @@ fn events( let mut any_change = false; - let mut events = ui.input(|i| i.filtered_events(&event_filter)); - - if state.ime_enabled { - remove_ime_incompatible_events(&mut events); - // Process IME events first: - events.sort_by_key(|e| !matches!(e, Event::Ime(_))); - } + let events = ui.input(|i| i.filtered_events(&event_filter)); for event in &events { let did_mutate_text = match event { @@ -1232,27 +1226,6 @@ fn events( // ---------------------------------------------------------------------------- -fn remove_ime_incompatible_events(events: &mut Vec) { - // Remove key events which cause problems while 'IME' is being used. - // See https://github.com/emilk/egui/pull/4509 - events.retain(|event| { - !matches!( - event, - Event::Key { repeat: true, .. } - | Event::Key { - key: Key::Backspace - | Key::ArrowUp - | Key::ArrowDown - | Key::ArrowLeft - | Key::ArrowRight, - .. - } - ) - }); -} - -// ---------------------------------------------------------------------------- - /// Returns `Some(new_cursor)` if we did mutate `text`. fn check_for_mutating_key_press( os: OperatingSystem,