Layout bugfixes

13 files changed, 139 insertions, 111 deletions

diff --git a/src/geom/spec.rs b/src/geom/spec.rs
index 16d63959..5d89c894 100644
--- a/src/geom/spec.rs
+++ b/src/geom/spec.rs
@@ -85,6 +85,27 @@ impl<T> Spec<T> {
     }
 }
 
+impl<T> Spec<T>
+where
+    T: Ord,
+{
+    /// The component-wise minimum of this and another instance.
+    pub fn min(self, other: Self) -> Self {
+        Self {
+            x: self.x.min(other.x),
+            y: self.y.min(other.y),
+        }
+    }
+
+    /// The component-wise minimum of this and another instance.
+    pub fn max(self, other: Self) -> Self {
+        Self {
+            x: self.x.max(other.x),
+            y: self.y.max(other.y),
+        }
+    }
+}
+
 impl<T> Get<SpecAxis> for Spec<T> {
     type Component = T;
 
diff --git a/src/library/align.rs b/src/library/align.rs
index a2881cce..18920369 100644
--- a/src/library/align.rs
+++ b/src/library/align.rs
@@ -61,7 +61,7 @@ impl Layout for AlignNode {
             // Set constraints.
             cts.expand = regions.expand;
             cts.base = base.filter(cts.base.map_is_some());
-            cts.exact = current.filter(regions.expand);
+            cts.exact = current.filter(regions.expand | cts.exact.map_is_some());
         }
 
         frames
diff --git a/src/library/flow.rs b/src/library/flow.rs
index 587b5a80..9494d6c0 100644
--- a/src/library/flow.rs
+++ b/src/library/flow.rs
@@ -132,10 +132,6 @@ impl<'a> FlowLayouter<'a> {
     /// Layout all children.
     fn layout(mut self, ctx: &mut LayoutContext) -> Vec<Constrained<Rc<Frame>>> {
         for child in self.children {
-            if self.regions.is_full() {
-                self.finish_region();
-            }
-
             match *child {
                 FlowChild::Spacing(Spacing::Linear(v)) => {
                     self.layout_absolute(v);
@@ -145,6 +141,10 @@ impl<'a> FlowLayouter<'a> {
                     self.fr += v;
                 }
                 FlowChild::Node(ref node) => {
+                    if self.regions.is_full() {
+                        self.finish_region();
+                    }
+
                     self.layout_node(ctx, node);
                 }
             }
diff --git a/src/library/grid.rs b/src/library/grid.rs
index 9dd156da..7a9d88c3 100644
--- a/src/library/grid.rs
+++ b/src/library/grid.rs
@@ -552,7 +552,7 @@ impl<'a> GridLayouter<'a> {
             size.y = self.full;
             self.cts.exact.y = Some(self.full);
         } else {
-            self.cts.min.y = Some(size.y);
+            self.cts.min.y = Some(size.y.min(self.full));
         }
 
         // The frame for the region.
@@ -575,11 +575,12 @@ impl<'a> GridLayouter<'a> {
             pos.y += height;
         }
 
+        self.cts.base = self.regions.base.map(Some);
+        self.finished.push(output.constrain(self.cts));
         self.regions.next();
         self.full = self.regions.current.y;
         self.used.y = Length::zero();
         self.fr = Fractional::zero();
-        self.finished.push(output.constrain(self.cts));
         self.cts = Constraints::new(self.expand);
     }
 
diff --git a/src/library/image.rs b/src/library/image.rs
index a20f3856..7ef45d6d 100644
--- a/src/library/image.rs
+++ b/src/library/image.rs
@@ -40,34 +40,34 @@ impl Layout for ImageNode {
         ctx: &mut LayoutContext,
         regions: &Regions,
     ) -> Vec<Constrained<Rc<Frame>>> {
-        let &Regions { current, expand, .. } = regions;
-
         let img = ctx.images.get(self.id);
         let pxw = img.width() as f64;
         let pxh = img.height() as f64;
+        let px_ratio = pxw / pxh;
 
-        let pixel_ratio = pxw / pxh;
+        // Find out whether the image is wider or taller than the target size.
+        let current = regions.current;
         let current_ratio = current.x / current.y;
-        let wide = pixel_ratio > current_ratio;
+        let wide = px_ratio > current_ratio;
 
         // The space into which the image will be placed according to its fit.
-        let target = if expand.x && expand.y {
+        let target = if regions.expand.x && regions.expand.y {
             current
-        } else if expand.x || (wide && current.x.is_finite()) {
-            Size::new(current.x, current.y.min(current.x.safe_div(pixel_ratio)))
+        } else if regions.expand.x || (wide && current.x.is_finite()) {
+            Size::new(current.x, current.y.min(current.x.safe_div(px_ratio)))
         } else if current.y.is_finite() {
-            Size::new(current.x.min(current.y * pixel_ratio), current.y)
+            Size::new(current.x.min(current.y * px_ratio), current.y)
         } else {
             Size::new(Length::pt(pxw), Length::pt(pxh))
         };
 
         // The actual size of the fitted image.
-        let size = match self.fit {
+        let fitted = match self.fit {
             ImageFit::Contain | ImageFit::Cover => {
                 if wide == (self.fit == ImageFit::Contain) {
-                    Size::new(target.x, target.x / pixel_ratio)
+                    Size::new(target.x, target.x / px_ratio)
                 } else {
-                    Size::new(target.y * pixel_ratio, target.y)
+                    Size::new(target.y * px_ratio, target.y)
                 }
             }
             ImageFit::Stretch => target,
@@ -76,8 +76,8 @@ impl Layout for ImageNode {
         // First, place the image in a frame of exactly its size and then resize
         // the frame to the target size, center aligning the image in the
         // process.
-        let mut frame = Frame::new(size);
-        frame.push(Point::zero(), Element::Image(self.id, size));
+        let mut frame = Frame::new(fitted);
+        frame.push(Point::zero(), Element::Image(self.id, fitted));
         frame.resize(target, Align::CENTER_HORIZON);
 
         // Create a clipping group if the fit mode is "cover".
diff --git a/src/library/par.rs b/src/library/par.rs
index 5f900dff..c29e5f9e 100644
--- a/src/library/par.rs
+++ b/src/library/par.rs
@@ -231,8 +231,7 @@ impl<'a> ParLayouter<'a> {
                 }
                 ParChild::Node(ref node) => {
                     let size = Size::new(regions.current.x, regions.base.y);
-                    let expand = Spec::splat(false);
-                    let pod = Regions::one(size, regions.base, expand);
+                    let pod = Regions::one(size, regions.base, Spec::splat(false));
                     let frame = node.layout(ctx, &pod).remove(0);
                     items.push(ParItem::Frame(Rc::take(frame.item)));
                     ranges.push(range);
@@ -288,25 +287,31 @@ impl<'a> ParLayouter<'a> {
             // 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 !stack.regions.current.x.fits(line.size.x) {
+                    if !fits.x {
                         stack.cts.max.x.set_min(line.size.x);
                     }
 
                     // Same as above, but for height.
-                    if !stack.regions.current.y.fits(line.size.y) {
+                    if !fits.y {
                         let too_large = stack.size.y + self.leading + line.size.y;
                         stack.cts.max.y.set_min(too_large);
                     }
 
-                    stack.push(last_line);
-
-                    stack.cts.min.y = Some(stack.size.y);
-                    start = last_end;
-                    line = LineLayout::new(ctx, &self, start .. end);
+                    // 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(ctx, &self, start .. end);
+                    }
                 }
             }
 
@@ -322,7 +327,6 @@ impl<'a> ParLayouter<'a> {
                 // below.
                 let too_large = stack.size.y + self.leading + line.size.y;
                 stack.cts.max.y.set_min(too_large);
-
                 stack.finish_region(ctx);
             }
 
@@ -644,12 +648,12 @@ impl<'a> LineStack<'a> {
             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.cts = Constraints::new(self.regions.expand);
-        self.cts.base = self.regions.base.map(Some);
         self.size = Size::zero();
+        self.cts = Constraints::new(self.regions.expand);
     }
 
     /// Finish the last region and return the built frames.
diff --git a/src/library/placed.rs b/src/library/placed.rs
index 1ae51530..722e0035 100644
--- a/src/library/placed.rs
+++ b/src/library/placed.rs
@@ -43,7 +43,8 @@ impl Layout for PlacedNode {
         // The pod is the base area of the region because for absolute
         // placement we don't really care about the already used area (current).
         let pod = {
-            let expand = if out_of_flow { Spec::splat(true) } else { regions.expand };
+            let finite = regions.base.map(Length::is_finite);
+            let expand = finite & (regions.expand | out_of_flow);
             Regions::one(regions.base, regions.base, expand)
         };
 
diff --git a/src/library/shape.rs b/src/library/shape.rs
index 208ca2a3..2f2f9686 100644
--- a/src/library/shape.rs
+++ b/src/library/shape.rs
@@ -1,7 +1,6 @@
 use std::f64::consts::SQRT_2;
 
 use super::prelude::*;
-use crate::util::RcExt;
 
 /// `rect`: A rectangle with optional content.
 pub fn rect(_: &mut EvalContext, args: &mut Args) -> TypResult<Value> {
@@ -68,7 +67,13 @@ fn shape_impl(
     };
 
     // Shorthand for padding.
-    let padding = Sides::splat(args.named("padding")?.unwrap_or_default());
+    let mut padding = args.named::<Linear>("padding")?.unwrap_or_default();
+
+    // Padding with this ratio ensures that a rectangular child fits
+    // perfectly into a circle / an ellipse.
+    if kind.is_round() {
+        padding.rel += Relative::new(0.5 - SQRT_2 / 4.0);
+    }
 
     // The shape's contents.
     let body = args.find::<Template>();
@@ -78,7 +83,9 @@ fn shape_impl(
             kind,
             fill,
             stroke,
-            child: body.as_ref().map(|body| body.pack(style).padded(padding)),
+            child: body
+                .as_ref()
+                .map(|body| body.pack(style).padded(Sides::splat(padding))),
         }
         .pack()
         .sized(Spec::new(width, height))
@@ -98,84 +105,49 @@ pub struct ShapeNode {
     pub child: Option<PackedNode>,
 }
 
-/// The type of a shape.
-#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
-pub enum ShapeKind {
-    /// A rectangle with equal side lengths.
-    Square,
-    /// A quadrilateral with four right angles.
-    Rect,
-    /// An ellipse with coinciding foci.
-    Circle,
-    /// A curve around two focal points.
-    Ellipse,
-}
-
 impl Layout for ShapeNode {
     fn layout(
         &self,
         ctx: &mut LayoutContext,
         regions: &Regions,
     ) -> Vec<Constrained<Rc<Frame>>> {
-        // Layout, either with or without child.
-        let mut frame = if let Some(child) = &self.child {
-            let mut node: &dyn Layout = child;
-
-            let storage;
-            if matches!(self.kind, ShapeKind::Circle | ShapeKind::Ellipse) {
-                // Padding with this ratio ensures that a rectangular child fits
-                // perfectly into a circle / an ellipse.
-                let ratio = Relative::new(0.5 - SQRT_2 / 4.0);
-                storage = child.clone().padded(Sides::splat(ratio.into()));
-                node = &storage;
-            }
-
-            // Now, layout the child.
-            let mut frames = node.layout(ctx, regions);
-
-            if matches!(self.kind, ShapeKind::Square | ShapeKind::Circle) {
-                // Relayout with full expansion into square region to make sure
-                // the result is really a square or circle.
+        let mut frames;
+        if let Some(child) = &self.child {
+            let mut pod = Regions::one(regions.current, regions.base, regions.expand);
+            frames = child.layout(ctx, &pod);
+
+            // Relayout with full expansion into square region to make sure
+            // the result is really a square or circle.
+            if self.kind.is_quadratic() {
                 let size = frames[0].item.size;
-                let mut pod = regions.clone();
-                pod.current.x = size.x.max(size.y).min(pod.current.x);
-                pod.current.y = pod.current.x;
+                let desired = size.x.max(size.y);
+                let limit = regions.current.x.min(regions.current.y);
+                pod.current = Size::splat(desired.min(limit));
                 pod.expand = Spec::splat(true);
-                frames = node.layout(ctx, &pod);
+                frames = child.layout(ctx, &pod);
+                frames[0].cts = Constraints::tight(regions);
             }
-
-            // TODO: What if there are multiple or no frames?
-            // Extract the frame.
-            Rc::take(frames.into_iter().next().unwrap().item)
         } else {
-            // When there's no child, fill the area if expansion is on,
-            // otherwise fall back to a default size.
-            let default = Length::pt(30.0);
-            let mut size = Size::new(
-                if regions.expand.x {
-                    regions.current.x
-                } else {
-                    // For rectangle and ellipse, the default shape is a bit
-                    // wider than high.
-                    match self.kind {
-                        ShapeKind::Square | ShapeKind::Circle => default,
-                        ShapeKind::Rect | ShapeKind::Ellipse => 1.5 * default,
-                    }
-                },
-                if regions.expand.y { regions.current.y } else { default },
-            );
-
-            // Don't overflow the region.
-            size.x = size.x.min(regions.current.x);
-            size.y = size.y.min(regions.current.y);
-
-            if matches!(self.kind, ShapeKind::Square | ShapeKind::Circle) {
+            // The default size that a shape takes on if it has no child and
+            // enough space.
+            let mut default = Size::splat(Length::pt(30.0));
+            if !self.kind.is_quadratic() {
+                default.x *= 1.5;
+            }
+
+            let mut size =
+                regions.expand.select(regions.current, default).min(regions.current);
+
+            // Make sure the result is really a square or circle.
+            if self.kind.is_quadratic() {
                 size.x = size.x.min(size.y);
                 size.y = size.x;
             }
 
-            Frame::new(size)
-        };
+            frames = vec![Frame::new(size).constrain(Constraints::tight(regions))];
+        }
+
+        let frame = Rc::make_mut(&mut frames.last_mut().unwrap().item);
 
         // Add fill and/or stroke.
         if self.fill.is_some() || self.stroke.is_some() {
@@ -194,9 +166,34 @@ impl Layout for ShapeNode {
         }
 
         // Ensure frame size matches regions size if expansion is on.
-        frame.size = regions.expand.select(regions.current, frame.size);
+        let target = regions.expand.select(regions.current, frame.size);
+        frame.resize(target, Align::LEFT_TOP);
+
+        frames
+    }
+}
+
+/// The type of a shape.
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
+pub enum ShapeKind {
+    /// A rectangle with equal side lengths.
+    Square,
+    /// A quadrilateral with four right angles.
+    Rect,
+    /// An ellipse with coinciding foci.
+    Circle,
+    /// A curve around two focal points.
+    Ellipse,
+}
+
+impl ShapeKind {
+    /// Whether the shape is curved.
+    pub fn is_round(self) -> bool {
+        matches!(self, Self::Circle | Self::Ellipse)
+    }
 
-        // Return tight constraints for now.
-        vec![frame.constrain(Constraints::tight(regions))]
+    /// Whether the shape has a fixed 1-1 aspect ratio.
+    pub fn is_quadratic(self) -> bool {
+        matches!(self, Self::Square | Self::Circle)
     }
 }
diff --git a/src/library/sized.rs b/src/library/sized.rs
index f421637b..daf2b407 100644
--- a/src/library/sized.rs
+++ b/src/library/sized.rs
@@ -56,14 +56,18 @@ impl Layout for SizedNode {
         };
 
         let mut frames = self.child.layout(ctx, &pod);
-        let Constrained { cts, .. } = &mut frames[0];
+        let Constrained { item: frame, cts } = &mut frames[0];
+
+        // Ensure frame size matches regions size if expansion is on.
+        let target = regions.expand.select(regions.current, frame.size);
+        Rc::make_mut(frame).resize(target, Align::LEFT_TOP);
 
         // Set base & exact constraints if the child is automatically sized
         // since we don't know what the child might have done. Also set base if
         // our sizing is relative.
         *cts = Constraints::new(regions.expand);
-        cts.exact = regions.current.filter(is_auto);
-        cts.base = regions.base.filter(is_auto | is_rel);
+        cts.exact = regions.current.filter(regions.expand | is_auto);
+        cts.base = regions.base.filter(is_rel | is_auto);
 
         frames
     }
diff --git a/src/library/stack.rs b/src/library/stack.rs
index 91f1ef62..2b1371ab 100644
--- a/src/library/stack.rs
+++ b/src/library/stack.rs
@@ -148,10 +148,6 @@ impl<'a> StackLayouter<'a> {
     /// Layout all children.
     fn layout(mut self, ctx: &mut LayoutContext) -> Vec<Constrained<Rc<Frame>>> {
         for child in &self.stack.children {
-            if self.regions.is_full() {
-                self.finish_region();
-            }
-
             match *child {
                 StackChild::Spacing(Spacing::Linear(v)) => {
                     self.layout_absolute(v);
@@ -161,6 +157,10 @@ impl<'a> StackLayouter<'a> {
                     self.fr += v;
                 }
                 StackChild::Node(ref node) => {
+                    if self.regions.is_full() {
+                        self.finish_region();
+                    }
+
                     self.layout_node(ctx, node);
                 }
             }
diff --git a/tests/ref/elements/square.png b/tests/ref/elements/square.png
index 12d00d9b..9663015f 100644
--- a/tests/ref/elements/square.png
+++ b/tests/ref/elements/square.png
diff --git a/tests/ref/layout/grid-3.png b/tests/ref/layout/grid-3.png
index 7ef5df73..6023170c 100644
--- a/tests/ref/layout/grid-3.png
+++ b/tests/ref/layout/grid-3.png
diff --git a/tests/typ/elements/square.typ b/tests/typ/elements/square.typ
index 32a26ea6..f09fc3e0 100644
--- a/tests/typ/elements/square.typ
+++ b/tests/typ/elements/square.typ
@@ -27,7 +27,7 @@
 ]
 
 ---
-// Test required height overflowing page.
+// Test that square does not overflow page.
 #page(width: 100pt, height: 75pt)
 #square(fill: conifer)[
   But, soft! what light through yonder window breaks?