From 806946bb2ad2ae8681d4f3e5722f1f463843e135 Mon Sep 17 00:00:00 2001 From: Tom Churchman Date: Wed, 11 Dec 2024 17:21:18 +0100 Subject: [PATCH] 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 <36049421+DJMcNab@users.noreply.github.com> --- Cargo.lock | 5 +++-- Cargo.toml | 2 +- masonry/src/contexts.rs | 19 ++++++++++++++++++ masonry/src/passes/compose.rs | 8 +------- masonry/src/passes/update.rs | 5 ----- masonry/src/render_root.rs | 34 ++++++++++++++++++++++++++++++++- masonry/src/widget/text_area.rs | 26 ++++++++++++++++++++++++- 7 files changed, 82 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eb16dc5d2..cb31d92b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1083,8 +1083,9 @@ dependencies = [ [[package]] name = "fontique" version = "0.2.0" -source = "git+https://github.com/linebender/parley?rev=211878fae20ba8b4bf4dcc39e6bf790eece8aa03#211878fae20ba8b4bf4dcc39e6bf790eece8aa03" +source = "git+https://github.com/linebender/parley?rev=1a8740d8d86ebf751201e45e89bb71019340137d#1a8740d8d86ebf751201e45e89bb71019340137d" dependencies = [ + "bytemuck", "core-foundation", "core-text", "fontconfig-cache-parser", @@ -2506,7 +2507,7 @@ dependencies = [ [[package]] name = "parley" version = "0.2.0" -source = "git+https://github.com/linebender/parley?rev=211878fae20ba8b4bf4dcc39e6bf790eece8aa03#211878fae20ba8b4bf4dcc39e6bf790eece8aa03" +source = "git+https://github.com/linebender/parley?rev=1a8740d8d86ebf751201e45e89bb71019340137d#1a8740d8d86ebf751201e45e89bb71019340137d" dependencies = [ "accesskit", "fontique", diff --git a/Cargo.toml b/Cargo.toml index 5e56895c9..4924394cf 100644 --- 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 index d068fd4a8..81f94b3c7 100644 --- 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 index 88cf7edc0..71eb76242 100644 --- 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 index b6c66ef69..e22f7ae42 100644 --- 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 index 623d2f31a..d52df7c76 100644 --- 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, pub(crate) mutate_callbacks: Vec, 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, /// 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 index 5dd81530a..bccd1cc32 100644 --- 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 TextArea { } } +// --- MARK: HELPERS --- +impl TextArea { + /// 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 TextArea { /// Set font styling for an active text area. @@ -499,6 +518,7 @@ impl Widget for TextArea { 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 Widget for TextArea { 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 Widget for TextArea { }, ); 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 Widget for TextArea { ctx.request_layout(); } else { ctx.request_render(); + ctx.set_ime_area(self.ime_area()); } self.rendered_generation = new_generation; } @@ -851,6 +874,7 @@ impl Widget for TextArea { 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,