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

@@ -61,12 +61,11 @@ pub struct GridLayouter<'a> {
     /// Sorted by increasing levels.
     pub(super) pending_headers: &'a [Repeatable<Header>],
     pub(super) upcoming_headers: &'a [Repeatable<Header>],
-    /// State of the row being currently laid out.
+    /// 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
-    /// This is kept as a field to avoid passing down too many parameters from
+    /// calculation.
-    /// `layout_row` into called functions, which would then have to pass them
+    /// TODO: consider refactoring this into something nicer.
-    /// down to `push_row`, which reads these values.
+    pub(super) current_row_height: Option<Abs>,
-    pub(super) row_state: RowState,
     /// The span of the grid element.
     pub(super) span: Span,
 }
@@ -117,9 +116,6 @@ 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.
@@ -140,25 +136,6 @@ 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.
@@ -230,7 +207,7 @@ impl<'a> GridLayouter<'a> {
             repeating_headers: vec![],
             upcoming_headers: &grid.headers,
             pending_headers: Default::default(),
-            row_state: RowState::default(),
+            current_row_height: None,
             current: Current {
                 initial: regions.size,
                 repeated_header_rows: 0,
@@ -274,7 +251,7 @@ impl<'a> GridLayouter<'a> {
                 if y >= footer.start {
                     if y == footer.start {
                         self.layout_footer(footer, engine, self.finished.len())?;
-                        self.flush_orphans();
+                        self.flush_pending_headers();
                     }
                     y = footer.end;
                     continue;
@@ -286,15 +263,7 @@ 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;
         }
@@ -319,46 +288,12 @@ impl<'a> GridLayouter<'a> {
         self.render_fills_strokes()
     }
 
-    /// Layout a row with a certain initial state, returning the final state.
+    /// Layout the given row.
-    #[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
@@ -382,9 +317,7 @@ impl<'a> GridLayouter<'a> {
                     self.layout_relative_row(engine, disambiguator, v, y)?
                 }
                 Sizing::Fr(v) => {
-                    if !self.row_state.in_active_repeatable {
+                    self.current.lrows_orphan_snapshot = None;
-                        self.flush_orphans();
-                    }
                     self.lrows.push(Row::Fr(v, y, disambiguator))
                 }
             }
@@ -1110,7 +1043,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.row_state.current_row_height {
+            if let Some(row_height) = &mut self.current_row_height {
                 // Add to header height, as we are in a header row.
                 *row_height += first;
             }
@@ -1345,7 +1278,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.row_state.current_row_height {
+        if let Some(row_height) = &mut self.current_row_height {
             // Add to header height, as we are in a header row.
             *row_height += resolved;
         }
@@ -1497,11 +1430,9 @@ 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) {
-        if !self.row_state.in_active_repeatable {
+        // There is now a row after the rows equipped with orphan prevention,
-            // There is now a row after the rows equipped with orphan
+        // so no need to remove them anymore.
-            // prevention, so no need to keep moving them anymore.
+        self.current.lrows_orphan_snapshot = None;
-            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, RowState};
+use super::layouter::{may_progress_with_offset, GridLayouter};
 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_orphans();
+            self.flush_pending_headers();
 
             // Layout each conflicting header independently, without orphan
             // prevention (as they don't go into 'pending_headers').
@@ -48,32 +48,36 @@ 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. Indicates to the laid out rows that they
+    /// height only for that header.
-    /// 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
+            header_height +=
-                .layout_row_with_state(
+                self.layout_header_row(y, engine, disambiguator)?.unwrap_or_default();
-                    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)
     }
@@ -135,26 +139,7 @@ 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:
@@ -266,8 +251,7 @@ 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 =
+            let header_height = self.layout_header_rows(header, engine, disambiguator)?;
-                self.layout_header_rows(header, engine, disambiguator, false)?;
             self.current.header_height += header_height;
             self.current.repeating_header_height += header_height;
 
@@ -304,7 +288,7 @@ impl<'a> GridLayouter<'a> {
 
         for header in self.pending_headers {
             let header_height =
-                self.layout_header_rows(header.unwrap(), engine, disambiguator, false)?;
+                self.layout_header_rows(header.unwrap(), engine, disambiguator)?;
             self.current.header_height += header_height;
             if matches!(header, Repeatable::Repeated(_)) {
                 self.current.repeating_header_height += header_height;
@@ -362,8 +346,7 @@ impl<'a> GridLayouter<'a> {
 
         let initial_row_count = self.lrows.len();
         for header in headers {
-            let header_height =
+            let header_height = self.layout_header_rows(header.unwrap(), engine, 0)?;
-                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
@@ -481,24 +464,10 @@ 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_with_state(
+            self.layout_row(y, engine, disambiguator)?;
-                y,
-                engine,
-                disambiguator,
-                RowState {
-                    in_active_repeatable: repeats,
-                    ..Default::default()
-                },
-            )?;
         }
 
         Ok(())

tests/suite/layout/grid/subheaders.typ

@@ -684,17 +684,6 @@
   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)