Code Review: Heap is Stack. Unsafe is Good.

Spaghetti Code is Style.

13 files changed, 148 insertions, 197 deletions

diff --git a/src/frame.rs b/src/frame.rs
index dcaa7581..80e25f3b 100644
--- a/src/frame.rs
+++ b/src/frame.rs
@@ -40,6 +40,14 @@ impl Frame {
         self.elements.insert(0, (pos, element));
     }
 
+    /// Add multiple elements at a position in the background.
+    pub fn prepend_multiple<I>(&mut self, insert: I)
+    where
+        I: IntoIterator<Item = (Point, Element)>,
+    {
+        self.elements.splice(0 .. 0, insert);
+    }
+
     /// Add an element at a position in the foreground.
     pub fn push(&mut self, pos: Point, element: Element) {
         self.elements.push((pos, element));
diff --git a/src/geom/angle.rs b/src/geom/angle.rs
index 65270ebd..888442f7 100644
--- a/src/geom/angle.rs
+++ b/src/geom/angle.rs
@@ -64,51 +64,6 @@ impl Angle {
     pub fn cos(self) -> f64 {
         self.to_rad().cos()
     }
-
-    /// Get the control points for a bezier curve that describes a circular arc
-    /// of this angle with the given radius.
-    pub fn bezier_arc(
-        self,
-        radius: Length,
-        rotate: bool,
-        mirror_x: bool,
-        mirror_y: bool,
-    ) -> [Point; 4] {
-        let end = Point::new(self.cos() * radius - radius, self.sin() * radius);
-        let center = Point::new(-radius, Length::zero());
-
-        let mut ts = if mirror_y {
-            Transform::mirror_y()
-        } else {
-            Transform::identity()
-        };
-
-        if mirror_x {
-            ts = ts.pre_concat(Transform::mirror_x());
-        }
-
-        if rotate {
-            ts = ts.pre_concat(Transform::rotate(Angle::deg(90.0)));
-        }
-
-        let a = center * -1.0;
-        let b = end - center;
-
-        let q1 = a.x.to_raw() * a.x.to_raw() + a.y.to_raw() * a.y.to_raw();
-        let q2 = q1 + a.x.to_raw() * b.x.to_raw() + a.y.to_raw() * b.y.to_raw();
-        let k2 = (4.0 / 3.0) * ((2.0 * q1 * q2).sqrt() - q2)
-            / (a.x.to_raw() * b.y.to_raw() - a.y.to_raw() * b.x.to_raw());
-
-        let control_1 = Point::new(center.x + a.x - k2 * a.y, center.y + a.y + k2 * a.x);
-        let control_2 = Point::new(center.x + b.x + k2 * b.y, center.y + b.y - k2 * b.x);
-
-        [
-            Point::zero(),
-            control_1.transform(ts),
-            control_2.transform(ts),
-            end.transform(ts),
-        ]
-    }
 }
 
 impl Numeric for Angle {
diff --git a/src/geom/path.rs b/src/geom/path.rs
index 836be1b4..721cc20b 100644
--- a/src/geom/path.rs
+++ b/src/geom/path.rs
@@ -71,3 +71,48 @@ impl Path {
         self.0.push(PathElement::ClosePath);
     }
 }
+
+/// Get the control points for a bezier curve that describes a circular arc
+/// of this angle with the given radius.
+pub fn bezier_arc(
+    angle: Angle,
+    radius: Length,
+    rotate: bool,
+    mirror_x: bool,
+    mirror_y: bool,
+) -> [Point; 4] {
+    let end = Point::new(angle.cos() * radius - radius, angle.sin() * radius);
+    let center = Point::new(-radius, Length::zero());
+
+    let mut ts = if mirror_y {
+        Transform::mirror_y()
+    } else {
+        Transform::identity()
+    };
+
+    if mirror_x {
+        ts = ts.pre_concat(Transform::mirror_x());
+    }
+
+    if rotate {
+        ts = ts.pre_concat(Transform::rotate(Angle::deg(90.0)));
+    }
+
+    let a = center * -1.0;
+    let b = end - center;
+
+    let q1 = a.x.to_raw() * a.x.to_raw() + a.y.to_raw() * a.y.to_raw();
+    let q2 = q1 + a.x.to_raw() * b.x.to_raw() + a.y.to_raw() * b.y.to_raw();
+    let k2 = (4.0 / 3.0) * ((2.0 * q1 * q2).sqrt() - q2)
+        / (a.x.to_raw() * b.y.to_raw() - a.y.to_raw() * b.x.to_raw());
+
+    let control_1 = Point::new(center.x + a.x - k2 * a.y, center.y + a.y + k2 * a.x);
+    let control_2 = Point::new(center.x + b.x + k2 * b.y, center.y + b.y - k2 * b.x);
+
+    [
+        Point::zero(),
+        control_1.transform(ts),
+        control_2.transform(ts),
+        end.transform(ts),
+    ]
+}
diff --git a/src/geom/rect.rs b/src/geom/rect.rs
index f0da2db6..aa670f0a 100644
--- a/src/geom/rect.rs
+++ b/src/geom/rect.rs
@@ -3,7 +3,7 @@ use super::*;
 use std::mem;
 
 /// A rectangle with rounded corners.
-#[derive(Debug, Clone)]
+#[derive(Debug, Copy, Clone, PartialEq)]
 pub struct Rect {
     size: Size,
     radius: Sides<Length>,
@@ -19,7 +19,7 @@ impl Rect {
     /// in the foreground. The function will output multiple items if the stroke
     /// properties differ by side.
     pub fn shapes(
-        &self,
+        self,
         fill: Option<Paint>,
         stroke: Sides<Option<Stroke>>,
     ) -> Vec<Shape> {
@@ -28,48 +28,64 @@ impl Rect {
             res.push(Shape {
                 geometry: self.fill_geometry(),
                 fill,
-                stroke: stroke.is_uniform().then(|| stroke.top).flatten(),
+                stroke: if stroke.is_uniform() { stroke.top } else { None },
             });
         }
 
         if !stroke.is_uniform() {
-            for (path, stroke) in self.stroke_segments(Some(stroke)) {
-                if !stroke.is_some() {
-                    continue;
+            for (path, stroke) in self.stroke_segments(stroke) {
+                if stroke.is_some() {
+                    res.push(Shape {
+                        geometry: Geometry::Path(path),
+                        fill: None,
+                        stroke,
+                    });
                 }
-                res.push(Shape {
-                    geometry: Geometry::Path(path),
-                    fill: None,
-                    stroke,
-                });
             }
         }
 
         res
     }
 
+    /// Output the shape of the rectangle as a path or primitive rectangle,
+    /// depending on whether it is rounded.
+    fn fill_geometry(self) -> Geometry {
+        if self.radius.iter().copied().all(Length::is_zero) {
+            Geometry::Rect(self.size)
+        } else {
+            let mut paths = self.stroke_segments(Sides::splat(None));
+            assert_eq!(paths.len(), 1);
+
+            Geometry::Path(paths.pop().unwrap().0)
+        }
+    }
+
     /// Output the minimum number of paths along the rectangles border.
     fn stroke_segments(
-        &self,
-        strokes: Option<Sides<Option<Stroke>>>,
+        self,
+        strokes: Sides<Option<Stroke>>,
     ) -> Vec<(Path, Option<Stroke>)> {
-        let strokes = strokes.unwrap_or_else(|| Sides::splat(None));
         let mut res = vec![];
 
-        let mut connection = Connection::None;
+        let mut connection = Connection::default();
         let mut path = Path::new();
         let mut always_continuous = true;
 
         for side in [Side::Top, Side::Right, Side::Bottom, Side::Left] {
-            let radius = [self.radius.get(side.next_ccw()), self.radius.get(side)];
-
-            let stroke_continuity = strokes.get(side) == strokes.get(side.next_cw());
-            connection = connection.advance(stroke_continuity && side != Side::Left);
-            always_continuous &= stroke_continuity;
-
-            draw_side(&mut path, side, self.size, radius[0], radius[1], connection);
-
-            if !stroke_continuity {
+            let is_continuous = strokes.get(side) == strokes.get(side.next_cw());
+            connection = connection.advance(is_continuous && side != Side::Left);
+            always_continuous &= is_continuous;
+
+            draw_side(
+                &mut path,
+                side,
+                self.size,
+                self.radius.get(side.next_ccw()),
+                self.radius.get(side),
+                connection,
+            );
+
+            if !is_continuous {
                 res.push((mem::take(&mut path), strokes.get(side)));
             }
         }
@@ -84,19 +100,6 @@ impl Rect {
 
         res
     }
-
-    /// Output the shape of the rectangle as a path or primitive rectangle,
-    /// depending on whether it is rounded.
-    fn fill_geometry(&self) -> Geometry {
-        if self.radius.iter().copied().all(Length::is_zero) {
-            Geometry::Rect(self.size)
-        } else {
-            let mut paths = self.stroke_segments(None);
-            assert_eq!(paths.len(), 1);
-
-            Geometry::Path(paths.pop().unwrap().0)
-        }
-    }
 }
 
 /// Draws one side of the rounded rectangle. Will always draw the left arc. The
@@ -110,19 +113,18 @@ fn draw_side(
     connection: Connection,
 ) {
     let reversed = |angle: Angle, radius, rotate, mirror_x, mirror_y| {
-        let [a, b, c, d] = angle.bezier_arc(radius, rotate, mirror_x, mirror_y);
+        let [a, b, c, d] = bezier_arc(angle, radius, rotate, mirror_x, mirror_y);
         [d, c, b, a]
     };
 
-    let angle_left = Angle::deg(if connection.left() { 90.0 } else { 45.0 });
-    let angle_right = Angle::deg(if connection.right() { 90.0 } else { 45.0 });
+    let angle_left = Angle::deg(if connection.prev { 90.0 } else { 45.0 });
+    let angle_right = Angle::deg(if connection.next { 90.0 } else { 45.0 });
 
     let (arc1, arc2) = match side {
         Side::Top => {
             let arc1 = reversed(angle_left, radius_left, true, true, false)
                 .map(|x| x + Point::with_x(radius_left));
-            let arc2 = (-angle_right)
-                .bezier_arc(radius_right, true, true, false)
+            let arc2 = bezier_arc(-angle_right, radius_right, true, true, false)
                 .map(|x| x + Point::with_x(size.x - radius_right));
 
             (arc1, arc2)
@@ -131,8 +133,7 @@ fn draw_side(
             let arc1 = reversed(-angle_left, radius_left, false, false, false)
                 .map(|x| x + Point::new(size.x, radius_left));
 
-            let arc2 = angle_right
-                .bezier_arc(radius_right, false, false, false)
+            let arc2 = bezier_arc(angle_right, radius_right, false, false, false)
                 .map(|x| x + Point::new(size.x, size.y - radius_right));
 
             (arc1, arc2)
@@ -141,8 +142,7 @@ fn draw_side(
             let arc1 = reversed(-angle_left, radius_left, true, false, false)
                 .map(|x| x + Point::new(size.x - radius_left, size.y));
 
-            let arc2 = angle_right
-                .bezier_arc(radius_right, true, false, false)
+            let arc2 = bezier_arc(angle_right, radius_right, true, false, false)
                 .map(|x| x + Point::new(radius_right, size.y));
 
             (arc1, arc2)
@@ -151,15 +151,14 @@ fn draw_side(
             let arc1 = reversed(angle_left, radius_left, false, false, true)
                 .map(|x| x + Point::with_y(size.y - radius_left));
 
-            let arc2 = (-angle_right)
-                .bezier_arc(radius_right, false, false, true)
+            let arc2 = bezier_arc(-angle_right, radius_right, false, false, true)
                 .map(|x| x + Point::with_y(radius_right));
 
             (arc1, arc2)
         }
     };
 
-    if !connection.left() {
+    if !connection.prev {
         path.move_to(if radius_left.is_zero() { arc1[3] } else { arc1[0] });
     }
 
@@ -169,51 +168,24 @@ fn draw_side(
 
     path.line_to(arc2[0]);
 
-    if !connection.right() && !radius_right.is_zero() {
+    if !connection.next && !radius_right.is_zero() {
         path.cubic_to(arc2[1], arc2[2], arc2[3]);
     }
 }
 
 /// A state machine that indicates which sides of the border strokes in a 2D
 /// polygon are connected to their neighboring sides.
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
-enum Connection {
-    None,
-    Left,
-    Right,
-    Both,
+#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)]
+struct Connection {
+    prev: bool,
+    next: bool,
 }
 
 impl Connection {
     /// Advance to the next clockwise side of the polygon. The argument
     /// indicates whether the border is connected on the right side of the next
     /// edge.
-    pub fn advance(self, right: bool) -> Self {
-        match self {
-            Self::Right | Self::Both => {
-                if right {
-                    Self::Both
-                } else {
-                    Self::Left
-                }
-            }
-            Self::Left | Self::None => {
-                if right {
-                    Self::Right
-                } else {
-                    Self::None
-                }
-            }
-        }
-    }
-
-    /// Whether there is a connection on the left.
-    fn left(self) -> bool {
-        matches!(self, Self::Left | Self::Both)
-    }
-
-    /// Whether there is a connection on the right.
-    fn right(self) -> bool {
-        matches!(self, Self::Right | Self::Both)
+    pub fn advance(self, next: bool) -> Self {
+        Self { prev: self.next, next }
     }
 }
diff --git a/src/geom/sides.rs b/src/geom/sides.rs
index f214a1bf..555bbd62 100644
--- a/src/geom/sides.rs
+++ b/src/geom/sides.rs
@@ -45,6 +45,19 @@ impl<T> Sides<T> {
         }
     }
 
+    /// Zip two instances into an instance.
+    pub fn zip<F, V, W>(self, other: Sides<V>, mut f: F) -> Sides<W>
+    where
+        F: FnMut(T, V, Side) -> W,
+    {
+        Sides {
+            left: f(self.left, other.left, Side::Left),
+            top: f(self.top, other.top, Side::Top),
+            right: f(self.right, other.right, Side::Right),
+            bottom: f(self.bottom, other.bottom, Side::Bottom),
+        }
+    }
+
     /// Returns an iterator over the sides.
     pub fn iter(&self) -> impl Iterator<Item = &T> {
         [&self.left, &self.top, &self.right, &self.bottom].into_iter()
diff --git a/src/geom/transform.rs b/src/geom/transform.rs
index de2a9781..961ba487 100644
--- a/src/geom/transform.rs
+++ b/src/geom/transform.rs
@@ -26,26 +26,12 @@ impl Transform {
 
     /// Transform by mirroring along the x-axis.
     pub fn mirror_x() -> Self {
-        Self {
-            sx: Ratio::one(),
-            ky: Ratio::zero(),
-            kx: Ratio::zero(),
-            sy: -Ratio::one(),
-            tx: Length::zero(),
-            ty: Length::zero(),
-        }
+        Self::scale(Ratio::one(), -Ratio::one())
     }
 
     /// Transform by mirroring along the y-axis.
     pub fn mirror_y() -> Self {
-        Self {
-            sx: -Ratio::one(),
-            ky: Ratio::zero(),
-            kx: Ratio::zero(),
-            sy: Ratio::one(),
-            tx: Length::zero(),
-            ty: Length::zero(),
-        }
+        Self::scale(-Ratio::one(), Ratio::one())
     }
 
     /// A translate transform.
diff --git a/src/library/graphics/shape.rs b/src/library/graphics/shape.rs
index a5523a2e..40b6e1e3 100644
--- a/src/library/graphics/shape.rs
+++ b/src/library/graphics/shape.rs
@@ -78,7 +78,7 @@ impl<const S: ShapeKind> ShapeNode<S> {
         styles.set_opt(Self::INSET, args.named("inset")?);
         styles.set_opt(Self::OUTSET, args.named("outset")?);
 
-        if S != CIRCLE {
+        if !is_round(S) {
             styles.set_opt(Self::RADIUS, args.named("radius")?);
         }
 
@@ -97,10 +97,7 @@ impl<const S: ShapeKind> Layout for ShapeNode<S> {
         if let Some(child) = &self.0 {
             let mut inset = styles.get(Self::INSET);
             if is_round(S) {
-                inset = inset.map(|mut side| {
-                    side.rel += Ratio::new(0.5 - SQRT_2 / 4.0);
-                    side
-                });
+                inset = inset.map(|side| side + Ratio::new(0.5 - SQRT_2 / 4.0));
             }
 
             // Pad the child.
@@ -158,18 +155,8 @@ impl<const S: ShapeKind> Layout for ShapeNode<S> {
             }
         };
 
-        let outset = styles.get(Self::OUTSET);
-        let outset = Sides {
-            left: outset.left.relative_to(frame.size.x),
-            top: outset.top.relative_to(frame.size.y),
-            right: outset.right.relative_to(frame.size.x),
-            bottom: outset.bottom.relative_to(frame.size.y),
-        };
-
-        let size = Spec::new(
-            frame.size.x + outset.left + outset.right,
-            frame.size.y + outset.top + outset.bottom,
-        );
+        let outset = styles.get(Self::OUTSET).relative_to(frame.size);
+        let size = frame.size + outset.sum_by_axis();
 
         let radius = styles
             .get(Self::RADIUS)
@@ -186,11 +173,12 @@ impl<const S: ShapeKind> Layout for ShapeNode<S> {
                 };
                 frame.prepend(pos, Element::Shape(shape));
             } else {
-                for shape in
-                    Rect::new(size, radius).shapes(fill, stroke).into_iter().rev()
-                {
-                    frame.prepend(pos, Element::Shape(shape));
-                }
+                frame.prepend_multiple(
+                    Rect::new(size, radius)
+                        .shapes(fill, stroke)
+                        .into_iter()
+                        .map(|x| (pos, Element::Shape(x))),
+                )
             }
         }
 
diff --git a/src/library/layout/page.rs b/src/library/layout/page.rs
index 801a137d..c8495e64 100644
--- a/src/library/layout/page.rs
+++ b/src/library/layout/page.rs
@@ -20,7 +20,8 @@ impl PageNode {
 
     /// The page margin.
     #[property(fold)]
-    pub const MARGINS: Sides<Smart<Relative<RawLength>>> = Sides::splat(Smart::Auto);
+    pub const MARGINS: Sides<Option<Smart<Relative<RawLength>>>> =
+        Sides::splat(Smart::Auto);
 
     /// How many columns the page has.
     pub const COLUMNS: NonZeroUsize = NonZeroUsize::new(1).unwrap();
@@ -48,9 +49,7 @@ impl PageNode {
 
         styles.set_opt(Self::WIDTH, args.named("width")?);
         styles.set_opt(Self::HEIGHT, args.named("height")?);
-
         styles.set_opt(Self::MARGINS, args.named("margins")?);
-
         styles.set_opt(Self::FLIPPED, args.named("flipped")?);
         styles.set_opt(Self::FILL, args.named("fill")?);
         styles.set_opt(Self::COLUMNS, args.named("columns")?);
diff --git a/src/model/styles.rs b/src/model/styles.rs
index 1fddfd0e..ae4c1586 100644
--- a/src/model/styles.rs
+++ b/src/model/styles.rs
@@ -466,12 +466,7 @@ where
     type Output = Sides<T::Output>;
 
     fn fold(self, outer: Self::Output) -> Self::Output {
-        Sides {
-            left: self.left.fold(outer.left),
-            top: self.top.fold(outer.top),
-            right: self.right.fold(outer.right),
-            bottom: self.bottom.fold(outer.bottom),
-        }
+        self.zip(outer, |inner, outer, _| inner.fold(outer))
     }
 }
 
@@ -479,25 +474,15 @@ impl Fold for Sides<Option<Relative<Length>>> {
     type Output = Sides<Relative<Length>>;
 
     fn fold(self, outer: Self::Output) -> Self::Output {
-        Sides {
-            left: self.left.unwrap_or(outer.left),
-            top: self.top.unwrap_or(outer.top),
-            right: self.right.unwrap_or(outer.right),
-            bottom: self.bottom.unwrap_or(outer.bottom),
-        }
+        self.zip(outer, |inner, outer, _| inner.unwrap_or(outer))
     }
 }
 
-impl Fold for Sides<Smart<Relative<RawLength>>> {
+impl Fold for Sides<Option<Smart<Relative<RawLength>>>> {
     type Output = Sides<Smart<Relative<RawLength>>>;
 
     fn fold(self, outer: Self::Output) -> Self::Output {
-        Sides {
-            left: self.left.or(outer.left),
-            top: self.top.or(outer.top),
-            right: self.right.or(outer.right),
-            bottom: self.bottom.or(outer.bottom),
-        }
+        self.zip(outer, |inner, outer, _| inner.unwrap_or(outer))
     }
 }
 
diff --git a/tests/ref/layout/page.png b/tests/ref/layout/page.png
index e0618e58..35716e4d 100644
--- a/tests/ref/layout/page.png
+++ b/tests/ref/layout/page.png
diff --git a/tests/typ/graphics/shape-rect.typ b/tests/typ/graphics/shape-rect.typ
index 3d157675..a29550b5 100644
--- a/tests/typ/graphics/shape-rect.typ
+++ b/tests/typ/graphics/shape-rect.typ
@@ -35,20 +35,20 @@
 
 // Different strokes.
 [
-  #set rect(stroke: (right: red,))
+  #set rect(stroke: (right: red))
   #rect(width: 100%, fill: lime, stroke: (x: 5pt, y: 1pt))
 ]
 
 ---
 // Outset padding.
-#let inline-code(body) = [
+#show node: raw as [
   #set text("IBM Plex Mono", 8pt)
   #h(.7em, weak: true)
-  #rect(radius: 3pt, outset: (y: 3pt, x: 2.5pt), fill: rgb(239, 241, 243), body)
+  #rect(radius: 3pt, outset: (y: 3pt, x: 2.5pt), fill: rgb(239, 241, 243))[{node.text}]
   #h(.7em, weak: true)
 ]
 
-Use the #inline-code[\*const ptr] pointer.
+Use the `*const ptr` pointer.
 
 ---
 // Error: 15-38 unexpected key "cake"
diff --git a/tests/typ/layout/page-margin.typ b/tests/typ/layout/page-margin.typ
index e30518b0..290c4081 100644
--- a/tests/typ/layout/page-margin.typ
+++ b/tests/typ/layout/page-margin.typ
@@ -11,10 +11,10 @@
 ---
 // Set individual margins.
 #set page(height: 40pt)
-[#set page(margins: (left: 0pt,)); #align(left)[Left]]
-[#set page(margins: (right: 0pt,)); #align(right)[Right]]
-[#set page(margins: (top: 0pt,)); #align(top)[Top]]
-[#set page(margins: (bottom: 0pt,)); #align(bottom)[Bottom]]
+[#set page(margins: (left: 0pt)); #align(left)[Left]]
+[#set page(margins: (right: 0pt)); #align(right)[Right]]
+[#set page(margins: (top: 0pt)); #align(top)[Top]]
+[#set page(margins: (bottom: 0pt)); #align(bottom)[Bottom]]
 
 // Ensure that specific margins override general margins.
 [#set page(margins: (rest: 0pt, left: 20pt)); Overriden]
diff --git a/tests/typeset.rs b/tests/typeset.rs
index d3f7844b..b0452163 100644
--- a/tests/typeset.rs
+++ b/tests/typeset.rs
@@ -66,7 +66,7 @@ fn main() {
     styles.set(PageNode::HEIGHT, Smart::Auto);
     styles.set(
         PageNode::MARGINS,
-        Sides::splat(Smart::Custom(Length::pt(10.0).into())),
+        Sides::splat(Some(Smart::Custom(Length::pt(10.0).into()))),
     );
     styles.set(TextNode::SIZE, TextSize(Length::pt(10.0).into()));