Merge pull request #7439 from dotCooCoo/fix/single-line-composer-remeasure

Type: the Single-line Composer measures a line's room across its tallest character

crates/text/src/layout/flow.rs

@@ -24,6 +24,10 @@ fn pushes_in(para: &ParaStyle) -> bool {
 /// Paragraphs longer than this (glyphs) are always broken line by line.
 const MAX_COMPOSE_GLYPHS: usize = 200_000;
 
+/// The widths tried, halving the difference each time, for the longest break of a line placed again
+/// at its own metrics in more room than it was broken in ([`place_flow_line`]).
+const REFIT_STEPS: usize = 24;
+
 /// Whether every line of area type in `regions` is as wide however tall it is and wherever it is set:
 /// rectangular cells without text wrap.
 fn constant_width(regions: Option<&[Region]>) -> bool {
@@ -415,25 +419,31 @@ fn place_flow_line(cx: &mut Ctx<'_>, pen: &mut Pen<'_>, context: &mut PlacementC
     let word = word_width(sg, *i, line_end, para.kinsoku);
     // A composed line in a frame whose lines are not all as wide is placed at its own tallest
     // metrics, as the composer placed it (`shaped::Shaped`): its room is the room across its height.
-    let est = match composed.as_ref().and_then(|lines| lines.get(*para_line)) {
+    let composed_line = composed.as_ref().and_then(|lines| lines.get(*para_line)).is_some_and(|&(end, _)| end > *i);
+    let mut est = match composed.as_ref().and_then(|lines| lines.get(*para_line)) {
         Some(&(end, _)) if end > *i && !constant_width(pen.regions) => sg.get(*i..end).and_then(Metrics::max).unwrap_or(est),
         _ => est,
     };
+    // A line broken here in such a frame is placed again when it turns out taller than it was
+    // placed: its room is the room across its own height.
+    let refit = !composed_line && pen.regions.is_some() && !constant_width(pen.regions);
     // Place, break, then settle the baseline on the line's real metrics (moving on to the
     // next row/column if it no longer fits).
     loop {
+        let before = refit.then(|| pen.clone());
         let first_in_region = pen.prev.is_none();
-        let Some((mut baseline, x0, x1, same_baseline)) = pen.place(est, ind_l, ind_r, word) else {
+        let Some((mut baseline, mut x0, mut x1, same_baseline)) = pen.place(est, ind_l, ind_r, word) else {
             cx.out.overflow = cx.text.len() > if *i < n { sg[*i].byte } else { pr.start };
             return None;
         };
-        let width = x1 - x0 - ind_l - ind_r;
         let hyphenate = (para.hyphenate && (*hyphen_limit == 0 || *hyphen_run < *hyphen_limit)).then_some((&para.hyphenation, *zone));
-        let (end, hyph, pushed_in) = match composed.as_ref().and_then(|lines| lines.get(*para_line)) {
+        let glyphs: &[SGlyph] = sg;
+        // The line broken in room `width` wide: its end, whether it ends with a hyphen, and whether
+        // it pushed characters in.
+        let break_in = |width: f64| match composed.as_ref().and_then(|lines| lines.get(*para_line)) {
             Some(&(end, hyph)) if end > *i => (end, hyph, false),
             _ if *i < n => {
                 let start = *i;
-                let glyphs: &[SGlyph] = sg;
                 // A horizontal line with right-to-left text is drawn in another order than its
                 // text's, and its tabs take their stops in the order they are drawn: whether the line
                 // ending before glyph `end` fits is measured as it is drawn ([`drawn_width`]).
@@ -478,8 +488,56 @@ fn place_flow_line(cx: &mut Ctx<'_>, pen: &mut Pen<'_>, context: &mut PlacementC
             }
             _ => (n, false, false),
         };
-        let m = Metrics::max(&sg[*i..end]).unwrap_or(*pm);
-        baseline += if same_baseline || (pen.regions.is_none() && first_in_region) {
+        let (mut end, mut hyph, mut pushed_in) = break_in(x1 - x0 - ind_l - ind_r);
+        let metrics = |end: usize| glyphs.get(*i..end).and_then(Metrics::max).unwrap_or(*pm);
+        let m = metrics(end);
+        // The baseline moves from where `est` placed it to where the line's metrics set it, unless
+        // the line was placed at metrics of its own (below).
+        let mut settle = true;
+        if let Some(before) = before
+            && m != est
+        {
+            if m.union(est) != est {
+                // Taller than it was placed: place it again at metrics that hold both, and break it
+                // again in the room there.
+                *pen = before;
+                est = est.union(m);
+                continue;
+            }
+            // Broken again in the room of a taller line, it no longer holds what made that line
+            // taller. It goes where its own metrics place it when the room there holds the room it
+            // was broken in, and otherwise stays where it was placed, inside the room measured.
+            settle = false;
+            let mut own = before;
+            if let Some((b, a, z, _)) = own.place(m, ind_l, ind_r, word)
+                && a <= x0 + EPS
+                && z >= x1 - EPS
+            {
+                // The room there may hold more of the line, up to what would make it taller than
+                // `m`: the longest break that does not is between the two widths (a line breaks no
+                // earlier in more room).
+                let (mut lo, mut hi) = (x1 - x0 - ind_l - ind_r, z - a - ind_l - ind_r);
+                let wider = break_in(hi);
+                if metrics(wider.0).union(m) == m {
+                    (end, hyph, pushed_in) = wider;
+                } else {
+                    for _ in 0..REFIT_STEPS {
+                        let mid = (lo + hi) / 2.0;
+                        let line = break_in(mid);
+                        if metrics(line.0).union(m) == m {
+                            (end, hyph, pushed_in) = line;
+                            lo = mid;
+                        } else {
+                            hi = mid;
+                        }
+                    }
+                }
+                *pen = own;
+                (baseline, x0, x1) = (b, a, z);
+            }
+        }
+        let m = metrics(end);
+        baseline += if !settle || same_baseline || (pen.regions.is_none() && first_in_region) {
             0.0
         } else if pen.model == LeadingModel::EmBoxTop {
             // The em box top is fixed (the frame's top, or the line above's): the baseline

crates/text/src/tests/combos.rs

@@ -378,8 +378,8 @@ fn x_width() -> f64 {
 fn composer_x_mixed_sizes_in_a_stepped_frame() {
     // A frame as wide as the first line down to 40 pt and half as wide below. A 36 pt word makes
     // the line it is on reach below the step, where the frame is narrow: the every-line composer
-    // breaks the paragraph with each line's measure where its height sets it, not as the
-    // single-line composer does, and every line keeps within its room.
+    // breaks the paragraph with each line's measure where its height sets it, and every line keeps
+    // within its room.
     let w = x_width();
     let set = |x: CharStyle, composer: Composer| {
         let mut t = stepped(w, w * 0.5, 40.0, 300.0);
@@ -394,8 +394,10 @@ fn composer_x_mixed_sizes_in_a_stepped_frame() {
     // The 36 pt line reaches below the step and is set at the narrow measure.
     let tall = &l.lines[1];
     assert!(tall.ascent > 30.0 && tall.baseline + tall.descent > 40.0 && tall.avail.1 < w * 0.5 + 0.01, "{:?}", l.lines);
+    // The single-line composer measures each line across its height too, and breaks this
+    // paragraph the same way.
     let (greedy, _) = set(sans(36.0), Composer::SingleLine);
-    assert_ne!(every, greedy);
+    assert_eq!(every, greedy);
     // Uniform sizes keep the breaks they had.
     let (uniform, _) = set(sans(12.0), Composer::EveryLine);
     assert_eq!(uniform, ["Destroy all creatures with toughness X or", "less. Then draw a card for each creature", "destroyed this way."]);
@@ -435,8 +437,10 @@ fn composer_x_mixed_sizes_around_text_wrap() {
         across && !text[g.byte..].starts_with(char::is_whitespace) && g.origin.x < object.x1 && g.origin.x + g.advance > object.x0
     });
     assert_eq!(hits.count(), 0, "{:?}", l.lines);
+    // The single-line composer measures each line across its height too, and breaks this
+    // paragraph the same way.
     let (greedy, _, _) = set(Composer::SingleLine);
-    assert_ne!(every, greedy);
+    assert_eq!(every, greedy);
 }
 
 #[test]
@@ -463,13 +467,42 @@ fn composer_x_mixed_sizes_in_a_frame_that_widens() {
     assert_eq!(greedy, ["Destroy all", "creatures", "with toughness X or less. Then draw", "a card for each creature destroyed this", "way."]);
 }
 
+#[test]
+fn single_line_composer_measures_a_line_across_its_tallest_glyph() {
+    // A frame wide enough for "Destroy all creatures X" down to 20 pt and as wide as "Destroy all"
+    // below, and a rectangle as wide with an object (6 pt wrap offset) beside that part. The first
+    // line starts with 12 pt words and takes in a 36 pt word, which sets it lower and reaches below
+    // 20 pt: the single-line composer measures its room across that height, every line that
+    // reaches the narrow part is set in it, and the first line keeps the 12 pt words that fit
+    // above it.
+    let w = natural_width("Destroy all creatures X", 12.0) + 30.0;
+    let narrow = natural_width("Destroy all", 12.0) + 2.0;
+    let runs = vec![run("Destroy all creatures ", sans(12.0)), run("X", sans(36.0)), run(" or less. Then draw a card.", sans(12.0))];
+    let object = Rect::new(narrow + 6.0, 26.0, w + 10.0, 300.0);
+    let mut beside = area(runs.clone(), Rect::new(0.0, 0.0, w, 300.0));
+    beside.wrap = vec![vectorcraft_doc::WrapShape { path: PathData::from_bezpath(&object.to_path(0.1)), wrap: Default::default() }];
+    let mut shaped = stepped(w, narrow, 20.0, 300.0);
+    shaped.runs = runs;
+    for (name, mut t) in [("stepped frame", shaped), ("text wrap", beside)] {
+        t.para.composer = Composer::SingleLine;
+        let l = layout(db(), &t);
+        assert!(past_their_room(&t, &l).is_empty() && !l.overflow, "{name}: {:?}", l.lines);
+        let low = l.lines.iter().filter(|li| li.baseline + li.descent > 20.01);
+        assert!(low.clone().all(|li| li.avail.1 <= narrow + 0.01), "{name}: {:?}", l.lines);
+        // The 36 pt word is set, on a line in the narrow part.
+        assert!(low.clone().any(|li| li.ascent > 30.0), "{name}: {:?}", l.lines);
+        assert_eq!(line_texts(&t, &l)[0], "Destroy all creatures", "{name}: {:?}", l.lines);
+        assert!(l.lines[0].baseline < 20.0 && l.lines[0].avail.1 > w - 0.01, "{name}: {:?}", l.lines);
+    }
+}
+
 #[test]
 fn composer_x_mixed_sizes_overflow_a_shaped_frame() {
     // A stepped frame too short for the paragraph: the every-line composer still breaks the lines
     // that fit as one paragraph, with the lines past the frame's end measured as wide as the last
     // line the frame holds. Here that is the narrow part, so the lines that fit break as in a frame
-    // tall enough for the whole paragraph, and not as the single-line composer breaks them. They
-    // are set in order, each once and within its room, and the frame overflows.
+    // tall enough for the whole paragraph. They are set in order, each once and within its room,
+    // and the frame overflows.
     let w = x_width();
     let set = |bottom: f64, composer: Composer| {
         let mut t = stepped(w, w * 0.5, 40.0, bottom);
@@ -487,8 +520,10 @@ fn composer_x_mixed_sizes_overflow_a_shaped_frame() {
     assert_eq!(every, ["Destroy all creatures with toughness", "X or less. Then", "draw a card for each"]);
     let (whole, ..) = set(300.0, Composer::EveryLine);
     assert_eq!(every, whole[..every.len()]);
+    // The single-line composer measures each line across its height too, and breaks the lines
+    // that fit the same way.
     let (greedy, ..) = set(75.0, Composer::SingleLine);
-    assert_ne!(every, greedy);
+    assert_eq!(every, greedy);
     // A frame with no room for a line sets none.
     for composer in [Composer::EveryLine, Composer::SingleLine] {
         let mut t = stepped(w, w * 0.5, 4.0, 8.0);
@@ -529,8 +564,10 @@ fn composer_x_mixed_sizes_overflow_around_text_wrap() {
     assert_eq!(hits.count(), 0, "{:?}", l.lines);
     // The lines that fit break as in `composer_x_mixed_sizes_around_text_wrap`'s taller frame.
     assert_eq!(every, ["Destroy all creatures with", "toughness\u{a0}X", "or less. Then", "draw a card for", "each creature"]);
+    // The single-line composer measures each line across its height too, and breaks the lines
+    // that fit the same way.
     let (greedy, ..) = set(Composer::SingleLine);
-    assert_ne!(every, greedy);
+    assert_eq!(every, greedy);
 }
 
 #[test]

crates/text/src/thread/tests.rs

@@ -415,8 +415,7 @@ fn stepped_frame(w: f64, narrow: f64, y: f64, bottom: f64) -> TextObject {
 
 /// A paragraph of mixed type sizes that a stepped frame cannot hold goes on in the next frame. The
 /// every-line composer breaks the lines the first frame holds as one paragraph, as in a frame tall
-/// enough for all of it and not as the single-line composer breaks them, and the first frame
-/// takes the text of those lines. The next frame sets the rest at its own width, without the
+/// enough for all of it, and the first frame takes the text of those lines. The next frame sets the rest at its own width, without the
 /// first-line indent, and no character is lost or repeated. The frames take the same shares as
 /// when each is measured with the whole rest of the story.
 #[test]
@@ -460,8 +459,10 @@ fn a_mixed_size_paragraph_continued_from_a_shaped_frame_is_composed_in_both() {
         assert_eq!(every, whole[..every.len()], "indent {indent}");
         if indent == 0.0 {
             assert_eq!(every, ["Destroy all creatures with toughness", "X or less. Then", "draw a card for each"]);
+            // The single-line composer measures each line across its height too, and breaks the
+            // first frame's lines the same way.
             let (greedy, ..) = shares(Composer::SingleLine, indent);
-            assert_eq!(greedy, ["Destroy all creatures with toughness X", "or less. Then draw a", "card for each", "creature destroyed"]);
+            assert_eq!(greedy, every);
         }
         // The next frame starts at its left edge, and its lines keep within it.
         assert!((line_start(&lays[1], 0) - 200.0).abs() < 1e-6, "{}", line_start(&lays[1], 0));

docs/mcp.md

@@ -10670,6 +10670,10 @@ threaded frame, which composes them again at its own widths. Such a paragraph fa
 frame has no room for its first line, or when the search for its breaks passes VectorCraft's own limits (1,000,000
 lines tried and 2,000,000 baseline steps for one paragraph, 2,000,000 lines and 4,000,000 steps for all the
 paragraphs of a frame).
+Single-line measures each line's room the same way: a line that takes in a taller character is placed again across its
+new height and broken again in the room there. When that leaves the taller character for the next line, the line is
+set at its own height where the room there is no narrower, with the words before that character that fit there, and
+otherwise stays where it was placed.
 The UI command `type.lineComposer` (Cmd+Alt+Shift+C, in remote mode `press_key {key: "C", mods: {cmd: true, alt: true,
 shift: true}}` or `invoke_menu`) sets `singleLine` when every paragraph of the selected type (or every paragraph the
 Type tool's selection touches) uses Every-line, else `everyLine`, and returns the composer it set.