Merge pull request #7444 from dotCooCoo/fix/later-span-taller-line-clears

Type: a taller line in a further span beside a wrap object no longer reaches into the line above

crates/text/src/layout/breaking.rs

@@ -53,6 +53,15 @@ impl Metrics {
     }
 }
 
+/// What a line band was placed below: the baseline of the line above it and the em box top that
+/// line left for the next one (both `None` for a cell's first band), and the space pending between.
+#[derive(Clone, Copy, Default)]
+pub(super) struct Above {
+    pub(super) prev: Option<f64>,
+    pub(super) next_top: Option<f64>,
+    pub(super) pending: f64,
+}
+
 /// Where the next line goes: the region being filled and the previous baseline in it.
 #[derive(Clone)]
 pub(super) struct Pen<'r> {
@@ -64,6 +73,8 @@ pub(super) struct Pen<'r> {
     pub(super) fb_min: f64,
     /// Further spans at the current baseline (text wrapping on both sides of an object).
     pub(super) queued: Vec<(f64, f64, f64)>,
+    /// What the line band at the current baseline was placed below (see [`Above`]).
+    pub(super) above: Above,
     /// Leading measured from em box top to em box top: where the next line's em box top goes.
     pub(super) model: LeadingModel,
     pub(super) next_top: Option<f64>,
@@ -108,16 +119,21 @@ impl Pen<'_> {
     /// [`Pen::place`], skipping lines of a non-rectangular frame narrower than `min`.
     fn place_at_least(&mut self, est: Metrics, ind_l: f64, ind_r: f64, min: f64) -> Option<(f64, f64, f64, bool)> {
         let Some(regions) = self.regions else {
-            let b = self.prev.map_or(0.0, |b| self.next_baseline(b, est));
+            let b = self.prev.map_or(0.0, |b| self.next_baseline(b, self.next_top, self.pending, est));
             return Some((b, f64::NEG_INFINITY, f64::INFINITY, false));
         };
         // A further span was measured across the height of the line set before it on its baseline.
         // This line gets the part of it that is free across its own height: a taller line can
         // reach a wrap object the line before it stayed clear of. A span with no such part is
-        // left empty, and so is a part the line's height made narrower than `min`.
+        // left empty, and so is a part the line's height made narrower than `min`. A span on a
+        // baseline nearer the line above the band than this line's own leading sets it is left
+        // empty too: a taller line would reach into that line.
         while !self.queued.is_empty() {
             let (b, x0, x1) = self.queued.remove(0);
             let Some(r) = regions.get(self.ri) else { return Some((b, x0, x1, true)) };
+            if b + EPS < self.band_baseline(r, est) {
+                continue;
+            }
             let mut free = r.spans(b - est.asc, b + est.desc).into_iter().filter_map(|(s, e)| {
                 let (s, e) = (s.max(x0), e.min(x1));
                 let room = e - s - ind_l - ind_r;
@@ -132,11 +148,8 @@ impl Pen<'_> {
         }
         loop {
             let r = regions.get(self.ri)?;
-            let mut baseline = match self.prev {
-                None if self.model == LeadingModel::EmBoxTop => r.top() + r.shift + est.top.max(self.fb_min),
-                None => r.top() + r.shift + est.first_baseline(self.fb, self.fb_min),
-                Some(b) => self.next_baseline(b, est) + r.gap,
-            };
+            self.above = Above { prev: self.prev, next_top: self.next_top, pending: self.pending };
+            let mut baseline = self.band_baseline(r, est);
             // The baseline moves down the cell a lead at a time, or a `MAX_STEPS`th of the cell's
             // height when that is more, in at most `MAX_STEPS` steps.
             let step = est.lead.max(1.0).max(r.cell.height() / MAX_STEPS as f64);
@@ -188,13 +201,30 @@ impl Pen<'_> {
     pub(super) fn bottom(&self) -> f64 {
         self.regions.and_then(|r| r.get(self.ri)).map_or(f64::INFINITY, |r| r.bottom())
     }
-    /// The baseline of the line after the one at `prev`, for a line of metrics `est`.
-    fn next_baseline(&self, prev: f64, est: Metrics) -> f64 {
-        match (self.model, self.next_top) {
-            (LeadingModel::EmBoxTop, Some(top)) => top + est.top + self.pending,
-            _ => prev + est.lead + self.pending,
+    /// The baseline of the line after the one at `prev`, which left the next em box top at
+    /// `next_top`, for a line of metrics `est` with `pending` space between them.
+    fn next_baseline(&self, prev: f64, next_top: Option<f64>, pending: f64, est: Metrics) -> f64 {
+        match (self.model, next_top) {
+            (LeadingModel::EmBoxTop, Some(top)) => top + est.top + pending,
+            _ => prev + est.lead + pending,
         }
     }
+    /// The highest baseline in cell `r` for a line of metrics `est` in the current line band: as
+    /// far below the line above the band ([`Pen::above`]) as its own leading sets it, or at the
+    /// cell's first baseline.
+    fn band_baseline(&self, r: &Region, est: Metrics) -> f64 {
+        let Above { prev, next_top, pending } = self.above;
+        match prev {
+            None if self.model == LeadingModel::EmBoxTop => r.top() + r.shift + est.top.max(self.fb_min),
+            None => r.top() + r.shift + est.first_baseline(self.fb, self.fb_min),
+            Some(b) => self.next_baseline(b, next_top, pending, est) + r.gap,
+        }
+    }
+    /// A line of metrics `m` set in a further span at baseline `b` is no nearer the line above
+    /// the band than its own leading sets it.
+    pub(super) fn clears_above(&self, b: f64, m: Metrics) -> bool {
+        self.regions.and_then(|r| r.get(self.ri)).is_none_or(|r| b + EPS >= self.band_baseline(r, m))
+    }
     /// A line was set at `baseline` with metrics `m`: where the next one goes from.
     pub(super) fn settled(&mut self, baseline: f64, m: Metrics) {
         self.prev = Some(baseline);
@@ -967,6 +997,7 @@ fn a_pen_out_of_baseline_steps_finds_no_room() {
         fb: FirstBaseline::Ascent,
         fb_min: 0.0,
         queued: vec![],
+        above: Default::default(),
         model: LeadingModel::default(),
         next_top: None,
         no_room_for: f64::INFINITY,

crates/text/src/layout/flow.rs

@@ -177,6 +177,7 @@ pub(super) fn flow(cx: &mut Ctx<'_>, paras: &[Range<usize>], t: &TextObject, reg
         fb: cx.opts.first_baseline,
         fb_min: cx.opts.first_baseline_min,
         queued: vec![],
+        above: Default::default(),
         model: t.para_at(0).leading_model,
         next_top: None,
         no_room_for: f64::INFINITY,
@@ -538,6 +539,11 @@ fn place_flow_line(cx: &mut Ctx<'_>, pen: &mut Pen<'_>, context: &mut PlacementC
             }
         }
         let m = metrics(end);
+        if same_baseline && !pen.clears_above(baseline, m) {
+            // A glyph taller than the first one sets the line lower than the span's baseline: the
+            // span is left empty, and the line goes in the next span or on the next baseline.
+            continue;
+        }
         baseline += if !settle || same_baseline || (pen.regions.is_none() && first_in_region) {
             0.0
         } else if pen.model == LeadingModel::EmBoxTop {

crates/text/src/layout/shaped.rs

@@ -91,8 +91,9 @@ struct Placed<'r> {
 
 /// What a line's placement depends on: its first glyph, whether it is the paragraph's first line,
 /// the pen's cell, baselines, pending space, narrowest word without room and further spans, the
-/// span of the line above and whether the line is past the frame's end.
-type Key = (usize, bool, usize, Option<u64>, Option<u64>, u64, u64, Vec<[u64; 3]>, Option<u64>, bool);
+/// span of the line above, whether the line is past the frame's end, and what the further spans'
+/// band was placed below.
+type Key = (usize, bool, usize, Option<u64>, Option<u64>, u64, u64, Vec<[u64; 3]>, Option<u64>, bool, (Option<u64>, Option<u64>, u64));
 
 fn key(pos: usize, first: bool, pen: &Pen<'_>, span: Option<f64>, past: bool) -> Key {
     (
@@ -106,6 +107,7 @@ fn key(pos: usize, first: bool, pen: &Pen<'_>, span: Option<f64>, past: bool) ->
         pen.queued.iter().map(|&(b, x0, x1)| [b.to_bits(), x0.to_bits(), x1.to_bits()]).collect(),
         span.map(f64::to_bits),
         past,
+        (pen.above.prev.map(f64::to_bits), pen.above.next_top.map(f64::to_bits), pen.above.pending.to_bits()),
     )
 }
 

crates/text/src/layout/wrapping_tests.rs

@@ -911,7 +911,8 @@ fn a_further_span_is_measured_across_the_height_of_its_line() {
         wrap: vectorcraft_doc::TextWrap { offset: 0.0, invert: false },
     };
     let small = breaking::Metrics { asc: 10.0, desc: 3.0, lead: 14.0, cap: 7.0, xh: 5.0, top: 10.0 };
-    let tall = breaking::Metrics { asc: 30.0, desc: 9.0, lead: 42.0, cap: 21.0, xh: 15.0, top: 30.0 };
+    // The tall line is set at the small line's leading, so it may share the small line's baseline.
+    let tall = breaking::Metrics { asc: 30.0, desc: 9.0, lead: 14.0, cap: 21.0, xh: 15.0, top: 30.0 };
     // An object down the middle of the frame splits every line in two, and `above` lies right of
     // it over y 20 to 40. A small line after a baseline of 50 is set at 64, over y 54 to 67, clear
     // of `above`; a tall line in the span right of it on that baseline covers y 34 to 73.
@@ -926,6 +927,7 @@ fn a_further_span_is_measured_across_the_height_of_its_line() {
             fb: FirstBaseline::Ascent,
             fb_min: 0.0,
             queued: vec![],
+            above: Default::default(),
             model: LeadingModel::RomanBaseline,
             next_top: None,
             no_room_for: f64::INFINITY,
@@ -946,17 +948,72 @@ fn a_further_span_is_measured_across_the_height_of_its_line() {
     assert!(near(right, (64.0, 220.0, 300.0, true)), "{right:?}");
     // That part is 80 wide: a tall line that starts with a wider word goes on the next baseline.
     let (_, right) = set(Rect::new(300.0, 20.0, 400.0, 40.0), 100.0);
-    assert!(near(right, (106.0, 0.0, 180.0, false)), "{right:?}");
+    assert!(near(right, (78.0, 0.0, 180.0, false)), "{right:?}");
     // `above` covers the whole span: the tall line goes on the next baseline, left of the object.
     let (left, right) = set(Rect::new(220.0, 20.0, 400.0, 40.0), 0.0);
     assert!(near(left, (64.0, 0.0, 180.0, false)), "{left:?}");
-    assert!(near(right, (106.0, 0.0, 180.0, false)), "{right:?}");
+    assert!(near(right, (78.0, 0.0, 180.0, false)), "{right:?}");
     // `above` lies below the tall line: the span is as wide for it as for the small line, and it
     // takes the span whatever its first word, as a further span always did.
     let (_, right) = set(Rect::new(300.0, 300.0, 400.0, 320.0), 200.0);
     assert!(near(right, (64.0, 220.0, 400.0, true)), "{right:?}");
 }
 
+/// A further span goes only to a line whose own leading sets it no higher than the span's
+/// baseline: a taller line leaves the span empty and goes on the next baseline, so its ascent does
+/// not reach into the line above the band. In a cell's first band, a taller line leaves a span
+/// whose baseline is above its own first baseline in the cell.
+#[test]
+fn a_further_span_is_left_to_a_line_its_leading_sets_there() {
+    use std::cell::Cell;
+    use std::rc::Rc;
+    let frame = Rect::new(0.0, 0.0, 400.0, 400.0).to_path(0.1);
+    let wraps = [vectorcraft_doc::WrapShape {
+        path: PathData::from_bezpath(&Rect::new(180.0, 0.0, 220.0, 400.0).to_path(0.1)),
+        wrap: vectorcraft_doc::TextWrap { offset: 0.0, invert: false },
+    }];
+    let regions = region::Region::cells(&frame, &LayoutOptions::default(), &wraps);
+    let small = breaking::Metrics { asc: 10.0, desc: 3.0, lead: 14.0, cap: 7.0, xh: 5.0, top: 10.0 };
+    let tall = breaking::Metrics { asc: 30.0, desc: 9.0, lead: 42.0, cap: 21.0, xh: 15.0, top: 30.0 };
+    let set = |prev: Option<f64>, model: LeadingModel, next: breaking::Metrics| {
+        let mut pen = breaking::Pen {
+            regions: Some(&regions),
+            ri: 0,
+            prev,
+            pending: 0.0,
+            fb: FirstBaseline::Ascent,
+            fb_min: 0.0,
+            queued: vec![],
+            above: Default::default(),
+            model,
+            next_top: prev.map(|b| b - small.top + small.lead),
+            no_room_for: f64::INFINITY,
+            steps_left: Rc::new(Cell::new(breaking::MAX_TOTAL_STEPS)),
+            search: Default::default(),
+        };
+        let left = pen.place(small, 0.0, 0.0, 0.0);
+        let at = left.map_or(f64::NAN, |l| l.0);
+        pen.settled(at, small);
+        let ok = pen.clears_above(at, next);
+        (left, pen.place(next, 0.0, 0.0, 0.0), ok)
+    };
+    let near = |got: Option<(f64, f64, f64, bool)>, want: (f64, f64, f64, bool)| {
+        got.is_some_and(|g| (g.0 - want.0).abs() < 0.01 && (g.1 - want.1).abs() < 0.01 && (g.2 - want.2).abs() < 0.01 && g.3 == want.3)
+    };
+    // The next baseline for the tall line after the small one at 64, and after the small one at 10.
+    for (model, after_64, after_10) in [(LeadingModel::RomanBaseline, 106.0, 52.0), (LeadingModel::EmBoxTop, 98.0, 44.0)] {
+        // After a line at 50, the small line is set at 64. A small line takes the span right of the
+        // object on that baseline; a tall line, which its leading sets lower, goes on the next one.
+        let (left, right, ok) = set(Some(50.0), model, small);
+        assert!(near(left, (64.0, 0.0, 180.0, false)) && near(right, (64.0, 220.0, 400.0, true)) && ok, "{model:?}: {left:?} {right:?}");
+        let (_, right, ok) = set(Some(50.0), model, tall);
+        assert!(near(right, (after_64, 0.0, 180.0, false)) && !ok, "{model:?}: {right:?}");
+        // The cell's first band: the small line at 10, the tall line on the next baseline.
+        let (left, right, ok) = set(None, model, tall);
+        assert!(near(left, (10.0, 0.0, 180.0, false)) && near(right, (after_10, 0.0, 180.0, false)) && !ok, "{model:?}: {left:?} {right:?}");
+    }
+}
+
 /// The every-line composer's searches for the paragraphs of one frame share one budget: each takes
 /// the lines it tried and the baseline steps it spent from it, the layout's own steps stay as they
 /// were, and once the budget runs out the next paragraph's search stops (it is broken line by line).
@@ -990,6 +1047,7 @@ fn shaped_composition_searches_share_the_frame_budget() {
         fb: FirstBaseline::Ascent,
         fb_min: 0.0,
         queued: vec![],
+        above: Default::default(),
         model: LeadingModel::default(),
         next_top: None,
         no_room_for: f64::INFINITY,

crates/text/src/tests/combos.rs

@@ -443,6 +443,36 @@ fn composer_x_mixed_sizes_around_text_wrap() {
     assert_eq!(every, greedy);
 }
 
+#[test]
+fn composer_x_taller_line_in_a_further_span() {
+    // A wrap object down the middle of the frame splits every line in two. The 36 pt word falls in
+    // the span right of it on the second baseline, after a 12 pt line left of it: the line that
+    // holds it is set as far below the line above it as its own leading sets it, and its ascent
+    // stays clear of that line, with either composer.
+    let w = x_width();
+    let object = Rect::new(w, -10.0, w + 40.0, 310.0);
+    for composer in [Composer::SingleLine, Composer::EveryLine] {
+        let mut runs = vec![run(&format!("{RAG} Then draw a card for each creature destroyed this way. "), sans(12.0))];
+        runs.extend(with_x(sans(36.0)));
+        let mut t = area(runs, Rect::new(0.0, 0.0, 2.0 * w + 40.0, 300.0));
+        t.wrap = vec![vectorcraft_doc::WrapShape { path: PathData::from_bezpath(&object.to_path(0.1)), wrap: Default::default() }];
+        t.para.composer = composer;
+        let l = layout(db(), &t);
+        assert!(!l.overflow, "{composer:?}: {:?}", l.lines);
+        let tall: Vec<_> = l.lines.iter().filter(|line| line.ascent > 30.0).collect();
+        assert_eq!(tall.len(), 1, "{composer:?}: {:?}", l.lines);
+        for line in tall {
+            for above in l.lines.iter().filter(|a| a.baseline < line.baseline - 0.01) {
+                let beside = above.avail.0 < line.avail.1 && line.avail.0 < above.avail.1;
+                assert!(
+                    !beside || line.baseline - line.ascent >= above.baseline + above.descent - 0.01,
+                    "{composer:?}: {line:?} reaches into {above:?}"
+                );
+            }
+        }
+    }
+}
+
 #[test]
 fn composer_x_mixed_sizes_in_a_frame_that_widens() {
     // A frame as wide as "Destroy all" down to 20 pt and as wide as the first line of