diff --git a/crates/typst-layout/src/grid/layouter.rs b/crates/typst-layout/src/grid/layouter.rs index 316c4aecb..9db5657a4 100644 --- a/crates/typst-layout/src/grid/layouter.rs +++ b/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())?; diff --git a/crates/typst-layout/src/grid/lines.rs b/crates/typst-layout/src/grid/lines.rs index b377a2112..d5da7e263 100644 --- a/crates/typst-layout/src/grid/lines.rs +++ b/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 }, ); diff --git a/crates/typst-layout/src/grid/repeated.rs b/crates/typst-layout/src/grid/repeated.rs index 907665ea4..df206ea9c 100644 --- a/crates/typst-layout/src/grid/repeated.rs +++ b/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 diff --git a/crates/typst-layout/src/grid/rowspans.rs b/crates/typst-layout/src/grid/rowspans.rs index 18ca934e7..3f5f902b3 100644 --- a/crates/typst-layout/src/grid/rowspans.rs +++ b/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); } }