From db59e3ffcbe80f6274d213979e6930d3a9ea8a51 Mon Sep 17 00:00:00 2001 From: PgBiel <9021226+PgBiel@users.noreply.github.com> Date: Sun, 4 May 2025 04:05:28 -0300 Subject: [PATCH] fix pending header repetition with may progress = false --- crates/typst-layout/src/grid/repeated.rs | 45 ++++++++++++++---- ...-header-too-large-non-repeating-orphan.png | Bin 0 -> 372 bytes tests/suite/layout/grid/headers.typ | 10 ++++ 3 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 tests/ref/grid-header-too-large-non-repeating-orphan.png diff --git a/crates/typst-layout/src/grid/repeated.rs b/crates/typst-layout/src/grid/repeated.rs index 3de3795b6..54f5cda7c 100644 --- a/crates/typst-layout/src/grid/repeated.rs +++ b/crates/typst-layout/src/grid/repeated.rs @@ -42,7 +42,9 @@ impl<'a> GridLayouter<'a> { // These headers are short-lived as they are immediately followed by a // header of the same or lower level, such that they never actually get // to repeat. - self.layout_new_headers(consecutive_headers, true, engine) + self.layout_new_headers(consecutive_headers, true, engine)?; + + Ok(()) } else { self.layout_new_pending_headers(consecutive_headers, engine) } @@ -125,13 +127,21 @@ impl<'a> GridLayouter<'a> { // This might be a waste as we could generate an orphan and thus have // to try to place old and new headers all over again, but that happens // for every new region anyway, so it's rather unavoidable. - self.layout_new_headers(headers, false, engine)?; + let snapshot_created = self.layout_new_headers(headers, false, engine)?; // After the first subsequent row is laid out, move to repeating, as // it's then confirmed the headers won't be moved due to orphan // prevention anymore. self.pending_headers = headers; + if !snapshot_created { + // Region probably couldn't progress. + // + // Mark new pending headers as final and ensure there isn't a + // snapshot. + self.flush_orphans(); + } + Ok(()) } @@ -264,8 +274,13 @@ impl<'a> GridLayouter<'a> { debug_assert!(self.current.lrows.is_empty()); debug_assert!(self.current.lrows_orphan_snapshot.is_none()); - if may_progress_with_offset(self.regions, self.current.footer_height) { + let may_progress = + may_progress_with_offset(self.regions, self.current.footer_height); + + if may_progress { // Enable orphan prevention for headers at the top of the region. + // Otherwise, we will flush pending headers below, after laying + // them out. // // 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 @@ -317,6 +332,12 @@ impl<'a> GridLayouter<'a> { } } + if !may_progress { + // Flush pending headers immediately, as placing them again later + // won't help. + self.flush_orphans(); + } + Ok(()) } @@ -325,12 +346,15 @@ impl<'a> GridLayouter<'a> { /// If 'short_lived' is true, these headers are immediately followed by /// a conflicting header, so it is assumed they will not be pushed to /// pending headers. + /// + /// Returns whether orphan prevention was successfully setup, or couldn't + /// due to short-lived headers or the region couldn't progress. pub fn layout_new_headers( &mut self, headers: &'a [Repeatable
], short_lived: bool, engine: &mut Engine, - ) -> SourceResult<()> { + ) -> SourceResult { // At first, only consider the height of the given headers. However, // for upcoming regions, we will have to consider repeating headers as // well. @@ -365,13 +389,18 @@ impl<'a> GridLayouter<'a> { // 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 + let should_snapshot = !short_lived && self.current.lrows_orphan_snapshot.is_none() && may_progress_with_offset( self.regions, self.current.header_height + self.current.footer_height, - ) - { + ); + + if should_snapshot { + // If we don't enter this branch while laying out non-short lived + // headers, that means we will have to immediately flush pending + // headers and mark them as final, since trying to place them in + // the next page won't help get more space. self.current.lrows_orphan_snapshot = Some(self.current.lrows.len()); } @@ -397,7 +426,7 @@ impl<'a> GridLayouter<'a> { } } - Ok(()) + Ok(should_snapshot) } /// Calculates the total expected height of several headers. diff --git a/tests/ref/grid-header-too-large-non-repeating-orphan.png b/tests/ref/grid-header-too-large-non-repeating-orphan.png new file mode 100644 index 0000000000000000000000000000000000000000..a4e7843b0f391bffb50fe9a23aa6dda8593bc742 GIT binary patch literal 372 zcmeAS@N?(olHy`uVBq!ia0vp^6+pa@14uAv+Nr!`U|_WNba4!+xb^nxSudwRk+#JA zRn{E8U+W)cVtcz1$K&aDzdvWqp^BMfln`UJ1W7cPKXMKRzqU8T0L*-1P|cgoFg) ziZ6xH(+}KC%8Y0F{)f?5Wl#NEQ_h^&nzsU+KxyR)p}qN@54MC_OgeBlAt9LQ{!y!0 z9)ITV%NGVo8(aL#@qQ3;$ZFz&$NqsTa{J$wa{hVDsSlO>du#fElM%0CrX1a_%lSv> zWtNg)Vf(ZW76Su=4>pHZJ?w96Y)of-t}FP)UgV%;KUdt>*nZUy^NPhpZA)J`GRa%K oYhzk>@5iJVJ)m8