From 33e89e33bec3e0009096323f0832e73265c29e4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uma=C4=B5o?= <107099960+umajho@users.noreply.github.com> Date: Mon, 6 Apr 2026 16:24:50 +0800 Subject: [PATCH] Improve IME handling, add public method `owns_ime_events` on `Memory` (#7983) * Depends on #7967 * Closes #7485 * Should fix #7906 (This issue doesn't seem to have been resolved, but the author closed it; I personally don't have the environment to verify whether it is fixed.) * Replaces #4137, #4896, and partially #7810 * [x] I have followed the instructions in the PR template This PR started as a fix for #7485, but has since evolved into a broader rewrite of IME-related logic. ## Overview This PR primarily introduces a new public method, `owns_ime_events`, on [`Memory`], and refactors parts of [`TextEdit`] to integrate with it. Previously, each [`TextEdit`] widget independently determined whether to handle IME events and stored its own IME-related state. This approach made ownership-handling fragmented and was therefore error-prone. With this PR: - IME event ownership is centralized, ensuring that at most a single widget owns IME events per frame. - [`PlatformOutput`]'s `ime` field can be set to `None` for at least one frame when IME composition is interrupted, allowing the IME to be properly dismissed. ## Details Two new public methods are introduced on [`Memory`]: - `fn owns_ime_events(&self, id: Id) -> bool`: check IME event ownership for the current frame for the widget with the given `id`. - `fn interrupt_ime(&mut self)`: interrupt the current IME composition, if any. Since the newly added methods on [`Memory`] are public, other widgets can also participate in IME handling without risking ownership conflicts of IME events. I also added an internal (`pub(crate)`) field on [`TextEditState`], called `cursor_purpose`, to distinguish the role of the [`TextEdit`] cursor. Additionally, `egui::ImeEvent::Enabled` and `egui::ImeEvent::Disabled` have been removed, as they are no longer used anywhere. ## Demonstrations ### Windows: The Korean IME text duplication bug fixed in #4137 does not reappear.
With this PR Without this PR
Behavior Correct (no regression) Correct
Screencast ![win-kor-after](https://github.com/user-attachments/assets/1b080c8f-2031-406f-8781-aacafd5c879a) ![win-kor-before](https://github.com/user-attachments/assets/20258841-72fe-4652-b9a9-9b40e338ccf2)
### Windows: Chinese and Japanese IMEs now behave more consistently with the Korean IME in similar scenarios. This change does not matter much, as composition is rarely interrupted mid-process with these IMEs in typical usage.
With this PR Without this PR
Behavior Composition can be interrupted by clicking (like Korean IMEs) Composition can not interrupted by clicking
Screencast (Builtin Chinese IME) ![win-cmn-after](https://github.com/user-attachments/assets/2c76b0a9-da6d-48e1-84e0-47d9631f1196) ![win-cmn-before](https://github.com/user-attachments/assets/ea125fb8-c325-48d5-abaf-17d495b8f075)
Screencast (Builtin Japanese IME) ![win-jpn-after](https://github.com/user-attachments/assets/c69e5f48-65b1-4c0f-af4a-522d2f47b75d) ![win-jpn-before](https://github.com/user-attachments/assets/a0f1fdad-4f6c-40c2-af57-029f42acf6d5)
### macOS: was buggy, still buggy Likely due to this upstream bug in `winit`: https://github.com/rust-windowing/winit/issues/4432 Once `winit` is updated to a version that includes the fix, the behavior should become correct with this PR.
With this PR Without this PR
Behavior Buggy as before Buggy: Characters are duplicated
Screencast ![mac-kor-after](https://github.com/user-attachments/assets/c2bd90e8-e473-49c8-9537-c970c92889bf) ![mac-kor-before](https://github.com/user-attachments/assets/63b6cd8a-8903-4743-98bf-ee15296354ba)
### Wayland + iBus: Korean IME duplication bug fixed
With this PR Without this PR
Behavior Correct Buggy: Characters are duplicated
Screencast ![wayland-kor-after-2](https://github.com/user-attachments/assets/b154add5-a1ce-4e3a-b243-e72480820c1b) ![wayland-kor-before-2](https://github.com/user-attachments/assets/43b28374-f273-4b6f-9845-3efd96ec9a37)
### Wayland + iBus: #7485 is fixed
With this PR Without this PR
Behavior Correct Buggy: Only a single ASCII character can be typed after TextEdit is focused
Screencast ![wayland-7485-after](https://github.com/user-attachments/assets/ec33a54d-1d4e-40f9-8c82-202104bd2d85) ![wayland-7485-before](https://github.com/user-attachments/assets/20d2d395-03fd-4966-a376-87249a41aab3)
### Wayland + iBus: selection is also not broken This PR does not reintroduce the selection bug fixed in #7973.
With this PR
Behavior Correct
Screencast ![wayland-focus-after](https://github.com/user-attachments/assets/daa29197-f7f7-4a7b-b454-c28ee9afa9c1)
### X11 + Fcitx5: IME composition can be interrupted But due to #7975, the experience is still subpar. (Uncommitted text is lost after interruption.)
With this PR Without this PR
Screencast ![x11-after](https://github.com/user-attachments/assets/e626d9ed-89a2-4825-9cde-3a67723bcb82) ![x11-before](https://github.com/user-attachments/assets/da93b351-9488-4da9-aa56-b64190e84ec3)
[`Memory`]: https://docs.rs/egui/latest/egui/struct.Memory.html [`TextEdit`]: https://docs.rs/egui/latest/egui/widgets/text_edit/struct.TextEdit.html [`PlatformOutput`]: https://docs.rs/egui/latest/egui/struct.PlatformOutput.html [`TextEditState`]: https://docs.rs/egui/latest/egui/widgets/text_edit/struct.TextEditState.html --- crates/eframe/src/web/text_agent.rs | 4 - crates/egui-winit/src/lib.rs | 64 +--------- crates/egui/src/data/input.rs | 7 ++ crates/egui/src/data/output.rs | 3 + crates/egui/src/memory/mod.rs | 60 ++++++++++ crates/egui/src/widgets/text_edit/builder.rs | 120 ++++++++++--------- crates/egui/src/widgets/text_edit/state.rs | 22 ++-- 7 files changed, 154 insertions(+), 126 deletions(-) diff --git a/crates/eframe/src/web/text_agent.rs b/crates/eframe/src/web/text_agent.rs index ac917329f..e3d4f8860 100644 --- a/crates/eframe/src/web/text_agent.rs +++ b/crates/eframe/src/web/text_agent.rs @@ -75,11 +75,7 @@ impl TextAgent { }; let on_composition_start = { - let input = input.clone(); move |_: web_sys::CompositionEvent, runner: &mut AppRunner| { - input.set_value(""); - let event = egui::Event::Ime(egui::ImeEvent::Enabled); - runner.input.raw.events.push(event); // Repaint moves the text agent into place, // see `move_to` in `AppRunner::handle_platform_output`. runner.needs_repaint.repaint_asap(); diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index 90f0311d5..99a9894f3 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -101,9 +101,6 @@ pub struct State { /// Only one touch will be interpreted as pointer at any time. pointer_touch_id: Option, - /// track ime state - has_sent_ime_enabled: bool, - #[cfg(feature = "accesskit")] pub accesskit: Option, @@ -150,8 +147,6 @@ impl State { simulate_touch_screen: false, pointer_touch_id: None, - has_sent_ime_enabled: false, - #[cfg(feature = "accesskit")] accesskit: None, @@ -689,17 +684,11 @@ impl State { // } match ime { - winit::event::Ime::Enabled => { - if cfg!(target_os = "linux") { - // This event means different things in X11 and Wayland, but we can just - // ignore it and enable IME on the preedit event. - // See - } else { - self.ime_event_enable(); - } - } - winit::event::Ime::Preedit(text, Some(_cursor)) => { - self.ime_event_enable(); + // [`winit::event::Ime::Enabled`] means different things in X11 and + // Wayland, but it doesn't matter to us. + // See + winit::event::Ime::Enabled | winit::event::Ime::Disabled => {} + winit::event::Ime::Preedit(text, _) => { self.egui_input .events .push(egui::Event::Ime(egui::ImeEvent::Preedit(text.clone()))); @@ -708,53 +697,10 @@ impl State { self.egui_input .events .push(egui::Event::Ime(egui::ImeEvent::Commit(text.clone()))); - self.ime_event_disable(); - } - winit::event::Ime::Disabled => { - self.ime_event_disable(); - } - winit::event::Ime::Preedit(_, None) => { - if cfg!(target_os = "macos") { - // On macOS, when the user presses backspace to delete the - // last character in an IME composition, `winit` only emits - // `winit::event::Ime::Preedit("", None)` without a - // preceding `winit::event::Ime::Preedit("", Some(0, 0))`. - // - // The current implementation of `egui::TextEdit` relies on - // receiving an `egui::ImeEvent::Preedit("")` to remove the - // last character in the composition in this case, so we - // emit it here. - // - // This is guarded to macOS-only, as applying it on other - // platforms is unnecessary and can cause undesired - // behavior. - // See: https://github.com/emilk/egui/pull/7973 - self.egui_input - .events - .push(egui::Event::Ime(egui::ImeEvent::Preedit(String::new()))); - } - - self.ime_event_disable(); } } } - pub fn ime_event_enable(&mut self) { - if !self.has_sent_ime_enabled { - self.egui_input - .events - .push(egui::Event::Ime(egui::ImeEvent::Enabled)); - self.has_sent_ime_enabled = true; - } - } - - pub fn ime_event_disable(&mut self) { - self.egui_input - .events - .push(egui::Event::Ime(egui::ImeEvent::Disabled)); - self.has_sent_ime_enabled = false; - } - /// Returns `true` if the event was sent to egui. pub fn on_mouse_motion(&mut self, delta: (f64, f64)) -> bool { if !self.is_pointer_in_window() && !self.any_pointer_button_down { diff --git a/crates/egui/src/data/input.rs b/crates/egui/src/data/input.rs index 00cf59cba..7a104a95e 100644 --- a/crates/egui/src/data/input.rs +++ b/crates/egui/src/data/input.rs @@ -605,15 +605,22 @@ pub enum Event { #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub enum ImeEvent { /// Notifies when the IME was enabled. + #[deprecated = "No longer used by egui"] Enabled, /// A new IME candidate is being suggested. + /// + /// An empty preedit string indicates that the IME has been dismissed, while + /// a non-empty preedit string indicates that the IME is active. Preedit(String), /// IME composition ended with this final result. + /// + /// The IME is considered dismissed after this event. Commit(String), /// Notifies when the IME was disabled. + #[deprecated = "No longer used by egui"] Disabled, } diff --git a/crates/egui/src/data/output.rs b/crates/egui/src/data/output.rs index 2d2c74430..ea3ff4eec 100644 --- a/crates/egui/src/data/output.rs +++ b/crates/egui/src/data/output.rs @@ -123,6 +123,9 @@ pub struct PlatformOutput { /// This is set if, and only if, the user is currently editing text. /// /// Useful for IME. + /// + /// This field should only be set by the widget that currently owns IME + /// events (see [`crate::Memory::owns_ime_events`]). pub ime: Option, /// The difference in the widget tree since last frame. diff --git a/crates/egui/src/memory/mod.rs b/crates/egui/src/memory/mod.rs index 08b08a462..34e0fe319 100644 --- a/crates/egui/src/memory/mod.rs +++ b/crates/egui/src/memory/mod.rs @@ -116,6 +116,22 @@ pub struct Memory { /// (e.g. relative to some other widget). #[cfg_attr(feature = "persistence", serde(skip))] popups: ViewportIdMap, + + /// When the last IME interruption was made. + #[cfg_attr(feature = "persistence", serde(skip))] + ime_interruption_time: ImeInterruptionTime, +} + +#[derive(Clone, Copy, Debug, Default)] +enum ImeInterruptionTime { + #[default] + None, + + /// The IME was interrupted in the current frame. + ThisFrame, + + /// The IME was interrupted in the previous frame. + LastFrame, } impl Default for Memory { @@ -133,6 +149,7 @@ impl Default for Memory { popups: Default::default(), everything_is_visible: Default::default(), add_fonts: Default::default(), + ime_interruption_time: Default::default(), }; slf.interactions.entry(slf.viewport_id).or_default(); slf.areas.entry(slf.viewport_id).or_default(); @@ -761,6 +778,16 @@ impl Memory { self.areas.entry(self.viewport_id).or_default(); + match self.ime_interruption_time { + ImeInterruptionTime::ThisFrame => { + self.ime_interruption_time = ImeInterruptionTime::LastFrame; + } + ImeInterruptionTime::LastFrame => { + self.ime_interruption_time = ImeInterruptionTime::None; + } + ImeInterruptionTime::None => {} + } + // self.interactions is handled elsewhere self.options.begin_pass(new_raw_input); @@ -875,9 +902,12 @@ impl Memory { /// Give keyboard focus to a specific widget. /// See also [`crate::Response::request_focus`]. + /// + /// Calling this will interrupt IME composition. #[inline(always)] pub fn request_focus(&mut self, id: Id) { self.focus_mut().focused_widget = Some(FocusWidget::new(id)); + self.interrupt_ime(); } /// Surrender keyboard focus for a specific widget. @@ -993,6 +1023,36 @@ impl Memory { pub(crate) fn focus_mut(&mut self) -> &mut Focus { self.focus.entry(self.viewport_id).or_default() } + + /// Check if the widget owns IME events. + /// + /// A widget should only consume IME events if this returns `true`. At most + /// one widget can own IME events for each frame. + pub fn owns_ime_events(&self, id: Id) -> bool { + let Some(focus) = self.focus() else { + return false; + }; + // We check across two frames because the widget that called + // `interrupt_ime` may run after other widgets that call this method + // within the same frame. + if matches!( + self.ime_interruption_time, + ImeInterruptionTime::ThisFrame | ImeInterruptionTime::LastFrame + ) { + return false; + } + focus.focused() == Some(id) + } + + /// Interrupt the current IME composition, if any. + /// + /// This causes [`Self::owns_ime_events`] to return `false` for all widgets + /// for the remainder of this frame and the next frame, giving time + /// for the IME to be dismissed (by making `platform_output.ime` be `None` + /// for at least one frame). + pub fn interrupt_ime(&mut self) { + self.ime_interruption_time = ImeInterruptionTime::ThisFrame; + } } /// State of an open popup. diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index 6f4d9a044..9905a2a55 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -10,8 +10,11 @@ use crate::{ TextStyle, Ui, Vec2, Widget, WidgetInfo, WidgetWithState, epaint, os::OperatingSystem, output::OutputEvent, - response, text_selection, - text_selection::{CCursorRange, text_cursor_state::cursor_rect, visuals::paint_text_selection}, + response, + text_edit::state::TextEditCursorPurpose, + text_selection::{ + self, CCursorRange, text_cursor_state::cursor_rect, visuals::paint_text_selection, + }, vec2, }; @@ -858,33 +861,23 @@ impl TextEdit<'_> { now - state.last_interaction_time, ); } - - // Set IME output (in screen coords) when text is editable and visible - let to_global = ui - .ctx() - .layer_transform_to_global(ui.layer_id()) - .unwrap_or_default(); - - ui.output_mut(|o| { - o.ime = Some(crate::output::IMEOutput { - rect: to_global * inner_rect, - cursor_rect: to_global * primary_cursor_rect, + if ui.memory(|mem| mem.owns_ime_events(id)) { + // Set IME output (in screen coords) when text is editable and visible + let to_global = ui + .ctx() + .layer_transform_to_global(ui.layer_id()) + .unwrap_or_default(); + ui.output_mut(|o| { + o.ime = Some(crate::output::IMEOutput { + rect: to_global * inner_rect, + cursor_rect: to_global * primary_cursor_rect, + }); }); - }); + } } } } - // Ensures correct IME behavior when the text input area gains or loses focus. - if state.ime_enabled && (response.gained_focus() || response.lost_focus()) { - state.ime_enabled = false; - if let Some(mut ccursor_range) = state.cursor.char_range() { - ccursor_range.secondary.index = ccursor_range.primary.index; - state.cursor.set_char_range(Some(ccursor_range)); - } - ui.input_mut(|i| i.events.retain(|e| !matches!(e, Event::Ime(_)))); - } - state.clone().store(ui.ctx(), id); if response.changed() { @@ -999,6 +992,11 @@ fn events( let events = ui.input(|i| i.filtered_events(&event_filter)); + let owns_ime_events = ui.memory(|mem| mem.owns_ime_events(id)); + if !owns_ime_events { + state.cursor_purpose = TextEditCursorPurpose::Selection; + } + for event in &events { let did_mutate_text = match event { // First handle events that only changes the selection cursor, not the text: @@ -1126,7 +1124,7 @@ fn events( .. } => check_for_mutating_key_press(os, &cursor_range, text, galley, modifiers, *key), - Event::Ime(ime_event) => { + Event::Ime(ime_event) if owns_ime_events => { /// Both `ImeEvent::Preedit("")` and `ImeEvent::Commit("")` /// might be emitted from different integrations to signify that /// the current IME composition should be cleared. @@ -1160,46 +1158,58 @@ fn events( } match ime_event { - ImeEvent::Enabled => { - state.ime_enabled = true; - state.ime_cursor_range = cursor_range; + #[expect(deprecated)] + ImeEvent::Enabled | ImeEvent::Disabled => None, + // Ignore `Preedit`/`Commit` events with empty text when + // there is no active IME composition. + // + // Some integrations may emit these events when there is no + // active IME composition (e.g. when `set_ime_allowed` or + // `set_ime_cursor_area` is called on `winit`'s `Window` on + // Wayland). Without this guard, they would clear any + // selected text. + // + // TODO(umajho): Ideally this would be handled by the + // integration, but since this guard is harmless for well- + // behaved integrations and also fixes the issue described + // above, it is good enough for now. + ImeEvent::Preedit(composition_text) | ImeEvent::Commit(composition_text) + if composition_text.is_empty() + && !matches!( + state.cursor_purpose, + TextEditCursorPurpose::ImeComposition + ) => + { + None + } + ImeEvent::Preedit(composition_text) | ImeEvent::Commit(composition_text) + if composition_text == "\n" || composition_text == "\r" => + { None } ImeEvent::Preedit(preedit_text) => { - if preedit_text == "\n" || preedit_text == "\r" { - None + state.cursor_purpose = if preedit_text.is_empty() { + TextEditCursorPurpose::Selection } else { - let mut ccursor = clear_preedit_text(text, &cursor_range); + TextEditCursorPurpose::ImeComposition + }; + let mut ccursor = clear_preedit_text(text, &cursor_range); - let start_cursor = ccursor; - if !preedit_text.is_empty() { - text.insert_text_at(&mut ccursor, preedit_text, char_limit); - } - state.ime_cursor_range = cursor_range; - Some(CCursorRange::two(start_cursor, ccursor)) + let start_cursor = ccursor; + if !preedit_text.is_empty() { + text.insert_text_at(&mut ccursor, preedit_text, char_limit); } + Some(CCursorRange::two(start_cursor, ccursor)) } ImeEvent::Commit(commit_text) => { - if commit_text == "\n" || commit_text == "\r" { - None - } else { - state.ime_enabled = false; + state.cursor_purpose = TextEditCursorPurpose::Selection; + let mut ccursor = clear_preedit_text(text, &cursor_range); - let mut ccursor = clear_preedit_text(text, &cursor_range); - - if !commit_text.is_empty() - && cursor_range.secondary.index - == state.ime_cursor_range.secondary.index - { - text.insert_text_at(&mut ccursor, commit_text, char_limit); - } - - Some(CCursorRange::one(ccursor)) + if !commit_text.is_empty() { + text.insert_text_at(&mut ccursor, commit_text, char_limit); } - } - ImeEvent::Disabled => { - state.ime_enabled = false; - None + + Some(CCursorRange::one(ccursor)) } } } diff --git a/crates/egui/src/widgets/text_edit/state.rs b/crates/egui/src/widgets/text_edit/state.rs index 5827aac4b..48935c4f7 100644 --- a/crates/egui/src/widgets/text_edit/state.rs +++ b/crates/egui/src/widgets/text_edit/state.rs @@ -37,18 +37,14 @@ pub struct TextEditState { /// Controls the text selection. pub cursor: TextCursorState, + /// The purpose of the cursor. + #[cfg_attr(feature = "serde", serde(skip))] + pub(crate) cursor_purpose: TextEditCursorPurpose, + /// Wrapped in Arc for cheaper clones. #[cfg_attr(feature = "serde", serde(skip))] pub(crate) undoer: Arc>, - // If IME candidate window is shown on this text edit. - #[cfg_attr(feature = "serde", serde(skip))] - pub(crate) ime_enabled: bool, - - // cursor range for IME candidate. - #[cfg_attr(feature = "serde", serde(skip))] - pub(crate) ime_cursor_range: CCursorRange, - // Text offset within the widget area. // Used for sensing and singleline text clipping. #[cfg_attr(feature = "serde", serde(skip))] @@ -82,3 +78,13 @@ impl TextEditState { self.set_undoer(TextEditUndoer::default()); } } + +#[derive(Clone, Default)] +pub(crate) enum TextEditCursorPurpose { + /// The cursor is used for text selection. + #[default] + Selection, + + /// The cursor is used for IME composition. + ImeComposition, +}