M4: a justify gap stays out of grapheme clusters and opens beside a right-to-left run (#1970)

Review: a combining mark shaped as a glyph of its own (U+20DD on an ideograph, say) got a gap
between it and its base; a gap after the last logical glyph of a right-to-left run opened after
that glyph's visual place, inside the word, instead of between the run and the ideograph after it.
A gap now needs the next item to start a grapheme cluster, and `visual_gaps` opens each logical
gap at its visual boundary: between logical neighbours where they are, between two runs where they
meet. Two more rows in the test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

crates/text/src/layout.rs

@@ -297,6 +297,48 @@ pub(crate) fn is_rtl_char(c: char) -> bool {
     matches!(c as u32, 0x0590..=0x08FF | 0xFB1D..=0xFDFF | 0xFE70..=0xFEFF | 0x10800..=0x10FFF | 0x1E800..=0x1EFFF | 0x200F | 0x202B | 0x202E | 0x2067)
 }
 
+/// Which visual positions of `order` (the logical index at each) a justify gap follows. A gap
+/// after logical item k (`after[k]`) opens between logically neighbouring glyphs where they are,
+/// or where two runs meet (a right-to-left run is laid out reversed) between the runs, so a gap
+/// beside a Hebrew word in Chinese text opens beside the word, not inside it; each opens once.
+/// From item `trailing` on (a line's trailing spaces) a gap opens after the item itself.
+fn visual_gaps(order: &[usize], after: &[bool], trailing: usize) -> Vec<bool> {
+    let run_min = visual_run_starts(order);
+    let mut applied = vec![false; after.len()];
+    let gap_after = |k: usize| after.get(k).copied().unwrap_or(false);
+    order
+        .iter()
+        .enumerate()
+        .map(|(i, &k)| {
+            if k >= trailing {
+                return gap_after(k);
+            }
+            let at = match order.get(i + 1) {
+                Some(&n) if n == k + 1 || k == n + 1 => Some(k.min(n)),
+                Some(_) => run_min.get(i).copied().max(run_min.get(i + 1).copied()).and_then(|m| m.checked_sub(1)),
+                None => None,
+            };
+            at.is_some_and(|g| gap_after(g) && applied.get_mut(g).is_some_and(|a| !std::mem::replace(a, true)))
+        })
+        .collect()
+}
+
+/// For each position of `order` (the logical index at each visual position), the smallest
+/// logical index of its run: the visually consecutive glyphs whose logical indices step one way.
+fn visual_run_starts(order: &[usize]) -> Vec<usize> {
+    let mut out = Vec::with_capacity(order.len());
+    let mut start = 0;
+    for i in 1..=order.len() {
+        let joined = i < order.len() && order.get(i - 1).zip(order.get(i)).is_some_and(|(&p, &k)| p + 1 == k || k + 1 == p);
+        if !joined {
+            let m = order.get(start..i).and_then(|run| run.iter().copied().min()).unwrap_or(0);
+            out.extend(std::iter::repeat_n(m, i - start));
+            start = i;
+        }
+    }
+    out
+}
+
 /// Visual order of a line's items from their levels (rule L2: from the highest level down to the
 /// lowest odd one, reverse every run at that level or above).
 pub(crate) fn visual_order(levels: &[u8]) -> Vec<usize> {
@@ -1259,10 +1301,17 @@ pub fn layout(text: &TextData, box_width: Option<f64>, box_height: Option<f64>)
         let hang_r =
             items.get(vis_end.wrapping_sub(1)).filter(|it| hang_end && !rtl_line && vis_end > *s + 1 && closes(it.ch)).map_or(0.0, |it| it.adv);
         let gstart = out.glyphs.len();
-        // Justify: extra space per gap (a space, or between ideographs, whose text has no spaces).
+        // Justify: extra space per gap (a space, or between ideographs, whose text has no spaces;
+        // never inside a grapheme cluster, so a mark stays on its base).
         let widens = |k: usize| {
             items.get(k).is_some_and(|it| {
-                it.stretches() || (k + 1 < vis_end && items.get(k + 1).is_some_and(|next| crate::breaks::justify_between(it.ch, next.ch)))
+                it.stretches()
+                    || (k + 1 < vis_end
+                        && items.get(k + 1).is_some_and(|next| {
+                            next.char_start > it.char_start
+                                && cluster_start.get(next.char_start).copied().unwrap_or(true)
+                                && crate::breaks::justify_between(it.ch, next.ch)
+                        }))
             })
         };
         let gaps = (*s..vis_end).filter(|k| widens(*k)).count();
@@ -1354,14 +1403,17 @@ pub fn layout(text: &TextData, box_width: Option<f64>, box_height: Option<f64>)
         } else {
             (0..line_items.len()).collect()
         };
-        for k in order.iter().copied() {
+        // Each gap opens at its visual boundary; trailing spaces widen where they are.
+        let after: Vec<bool> = (0..line_items.len()).map(|k| widens(*s + k)).collect();
+        let gaps_at = visual_gaps(&order, &after, vis_end - *s);
+        for (i, &k) in order.iter().enumerate() {
             let Some(it) = line_items.get(k) else { continue };
             if is_line_break(it.ch) {
                 continue;
             }
             out.glyphs.extend(glyph_at(&styles, it, pen, baseline));
             pen += it.adv;
-            if widens(*s + k) {
+            if gaps_at.get(i).copied().unwrap_or(false) {
                 pen += justify_extra;
             }
         }

crates/text/src/tests.rs

@@ -608,11 +608,14 @@ fn justified_ideographs_spread_the_room_between_characters() {
     };
     // Box width cut inside char `fit`, so the first line is chars 0..fit; how many gaps lie before
     // each of its chars.
-    for (s, fit, gaps_before, why) in [
-        ("一二三四五六七八九十", 6, [0, 1, 2, 3, 4, 5], "one gap between each pair of ideographs"),
-        ("一二三。四五六七八九", 6, [0, 1, 2, 2, 3, 4], "none before the full stop, which never starts a line"),
-        ("一二三「四五六七八九", 6, [0, 1, 2, 3, 3, 4], "none after the opening bracket, which never ends a line"),
-    ] {
+    let rows: [(&str, usize, &[i32], &str); 5] = [
+        ("一二三四五六七八九十", 6, &[0, 1, 2, 3, 4, 5], "one gap between each pair of ideographs"),
+        ("一二三。四五六七八九", 6, &[0, 1, 2, 2, 3, 4], "none before the full stop, which never starts a line"),
+        ("一二三「四五六七八九", 6, &[0, 1, 2, 3, 3, 4], "none after the opening bracket, which never ends a line"),
+        ("一二三\u{20DD}四五六七八九", 6, &[0, 1, 2, 2, 3, 4], "none inside a grapheme cluster: a combining mark stays on its base"),
+        ("漢אב字一二三", 4, &[0, 1, 1, 2], "beside a right-to-left word (laid out ב then א), not inside it"),
+    ];
+    for (s, fit, gaps_before, why) in rows {
         let natural = lay_out(s, TextAlignH::Left, None);
         let (fits, next) = (natural.caret(fit).0, natural.caret(fit + 1).0);
         assert!(next > fits, "{why}: chars need advances");
@@ -621,6 +624,7 @@ fn justified_ideographs_spread_the_room_between_characters() {
         let just = lay_out(s, TextAlignH::Justified, Some(w));
         assert!(just.lines.len() >= 2 && just.lines[0].char_end == fit, "{why}: {:?}", just.lines);
         let extra = (w - left.lines[0].width) / f64::from(gaps_before[fit - 1]);
+        assert_eq!(left.lines[0].char_end, fit, "{why}: {:?}", left.lines);
         assert!(extra > 0.0, "{why}");
         assert_eq!(left.glyphs.len(), just.glyphs.len());
         for (l, j) in left.glyphs.iter().zip(&just.glyphs).filter(|(g, _)| g.char_end <= fit) {