create RowState abstraction

nicer to use than what that was before

store temp values during row layout

crates/typst-layout/src/grid/layouter.rs

@@ -61,11 +61,12 @@ pub struct GridLayouter<'a> {
     /// Sorted by increasing levels.
     pub(super) pending_headers: &'a [Repeatable<Header>],
     pub(super) upcoming_headers: &'a [Repeatable<Header>],
-    /// If this is `Some`, this will receive the currently laid out row's
-    /// height if it is auto or relative. This is used for header height
-    /// calculation.
-    /// TODO: consider refactoring this into something nicer.
-    pub(super) current_row_height: Option<Abs>,
+    /// State of the row being currently laid out.
+    ///
+    /// This is kept as a field to avoid passing down too many parameters from
+    /// `layout_row` into called functions, which would then have to pass them
+    /// down to `push_row`, which reads these values.
+    pub(super) row_state: RowState,
     /// The span of the grid element.
     pub(super) span: Span,
 }
@@ -116,6 +117,9 @@ pub(super) struct Current {
     /// before all header rows are fully laid out. It is usually fine because
     /// header rows themselves are unbreakable, and unbreakable rows do not
     /// need to read this field at all.
+    ///
+    /// This height is not only computed at the beginning of the region. It is
+    /// updated whenever a new header is found.
     pub(super) header_height: Abs,
     /// The height of effectively repeating headers, that is, ignoring
     /// non-repeating pending headers.
@@ -136,6 +140,25 @@ pub(super) struct Current {
     pub(super) footer_height: Abs,
 }
 
+/// Data about the row being laid out right now.
+#[derive(Debug, Default)]
+pub(super) struct RowState {
+    /// If this is `Some`, this will receive the currently laid out row's
+    /// height if it is auto or relative. This is used for header height
+    /// calculation.
+    pub(super) current_row_height: Option<Abs>,
+    /// This is `true` when laying out non-short lived headers and footers.
+    /// That is, headers and footers which are not immediately followed or
+    /// preceded (respectively) by conflicting headers and footers of same or
+    /// lower level, or the end or start of the table (respectively), which
+    /// would cause them to stop repeating.
+    ///
+    /// If this is `false`, the next row to be laid out will remove an active
+    /// orphan snapshot and will flush pending headers, as there is no risk
+    /// that they will be orphans anymore.
+    pub(super) in_active_repeatable: bool,
+}
+
 #[derive(Debug, Default)]
 pub(super) struct FinishedHeaderRowInfo {
     /// The amount of repeated headers at the top of the region.
@@ -207,7 +230,7 @@ impl<'a> GridLayouter<'a> {
             repeating_headers: vec![],
             upcoming_headers: &grid.headers,
             pending_headers: Default::default(),
-            current_row_height: None,
+            row_state: RowState::default(),
             current: Current {
                 initial: regions.size,
                 repeated_header_rows: 0,
@@ -251,7 +274,7 @@ impl<'a> GridLayouter<'a> {
                 if y >= footer.start {
                     if y == footer.start {
                         self.layout_footer(footer, engine, self.finished.len())?;
-                        self.flush_pending_headers();
+                        self.flush_orphans();
                     }
                     y = footer.end;
                     continue;
@@ -263,7 +286,15 @@ impl<'a> GridLayouter<'a> {
             // After the first non-header row is placed, pending headers are no
             // longer orphans and can repeat, so we move them to repeating
             // headers.
-            self.flush_pending_headers();
+            //
+            // Note that this is usually done in `push_row`, since the call to
+            // `layout_row` above might trigger region breaks (for multi-page
+            // auto rows), whereas this needs to be called as soon as any part
+            // of a row is laid out. However, it's possible a row has no
+            // visible output and thus does not push any rows even though it
+            // was successfully laid out, in which case we additionally flush
+            // here just in case.
+            self.flush_orphans();
 
             y += 1;
         }
@@ -288,12 +319,46 @@ impl<'a> GridLayouter<'a> {
         self.render_fills_strokes()
     }
 
-    /// Layout the given row.
+    /// Layout a row with a certain initial state, returning the final state.
+    #[inline]
+    pub(super) fn layout_row_with_state(
+        &mut self,
+        y: usize,
+        engine: &mut Engine,
+        disambiguator: usize,
+        initial_state: RowState,
+    ) -> SourceResult<RowState> {
+        // Keep a copy of the previous value in the stack, as this function can
+        // call itself recursively (e.g. if a region break is triggered and a
+        // header is placed), so we shouldn't outright overwrite it, but rather
+        // save and later restore the state when back to this call.
+        let previous = std::mem::replace(&mut self.row_state, initial_state);
+
+        // Keep it as a separate function to allow inlining the return below,
+        // as it's usually not needed.
+        self.layout_row_internal(y, engine, disambiguator)?;
+
+        Ok(std::mem::replace(&mut self.row_state, previous))
+    }
+
+    /// Layout the given row with the default row state.
+    #[inline]
     pub(super) fn layout_row(
         &mut self,
         y: usize,
         engine: &mut Engine,
         disambiguator: usize,
+    ) -> SourceResult<()> {
+        self.layout_row_with_state(y, engine, disambiguator, RowState::default())?;
+        Ok(())
+    }
+
+    /// Layout the given row using the current state.
+    pub(super) fn layout_row_internal(
+        &mut self,
+        y: usize,
+        engine: &mut Engine,
+        disambiguator: usize,
     ) -> SourceResult<()> {
         // Skip to next region if current one is full, but only for content
         // rows, not for gutter rows, and only if we aren't laying out an
@@ -317,7 +382,9 @@ impl<'a> GridLayouter<'a> {
                     self.layout_relative_row(engine, disambiguator, v, y)?
                 }
                 Sizing::Fr(v) => {
-                    self.current.lrows_orphan_snapshot = None;
+                    if !self.row_state.in_active_repeatable {
+                        self.flush_orphans();
+                    }
                     self.lrows.push(Row::Fr(v, y, disambiguator))
                 }
             }
@@ -1043,7 +1110,7 @@ impl<'a> GridLayouter<'a> {
             let frame = self.layout_single_row(engine, disambiguator, first, y)?;
             self.push_row(frame, y, true);
 
-            if let Some(row_height) = &mut self.current_row_height {
+            if let Some(row_height) = &mut self.row_state.current_row_height {
                 // Add to header height, as we are in a header row.
                 *row_height += first;
             }
@@ -1278,7 +1345,7 @@ impl<'a> GridLayouter<'a> {
         let resolved = v.resolve(self.styles).relative_to(self.regions.base().y);
         let frame = self.layout_single_row(engine, disambiguator, resolved, y)?;
 
-        if let Some(row_height) = &mut self.current_row_height {
+        if let Some(row_height) = &mut self.row_state.current_row_height {
             // Add to header height, as we are in a header row.
             *row_height += resolved;
         }
@@ -1430,9 +1497,11 @@ impl<'a> GridLayouter<'a> {
     /// will be pushed for this particular row. It can be `false` for rows
     /// spanning multiple regions.
     fn push_row(&mut self, frame: Frame, y: usize, is_last: bool) {
-        // There is now a row after the rows equipped with orphan prevention,
-        // so no need to remove them anymore.
-        self.current.lrows_orphan_snapshot = None;
+        if !self.row_state.in_active_repeatable {
+            // There is now a row after the rows equipped with orphan
+            // prevention, so no need to keep moving them anymore.
+            self.flush_orphans();
+        }
         self.regions.size.y -= frame.height();
         self.lrows.push(Row::Frame(frame, y, is_last));
     }

crates/typst-layout/src/grid/repeated.rs

@@ -3,7 +3,7 @@ use typst_library::engine::Engine;
 use typst_library::layout::grid::resolve::{Footer, Header, Repeatable};
 use typst_library::layout::{Abs, Axes, Frame, Regions};
 
-use super::layouter::{may_progress_with_offset, GridLayouter};
+use super::layouter::{may_progress_with_offset, GridLayouter, RowState};
 use super::rowspans::UnbreakableRowGroup;
 
 impl<'a> GridLayouter<'a> {
@@ -35,7 +35,7 @@ impl<'a> GridLayouter<'a> {
             // headers afterwards, which basically are not headers, for all intents
             // and purposes. It is therefore guaranteed that all new headers have
             // been placed at least once.
-            self.flush_pending_headers();
+            self.flush_orphans();
 
             // Layout each conflicting header independently, without orphan
             // prevention (as they don't go into 'pending_headers').
@@ -48,36 +48,32 @@ impl<'a> GridLayouter<'a> {
         }
     }
 
-    /// Lays out a row while indicating that it should store its persistent
-    /// height as a header row, which will be its height if relative or auto,
-    /// or zero otherwise (fractional).
-    #[inline]
-    fn layout_header_row(
-        &mut self,
-        y: usize,
-        engine: &mut Engine,
-        disambiguator: usize,
-    ) -> SourceResult<Option<Abs>> {
-        let previous_row_height =
-            std::mem::replace(&mut self.current_row_height, Some(Abs::zero()));
-        self.layout_row(y, engine, disambiguator)?;
-
-        Ok(std::mem::replace(&mut self.current_row_height, previous_row_height))
-    }
-
     /// Lays out rows belonging to a header, returning the calculated header
-    /// height only for that header.
+    /// height only for that header. Indicates to the laid out rows that they
+    /// should inform their laid out heights if appropriate (auto or fixed
+    /// size rows only).
     #[inline]
     fn layout_header_rows(
         &mut self,
         header: &Header,
         engine: &mut Engine,
         disambiguator: usize,
+        as_short_lived: bool,
     ) -> SourceResult<Abs> {
         let mut header_height = Abs::zero();
         for y in header.range() {
-            header_height +=
-                self.layout_header_row(y, engine, disambiguator)?.unwrap_or_default();
+            header_height += self
+                .layout_row_with_state(
+                    y,
+                    engine,
+                    disambiguator,
+                    RowState {
+                        current_row_height: Some(Abs::zero()),
+                        in_active_repeatable: !as_short_lived,
+                    },
+                )?
+                .current_row_height
+                .unwrap_or_default();
         }
         Ok(header_height)
     }
@@ -139,7 +135,26 @@ impl<'a> GridLayouter<'a> {
         Ok(())
     }
 
+    /// This function should be called each time an additional row has been
+    /// laid out in a region to indicate that orphan prevention has succeeded.
+    ///
+    /// It removes the current orphan snapshot and flushes pending headers,
+    /// such that a non-repeating header won't try to be laid out again
+    /// anymore, and a repeating header will begin to be part of
+    /// `repeating_headers`.
+    pub fn flush_orphans(&mut self) {
+        self.current.lrows_orphan_snapshot = None;
+        self.flush_pending_headers();
+    }
+
+    /// Indicates all currently pending headers have been successfully placed
+    /// once, since another row has been placed after them, so they are
+    /// certainly not orphans.
     pub fn flush_pending_headers(&mut self) {
+        if self.pending_headers.is_empty() {
+            return;
+        }
+
         for header in self.pending_headers {
             if let Repeatable::Repeated(header) = header {
                 // Vector remains sorted by increasing levels:
@@ -251,7 +266,8 @@ impl<'a> GridLayouter<'a> {
         // 'layout_row' so this is fine.
         let mut i = 0;
         while let Some(&header) = self.repeating_headers.get(i) {
-            let header_height = self.layout_header_rows(header, engine, disambiguator)?;
+            let header_height =
+                self.layout_header_rows(header, engine, disambiguator, false)?;
             self.current.header_height += header_height;
             self.current.repeating_header_height += header_height;
 
@@ -288,7 +304,7 @@ impl<'a> GridLayouter<'a> {
 
         for header in self.pending_headers {
             let header_height =
-                self.layout_header_rows(header.unwrap(), engine, disambiguator)?;
+                self.layout_header_rows(header.unwrap(), engine, disambiguator, false)?;
             self.current.header_height += header_height;
             if matches!(header, Repeatable::Repeated(_)) {
                 self.current.repeating_header_height += header_height;
@@ -346,7 +362,8 @@ impl<'a> GridLayouter<'a> {
 
         let initial_row_count = self.lrows.len();
         for header in headers {
-            let header_height = self.layout_header_rows(header.unwrap(), engine, 0)?;
+            let header_height =
+                self.layout_header_rows(header.unwrap(), engine, 0, false)?;
 
             // Only store this header height if it is actually going to
             // become a pending header. Otherwise, pretend it's not a
@@ -464,10 +481,24 @@ impl<'a> GridLayouter<'a> {
         // anyway, so this is mostly for correctness.
         self.regions.size.y += self.current.footer_height;
 
+        let repeats = self
+            .grid
+            .footer
+            .as_ref()
+            .is_some_and(|f| matches!(f, Repeatable::Repeated(_)));
         let footer_len = self.grid.rows.len() - footer.start;
         self.unbreakable_rows_left += footer_len;
+
         for y in footer.start..self.grid.rows.len() {
-            self.layout_row(y, engine, disambiguator)?;
+            self.layout_row_with_state(
+                y,
+                engine,
+                disambiguator,
+                RowState {
+                    in_active_repeatable: repeats,
+                    ..Default::default()
+                },
+            )?;
         }
 
         Ok(())

tests/suite/layout/grid/subheaders.typ

@@ -684,6 +684,17 @@
   grid.cell(x: 0)[end],
 )
 
+--- grid-subheaders-non-repeating-header-before-multi-page-row ---
+#set page(height: 6em)
+#grid(
+  grid.header(
+    repeat: false,
+    [h]
+  ),
+  [row #colbreak() row]
+)
+
+
 --- grid-subheaders-short-lived-no-orphan-prevention ---
 // No orphan prevention for short-lived headers.
 #set page(height: 8em)