From 6e6f21f78b9caa3203f068b39f00f9b9ad0cb1d3 Mon Sep 17 00:00:00 2001 From: PgBiel <9021226+PgBiel@users.noreply.github.com> Date: Tue, 20 May 2025 21:57:34 -0300 Subject: [PATCH] ensure short-lived headers also replace predecessors Even if they don't repeat, they should cause previous headers to stop repeating. --- crates/typst-layout/src/grid/repeated.rs | 114 ++++++++---------- ...aders-repeat-short-lived-also-replaces.png | Bin 0 -> 899 bytes tests/suite/layout/grid/subheaders.typ | 53 ++++++++ 3 files changed, 104 insertions(+), 63 deletions(-) create mode 100644 tests/ref/grid-subheaders-repeat-short-lived-also-replaces.png diff --git a/crates/typst-layout/src/grid/repeated.rs b/crates/typst-layout/src/grid/repeated.rs index e65ff5318..fc717b2ff 100644 --- a/crates/typst-layout/src/grid/repeated.rs +++ b/crates/typst-layout/src/grid/repeated.rs @@ -49,6 +49,33 @@ impl<'a> GridLayouter<'a> { self.upcoming_headers = new_upcoming_headers; *consecutive_header_count = 0; + let [first_header, ..] = consecutive_headers else { + self.flush_orphans(); + return Ok(()); + }; + + // Assuming non-conflicting headers sorted by increasing y, this must + // be the header with the lowest level (sorted by increasing levels). + let first_level = first_header.level; + + // Stop repeating conflicting headers, even if the new headers are + // short-lived or won't repeat. + // + // If we go to a new region before the new headers fit alongside their + // children (or in general, for short-lived), the old headers should + // not be displayed anymore. + let first_conflicting_pos = + self.repeating_headers.partition_point(|h| h.level < first_level); + self.repeating_headers.truncate(first_conflicting_pos); + + // Ensure upcoming rows won't see that these headers will occupy any + // space in future regions anymore. + for removed_height in + self.current.repeating_header_heights.drain(first_conflicting_pos..) + { + self.current.repeating_header_height -= removed_height; + } + // Layout short-lived headers immediately. if consecutive_headers.last().is_some_and(|h| h.short_lived) { // No chance of orphans as we're immediately placing conflicting @@ -63,11 +90,32 @@ impl<'a> GridLayouter<'a> { // header of the same or lower level, such that they never actually get // to repeat. self.layout_new_headers(consecutive_headers, true, engine)?; - - Ok(()) } else { - self.layout_new_pending_headers(consecutive_headers, engine) + // Let's try to place pending headers at least once. + // 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. + let snapshot_created = + self.layout_new_headers(consecutive_headers, false, engine)?; + + // Queue the new headers for layout. They will remain in this + // vector due to orphan prevention. + // + // 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 = consecutive_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(()) } /// Lays out rows belonging to a header, returning the calculated header @@ -100,66 +148,6 @@ impl<'a> GridLayouter<'a> { Ok(header_height) } - /// Queues new pending headers for layout. Headers remain pending until - /// they are successfully laid out in some page once. Then, they will be - /// moved to `repeating_headers`, at which point it is safe to stop them - /// from repeating at any time. - fn layout_new_pending_headers( - &mut self, - headers: &'a [Repeatable
], - engine: &mut Engine, - ) -> SourceResult<()> { - let [first_header, ..] = headers else { - return Ok(()); - }; - - // Should be impossible to have two consecutive chunks of pending - // headers since they are always as long as possible, only being - // interrupted by direct conflict between consecutive headers, in which - // case we flush pending headers immediately. - assert!(self.pending_headers.is_empty()); - - // Assuming non-conflicting headers sorted by increasing y, this must - // be the header with the lowest level (sorted by increasing levels). - let first_level = first_header.level; - - // Stop repeating conflicting headers. - // If we go to a new region before the pending headers fit alongside - // their children, the old headers should not be displayed anymore. - let first_conflicting_pos = - self.repeating_headers.partition_point(|h| h.level < first_level); - self.repeating_headers.truncate(first_conflicting_pos); - - // Ensure upcoming rows won't see that these headers will occupy any - // space in future regions anymore. - for removed_height in - self.current.repeating_header_heights.drain(first_conflicting_pos..) - { - self.current.repeating_header_height -= removed_height; - } - - // Let's try to place them at least once. - // 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. - 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(()) - } - /// This function should be called each time an additional row has been /// laid out in a region to indicate that orphan prevention has succeeded. /// diff --git a/tests/ref/grid-subheaders-repeat-short-lived-also-replaces.png b/tests/ref/grid-subheaders-repeat-short-lived-also-replaces.png new file mode 100644 index 0000000000000000000000000000000000000000..788837c78f1278d9d7228804d2883f554cdfef88 GIT binary patch literal 899 zcmV-}1AP36P)FRFXa@=%zRy&&rIF3t548L#QxNC-Cy9x!~MUx@9W{ejX-M~!wfUbFvEWU z3&z{S5cvJLGZ6wWmDbB4u%^ou4S^kbo$3imrFzEYom%ottpy#fIkXVk^(}58=rA4h zr4~+_0ef6t+8S94*kg}#k6%*(fN!QOlJ_S7?n?(9r@mNiGX{<)RSyJCr^X@>oMlJA zgUJ<*QhRgxF|d$jxCDWC?N3l z4$6eU`6ZOrjezG*+)-xoU>+2>tVs)jpQsvl_z`esOF$$9+#cf}gTRz?CJqBDXbl28 zLI(%}k4H>nVBZmv82mJ37z2CVg48YvQ&p)E0b2^`Fd^Vk#l%$z{7DZ=wRZOvl??$W z$w|0{fa`V{A1or^&$4`F3j~g#H`x&Q9NE+e_@!*U?$ibV9_n@5d%ZluFw8K+4F48J zE#6~>8D{vOh69Blm_!i#>M{rpgo9v;0KuDkL2#R@9|Kc0DqJ5yussBpYN^3O7zi%g zj)A8Q`!MiI(}ndM696XHMQ1%h;K=Yr2t2Orj`Aa5o$^OfdO%8#JrM<0MkRGET1)g=2;An&0M#+xrV#{Q zi}slz@Z(&41`!v@xJbrDGA@#Fk&KIETqNTn85ha8NX9V33^U9y!wfUbFvAQp%<%sV Z{{rJ)W=rgkmcRf2002ovPDHLkV1g23iL3ws literal 0 HcmV?d00001 diff --git a/tests/suite/layout/grid/subheaders.typ b/tests/suite/layout/grid/subheaders.typ index 8123fc201..24b88f8ac 100644 --- a/tests/suite/layout/grid/subheaders.typ +++ b/tests/suite/layout/grid/subheaders.typ @@ -389,6 +389,59 @@ ..([z],) * 10, ) +--- grid-subheaders-repeat-short-lived-also-replaces --- +// Short-lived subheaders must still replace their conflicting predecessors. +#set page(height: 8em) +#grid( + // This has to go + grid.header( + [a], + level: 3, + ), + [w], + grid.header( + level: 2, + [b] + ), + grid.header( + level: 2, + [c] + ), + grid.header( + level: 2, + [d] + ), + grid.header( + level: 2, + [e] + ), + grid.header( + level: 2, + [f] + ), + grid.header( + level: 2, + [g] + ), + grid.header( + level: 2, + [h] + ), + grid.header( + level: 2, + [i] + ), + grid.header( + level: 2, + [j] + ), + grid.header( + level: 3, + [k] + ), + ..([z],) * 10, +) + --- grid-subheaders-multi-page-row --- #set page(height: 8em) #grid(