Unify druid & druid-shell Movement types

druid-shell/src/text.rs

@@ -611,6 +611,25 @@ pub enum Direction {
     Downstream,
 }
 
+impl Direction {
+    /// Returns `true` if this direction is byte-wise backwards for
+    /// the provided [`WritingDirection`].
+    ///
+    /// The provided direction *must not be* `WritingDirection::Natural`.
+    pub fn is_upstream_for_direction(self, direction: WritingDirection) -> bool {
+        assert!(
+            !matches!(direction, WritingDirection::Natural),
+            "writing direction must be resolved"
+        );
+        match self {
+            Direction::Upstream => true,
+            Direction::Downstream => false,
+            Direction::Left => matches!(direction, WritingDirection::LeftToRight),
+            Direction::Right => matches!(direction, WritingDirection::RightToLeft),
+        }
+    }
+}
+
 /// Distinguishes between two visually distinct locations with the same byte
 /// index.
 ///

druid/src/text/input_component.rs

@@ -564,14 +564,12 @@ impl<T: TextStorage + EditableText> EditSession<T> {
     fn do_action(&mut self, buffer: &mut T, action: ImeAction) {
         match action {
             ImeAction::Move(movement) => {
-                let sel =
-                    crate::text::movement(movement.into(), self.selection, &self.layout, false);
+                let sel = crate::text::movement(movement, self.selection, &self.layout, false);
                 self.external_selection_change = Some(sel);
                 self.scroll_to_selection_end(false);
             }
             ImeAction::MoveSelecting(movement) => {
-                let sel =
-                    crate::text::movement(movement.into(), self.selection, &self.layout, true);
+                let sel = crate::text::movement(movement, self.selection, &self.layout, true);
                 self.external_selection_change = Some(sel);
                 self.scroll_to_selection_end(false);
             }
@@ -583,8 +581,7 @@ impl<T: TextStorage + EditableText> EditSession<T> {
             //tracing::warn!("Line/Word selection actions are not implemented");
             //}
             ImeAction::Delete(movement) if self.selection.is_caret() => {
-                let movement: Movement = movement.into();
-                if movement == Movement::Left {
+                if movement == Movement::Grapheme(druid_shell::text::Direction::Upstream) {
                     self.backspace(buffer);
                 } else {
                     let to_delete =

druid/src/text/layout.rs

@@ -57,6 +57,7 @@ pub struct TextLayout<T> {
     wrap_width: f64,
     alignment: TextAlignment,
     links: Rc<[(Rect, usize)]>,
+    text_is_rtl: bool,
 }
 
 /// Metrics describing the layout text.
@@ -87,16 +88,7 @@ impl<T> TextLayout<T> {
             wrap_width: f64::INFINITY,
             alignment: Default::default(),
             links: Rc::new([]),
-        }
-    }
-
-    /// Create a new `TextLayout` with the provided text.
-    ///
-    /// This is useful when the text is not died to application data.
-    pub fn from_text(text: impl Into<T>) -> Self {
-        TextLayout {
-            text: Some(text.into()),
-            ..TextLayout::new()
+            text_is_rtl: false,
         }
     }
 
@@ -162,9 +154,27 @@ impl<T> TextLayout<T> {
             self.layout = None;
         }
     }
+
+    /// 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<T: TextStorage> TextLayout<T> {
+    /// Create a new `TextLayout` with the provided text.
+    ///
+    /// This is useful when the text is not tied to application data.
+    pub fn from_text(text: impl Into<T>) -> Self {
+        let mut this = TextLayout::new();
+        this.set_text(text.into());
+        this
+    }
+
     /// Returns `true` if this layout needs to be rebuilt.
     ///
     /// This happens (for instance) after style attributes are modified.
@@ -178,6 +188,7 @@ impl<T: TextStorage> TextLayout<T> {
     /// Set the text to display.
     pub fn set_text(&mut self, text: T) {
         if self.text.is_none() || !self.text.as_ref().unwrap().same(&text) {
+            self.text_is_rtl = crate::piet::util::first_strong_rtl(text.as_str());
             self.text = Some(text);
             self.layout = None;
         }

druid/src/text/movement.rs

@@ -16,60 +16,9 @@
 
 use crate::kurbo::Point;
 use crate::piet::TextLayout as _;
+pub use crate::shell::text::{Direction, Movement, VerticalMovement, WritingDirection};
 use crate::text::{EditableText, Selection, TextLayout, TextStorage};
 
-/// The specification of a movement.
-#[derive(Debug, PartialEq, Clone, Copy)]
-pub enum Movement {
-    /// Move to the left by one grapheme cluster.
-    Left,
-    /// Move to the right by one grapheme cluster.
-    Right,
-    /// Move up one visible line.
-    Up,
-    /// Move down one visible line.
-    Down,
-    /// Move to the left by one word.
-    LeftWord,
-    /// Move to the right by one word.
-    RightWord,
-    /// Move to left end of visible line.
-    PrecedingLineBreak,
-    /// Move to right end of visible line.
-    NextLineBreak,
-    /// Move to the beginning of the document
-    StartOfDocument,
-    /// Move to the end of the document
-    EndOfDocument,
-}
-
-//FIXME: we should remove this whole file, and use the Movement type defined in druid-shell?
-impl From<crate::shell::text::Movement> for Movement {
-    fn from(src: crate::shell::text::Movement) -> Movement {
-        use crate::shell::text::{Direction, Movement as SMovemement, VerticalMovement};
-        match src {
-            SMovemement::Grapheme(Direction::Left) | SMovemement::Grapheme(Direction::Upstream) => {
-                Movement::Left
-            }
-            SMovemement::Grapheme(_) => Movement::Right,
-            SMovemement::Word(Direction::Left) => Movement::LeftWord,
-            SMovemement::Word(_) => Movement::RightWord,
-            SMovemement::Line(Direction::Left) | SMovemement::ParagraphStart => {
-                Movement::PrecedingLineBreak
-            }
-            SMovemement::Line(_) | SMovemement::ParagraphEnd => Movement::NextLineBreak,
-            SMovemement::Vertical(VerticalMovement::LineUp)
-            | SMovemement::Vertical(VerticalMovement::PageUp) => Movement::Up,
-            SMovemement::Vertical(VerticalMovement::LineDown)
-            | SMovemement::Vertical(VerticalMovement::PageDown) => Movement::Down,
-            SMovemement::Vertical(VerticalMovement::DocumentStart) => Movement::StartOfDocument,
-            SMovemement::Vertical(VerticalMovement::DocumentEnd) => Movement::EndOfDocument,
-            // the source enum is non_exhaustive
-            _ => panic!("unhandled movement {:?}", src),
-        }
-    }
-}
-
 /// Compute the result of movement on a selection.
 ///
 /// returns a new selection representing the state after the movement.
@@ -91,8 +40,14 @@ pub fn movement<T: EditableText + TextStorage>(
         }
     };
 
+    let writing_direction = if crate::piet::util::first_strong_rtl(text.as_str()) {
+        WritingDirection::RightToLeft
+    } else {
+        WritingDirection::LeftToRight
+    };
+
     let (offset, h_pos) = match m {
-        Movement::Left => {
+        Movement::Grapheme(d) if d.is_upstream_for_direction(writing_direction) => {
             if s.is_caret() || modify {
                 text.prev_grapheme_offset(s.active)
                     .map(|off| (off, None))
@@ -101,7 +56,7 @@ pub fn movement<T: EditableText + TextStorage>(
                 (s.min(), None)
             }
         }
-        Movement::Right => {
+        Movement::Grapheme(_) => {
             if s.is_caret() || modify {
                 text.next_grapheme_offset(s.active)
                     .map(|off| (off, None))
@@ -110,8 +65,7 @@ pub fn movement<T: EditableText + TextStorage>(
                 (s.max(), None)
             }
         }
-
-        Movement::Up => {
+        Movement::Vertical(VerticalMovement::LineUp) => {
             let cur_pos = layout.hit_test_text_position(s.active);
             let h_pos = s.h_pos.unwrap_or(cur_pos.point.x);
             if cur_pos.line == 0 {
@@ -123,7 +77,7 @@ pub fn movement<T: EditableText + TextStorage>(
                 (up_pos.idx, Some(point_above.x))
             }
         }
-        Movement::Down => {
+        Movement::Vertical(VerticalMovement::LineDown) => {
             let cur_pos = layout.hit_test_text_position(s.active);
             let h_pos = s.h_pos.unwrap_or(cur_pos.point.x);
             if cur_pos.line == layout.line_count() - 1 {
@@ -137,29 +91,47 @@ pub fn movement<T: EditableText + TextStorage>(
                 (up_pos.idx, Some(point_below.x))
             }
         }
+        Movement::Vertical(VerticalMovement::DocumentStart) => (0, None),
+        Movement::Vertical(VerticalMovement::DocumentEnd) => (text.len(), None),
 
-        Movement::PrecedingLineBreak => (text.preceding_line_break(s.active), None),
-        Movement::NextLineBreak => (text.next_line_break(s.active), None),
-
-        Movement::StartOfDocument => (0, None),
-        Movement::EndOfDocument => (text.len(), None),
+        Movement::ParagraphStart => (text.preceding_line_break(s.active), None),
+        Movement::ParagraphEnd => (text.next_line_break(s.active), None),
 
-        Movement::LeftWord => {
+        Movement::Line(d) => {
+            let hit = layout.hit_test_text_position(s.active);
+            let lm = layout.line_metric(hit.line).unwrap();
+            let offset = if d.is_upstream_for_direction(writing_direction) {
+                lm.start_offset
+            } else {
+                lm.end_offset
+            };
+            (offset, None)
+        }
+        Movement::Word(d) if d.is_upstream_for_direction(writing_direction) => {
             let offset = if s.is_caret() || modify {
                 text.prev_word_offset(s.active).unwrap_or(0)
             } else {
                 s.min()
             };
             (offset, None)
         }
-        Movement::RightWord => {
+        Movement::Word(_) => {
             let offset = if s.is_caret() || modify {
                 text.next_word_offset(s.active).unwrap_or(s.active)
             } else {
                 s.max()
             };
             (offset, None)
         }
+
+        // These two are not handled; they require knowledge of the size
+        // of the viewport.
+        Movement::Vertical(VerticalMovement::PageDown)
+        | Movement::Vertical(VerticalMovement::PageUp) => (s.active, s.h_pos),
+        other => {
+            tracing::warn!("unhandled movement {:?}", other);
+            (s.anchor, s.h_pos)
+        }
     };
 
     let start = if modify { s.anchor } else { offset };