From e039070277d6c2d75335985f07f2e17d67003bdd Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Wed, 4 Sep 2024 21:08:45 +0200 Subject: [PATCH 01/14] parley: add IME support to text editor example --- examples/vello_editor/src/main.rs | 3 +- examples/vello_editor/src/text.rs | 127 +++++++++++++++++++++++++++++- parley/src/layout/cursor.rs | 10 +++ 3 files changed, 137 insertions(+), 3 deletions(-) diff --git a/examples/vello_editor/src/main.rs b/examples/vello_editor/src/main.rs index 4fc2e071..d3ef17d5 100644 --- a/examples/vello_editor/src/main.rs +++ b/examples/vello_editor/src/main.rs @@ -60,6 +60,7 @@ impl<'s> ApplicationHandler for SimpleVelloApp<'s> { let window = cached_window .take() .unwrap_or_else(|| create_winit_window(event_loop)); + window.set_ime_allowed(true); // Create a vello Surface let size = window.inner_size(); @@ -107,7 +108,7 @@ impl<'s> ApplicationHandler for SimpleVelloApp<'s> { _ => return, }; - self.editor.handle_event(&event); + self.editor.handle_event(&render_state.window, &event); render_state.window.request_redraw(); // render_state // .window diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index 897654a8..f3077976 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -3,12 +3,15 @@ #[cfg(not(target_os = "android"))] use clipboard_rs::{Clipboard, ClipboardContext}; -use parley::layout::cursor::{Selection, VisualMode}; +use parley::layout::cursor::{Cursor, Selection, VisualMode}; use parley::layout::Affinity; use parley::{layout::PositionedLayoutItem, FontContext}; use peniko::{kurbo::Affine, Color, Fill}; use std::time::Instant; use vello::Scene; +use winit::dpi::{LogicalPosition, LogicalSize}; +use winit::event::Ime; +use winit::window::Window; use winit::{ event::{Modifiers, WindowEvent}, keyboard::{KeyCode, PhysicalKey}, @@ -26,6 +29,16 @@ pub enum ActiveText<'a> { Selection(&'a str), } +#[derive(Default)] +enum ComposeState { + #[default] + None, + Preedit { + /// The location of the (uncommitted) preedit text + text_at: Selection, + }, +} + #[derive(Default)] pub struct Editor { font_cx: FontContext, @@ -33,6 +46,7 @@ pub struct Editor { buffer: String, layout: Layout, selection: Selection, + compose_state: ComposeState, cursor_mode: VisualMode, last_click_time: Option, click_count: u32, @@ -126,7 +140,28 @@ impl Editor { // TODO: support clipboard on Android } - pub fn handle_event(&mut self, event: &WindowEvent) { + pub fn handle_event(&mut self, window: &Window, event: &WindowEvent) { + if let ComposeState::Preedit { text_at } = self.compose_state { + // Clear old preedit state when handling events that potentially mutate text/selection. + // This is a bit overzealous, e.g., pressing and releasing shift probably shouldnt't + // clear the preedit. + if matches!( + event, + WindowEvent::KeyboardInput { .. } + | WindowEvent::MouseInput { .. } + | WindowEvent::Ime(..) + ) { + let range = text_at.text_range(); + self.selection = + Selection::from_index(&self.layout, range.start - 1, Affinity::Upstream); + self.buffer.replace_range(range, ""); + self.compose_state = ComposeState::None; + // TODO: defer updating layout. If the event itself also causes an update, we now + // update twice. + self.update_layout(self.width, 1.0); + } + } + match event { WindowEvent::Resized(size) => { self.update_layout(size.width as f32, 1.0); @@ -265,6 +300,87 @@ impl Editor { // println!("Active text: {:?}", self.active_text()); } + WindowEvent::Ime(ime) => { + match ime { + Ime::Commit(text) => { + let start = self + .delete_current_selection() + .unwrap_or_else(|| self.selection.focus().text_range().start); + self.buffer.insert_str(start, text); + self.update_layout(self.width, 1.0); + self.selection = Selection::from_index( + &self.layout, + start + text.len() - 1, + Affinity::Upstream, + ); + } + Ime::Preedit(text, compose_cursor) => { + if text.is_empty() { + // Winit sends empty preedit text to indicate the preedit was cleared. + return; + } + + let start = self + .delete_current_selection() + .unwrap_or_else(|| self.selection.focus().text_range().start); + self.buffer.insert_str(start, text); + self.update_layout(self.width, 1.0); + + { + // winit says the cursor should be hidden when compose_cursor is None. + // Do we handle that? We also don't extend the cursor to the end + // indicated by winit, instead IME composing is currently indicated by + // highlighting the entire preedit text. Should we even update the + // selection at all? + let compose_cursor = compose_cursor.unwrap_or((0, 0)); + self.selection = Selection::from_index( + &self.layout, + start - 1 + compose_cursor.0, + Affinity::Upstream, + ); + } + + { + let text_end = Cursor::from_index( + &self.layout, + start - 1 + text.len(), + Affinity::Upstream, + ); + let ime_cursor = self.selection.extend_to_cursor(text_end); + self.compose_state = ComposeState::Preedit { + text_at: ime_cursor, + }; + + // Find the smallest rectangle that contains the entire preedit text. + // Send that rectangle to the platform to suggest placement for the IME + // candidate box. + let mut union_rect = None; + ime_cursor.geometry_with(&self.layout, |rect| { + if union_rect.is_none() { + union_rect = Some(rect); + } + union_rect = Some(union_rect.unwrap().union(rect)); + }); + if let Some(union_rect) = union_rect { + window.set_ime_cursor_area( + LogicalPosition::new(union_rect.x0, union_rect.y0), + LogicalSize::new( + union_rect.width(), + // TODO: an offset is added here to prevent the IME + // candidate box from overlapping with the IME cursor. From + // the Winit docs I would've expected the IME candidate box + // not to overlap the indicated IME cursor area, but for + // some reason it does (tested using fcitx5 + // on wayland) + union_rect.height() + 40.0, + ), + ); + } + } + } + _ => {} + } + } WindowEvent::MouseInput { state, button, .. } => { if *button == winit::event::MouseButton::Left { self.pointer_down = state.is_pressed(); @@ -350,6 +466,13 @@ impl Editor { if let Some(cursor) = self.selection.focus().weak_geometry(&self.layout, 1.5) { scene.fill(Fill::NonZero, transform, Color::LIGHT_GRAY, None, &cursor); }; + if let ComposeState::Preedit { text_at } = self.compose_state { + // TODO: underline rather than fill, requires access to underline_offset metric for + // each run + text_at.geometry_with(&self.layout, |rect| { + scene.fill(Fill::NonZero, transform, Color::SPRING_GREEN, None, &rect); + }); + } for line in self.layout.lines() { for item in line.items() { let PositionedLayoutItem::GlyphRun(glyph_run) = item else { diff --git a/parley/src/layout/cursor.rs b/parley/src/layout/cursor.rs index ca3dd5c7..dd69e248 100644 --- a/parley/src/layout/cursor.rs +++ b/parley/src/layout/cursor.rs @@ -505,6 +505,16 @@ impl Selection { } } + /// Returns a new selection with the focus extended to the given cursor. + #[must_use] + pub fn extend_to_cursor(&self, focus: Cursor) -> Self { + Self { + anchor: self.anchor, + focus, + h_pos: None, + } + } + /// Returns a new selection with the focus moved to the next cluster in /// visual order. /// From dd0f807012f3e3db70277c978a838f344e330049 Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Fri, 6 Sep 2024 17:49:04 +0200 Subject: [PATCH 02/14] wip: underline IME preedit instead of highlighting --- examples/vello_editor/src/text.rs | 49 ++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index f3077976..9e8aa79a 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -8,6 +8,7 @@ use parley::layout::Affinity; use parley::{layout::PositionedLayoutItem, FontContext}; use peniko::{kurbo::Affine, Color, Fill}; use std::time::Instant; +use vello::kurbo::{Line, Stroke}; use vello::Scene; use winit::dpi::{LogicalPosition, LogicalSize}; use winit::event::Ime; @@ -71,6 +72,14 @@ impl Editor { builder.push_default(&parley::style::StyleProperty::FontStack( parley::style::FontStack::Source("system-ui"), )); + if let ComposeState::Preedit { text_at } = self.compose_state { + let text_range = text_at.text_range(); + builder.push( + &parley::style::StyleProperty::UnderlineBrush(Some(Color::SPRING_GREEN)), + text_range.clone(), + ); + builder.push(&parley::style::StyleProperty::Underline(true), text_range); + } builder.build_into(&mut self.layout); self.layout.break_all_lines(Some(width - INSET * 2.0)); self.layout @@ -324,7 +333,6 @@ impl Editor { .delete_current_selection() .unwrap_or_else(|| self.selection.focus().text_range().start); self.buffer.insert_str(start, text); - self.update_layout(self.width, 1.0); { // winit says the cursor should be hidden when compose_cursor is None. @@ -377,6 +385,8 @@ impl Editor { ); } } + + self.update_layout(self.width, 1.0); } _ => {} } @@ -466,13 +476,6 @@ impl Editor { if let Some(cursor) = self.selection.focus().weak_geometry(&self.layout, 1.5) { scene.fill(Fill::NonZero, transform, Color::LIGHT_GRAY, None, &cursor); }; - if let ComposeState::Preedit { text_at } = self.compose_state { - // TODO: underline rather than fill, requires access to underline_offset metric for - // each run - text_at.geometry_with(&self.layout, |rect| { - scene.fill(Fill::NonZero, transform, Color::SPRING_GREEN, None, &rect); - }); - } for line in self.layout.lines() { for item in line.items() { let PositionedLayoutItem::GlyphRun(glyph_run) = item else { @@ -487,6 +490,8 @@ impl Editor { let glyph_xform = synthesis .skew() .map(|angle| Affine::skew(angle.to_radians().tan() as f64, 0.0)); + + let style = glyph_run.style(); let coords = run .normalized_coords() .iter() @@ -513,6 +518,34 @@ impl Editor { } }), ); + if let Some(underline) = &style.underline { + let underline_brush = &underline.brush; + let run_metrics = glyph_run.run().metrics(); + let offset = match underline.offset { + Some(offset) => offset, + None => run_metrics.underline_offset, + }; + let width = match underline.size { + Some(size) => size, + None => run_metrics.underline_size, + }; + // The `offset` is the distance from the baseline to the *top* of the underline + // so we move the line down by half the width + // Remember that we are using a y-down coordinate system + let y = glyph_run.baseline() - offset + width / 2.; + + let line = Line::new( + (glyph_run.offset() as f64, y as f64), + ((glyph_run.offset() + glyph_run.advance()) as f64, y as f64), + ); + scene.stroke( + &Stroke::new(width.into()), + transform, + underline_brush, + None, + &line, + ); + } } } } From 0014a8d0d70a2a0c01a746071f3b8f42147ef04e Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Fri, 6 Sep 2024 19:06:14 +0200 Subject: [PATCH 03/14] wip: improve selection handling --- examples/vello_editor/src/text.rs | 52 ++++++++++++++++++------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index 9e8aa79a..1e8b578d 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -162,7 +162,7 @@ impl Editor { ) { let range = text_at.text_range(); self.selection = - Selection::from_index(&self.layout, range.start - 1, Affinity::Upstream); + Selection::from_index(&self.layout, range.start, Affinity::Downstream); self.buffer.replace_range(range, ""); self.compose_state = ComposeState::None; // TODO: defer updating layout. If the event itself also causes an update, we now @@ -329,41 +329,34 @@ impl Editor { return; } - let start = self - .delete_current_selection() - .unwrap_or_else(|| self.selection.focus().text_range().start); - self.buffer.insert_str(start, text); + if let Some(start) = self.delete_current_selection() { + self.selection = + Selection::from_index(&self.layout, start, Affinity::Downstream); + } { - // winit says the cursor should be hidden when compose_cursor is None. - // Do we handle that? We also don't extend the cursor to the end - // indicated by winit, instead IME composing is currently indicated by - // highlighting the entire preedit text. Should we even update the - // selection at all? - let compose_cursor = compose_cursor.unwrap_or((0, 0)); - self.selection = Selection::from_index( + let insertion_index = self.selection.insertion_index(); + self.buffer.insert_str(insertion_index, text); + let text_start = Selection::from_index( &self.layout, - start - 1 + compose_cursor.0, - Affinity::Upstream, + insertion_index, + Affinity::Downstream, ); - } - - { let text_end = Cursor::from_index( &self.layout, - start - 1 + text.len(), - Affinity::Upstream, + insertion_index + text.len(), + Affinity::Downstream, ); - let ime_cursor = self.selection.extend_to_cursor(text_end); + let text_at = text_start.extend_to_cursor(text_end); self.compose_state = ComposeState::Preedit { - text_at: ime_cursor, + text_at: text_start.extend_to_cursor(text_end), }; // Find the smallest rectangle that contains the entire preedit text. // Send that rectangle to the platform to suggest placement for the IME // candidate box. let mut union_rect = None; - ime_cursor.geometry_with(&self.layout, |rect| { + text_at.geometry_with(&self.layout, |rect| { if union_rect.is_none() { union_rect = Some(rect); } @@ -386,6 +379,21 @@ impl Editor { } } + { + // winit says the cursor should be hidden when compose_cursor is None. + // Do we handle that? We also don't extend the cursor to the end + // indicated by winit, instead IME composing is currently indicated by + // underlining the entire preedit text, and the IME candidate box + // placement is based on the preedit text location. Should we even + // update the selection based on the compose cursor at all? + let compose_cursor = compose_cursor.unwrap_or((0, 0)); + self.selection = Selection::from_index( + &self.layout, + self.selection.insertion_index() + compose_cursor.0, + Affinity::Downstream, + ); + } + self.update_layout(self.width, 1.0); } _ => {} From 173bb8c5f1e5bd876da297a8280e17e8f14b20f3 Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Fri, 6 Sep 2024 22:33:37 +0200 Subject: [PATCH 04/14] wip: keep preedit and selection anchor in sync --- examples/vello_editor/src/text.rs | 254 ++++++++++++++++++++---------- 1 file changed, 168 insertions(+), 86 deletions(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index 1e8b578d..8041497d 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -7,6 +7,7 @@ use parley::layout::cursor::{Cursor, Selection, VisualMode}; use parley::layout::Affinity; use parley::{layout::PositionedLayoutItem, FontContext}; use peniko::{kurbo::Affine, Color, Fill}; +use std::ops::Range; use std::time::Instant; use vello::kurbo::{Line, Stroke}; use vello::Scene; @@ -30,13 +31,13 @@ pub enum ActiveText<'a> { Selection(&'a str), } -#[derive(Default)] +#[derive(Default, Clone)] enum ComposeState { #[default] None, Preedit { /// The location of the (uncommitted) preedit text - text_at: Selection, + text_range: Range, }, } @@ -57,6 +58,33 @@ pub struct Editor { width: f32, } +/// Shrink the selection by the given amount of bytes by moving the focus towards the anchor. +fn shrink_selection(layout: &Layout, selection: Selection, bytes: usize) -> Selection { + let mut selection = selection; + let shrink = bytes.min(selection.text_range().len()); + if shrink == 0 { + return selection; + } + + let anchor = *selection.anchor(); + let focus = *selection.focus(); + + let new_focus_index = if focus.text_range().start > anchor.text_range().start { + focus.index() - shrink + } else { + focus.index() + shrink + }; + + selection = Selection::from_index(layout, anchor.index(), anchor.affinity()); + selection = selection.extend_to_cursor(Cursor::from_index( + layout, + new_focus_index, + focus.affinity(), + )); + + selection +} + impl Editor { pub fn set_text(&mut self, text: &str) { self.buffer.clear(); @@ -72,13 +100,15 @@ impl Editor { builder.push_default(&parley::style::StyleProperty::FontStack( parley::style::FontStack::Source("system-ui"), )); - if let ComposeState::Preedit { text_at } = self.compose_state { - let text_range = text_at.text_range(); + if let ComposeState::Preedit { ref text_range } = self.compose_state { builder.push( &parley::style::StyleProperty::UnderlineBrush(Some(Color::SPRING_GREEN)), text_range.clone(), ); - builder.push(&parley::style::StyleProperty::Underline(true), text_range); + builder.push( + &parley::style::StyleProperty::Underline(true), + text_range.clone(), + ); } builder.build_into(&mut self.layout); self.layout.break_all_lines(Some(width - INSET * 2.0)); @@ -149,32 +179,51 @@ impl Editor { // TODO: support clipboard on Android } - pub fn handle_event(&mut self, window: &Window, event: &WindowEvent) { - if let ComposeState::Preedit { text_at } = self.compose_state { - // Clear old preedit state when handling events that potentially mutate text/selection. - // This is a bit overzealous, e.g., pressing and releasing shift probably shouldnt't - // clear the preedit. - if matches!( - event, - WindowEvent::KeyboardInput { .. } - | WindowEvent::MouseInput { .. } - | WindowEvent::Ime(..) - ) { - let range = text_at.text_range(); - self.selection = - Selection::from_index(&self.layout, range.start, Affinity::Downstream); - self.buffer.replace_range(range, ""); - self.compose_state = ComposeState::None; - // TODO: defer updating layout. If the event itself also causes an update, we now - // update twice. - self.update_layout(self.width, 1.0); + /// Suggest an area for IME candidate box placement based on the current IME state. + fn set_ime_cursor_area(&self, window: &Window) { + if let ComposeState::Preedit { ref text_range } = self.compose_state { + // Find the smallest rectangle that contains the entire preedit text. + // Send that rectangle to the platform to suggest placement for the IME + // candidate box. + let mut union_rect = None; + let preedit_selection = + Selection::from_index(&self.layout, text_range.start, Affinity::Downstream); + let preedit_selection = preedit_selection.extend_to_cursor(Cursor::from_index( + &self.layout, + text_range.end, + Affinity::Downstream, + )); + + preedit_selection.geometry_with(&self.layout, |rect| { + if union_rect.is_none() { + union_rect = Some(rect); + } + union_rect = Some(union_rect.unwrap().union(rect)); + }); + if let Some(union_rect) = union_rect { + window.set_ime_cursor_area( + LogicalPosition::new(union_rect.x0, union_rect.y0), + LogicalSize::new( + union_rect.width(), + // TODO: an offset is added here to prevent the IME + // candidate box from overlapping with the IME cursor. From + // the Winit docs I would've expected the IME candidate box + // not to overlap the indicated IME cursor area, but for + // some reason it does (tested using fcitx5 + // on wayland) + union_rect.height() + 40.0, + ), + ); } } + } + pub fn handle_event(&mut self, window: &Window, event: &WindowEvent) { match event { WindowEvent::Resized(size) => { self.update_layout(size.width as f32, 1.0); self.selection = self.selection.refresh(&self.layout); + self.set_ime_cursor_area(window); } WindowEvent::ModifiersChanged(modifiers) => { self.modifiers = Some(*modifiers); @@ -311,6 +360,7 @@ impl Editor { } WindowEvent::Ime(ime) => { match ime { + Ime::Enabled => {} Ime::Commit(text) => { let start = self .delete_current_selection() @@ -323,10 +373,20 @@ impl Editor { Affinity::Upstream, ); } - Ime::Preedit(text, compose_cursor) => { - if text.is_empty() { - // Winit sends empty preedit text to indicate the preedit was cleared. - return; + Ime::Preedit(text, _compose_cursor) => { + if let ComposeState::Preedit { text_range } = self.compose_state.clone() { + self.buffer.replace_range(text_range.clone(), ""); + + // Invariant: the selection anchor and start of preedit text are at the same + // position. + // If the focus extends into the preedit range, shrink the selection. + if self.selection.focus().text_range().start > text_range.start { + self.selection = shrink_selection( + &self.layout, + self.selection, + text_range.len(), + ); + } } if let Some(start) = self.delete_current_selection() { @@ -334,69 +394,46 @@ impl Editor { Selection::from_index(&self.layout, start, Affinity::Downstream); } - { - let insertion_index = self.selection.insertion_index(); - self.buffer.insert_str(insertion_index, text); - let text_start = Selection::from_index( - &self.layout, - insertion_index, - Affinity::Downstream, - ); - let text_end = Cursor::from_index( - &self.layout, - insertion_index + text.len(), - Affinity::Downstream, - ); - let text_at = text_start.extend_to_cursor(text_end); - self.compose_state = ComposeState::Preedit { - text_at: text_start.extend_to_cursor(text_end), - }; + let insertion_index = self.selection.insertion_index(); + self.buffer.insert_str(insertion_index, text); + self.compose_state = ComposeState::Preedit { + text_range: insertion_index..insertion_index + text.len(), + }; - // Find the smallest rectangle that contains the entire preedit text. - // Send that rectangle to the platform to suggest placement for the IME - // candidate box. - let mut union_rect = None; - text_at.geometry_with(&self.layout, |rect| { - if union_rect.is_none() { - union_rect = Some(rect); - } - union_rect = Some(union_rect.unwrap().union(rect)); - }); - if let Some(union_rect) = union_rect { - window.set_ime_cursor_area( - LogicalPosition::new(union_rect.x0, union_rect.y0), - LogicalSize::new( - union_rect.width(), - // TODO: an offset is added here to prevent the IME - // candidate box from overlapping with the IME cursor. From - // the Winit docs I would've expected the IME candidate box - // not to overlap the indicated IME cursor area, but for - // some reason it does (tested using fcitx5 - // on wayland) - union_rect.height() + 40.0, - ), + self.update_layout(self.width, 1.0); + + // winit says the cursor should be hidden when compose_cursor is None. + // Do we handle that? We also don't set the cursor based on the cursor + // indicated by winit, instead IME composing is currently indicated by + // underlining the entire preedit text, and the IME candidate box + // placement is based on the preedit text location. + self.selection = Selection::from_index( + &self.layout, + self.selection.insertion_index(), + Affinity::Downstream, + ); + self.set_ime_cursor_area(window); + } + Ime::Disabled => { + if let ComposeState::Preedit { text_range } = self.compose_state.clone() { + self.buffer.replace_range(text_range.clone(), ""); + self.compose_state = ComposeState::None; + self.update_layout(self.width, 1.0); + + // Invariant: the selection anchor and start of preedit text are at the same + // position. + // If the focus extends into the preedit range, shrink the selection. + if self.selection.focus().text_range().start > text_range.start { + self.selection = shrink_selection( + &self.layout, + self.selection, + text_range.len(), ); + } else { + self.selection = self.selection.refresh(&self.layout); } } - - { - // winit says the cursor should be hidden when compose_cursor is None. - // Do we handle that? We also don't extend the cursor to the end - // indicated by winit, instead IME composing is currently indicated by - // underlining the entire preedit text, and the IME candidate box - // placement is based on the preedit text location. Should we even - // update the selection based on the compose cursor at all? - let compose_cursor = compose_cursor.unwrap_or((0, 0)); - self.selection = Selection::from_index( - &self.layout, - self.selection.insertion_index() + compose_cursor.0, - Affinity::Downstream, - ); - } - - self.update_layout(self.width, 1.0); } - _ => {} } } WindowEvent::MouseInput { state, button, .. } => { @@ -460,6 +497,51 @@ impl Editor { } _ => {} } + + if let ComposeState::Preedit { text_range } = self.compose_state.clone() { + if text_range.start != self.selection.anchor().text_range().start { + // If the selection anchor is no longer at the same position as the preedit text, the + // selection has been moved. Move the preedit to the selection's new anchor position. + + // TODO: we can be smarter here to prevent need of the String allocation + let text = self.buffer[text_range.clone()].to_owned(); + self.buffer.replace_range(text_range.clone(), ""); + + if self.selection.anchor().text_range().start > text_range.start { + // shift the selection to the left to account for the preedit text that was + // just removed + let anchor = *self.selection.anchor(); + let focus = *self.selection.focus(); + let shift = text_range + .len() + .min(anchor.text_range().start - text_range.start); + self.selection = Selection::from_index( + &self.layout, + anchor.index() - shift, + anchor.affinity(), + ); + self.selection = self.selection.extend_to_cursor(Cursor::from_index( + &self.layout, + focus.index() - shift, + focus.affinity(), + )); + } + + let insertion_index = self.selection.insertion_index(); + self.buffer.insert_str(insertion_index, &text); + { + self.compose_state = ComposeState::Preedit { + text_range: insertion_index..insertion_index + text.len(), + }; + } + // TODO: events that caused the preedit to be moved may also have updated the + // layout, in that case we're now updating twice. + self.update_layout(self.width, 1.0); + + self.set_ime_cursor_area(window); + self.selection = self.selection.refresh(&self.layout); + } + } } fn delete_current_selection(&mut self) -> Option { From 4ec366eecad9091d402909799ed88c2cec8fd5b7 Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Fri, 6 Sep 2024 22:53:12 +0200 Subject: [PATCH 05/14] wip: remove vestigial block --- examples/vello_editor/src/text.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index 8041497d..f97e167f 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -529,11 +529,10 @@ impl Editor { let insertion_index = self.selection.insertion_index(); self.buffer.insert_str(insertion_index, &text); - { - self.compose_state = ComposeState::Preedit { - text_range: insertion_index..insertion_index + text.len(), - }; - } + self.compose_state = ComposeState::Preedit { + text_range: insertion_index..insertion_index + text.len(), + }; + // TODO: events that caused the preedit to be moved may also have updated the // layout, in that case we're now updating twice. self.update_layout(self.width, 1.0); From 427826de9014c1cb920d58918d72a655c9d69872 Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Wed, 11 Sep 2024 11:38:28 +0200 Subject: [PATCH 06/14] wip: remove ComposeState enum --- examples/vello_editor/src/text.rs | 32 +++++++++---------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index f97e167f..7a64101b 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -31,16 +31,6 @@ pub enum ActiveText<'a> { Selection(&'a str), } -#[derive(Default, Clone)] -enum ComposeState { - #[default] - None, - Preedit { - /// The location of the (uncommitted) preedit text - text_range: Range, - }, -} - #[derive(Default)] pub struct Editor { font_cx: FontContext, @@ -48,7 +38,8 @@ pub struct Editor { buffer: String, layout: Layout, selection: Selection, - compose_state: ComposeState, + /// The portion of the text currently marked as preedit by the IME. + preedit_range: Option>, cursor_mode: VisualMode, last_click_time: Option, click_count: u32, @@ -100,7 +91,7 @@ impl Editor { builder.push_default(&parley::style::StyleProperty::FontStack( parley::style::FontStack::Source("system-ui"), )); - if let ComposeState::Preedit { ref text_range } = self.compose_state { + if let Some(ref text_range) = self.preedit_range { builder.push( &parley::style::StyleProperty::UnderlineBrush(Some(Color::SPRING_GREEN)), text_range.clone(), @@ -181,7 +172,7 @@ impl Editor { /// Suggest an area for IME candidate box placement based on the current IME state. fn set_ime_cursor_area(&self, window: &Window) { - if let ComposeState::Preedit { ref text_range } = self.compose_state { + if let Some(ref text_range) = self.preedit_range { // Find the smallest rectangle that contains the entire preedit text. // Send that rectangle to the platform to suggest placement for the IME // candidate box. @@ -374,7 +365,7 @@ impl Editor { ); } Ime::Preedit(text, _compose_cursor) => { - if let ComposeState::Preedit { text_range } = self.compose_state.clone() { + if let Some(text_range) = self.preedit_range.take() { self.buffer.replace_range(text_range.clone(), ""); // Invariant: the selection anchor and start of preedit text are at the same @@ -396,9 +387,7 @@ impl Editor { let insertion_index = self.selection.insertion_index(); self.buffer.insert_str(insertion_index, text); - self.compose_state = ComposeState::Preedit { - text_range: insertion_index..insertion_index + text.len(), - }; + self.preedit_range = Some(insertion_index..insertion_index + text.len()); self.update_layout(self.width, 1.0); @@ -415,9 +404,8 @@ impl Editor { self.set_ime_cursor_area(window); } Ime::Disabled => { - if let ComposeState::Preedit { text_range } = self.compose_state.clone() { + if let Some(text_range) = self.preedit_range.take() { self.buffer.replace_range(text_range.clone(), ""); - self.compose_state = ComposeState::None; self.update_layout(self.width, 1.0); // Invariant: the selection anchor and start of preedit text are at the same @@ -498,7 +486,7 @@ impl Editor { _ => {} } - if let ComposeState::Preedit { text_range } = self.compose_state.clone() { + if let Some(ref text_range) = self.preedit_range { if text_range.start != self.selection.anchor().text_range().start { // If the selection anchor is no longer at the same position as the preedit text, the // selection has been moved. Move the preedit to the selection's new anchor position. @@ -529,9 +517,7 @@ impl Editor { let insertion_index = self.selection.insertion_index(); self.buffer.insert_str(insertion_index, &text); - self.compose_state = ComposeState::Preedit { - text_range: insertion_index..insertion_index + text.len(), - }; + self.preedit_range = Some(insertion_index..insertion_index + text.len()); // TODO: events that caused the preedit to be moved may also have updated the // layout, in that case we're now updating twice. From b01f737e652101040a964b7ca8ad1e1375fbdb21 Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Wed, 11 Sep 2024 15:50:32 +0200 Subject: [PATCH 07/14] wip: improve selection handling and improve performance - Use `String::replace_range` - Select area indicated by IME - Refactor to `Selection::from_cursors` rather than `Selection::extend_to_cursor` --- examples/vello_editor/src/text.rs | 122 +++++++++++++++--------------- parley/src/layout/cursor.rs | 19 +++-- 2 files changed, 71 insertions(+), 70 deletions(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index 7a64101b..811c521f 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -66,12 +66,10 @@ fn shrink_selection(layout: &Layout, selection: Selection, bytes: usize) -> Sele focus.index() + shrink }; - selection = Selection::from_index(layout, anchor.index(), anchor.affinity()); - selection = selection.extend_to_cursor(Cursor::from_index( - layout, - new_focus_index, - focus.affinity(), - )); + selection = Selection::from_cursors( + anchor, + Cursor::from_index(layout, new_focus_index, focus.affinity()), + ); selection } @@ -177,13 +175,10 @@ impl Editor { // Send that rectangle to the platform to suggest placement for the IME // candidate box. let mut union_rect = None; - let preedit_selection = - Selection::from_index(&self.layout, text_range.start, Affinity::Downstream); - let preedit_selection = preedit_selection.extend_to_cursor(Cursor::from_index( - &self.layout, - text_range.end, - Affinity::Downstream, - )); + let preedit_selection = Selection::from_cursors( + Cursor::from_index(&self.layout, text_range.start, Affinity::Downstream), + Cursor::from_index(&self.layout, text_range.end, Affinity::Downstream), + ); preedit_selection.geometry_with(&self.layout, |rect| { if union_rect.is_none() { @@ -353,54 +348,67 @@ impl Editor { match ime { Ime::Enabled => {} Ime::Commit(text) => { - let start = self - .delete_current_selection() - .unwrap_or_else(|| self.selection.focus().text_range().start); - self.buffer.insert_str(start, text); + let commit_start = if self.selection.is_collapsed() { + let start = self.selection.insertion_index(); + self.buffer.insert_str(start, text); + start + } else { + let range = self.selection.text_range(); + self.buffer.replace_range(range.clone(), text); + range.start + }; + self.update_layout(self.width, 1.0); self.selection = Selection::from_index( &self.layout, - start + text.len() - 1, - Affinity::Upstream, + commit_start + text.len(), + Affinity::Downstream, ); } - Ime::Preedit(text, _compose_cursor) => { - if let Some(text_range) = self.preedit_range.take() { - self.buffer.replace_range(text_range.clone(), ""); - - // Invariant: the selection anchor and start of preedit text are at the same - // position. - // If the focus extends into the preedit range, shrink the selection. - if self.selection.focus().text_range().start > text_range.start { - self.selection = shrink_selection( - &self.layout, - self.selection, - text_range.len(), - ); - } - } + Ime::Preedit(text, compose_cursor) => { + let preedit_start = if let Some(text_range) = self.preedit_range.take() { + self.buffer.replace_range(text_range.clone(), text); + text_range.start + } else { + let insertion_idx = self + .delete_current_selection() + .unwrap_or(self.selection.insertion_index()); + self.buffer.insert_str(insertion_idx, text); + insertion_idx + }; - if let Some(start) = self.delete_current_selection() { - self.selection = - Selection::from_index(&self.layout, start, Affinity::Downstream); + if text.is_empty() { + self.preedit_range = None; + } else { + self.preedit_range = Some(preedit_start..preedit_start + text.len()); } - let insertion_index = self.selection.insertion_index(); - self.buffer.insert_str(insertion_index, text); - self.preedit_range = Some(insertion_index..insertion_index + text.len()); - self.update_layout(self.width, 1.0); - // winit says the cursor should be hidden when compose_cursor is None. - // Do we handle that? We also don't set the cursor based on the cursor - // indicated by winit, instead IME composing is currently indicated by - // underlining the entire preedit text, and the IME candidate box - // placement is based on the preedit text location. - self.selection = Selection::from_index( - &self.layout, - self.selection.insertion_index(), - Affinity::Downstream, - ); + if let Some(compose_cursor) = compose_cursor { + // Select the location indicated by the IME. + self.selection = Selection::from_cursors( + Cursor::from_index( + &self.layout, + preedit_start + compose_cursor.0, + Affinity::Downstream, + ), + Cursor::from_index( + &self.layout, + preedit_start + compose_cursor.1, + Affinity::Downstream, + ), + ); + } else { + // IME indicates nothing is to be selected: collapse the selection to a + // caret just in front of the preedit. + self.selection = Selection::from_index( + &self.layout, + preedit_start, + Affinity::Downstream, + ) + } + self.set_ime_cursor_area(window); } Ime::Disabled => { @@ -503,16 +511,10 @@ impl Editor { let shift = text_range .len() .min(anchor.text_range().start - text_range.start); - self.selection = Selection::from_index( - &self.layout, - anchor.index() - shift, - anchor.affinity(), + self.selection = Selection::from_cursors( + Cursor::from_index(&self.layout, anchor.index() - shift, anchor.affinity()), + Cursor::from_index(&self.layout, focus.index() - shift, focus.affinity()), ); - self.selection = self.selection.extend_to_cursor(Cursor::from_index( - &self.layout, - focus.index() - shift, - focus.affinity(), - )); } let insertion_index = self.selection.insertion_index(); diff --git a/parley/src/layout/cursor.rs b/parley/src/layout/cursor.rs index dd69e248..86718594 100644 --- a/parley/src/layout/cursor.rs +++ b/parley/src/layout/cursor.rs @@ -407,6 +407,15 @@ impl Selection { Cursor::from_point(layout, x, y).into() } + /// Creates a selection bounding the given anchor and focus cursors. + pub fn from_cursors(anchor: Cursor, focus: Cursor) -> Self { + Self { + anchor, + focus, + h_pos: None, + } + } + /// Creates a new selection bounding the word at the given point. pub fn word_from_point(layout: &Layout, x: f32, y: f32) -> Self { let mut anchor = Cursor::from_point(layout, x, y); @@ -505,16 +514,6 @@ impl Selection { } } - /// Returns a new selection with the focus extended to the given cursor. - #[must_use] - pub fn extend_to_cursor(&self, focus: Cursor) -> Self { - Self { - anchor: self.anchor, - focus, - h_pos: None, - } - } - /// Returns a new selection with the focus moved to the next cluster in /// visual order. /// From eee4bf2156a75b5d28bec99069c606dde04a275c Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Wed, 11 Sep 2024 15:55:15 +0200 Subject: [PATCH 08/14] wip: clippy --- examples/vello_editor/src/text.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index 811c521f..6a1b1c82 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -406,7 +406,7 @@ impl Editor { &self.layout, preedit_start, Affinity::Downstream, - ) + ); } self.set_ime_cursor_area(window); From 62a469115e6c212ddc6f46a45f98856f77efa1c7 Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Wed, 11 Sep 2024 16:05:04 +0200 Subject: [PATCH 09/14] wip: block some events when composing --- examples/vello_editor/src/text.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index 6a1b1c82..c5b1b66f 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -204,6 +204,10 @@ impl Editor { } } + fn ime_is_composing(&self) -> bool { + self.preedit_range.is_some() + } + pub fn handle_event(&mut self, window: &Window, event: &WindowEvent) { match event { WindowEvent::Resized(size) => { @@ -236,12 +240,21 @@ impl Editor { if let PhysicalKey::Code(code) = event.physical_key { match code { KeyCode::KeyC if action_mod => { + if self.ime_is_composing() { + return; + } self.handle_clipboard(code); } KeyCode::KeyX if action_mod => { + if self.ime_is_composing() { + return; + } self.handle_clipboard(code); } KeyCode::KeyV if action_mod => { + if self.ime_is_composing() { + return; + } self.handle_clipboard(code); } KeyCode::ArrowLeft => { @@ -286,6 +299,11 @@ impl Editor { } } KeyCode::Delete => { + // The IME or Winit probably intercepts this event when composing, but + // I'm not sure. + if self.ime_is_composing() { + return; + } let start = if self.selection.is_collapsed() { let range = self.selection.focus().text_range(); let start = range.start; @@ -301,6 +319,11 @@ impl Editor { } } KeyCode::Backspace => { + // The IME or Winit probably intercepts this event when composing, but + // I'm not sure. + if self.ime_is_composing() { + return; + } let start = if self.selection.is_collapsed() { let end = self.selection.focus().text_range().start; if let Some((start, _)) = @@ -326,6 +349,9 @@ impl Editor { } } _ => { + if self.ime_is_composing() { + return; + } if let Some(text) = &event.text { let start = self .delete_current_selection() From 37d1ad0de8657a5cf9881dc7fb6430d923843e3f Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Fri, 13 Sep 2024 14:55:02 +0200 Subject: [PATCH 10/14] wip: bidi and affinity --- examples/vello_editor/src/text.rs | 93 +++++++++++++++++++++---------- 1 file changed, 63 insertions(+), 30 deletions(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index c5b1b66f..80866730 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -175,9 +175,11 @@ impl Editor { // Send that rectangle to the platform to suggest placement for the IME // candidate box. let mut union_rect = None; + + debug_assert!(!text_range.is_empty()); let preedit_selection = Selection::from_cursors( Cursor::from_index(&self.layout, text_range.start, Affinity::Downstream), - Cursor::from_index(&self.layout, text_range.end, Affinity::Downstream), + Cursor::from_index(&self.layout, text_range.end - 1, Affinity::Upstream), ); preedit_selection.geometry_with(&self.layout, |rect| { @@ -385,11 +387,21 @@ impl Editor { }; self.update_layout(self.width, 1.0); - self.selection = Selection::from_index( - &self.layout, - commit_start + text.len(), - Affinity::Downstream, - ); + + if text.is_empty() { + // is this case ever hit? + self.selection = Selection::from_index( + &self.layout, + commit_start, + Affinity::Downstream, + ); + } else { + self.selection = Selection::from_index( + &self.layout, + commit_start + text.len() - 1, + Affinity::Upstream, + ); + } } Ime::Preedit(text, compose_cursor) => { let preedit_start = if let Some(text_range) = self.preedit_range.take() { @@ -404,35 +416,56 @@ impl Editor { }; if text.is_empty() { - self.preedit_range = None; - } else { - self.preedit_range = Some(preedit_start..preedit_start + text.len()); - } - - self.update_layout(self.width, 1.0); + self.update_layout(self.width, 1.0); - if let Some(compose_cursor) = compose_cursor { - // Select the location indicated by the IME. - self.selection = Selection::from_cursors( - Cursor::from_index( + if preedit_start > 0 { + self.selection = Selection::from_index( &self.layout, - preedit_start + compose_cursor.0, - Affinity::Downstream, - ), - Cursor::from_index( + preedit_start - 1, + Affinity::Upstream, + ); + } else { + self.selection = Selection::from_index( &self.layout, - preedit_start + compose_cursor.1, + preedit_start, Affinity::Downstream, - ), - ); + ); + } } else { - // IME indicates nothing is to be selected: collapse the selection to a - // caret just in front of the preedit. - self.selection = Selection::from_index( - &self.layout, - preedit_start, - Affinity::Downstream, - ); + self.preedit_range = Some(preedit_start..preedit_start + text.len()); + self.update_layout(self.width, 1.0); + + if let Some(compose_cursor) = compose_cursor { + // Select the location indicated by the IME. + if compose_cursor.0 == compose_cursor.1 { + self.selection = Selection::from_index( + &self.layout, + preedit_start + compose_cursor.0, + Affinity::Downstream, + ) + } else { + self.selection = Selection::from_cursors( + Cursor::from_index( + &self.layout, + preedit_start + compose_cursor.0, + Affinity::Downstream, + ), + Cursor::from_index( + &self.layout, + preedit_start + compose_cursor.1 - 1, + Affinity::Upstream, + ), + ); + } + } else { + // IME indicates nothing is to be selected: collapse the selection to a + // caret just in front of the preedit. + self.selection = Selection::from_index( + &self.layout, + preedit_start, + Affinity::Downstream, + ); + } } self.set_ime_cursor_area(window); From cab9fc370513ec59d35d42ead70aa245ce7f0cbd Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Fri, 13 Sep 2024 14:55:36 +0200 Subject: [PATCH 11/14] wip: clippy --- examples/vello_editor/src/text.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index 80866730..a2b7ac38 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -442,7 +442,7 @@ impl Editor { &self.layout, preedit_start + compose_cursor.0, Affinity::Downstream, - ) + ); } else { self.selection = Selection::from_cursors( Cursor::from_index( From 0a83ab7ac7da06ff75cb2f2b9b972918035ab91c Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Fri, 4 Oct 2024 11:20:07 +0200 Subject: [PATCH 12/14] Prevent repeatedly sending duplicate IME cursor areas --- examples/vello_editor/src/text.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index a2b7ac38..6b413a35 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -404,6 +404,8 @@ impl Editor { } } Ime::Preedit(text, compose_cursor) => { + let old_selection = self.selection; + let preedit_start = if let Some(text_range) = self.preedit_range.take() { self.buffer.replace_range(text_range.clone(), text); text_range.start @@ -468,7 +470,9 @@ impl Editor { } } - self.set_ime_cursor_area(window); + if self.selection != old_selection { + self.set_ime_cursor_area(window); + } } Ime::Disabled => { if let Some(text_range) = self.preedit_range.take() { From b4f658da3b76d608f0a4ea1cd0b541f8141e1fb1 Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Sat, 5 Oct 2024 09:00:13 +0200 Subject: [PATCH 13/14] Mark must-use --- examples/vello_editor/src/text.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index 6b413a35..5a2f13a0 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -50,6 +50,7 @@ pub struct Editor { } /// Shrink the selection by the given amount of bytes by moving the focus towards the anchor. +#[must_use] fn shrink_selection(layout: &Layout, selection: Selection, bytes: usize) -> Selection { let mut selection = selection; let shrink = bytes.min(selection.text_range().len()); From b0d4fcf48a545bfa5048343521cbec5bdfbe08a2 Mon Sep 17 00:00:00 2001 From: Thomas Churchman Date: Sat, 5 Oct 2024 11:24:01 +0200 Subject: [PATCH 14/14] Send candidate area when IME is enabled, avoid sending duplicate areas --- examples/vello_editor/src/text.rs | 175 +++++++++++++++++++----------- 1 file changed, 113 insertions(+), 62 deletions(-) diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs index 5a2f13a0..cf2262ad 100644 --- a/examples/vello_editor/src/text.rs +++ b/examples/vello_editor/src/text.rs @@ -9,7 +9,7 @@ use parley::{layout::PositionedLayoutItem, FontContext}; use peniko::{kurbo::Affine, Color, Fill}; use std::ops::Range; use std::time::Instant; -use vello::kurbo::{Line, Stroke}; +use vello::kurbo::{Line, Rect, Stroke}; use vello::Scene; use winit::dpi::{LogicalPosition, LogicalSize}; use winit::event::Ime; @@ -31,6 +31,22 @@ pub enum ActiveText<'a> { Selection(&'a str), } +/// The input method state of the text editor. +/// +/// If `Enabled` or `Preedit`, the editor should send appropriate +/// [IME candidate box areas](`Window::set_ime_cursor_area`) to the platform. +#[derive(Default, PartialEq, Eq)] +enum ImeState { + /// The IME is disabled. + #[default] + Disabled, + /// The IME is enabled. + Enabled, + /// The IME is enabled and is composing text. The range is non-empty, encoding the byte offsets + /// of the text currently marked as preedit by the IME. + Preedit(Range), +} + #[derive(Default)] pub struct Editor { font_cx: FontContext, @@ -38,8 +54,10 @@ pub struct Editor { buffer: String, layout: Layout, selection: Selection, - /// The portion of the text currently marked as preedit by the IME. - preedit_range: Option>, + ime: ImeState, + /// The IME candidate area last sent to the platform (through [`Self::set_ime_cursor_area`]). + /// Cached to avoid repeatedly sending unchanged areas. + last_sent_ime_candidate_area: Rect, cursor_mode: VisualMode, last_click_time: Option, click_count: u32, @@ -90,7 +108,7 @@ impl Editor { builder.push_default(&parley::style::StyleProperty::FontStack( parley::style::FontStack::Source("system-ui"), )); - if let Some(ref text_range) = self.preedit_range { + if let ImeState::Preedit(ref text_range) = self.ime { builder.push( &parley::style::StyleProperty::UnderlineBrush(Some(Color::SPRING_GREEN)), text_range.clone(), @@ -170,37 +188,45 @@ impl Editor { } /// Suggest an area for IME candidate box placement based on the current IME state. - fn set_ime_cursor_area(&self, window: &Window) { - if let Some(ref text_range) = self.preedit_range { - // Find the smallest rectangle that contains the entire preedit text. - // Send that rectangle to the platform to suggest placement for the IME - // candidate box. - let mut union_rect = None; - - debug_assert!(!text_range.is_empty()); - let preedit_selection = Selection::from_cursors( - Cursor::from_index(&self.layout, text_range.start, Affinity::Downstream), - Cursor::from_index(&self.layout, text_range.end - 1, Affinity::Upstream), - ); + fn set_ime_cursor_area(&mut self, window: &Window) { + let selection = match self.ime { + ImeState::Preedit(ref text_range) => { + debug_assert!(!text_range.is_empty()); + Selection::from_cursors( + Cursor::from_index(&self.layout, text_range.start, Affinity::Downstream), + Cursor::from_index(&self.layout, text_range.end - 1, Affinity::Upstream), + ) + } + _ => self.selection, + }; - preedit_selection.geometry_with(&self.layout, |rect| { + let area = if selection.is_collapsed() { + selection.focus().strong_geometry(&self.layout, 1.5) + } else { + // Find the smallest rectangle that contains the entire selection. + let mut union_rect = None; + selection.geometry_with(&self.layout, |rect| { if union_rect.is_none() { union_rect = Some(rect); } union_rect = Some(union_rect.unwrap().union(rect)); }); - if let Some(union_rect) = union_rect { + union_rect + }; + + if let Some(area) = area { + if area != self.last_sent_ime_candidate_area { + self.last_sent_ime_candidate_area = area; + window.set_ime_cursor_area( - LogicalPosition::new(union_rect.x0, union_rect.y0), + LogicalPosition::new(area.x0, area.y0), LogicalSize::new( - union_rect.width(), - // TODO: an offset is added here to prevent the IME - // candidate box from overlapping with the IME cursor. From - // the Winit docs I would've expected the IME candidate box - // not to overlap the indicated IME cursor area, but for - // some reason it does (tested using fcitx5 - // on wayland) - union_rect.height() + 40.0, + area.width(), + // TODO: an offset is added here to prevent the IME candidate box from + // overlapping with the IME cursor. From the Winit docs I would've expected the + // IME candidate box not to overlap the indicated IME cursor area, but for some + // reason it does (tested using fcitx5 on wayland) + area.height() + 40.0, ), ); } @@ -208,7 +234,7 @@ impl Editor { } fn ime_is_composing(&self) -> bool { - self.preedit_range.is_some() + matches!(&self.ime, ImeState::Preedit(_)) } pub fn handle_event(&mut self, window: &Window, event: &WindowEvent) { @@ -216,7 +242,6 @@ impl Editor { WindowEvent::Resized(size) => { self.update_layout(size.width as f32, 1.0); self.selection = self.selection.refresh(&self.layout); - self.set_ime_cursor_area(window); } WindowEvent::ModifiersChanged(modifiers) => { self.modifiers = Some(*modifiers); @@ -375,8 +400,13 @@ impl Editor { } WindowEvent::Ime(ime) => { match ime { - Ime::Enabled => {} + Ime::Enabled => { + debug_assert!(self.ime == ImeState::Disabled); + self.ime = ImeState::Enabled; + } Ime::Commit(text) => { + debug_assert!(self.ime != ImeState::Disabled); + let commit_start = if self.selection.is_collapsed() { let start = self.selection.insertion_index(); self.buffer.insert_str(start, text); @@ -405,20 +435,24 @@ impl Editor { } } Ime::Preedit(text, compose_cursor) => { - let old_selection = self.selection; + debug_assert!(self.ime != ImeState::Disabled); - let preedit_start = if let Some(text_range) = self.preedit_range.take() { - self.buffer.replace_range(text_range.clone(), text); - text_range.start - } else { - let insertion_idx = self - .delete_current_selection() - .unwrap_or(self.selection.insertion_index()); - self.buffer.insert_str(insertion_idx, text); - insertion_idx + let preedit_start = match self.ime { + ImeState::Preedit(ref text_range) => { + self.buffer.replace_range(text_range.clone(), text); + text_range.start + } + _ => { + let insertion_idx = self + .delete_current_selection() + .unwrap_or(self.selection.insertion_index()); + self.buffer.insert_str(insertion_idx, text); + insertion_idx + } }; if text.is_empty() { + self.ime = ImeState::Enabled; self.update_layout(self.width, 1.0); if preedit_start > 0 { @@ -435,7 +469,7 @@ impl Editor { ); } } else { - self.preedit_range = Some(preedit_start..preedit_start + text.len()); + self.ime = ImeState::Preedit(preedit_start..preedit_start + text.len()); self.update_layout(self.width, 1.0); if let Some(compose_cursor) = compose_cursor { @@ -470,27 +504,30 @@ impl Editor { ); } } - - if self.selection != old_selection { - self.set_ime_cursor_area(window); - } } Ime::Disabled => { - if let Some(text_range) = self.preedit_range.take() { - self.buffer.replace_range(text_range.clone(), ""); - self.update_layout(self.width, 1.0); + debug_assert!(matches!(self.ime, ImeState::Enabled | ImeState::Preedit(_))); + + self.last_sent_ime_candidate_area = Rect::default(); + if let ImeState::Preedit(text_range) = + std::mem::replace(&mut self.ime, ImeState::Disabled) + { + if !text_range.is_empty() { + self.buffer.replace_range(text_range.clone(), ""); + self.update_layout(self.width, 1.0); - // Invariant: the selection anchor and start of preedit text are at the same - // position. - // If the focus extends into the preedit range, shrink the selection. - if self.selection.focus().text_range().start > text_range.start { - self.selection = shrink_selection( - &self.layout, - self.selection, - text_range.len(), - ); - } else { - self.selection = self.selection.refresh(&self.layout); + // Invariant: the selection anchor and start of preedit text are at the same + // position. + // If the focus extends into the preedit range, shrink the selection. + if self.selection.focus().text_range().start > text_range.start { + self.selection = shrink_selection( + &self.layout, + self.selection, + text_range.len(), + ); + } else { + self.selection = self.selection.refresh(&self.layout); + } } } } @@ -558,7 +595,7 @@ impl Editor { _ => {} } - if let Some(ref text_range) = self.preedit_range { + if let ImeState::Preedit(ref text_range) = self.ime { if text_range.start != self.selection.anchor().text_range().start { // If the selection anchor is no longer at the same position as the preedit text, the // selection has been moved. Move the preedit to the selection's new anchor position. @@ -583,16 +620,30 @@ impl Editor { let insertion_index = self.selection.insertion_index(); self.buffer.insert_str(insertion_index, &text); - self.preedit_range = Some(insertion_index..insertion_index + text.len()); + self.ime = ImeState::Preedit(insertion_index..insertion_index + text.len()); // TODO: events that caused the preedit to be moved may also have updated the // layout, in that case we're now updating twice. self.update_layout(self.width, 1.0); - self.set_ime_cursor_area(window); self.selection = self.selection.refresh(&self.layout); } } + + if self.ime != ImeState::Disabled + && !matches!( + event, + WindowEvent::RedrawRequested | WindowEvent::CursorMoved { .. } + ) + { + // TODO: this is a bit overzealous in recalculating the IME cursor area: there are + // cases where it can cheaply be known the area is unchanged. (If the calculated area + // is unchanged from the previous one sent, it is not re-sent to the platform, though). + // + // Ideally this is called only when the layout, selection, or preedit has changed, or + // if the IME has just been enabled. + self.set_ime_cursor_area(window); + } } fn delete_current_selection(&mut self) -> Option {