Fix grid sizing

6 files changed, 71 insertions, 60 deletions

diff --git a/library/src/layout/flow.rs b/library/src/layout/flow.rs
index 00eeb537..ee845f06 100644
--- a/library/src/layout/flow.rs
+++ b/library/src/layout/flow.rs
@@ -70,9 +70,9 @@ struct FlowLayouter<'a> {
     regions: Regions<'a>,
     /// Whether the flow should expand to fill the region.
     expand: Axes<bool>,
-    /// The full size of `regions.size` that was available before we started
+    /// The intial size of `regions.size` that was available before we started
     /// subtracting.
-    full: Size,
+    initial: Size,
     /// Whether the last block was a paragraph.
     last_was_par: bool,
     /// Spacing and layouted blocks.
@@ -98,7 +98,6 @@ impl<'a> FlowLayouter<'a> {
     /// Create a new flow layouter.
     fn new(mut regions: Regions<'a>) -> Self {
         let expand = regions.expand;
-        let full = regions.size;
 
         // Disable vertical expansion for children.
         regions.expand.y = false;
@@ -106,7 +105,7 @@ impl<'a> FlowLayouter<'a> {
         Self {
             regions,
             expand,
-            full,
+            initial: regions.size,
             last_was_par: false,
             items: vec![],
             finished: vec![],
@@ -117,7 +116,7 @@ impl<'a> FlowLayouter<'a> {
     fn layout_spacing(&mut self, node: VNode, styles: StyleChain) {
         self.layout_item(match node.amount {
             Spacing::Rel(v) => FlowItem::Absolute(
-                v.resolve(styles).relative_to(self.full.y),
+                v.resolve(styles).relative_to(self.initial.y),
                 node.weakness > 0,
             ),
             Spacing::Fr(v) => FlowItem::Fractional(v),
@@ -256,10 +255,10 @@ impl<'a> FlowLayouter<'a> {
         let mut fr = Fr::zero();
         let mut used = Size::zero();
         for item in &self.items {
-            match *item {
-                FlowItem::Absolute(v, _) => used.y += v,
-                FlowItem::Fractional(v) => fr += v,
-                FlowItem::Frame(ref frame, ..) => {
+            match item {
+                FlowItem::Absolute(v, _) => used.y += *v,
+                FlowItem::Fractional(v) => fr += *v,
+                FlowItem::Frame(frame, ..) => {
                     let size = frame.size();
                     used.y += size.y;
                     used.x.set_max(size.x);
@@ -269,14 +268,10 @@ impl<'a> FlowLayouter<'a> {
         }
 
         // Determine the size of the flow in this region depending on whether
-        // the region expands.
-        let mut size = self.expand.select(self.full, used).min(self.full);
-
-        // Account for fractional spacing in the size calculation.
-        let remaining = self.full.y - used.y;
-        if fr.get() > 0.0 && self.full.y.is_finite() {
-            used.y = self.full.y;
-            size.y = self.full.y;
+        // the region expands. Also account for fractional spacing.
+        let mut size = self.expand.select(self.initial, used).min(self.initial);
+        if fr.get() > 0.0 && self.initial.y.is_finite() {
+            size.y = self.initial.y;
         }
 
         let mut output = Frame::new(size);
@@ -290,6 +285,7 @@ impl<'a> FlowLayouter<'a> {
                     offset += v;
                 }
                 FlowItem::Fractional(v) => {
+                    let remaining = self.initial.y - used.y;
                     offset += v.share(fr, remaining);
                 }
                 FlowItem::Frame(frame, aligns, _) => {
@@ -309,7 +305,7 @@ impl<'a> FlowLayouter<'a> {
         // Advance to the next region.
         self.finished.push(output);
         self.regions.next();
-        self.full = self.regions.size;
+        self.initial = self.regions.size;
     }
 
     /// Finish layouting and return the resulting fragment.
diff --git a/library/src/layout/grid.rs b/library/src/layout/grid.rs
index c5bbe9e5..626cb82d 100644
--- a/library/src/layout/grid.rs
+++ b/library/src/layout/grid.rs
@@ -184,15 +184,14 @@ pub struct GridLayouter<'a, 'v> {
     styles: StyleChain<'a>,
     /// Resolved column sizes.
     rcols: Vec<Abs>,
+    /// The sum of `rcols`.
+    width: Abs,
     /// Resolve row sizes, by region.
     rrows: Vec<Vec<RowPiece>>,
     /// Rows in the current region.
     lrows: Vec<Row>,
-    /// The used-up size of the current region. The horizontal size is
-    /// determined once after columns are resolved and not touched again.
-    used: Size,
-    /// The sum of fractions in the current region.
-    fr: Fr,
+    /// The initial size of the current region before we started subtracting.
+    initial: Size,
     /// Frames for finished regions.
     finished: Vec<Frame>,
 }
@@ -306,10 +305,10 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
             regions,
             styles,
             rcols,
+            width: Abs::zero(),
             rrows: vec![],
             lrows,
-            used: Size::zero(),
-            fr: Fr::zero(),
+            initial: regions.size,
             finished: vec![],
         }
     }
@@ -328,10 +327,7 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
             match self.rows[y] {
                 Sizing::Auto => self.layout_auto_row(y)?,
                 Sizing::Rel(v) => self.layout_relative_row(v, y)?,
-                Sizing::Fr(v) => {
-                    self.lrows.push(Row::Fr(v, y));
-                    self.fr += v;
-                }
+                Sizing::Fr(v) => self.lrows.push(Row::Fr(v, y)),
             }
         }
 
@@ -384,7 +380,7 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
         }
 
         // Sum up the resolved column sizes once here.
-        self.used.x = self.rcols.iter().sum();
+        self.width = self.rcols.iter().sum();
 
         Ok(())
     }
@@ -523,13 +519,16 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
             resolved.remove(0);
         }
 
-        // Expand all but the last region if the space is not
-        // eaten up by any fr rows.
-        if self.fr.is_zero() {
-            let len = resolved.len();
-            for (region, target) in self.regions.iter().zip(&mut resolved[..len - 1]) {
-                target.set_max(region.y);
-            }
+        // Expand all but the last region.
+        // Skip the first region if the space is eaten up by an fr row.
+        let len = resolved.len();
+        for (region, target) in self
+            .regions
+            .iter()
+            .zip(&mut resolved[..len - 1])
+            .skip(self.lrows.iter().any(|row| matches!(row, Row::Fr(..))) as usize)
+        {
+            target.set_max(region.y);
         }
 
         // Layout into multiple regions.
@@ -569,7 +568,7 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
 
     /// Layout a row with fixed height and return its frame.
     fn layout_single_row(&mut self, height: Abs, y: usize) -> SourceResult<Frame> {
-        let mut output = Frame::new(Size::new(self.used.x, height));
+        let mut output = Frame::new(Size::new(self.width, height));
         let mut pos = Point::zero();
 
         for (x, &rcol) in self.rcols.iter().enumerate() {
@@ -594,11 +593,11 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
         // Prepare frames.
         let mut outputs: Vec<_> = heights
             .iter()
-            .map(|&h| Frame::new(Size::new(self.used.x, h)))
+            .map(|&h| Frame::new(Size::new(self.width, h)))
             .collect();
 
         // Prepare regions.
-        let size = Size::new(self.used.x, heights[0]);
+        let size = Size::new(self.width, heights[0]);
         let mut pod = Regions::one(size, Axes::splat(true));
         pod.full = self.regions.full;
         pod.backlog = &heights[1..];
@@ -625,17 +624,26 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
     /// Push a row frame into the current region.
     fn push_row(&mut self, frame: Frame, y: usize) {
         self.regions.size.y -= frame.height();
-        self.used.y += frame.height();
         self.lrows.push(Row::Frame(frame, y));
     }
 
     /// Finish rows for one region.
     fn finish_region(&mut self) -> SourceResult<()> {
+        // Determine the height of existing rows in the region.
+        let mut used = Abs::zero();
+        let mut fr = Fr::zero();
+        for row in &self.lrows {
+            match row {
+                Row::Frame(frame, _) => used += frame.height(),
+                Row::Fr(v, _) => fr += *v,
+            }
+        }
+
         // Determine the size of the grid in this region, expanding fully if
         // there are fr rows.
-        let mut size = self.used;
-        if self.fr.get() > 0.0 && self.regions.full.is_finite() {
-            size.y = self.regions.full;
+        let mut size = Size::new(self.width, used).min(self.initial);
+        if fr.get() > 0.0 && self.initial.y.is_finite() {
+            size.y = self.initial.y;
         }
 
         // The frame for the region.
@@ -648,8 +656,8 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
             let (frame, y) = match row {
                 Row::Frame(frame, y) => (frame, y),
                 Row::Fr(v, y) => {
-                    let remaining = self.regions.full - self.used.y;
-                    let height = v.share(self.fr, remaining);
+                    let remaining = self.regions.full - used;
+                    let height = v.share(fr, remaining);
                     (self.layout_single_row(height, y)?, y)
                 }
             };
@@ -661,10 +669,9 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
         }
 
         self.finished.push(output);
-        self.regions.next();
         self.rrows.push(rrows);
-        self.used.y = Abs::zero();
-        self.fr = Fr::zero();
+        self.regions.next();
+        self.initial = self.regions.size;
 
         Ok(())
     }
diff --git a/library/src/layout/stack.rs b/library/src/layout/stack.rs
index 1452a889..864b7b42 100644
--- a/library/src/layout/stack.rs
+++ b/library/src/layout/stack.rs
@@ -129,8 +129,8 @@ struct StackLayouter<'a> {
     styles: StyleChain<'a>,
     /// Whether the stack itself should expand to fill the region.
     expand: Axes<bool>,
-    /// The full size of the current region that was available at the start.
-    full: Size,
+    /// The initial size of the current region before we started subtracting.
+    initial: Size,
     /// The generic size used by the frames for the current region.
     used: Gen<Abs>,
     /// The sum of fractions in the current region.
@@ -154,13 +154,11 @@ enum StackItem {
 
 impl<'a> StackLayouter<'a> {
     /// Create a new stack layouter.
-    fn new(dir: Dir, regions: Regions<'a>, styles: StyleChain<'a>) -> Self {
+    fn new(dir: Dir, mut regions: Regions<'a>, styles: StyleChain<'a>) -> Self {
         let axis = dir.axis();
         let expand = regions.expand;
-        let full = regions.size;
 
         // Disable expansion along the block axis for children.
-        let mut regions = regions.clone();
         regions.expand.set(axis, false);
 
         Self {
@@ -169,7 +167,7 @@ impl<'a> StackLayouter<'a> {
             regions,
             styles,
             expand,
-            full,
+            initial: regions.size,
             used: Gen::zero(),
             fr: Fr::zero(),
             items: vec![],
@@ -251,11 +249,13 @@ impl<'a> StackLayouter<'a> {
     fn finish_region(&mut self) {
         // Determine the size of the stack in this region dependening on whether
         // the region expands.
-        let used = self.used.to_axes(self.axis);
-        let mut size = self.expand.select(self.full, used);
+        let mut size = self
+            .expand
+            .select(self.initial, self.used.to_axes(self.axis))
+            .min(self.initial);
 
         // Expand fully if there are fr spacings.
-        let full = self.full.get(self.axis);
+        let full = self.initial.get(self.axis);
         let remaining = full - self.used.main;
         if self.fr.get() > 0.0 && full.is_finite() {
             self.used.main = full;
@@ -303,7 +303,7 @@ impl<'a> StackLayouter<'a> {
 
         // Advance to the next region.
         self.regions.next();
-        self.full = self.regions.size;
+        self.initial = self.regions.size;
         self.used = Gen::zero();
         self.fr = Fr::zero();
         self.finished.push(output);
diff --git a/src/geom/fr.rs b/src/geom/fr.rs
index a737c953..c602634d 100644
--- a/src/geom/fr.rs
+++ b/src/geom/fr.rs
@@ -34,7 +34,7 @@ impl Fr {
     pub fn share(self, total: Self, remaining: Abs) -> Abs {
         let ratio = self / total;
         if ratio.is_finite() && remaining.is_finite() {
-            ratio * remaining
+            (ratio * remaining).max(Abs::zero())
         } else {
             Abs::zero()
         }
diff --git a/tests/ref/bugs/grid-3.png b/tests/ref/bugs/grid-3.png
new file mode 100644
index 00000000..c4569851
--- /dev/null
+++ b/tests/ref/bugs/grid-3.png
diff --git a/tests/typ/bugs/grid-3.typ b/tests/typ/bugs/grid-3.typ
new file mode 100644
index 00000000..19317c50
--- /dev/null
+++ b/tests/typ/bugs/grid-3.typ
@@ -0,0 +1,8 @@
+// Ensure that the list does not jump to the third page.
+
+---
+#set page(height: 70pt)
+#v(40pt)
+The following:
++ A
++ B