update comments and todos

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

@@ -1375,14 +1375,9 @@ impl<'a> GridLayouter<'a> {
         }
 
         // Skip to fitting region, but only if we aren't part of an unbreakable
-        // row group. We use 'may_progress_with_offset' so our 'may_progress'
-        // call properly considers that a header and a footer would be added
-        // on each region break, so we only keep skipping regions until we
-        // reach one with the same height of the 'last' region (which can be
-        // endlessly repeated) when subtracting header and footer height.
-        //
-        // See 'check_for_unbreakable_rows' as for why we're using
-        // 'repeating_header_height' to predict header height.
+        // row group. We use 'may_progress_with_repeats' to stop trying if we
+        // would skip to a region with the same height and where the same
+        // headers would be repeated.
         let height = frame.height();
         while self.unbreakable_rows_left == 0
             && !self.regions.size.y.fits(height)
@@ -1525,6 +1520,9 @@ impl<'a> GridLayouter<'a> {
         engine: &mut Engine,
         last: bool,
     ) -> SourceResult<()> {
+        // The latest rows have orphan prevention (headers) and no other rows
+        // were placed, so remove those rows and try again in a new region,
+        // unless this is the last region.
         if let Some(orphan_snapshot) = self.current.lrows_orphan_snapshot.take() {
             if !last {
                 self.current.lrows.truncate(orphan_snapshot);
@@ -1572,9 +1570,11 @@ impl<'a> GridLayouter<'a> {
             if let Some(Repeatable::Repeated(footer)) = &self.grid.footer {
                 // Don't layout the footer if it would be alone with the header
                 // in the page (hence the widow check), and don't layout it
-                // twice.
-                // TODO: this check can be replaced by a vector of repeating
-                // footers in the future.
+                // twice (check below).
+                //
+                // TODO(subfooters): this check can be replaced by a vector of
+                // repeating footers in the future, and/or some "pending
+                // footers" vector for footers we're about to place.
                 if self.current.lrows.iter().all(|row| row.index() < footer.start) {
                     laid_out_footer_start = Some(footer.start);
                     self.layout_footer(footer, engine, self.finished.len())?;

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

@@ -503,6 +503,10 @@ pub fn hline_stroke_at_column(
     // hlines would appear at the same position, which then are prioritized.
     let top_stroke_comes_from_header = header_end_above.zip(local_top_y).is_some_and(
         |(last_repeated_header_end, local_top_y)| {
+            // Check if the last repeated header row is above this line.
+            //
+            // Note that `y == last_repeated_header_end` is impossible for a
+            // strictly repeated header (not in its original position).
             local_top_y < last_repeated_header_end && y > last_repeated_header_end
         },
     );

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

@@ -42,7 +42,6 @@ impl<'a> GridLayouter<'a> {
             }) && !next_header.short_lived
         }) {
             // More headers coming, so wait until we reach them.
-            // TODO: refactor
             return Ok(());
         }
 
@@ -238,8 +237,9 @@ impl<'a> GridLayouter<'a> {
                 Default::default(),
             );
 
-            // TODO: re-calculate heights of headers and footers on each region
-            // if 'full' changes? (Assuming height doesn't change for now...)
+            // TODO(layout model): re-calculate heights of headers and footers
+            // on each region if 'full' changes? (Assuming height doesn't
+            // change for now...)
             skipped_region = true;
 
             self.regions.size.y -= self.current.footer_height;
@@ -370,8 +370,6 @@ impl<'a> GridLayouter<'a> {
             0,
         )?;
 
-        // TODO: remove this 'unbreakable rows left check',
-        // consider if we can already be in an unbreakable row group?
         while self.unbreakable_rows_left == 0
             && !self.regions.size.y.fits(header_height)
             && self.may_progress_with_repeats()
@@ -482,9 +480,12 @@ impl<'a> GridLayouter<'a> {
             skipped_region = true;
         }
 
-        // TODO: Consider resetting header height etc. if we skip region.
+        // TODO(subfooters): Consider resetting header height etc. if we skip
+        // region. (Maybe move that step to `finish_region_internal`.)
+        //
         // That is unnecessary at the moment as 'prepare_footers' is only
-        // called at the start of the region, but what about when we can have
+        // called at the start of the region, so header height is always zero
+        // and no headers were placed so far, but what about when we can have
         // footers in the middle of the region? Let's think about this then.
         self.current.footer_height = if skipped_region {
             // Simulate the footer again; the region's 'full' might have

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

@@ -241,9 +241,10 @@ impl GridLayouter<'_> {
             if let Some(Repeatable::NotRepeated(footer)) = &self.grid.footer {
                 if current_row >= footer.start {
                     // Non-repeated footer, so keep it unbreakable.
-                    // TODO: This will become unnecessary once non-repeated
-                    // footers are treated differently and have widow
-                    // prevention.
+                    //
+                    // TODO(subfooters): This will become unnecessary
+                    // once non-repeated footers are treated differently and
+                    // have widow prevention.
                     amount_unbreakable_rows = Some(self.grid.rows.len() - footer.start);
                 }
             }