Replace fns Text{,Display}::text_is_rtl

Note: the removed fn line_is_rtl was incorrect for BiDi text

Author: dhardy

src/display/mod.rs

@@ -5,10 +5,10 @@
 
 //! Text prepared for display
 
-#[allow(unused)]
-use crate::{Status, Text};
 use crate::conv::to_usize;
-use crate::{Direction, Vec2, shaper};
+#[allow(unused)]
+use crate::{Direction, Status, Text};
+use crate::{Vec2, shaper};
 use smallvec::SmallVec;
 use tinyvec::TinyVec;
 
@@ -76,12 +76,8 @@ pub struct NotReady;
 /// which provides a
 /// [`GraphemeCursor`](https://unicode-rs.github.io/unicode-segmentation/unicode_segmentation/struct.GraphemeCursor.html)
 /// to step back or forward one "grapheme", in logical text order.
-/// Optionally, the direction may
-/// be reversed for right-to-left lines [`TextDisplay::line_is_rtl`], but note
-/// that the result may be confusing since not all text on the line follows the
-/// line's base direction and adjacent lines may have different directions.
-///
-/// Navigating glyphs left or right in display-order is not currently supported.
+/// The direction of navigation may be reversed for [right-to-left text](Self::text_is_rtl)
+/// (i.e. reversed logical-order navigation).
 ///
 /// To navigate "up" and "down" lines, use [`TextDisplay::text_glyph_pos`] to
 /// get the position of the cursor, [`TextDisplay::find_line`] to get the line
@@ -105,6 +101,7 @@ pub struct TextDisplay {
     wrapped_runs: TinyVec<[RunPart; 1]>,
     /// Visual (wrapped) lines, in visual and logical order
     lines: TinyVec<[Line; 1]>,
+    is_rtl: bool,
     l_bound: f32,
     r_bound: f32,
 }
@@ -117,7 +114,7 @@ fn size_of_elts() {
     assert_eq!(size_of::<shaper::GlyphRun>(), 112);
     assert_eq!(size_of::<RunPart>(), 24);
     assert_eq!(size_of::<Line>(), 24);
-    assert_eq!(size_of::<TextDisplay>(), 200);
+    assert_eq!(size_of::<TextDisplay>(), 208);
 }
 
 impl Default for TextDisplay {
@@ -126,6 +123,7 @@ impl Default for TextDisplay {
             runs: Default::default(),
             wrapped_runs: Default::default(),
             lines: Default::default(),
+            is_rtl: false,
             l_bound: 0.0,
             r_bound: 0.0,
         }
@@ -201,45 +199,13 @@ impl TextDisplay {
 
     /// Get the base directionality of the text
     ///
-    /// [Requires status][Self#status-of-preparation]: none.
-    pub fn text_is_rtl(&self, text: &str, direction: Direction) -> bool {
-        let (is_auto, mut is_rtl) = match direction {
-            Direction::Ltr => (false, false),
-            Direction::Rtl => (false, true),
-            Direction::Auto => (true, false),
-            Direction::AutoRtl => (true, true),
-        };
-
-        if is_auto {
-            match unicode_bidi::get_base_direction(text) {
-                unicode_bidi::Direction::Ltr => is_rtl = false,
-                unicode_bidi::Direction::Rtl => is_rtl = true,
-                unicode_bidi::Direction::Mixed => (),
-            }
-        }
-
-        is_rtl
-    }
-
-    /// Get the directionality of the current line
-    ///
-    /// [Requires status][Self#status-of-preparation]: lines have been wrapped.
-    ///
-    /// Returns:
-    ///
-    /// - `None` if text is empty
-    /// - `Some(line_is_right_to_left)` otherwise
+    /// [Requires status][Self#status-of-preparation]: run-breaking is complete.
     ///
-    /// Note: indeterminate lines (e.g. empty lines) have their direction
-    /// determined from the passed environment, by default left-to-right.
-    pub fn line_is_rtl(&self, line: usize) -> Option<bool> {
-        if let Some(line) = self.lines.get(line) {
-            let first_run = line.run_range.start();
-            let glyph_run = to_usize(self.wrapped_runs[first_run].glyph_run);
-            Some(self.runs[glyph_run].level.is_rtl())
-        } else {
-            None
-        }
+    /// This returns the direction inferred from the `text` and [`Direction`]
+    /// used during run-breaking. See also [`Direction::text_is_rtl`].
+    #[inline]
+    pub fn text_is_rtl(&self) -> bool {
+        self.is_rtl
     }
 
     /// Find the text index for the glyph nearest the given `pos`

src/display/text_runs.rs

@@ -213,6 +213,11 @@ impl TextDisplay {
             Direction::Rtl => Some(RTL_LEVEL),
         };
         let info = BidiInfo::new(text, default_para_level);
+        self.is_rtl = info
+            .paragraphs
+            .first()
+            .map(|p| p.level.is_rtl())
+            .unwrap_or_default();
         let levels = info.levels;
         assert_eq!(text.len(), levels.len());
 

src/text.rs

@@ -310,23 +310,15 @@ impl<T: FormattableText + ?Sized> Text<T> {
 
     /// Get the base directionality of the text
     ///
-    /// This does not require that the text is prepared.
+    /// This returns the direction inferred during run-breaking if available,
+    /// falling back to [`Direction::text_is_rtl`].
+    #[inline]
     pub fn text_is_rtl(&self) -> bool {
-        let cached_is_rtl = match self.line_is_rtl(0) {
-            Ok(None) => Some(self.direction == Direction::Rtl),
-            Ok(Some(is_rtl)) => Some(is_rtl),
-            Err(NotReady) => None,
-        };
-        #[cfg(not(debug_assertions))]
-        if let Some(cached) = cached_is_rtl {
-            return cached;
-        }
-
-        let is_rtl = self.display.text_is_rtl(self.as_str(), self.direction);
-        if let Some(cached) = cached_is_rtl {
-            debug_assert_eq!(cached, is_rtl);
+        if self.status >= Status::ResizeLevelRuns {
+            self.display.text_is_rtl()
+        } else {
+            self.direction.text_is_rtl(&self.text.as_str())
         }
-        is_rtl
     }
 
     /// Return the sequence of effect tokens
@@ -509,14 +501,6 @@ impl<T: FormattableText + ?Sized> Text<T> {
         Ok(self.wrapped_display()?.find_line(index))
     }
 
-    /// Get the directionality of the current line
-    ///
-    /// See [`TextDisplay::line_is_rtl`].
-    #[inline]
-    pub fn line_is_rtl(&self, line: usize) -> Result<Option<bool>, NotReady> {
-        Ok(self.wrapped_display()?.line_is_rtl(line))
-    }
-
     /// Find the text index for the glyph nearest the given `pos`
     ///
     /// See [`TextDisplay::text_index_nearest`].