From fddd06508e0d0a704735585748de909749c571a9 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Thu, 3 Oct 2024 12:20:45 +0200 Subject: [PATCH 1/5] Remove obsolete types. --- masonry/src/contexts.rs | 11 ----------- masonry/src/text_helpers.rs | 24 ------------------------ 2 files changed, 35 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 2455a5a82..c18d397ba 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -14,7 +14,6 @@ use crate::action::Action; use crate::passes::layout::run_layout_on; use crate::render_root::{MutateCallback, RenderRootSignal, RenderRootState}; use crate::text::TextBrush; -use crate::text_helpers::ImeChangeSignal; use crate::tree_arena::{ArenaMutChildren, ArenaRefChildren}; use crate::widget::{WidgetMut, WidgetRef, WidgetState}; use crate::{ @@ -550,16 +549,6 @@ impl_context_method!(MutateCtx<'_>, EventCtx<'_>, LifeCycleCtx<'_>, { self.widget_state.needs_update_disabled = true; self.widget_state.is_explicitly_disabled = disabled; } - - #[allow(unused)] - /// Indicate that text input state has changed. - /// - /// A widget that accepts text input should call this anytime input state - /// (such as the text or the selection) changes as a result of a non text-input - /// event. - pub fn invalidate_text_input(&mut self, event: ImeChangeSignal) { - todo!("invalidate_text_input"); - } }); // --- MARK: OTHER METHODS --- diff --git a/masonry/src/text_helpers.rs b/masonry/src/text_helpers.rs index 9a93805f1..5cf90b68d 100644 --- a/masonry/src/text_helpers.rs +++ b/masonry/src/text_helpers.rs @@ -18,30 +18,6 @@ use crate::text::TextBrush; /// it cannot be mutated, but unlike `String` it can be cheaply cloned. pub type ArcStr = std::sync::Arc; -// Copy-pasted from druid_shell -/// An event representing an application-initiated change in [`InputHandler`] -/// state. -/// -/// When we change state that may have previously been retrieved from an -/// [`InputHandler`], we notify the platform so that it can invalidate any -/// data if necessary. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -#[non_exhaustive] -pub enum ImeChangeSignal { - /// Indicates the value returned by `InputHandler::selection` may have changed. - SelectionChanged, - - /// Indicates the values returned by one or more of these methods may have changed: - /// - `InputHandler::hit_test_point` - /// - `InputHandler::line_range` - /// - `InputHandler::bounding_box` - /// - `InputHandler::slice_bounding_box` - LayoutChanged, - - /// Indicates any value returned from any `InputHandler` method may have changed. - Reset, -} - /// A function that renders laid out glyphs to a [Scene]. pub fn render_text( scene: &mut Scene, From 503e2690d7fd14e47e3998eaf851f866c333deb5 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Thu, 3 Oct 2024 12:25:55 +0200 Subject: [PATCH 2/5] Remove text_helpers module --- masonry/examples/custom_widget.rs | 2 +- masonry/src/lib.rs | 4 +--- masonry/src/text/layout.rs | 4 +++- masonry/src/text/mod.rs | 11 +++++++++++ masonry/src/{text_helpers.rs => text/render_text.rs} | 6 ------ 5 files changed, 16 insertions(+), 11 deletions(-) rename masonry/src/{text_helpers.rs => text/render_text.rs} (96%) diff --git a/masonry/examples/custom_widget.rs b/masonry/examples/custom_widget.rs index 3f1959d36..cacc0b25f 100644 --- a/masonry/examples/custom_widget.rs +++ b/masonry/examples/custom_widget.rs @@ -114,7 +114,7 @@ impl Widget for CustomWidget { let mut scratch_scene = Scene::new(); // We can pass a transform matrix to rotate the text we render - masonry::text_helpers::render_text( + masonry::text::render_text( scene, &mut scratch_scene, Affine::rotate(std::f64::consts::FRAC_PI_4).then_translate((80.0, 40.0).into()), diff --git a/masonry/src/lib.rs b/masonry/src/lib.rs index d0a72b19d..c248bd37b 100644 --- a/masonry/src/lib.rs +++ b/masonry/src/lib.rs @@ -119,8 +119,6 @@ mod event; pub mod paint_scene_helpers; pub mod render_root; pub mod testing; -// mod text; -pub mod text_helpers; pub mod theme; pub mod widget; @@ -151,4 +149,4 @@ pub use vello::peniko::{Color, Gradient}; pub use widget::widget::{AllowRawMut, Widget, WidgetId}; pub use widget::{WidgetPod, WidgetState}; -pub use text_helpers::ArcStr; +pub use text::ArcStr; diff --git a/masonry/src/text/layout.rs b/masonry/src/text/layout.rs index 9355a9c15..07195c864 100644 --- a/masonry/src/text/layout.rs +++ b/masonry/src/text/layout.rs @@ -14,6 +14,8 @@ use vello::kurbo::{Affine, Line, Point, Rect, Size}; use vello::peniko::{self, Color, Gradient}; use vello::Scene; +use crate::text::render_text; + /// A component for displaying text on screen. /// /// This is a type intended to be used by other widgets that display text. @@ -494,7 +496,7 @@ impl + Eq> TextLayout { self.assert_rebuilt("draw"); // TODO: This translation doesn't seem great let p: Point = point.into(); - crate::text_helpers::render_text( + render_text( scene, &mut self.scratch_scene, Affine::translate((p.x, p.y)), diff --git a/masonry/src/text/mod.rs b/masonry/src/text/mod.rs index 129a196eb..e2d765326 100644 --- a/masonry/src/text/mod.rs +++ b/masonry/src/text/mod.rs @@ -23,3 +23,14 @@ pub use edit::TextEditor; mod backspace; pub use backspace::offset_for_delete_backwards; + +mod render_text; +pub use render_text::render_text; + +// --- + +/// A reference counted string slice. +/// +/// This is a data-friendly way to represent strings in Masonry. Unlike `String` +/// it cannot be mutated, but unlike `String` it can be cheaply cloned. +pub type ArcStr = std::sync::Arc; diff --git a/masonry/src/text_helpers.rs b/masonry/src/text/render_text.rs similarity index 96% rename from masonry/src/text_helpers.rs rename to masonry/src/text/render_text.rs index 5cf90b68d..8cf2f2d58 100644 --- a/masonry/src/text_helpers.rs +++ b/masonry/src/text/render_text.rs @@ -12,12 +12,6 @@ use vello::{ use crate::text::TextBrush; -/// A reference counted string slice. -/// -/// This is a data-friendly way to represent strings in Masonry. Unlike `String` -/// it cannot be mutated, but unlike `String` it can be cheaply cloned. -pub type ArcStr = std::sync::Arc; - /// A function that renders laid out glyphs to a [Scene]. pub fn render_text( scene: &mut Scene, From c9982f597a4e65593bb761bbebe848bce601e253 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Thu, 3 Oct 2024 12:30:45 +0200 Subject: [PATCH 3/5] Rename files and reorder imports --- masonry/src/text/mod.rs | 19 +++++++------------ .../src/text/{layout.rs => text_layout.rs} | 0 2 files changed, 7 insertions(+), 12 deletions(-) rename masonry/src/text/{layout.rs => text_layout.rs} (100%) diff --git a/masonry/src/text/mod.rs b/masonry/src/text/mod.rs index e2d765326..af5f44d91 100644 --- a/masonry/src/text/mod.rs +++ b/masonry/src/text/mod.rs @@ -10,22 +10,17 @@ //! //! All of these have the same set of global styling options, and can contain rich text -mod layout; -pub use layout::{Hinting, LayoutMetrics, TextBrush, TextLayout}; - -mod selection; -pub use selection::{len_utf8_from_first_byte, Selectable, StringCursor, TextWithSelection}; - -// mod movement; - +mod backspace; mod edit; -pub use edit::TextEditor; +mod render_text; +mod selection; +mod text_layout; -mod backspace; pub use backspace::offset_for_delete_backwards; - -mod render_text; +pub use edit::TextEditor; pub use render_text::render_text; +pub use selection::{len_utf8_from_first_byte, Selectable, StringCursor, TextWithSelection}; +pub use text_layout::{Hinting, LayoutMetrics, TextBrush, TextLayout}; // --- diff --git a/masonry/src/text/layout.rs b/masonry/src/text/text_layout.rs similarity index 100% rename from masonry/src/text/layout.rs rename to masonry/src/text/text_layout.rs From 068dd79622ff572f4672c1d8e5f0544c72f2b373 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Thu, 3 Oct 2024 13:30:32 +0200 Subject: [PATCH 4/5] Remove type parameter from text layout --- masonry/src/text/selection.rs | 38 ++++++++++--- masonry/src/text/text_layout.rs | 82 +++++++++++----------------- masonry/src/widget/label.rs | 35 +++++++----- masonry/src/widget/progress_bar.rs | 22 ++++---- masonry/src/widget/prose.rs | 5 +- masonry/src/widget/textbox.rs | 5 +- masonry/src/widget/variable_label.rs | 37 +++++++++---- 7 files changed, 127 insertions(+), 97 deletions(-) diff --git a/masonry/src/text/selection.rs b/masonry/src/text/selection.rs index 1915ea521..78877bef2 100644 --- a/masonry/src/text/selection.rs +++ b/masonry/src/text/selection.rs @@ -21,7 +21,9 @@ use crate::{Handled, TextEvent}; use super::{TextBrush, TextLayout}; pub struct TextWithSelection { - pub layout: TextLayout, + text: T, + text_changed: bool, + pub layout: TextLayout, /// The current selection within this widget // TODO: Allow multiple selections (i.e. by holding down control) pub selection: Option, @@ -35,7 +37,9 @@ pub struct TextWithSelection { impl TextWithSelection { pub fn new(text: T, text_size: f32) -> Self { Self { - layout: TextLayout::new(text, text_size), + text, + text_changed: false, + layout: TextLayout::new(text_size), selection: None, needs_selection_update: false, selecting_with_mouse: false, @@ -48,14 +52,24 @@ impl TextWithSelection { } } + pub fn text(&self) -> &T { + &self.text + } + + pub fn text_mut(&mut self) -> &mut T { + self.text_changed = true; + &mut self.text + } + pub fn set_text(&mut self, text: T) { + self.text_changed = true; self.selection = None; self.needs_selection_update = true; - self.layout.set_text(text); + self.text = text; } pub fn needs_rebuild(&self) -> bool { - self.layout.needs_rebuild() || self.needs_selection_update + self.layout.needs_rebuild() || self.needs_selection_update || self.text_changed } pub fn pointer_down( @@ -245,10 +259,14 @@ impl TextWithSelection { ) { // In theory, we could be clever here and only rebuild the layout if the // selected range was previously or currently non-zero size (i.e. there is a selected range) - if self.needs_selection_update || self.layout.needs_rebuild() { + if self.needs_selection_update || self.layout.needs_rebuild() || self.text_changed { self.layout.invalidate(); - self.layout - .rebuild_with_attributes(font_ctx, layout_ctx, |mut builder| { + self.layout.rebuild_with_attributes( + font_ctx, + layout_ctx, + self.text.as_ref(), + self.text_changed, + |mut builder| { if let Some(selection) = self.selection { let range = selection.range(); if !range.is_empty() { @@ -259,8 +277,10 @@ impl TextWithSelection { } } attributes(builder) - }); + }, + ); self.needs_selection_update = false; + self.text_changed = false; } } @@ -301,7 +321,7 @@ fn shortcut_key(key: &winit::event::KeyEvent) -> winit::keyboard::Key { } impl Deref for TextWithSelection { - type Target = TextLayout; + type Target = TextLayout; fn deref(&self) -> &Self::Target { &self.layout diff --git a/masonry/src/text/text_layout.rs b/masonry/src/text/text_layout.rs index 07195c864..55a0466ee 100644 --- a/masonry/src/text/text_layout.rs +++ b/masonry/src/text/text_layout.rs @@ -37,8 +37,7 @@ use crate::text::render_text; /// /// TODO: Update docs to mentionParley #[derive(Clone)] -pub struct TextLayout { - text: T, +pub struct TextLayout { // TODO: Find a way to let this use borrowed data scale: f32, @@ -57,6 +56,8 @@ pub struct TextLayout { needs_line_breaks: bool, layout: Layout, scratch_scene: Scene, + // TODO - Add field to check whether text has changed since last layout + // #[cfg(debug_assertions)] last_text_start: String, } /// Whether a section of text should be hinted. @@ -140,11 +141,10 @@ pub struct LayoutMetrics { //TODO: add inking_rect } -impl TextLayout { +impl TextLayout { /// Create a new `TextLayout` object. - pub fn new(text: T, text_size: f32) -> Self { + pub fn new(text_size: f32) -> Self { TextLayout { - text, scale: 1.0, brush: crate::theme::TEXT_COLOR.into(), @@ -264,50 +264,22 @@ impl TextLayout { pub fn needs_rebuild(&self) -> bool { self.needs_layout || self.needs_line_breaks } - - // TODO: What are the valid use cases for this, where we shouldn't use a run-specific check instead? - // /// Returns `true` if this layout's text appears to be right-to-left. - // /// - // /// See [`piet::util::first_strong_rtl`] for more information. - // /// - // /// [`piet::util::first_strong_rtl`]: crate::piet::util::first_strong_rtl - // pub fn text_is_rtl(&self) -> bool { - // self.text_is_rtl - // } } -impl + Eq> TextLayout { +impl TextLayout { #[track_caller] fn assert_rebuilt(&self, method: &str) { if self.needs_layout || self.needs_line_breaks { - debug_panic!( - "TextLayout::{method} called without rebuilding layout object. Text was '{}'", - self.text.as_ref().chars().take(250).collect::() - ); - } - } - - /// Set the text to display. - pub fn set_text(&mut self, text: T) { - if self.text != text { - self.text = text; - self.invalidate(); + if cfg!(debug_assertions) { + // TODO - Include self.last_text_start + #[cfg(debug_assertions)] + panic!("TextLayout::{method} called without rebuilding layout object."); + } else { + tracing::error!("TextLayout::{method} called without rebuilding layout object.",); + }; } } - /// Returns the string backing this layout, if it exists. - pub fn text(&self) -> &T { - &self.text - } - - /// Returns the string backing this layout, if it exists. - /// - /// Invalidates the layout and so should only be used when definitely applying an edit - pub fn text_mut(&mut self) -> &mut T { - self.invalidate(); - &mut self.text - } - /// Returns the inner Parley [`Layout`] value. pub fn layout(&self) -> &Layout { self.assert_rebuilt("layout"); @@ -431,18 +403,23 @@ impl + Eq> TextLayout { /// Rebuild the inner layout as needed. /// /// This `TextLayout` object manages a lower-level layout object that may - /// need to be rebuilt in response to changes to the text or attributes - /// like the font. + /// need to be rebuilt in response to changes to text attributes like the font. /// /// This method should be called whenever any of these things may have changed. /// A simple way to ensure this is correct is to always call this method /// as part of your widget's [`layout`][crate::Widget::layout] method. + /// + /// The `text_changed` parameter should be set to `true` if the text changed since + /// the last rebuild. Always setting it to true may lead to redundant work, wrongly + /// setting it to false may lead to invalidation bugs. pub fn rebuild( &mut self, font_ctx: &mut FontContext, layout_ctx: &mut LayoutContext, + text: &str, + text_changed: bool, ) { - self.rebuild_with_attributes(font_ctx, layout_ctx, |builder| builder); + self.rebuild_with_attributes(font_ctx, layout_ctx, text, text_changed, |builder| builder); } /// Rebuild the inner layout as needed, adding attributes to the underlying layout. @@ -452,14 +429,18 @@ impl + Eq> TextLayout { &mut self, font_ctx: &mut FontContext, layout_ctx: &mut LayoutContext, + text: &str, + text_changed: bool, attributes: impl for<'b> FnOnce( RangedBuilder<'b, TextBrush, &'b str>, ) -> RangedBuilder<'b, TextBrush, &'b str>, ) { - if self.needs_layout { + // TODO - check against self.last_text_start + + if self.needs_layout || text_changed { self.needs_layout = false; - let mut builder = layout_ctx.ranged_builder(font_ctx, self.text.as_ref(), self.scale); + let mut builder = layout_ctx.ranged_builder(font_ctx, text, self.scale); builder.push_default(&StyleProperty::Brush(self.brush.clone())); builder.push_default(&StyleProperty::FontSize(self.text_size)); builder.push_default(&StyleProperty::FontStack(self.font)); @@ -474,7 +455,7 @@ impl + Eq> TextLayout { self.needs_line_breaks = true; } - if self.needs_line_breaks { + if self.needs_line_breaks || text_changed { self.needs_line_breaks = false; self.layout .break_all_lines(self.max_advance, self.alignment); @@ -505,10 +486,9 @@ impl + Eq> TextLayout { } } -impl + Eq> std::fmt::Debug for TextLayout { +impl std::fmt::Debug for TextLayout { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { f.debug_struct("TextLayout") - .field("text", &self.text.as_ref()) .field("scale", &self.scale) .field("brush", &self.brush) .field("font", &self.font) @@ -525,8 +505,8 @@ impl + Eq> std::fmt::Debug for TextLayout { } } -impl + Eq + Default> Default for TextLayout { +impl Default for TextLayout { fn default() -> Self { - Self::new(Default::default(), crate::theme::TEXT_SIZE_NORMAL as f32) + Self::new(crate::theme::TEXT_SIZE_NORMAL as f32) } } diff --git a/masonry/src/widget/label.rs b/masonry/src/widget/label.rs index 400d2c985..4f53a074c 100644 --- a/masonry/src/widget/label.rs +++ b/masonry/src/widget/label.rs @@ -35,11 +35,9 @@ pub enum LineBreaking { /// A widget displaying non-editable text. pub struct Label { - // We hardcode the underlying storage type as `ArcStr` for `Label` - // More advanced use cases will almost certainly need a custom widget, anyway - // (Rich text is not yet fully integrated, and so the architecture by which a label - // has rich text properties specified still needs to be designed) - text_layout: TextLayout, + text: ArcStr, + text_changed: bool, + text_layout: TextLayout, line_break_mode: LineBreaking, show_disabled: bool, brush: TextBrush, @@ -51,7 +49,9 @@ impl Label { /// Create a new label. pub fn new(text: impl Into) -> Self { Self { - text_layout: TextLayout::new(text.into(), crate::theme::TEXT_SIZE_NORMAL as f32), + text: text.into(), + text_changed: false, + text_layout: TextLayout::new(crate::theme::TEXT_SIZE_NORMAL as f32), line_break_mode: LineBreaking::Overflow, show_disabled: true, brush: crate::theme::TEXT_COLOR.into(), @@ -67,7 +67,7 @@ impl Label { } pub fn text(&self) -> &ArcStr { - self.text_layout.text() + &self.text } #[doc(alias = "with_text_color")] @@ -109,10 +109,10 @@ impl Label { // --- MARK: WIDGETMUT --- impl WidgetMut<'_, Label> { pub fn text(&self) -> &ArcStr { - self.widget.text_layout.text() + &self.widget.text } - pub fn set_text_properties(&mut self, f: impl FnOnce(&mut TextLayout) -> R) -> R { + pub fn set_text_properties(&mut self, f: impl FnOnce(&mut TextLayout) -> R) -> R { let ret = f(&mut self.widget.text_layout); if self.widget.text_layout.needs_rebuild() { self.ctx.request_layout(); @@ -122,7 +122,9 @@ impl WidgetMut<'_, Label> { pub fn set_text(&mut self, new_text: impl Into) { let new_text = new_text.into(); - self.set_text_properties(|layout| layout.set_text(new_text)); + self.widget.text = new_text; + self.widget.text_changed = true; + self.ctx.request_layout(); } #[doc(alias = "set_text_color")] @@ -221,9 +223,11 @@ impl Widget for Label { None }; self.text_layout.set_max_advance(max_advance); - if self.text_layout.needs_rebuild() { + if self.text_layout.needs_rebuild() || self.text_changed { let (font_ctx, layout_ctx) = ctx.text_contexts(); - self.text_layout.rebuild(font_ctx, layout_ctx); + self.text_layout + .rebuild(font_ctx, layout_ctx, &self.text, self.text_changed); + self.text_changed = false; } // We ignore trailing whitespace for a label let text_size = self.text_layout.size(); @@ -243,7 +247,10 @@ impl Widget for Label { fn paint(&mut self, ctx: &mut PaintCtx, scene: &mut Scene) { if self.text_layout.needs_rebuild() { - debug_panic!("Called Label paint before layout"); + debug_panic!( + "Called {name}::paint with invalid layout", + name = self.short_type_name() + ); } if self.line_break_mode == LineBreaking::Clip { let clip_rect = ctx.size().to_rect(); @@ -278,7 +285,7 @@ impl Widget for Label { } fn get_debug_text(&self) -> Option { - Some(self.text_layout.text().as_ref().to_string()) + Some(self.text.to_string()) } } diff --git a/masonry/src/widget/progress_bar.rs b/masonry/src/widget/progress_bar.rs index fec22c077..5d1ab7cbd 100644 --- a/masonry/src/widget/progress_bar.rs +++ b/masonry/src/widget/progress_bar.rs @@ -25,7 +25,8 @@ pub struct ProgressBar { /// `None` variant can be used to show a progress bar without a percentage. /// It is also used if an invalid float (outside of [0, 1]) is passed. progress: Option, - label: TextLayout, + progress_changed: bool, + label: TextLayout, } impl ProgressBar { @@ -43,7 +44,8 @@ impl ProgressBar { fn new_indefinite() -> Self { Self { progress: None, - label: TextLayout::new("".into(), crate::theme::TEXT_SIZE_NORMAL as f32), + progress_changed: false, + label: TextLayout::new(crate::theme::TEXT_SIZE_NORMAL as f32), } } @@ -52,15 +54,10 @@ impl ProgressBar { // check to see if we can avoid doing work if self.progress != progress { self.progress = progress; - self.update_text(); + self.progress_changed = true; } } - /// Updates the text layout with the current part-complete value - fn update_text(&mut self) { - self.label.set_text(self.value()); - } - fn value(&self) -> ArcStr { if let Some(value) = self.progress { format!("{:.0}%", value * 100.).into() @@ -123,7 +120,9 @@ impl Widget for ProgressBar { if self.label.needs_rebuild() { let (font_ctx, layout_ctx) = ctx.text_contexts(); - self.label.rebuild(font_ctx, layout_ctx); + self.label + .rebuild(font_ctx, layout_ctx, &self.value(), self.progress_changed); + self.progress_changed = false; } let label_size = self.label.size(); @@ -140,7 +139,10 @@ impl Widget for ProgressBar { let border_width = 1.; if self.label.needs_rebuild() { - debug_panic!("Called ProgressBar paint before layout"); + debug_panic!( + "Called {name}::paint with invalid layout", + name = self.short_type_name() + ); } let rect = ctx diff --git a/masonry/src/widget/prose.rs b/masonry/src/widget/prose.rs index a44265ab3..30a23f05f 100644 --- a/masonry/src/widget/prose.rs +++ b/masonry/src/widget/prose.rs @@ -266,7 +266,10 @@ impl Widget for Prose { fn paint(&mut self, ctx: &mut PaintCtx, scene: &mut Scene) { if self.text_layout.needs_rebuild() { - debug_panic!("Called Label paint before layout"); + debug_panic!( + "Called {name}::paint with invalid layout", + name = self.short_type_name() + ); } if self.line_break_mode == LineBreaking::Clip { let clip_rect = ctx.size().to_rect(); diff --git a/masonry/src/widget/textbox.rs b/masonry/src/widget/textbox.rs index b2c8e1a91..d55e2a34c 100644 --- a/masonry/src/widget/textbox.rs +++ b/masonry/src/widget/textbox.rs @@ -306,7 +306,10 @@ impl Widget for Textbox { fn paint(&mut self, ctx: &mut PaintCtx, scene: &mut Scene) { if self.editor.needs_rebuild() { - debug_panic!("Called Label paint before layout"); + debug_panic!( + "Called {name}::paint with invalid layout", + name = self.short_type_name() + ); } if self.line_break_mode == LineBreaking::Clip { let clip_rect = ctx.size().to_rect(); diff --git a/masonry/src/widget/variable_label.rs b/masonry/src/widget/variable_label.rs index baf963bc1..3cb1cba8e 100644 --- a/masonry/src/widget/variable_label.rs +++ b/masonry/src/widget/variable_label.rs @@ -136,7 +136,9 @@ impl AnimationStatus { // TODO: Make this a wrapper (around `Label`?) /// A widget displaying non-editable text, with a variable [weight](parley::style::FontWeight). pub struct VariableLabel { - text_layout: TextLayout, + text: ArcStr, + text_changed: bool, + text_layout: TextLayout, line_break_mode: LineBreaking, show_disabled: bool, brush: TextBrush, @@ -148,7 +150,9 @@ impl VariableLabel { /// Create a new label. pub fn new(text: impl Into) -> Self { Self { - text_layout: TextLayout::new(text.into(), crate::theme::TEXT_SIZE_NORMAL as f32), + text: text.into(), + text_changed: false, + text_layout: TextLayout::new(crate::theme::TEXT_SIZE_NORMAL as f32), line_break_mode: LineBreaking::Overflow, show_disabled: true, brush: crate::theme::TEXT_COLOR.into(), @@ -157,7 +161,7 @@ impl VariableLabel { } pub fn text(&self) -> &ArcStr { - self.text_layout.text() + &self.text } #[doc(alias = "with_text_color")] @@ -218,13 +222,13 @@ impl VariableLabel { impl WidgetMut<'_, VariableLabel> { /// Read the text. pub fn text(&self) -> &ArcStr { - self.widget.text_layout.text() + &self.widget.text } /// Set a property on the underlying text. /// /// This cannot be used to set attributes. - pub fn set_text_properties(&mut self, f: impl FnOnce(&mut TextLayout) -> R) -> R { + pub fn set_text_properties(&mut self, f: impl FnOnce(&mut TextLayout) -> R) -> R { let ret = f(&mut self.widget.text_layout); if self.widget.text_layout.needs_rebuild() { self.ctx.request_layout(); @@ -235,7 +239,9 @@ impl WidgetMut<'_, VariableLabel> { /// Modify the underlying text. pub fn set_text(&mut self, new_text: impl Into) { let new_text = new_text.into(); - self.set_text_properties(|layout| layout.set_text(new_text)); + self.widget.text = new_text; + self.widget.text_changed = true; + self.ctx.request_layout(); } #[doc(alias = "set_text_color")] @@ -358,8 +364,12 @@ impl Widget for VariableLabel { self.text_layout .set_brush(self.brush(ctx.widget_state.is_disabled)); let (font_ctx, layout_ctx) = ctx.text_contexts(); - self.text_layout - .rebuild_with_attributes(font_ctx, layout_ctx, |mut builder| { + self.text_layout.rebuild_with_attributes( + font_ctx, + layout_ctx, + &self.text, + self.text_changed, + |mut builder| { builder.push_default(&parley::style::StyleProperty::FontWeight(Weight::new( self.weight.value, ))); @@ -367,7 +377,9 @@ impl Widget for VariableLabel { // parley::style::FontSettings::List(&[]), // )); builder - }); + }, + ); + self.text_changed = false; } // We ignore trailing whitespace for a label let text_size = self.text_layout.size(); @@ -387,7 +399,10 @@ impl Widget for VariableLabel { fn paint(&mut self, ctx: &mut PaintCtx, scene: &mut Scene) { if self.text_layout.needs_rebuild() { - debug_panic!("Called Label paint before layout"); + debug_panic!( + "Called {name}::paint with invalid layout", + name = self.short_type_name() + ); } if self.line_break_mode == LineBreaking::Clip { let clip_rect = ctx.size().to_rect(); @@ -418,7 +433,7 @@ impl Widget for VariableLabel { } fn get_debug_text(&self) -> Option { - Some(self.text_layout.text().as_ref().to_string()) + Some(self.text.to_string()) } } From d3346e0b4a2f983da24cec1ad09a4dbe840e82db Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Thu, 3 Oct 2024 15:35:08 +0200 Subject: [PATCH 5/5] TMP: Switch to new Parley version Note: cursor handling is pretty buggy in this one. --- Cargo.lock | 218 +----------------------------- Cargo.toml | 2 +- masonry/examples/custom_widget.rs | 5 +- masonry/src/text/edit.rs | 56 ++++---- masonry/src/text/movement.rs | 186 +++++++++++++++++++++++++ masonry/src/text/render_text.rs | 14 +- masonry/src/text/selection.rs | 86 ++++-------- masonry/src/text/text_layout.rs | 100 +++----------- 8 files changed, 273 insertions(+), 394 deletions(-) create mode 100644 masonry/src/text/movement.rs diff --git a/Cargo.lock b/Cargo.lock index d5bfe2769..80bfc96d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -945,20 +945,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f25c0e292a7ca6d6498557ff1df68f32c99850012b6ea401cf8daf771f22ff53" -[[package]] -name = "dwrote" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "439a1c2ba5611ad3ed731280541d36d2e9c4ac5e7fb818a27b604bdc5a6aa65b" -dependencies = [ - "lazy_static", - "libc", - "serde", - "serde_derive", - "winapi", - "wio", -] - [[package]] name = "elm" version = "0.0.0" @@ -1150,23 +1136,22 @@ dependencies = [ [[package]] name = "fontique" version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "00db74e5e46a808cfa00b871c1dc526e33e7e151f205c6011defd9ba6df17d8a" +source = "git+https://github.com/linebender/parley?branch=main#41821d4068e35b826208bd6073f856fae620c36f" dependencies = [ "core-foundation", "core-text", - "dwrote", "fontconfig-cache-parser", "hashbrown", "icu_locid", - "icu_properties", "memmap2", + "objc2", + "objc2-foundation", "peniko", "roxmltree", "skrifa 0.19.3", "smallvec", - "winapi", - "wio", + "windows 0.58.0", + "windows-core 0.58.0", ] [[package]] @@ -1665,18 +1650,6 @@ dependencies = [ "tracing", ] -[[package]] -name = "icu_collections" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db2fa452206ebee18c4b5c2274dbf1de17008e874b4dc4f0aea9d01ca79e4526" -dependencies = [ - "displaydoc", - "yoke", - "zerofrom", - "zerovec", -] - [[package]] name = "icu_locid" version = "1.5.0" @@ -1687,76 +1660,6 @@ dependencies = [ "litemap", "tinystr", "writeable", - "zerovec", -] - -[[package]] -name = "icu_locid_transform" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01d11ac35de8e40fdeda00d9e1e9d92525f3f9d887cdd7aa81d727596788b54e" -dependencies = [ - "displaydoc", - "icu_locid", - "icu_locid_transform_data", - "icu_provider", - "tinystr", - "zerovec", -] - -[[package]] -name = "icu_locid_transform_data" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fdc8ff3388f852bede6b579ad4e978ab004f139284d7b28715f773507b946f6e" - -[[package]] -name = "icu_properties" -version = "1.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "93d6020766cfc6302c15dbbc9c8778c37e62c14427cb7f6e601d849e092aeef5" -dependencies = [ - "displaydoc", - "icu_collections", - "icu_locid_transform", - "icu_properties_data", - "icu_provider", - "tinystr", - "zerovec", -] - -[[package]] -name = "icu_properties_data" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67a8effbc3dd3e4ba1afa8ad918d5684b8868b3b26500753effea8d2eed19569" - -[[package]] -name = "icu_provider" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ed421c8a8ef78d3e2dbc98a973be2f3770cb42b606e3ab18d6237c4dfde68d9" -dependencies = [ - "displaydoc", - "icu_locid", - "icu_provider_macros", - "stable_deref_trait", - "tinystr", - "writeable", - "yoke", - "zerofrom", - "zerovec", -] - -[[package]] -name = "icu_provider_macros" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ec89e9337638ecdc08744df490b221a7399bf8d164eb52a665454e60e075ad6" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.72", ] [[package]] @@ -2593,8 +2496,7 @@ dependencies = [ [[package]] name = "parley" version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be05c1c99b0bd9272eb75f495d47090add32275015f428aefa370b41179379c3" +source = "git+https://github.com/linebender/parley?branch=main#41821d4068e35b826208bd6073f856fae620c36f" dependencies = [ "fontique", "peniko", @@ -3415,12 +3317,6 @@ dependencies = [ "bitflags 2.6.0", ] -[[package]] -name = "stable_deref_trait" -version = "1.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" - [[package]] name = "static_assertions" version = "1.1.0" @@ -3462,8 +3358,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "93cdc334a50fcc2aa3f04761af3b28196280a6aaadb1ef11215c478ae32615ac" dependencies = [ "skrifa 0.20.0", - "yazi", - "zeno", ] [[package]] @@ -3497,17 +3391,6 @@ dependencies = [ "futures-core", ] -[[package]] -name = "synstructure" -version = "0.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8af7666ab7b6390ab78131fb5b0fce11d6b7a6951602017c35fa82800708971" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.72", -] - [[package]] name = "tempfile" version = "3.11.0" @@ -3625,7 +3508,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9117f5d4db391c1cf6927e7bea3db74b9a1c1add8f7eda9ffd5364f40f57b82f" dependencies = [ "displaydoc", - "zerovec", ] [[package]] @@ -4813,15 +4695,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "wio" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d129932f4644ac2396cb456385cbf9e63b5b30c6e8dc4820bdca4eb082037a5" -dependencies = [ - "winapi", -] - [[package]] name = "writeable" version = "0.5.5" @@ -4946,36 +4819,6 @@ version = "0.8.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "791978798f0597cfc70478424c2b4fdc2b7a8024aaff78497ef00f24ef674193" -[[package]] -name = "yazi" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c94451ac9513335b5e23d7a8a2b61a7102398b8cca5160829d313e84c9d98be1" - -[[package]] -name = "yoke" -version = "0.7.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c5b1314b079b0930c31e3af543d8ee1757b1951ae1e1565ec704403a7240ca5" -dependencies = [ - "serde", - "stable_deref_trait", - "yoke-derive", - "zerofrom", -] - -[[package]] -name = "yoke-derive" -version = "0.7.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28cc31741b18cb6f1d5ff12f5b7523e3d6eb0852bbbad19d73905511d9849b95" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.72", - "synstructure", -] - [[package]] name = "zbus" version = "3.15.2" @@ -5042,12 +4885,6 @@ dependencies = [ "zvariant", ] -[[package]] -name = "zeno" -version = "0.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd15f8e0dbb966fd9245e7498c7e9e5055d9e5c8b676b95bd67091cd11a1e697" - [[package]] name = "zerocopy" version = "0.7.35" @@ -5069,55 +4906,12 @@ dependencies = [ "syn 2.0.72", ] -[[package]] -name = "zerofrom" -version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91ec111ce797d0e0784a1116d0ddcdbea84322cd79e5d5ad173daeba4f93ab55" -dependencies = [ - "zerofrom-derive", -] - -[[package]] -name = "zerofrom-derive" -version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ea7b4a3637ea8669cedf0f1fd5c286a17f3de97b8dd5a70a6c167a1730e63a5" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.72", - "synstructure", -] - [[package]] name = "zeroize" version = "1.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ced3678a2879b30306d323f4542626697a464a97c0a07c9aebf7ebca65cd4dde" -[[package]] -name = "zerovec" -version = "0.10.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa2b893d79df23bfb12d5461018d408ea19dfafe76c2c7ef6d4eba614f8ff079" -dependencies = [ - "yoke", - "zerofrom", - "zerovec-derive", -] - -[[package]] -name = "zerovec-derive" -version = "0.10.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6eafa6dfb17584ea3e2bd6e76e0cc15ad7af12b09abdd1ca55961bed9b1063c6" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.72", -] - [[package]] name = "zune-core" version = "0.4.12" diff --git a/Cargo.toml b/Cargo.toml index 1d14b2345..a0c000f6f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -107,7 +107,7 @@ xilem_core = { version = "0.1.0", path = "xilem_core" } vello = "0.2.1" wgpu = "0.20.1" kurbo = "0.11.0" -parley = "0.1.0" +parley = { version = "0.1.0", git = "https://github.com/linebender/parley", branch = "main" } peniko = "0.1.1" winit = "0.30.4" tracing = { version = "0.1.40", default-features = false } diff --git a/masonry/examples/custom_widget.rs b/masonry/examples/custom_widget.rs index cacc0b25f..e3923970c 100644 --- a/masonry/examples/custom_widget.rs +++ b/masonry/examples/custom_widget.rs @@ -109,8 +109,9 @@ impl Widget for CustomWidget { text_layout_builder.push_default(&StyleProperty::FontSize(24.0)); text_layout_builder.push_default(&StyleProperty::Brush(Brush::Solid(fill_color).into())); - let mut text_layout = text_layout_builder.build(); - text_layout.break_all_lines(None, Alignment::Start); + let mut text_layout = text_layout_builder.build(&self.0); + text_layout.break_all_lines(None); + text_layout.align(None, Alignment::Start); let mut scratch_scene = Scene::new(); // We can pass a transform matrix to rotate the text we render diff --git a/masonry/src/text/edit.rs b/masonry/src/text/edit.rs index 89b9e5592..f31c2fe90 100644 --- a/masonry/src/text/edit.rs +++ b/masonry/src/text/edit.rs @@ -3,25 +3,17 @@ use std::ops::{Deref, DerefMut, Range}; -use parley::{FontContext, LayoutContext}; +use parley::{layout::Affinity, FontContext, LayoutContext}; use tracing::warn; use vello::kurbo::Point; use vello::Scene; -use winit::{ - event::Ime, - keyboard::{Key, NamedKey}, -}; +use winit::event::Ime; +use winit::keyboard::{Key, NamedKey}; -use crate::{ - event::{PointerButton, PointerState}, - Action, EventCtx, Handled, TextEvent, -}; - -use super::{ - offset_for_delete_backwards, - selection::{Affinity, Selection}, - Selectable, TextBrush, TextWithSelection, -}; +use crate::event::{PointerButton, PointerState}; +use crate::text::selection::Selection; +use crate::text::{offset_for_delete_backwards, Selectable, TextBrush, TextWithSelection}; +use crate::{Action, EventCtx, Handled, TextEvent}; /// A region of text which can support editing operations pub struct TextEditor { @@ -91,8 +83,10 @@ impl TextEditor { if let Some(selection) = self.inner.selection { if !selection.is_caret() { self.text_mut().replace_range(selection.range(), ""); - self.inner.selection = - Some(Selection::caret(selection.min(), Affinity::Upstream)); + self.inner.selection = Some(Selection::caret( + selection.min(), + Affinity::Downstream, + )); let contents = self.text().clone(); ctx.submit_action(Action::TextChanged(contents)); @@ -189,14 +183,16 @@ impl TextEditor { if let Some(selection) = self.inner.selection { if !selection.is_caret() { self.text_mut().replace_range(selection.range(), ""); - self.inner.selection = - Some(Selection::caret(selection.min(), Affinity::Upstream)); + self.inner.selection = Some(Selection::caret( + selection.min(), + Affinity::Downstream, + )); } let offset = self.text().prev_word_offset(selection.active).unwrap_or(0); self.text_mut().replace_range(offset..selection.active, ""); self.inner.selection = - Some(Selection::caret(offset, Affinity::Upstream)); + Some(Selection::caret(offset, Affinity::Downstream)); let contents = self.text().clone(); ctx.submit_action(Action::TextChanged(contents)); @@ -217,8 +213,10 @@ impl TextEditor { self.text().next_word_offset(selection.active) { self.text_mut().replace_range(selection.active..offset, ""); - self.inner.selection = - Some(Selection::caret(selection.min(), Affinity::Upstream)); + self.inner.selection = Some(Selection::caret( + selection.min(), + Affinity::Downstream, + )); } let contents = self.text().clone(); ctx.submit_action(Action::TextChanged(contents)); @@ -240,7 +238,7 @@ impl TextEditor { self.text_mut().replace_range(selection_range.clone(), text); self.selection = Some(Selection::caret( selection_range.start + text.len(), - Affinity::Upstream, + Affinity::Downstream, )); } let contents = self.text().clone(); @@ -262,10 +260,10 @@ impl TextEditor { Some(Selection::new( np.start + pec.0, np.start + pec.1, - Affinity::Upstream, + Affinity::Downstream, )) } else { - Some(Selection::caret(np.end, Affinity::Upstream)) + Some(Selection::caret(np.end, Affinity::Downstream)) }; } else { // If we've been sent an event to clear the preedit, @@ -289,10 +287,10 @@ impl TextEditor { Some(Selection::new( np.start + pec.0, np.start + pec.1, - Affinity::Upstream, + Affinity::Downstream, )) } else { - Some(Selection::caret(np.start, Affinity::Upstream)) + Some(Selection::caret(np.start, Affinity::Downstream)) }; } Handled::Yes @@ -303,7 +301,7 @@ impl TextEditor { self.text_mut().replace_range(preedit.clone(), ""); self.selection = Some( self.selection - .unwrap_or(Selection::caret(0, Affinity::Upstream)), + .unwrap_or(Selection::caret(0, Affinity::Downstream)), ); self.preedit_range = None; } @@ -316,7 +314,7 @@ impl TextEditor { let sm = self.selection.map(|x| x.min()).unwrap_or(0); if preedit.contains(&sm) { self.selection = - Some(Selection::caret(preedit.start, Affinity::Upstream)); + Some(Selection::caret(preedit.start, Affinity::Downstream)); } } Handled::Yes diff --git a/masonry/src/text/movement.rs b/masonry/src/text/movement.rs new file mode 100644 index 000000000..aaccdace0 --- /dev/null +++ b/masonry/src/text/movement.rs @@ -0,0 +1,186 @@ +// Copyright 2024 the Xilem Authors and the Druid Authors +// SPDX-License-Identifier: Apache-2.0 + +#![expect(unused)] + +use parley::layout::cursor::{Selection, VisualMode}; +use parley::layout::Affinity; +use parley::style::Brush; +use parley::Layout; +use winit::event::KeyEvent; +use winit::keyboard::{KeyCode, ModifiersState, PhysicalKey}; + +// TODO - Add ParagraphStart and ParagraphEnd. + +/// Different ways a cursor can move. +/// +/// This enum tries to match the API of parley::layout::cursor::Selection, +/// and may change in the future. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum Movement { + /// Backspace may behave differently than LeftArrow for emoji. + Backspace, + GraphemeLeft, + GraphemeRight, + WordPrev, + WordNext, + LineStart, + LineEnd, + LineUp(usize), + LineDown(usize), + DocumentStart, + DocumentEnd, +} + +impl Movement { + pub fn apply_to( + &self, + selection: &Selection, + layout: &Layout, + extend: bool, + ) -> Selection { + match self { + Movement::Backspace => { + // TODO - Use backspace-specific algo. + selection.previous_visual(layout, VisualMode::Strong, extend) + } + Movement::GraphemeLeft => selection.previous_visual(layout, VisualMode::Strong, extend), + Movement::GraphemeRight => selection.next_visual(layout, VisualMode::Strong, extend), + Movement::WordPrev => selection.previous_word(layout, extend), + Movement::WordNext => selection.next_word(layout, extend), + Movement::LineStart => selection.line_start(layout, extend), + Movement::LineEnd => selection.line_end(layout, extend), + Movement::LineUp(count) => selection.move_lines(layout, -(*count as isize), extend), + Movement::LineDown(count) => selection.move_lines(layout, (*count as isize), extend), + // TODO - Express this in a less hacky way. + Movement::DocumentStart => selection.move_lines(&layout, isize::MIN, extend), + Movement::DocumentEnd => selection.move_lines(&layout, isize::MAX, extend), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum TextAction { + /// Arrows, Home, End, etc + Move(Movement), + /// Ctrl+A + SelectAll, + /// Shift+Arrows, Shift+Home, Shift+End, etc + Select(Movement), + /// Backspace, Delete, Ctrl+Backspace, Ctrl+Delete. + /// + /// If the selection is empty, will extend the cursor with the given movement, the delete the section. + /// Otherwise, will delete the selection. + SelectAndDelete(Movement), + /// Regular input, IME commit + Splice(String), + /// IME preedit + Preedit(String), + /// Clipboard copy + Copy, + /// Clipboard cut + Cut, + /// Clipboard paste + Paste, +} + +impl TextAction { + pub fn move_or_select(movement: Movement, select: bool) -> Self { + if select { + TextAction::Select(movement) + } else { + TextAction::Move(movement) + } + } +} + +pub fn convert_key(event: KeyEvent, mods: ModifiersState) -> Option { + #[allow(unused)] + let (shift, ctrl, cmd) = (mods.shift_key(), mods.control_key(), mods.super_key()); + let action_mod = if cfg!(target_os = "macos") { cmd } else { ctrl }; + + let PhysicalKey::Code(code) = event.physical_key else { + return None; + }; + + #[rustfmt::skip] + let action = match code { + KeyCode::KeyA if !shift && action_mod => { + TextAction::SelectAll + } + KeyCode::ArrowLeft if action_mod => { + TextAction::move_or_select(Movement::WordPrev, shift) + } + KeyCode::ArrowRight if action_mod => { + TextAction::move_or_select(Movement::WordNext, shift) + } + KeyCode::Home if action_mod => { + TextAction::move_or_select(Movement::DocumentStart, shift) + } + KeyCode::End if action_mod => { + TextAction::move_or_select(Movement::DocumentEnd, shift) + } + KeyCode::Delete if action_mod && shift => { + TextAction::SelectAndDelete(Movement::LineEnd) + } + KeyCode::Backspace if action_mod && shift => { + TextAction::SelectAndDelete(Movement::LineStart) + } + KeyCode::Delete if action_mod => { + TextAction::SelectAndDelete(Movement::WordNext) + } + KeyCode::Backspace if action_mod => { + TextAction::SelectAndDelete(Movement::WordPrev) + } + KeyCode::ArrowLeft => { + TextAction::move_or_select(Movement::GraphemeLeft, shift) + } + KeyCode::ArrowRight => { + TextAction::move_or_select(Movement::GraphemeRight, shift) + } + KeyCode::ArrowUp => { + TextAction::move_or_select(Movement::LineUp(1), shift) + } + KeyCode::ArrowDown => { + TextAction::move_or_select(Movement::LineDown(1), shift) + } + KeyCode::Home => { + TextAction::move_or_select(Movement::LineStart, shift) + } + KeyCode::End => { + TextAction::move_or_select(Movement::LineEnd, shift) + } + KeyCode::Delete => { + TextAction::SelectAndDelete(Movement::GraphemeRight) + } + KeyCode::Backspace => { + TextAction::SelectAndDelete(Movement::Backspace) + } + KeyCode::KeyC if action_mod => { + TextAction::Copy + } + KeyCode::KeyX if action_mod => { + TextAction::Cut + } + KeyCode::KeyV if action_mod => { + TextAction::Paste + } + _ => { + let text = event.text.map(|text| text.to_string()).unwrap_or_default(); + TextAction::Splice(text) + } + }; + Some(action) +} + +pub fn convert_key_readonly(event: KeyEvent, mods: ModifiersState) -> Option { + let action = convert_key(event, mods)?; + + match action { + TextAction::SelectAndDelete(_) => None, + TextAction::Splice(_) => None, + TextAction::Cut => None, + TextAction::Paste => None, + _ => Some(action), + } +} diff --git a/masonry/src/text/render_text.rs b/masonry/src/text/render_text.rs index 8cf2f2d58..bba0ab2ad 100644 --- a/masonry/src/text/render_text.rs +++ b/masonry/src/text/render_text.rs @@ -3,12 +3,11 @@ //! Helper functions for working with text in Masonry. +use parley::layout::PositionedLayoutItem; use parley::Layout; -use vello::{ - kurbo::{Affine, Line, Rect, Stroke}, - peniko::Fill, - Scene, -}; +use vello::kurbo::{Affine, Line, Rect, Stroke}; +use vello::peniko::Fill; +use vello::Scene; use crate::text::TextBrush; @@ -22,7 +21,10 @@ pub fn render_text( scratch_scene.reset(); for line in layout.lines() { let metrics = &line.metrics(); - for glyph_run in line.glyph_runs() { + for item in line.items() { + let PositionedLayoutItem::GlyphRun(glyph_run) = item else { + continue; + }; let mut x = glyph_run.offset(); let y = glyph_run.baseline(); let run = glyph_run.run(); diff --git a/masonry/src/text/selection.rs b/masonry/src/text/selection.rs index 78877bef2..977397a88 100644 --- a/masonry/src/text/selection.rs +++ b/masonry/src/text/selection.rs @@ -6,9 +6,10 @@ use std::borrow::Cow; use std::ops::{Deref, DerefMut, Range}; -use parley::context::RangedBuilder; +use parley::builder::RangedBuilder; +use parley::layout::{Affinity, Cursor}; use parley::{FontContext, LayoutContext}; -use tracing::debug; +use tracing::{debug, warn}; use unicode_segmentation::{GraphemeCursor, UnicodeSegmentation}; use vello::kurbo::{Affine, Line, Point, Stroke}; use vello::peniko::{Brush, Color}; @@ -82,23 +83,19 @@ impl TextWithSelection { if button == PointerButton::Primary { self.selecting_with_mouse = true; self.needs_selection_update = true; - // TODO: Much of this juggling seems unnecessary - let position = Point::new(state.position.x, state.position.y) - origin; - let position = self - .layout - .cursor_for_point(Point::new(position.x, position.y)); - tracing::warn!("Got cursor point without getting affinity"); + + let pos = Point::new(state.position.x, state.position.y) - origin; + let new_cursor = Cursor::from_point(self.layout.layout(), pos.x as f32, pos.y as f32); + if state.mods.state().shift_key() { if let Some(selection) = self.selection.as_mut() { - selection.active = position.insert_point; - selection.active_affinity = Affinity::Downstream; + selection.active = new_cursor.index(); + selection.active_affinity = new_cursor.affinity(); return true; } } - self.selection = Some(Selection::caret( - position.insert_point, - Affinity::Downstream, - )); + self.selection = Some(Selection::caret(new_cursor.index(), new_cursor.affinity())); + true } else { false @@ -114,16 +111,15 @@ impl TextWithSelection { pub fn pointer_move(&mut self, origin: Point, state: &PointerState) -> bool { if self.selecting_with_mouse { self.needs_selection_update = true; - let position = Point::new(state.position.x, state.position.y) - origin; - let position = self - .layout - .cursor_for_point(Point::new(position.x, position.y)); - tracing::warn!("Got cursor point without getting affinity"); + + let pos = Point::new(state.position.x, state.position.y) - origin; + let new_cursor = Cursor::from_point(self.layout.layout(), pos.x as f32, pos.y as f32); + if let Some(selection) = self.selection.as_mut() { - selection.active = position.insert_point; - selection.active_affinity = Affinity::Downstream; + selection.active = new_cursor.index(); + selection.active_affinity = new_cursor.affinity(); } else { - debug_panic!("No selection set whilst still dragging"); + warn!("No selection set whilst still dragging"); } true } else { @@ -253,9 +249,7 @@ impl TextWithSelection { &mut self, font_ctx: &mut FontContext, layout_ctx: &mut LayoutContext, - attributes: impl for<'b> FnOnce( - RangedBuilder<'b, TextBrush, &'b str>, - ) -> RangedBuilder<'b, TextBrush, &'b str>, + attributes: impl for<'b> FnOnce(RangedBuilder<'b, TextBrush>) -> RangedBuilder<'b, TextBrush>, ) { // In theory, we could be clever here and only rebuild the layout if the // selected range was previously or currently non-zero size (i.e. there is a selected range) @@ -287,7 +281,10 @@ impl TextWithSelection { pub fn draw(&mut self, scene: &mut Scene, point: impl Into) { // TODO: Calculate the location for this in layout lazily? if let Some(selection) = self.selection { - self.cursor_line = Some(self.layout.cursor_line_for_text_position(selection.active)); + self.cursor_line = Some( + self.layout + .caret_line_from_index(selection.active, selection.active_affinity), + ); } else { self.cursor_line = None; } @@ -480,43 +477,6 @@ impl Selection { } } -/// Distinguishes between two visually distinct locations with the same byte -/// index. -/// -/// Sometimes, a byte location in a document has two visual locations. For -/// example, the end of a soft-wrapped line and the start of the subsequent line -/// have different visual locations (and we want to be able to place an input -/// caret in either place!) but the same byte-wise location. This also shows up -/// in bidirectional text contexts. Affinity allows us to disambiguate between -/// these two visual locations. -/// -/// Note that in scenarios where soft line breaks interact with bidi text, this gets -/// more complicated. -/// -/// This also has an impact on rich text editing. -/// For example, if the cursor is in a region like `a|1`, where `a` is bold and `1` is not. -/// When editing, if we came from the start of the string, we should assume that the next -/// character will be bold, from the right italic. -#[derive(Copy, Clone, Debug, Hash, PartialEq)] -pub enum Affinity { - /// The position which has an apparent position "earlier" in the text. - /// For soft line breaks, this is the position at the end of the first line. - /// - /// For positions in-between bidi contexts, this is the position which is - /// related to the "outgoing" text section. E.g. for the string "abcDEF" (rendered `abcFED`), - /// with the cursor at "abc|DEF" with upstream affinity, the cursor would be rendered at the - /// position `abc|DEF` - Upstream, - /// The position which has a higher apparent position in the text. - /// For soft line breaks, this is the position at the beginning of the second line. - /// - /// For positions in-between bidi contexts, this is the position which is - /// related to the "incoming" text section. E.g. for the string "abcDEF" (rendered `abcFED`), - /// with the cursor at "abc|DEF" with downstream affinity, the cursor would be rendered at the - /// position `abcDEF|` - Downstream, -} - /// Text which can have internal selections pub trait Selectable: Sized + AsRef + Eq { /// Get slice of text at range. diff --git a/masonry/src/text/text_layout.rs b/masonry/src/text/text_layout.rs index 55a0466ee..7a6425edf 100644 --- a/masonry/src/text/text_layout.rs +++ b/masonry/src/text/text_layout.rs @@ -5,10 +5,10 @@ use std::rc::Rc; -use parley::context::RangedBuilder; +use parley::builder::RangedBuilder; use parley::fontique::{Style, Weight}; -use parley::layout::{Alignment, Cursor}; -use parley::style::{Brush as BrushTrait, FontFamily, FontStack, GenericFamily, StyleProperty}; +use parley::layout::{Affinity, Alignment, Cursor}; +use parley::style::{FontFamily, FontStack, GenericFamily, StyleProperty}; use parley::{FontContext, Layout, LayoutContext}; use vello::kurbo::{Affine, Line, Point, Rect, Size}; use vello::peniko::{self, Color, Gradient}; @@ -102,8 +102,6 @@ impl TextBrush { } } -impl BrushTrait for TextBrush {} - impl From for TextBrush { fn from(value: peniko::Brush) -> Self { Self::Normal(value, Hinting::default()) @@ -317,86 +315,25 @@ impl TextLayout { } } - /// For a given `Point` (relative to this object's origin), returns index - /// into the underlying text of the nearest grapheme boundary. - /// - /// This is not meaningful until [`Self::rebuild`] has been called. - pub fn cursor_for_point(&self, point: Point) -> Cursor { - self.assert_rebuilt("text_position_for_point"); - - // TODO: This is a mostly good first pass, but doesn't handle cursor positions in - // grapheme clusters within a parley cluster. - // We can also try - Cursor::from_point(&self.layout, point.x as f32, point.y as f32) - } - - /// Given the utf-8 position of a character boundary in the underlying text, - /// return the `Point` (relative to this object's origin) representing the - /// boundary of the containing grapheme. - /// - /// # Panics - /// - /// Panics if `text_pos` is not a character boundary. - /// - /// This is not meaningful until [`Self::rebuild`] has been called. - pub fn cursor_for_text_position(&self, text_pos: usize) -> Cursor { - self.assert_rebuilt("cursor_for_text_position"); - - // TODO: As a reminder, `is_leading` is not very useful to us; we don't know this ahead of time - // We're going to need to do quite a bit of remedial work on these - // e.g. to handle a inside a ligature made of multiple (unicode) grapheme clusters - // https://raphlinus.github.io/text/2020/10/26/text-layout.html#shaping-cluster - // But we're choosing to defer this work - // This also needs to handle affinity. - Cursor::from_position(&self.layout, text_pos, true) - } - - /// Given the utf-8 position of a character boundary in the underlying text, - /// return the `Point` (relative to this object's origin) representing the - /// boundary of the containing grapheme. - /// - /// # Panics - /// - /// Panics if `text_pos` is not a character boundary. - /// - /// This is not meaningful until [`Self::rebuild`] has been called. - pub fn point_for_text_position(&self, text_pos: usize) -> Point { - let cursor = self.cursor_for_text_position(text_pos); - Point::new( - cursor.advance as f64, - (cursor.baseline + cursor.offset) as f64, - ) - } - - // TODO: needed for text selection - // /// Given a utf-8 range in the underlying text, return a `Vec` of `Rect`s - // /// representing the nominal bounding boxes of the text in that range. - // /// - // /// # Panics - // /// - // /// Panics if the range start or end is not a character boundary. - // pub fn rects_for_range(&self, range: Range) -> Vec { - // self.layout.rects_for_range(range) - // } - /// Given the utf-8 position of a character boundary in the underlying text, /// return a `Line` suitable for drawing a vertical cursor at that boundary. /// /// This is not meaningful until [`Self::rebuild`] has been called. // TODO: This is too simplistic. See https://raphlinus.github.io/text/2020/10/26/text-layout.html#shaping-cluster // for example. This would break in a `fi` ligature - pub fn cursor_line_for_text_position(&self, text_pos: usize) -> Line { - let from_position = self.cursor_for_text_position(text_pos); + pub fn caret_line_from_index(&self, text_pos: usize, affinity: Affinity) -> Line { + let caret = Cursor::from_index(&self.layout, text_pos, affinity); - let line = from_position.path.line(&self.layout).unwrap(); + let Some(line) = caret.cluster_path().line(&self.layout) else { + // TODO + return Line::new((0., 0.), (0., 0.)); + }; let line_metrics = line.metrics(); let baseline = line_metrics.baseline + line_metrics.descent; - let p1 = (from_position.offset as f64, baseline as f64); - let p2 = ( - from_position.offset as f64, - (baseline - line_metrics.size()) as f64, - ); + let line_size = line_metrics.size(); + let p1 = (caret.visual_offset() as f64, baseline as f64); + let p2 = (caret.visual_offset() as f64, (baseline - line_size) as f64); Line::new(p1, p2) } @@ -431,15 +368,16 @@ impl TextLayout { layout_ctx: &mut LayoutContext, text: &str, text_changed: bool, - attributes: impl for<'b> FnOnce( - RangedBuilder<'b, TextBrush, &'b str>, - ) -> RangedBuilder<'b, TextBrush, &'b str>, + attributes: impl for<'b> FnOnce(RangedBuilder<'b, TextBrush>) -> RangedBuilder<'b, TextBrush>, ) { // TODO - check against self.last_text_start if self.needs_layout || text_changed { self.needs_layout = false; + // Workaround for how parley treats empty lines. + //let text = if !text.is_empty() { text } else { " " }; + let mut builder = layout_ctx.ranged_builder(font_ctx, text, self.scale); builder.push_default(&StyleProperty::Brush(self.brush.clone())); builder.push_default(&StyleProperty::FontSize(self.text_size)); @@ -451,14 +389,14 @@ impl TextLayout { // - underlining IME suggestions // - applying a brush to selected text. let mut builder = attributes(builder); - builder.build_into(&mut self.layout); + builder.build_into(&mut self.layout, text); self.needs_line_breaks = true; } if self.needs_line_breaks || text_changed { self.needs_line_breaks = false; - self.layout - .break_all_lines(self.max_advance, self.alignment); + self.layout.break_all_lines(self.max_advance); + self.layout.align(self.max_advance, self.alignment); // TODO: // self.links = text