improve variable names related to repeating headers

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

@@ -32,7 +32,7 @@ pub struct GridLayouter<'a> {
     pub(super) rcols: Vec<Abs>,
     /// The sum of `rcols`.
     pub(super) width: Abs,
-    /// Resolve row sizes, by region.
+    /// Resolved row sizes, by region.
     pub(super) rrows: Vec<Vec<RowPiece>>,
     /// Rows in the current region.
     pub(super) lrows: Vec<Row>,
@@ -51,15 +51,16 @@ pub struct GridLayouter<'a> {
     pub(super) finished_header_rows: Vec<FinishedHeaderRowInfo>,
     /// Whether this is an RTL grid.
     pub(super) is_rtl: bool,
-    /// Currently repeating headers, one per level.
+    /// Currently repeating headers, one per level. Sorted by increasing
-    /// Sorted by increasing levels.
+    /// levels.
     ///
     /// Note that some levels may be absent, in particular level 0, which does
-    /// not exist (so the first level is >= 1).
+    /// not exist (so all levels are >= 1).
     pub(super) repeating_headers: Vec<&'a Header>,
     /// Headers, repeating or not, awaiting their first successful layout.
     /// Sorted by increasing levels.
     pub(super) pending_headers: &'a [Repeatable<Header>],
+    /// Next headers to be processed.
     pub(super) upcoming_headers: &'a [Repeatable<Header>],
     /// State of the row being currently laid out.
     ///
@@ -84,42 +85,47 @@ pub(super) struct Current {
     /// find a new header inside the region (not at the top), so this field
     /// is required to access information from the top of the region.
     ///
-    /// This is used for orphan prevention checks (if there are no rows other
+    /// This information is used on finish region to calculate the total height
-    /// than repeated header rows upon finishing a region, we'd have orphans).
+    /// of resolved header rows at the top of the region, which is used by
-    /// Note that non-repeated and pending repeated header rows are not included
+    /// multi-page rowspans so they can properly skip the header rows at the
-    /// in this number as they use a separate mechanism for orphan prevention
+    /// top of each region during layout.
-    /// (`lrows_orphan_shapshot` field).
-    ///
-    /// In addition, this information is used on finish region to calculate the
-    /// total height of resolved header rows at the top of the region, which is
-    /// used by multi-page rowspans so they can properly skip the header rows
-    /// at the top of the region during layout.
     pub(super) repeated_header_rows: usize,
-    /// The end bound of the last repeating header at the start of the region.
+    /// The end bound of the row range of the last repeating header at the
-    /// The last row might have disappeared due to being empty, so this is how
+    /// start of the region.
-    /// we can become aware of that. Line layout uses this to determine when to
-    /// prioritize the last lines under a header.
     ///
-    /// A value of zero indicates no headers were placed.
+    /// The last row might have disappeared from layout due to being empty, so
+    /// this is how we can become aware of where the last header ends without
+    /// having to check the vector of rows. Line layout uses this to determine
+    /// when to prioritize the last lines under a header.
+    ///
+    /// A value of zero indicates no repeated headers were placed.
     pub(super) last_repeated_header_end: usize,
-    /// Stores the length of `lrows` before a sequence of trailing rows
+    /// Stores the length of `lrows` before a sequence of rows equipped with
-    /// equipped with orphan prevention were laid out. In this case, if no more
+    /// orphan prevention was laid out. In this case, if no more rows without
-    /// rows are laid out after those rows before the region ends, the rows
+    /// orphan prevention are laid out after those rows before the region ends,
-    /// will be removed. For new headers in particular, which use this, those
+    /// the rows will be removed, and there may be an attempt to place them
-    /// headers will have been moved to the `pending_headers` vector and so
+    /// again in the new region. Effectively, this is the mechanism used for
-    /// will automatically be placed again until they fit.
+    /// orphan prevention of rows.
+    ///
+    /// At the moment, this is only used by repeated headers (they aren't laid
+    /// out if alone in the region) and by new headers, which are moved to the
+    /// `pending_headers` vector and so will automatically be placed again
+    /// until they fit and are not orphans in at least one region (or exactly
+    /// one, for non-repeated headers).
     pub(super) lrows_orphan_snapshot: Option<usize>,
     /// The total simulated height for all headers currently in
     /// `repeating_headers` and `pending_headers`.
     ///
-    /// This field is reset in `layout_header` and properly updated by
+    /// This field is reset on each new region and properly updated by
     /// `layout_auto_row` and `layout_relative_row`, and should not be read
     /// 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.
+    /// updated whenever a new header is found, subtracting the height of
+    /// headers which stopped repeating and adding the height of all new
+    /// headers.
     pub(super) header_height: Abs,
     /// The height of effectively repeating headers, that is, ignoring
     /// non-repeating pending headers.
@@ -130,9 +136,14 @@ pub(super) struct Current {
     /// but disappear on new regions, so they can be ignored.
     pub(super) repeating_header_height: Abs,
     /// The height for each repeating header that was placed in this region.
-    /// Note that this includes headers not at the top of the region (pending
+    /// Note that this includes headers not at the top of the region, before
-    /// headers), and excludes headers removed by virtue of a new, conflicting
+    /// their first repetition (pending headers), and excludes headers removed
-    /// header being found.
+    /// by virtue of a new, conflicting header being found (short-lived
+    /// headers).
+    ///
+    /// This is used to know how much to update `repeating_header_height` by
+    /// when finding a new header and causing existing repeating headers to
+    /// stop.
     pub(super) repeating_header_heights: Vec<Abs>,
     /// The simulated footer height for this region.
     ///
@@ -143,7 +154,7 @@ pub(super) struct Current {
 /// 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
+    /// If this is `Some`, this will be updated by 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>,
@@ -151,7 +162,7 @@ pub(super) struct RowState {
     /// 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.
+    /// would cause them to never repeat, even once.
     ///
     /// 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
@@ -159,13 +170,16 @@ pub(super) struct RowState {
     pub(super) in_active_repeatable: bool,
 }
 
+/// Data about laid out repeated header rows for a specific finished region.
 #[derive(Debug, Default)]
 pub(super) struct FinishedHeaderRowInfo {
     /// The amount of repeated headers at the top of the region.
-    pub(super) repeated: usize,
+    pub(super) repeated_amount: usize,
-    /// The end bound of the last repeated header at the top of the region.
+    /// The end bound of the row range of the last repeated header at the top
+    /// of the region.
     pub(super) last_repeated_header_end: usize,
-    pub(super) height: Abs,
+    /// The total height of repeated headers at the top of the region.
+    pub(super) repeated_height: Abs,
 }
 
 /// Details about a resulting row piece.
@@ -563,9 +577,11 @@ impl<'a> GridLayouter<'a> {
                     })
                     .unwrap_or(LinePosition::Before);
 
-                // Header's lines have priority when repeated.
+                // Header's lines at the bottom have priority when repeated.
-                let end_under_repeated_header = finished_header_rows
+                // This will store the end bound of the last header if the
-                    .filter(|info| prev_y.is_some() && i == info.repeated)
+                // current iteration is calculating lines under it.
+                let last_repeated_header_end_above = finished_header_rows
+                    .filter(|info| prev_y.is_some() && i == info.repeated_amount)
                     .map(|info| info.last_repeated_header_end);
 
                 // If some grid rows were omitted between the previous resolved
@@ -573,14 +589,15 @@ impl<'a> GridLayouter<'a> {
                 // row don't "disappear" and are considered, albeit with less
                 // priority. However, don't do this when we're below a header,
                 // as it must have more priority instead of less, so it is
-                // chained later instead of before. The exception is when the
+                // chained later instead of before (stored in the
+                // 'header_hlines' variable below). The exception is when the
                 // last row in the header is removed, in which case we append
                 // both the lines under the row above us and also (later) the
                 // lines under the header's (removed) last row.
                 let prev_lines = prev_y
                     .filter(|prev_y| {
                         prev_y + 1 != y
-                            && end_under_repeated_header.is_none_or(
+                            && last_repeated_header_end_above.is_none_or(
                                 |last_repeated_header_end| {
                                     prev_y + 1 != last_repeated_header_end
                                 },
@@ -605,8 +622,8 @@ impl<'a> GridLayouter<'a> {
                 };
 
                 let mut expected_header_line_position = LinePosition::Before;
-                let header_hlines = if let Some((under_header_end, prev_y)) =
+                let header_hlines = if let Some((header_end_above, prev_y)) =
-                    end_under_repeated_header.zip(prev_y)
+                    last_repeated_header_end_above.zip(prev_y)
                 {
                     if !self.grid.has_gutter
                         || matches!(
@@ -631,10 +648,10 @@ impl<'a> GridLayouter<'a> {
                         // column-gutter is specified, for example. In that
                         // case, we still repeat the line under the gutter.
                         expected_header_line_position = expected_line_position(
-                            under_header_end,
+                            header_end_above,
-                            under_header_end == self.grid.rows.len(),
+                            header_end_above == self.grid.rows.len(),
                         );
-                        get_hlines_at(under_header_end)
+                        get_hlines_at(header_end_above)
                     } else {
                         &[]
                     }
@@ -692,7 +709,7 @@ impl<'a> GridLayouter<'a> {
                             grid,
                             rows,
                             local_top_y,
-                            end_under_repeated_header,
+                            last_repeated_header_end_above,
                             in_last_region,
                             y,
                             x,
@@ -1517,6 +1534,11 @@ impl<'a> GridLayouter<'a> {
                 self.lrows.truncate(orphan_snapshot);
                 self.current.repeated_header_rows =
                     self.current.repeated_header_rows.min(orphan_snapshot);
+
+                if orphan_snapshot == 0 {
+                    // Removed all repeated headers.
+                    self.current.last_repeated_header_end = 0;
+                }
             }
         }
 
@@ -1531,55 +1553,28 @@ impl<'a> GridLayouter<'a> {
                 self.current.repeated_header_rows.min(self.lrows.len());
         }
 
+        // If no rows other than the footer have been laid out so far
+        // (e.g. due to header orphan prevention), and there are rows
+        // beside the footer, then don't lay it out at all.
+        //
+        // It is worth noting that the footer is made non-repeatable at
+        // the grid resolving stage if it is short-lived, that is, if
+        // it is at the start of the table (or right after headers at
+        // the start of the table).
+        // TODO(subfooters): explicitly check for short-lived footers.
+        // TODO(subfooters): widow prevention for non-repeated footers with a
+        // similar mechanism / when implementing multiple footers.
         let footer_would_be_widow =
-            if !self.lrows.is_empty() && self.current.repeated_header_rows > 0 {
+            matches!(self.grid.footer, Some(Repeatable::Repeated(_)))
-                // If headers are repeating, then we already know they are not
+                && self.lrows.is_empty()
-                // short-lived as that is checked, so they have orphan prevention.
+                && may_progress_with_offset(
-                if self.lrows.len() == self.current.repeated_header_rows
+                    self.regions,
-                    && may_progress_with_offset(
+                    // This header height isn't doing much as we just
-                        self.regions,
+                    // confirmed that there are no headers in this region,
-                        // Since we're trying to find a region where to place all
+                    // but let's keep it here for correctness. It will add
-                        // repeating + pending headers, it makes sense to use
+                    // zero anyway.
-                        // 'header_height' and include even non-repeating pending
+                    self.current.header_height + self.current.footer_height,
-                        // headers for this check.
+                );
-                        self.current.header_height + self.current.footer_height,
-                    )
-                {
-                    // Header and footer would be alone in this region, but
-                    // there are more rows beyond the headers and the footer.
-                    // Push an empty region.
-                    self.lrows.clear();
-                    self.current.last_repeated_header_end = 0;
-                    self.current.repeated_header_rows = 0;
-                    true
-                } else {
-                    false
-                }
-            } else if let Some(Repeatable::Repeated(_)) = &self.grid.footer {
-                // If no rows other than the footer have been laid out so far,
-                // and there are rows beside the footer, then don't lay it out
-                // at all. (Similar check from above, but for the case without
-                // headers.)
-                //
-                // It is worth noting that the footer is made non-repeatable at
-                // the grid resolving stage if it is short-lived, that is, if
-                // it is at the start of the table (or right after headers at
-                // the start of the table).
-                // TODO(subfooters): explicitly check for short-lived footers.
-                // TODO(subfooters): widow prevention for non-repeated footers with a
-                // similar mechanism / when implementing multiple footers.
-                self.lrows.is_empty()
-                    && may_progress_with_offset(
-                        self.regions,
-                        // This header height isn't doing much as we just
-                        // confirmed that there are no headers in this region,
-                        // but let's keep it here for correctness. It will add
-                        // zero anyway.
-                        self.current.header_height + self.current.footer_height,
-                    )
-            } else {
-                false
-            };
 
         let mut laid_out_footer_start = None;
         if !footer_would_be_widow {
@@ -1618,7 +1613,7 @@ impl<'a> GridLayouter<'a> {
         let mut pos = Point::zero();
         let mut rrows = vec![];
         let current_region = self.finished.len();
-        let mut header_row_height = Abs::zero();
+        let mut repeated_header_row_height = Abs::zero();
 
         // Place finished rows and layout fractional rows.
         for (i, row) in std::mem::take(&mut self.lrows).into_iter().enumerate() {
@@ -1633,7 +1628,7 @@ impl<'a> GridLayouter<'a> {
 
             let height = frame.height();
             if i < self.current.repeated_header_rows {
-                header_row_height += height;
+                repeated_header_row_height += height;
             }
 
             // Ensure rowspans which span this row will have enough space to
@@ -1715,7 +1710,7 @@ impl<'a> GridLayouter<'a> {
                     let rowspan = self.rowspans.remove(i);
                     self.layout_rowspan(
                         rowspan,
-                        Some((&mut output, header_row_height)),
+                        Some((&mut output, repeated_header_row_height)),
                         engine,
                     )?;
                 } else {
@@ -1732,9 +1727,9 @@ impl<'a> GridLayouter<'a> {
             output,
             rrows,
             FinishedHeaderRowInfo {
-                repeated: self.current.repeated_header_rows,
+                repeated_amount: self.current.repeated_header_rows,
                 last_repeated_header_end: self.current.last_repeated_header_end,
-                height: header_row_height,
+                repeated_height: repeated_header_row_height,
             },
         );
 

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

@@ -396,7 +396,7 @@ pub fn hline_stroke_at_column(
     grid: &CellGrid,
     rows: &[RowPiece],
     local_top_y: Option<usize>,
-    end_under_repeated_header: Option<usize>,
+    header_end_above: Option<usize>,
     in_last_region: bool,
     y: usize,
     x: usize,
@@ -501,12 +501,11 @@ pub fn hline_stroke_at_column(
     // Top border stroke and header stroke are generally prioritized, unless
     // they don't have explicit hline overrides and one or more user-provided
     // hlines would appear at the same position, which then are prioritized.
-    let top_stroke_comes_from_header = end_under_repeated_header
+    let top_stroke_comes_from_header = header_end_above.zip(local_top_y).is_some_and(
-        .zip(local_top_y)
+        |(last_repeated_header_end, local_top_y)| {
-        .is_some_and(|(last_repeated_header_end, local_top_y)| {
-            // Ensure the row above us is a repeated header.
             local_top_y < last_repeated_header_end && y > last_repeated_header_end
-        });
+        },
+    );
 
     // Prioritize the footer's top stroke as well where applicable.
     let bottom_stroke_comes_from_footer = grid

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

@@ -262,6 +262,18 @@ impl<'a> GridLayouter<'a> {
         self.current.repeating_header_height = Abs::zero();
         self.current.repeating_header_heights.clear();
 
+        debug_assert!(self.lrows.is_empty());
+        debug_assert!(self.current.lrows_orphan_snapshot.is_none());
+        if may_progress_with_offset(self.regions, self.current.footer_height) {
+            // Enable orphan prevention for headers at the top of the region.
+            //
+            // It is very rare for this to make a difference as we're usually
+            // at the 'last' region after the first skip, at which the snapshot
+            // is handled by 'layout_new_headers'. Either way, we keep this
+            // here for correctness.
+            self.current.lrows_orphan_snapshot = Some(self.lrows.len());
+        }
+
         // Use indices to avoid double borrow. We don't mutate headers in
         // 'layout_row' so this is fine.
         let mut i = 0;
@@ -295,13 +307,6 @@ impl<'a> GridLayouter<'a> {
         }
 
         self.current.repeated_header_rows = self.lrows.len();
-
-        if !self.pending_headers.is_empty() {
-            // Restore snapshot: if pending headers placed again turn out to be
-            // orphans, remove their rows again.
-            self.current.lrows_orphan_snapshot = Some(self.lrows.len());
-        }
-
         for header in self.pending_headers {
             let header_height =
                 self.layout_header_rows(header.unwrap(), engine, disambiguator, false)?;
@@ -357,10 +362,22 @@ impl<'a> GridLayouter<'a> {
             self.finish_region(engine, false)?;
         }
 
+        // Remove new headers at the end of the region if the upcoming row
+        // doesn't fit.
+        // TODO(subfooters): what if there is a footer right after it?
+        if !short_lived
+            && self.current.lrows_orphan_snapshot.is_none()
+            && may_progress_with_offset(
+                self.regions,
+                self.current.header_height + self.current.footer_height,
+            )
+        {
+            self.current.lrows_orphan_snapshot = Some(self.lrows.len());
+        }
+
         self.unbreakable_rows_left +=
             total_header_row_count(headers.iter().map(Repeatable::unwrap));
 
-        let initial_row_count = self.lrows.len();
         for header in headers {
             let header_height =
                 self.layout_header_rows(header.unwrap(), engine, 0, false)?;
@@ -380,12 +397,6 @@ impl<'a> GridLayouter<'a> {
             }
         }
 
-        // Remove new headers at the end of the region if upcoming child doesn't fit.
-        // TODO: Short lived if footer comes afterwards
-        if !short_lived {
-            self.current.lrows_orphan_snapshot = Some(initial_row_count);
-        }
-
         Ok(())
     }
 

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

@@ -158,7 +158,7 @@ impl GridLayouter<'_> {
         let finished_header_rows = self
             .finished_header_rows
             .iter()
-            .map(|info| info.height)
+            .map(|info| info.repeated_height)
             .chain(current_header_row_height)
             .chain(std::iter::repeat(Abs::zero()));