Tidy up layouting code

6 files changed, 124 insertions, 139 deletions

diff --git a/src/layout/fixed.rs b/src/layout/fixed.rs
index a42eab5a..cdd1033b 100644
--- a/src/layout/fixed.rs
+++ b/src/layout/fixed.rs
@@ -13,14 +13,14 @@ pub struct FixedNode {
 
 impl Layout for FixedNode {
     fn layout(&self, ctx: &mut LayoutContext, areas: &Areas) -> Vec<Frame> {
-        let Areas { current, full, .. } = areas;
+        let Areas { current, base, .. } = areas;
         let size = Size::new(
-            self.width.map_or(current.width, |w| w.resolve(full.width)),
-            self.height.map_or(current.height, |h| h.resolve(full.height)),
+            self.width.map_or(current.width, |w| w.resolve(base.width)),
+            self.height.map_or(current.height, |h| h.resolve(base.height)),
         );
 
         let fixed = Spec::new(self.width.is_some(), self.height.is_some());
-        let areas = Areas::once(size, size, fixed);
+        let areas = Areas::once(size, fixed);
         self.child.layout(ctx, &areas)
     }
 }
diff --git a/src/layout/mod.rs b/src/layout/mod.rs
index cb138753..83334a9f 100644
--- a/src/layout/mod.rs
+++ b/src/layout/mod.rs
@@ -144,67 +144,63 @@ pub struct LayoutContext<'a> {
 pub struct Areas {
     /// The remaining size of the current area.
     pub current: Size,
-    /// The full size the current area once had (used for relative sizing).
-    pub full: Size,
-    /// A stack of followup areas (the next area is the last element).
+    /// The base size for relative sizing.
+    pub base: Size,
+    /// A stack of followup areas.
+    ///
+    /// Note that this is a stack and not a queue! The size of the next area is
+    /// `backlog.last()`.
     pub backlog: Vec<Size>,
-    /// The final area that is repeated when the backlog is empty.
+    /// The final area that is repeated once the backlog is drained.
     pub last: Option<Size>,
-    /// Whether the frames resulting from layouting into this areas should
-    /// expand to the fixed size defined by `current`.
+    /// Whether layouting into these areas should produce frames of the exact
+    /// size of `current` instead of shrinking to fit the content.
     ///
-    /// If this is false, the frame will shrink to fit its content.
+    /// This property is only handled by nodes that have the ability to control
+    /// their own size.
     pub fixed: Spec<bool>,
 }
 
 impl Areas {
-    /// Create a new length-1 sequence of areas with just one `area`.
-    pub fn once(size: Size, full: Size, fixed: Spec<bool>) -> Self {
+    /// Create a new area sequence of length one.
+    pub fn once(size: Size, fixed: Spec<bool>) -> Self {
         Self {
             current: size,
-            full,
+            base: size,
             backlog: vec![],
             last: None,
             fixed,
         }
     }
 
-    /// Create a new sequence of areas that repeats `area` indefinitely.
+    /// Create a new sequence of same-size areas that repeats indefinitely.
     pub fn repeat(size: Size, fixed: Spec<bool>) -> Self {
         Self {
             current: size,
-            full: size,
+            base: size,
             backlog: vec![],
             last: Some(size),
             fixed,
         }
     }
 
-    /// Map all areas.
+    /// Map the size of all areas.
     pub fn map<F>(&self, mut f: F) -> Self
     where
         F: FnMut(Size) -> Size,
     {
         Self {
             current: f(self.current),
-            full: f(self.full),
+            base: f(self.base),
             backlog: self.backlog.iter().copied().map(|s| f(s)).collect(),
             last: self.last.map(f),
             fixed: self.fixed,
         }
     }
 
-    /// Advance to the next area if there is any.
-    pub fn next(&mut self) {
-        if let Some(size) = self.backlog.pop().or(self.last) {
-            self.current = size;
-            self.full = size;
-        }
-    }
-
     /// Whether `current` is a fully sized (untouched) copy of the last area.
     ///
-    /// If this is false calling `next()` will have no effect.
+    /// If this is true, calling `next()` will have no effect.
     pub fn in_full_last(&self) -> bool {
         self.backlog.is_empty()
             && self.last.map_or(true, |size| {
@@ -212,9 +208,18 @@ impl Areas {
             })
     }
 
+    /// Advance to the next area if there is any.
+    pub fn next(&mut self) {
+        if let Some(size) = self.backlog.pop().or(self.last) {
+            self.current = size;
+            self.base = size;
+        }
+    }
+
     /// Shrink `current` to ensure that the aspect ratio can be satisfied.
-    pub fn apply_aspect_ratio(&mut self, ratio: f64) {
-        let Size { width, height } = self.current;
-        self.current = Size::new(width.min(ratio * height), height.min(width / ratio));
+    pub fn apply_aspect_ratio(&mut self, aspect: f64) {
+        let width = self.current.width.min(aspect * self.current.height);
+        let height = width / aspect;
+        self.current = Size::new(width, height);
     }
 }
diff --git a/src/layout/pad.rs b/src/layout/pad.rs
index d24ca654..7ceb3d1b 100644
--- a/src/layout/pad.rs
+++ b/src/layout/pad.rs
@@ -11,37 +11,23 @@ pub struct PadNode {
 
 impl Layout for PadNode {
     fn layout(&self, ctx: &mut LayoutContext, areas: &Areas) -> Vec<Frame> {
-        let areas = shrink(areas, self.padding);
+        let areas = areas.map(|size| size - self.padding.resolve(size).size());
+
         let mut frames = self.child.layout(ctx, &areas);
         for frame in &mut frames {
-            pad(frame, self.padding);
-        }
-        frames
-    }
-}
+            let padded = solve(self.padding, frame.size);
+            let padding = self.padding.resolve(padded);
+            let origin = Point::new(padding.left, padding.top);
 
-impl From<PadNode> for AnyNode {
-    fn from(pad: PadNode) -> Self {
-        Self::new(pad)
-    }
-}
+            frame.size = padded;
+            frame.baseline += origin.y;
 
-/// Shrink all areas by the padding.
-fn shrink(areas: &Areas, padding: Sides<Linear>) -> Areas {
-    areas.map(|size| size - padding.resolve(size).size())
-}
-
-/// Pad the frame and move all elements inwards.
-fn pad(frame: &mut Frame, padding: Sides<Linear>) {
-    let padded = solve(padding, frame.size);
-    let padding = padding.resolve(padded);
-    let origin = Point::new(padding.left, padding.top);
-
-    frame.size = padded;
-    frame.baseline += origin.y;
+            for (point, _) in &mut frame.elements {
+                *point += origin;
+            }
+        }
 
-    for (point, _) in &mut frame.elements {
-        *point += origin;
+        frames
     }
 }
 
@@ -57,3 +43,9 @@ fn solve(padding: Sides<Linear>, size: Size) -> Size {
         solve_axis(size.height, padding.top + padding.bottom),
     )
 }
+
+impl From<PadNode> for AnyNode {
+    fn from(pad: PadNode) -> Self {
+        Self::new(pad)
+    }
+}
diff --git a/src/layout/par.rs b/src/layout/par.rs
index 7d7f5951..eb220cf9 100644
--- a/src/layout/par.rs
+++ b/src/layout/par.rs
@@ -1,5 +1,4 @@
 use std::fmt::{self, Debug, Formatter};
-use std::mem;
 
 use unicode_bidi::{BidiInfo, Level};
 use xi_unicode::LineBreakIterator;
@@ -142,10 +141,9 @@ impl<'a> ParLayout<'a> {
                     }
                 }
                 ParChild::Any(ref node, align) => {
-                    let frames = node.layout(ctx, areas);
-                    assert_eq!(frames.len(), 1);
-
-                    let frame = frames.into_iter().next().unwrap();
+                    let mut frames = node.layout(ctx, areas).into_iter();
+                    let frame = frames.next().unwrap();
+                    assert!(frames.next().is_none());
                     items.push(ParItem::Frame(frame, align));
                     ranges.push(range);
                 }
@@ -184,16 +182,16 @@ impl<'a> ParLayout<'a> {
             }
 
             // If the line does not fit vertically, we start a new area.
-            if !stack.areas.current.height.fits(line.size.height)
+            while !stack.areas.current.height.fits(line.size.height)
                 && !stack.areas.in_full_last()
             {
                 stack.finish_area(ctx);
             }
 
+            // 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.areas.current.width.fits(line.size.width) {
-                // 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.
                 stack.push(line);
                 start = end;
                 last = None;
@@ -289,14 +287,13 @@ impl<'a> LineStack<'a> {
     }
 
     fn push(&mut self, line: LineLayout<'a>) {
-        self.areas.current.height -= line.size.height + self.line_spacing;
-
-        self.size.width = self.size.width.max(line.size.width);
         self.size.height += line.size.height;
+        self.size.width.set_max(line.size.width);
         if !self.lines.is_empty() {
             self.size.height += self.line_spacing;
         }
 
+        self.areas.current.height -= line.size.height + self.line_spacing;
         self.lines.push(line);
     }
 
@@ -306,22 +303,20 @@ impl<'a> LineStack<'a> {
         }
 
         let mut output = Frame::new(self.size, self.size.height);
-        let mut first = true;
         let mut offset = Length::ZERO;
+        let mut first = true;
 
-        for line in mem::take(&mut self.lines) {
+        for line in std::mem::take(&mut self.lines) {
             let frame = line.build(ctx, self.size.width);
-            let Frame { size, baseline, .. } = frame;
 
             let pos = Point::new(Length::ZERO, offset);
-            output.push_frame(pos, frame);
-
             if first {
-                output.baseline = offset + baseline;
+                output.baseline = pos.y + frame.baseline;
                 first = false;
             }
 
-            offset += size.height + self.line_spacing;
+            offset += frame.size.height + self.line_spacing;
+            output.push_frame(pos, frame);
         }
 
         self.finished.push(output);
@@ -430,8 +425,8 @@ impl<'a> LineLayout<'a> {
             let size = item.size();
             let baseline = item.baseline();
             width += size.width;
-            top = top.max(baseline);
-            bottom = bottom.max(size.height - baseline);
+            top.set_max(baseline);
+            bottom.set_max(size.height - baseline);
         }
 
         Self {
@@ -448,13 +443,12 @@ impl<'a> LineLayout<'a> {
 
     /// Build the line's frame.
     fn build(&self, ctx: &mut LayoutContext, width: Length) -> Frame {
-        let full_width = self.size.width.max(width);
-        let full_size = Size::new(full_width, self.size.height);
-        let free_width = full_width - self.size.width;
+        let size = Size::new(self.size.width.max(width), self.size.height);
+        let free = size.width - self.size.width;
 
-        let mut output = Frame::new(full_size, self.baseline);
-        let mut ruler = Align::Start;
+        let mut output = Frame::new(size, self.baseline);
         let mut offset = Length::ZERO;
+        let mut ruler = Align::Start;
 
         self.reordered(|item| {
             let frame = match *item {
@@ -472,14 +466,14 @@ impl<'a> LineLayout<'a> {
                 }
             };
 
-            let Frame { size, baseline, .. } = frame;
+            // FIXME: Ruler alignment for RTL.
             let pos = Point::new(
-                ruler.resolve(self.par.dir, offset .. free_width + offset),
-                self.baseline - baseline,
+                ruler.resolve(self.par.dir, offset .. free + offset),
+                self.baseline - frame.baseline,
             );
 
+            offset += frame.size.width;
             output.push_frame(pos, frame);
-            offset += size.width;
         });
 
         output
diff --git a/src/layout/shaping.rs b/src/layout/shaping.rs
index 8a407260..dd6c55bd 100644
--- a/src/layout/shaping.rs
+++ b/src/layout/shaping.rs
@@ -336,9 +336,8 @@ fn measure(
     let mut top = Length::ZERO;
     let mut bottom = Length::ZERO;
     let mut expand_vertical = |face: &Face| {
-        top = top.max(face.vertical_metric(props.top_edge).to_length(props.size));
-        bottom =
-            bottom.max(-face.vertical_metric(props.bottom_edge).to_length(props.size));
+        top.set_max(face.vertical_metric(props.top_edge).to_length(props.size));
+        bottom.set_max(-face.vertical_metric(props.bottom_edge).to_length(props.size));
     };
 
     if glyphs.is_empty() {
diff --git a/src/layout/stack.rs b/src/layout/stack.rs
index 463f38a7..38e9a517 100644
--- a/src/layout/stack.rs
+++ b/src/layout/stack.rs
@@ -61,6 +61,7 @@ struct StackLayouter {
     areas: Areas,
     finished: Vec<Frame>,
     frames: Vec<(Length, Frame, Gen<Align>)>,
+    full: Size,
     size: Gen<Length>,
     ruler: Align,
 }
@@ -75,18 +76,19 @@ impl StackLayouter {
             dirs,
             aspect,
             main: dirs.main.axis(),
-            areas,
             finished: vec![],
             frames: vec![],
+            full: areas.current,
             size: Gen::ZERO,
             ruler: Align::Start,
+            areas,
         }
     }
 
     fn push_spacing(&mut self, amount: Length) {
-        let main_rest = self.areas.current.get_mut(self.main);
-        let capped = amount.min(*main_rest);
-        *main_rest -= capped;
+        let remaining = self.areas.current.get_mut(self.main);
+        let capped = amount.min(*remaining);
+        *remaining -= capped;
         self.size.main += capped;
     }
 
@@ -95,71 +97,64 @@ impl StackLayouter {
             self.finish_area();
         }
 
-        while !self.areas.current.fits(frame.size) {
-            if self.areas.in_full_last() {
-                // TODO: Diagnose once the necessary spans exist.
-                break;
-            } else {
-                self.finish_area();
-            }
+        while !self.areas.current.fits(frame.size) && !self.areas.in_full_last() {
+            self.finish_area();
         }
 
+        let offset = self.size.main;
         let size = frame.size.switch(self.main);
-        self.frames.push((self.size.main, frame, aligns));
-        self.ruler = aligns.main;
         self.size.main += size.main;
-        self.size.cross = self.size.cross.max(size.cross);
+        self.size.cross.set_max(size.cross);
+        self.ruler = aligns.main;
         *self.areas.current.get_mut(self.main) -= size.main;
+        self.frames.push((offset, frame, aligns));
     }
 
     fn finish_area(&mut self) {
-        let full_size = {
-            let Areas { current, full, fixed, .. } = self.areas;
+        let fixed = self.areas.fixed;
 
-            let used = self.size.switch(self.main).to_size();
-            let mut size = Size::new(
-                if fixed.horizontal { full.width } else { used.width },
-                if fixed.vertical { full.height } else { used.height },
-            );
+        let used = self.size.switch(self.main).to_size();
+        let mut size = Size::new(
+            if fixed.horizontal { self.full.width } else { used.width },
+            if fixed.vertical { self.full.height } else { used.height },
+        );
 
-            if let Some(aspect) = self.aspect {
-                let width = size
-                    .width
-                    .max(aspect * size.height)
-                    .min(current.width)
-                    .min((current.height + used.height) / aspect);
-
-                size = Size::new(width, width / aspect);
-            }
+        if let Some(aspect) = self.aspect {
+            let width = size
+                .width
+                .max(aspect * size.height)
+                .min(self.full.width)
+                .min(aspect * self.full.height);
 
-            size
-        };
+            size = Size::new(width, width / aspect);
+        }
 
-        let mut output = Frame::new(full_size, full_size.height);
+        let mut output = Frame::new(size, size.height);
         let mut first = true;
 
-        let full_size = full_size.switch(self.main);
-        for (before, frame, aligns) in std::mem::take(&mut self.frames) {
-            let child_size = frame.size.switch(self.main);
+        let used = self.size;
+        let size = size.switch(self.main);
+
+        for (offset, frame, aligns) in std::mem::take(&mut self.frames) {
+            let child = frame.size.switch(self.main);
+
+            // Align along the cross axis.
+            let cross = aligns
+                .cross
+                .resolve(self.dirs.cross, Length::ZERO .. size.cross - child.cross);
 
             // Align along the main axis.
             let main = aligns.main.resolve(
                 self.dirs.main,
                 if self.dirs.main.is_positive() {
-                    before .. before + full_size.main - self.size.main
+                    offset .. size.main - used.main + offset
                 } else {
-                    self.size.main - (before + child_size.main)
-                        .. full_size.main - (before + child_size.main)
+                    let offset_with_self = offset + child.main;
+                    used.main - offset_with_self .. size.main - offset_with_self
                 },
             );
 
-            // Align along the cross axis.
-            let cross = aligns.cross.resolve(
-                self.dirs.cross,
-                Length::ZERO .. full_size.cross - child_size.cross,
-            );
-
-            let pos = Gen::new(main, cross).switch(self.main).to_point();
+            let pos = Gen::new(cross, main).switch(self.main).to_point();
             if first {
                 output.baseline = pos.y + frame.baseline;
                 first = false;
@@ -168,14 +163,14 @@ impl StackLayouter {
             output.push_frame(pos, frame);
         }
 
-        self.finished.push(output);
-        self.areas.next();
-        self.ruler = Align::Start;
         self.size = Gen::ZERO;
-
+        self.ruler = Align::Start;
+        self.areas.next();
         if let Some(aspect) = self.aspect {
             self.areas.apply_aspect_ratio(aspect);
         }
+
+        self.finished.push(output);
     }
 
     fn finish(mut self) -> Vec<Frame> {