Merge pull request #471 from hojjatabdollahi/hojjat/fix-bidi-zw

Fix aggressive ellipsizing

src/shape.rs

@@ -8,7 +8,7 @@ use crate::{
     FontSystem, Hinting, LayoutGlyph, LayoutLine, Metrics, Wrap,
 };
 #[cfg(not(feature = "std"))]
-use alloc::{format, vec, vec::Vec};
+use alloc::{boxed::Box, format, vec, vec::Vec};
 
 use alloc::collections::VecDeque;
 use core::cmp::{max, min};
@@ -1482,6 +1482,52 @@ impl ShapeLine {
         vl.spaces += number_of_blanks;
     }
 
+    fn remaining_content_exceeds(
+        spans: &[ShapeSpan],
+        font_size: f32,
+        span_index: usize,
+        word_idx: usize,
+        word_count: usize,
+        starting_word_index: usize,
+        direction: LayoutDirection,
+        congruent: bool,
+        start_span: usize,
+        span_count: usize,
+        threshold: f32,
+    ) -> bool {
+        let mut acc: f32 = 0.0;
+
+        // Remaining words in the current span
+        let word_range: Box<dyn Iterator<Item = usize>> = match (direction, congruent) {
+            (LayoutDirection::Forward, true) => Box::new(word_idx + 1..word_count),
+            (LayoutDirection::Forward, false) => Box::new(0..word_idx),
+            (LayoutDirection::Backward, true) => Box::new(starting_word_index..word_idx),
+            (LayoutDirection::Backward, false) => Box::new(word_idx + 1..word_count),
+        };
+        for wi in word_range {
+            acc += spans[span_index].words[wi].width(font_size);
+            if acc > threshold {
+                return true;
+            }
+        }
+
+        // Remaining spans
+        let span_range: Box<dyn Iterator<Item = usize>> = match direction {
+            LayoutDirection::Forward => Box::new(span_index + 1..span_count),
+            LayoutDirection::Backward => Box::new(start_span..span_index),
+        };
+        for si in span_range {
+            for w in &spans[si].words {
+                acc += w.width(font_size);
+                if acc > threshold {
+                    return true;
+                }
+            }
+        }
+
+        false
+    }
+
     /// This will fit as much as possible in one line
     /// If forward is false, it will fit as much as possible from the end of the spans
     /// it will stop when it gets to "start".
@@ -1582,24 +1628,20 @@ impl ShapeLine {
                         && (
                             // if this  word doesn't fit, then we have an overflow
                             (total_w + word_range_width + word_width > max_width)
-                                // otherwise if this is not the last word of the last span
-                                // and we can't fit the ellipsis
-                                ||(
-                                    !(match (direction, congruent) {
-                                        (LayoutDirection::Forward, true) => {
-                                            (span_index == span_count - 1) && (word_idx == word_count - 1)
-                                        }
-                                        (LayoutDirection::Forward, false) => (span_index == span_count - 1) && (word_idx == 0),
-                                        (LayoutDirection::Backward, true) => {
-                                            (span_index == start.span) && (word_idx == starting_word_index)
-                                        }
-                                        (LayoutDirection::Backward, false) => {
-                                            (span_index == start.span) && (word_idx == word_count - 1)
-                                        }
-                                    })
-
-                                    && total_w + word_range_width + word_width + ellipsis_w > max_width
-                                )
+                                || (Self::remaining_content_exceeds(
+                                    spans,
+                                    font_size,
+                                    span_index,
+                                    word_idx,
+                                    word_count,
+                                    starting_word_index,
+                                    direction,
+                                    congruent,
+                                    start.span,
+                                    span_count,
+                                    ellipsis_w,
+                                ) && total_w + word_range_width + word_width + ellipsis_w
+                                    > max_width)
                         )
                 };
 
@@ -1739,6 +1781,27 @@ impl ShapeLine {
         ellipsis_w: f32,
     ) {
         assert!(matches!(ellipsize, Ellipsize::Middle(_)));
+
+        // First check if all content fits without any ellipsis.
+        {
+            let mut test_line = VisualLine::default();
+            self.layout_spans(
+                &mut test_line,
+                font_size,
+                spans,
+                start_opt,
+                rtl,
+                Some(width),
+                Ellipsize::End(EllipsizeHeightLimit::Lines(1)),
+                ellipsis_w,
+                LayoutDirection::Forward,
+            );
+            if !test_line.ellipsized && test_line.w <= width {
+                *current_visual_line = test_line;
+                return;
+            }
+        }
+
         let mut starting_line = VisualLine::default();
         self.layout_spans(
             &mut starting_line,
@@ -1848,6 +1911,7 @@ impl ShapeLine {
             }
             _ => {
                 // everything fit in the forward pass
+                log::warn!("This should be unreachable!");
                 current_visual_line.ranges = starting_line.ranges;
                 current_visual_line.w = starting_line.w;
                 current_visual_line.spaces = starting_line.spaces;

tests/common/mod.rs

@@ -21,6 +21,9 @@ pub struct DrawTestCfg {
     name: String,
     /// The text to render to image
     text: String,
+    /// Optional rich text spans. When set, `set_rich_text` is used instead of `set_text`.
+    /// Each entry is `(text, attrs)` for one styled span.
+    rich_spans: Option<Vec<(String, AttrsOwned)>>,
     /// The name, details of the font to be used.
     /// Expected to be one of the fonts found under the `fonts` directory in this repository.
     font: AttrsOwned,
@@ -41,6 +44,7 @@ impl Default for DrawTestCfg {
             name: "default".into(),
             font: AttrsOwned::new(&font),
             text: "".into(),
+            rich_spans: None,
             font_size: 16.0,
             line_height: 20.0,
             canvas_width: 300,
@@ -65,6 +69,19 @@ impl DrawTestCfg {
         self
     }
 
+    pub fn rich_text(
+        mut self,
+        spans: impl IntoIterator<Item = (&'static str, Attrs<'static>)>,
+    ) -> Self {
+        self.rich_spans = Some(
+            spans
+                .into_iter()
+                .map(|(text, attrs)| (text.to_string(), AttrsOwned::new(&attrs)))
+                .collect(),
+        );
+        self
+    }
+
     pub fn font_attrs(mut self, attrs: Attrs) -> Self {
         self.font = AttrsOwned::new(&attrs);
         self
@@ -115,12 +132,23 @@ impl DrawTestCfg {
             Some((self.canvas_width - margins * 2) as f32),
             Some((self.canvas_height - margins * 2) as f32),
         );
-        buffer.set_text(
-            &self.text,
-            &self.font.as_attrs(),
-            Shaping::Advanced,
-            self.alignment,
-        );
+        if let Some(ref spans) = self.rich_spans {
+            buffer.set_rich_text(
+                spans
+                    .iter()
+                    .map(|(text, attrs)| (text.as_str(), attrs.as_attrs())),
+                &self.font.as_attrs(),
+                Shaping::Advanced,
+                self.alignment,
+            );
+        } else {
+            buffer.set_text(
+                &self.text,
+                &self.font.as_attrs(),
+                Shaping::Advanced,
+                self.alignment,
+            );
+        }
         buffer.shape_until_scroll(true);
 
         // Black

tests/ellipsize_rendering.rs

@@ -1,5 +1,10 @@
+use std::path::PathBuf;
+
 use common::DrawTestCfg;
-use cosmic_text::{Align, Attrs, Ellipsize, EllipsizeHeightLimit, Family, Wrap};
+use cosmic_text::{
+    fontdb::Database, Align, Attrs, Buffer, Ellipsize, EllipsizeHeightLimit, Family, FontSystem,
+    Metrics, Shaping, Wrap,
+};
 
 mod common;
 
@@ -159,3 +164,22 @@ fn test_ellipsize_mixed_ltr_rtl_ltr_middle_three_lines() {
         .canvas(200, 100)
         .validate_text_rendering();
 }
+
+/// Regression rendering test: Fluent's fl!() wraps interpolated values with BiDi
+/// isolation characters. With the bug, this rendered as "Workspace… 2"
+/// After the fix it must render the full "Workspace 2" without ellipsis.
+#[test]
+fn test_ellipsize_bidi_isolates_middle_bug() {
+    let attrs = Attrs::new().family(Family::Name("Inter"));
+    DrawTestCfg::new("ellipsize_bidi_isolates_middle_bug")
+        .font_size(20., 26.)
+        .font_attrs(attrs)
+        .rich_text([(
+            "\u{2068}Workspace\u{2069}\u{2068} \u{2069}\u{2068}2\u{2069}",
+            Attrs::new().family(Family::Name("Inter")),
+        )])
+        .wrap(Wrap::WordOrGlyph)
+        .ellipsize(Ellipsize::Middle(EllipsizeHeightLimit::Lines(1)))
+        .canvas(220, 50)
+        .validate_text_rendering();
+}

tests/images/ellipsize_bidi_isolates_middle_bug.png

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:ec1a2e5611f55301bb332c354eab1f342b0e40361ee19549360e2b1826425614
+size 5136