Refactor paragraph layout

src/library/align.rs

@@ -40,7 +40,6 @@ impl Layout for AlignNode {
 
         // Layout the child.
         let mut frames = self.child.layout(vm, &pod, passed.chain(&styles))?;
-
         for ((current, base), Constrained { item: frame, cts }) in
             regions.iter().zip(&mut frames)
         {
@@ -78,5 +77,4 @@ castable! {
         aligns
     },
     @aligns: Spec<Align> => aligns.map(Some),
-
 }

src/library/par.rs

@@ -8,6 +8,7 @@ use xi_unicode::LineBreakIterator;
 
 use super::prelude::*;
 use super::{shape, ShapedText, SpacingKind, TextNode};
+use crate::font::FontStore;
 use crate::util::{ArcExt, EcoString, RangeExt, SliceExt};
 
 /// Arrange text, spacing and inline-level nodes into a paragraph.
@@ -88,17 +89,93 @@ impl Layout for ParNode {
     ) -> TypResult<Vec<Constrained<Arc<Frame>>>> {
         // Collect all text into one string used for BiDi analysis.
         let text = self.collect_text();
-
-        // Find out the BiDi embedding levels.
         let level = Level::from_dir(styles.get(Self::DIR));
         let bidi = BidiInfo::new(&text, level);
 
         // Prepare paragraph layout by building a representation on which we can
         // do line breaking without layouting each and every line from scratch.
-        let layouter = ParLayouter::new(self, vm, regions, &styles, bidi)?;
+        let par = ParLayout::new(vm, self, bidi, regions, &styles)?;
+        let fonts = &mut *vm.fonts;
+        let em = styles.get(TextNode::SIZE).abs;
+        let align = styles.get(ParNode::ALIGN);
+        let leading = styles.get(ParNode::LEADING).resolve(em);
 
-        // Find suitable linebreaks.
+        // The already determined lines and the current line attempt.
-        Ok(layouter.layout(vm, regions.clone()))
+        let mut lines = vec![];
+        let mut start = 0;
+        let mut last = None;
+
+        // Find suitable line breaks.
+        for (end, mandatory) in LineBreakIterator::new(&text) {
+            // Compute the line and its size.
+            let mut line = par.line(fonts, start .. end);
+
+            // If the line doesn't fit anymore, we push the last fitting attempt
+            // into the stack and rebuild the line from its end. The resulting
+            // line cannot be broken up further.
+            if !regions.current.x.fits(line.size.x) {
+                if let Some((last_line, last_end)) = last.take() {
+                    lines.push(last_line);
+                    start = last_end;
+                    line = par.line(fonts, start .. end);
+                }
+            }
+
+            // Finish the current line if there is a mandatory line break (i.e.
+            // due to "\n") or if the line doesn't fit horizontally already
+            // since no shorter line will be possible.
+            if mandatory || !regions.current.x.fits(line.size.x) {
+                lines.push(line);
+                start = end;
+                last = None;
+            } else {
+                last = Some((line, end));
+            }
+        }
+
+        if let Some((line, _)) = last {
+            lines.push(line);
+        }
+
+        // Determine the paragraph's width: Fit to width if we shoudn't expand
+        // and there's no fractional spacing.
+        let mut width = regions.current.x;
+        if !regions.expand.x && lines.iter().all(|line| line.fr.is_zero()) {
+            width = lines.iter().map(|line| line.size.x).max().unwrap_or_default();
+        }
+
+        // State for final frame building.
+        let mut regions = regions.clone();
+        let mut finished = vec![];
+        let mut first = true;
+        let mut output = Frame::new(Size::with_x(width));
+        let mut cts = Constraints::tight(&regions);
+
+        // Stack the lines into one frame per region.
+        for line in lines {
+            while !regions.current.y.fits(line.size.y) && !regions.in_last() {
+                finished.push(output.constrain(cts));
+                output = Frame::new(Size::with_x(width));
+                regions.next();
+                cts = Constraints::tight(&regions);
+                first = true;
+            }
+
+            if !first {
+                output.size.y += leading;
+            }
+
+            let frame = line.build(fonts, width, align);
+            let pos = Point::with_y(output.size.y);
+            output.size.y += frame.size.y;
+            output.merge_frame(pos, frame);
+
+            regions.current.y -= line.size.y + leading;
+            first = false;
+        }
+
+        finished.push(output.constrain(cts));
+        Ok(finished)
     }
 }
 
@@ -184,11 +261,7 @@ impl LinebreakNode {
 
 /// A paragraph representation in which children are already layouted and text
 /// is separated into shapable runs.
-struct ParLayouter<'a> {
+struct ParLayout<'a> {
-    /// How to align text in its line.
-    align: Align,
-    /// The spacing to insert between each line.
-    leading: Length,
     /// Bidirectional text embedding levels for the paragraph.
     bidi: BidiInfo<'a>,
     /// Spacing, separated text runs and layouted nodes.
@@ -212,14 +285,14 @@ enum ParItem<'a> {
     Frame(Frame),
 }
 
-impl<'a> ParLayouter<'a> {
+impl<'a> ParLayout<'a> {
     /// Prepare initial shaped text and layouted children.
     fn new(
-        par: &'a ParNode,
         vm: &mut Vm,
+        par: &'a ParNode,
+        bidi: BidiInfo<'a>,
         regions: &Regions,
         styles: &'a StyleChain<'a>,
-        bidi: BidiInfo<'a>,
     ) -> TypResult<Self> {
         let mut items = vec![];
         let mut ranges = vec![];
@@ -262,184 +335,46 @@ impl<'a> ParLayouter<'a> {
             }
         }
 
-        let em = styles.get(TextNode::SIZE).abs;
+        Ok(Self { bidi, items, ranges })
-        let align = styles.get(ParNode::ALIGN);
-        let leading = styles.get(ParNode::LEADING).resolve(em);
-
-        Ok(Self { align, leading, bidi, items, ranges })
     }
 
-    /// Find first-fit line breaks and build the paragraph.
-    fn layout(self, vm: &mut Vm, regions: Regions) -> Vec<Constrained<Arc<Frame>>> {
-        let mut stack = LineStack::new(self.leading, regions);
-
-        // The current line attempt.
-        // Invariant: Always fits into `stack.regions.current`.
-        let mut last = None;
-
-        // The start of the line in `last`.
-        let mut start = 0;
-
-        // Find suitable line breaks.
-        // TODO: Provide line break opportunities on alignment changes.
-        for (end, mandatory) in LineBreakIterator::new(self.bidi.text) {
-            // Compute the line and its size.
-            let mut line = LineLayout::new(vm, &self, start .. end);
-
-            // If the line doesn't fit anymore, we push the last fitting attempt
-            // into the stack and rebuild the line from its end. The resulting
-            // line cannot be broken up further.
-            if !stack.regions.current.fits(line.size) {
-                if let Some((last_line, last_end)) = last.take() {
-                    let fits =
-                        stack.regions.current.zip(line.size).map(|(c, s)| c.fits(s));
-
-                    // Since the new line try did not fit, no region that would
-                    // fit the line will yield the same line break. Therefore,
-                    // the width of the region must not fit the width of the
-                    // tried line.
-                    if !fits.x {
-                        stack.cts.max.x.set_min(line.size.x);
-                    }
-
-                    // Same as above, but for height.
-                    if !fits.y {
-                        let too_large = stack.size.y + self.leading + line.size.y;
-                        stack.cts.max.y.set_min(too_large);
-                    }
-
-                    // Don't start new lines at every opportunity when we are
-                    // overflowing.
-                    if !stack.overflowing || !fits.x {
-                        stack.push(last_line);
-                        stack.cts.min.y = Some(stack.size.y);
-                        start = last_end;
-                        line = LineLayout::new(vm, &self, start .. end);
-                    }
-                }
-            }
-
-            // If the line does not fit vertically, we start a new region.
-            while !stack.regions.current.y.fits(line.size.y) {
-                if stack.regions.in_last() {
-                    stack.overflowing = true;
-                    break;
-                }
-
-                // Again, the line must not fit. It would if the space taken up
-                // plus the line height would fit, therefore the constraint
-                // below.
-                let too_large = stack.size.y + self.leading + line.size.y;
-                stack.cts.max.y.set_min(too_large);
-                stack.finish_region(vm);
-            }
-
-            // If the line does not fit horizontally or we have a mandatory
-            // line break (i.e. due to "\n"), we push the line into the
-            // stack.
-            if mandatory || !stack.regions.current.x.fits(line.size.x) {
-                start = end;
-                last = None;
-
-                stack.push(line);
-
-                // If there is a trailing line break at the end of the
-                // paragraph, we want to force an empty line.
-                if mandatory && end == self.bidi.text.len() {
-                    let line = LineLayout::new(vm, &self, end .. end);
-                    if stack.regions.current.y.fits(line.size.y) {
-                        stack.push(line);
-                    }
-                }
-
-                stack.cts.min.y = Some(stack.size.y);
-            } else {
-                // Otherwise, the line fits both horizontally and vertically
-                // and we remember it.
-                stack.cts.min.x.set_max(line.size.x);
-                last = Some((line, end));
-            }
-        }
-
-        if let Some((line, _)) = last {
-            stack.push(line);
-            stack.cts.min.y = Some(stack.size.y);
-        }
-
-        stack.finish(vm)
-    }
-
-    /// Find the index of the item whose range contains the `text_offset`.
-    fn find(&self, text_offset: usize) -> Option<usize> {
-        self.ranges.binary_search_by(|r| r.locate(text_offset)).ok()
-    }
-}
-
-/// A lightweight representation of a line that spans a specific range in a
-/// paragraph's text. This type enables you to cheaply measure the size of a
-/// line in a range before comitting to building the line's frame.
-struct LineLayout<'a> {
-    /// Bidi information about the paragraph.
-    par: &'a ParLayouter<'a>,
-    /// The range the line spans in the paragraph.
-    line: Range,
-    /// A reshaped text item if the line sliced up a text item at the start.
-    first: Option<ParItem<'a>>,
-    /// Middle items which don't need to be reprocessed.
-    items: &'a [ParItem<'a>],
-    /// A reshaped text item if the line sliced up a text item at the end. If
-    /// there is only one text item, this takes precedence over `first`.
-    last: Option<ParItem<'a>>,
-    /// The ranges, indexed as `[first, ..items, last]`. The ranges for `first`
-    /// and `last` aren't trimmed to the line, but it doesn't matter because
-    /// we're just checking which range an index falls into.
-    ranges: &'a [Range],
-    /// The size of the line.
-    size: Size,
-    /// The baseline of the line.
-    baseline: Length,
-    /// The sum of fractional ratios in the line.
-    fr: Fractional,
-}
-
-impl<'a> LineLayout<'a> {
     /// Create a line which spans the given range.
-    fn new(vm: &mut Vm, par: &'a ParLayouter<'a>, mut line: Range) -> Self {
+    fn line(&'a self, fonts: &mut FontStore, mut range: Range) -> LineLayout<'a> {
         // Find the items which bound the text range.
-        let last_idx = par.find(line.end.saturating_sub(1)).unwrap();
+        let last_idx = self.find(range.end.saturating_sub(1)).unwrap();
-        let first_idx = if line.is_empty() {
+        let first_idx = if range.is_empty() {
             last_idx
         } else {
-            par.find(line.start).unwrap()
+            self.find(range.start).unwrap()
         };
 
         // Slice out the relevant items and ranges.
-        let mut items = &par.items[first_idx ..= last_idx];
+        let mut items = &self.items[first_idx ..= last_idx];
-        let ranges = &par.ranges[first_idx ..= last_idx];
+        let ranges = &self.ranges[first_idx ..= last_idx];
 
         // Reshape the last item if it's split in half.
         let mut last = None;
         if let Some((ParItem::Text(shaped), rest)) = items.split_last() {
             // Compute the range we want to shape, trimming whitespace at the
             // end of the line.
-            let base = par.ranges[last_idx].start;
+            let base = self.ranges[last_idx].start;
-            let start = line.start.max(base);
+            let start = range.start.max(base);
-            let end = start + par.bidi.text[start .. line.end].trim_end().len();
+            let end = start + self.bidi.text[start .. range.end].trim_end().len();
-            let range = start - base .. end - base;
+            let shifted = start - base .. end - base;
 
             // Reshape if necessary.
-            if range.len() < shaped.text.len() {
+            if shifted.len() < shaped.text.len() {
                 // If start == end and the rest is empty, then we have an empty
                 // line. To make that line have the appropriate height, we shape the
                 // empty string.
-                if !range.is_empty() || rest.is_empty() {
+                if !shifted.is_empty() || rest.is_empty() {
                     // Reshape that part.
-                    let reshaped = shaped.reshape(vm.fonts, range);
+                    let reshaped = shaped.reshape(fonts, shifted);
                     last = Some(ParItem::Text(reshaped));
                 }
 
                 items = rest;
-                line.end = end;
+                range.end = end;
             }
         }
 
@@ -447,15 +382,15 @@ impl<'a> LineLayout<'a> {
         let mut first = None;
         if let Some((ParItem::Text(shaped), rest)) = items.split_first() {
             // Compute the range we want to shape.
-            let Range { start: base, end: first_end } = par.ranges[first_idx];
+            let Range { start: base, end: first_end } = self.ranges[first_idx];
-            let start = line.start;
+            let start = range.start;
-            let end = line.end.min(first_end);
+            let end = range.end.min(first_end);
-            let range = start - base .. end - base;
+            let shifted = start - base .. end - base;
 
             // Reshape if necessary.
-            if range.len() < shaped.text.len() {
+            if shifted.len() < shaped.text.len() {
-                if !range.is_empty() {
+                if !shifted.is_empty() {
-                    let reshaped = shaped.reshape(vm.fonts, range);
+                    let reshaped = shaped.reshape(fonts, shifted);
                     first = Some(ParItem::Text(reshaped));
                 }
 
@@ -486,9 +421,9 @@ impl<'a> LineLayout<'a> {
             }
         }
 
-        Self {
+        LineLayout {
-            par,
+            bidi: &self.bidi,
-            line,
+            range,
             first,
             items,
             last,
@@ -499,8 +434,43 @@ impl<'a> LineLayout<'a> {
         }
     }
 
+
+    /// Find the index of the item whose range contains the `text_offset`.
+    fn find(&self, text_offset: usize) -> Option<usize> {
+        self.ranges.binary_search_by(|r| r.locate(text_offset)).ok()
+    }
+}
+
+/// A lightweight representation of a line that spans a specific range in a
+/// paragraph's text. This type enables you to cheaply measure the size of a
+/// line in a range before comitting to building the line's frame.
+struct LineLayout<'a> {
+    /// Bidi information about the paragraph.
+    bidi: &'a BidiInfo<'a>,
+    /// The range the line spans in the paragraph.
+    range: Range,
+    /// A reshaped text item if the line sliced up a text item at the start.
+    first: Option<ParItem<'a>>,
+    /// Middle items which don't need to be reprocessed.
+    items: &'a [ParItem<'a>],
+    /// A reshaped text item if the line sliced up a text item at the end. If
+    /// there is only one text item, this takes precedence over `first`.
+    last: Option<ParItem<'a>>,
+    /// The ranges, indexed as `[first, ..items, last]`. The ranges for `first`
+    /// and `last` aren't trimmed to the line, but it doesn't matter because
+    /// we're just checking which range an index falls into.
+    ranges: &'a [Range],
+    /// The size of the line.
+    size: Size,
+    /// The baseline of the line.
+    baseline: Length,
+    /// The sum of fractional ratios in the line.
+    fr: Fractional,
+}
+
+impl<'a> LineLayout<'a> {
     /// Build the line's frame.
-    fn build(&self, ctx: &Vm, width: Length) -> Frame {
+    fn build(&self, fonts: &FontStore, width: Length, align: Align) -> Frame {
         let size = Size::new(self.size.x.max(width), self.size.y);
         let remaining = size.x - self.size.x;
 
@@ -510,7 +480,7 @@ impl<'a> LineLayout<'a> {
 
         for item in self.reordered() {
             let mut position = |frame: Frame| {
-                let x = offset + self.par.align.resolve(remaining);
+                let x = offset + align.resolve(remaining);
                 let y = self.baseline - frame.baseline();
                 offset += frame.size.x;
                 output.merge_frame(Point::new(x, y), frame);
@@ -519,7 +489,7 @@ impl<'a> LineLayout<'a> {
             match item {
                 ParItem::Absolute(v) => offset += *v,
                 ParItem::Fractional(v) => offset += v.resolve(self.fr, remaining),
-                ParItem::Text(shaped) => position(shaped.build(ctx.fonts)),
+                ParItem::Text(shaped) => position(shaped.build(fonts)),
                 ParItem::Frame(frame) => position(frame.clone()),
             }
         }
@@ -530,18 +500,17 @@ impl<'a> LineLayout<'a> {
     /// Iterate through the line's items in visual order.
     fn reordered(&self) -> impl Iterator<Item = &ParItem<'a>> {
         // The bidi crate doesn't like empty lines.
-        let (levels, runs) = if !self.line.is_empty() {
+        let (levels, runs) = if !self.range.is_empty() {
             // Find the paragraph that contains the line.
             let para = self
-                .par
                 .bidi
                 .paragraphs
                 .iter()
-                .find(|para| para.range.contains(&self.line.start))
+                .find(|para| para.range.contains(&self.range.start))
                 .unwrap();
 
             // Compute the reordered ranges in visual order (left to right).
-            self.par.bidi.visual_runs(para, self.line.clone())
+            self.bidi.visual_runs(para, self.range.clone())
         } else {
             (vec![], vec![])
         };
@@ -574,87 +543,6 @@ impl<'a> LineLayout<'a> {
     }
 }
 
-/// Stacks lines on top of each other.
-struct LineStack<'a> {
-    leading: Length,
-    full: Size,
-    regions: Regions,
-    size: Size,
-    lines: Vec<LineLayout<'a>>,
-    finished: Vec<Constrained<Arc<Frame>>>,
-    cts: Constraints,
-    overflowing: bool,
-    fractional: bool,
-}
-
-impl<'a> LineStack<'a> {
-    /// Create an empty line stack.
-    fn new(leading: Length, regions: Regions) -> Self {
-        Self {
-            leading,
-            full: regions.current,
-            cts: Constraints::new(regions.expand),
-            regions,
-            size: Size::zero(),
-            lines: vec![],
-            finished: vec![],
-            overflowing: false,
-            fractional: false,
-        }
-    }
-
-    /// Push a new line into the stack.
-    fn push(&mut self, line: LineLayout<'a>) {
-        self.regions.current.y -= line.size.y + self.leading;
-
-        self.size.x.set_max(line.size.x);
-        self.size.y += line.size.y;
-        if !self.lines.is_empty() {
-            self.size.y += self.leading;
-        }
-
-        self.fractional |= !line.fr.is_zero();
-        self.lines.push(line);
-    }
-
-    /// Finish the frame for one region.
-    fn finish_region(&mut self, ctx: &Vm) {
-        if self.regions.expand.x || self.fractional {
-            self.size.x = self.regions.current.x;
-            self.cts.exact.x = Some(self.regions.current.x);
-        }
-
-        if self.overflowing {
-            self.cts.min.y = None;
-            self.cts.max.y = None;
-            self.cts.exact = self.full.map(Some);
-        }
-
-        let mut output = Frame::new(self.size);
-        let mut offset = Length::zero();
-
-        for line in self.lines.drain(..) {
-            let frame = line.build(ctx, self.size.x);
-            let pos = Point::with_y(offset);
-            offset += frame.size.y + self.leading;
-            output.merge_frame(pos, frame);
-        }
-
-        self.cts.base = self.regions.base.map(Some);
-        self.finished.push(output.constrain(self.cts));
-        self.regions.next();
-        self.full = self.regions.current;
-        self.size = Size::zero();
-        self.cts = Constraints::new(self.regions.expand);
-    }
-
-    /// Finish the last region and return the built frames.
-    fn finish(mut self, ctx: &Vm) -> Vec<Constrained<Arc<Frame>>> {
-        self.finish_region(ctx);
-        self.finished
-    }
-}
-
 /// Additional methods for BiDi levels.
 trait LevelExt: Sized {
     fn from_dir(dir: Dir) -> Option<Self>;

src/library/text.rs

@@ -802,11 +802,10 @@ impl<'a> ShapedText<'a> {
 
             // Apply line decorations.
             for deco in self.styles.get_cloned(TextNode::LINES) {
-                self.add_line_decos(&mut frame, &deco, fonts, &text, pos, width);
+                self.decorate(&mut frame, &deco, fonts, &text, pos, width);
             }
 
             frame.insert(text_layer, pos, Element::Text(text));
-
             offset += width;
         }
 
@@ -819,7 +818,7 @@ impl<'a> ShapedText<'a> {
     }
 
     /// Add line decorations to a run of shaped text of a single font.
-    fn add_line_decos(
+    fn decorate(
         &self,
         frame: &mut Frame,
         deco: &Decoration,
@@ -932,7 +931,6 @@ impl<'a> ShapedText<'a> {
         }
     }
 
-
     /// Reshape a range of the shaped text, reusing information from this
     /// shaping process if possible.
     pub fn reshape(

tests/typ/text/features.typ

@@ -18,7 +18,7 @@ a vs #text(alternates: true)[a] \
 
 ---
 // Test ligatures.
-fi vs. #text(ligatures: false)[No fi] \
+fi vs. #text(ligatures: false)[No fi]
 
 ---
 // Test number type.

tests/typ/text/linebreaks.typ

@@ -24,5 +24,5 @@ Hard break directly after \ normal break.
 Two consecutive \ \ breaks and three \ \ \ more.
 
 ---
-// Test trailing newline.
+// Test forcing an empty trailing line.
-Trailing break \
+Trailing break \ \