Clean up flow a bit (#4505)

crates/typst/src/layout/flow.rs

@@ -47,51 +47,7 @@ impl Packed<FlowElem> {
         styles: StyleChain,
         regions: Regions,
     ) -> SourceResult<Fragment> {
-        if !regions.size.x.is_finite() && regions.expand.x {
+        FlowLayouter::new(engine, self, locator, &styles, regions).layout()
-            bail!(self.span(), "cannot expand into infinite width");
-        }
-        if !regions.size.y.is_finite() && regions.expand.y {
-            bail!(self.span(), "cannot expand into infinite height");
-        }
-
-        // Check whether we have just a single multiple-layoutable element. In
-        // that case, we do not set `expand.y` to `false`, but rather keep it at
-        // its original value (since that element can take the full space).
-        //
-        // Consider the following code: `block(height: 5cm, pad(10pt, align(bottom, ..)))`
-        // Thanks to the code below, the expansion will be passed all the way
-        // through the block & pad and reach the innermost flow, so that things
-        // are properly bottom-aligned.
-        let mut alone = false;
-        if let [child] = self.children().elements() {
-            alone = child.is::<BlockElem>();
-        }
-
-        let mut layouter = FlowLayouter::new(locator, styles, regions, alone);
-        for (child, styles) in self.children().chain(&styles) {
-            if let Some(elem) = child.to_packed::<TagElem>() {
-                layouter.layout_tag(elem);
-            } else if child.is::<FlushElem>() {
-                layouter.flush(engine)?;
-            } else if let Some(elem) = child.to_packed::<VElem>() {
-                layouter.layout_spacing(engine, elem, styles)?;
-            } else if let Some(elem) = child.to_packed::<ParElem>() {
-                layouter.layout_par(engine, elem, styles)?;
-            } else if let Some(elem) = child.to_packed::<BlockElem>() {
-                layouter.layout_block(engine, elem, styles)?;
-            } else if let Some(placed) = child.to_packed::<PlaceElem>() {
-                layouter.layout_placed(engine, placed, styles)?;
-            } else if child.is::<ColbreakElem>() {
-                if !layouter.regions.backlog.is_empty() || layouter.regions.last.is_some()
-                {
-                    layouter.finish_region(engine, true)?;
-                }
-            } else {
-                bail!(child.span(), "unexpected flow child");
-            }
-        }
-
-        layouter.finish(engine)
     }
 }
 
@@ -103,13 +59,17 @@ impl Debug for FlowElem {
 }
 
 /// Performs flow layout.
-struct FlowLayouter<'a> {
+struct FlowLayouter<'a, 'e> {
+    /// The engine.
+    engine: &'a mut Engine<'e>,
+    /// The children that will be arranged into a flow.
+    flow: &'a Packed<FlowElem>,
     /// Whether this is the root flow.
     root: bool,
     /// Provides unique locations to the flow's children.
     locator: SplitLocator<'a>,
     /// The shared styles.
-    styles: StyleChain<'a>,
+    styles: &'a StyleChain<'a>,
     /// The regions to layout children into.
     regions: Regions<'a>,
     /// Whether the flow should expand to fill the region.
@@ -124,7 +84,7 @@ struct FlowLayouter<'a> {
     /// Spacing and layouted blocks for the current region.
     items: Vec<FlowItem>,
     /// A queue of tags that will be attached to the next frame.
-    pending_tags: Vec<Tag>,
+    pending_tags: Vec<&'a Tag>,
     /// A queue of floating elements.
     pending_floats: Vec<FlowItem>,
     /// Whether we have any footnotes in the current region.
@@ -157,18 +117,27 @@ enum FlowItem {
         align: Axes<FixedAlignment>,
         /// Whether the frame sticks to the item after it (for orphan prevention).
         sticky: bool,
-        /// Whether the frame is movable; that is, kept together with its footnotes.
+        /// Whether the frame is movable; that is, kept together with its
+        /// footnotes.
         ///
-        /// This is true for frames created by paragraphs and [`LayoutSingle`] elements.
+        /// This is true for frames created by paragraphs and
+        /// [`BlockElem::single_layouter`] elements.
         movable: bool,
     },
     /// An absolutely placed frame.
     Placed {
+        /// The layouted content.
         frame: Frame,
+        /// Where to place the content horizontally.
         x_align: FixedAlignment,
+        /// Where to place the content vertically.
         y_align: Smart<Option<FixedAlignment>>,
+        /// A translation to apply to the content.
         delta: Axes<Rel<Abs>>,
+        /// Whether the content floats --- i.e. collides with in-flow content.
         float: bool,
+        /// The amount of space that needs to be kept between the placed content
+        /// and in-flow content. Only relevant if `float` is `true`.
         clearance: Abs,
     },
     /// A footnote frame (can also be the separator).
@@ -193,24 +162,41 @@ impl FlowItem {
     }
 }
 
-impl<'a> FlowLayouter<'a> {
+impl<'a, 'e> FlowLayouter<'a, 'e> {
     /// Create a new flow layouter.
     fn new(
+        engine: &'a mut Engine<'e>,
+        flow: &'a Packed<FlowElem>,
         locator: Locator<'a>,
-        styles: StyleChain<'a>,
+        styles: &'a StyleChain<'a>,
         mut regions: Regions<'a>,
-        alone: bool,
     ) -> Self {
-        let expand = regions.expand;
+        // Check whether we have just a single multiple-layoutable element. In
-        let root = std::mem::replace(&mut regions.root, false);
+        // that case, we do not set `expand.y` to `false`, but rather keep it at
+        // its original value (since that element can take the full space).
+        //
+        // Consider the following code: `block(height: 5cm, pad(10pt,
+        // align(bottom, ..)))`. Thanks to the code below, the expansion will be
+        // passed all the way through the block & pad and reach the innermost
+        // flow, so that things are properly bottom-aligned.
+        let mut alone = false;
+        if let [child] = flow.children.elements() {
+            alone = child.is::<BlockElem>();
+        }
 
         // Disable vertical expansion when there are multiple or not directly
         // layoutable children.
+        let expand = regions.expand;
         if !alone {
             regions.expand.y = false;
         }
 
+        // The children aren't root.
+        let root = std::mem::replace(&mut regions.root, false);
+
         Self {
+            engine,
+            flow,
             root,
             locator: locator.split(),
             styles,
@@ -223,52 +209,84 @@ impl<'a> FlowLayouter<'a> {
             pending_floats: vec![],
             has_footnotes: false,
             footnote_config: FootnoteConfig {
-                separator: FootnoteEntry::separator_in(styles),
+                separator: FootnoteEntry::separator_in(*styles),
-                clearance: FootnoteEntry::clearance_in(styles),
+                clearance: FootnoteEntry::clearance_in(*styles),
-                gap: FootnoteEntry::gap_in(styles),
+                gap: FootnoteEntry::gap_in(*styles),
             },
             finished: vec![],
         }
     }
 
+    /// Layout the flow.
+    fn layout(mut self) -> SourceResult<Fragment> {
+        for (child, styles) in self.flow.children.chain(self.styles) {
+            if let Some(elem) = child.to_packed::<TagElem>() {
+                self.handle_tag(elem);
+            } else if let Some(elem) = child.to_packed::<VElem>() {
+                self.handle_v(elem, styles)?;
+            } else if let Some(elem) = child.to_packed::<ColbreakElem>() {
+                self.handle_colbreak(elem)?;
+            } else if let Some(elem) = child.to_packed::<ParElem>() {
+                self.handle_par(elem, styles)?;
+            } else if let Some(elem) = child.to_packed::<BlockElem>() {
+                self.handle_block(elem, styles)?;
+            } else if let Some(elem) = child.to_packed::<PlaceElem>() {
+                self.handle_place(elem, styles)?;
+            } else if let Some(elem) = child.to_packed::<FlushElem>() {
+                self.handle_flush(elem)?;
+            } else {
+                bail!(child.span(), "unexpected flow child");
+            }
+        }
+
+        self.finish()
+    }
+
     /// Place explicit metadata into the flow.
-    fn layout_tag(&mut self, elem: &Packed<TagElem>) {
+    fn handle_tag(&mut self, elem: &'a Packed<TagElem>) {
-        self.pending_tags.push(elem.tag.clone());
+        self.pending_tags.push(&elem.tag);
     }
 
     /// Layout vertical spacing.
-    fn layout_spacing(
+    fn handle_v(&mut self, v: &'a Packed<VElem>, styles: StyleChain) -> SourceResult<()> {
-        &mut self,
+        self.handle_item(match v.amount {
-        engine: &mut Engine,
+            Spacing::Rel(rel) => FlowItem::Absolute(
-        v: &Packed<VElem>,
+                // Resolve the spacing relative to the current base height.
-        styles: StyleChain,
+                rel.resolve(styles).relative_to(self.initial.y),
-    ) -> SourceResult<()> {
+                v.weakness(styles) > 0,
-        self.layout_item(
+            ),
-            engine,
+            Spacing::Fr(fr) => FlowItem::Fractional(fr),
-            match v.amount() {
+        })
-                Spacing::Rel(rel) => FlowItem::Absolute(
+    }
-                    rel.resolve(styles).relative_to(self.initial.y),
+
-                    v.weakness(styles) > 0,
+    /// Layout a column break.
-                ),
+    fn handle_colbreak(&mut self, _: &'a Packed<ColbreakElem>) -> SourceResult<()> {
-                Spacing::Fr(fr) => FlowItem::Fractional(*fr),
+        // If there is still an available region, skip to it.
-            },
+        // TODO: Turn this into a region abstraction.
-        )
+        if !self.regions.backlog.is_empty() || self.regions.last.is_some() {
+            self.finish_region(true)?;
+        }
+        Ok(())
     }
 
     /// Layout a paragraph.
-    fn layout_par(
+    fn handle_par(
         &mut self,
-        engine: &mut Engine,
+        par: &'a Packed<ParElem>,
-        par: &Packed<ParElem>,
         styles: StyleChain,
     ) -> SourceResult<()> {
+        // Fetch properties.
         let align = AlignElem::alignment_in(styles).resolve(styles);
         let leading = ParElem::leading_in(styles);
+
+        // Layout the paragraph into lines. This only depends on the base size,
+        // not on the Y position.
         let consecutive = self.last_was_par;
+        let locator = self.locator.next(&par.span());
         let lines = par
             .layout(
-                engine,
+                self.engine,
-                self.locator.next(&par.span()),
+                locator,
                 styles,
                 consecutive,
                 self.regions.base(),
@@ -280,39 +298,26 @@ impl<'a> FlowLayouter<'a> {
         // previous sticky frame to the next region (if available)
         if let Some(first) = lines.first() {
             while !self.regions.size.y.fits(first.height()) && !self.regions.in_last() {
-                let mut sticky = self.items.len();
+                let in_last = self.finish_region_with_migration()?;
-                for (i, item) in self.items.iter().enumerate().rev() {
-                    match *item {
-                        FlowItem::Absolute(_, _) => {}
-                        FlowItem::Frame { sticky: true, .. } => sticky = i,
-                        _ => break,
-                    }
-                }
-
-                let carry: Vec<_> = self.items.drain(sticky..).collect();
-                self.finish_region(engine, false)?;
-                let in_last = self.regions.in_last();
-
-                for item in carry {
-                    self.layout_item(engine, item)?;
-                }
-
                 if in_last {
                     break;
                 }
             }
         }
 
+        // Layout the lines.
         for (i, mut frame) in lines.into_iter().enumerate() {
             if i > 0 {
-                self.layout_item(engine, FlowItem::Absolute(leading, true))?;
+                self.handle_item(FlowItem::Absolute(leading, true))?;
             }
 
             self.drain_tag(&mut frame);
-            self.layout_item(
+            self.handle_item(FlowItem::Frame {
-                engine,
+                frame,
-                FlowItem::Frame { frame, align, sticky: false, movable: true },
+                align,
-            )?;
+                sticky: false,
+                movable: true,
+            })?;
         }
 
         self.last_was_par = true;
@@ -320,56 +325,54 @@ impl<'a> FlowLayouter<'a> {
     }
 
     /// Layout into multiple regions.
-    fn layout_block(
+    fn handle_block(
         &mut self,
-        engine: &mut Engine,
         block: &'a Packed<BlockElem>,
         styles: StyleChain<'a>,
     ) -> SourceResult<()> {
-        // Temporarily delegate rootness to the columns.
+        // Fetch properties.
+        let sticky = block.sticky(styles);
+        let align = AlignElem::alignment_in(styles).resolve(styles);
+
+        // If the block is "rootable" it may host footnotes. In that case, we
+        // defer rootness to it temporarily. We disable our own rootness to
+        // prevent duplicate footnotes.
         let is_root = self.root;
         if is_root && block.rootable(styles) {
             self.root = false;
             self.regions.root = true;
         }
 
+        // Skip directly if region is already full.
         if self.regions.is_full() {
-            // Skip directly if region is already full.
+            self.finish_region(false)?;
-            self.finish_region(engine, false)?;
         }
 
         // Layout the block itself.
-        let sticky = block.sticky(styles);
         let fragment = block.layout(
-            engine,
+            self.engine,
             self.locator.next(&block.span()),
             styles,
             self.regions,
         )?;
 
-        // How to align the block.
-        let align = AlignElem::alignment_in(styles).resolve(styles);
-
         let mut notes = Vec::new();
         for (i, mut frame) in fragment.into_iter().enumerate() {
             // Find footnotes in the frame.
             if self.root {
-                find_footnotes(&mut notes, &frame);
+                collect_footnotes(&mut notes, &frame);
             }
 
             if i > 0 {
-                self.finish_region(engine, false)?;
+                self.finish_region(false)?;
             }
 
             self.drain_tag(&mut frame);
             frame.post_process(styles);
-            self.layout_item(
+            self.handle_item(FlowItem::Frame { frame, align, sticky, movable: false })?;
-                engine,
-                FlowItem::Frame { frame, align, sticky, movable: false },
-            )?;
         }
 
-        self.try_handle_footnotes(engine, notes)?;
+        self.try_handle_footnotes(notes)?;
 
         self.root = is_root;
         self.regions.root = false;
@@ -379,50 +382,56 @@ impl<'a> FlowLayouter<'a> {
     }
 
     /// Layout a placed element.
-    fn layout_placed(
+    fn handle_place(
         &mut self,
-        engine: &mut Engine,
+        placed: &'a Packed<PlaceElem>,
-        placed: &Packed<PlaceElem>,
         styles: StyleChain,
     ) -> SourceResult<()> {
+        // Fetch properties.
         let float = placed.float(styles);
         let clearance = placed.clearance(styles);
         let alignment = placed.alignment(styles);
         let delta = Axes::new(placed.dx(styles), placed.dy(styles)).resolve(styles);
+
         let x_align = alignment.map_or(FixedAlignment::Center, |align| {
             align.x().unwrap_or_default().resolve(styles)
         });
         let y_align = alignment.map(|align| align.y().map(|y| y.resolve(styles)));
+
         let mut frame = placed
             .layout(
-                engine,
+                self.engine,
                 self.locator.next(&placed.span()),
                 styles,
                 self.regions.base(),
             )?
             .into_frame();
+
         frame.post_process(styles);
-        let item = FlowItem::Placed { frame, x_align, y_align, delta, float, clearance };
+
-        self.layout_item(engine, item)
+        self.handle_item(FlowItem::Placed {
+            frame,
+            x_align,
+            y_align,
+            delta,
+            float,
+            clearance,
+        })
     }
 
-    /// Attach currently pending metadata to the frame.
+    /// Lays out all floating elements before continuing with other content.
-    fn drain_tag(&mut self, frame: &mut Frame) {
+    fn handle_flush(&mut self, _: &'a Packed<FlushElem>) -> SourceResult<()> {
-        if !self.pending_tags.is_empty() && !frame.is_empty() {
+        for item in std::mem::take(&mut self.pending_floats) {
-            frame.prepend_multiple(
+            self.handle_item(item)?;
-                self.pending_tags
-                    .drain(..)
-                    .map(|tag| (Point::zero(), FrameItem::Tag(tag))),
-            );
         }
+        while !self.pending_floats.is_empty() {
+            self.finish_region(false)?;
+        }
+        Ok(())
     }
 
     /// Layout a finished frame.
-    fn layout_item(
+    fn handle_item(&mut self, mut item: FlowItem) -> SourceResult<()> {
-        &mut self,
-        engine: &mut Engine,
-        mut item: FlowItem,
-    ) -> SourceResult<()> {
         match item {
             FlowItem::Absolute(v, weak) => {
                 if weak
@@ -439,24 +448,24 @@ impl<'a> FlowLayouter<'a> {
             FlowItem::Frame { ref frame, movable, .. } => {
                 let height = frame.height();
                 while !self.regions.size.y.fits(height) && !self.regions.in_last() {
-                    self.finish_region(engine, false)?;
+                    self.finish_region(false)?;
                 }
 
                 let in_last = self.regions.in_last();
                 self.regions.size.y -= height;
                 if self.root && movable {
                     let mut notes = Vec::new();
-                    find_footnotes(&mut notes, frame);
+                    collect_footnotes(&mut notes, frame);
                     self.items.push(item);
 
                     // When we are already in_last, we can directly force the
                     // footnotes.
-                    if !self.handle_footnotes(engine, &mut notes, true, in_last)? {
+                    if !self.handle_footnotes(&mut notes, true, in_last)? {
                         let item = self.items.pop();
-                        self.finish_region(engine, false)?;
+                        self.finish_region(false)?;
                         self.items.extend(item);
                         self.regions.size.y -= height;
-                        self.handle_footnotes(engine, &mut notes, true, true)?;
+                        self.handle_footnotes(&mut notes, true, true)?;
                     }
                     return Ok(());
                 }
@@ -504,8 +513,8 @@ impl<'a> FlowLayouter<'a> {
                 // Find footnotes in the frame.
                 if self.root {
                     let mut notes = vec![];
-                    find_footnotes(&mut notes, frame);
+                    collect_footnotes(&mut notes, frame);
-                    self.try_handle_footnotes(engine, notes)?;
+                    self.try_handle_footnotes(notes)?;
                 }
             }
             FlowItem::Footnote(_) => {}
@@ -515,12 +524,49 @@ impl<'a> FlowLayouter<'a> {
         Ok(())
     }
 
+    /// Attach currently pending metadata to the frame.
+    fn drain_tag(&mut self, frame: &mut Frame) {
+        if !self.pending_tags.is_empty() && !frame.is_empty() {
+            frame.prepend_multiple(
+                self.pending_tags
+                    .drain(..)
+                    .map(|tag| (Point::zero(), FrameItem::Tag(tag.clone()))),
+            );
+        }
+    }
+
+    /// Finisht the region, migrating all sticky items to the next one.
+    ///
+    /// Returns whether we migrated into a last region.
+    fn finish_region_with_migration(&mut self) -> SourceResult<bool> {
+        // Find the suffix of sticky items.
+        let mut sticky = self.items.len();
+        for (i, item) in self.items.iter().enumerate().rev() {
+            match *item {
+                FlowItem::Absolute(_, _) => {}
+                FlowItem::Frame { sticky: true, .. } => sticky = i,
+                _ => break,
+            }
+        }
+
+        let carry: Vec<_> = self.items.drain(sticky..).collect();
+        self.finish_region(false)?;
+
+        let in_last = self.regions.in_last();
+        for item in carry {
+            self.handle_item(item)?;
+        }
+
+        Ok(in_last)
+    }
+
     /// Finish the frame for one region.
     ///
     /// Set `force` to `true` to allow creating a frame for out-of-flow elements
     /// only (this is used to force the creation of a frame in case the
     /// remaining elements are all out-of-flow).
-    fn finish_region(&mut self, engine: &mut Engine, force: bool) -> SourceResult<()> {
+    fn finish_region(&mut self, force: bool) -> SourceResult<()> {
+        // Early return if we don't have any relevant items.
         if !force
             && !self.items.is_empty()
             && self.items.iter().all(FlowItem::is_out_of_flow)
@@ -585,6 +631,13 @@ impl<'a> FlowLayouter<'a> {
             size.y = self.initial.y;
         }
 
+        if !self.regions.size.x.is_finite() && self.expand.x {
+            bail!(self.flow.span(), "cannot expand into infinite width");
+        }
+        if !self.regions.size.y.is_finite() && self.expand.y {
+            bail!(self.flow.span(), "cannot expand into infinite height");
+        }
+
         let mut output = Frame::soft(size);
         let mut ruler = FixedAlignment::Start;
         let mut float_top_offset = Abs::zero();
@@ -653,7 +706,9 @@ impl<'a> FlowLayouter<'a> {
         if force && !self.pending_tags.is_empty() {
             let pos = Point::with_y(offset);
             output.push_multiple(
-                self.pending_tags.drain(..).map(|tag| (pos, FrameItem::Tag(tag))),
+                self.pending_tags
+                    .drain(..)
+                    .map(|tag| (pos, FrameItem::Tag(tag.clone()))),
             );
         }
 
@@ -665,62 +720,42 @@ impl<'a> FlowLayouter<'a> {
 
         // Try to place floats into the next region.
         for item in std::mem::take(&mut self.pending_floats) {
-            self.layout_item(engine, item)?;
+            self.handle_item(item)?;
-        }
-
-        Ok(())
-    }
-
-    /// Lays out all floating elements before continuing with other content.
-    fn flush(&mut self, engine: &mut Engine) -> SourceResult<()> {
-        for item in std::mem::take(&mut self.pending_floats) {
-            self.layout_item(engine, item)?;
-        }
-        while !self.pending_floats.is_empty() {
-            self.finish_region(engine, false)?;
         }
 
         Ok(())
     }
 
     /// Finish layouting and return the resulting fragment.
-    fn finish(mut self, engine: &mut Engine) -> SourceResult<Fragment> {
+    fn finish(mut self) -> SourceResult<Fragment> {
         if self.expand.y {
             while !self.regions.backlog.is_empty() {
-                self.finish_region(engine, true)?;
+                self.finish_region(true)?;
             }
         }
 
-        self.finish_region(engine, true)?;
+        self.finish_region(true)?;
         while !self.items.is_empty() {
-            self.finish_region(engine, true)?;
+            self.finish_region(true)?;
         }
 
         Ok(Fragment::frames(self.finished))
     }
-}
 
-impl FlowLayouter<'_> {
     /// Tries to process all footnotes in the frame, placing them
     /// in the next region if they could not be placed in the current
     /// one.
     fn try_handle_footnotes(
         &mut self,
-        engine: &mut Engine,
         mut notes: Vec<Packed<FootnoteElem>>,
     ) -> SourceResult<()> {
         // When we are already in_last, we can directly force the
         // footnotes.
         if self.root
-            && !self.handle_footnotes(
+            && !self.handle_footnotes(&mut notes, false, self.regions.in_last())?
-                engine,
-                &mut notes,
-                false,
-                self.regions.in_last(),
-            )?
         {
-            self.finish_region(engine, false)?;
+            self.finish_region(false)?;
-            self.handle_footnotes(engine, &mut notes, false, true)?;
+            self.handle_footnotes(&mut notes, false, true)?;
         }
         Ok(())
     }
@@ -731,7 +766,6 @@ impl FlowLayouter<'_> {
     /// regions.
     fn handle_footnotes(
         &mut self,
-        engine: &mut Engine,
         notes: &mut Vec<Packed<FootnoteElem>>,
         movable: bool,
         force: bool,
@@ -750,16 +784,16 @@ impl FlowLayouter<'_> {
             }
 
             if !self.has_footnotes {
-                self.layout_footnote_separator(engine)?;
+                self.layout_footnote_separator()?;
             }
 
             self.regions.size.y -= self.footnote_config.gap;
             let frames = FootnoteEntry::new(notes[k].clone())
                 .pack()
                 .layout(
-                    engine,
+                    self.engine,
                     Locator::synthesize(notes[k].location().unwrap()),
-                    self.styles,
+                    *self.styles,
                     self.regions.with_root(false),
                 )?
                 .into_frames();
@@ -780,10 +814,10 @@ impl FlowLayouter<'_> {
 
             let prev = notes.len();
             for (i, frame) in frames.into_iter().enumerate() {
-                find_footnotes(notes, &frame);
+                collect_footnotes(notes, &frame);
                 if i > 0 {
-                    self.finish_region(engine, false)?;
+                    self.finish_region(false)?;
-                    self.layout_footnote_separator(engine)?;
+                    self.layout_footnote_separator()?;
                     self.regions.size.y -= self.footnote_config.gap;
                 }
                 self.regions.size.y -= frame.height();
@@ -804,14 +838,14 @@ impl FlowLayouter<'_> {
     }
 
     /// Layout and save the footnote separator, typically a line.
-    fn layout_footnote_separator(&mut self, engine: &mut Engine) -> SourceResult<()> {
+    fn layout_footnote_separator(&mut self) -> SourceResult<()> {
         let expand = Axes::new(self.regions.expand.x, false);
         let pod = Regions::one(self.regions.base(), expand);
         let separator = &self.footnote_config.separator;
 
         // FIXME: Shouldn't use `root()` here.
         let mut frame = separator
-            .layout(engine, Locator::root(), self.styles, pod)?
+            .layout(self.engine, Locator::root(), *self.styles, pod)?
             .into_frame();
         frame.size_mut().y += self.footnote_config.clearance;
         frame.translate(Point::with_y(self.footnote_config.clearance));
@@ -824,11 +858,11 @@ impl FlowLayouter<'_> {
     }
 }
 
-/// Finds all footnotes in the frame.
+/// Collect all footnotes in a frame.
-fn find_footnotes(notes: &mut Vec<Packed<FootnoteElem>>, frame: &Frame) {
+fn collect_footnotes(notes: &mut Vec<Packed<FootnoteElem>>, frame: &Frame) {
     for (_, item) in frame.items() {
         match item {
-            FrameItem::Group(group) => find_footnotes(notes, &group.frame),
+            FrameItem::Group(group) => collect_footnotes(notes, &group.frame),
             FrameItem::Tag(tag)
                 if !notes.iter().any(|note| note.location() == tag.elem.location()) =>
             {