Improve IME area reporting and set IME area in TextArea (#785)

This adds the required context methods for (re)setting the IME area. These are added to the mutable context types, which I believe is the right place. `TextArea` now uses `PlainEditor::ime_cursor_area` to report its IME area. Note Masonry currently reports new IME cursor areas only when the widget itself moves or the focus changes to a new widget, but Parley's `PlainEditor` determines the IME cursor area based on the selection and preedit state of the editor, meaning the area can change based on events as well. Other widgets may choose to have different behavior still. (It appears Masonry currently doesn't account for the IME area potentially changing based on relayout, which may be a bug.) This replaces the old IME area reporting with a check at the end of the rewrite pass to see if IME is active, and if it is, whether the IME area has moved. If it has, the new area is reported to the platform. --------- Co-authored-by: Daniel McNab <[email protected]>
linebender · Dec 11, 2024 · 806946b · 806946b
1 parent d5f1cd1
commit 806946b
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 17 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -110,7 +110,7 @@ tree_arena = { version = "0.1.0", path = "tree_arena" }
 vello = "0.3"
 wgpu = "22.1.0"
 kurbo = "0.11.1"
-parley = { git = "https://github.com/linebender/parley", rev = "211878fae20ba8b4bf4dcc39e6bf790eece8aa03", features = [
+parley = { git = "https://github.com/linebender/parley", rev = "1a8740d8d86ebf751201e45e89bb71019340137d", features = [
     "accesskit",
 ] }
 peniko = "0.2.0"

diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs
@@ -195,6 +195,25 @@ impl_context_method!(
                 .expect("get_child_state_mut: child not found");
             child_state_mut.item
         }
+
+        /// Set the IME cursor area.
+        ///
+        /// When this widget is [focused] and [accepts text input], the reported IME area is sent
+        /// to the platform. The area can be used by the platform to, for example, place a
+        /// candidate box near that area, while ensuring the area is not obscured.
+        ///
+        /// [focused]: EventCtx::request_focus
+        /// [accepts text input]: Widget::accepts_text_input
+        pub fn set_ime_area(&mut self, ime_area: Rect) {
+            self.widget_state.ime_area = Some(ime_area);
+        }
+
+        /// Remove the IME cursor area.
+        ///
+        /// See [`LayoutCtx::set_ime_area`](LayoutCtx::set_ime_area) for more details.
+        pub fn clear_ime_area(&mut self) {
+            self.widget_state.ime_area = None;
+        }
     }
 );
 

diff --git a/masonry/src/passes/compose.rs b/masonry/src/passes/compose.rs
@@ -6,7 +6,7 @@ use tree_arena::ArenaMut;
 use vello::kurbo::Vec2;
 
 use crate::passes::{enter_span_if, recurse_on_children};
-use crate::render_root::{RenderRoot, RenderRootSignal, RenderRootState};
+use crate::render_root::{RenderRoot, RenderRootState};
 use crate::{ComposeCtx, Widget, WidgetState};
 
 // --- MARK: RECURSE ---
@@ -42,12 +42,6 @@ fn compose_widget(
         widget.item.compose(&mut ctx);
     }
 
-    // TODO - Add unit tests for this.
-    if moved && state.item.accepts_text_input && global_state.is_focused(state.item.id) {
-        let ime_area = state.item.get_ime_area();
-        global_state.emit_signal(RenderRootSignal::new_ime_moved_signal(ime_area));
-    }
-
     // We need to update the accessibility node's coordinates and repaint it at the new position.
     state.item.request_accessibility = true;
     state.item.needs_accessibility = true;

diff --git a/masonry/src/passes/update.rs b/masonry/src/passes/update.rs
@@ -523,11 +523,6 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) {
             if widget_state.accepts_text_input {
                 root.global_state.emit_signal(RenderRootSignal::StartIme);
             }
-
-            root.global_state
-                .emit_signal(RenderRootSignal::new_ime_moved_signal(
-                    widget_state.get_ime_area(),
-                ));
         } else {
             root.global_state.is_ime_active = false;
         }

diff --git a/masonry/src/render_root.rs b/masonry/src/render_root.rs
@@ -39,6 +39,10 @@ use crate::text::BrushIndex;
 use crate::widget::{WidgetArena, WidgetMut, WidgetRef, WidgetState};
 use crate::{AccessEvent, Action, CursorIcon, Handled, QueryCtx, Widget, WidgetId, WidgetPod};
 
+/// We ensure that any valid initial IME area is sent to the platform by storing an invalid initial
+/// IME area as the `last_sent_ime_area`.
+const INVALID_IME_AREA: Rect = Rect::new(f64::NAN, f64::NAN, f64::NAN, f64::NAN);
+
 // --- MARK: STRUCTS ---
 
 pub struct RenderRoot {
@@ -74,6 +78,10 @@ pub(crate) struct RenderRootState {
     pub(crate) text_layout_context: LayoutContext<BrushIndex>,
     pub(crate) mutate_callbacks: Vec<MutateCallback>,
     pub(crate) is_ime_active: bool,
+    /// The IME area last sent to the platform.
+    ///
+    /// This allows only sending the area to the platform when the area has changed.
+    pub(crate) last_sent_ime_area: Rect,
     pub(crate) scenes: HashMap<WidgetId, Scene>,
     /// Whether data set in the pointer pass has been invalidated.
     pub(crate) needs_pointer_pass: bool,
@@ -170,6 +178,7 @@ impl RenderRoot {
                 text_layout_context: LayoutContext::new(),
                 mutate_callbacks: Vec::new(),
                 is_ime_active: false,
+                last_sent_ime_area: INVALID_IME_AREA,
                 scenes: HashMap::new(),
                 needs_pointer_pass: false,
                 trace: PassTracing::from_env(),
@@ -263,6 +272,12 @@ impl RenderRoot {
         let _span = info_span!("text_event");
         let handled = run_on_text_event_pass(self, &event);
         run_update_focus_pass(self);
+
+        if matches!(event, TextEvent::Ime(winit::event::Ime::Enabled)) {
+            // Reset the last sent IME area, as the platform reset the IME state and may have
+            // forgotten it.
+            self.global_state.last_sent_ime_area = INVALID_IME_AREA;
+        }
         self.run_rewrite_passes();
 
         handled
@@ -489,6 +504,19 @@ impl RenderRoot {
             self.global_state
                 .emit_signal(RenderRootSignal::RequestRedraw);
         }
+
+        if self.global_state.is_ime_active {
+            let widget = self
+                .global_state
+                .focused_widget
+                .expect("IME is active without a focused widget");
+            let ime_area = self.widget_arena.get_state(widget).item.get_ime_area();
+            if self.global_state.last_sent_ime_area != ime_area {
+                self.global_state.last_sent_ime_area = ime_area;
+                self.global_state
+                    .emit_signal(RenderRootSignal::new_ime_moved_signal(ime_area));
+            }
+        }
     }
 
     pub(crate) fn request_render_all(&mut self) {
@@ -580,6 +608,10 @@ impl RenderRootState {
         self.focused_widget != self.next_focused_widget
     }
 
+    #[expect(
+        dead_code,
+        reason = "no longer used, but may be useful again in the future"
+    )]
     pub(crate) fn is_focused(&self, id: WidgetId) -> bool {
         self.focused_widget == Some(id)
     }
@@ -594,7 +626,7 @@ impl RenderRootSignal {
         RenderRootSignal::ImeMoved(
             LogicalPosition {
                 x: area.origin().x,
-                y: area.origin().y + area.size().height,
+                y: area.origin().y,
             },
             LogicalSize {
                 width: area.size().width,

diff --git a/masonry/src/widget/text_area.rs b/masonry/src/widget/text_area.rs
@@ -14,7 +14,7 @@ use parley::layout::Alignment;
 use parley::PlainEditor;
 use smallvec::SmallVec;
 use tracing::{trace_span, Span};
-use vello::kurbo::Vec2;
+use vello::kurbo::{Rect, Vec2};
 use vello::peniko::{Brush, Color, Fill};
 use vello::Scene;
 use winit::keyboard::{Key, NamedKey};
@@ -299,6 +299,25 @@ impl<const EDITABLE: bool> TextArea<EDITABLE> {
     }
 }
 
+// --- MARK: HELPERS ---
+impl<const EDITABLE: bool> TextArea<EDITABLE> {
+    /// Get the IME area from the editor, accounting for padding.
+    ///
+    /// This should only be called when the editor layout is available.
+    fn ime_area(&self) -> Rect {
+        debug_assert!(
+            self.editor.try_layout().is_some(),
+            "TextArea::ime_area should only be called when the editor layout is available"
+        );
+        let is_rtl = self
+            .editor
+            .try_layout()
+            .map(|layout| layout.is_rtl())
+            .unwrap_or(false);
+        self.editor.ime_cursor_area() + Vec2::new(self.padding.get_left(is_rtl), self.padding.top)
+    }
+}
+
 // --- MARK: WIDGETMUT ---
 impl<const EDITABLE: bool> TextArea<EDITABLE> {
     /// Set font styling for an active text area.
@@ -499,6 +518,7 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
                     let new_generation = self.editor.generation();
                     if new_generation != self.rendered_generation {
                         ctx.request_render();
+                        ctx.set_ime_area(self.ime_area());
                         self.rendered_generation = new_generation;
                     }
                     ctx.request_focus();
@@ -515,6 +535,7 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
                     let new_generation = self.editor.generation();
                     if new_generation != self.rendered_generation {
                         ctx.request_render();
+                        ctx.set_ime_area(self.ime_area());
                         self.rendered_generation = new_generation;
                     }
                 }
@@ -539,6 +560,7 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
                     },
                 );
                 let (fctx, lctx) = ctx.text_contexts();
+                // Whether the text was changed.
                 let mut edited = false;
                 // Ideally we'd use key_without_modifiers, but that's broken
                 match &key_event.logical_key {
@@ -730,6 +752,7 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
                         ctx.request_layout();
                     } else {
                         ctx.request_render();
+                        ctx.set_ime_area(self.ime_area());
                     }
                     self.rendered_generation = new_generation;
                 }
@@ -851,6 +874,7 @@ impl<const EDITABLE: bool> Widget for TextArea<EDITABLE> {
         let layout = self.editor.layout(fctx, lctx);
         let text_width = max_advance.unwrap_or(layout.full_width());
         let text_size = Size::new(text_width.into(), layout.height().into());
+        ctx.set_ime_area(self.ime_area());
 
         let area_size = Size {
             height: text_size.height + padding_size.height,