switch to only snapshotting for orphan prevention

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

@@ -104,10 +104,14 @@ pub(super) struct Current {
     pub(super) last_repeated_header_end: usize,
     /// Stores the length of `lrows` before a sequence of trailing rows
     /// equipped with orphan prevention were laid out. In this case, if no more
-    /// rows are laid out after those rows before the region ends, the rows
+    /// rows without orphan prevention are laid out after those rows before the
-    /// will be removed. For new headers in particular, which use this, those
+    /// region ends, the rows will be removed.
-    /// headers will have been moved to the `pending_headers` vector and so
+    ///
-    /// will automatically be placed again until they fit.
+    /// 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`.
@@ -1517,6 +1521,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,35 +1540,9 @@ impl<'a> GridLayouter<'a> {
                 self.current.repeated_header_rows.min(self.lrows.len());
         }
 
-        let footer_would_be_widow =
+        // If no rows other than the footer have been laid out so far
-            if !self.lrows.is_empty() && self.current.repeated_header_rows > 0 {
+        // (e.g. due to header orphan prevention), and there are rows
-                // If headers are repeating, then we already know they are not
+        // beside the footer, then don't lay it out at all.
-                // short-lived as that is checked, so they have orphan prevention.
-                if self.lrows.len() == self.current.repeated_header_rows
-                    && may_progress_with_offset(
-                        self.regions,
-                        // Since we're trying to find a region where to place all
-                        // repeating + pending headers, it makes sense to use
-                        // 'header_height' and include even non-repeating pending
-                        // 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
@@ -1568,7 +1551,9 @@ impl<'a> GridLayouter<'a> {
         // 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()
+        let footer_would_be_widow =
+            matches!(self.grid.footer, Some(Repeatable::Repeated(_)))
+                && self.lrows.is_empty()
                 && may_progress_with_offset(
                     self.regions,
                     // This header height isn't doing much as we just
@@ -1576,10 +1561,7 @@ impl<'a> GridLayouter<'a> {
                     // 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 {

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(())
     }