From 60f246ece2a088a802759975f68745f29bef8deb Mon Sep 17 00:00:00 2001 From: PgBiel <9021226+PgBiel@users.noreply.github.com> Date: Tue, 17 Dec 2024 06:41:18 -0300 Subject: [PATCH] Fix sticky blocks at the top of blocks and pages (#5581) Co-authored-by: Laurenz --- crates/typst-layout/src/flow/distribute.rs | 49 +++++++++++++++--- ...ssue-5296-block-sticky-in-block-at-top.png | Bin 0 -> 277 bytes ...6-block-sticky-spaced-from-top-of-page.png | Bin 0 -> 277 bytes ...-sticky-weakly-spaced-from-top-of-page.png | Bin 0 -> 236 bytes tests/suite/layout/container.typ | 25 +++++++++ 5 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 tests/ref/issue-5296-block-sticky-in-block-at-top.png create mode 100644 tests/ref/issue-5296-block-sticky-spaced-from-top-of-page.png create mode 100644 tests/ref/issue-5296-block-sticky-weakly-spaced-from-top-of-page.png diff --git a/crates/typst-layout/src/flow/distribute.rs b/crates/typst-layout/src/flow/distribute.rs index 7a1cf4264..f504d22e7 100644 --- a/crates/typst-layout/src/flow/distribute.rs +++ b/crates/typst-layout/src/flow/distribute.rs @@ -17,7 +17,7 @@ pub fn distribute(composer: &mut Composer, regions: Regions) -> FlowResult { /// A snapshot which can be restored to migrate a suffix of sticky blocks to /// the next region. sticky: Option>, - /// Whether there was at least one proper block. Otherwise, sticky blocks - /// are disabled (or else they'd keep being migrated). - stickable: bool, + /// Whether the current group of consecutive sticky blocks are still sticky + /// and may migrate with the attached frame. This is `None` while we aren't + /// processing sticky blocks. On the first sticky block, this will become + /// `Some(true)` if migrating sticky blocks as usual would make a + /// difference - this is given by `regions.may_progress()`. Otherwise, it + /// is set to `Some(false)`, which is usually the case when the first + /// sticky block in the group is at the very top of the page (then, + /// migrating it would just lead us back to the top of the page, leading + /// to an infinite loop). In that case, all sticky blocks of the group are + /// also disabled, until this is reset to `None` on the first non-sticky + /// frame we find. + /// + /// While this behavior of disabling stickiness of sticky blocks at the + /// very top of the page may seem non-ideal, it is only problematic (that + /// is, may lead to orphaned sticky blocks / headings) if the combination + /// of 'sticky blocks + attached frame' doesn't fit in one page, in which + /// case there is nothing Typst can do to improve the situation, as sticky + /// blocks are supposed to always be in the same page as the subsequent + /// frame, but that is impossible in that case, which is thus pathological. + stickable: Option, } /// A snapshot of the distribution state. @@ -314,13 +331,31 @@ impl<'a, 'b> Distributor<'a, 'b, '_, '_, '_> { // If the frame is sticky and we haven't remembered a preceding // sticky element, make a checkpoint which we can restore should we // end on this sticky element. - if self.stickable && self.sticky.is_none() { + // + // The first sticky block within consecutive sticky blocks + // determines whether this group of sticky blocks has stickiness + // disabled or not. + // + // The criteria used here is: if migrating this group of sticky + // blocks together with the "attached" block can't improve the lack + // of space, since we're at the start of the region, then we don't + // do so, and stickiness is disabled (at least, for this region). + // Otherwise, migration is allowed. + // + // Note that, since the whole region is checked, this ensures sticky + // blocks at the top of a block - but not necessarily of the page - + // can still be migrated. + if self.sticky.is_none() + && *self.stickable.get_or_insert_with(|| self.regions.may_progress()) + { self.sticky = Some(self.snapshot()); } } else if !frame.is_empty() { - // If the frame isn't sticky, we can forget a previous snapshot. - self.stickable = true; + // If the frame isn't sticky, we can forget a previous snapshot. We + // interrupt a group of sticky blocks, if there was one, so we reset + // the saved stickable check for the next group of sticky blocks. self.sticky = None; + self.stickable = None; } // Handle footnotes. diff --git a/tests/ref/issue-5296-block-sticky-in-block-at-top.png b/tests/ref/issue-5296-block-sticky-in-block-at-top.png new file mode 100644 index 0000000000000000000000000000000000000000..ad0ace76f3b1919c07b1efd8b38f75faa1063f40 GIT binary patch literal 277 zcmeAS@N?(olHy`uVBq!ia0vp^6+pb214uA*EM@NpQg=LE978H@y}jbNsxeXIc;ffI z_e^V^-&_BViAT4oamtw^zWQ@T2%)m@>z_@TLc$sg<3L_2xJAo;@;o!E*3zApUXO@geCw|&~}*s literal 0 HcmV?d00001 diff --git a/tests/ref/issue-5296-block-sticky-spaced-from-top-of-page.png b/tests/ref/issue-5296-block-sticky-spaced-from-top-of-page.png new file mode 100644 index 0000000000000000000000000000000000000000..ad0ace76f3b1919c07b1efd8b38f75faa1063f40 GIT binary patch literal 277 zcmeAS@N?(olHy`uVBq!ia0vp^6+pb214uA*EM@NpQg=LE978H@y}jbNsxeXIc;ffI z_e^V^-&_BViAT4oamtw^zWQ@T2%)m@>z_@TLc$sg<3L_2xJAo;@;o!E*3zApUXO@geCw|&~}*s literal 0 HcmV?d00001 diff --git a/tests/ref/issue-5296-block-sticky-weakly-spaced-from-top-of-page.png b/tests/ref/issue-5296-block-sticky-weakly-spaced-from-top-of-page.png new file mode 100644 index 0000000000000000000000000000000000000000..9bcdbe569a54673aac2291453d9e1ec4cc3ded5c GIT binary patch literal 236 zcmeAS@N?(olHy`uVBq!ia0vp^6+j%y0VEif+txn=Qky+p978H@y}h!L^H6|<>%)w< z3^U#3EgF_OA8}&YxS307$uYb%S~{oB^8bG}}G7{GXb-TNJ%zw_6#rJcWa%6^X8%(v4|b{8=+ zGc#WopXcxT!BFVn^cR1w9=JQZIzujcm%zb~VT|*uW_zw|DE=L&mw*-6uV#3cKJ#Yo R$2PDxJYD@<);T3K0RR@2Vfp|7 literal 0 HcmV?d00001 diff --git a/tests/suite/layout/container.typ b/tests/suite/layout/container.typ index 799300f0d..9948a00e3 100644 --- a/tests/suite/layout/container.typ +++ b/tests/suite/layout/container.typ @@ -279,3 +279,28 @@ First! // Test box in 100% width block. #block(width: 100%, fill: red, box("a box")) #block(width: 100%, fill: red, [#box("a box") #box()]) + +--- issue-5296-block-sticky-in-block-at-top --- +#set page(height: 3cm) +#v(1.6cm) +#block(height: 2cm, breakable: true)[ + #block(sticky: true)[*A*] + + b +] + +--- issue-5296-block-sticky-spaced-from-top-of-page --- +#set page(height: 3cm) +#v(2cm) + +#block(sticky: true)[*A*] + +b + +--- issue-5296-block-sticky-weakly-spaced-from-top-of-page --- +#set page(height: 3cm) +#v(2cm, weak: true) + +#block(sticky: true)[*A*] + +b