From 560791afe84c87c9ef212b1e4289edc30ce50646 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Wed, 8 May 2024 11:31:28 +0200 Subject: [PATCH] Fix footnote migration (#4095) --- crates/typst/src/layout/flow.rs | 52 +++++++++--------- tests/ref/issue-3481-cite-location.png | Bin 0 -> 602 bytes tests/ref/issue-footnotes-skip-first-page.png | Bin 0 -> 571 bytes tests/suite/model/cite.typ | 17 ++++++ tests/suite/model/footnote.typ | 7 +++ 5 files changed, 50 insertions(+), 26 deletions(-) create mode 100644 tests/ref/issue-3481-cite-location.png create mode 100644 tests/ref/issue-footnotes-skip-first-page.png diff --git a/crates/typst/src/layout/flow.rs b/crates/typst/src/layout/flow.rs index 8f0def179..3cc53c6a4 100644 --- a/crates/typst/src/layout/flow.rs +++ b/crates/typst/src/layout/flow.rs @@ -141,15 +141,6 @@ enum FlowItem { } impl FlowItem { - /// The inherent height of the item. - fn height(&self) -> Abs { - match self { - Self::Absolute(v, _) => *v, - Self::Fractional(_) | Self::Placed { .. } => Abs::zero(), - Self::Frame { frame, .. } | Self::Footnote(frame) => frame.height(), - } - } - /// Whether this item is out-of-flow. /// /// Out-of-flow items are guaranteed to have a [`Size::zero()`]. @@ -411,12 +402,16 @@ impl<'a> FlowLayouter<'a> { self.finish_region(engine, false)?; } + let in_last = self.regions.in_last(); self.regions.size.y -= height; if self.root && movable { let mut notes = Vec::new(); find_footnotes(&mut notes, frame); self.items.push(item); - if !self.handle_footnotes(engine, &mut notes, true, false)? { + + // When we are already in_last, we can directly force the + // footnotes. + if !self.handle_footnotes(engine, &mut notes, true, in_last)? { let item = self.items.pop(); self.finish_region(engine, false)?; self.items.extend(item); @@ -651,7 +646,16 @@ impl FlowLayouter<'_> { engine: &mut Engine, mut notes: Vec>, ) -> SourceResult<()> { - if self.root && !self.handle_footnotes(engine, &mut notes, false, false)? { + // When we are already in_last, we can directly force the + // footnotes. + if self.root + && !self.handle_footnotes( + engine, + &mut notes, + false, + self.regions.in_last(), + )? + { self.finish_region(engine, false)?; self.handle_footnotes(engine, &mut notes, false, true)?; } @@ -666,8 +670,11 @@ impl FlowLayouter<'_> { movable: bool, force: bool, ) -> SourceResult { - let items_len = self.items.len(); - let notes_len = notes.len(); + let prev_notes_len = notes.len(); + let prev_items_len = self.items.len(); + let prev_size = self.regions.size; + let prev_has_footnotes = self.has_footnotes; + let prev_locator = engine.locator.clone(); // Process footnotes one at a time. let mut k = 0; @@ -682,7 +689,6 @@ impl FlowLayouter<'_> { } self.regions.size.y -= self.footnote_config.gap; - let checkpoint = engine.locator.clone(); let frames = FootnoteEntry::new(notes[k].clone()) .pack() .layout(engine, self.styles, self.regions.with_root(false))? @@ -694,18 +700,12 @@ impl FlowLayouter<'_> { && (k == 0 || movable) && frames.first().is_some_and(Frame::is_empty) { - // Remove existing footnotes attempts because we need to - // move the item to the next page. - notes.truncate(notes_len); - - // Undo region modifications. - for item in self.items.drain(items_len..) { - self.regions.size.y -= item.height(); - } - - // Undo locator modifications. - *engine.locator = checkpoint; - + // Undo everything. + notes.truncate(prev_notes_len); + self.items.truncate(prev_items_len); + self.regions.size = prev_size; + self.has_footnotes = prev_has_footnotes; + *engine.locator = prev_locator; return Ok(false); } diff --git a/tests/ref/issue-3481-cite-location.png b/tests/ref/issue-3481-cite-location.png new file mode 100644 index 0000000000000000000000000000000000000000..cfc13db5126fca92603d5cc969aca992c6be1292 GIT binary patch literal 602 zcmV-g0;T_{rmg;`}_O)`ug|x`SI@ayaC>g(_3=IG_+ z<>27r-QVZm-{0Qe-rL>f+T7&W+Tz*S+0@qF*4Eb3)!ot4+tAY5&CSos%+$%s&dJHi z#l^+J!^^(F$hy45w6?yswzjdcv97SVuduYQudl4GwyUkRs;soBtFx=Ct*fi6rKhi? zrmm)@rlqB&prE3kpP!tbrktFfnVO!OnwprIos*TFl9Zg2la-H=nUIl{gociDb$xVn zbZ~NdZf|pGYjbOBYiMe6X=-j}XmDj`Z(?L@WMpJvV{2n$V^dXKOHEZrNl`^dPe(^b zMn*Wqr1vuE zy_4Q4`(Fow1t1>47oqcQ;FoOKAR;0nA|fIpA|fIpA|k3r!4poNh-xAVT4p<7Z~8a2 zG5_LVPVZBCvEU&*`MtjPc;g|gWegjTvXF0uh^T?>SA_Z*sfdedCgIdTxP95@oa;G0 z2w_LARwOg36SqUEA7UBa-IC0(rn9R0Y8u{LEQR&UJgn+B{4;JGb|UVfTV}3TMV=jO z-5quUNXG9lmPiHw(ALVgR!{9`CjfR}PSQs}*WVnh^}d}0ar6Xhq6&P*`Gk+~(il#c oJ%pqGL9FHa;YGmvCZYya1evW((BL~UegFUf07*qoM6N<$f;~(|asU7T literal 0 HcmV?d00001 diff --git a/tests/ref/issue-footnotes-skip-first-page.png b/tests/ref/issue-footnotes-skip-first-page.png new file mode 100644 index 0000000000000000000000000000000000000000..d24387e3bad35ec8d7db6891723f70e3c97ad26a GIT binary patch literal 571 zcmV-B0>u4^P)_{rmg;`T6g(_7>gwp| z=;r3;t}($dn*&C$)x&C1Nw z$;rvc$jHXV#>K_O!NbeJ!NI=3$iKh8y1c}*v$L|YvazwTuCTbUu&}SMudJ@NuCA`D zt+k}4uBN7@oSvqfouQnZoSB-Qn3MbwYhaYdH)3tZ|aAHBqu9xk@L7>#Z^y`*`aO`(T z8~=T>74^K)) #cite(). #show bibliography: none #bibliography("/assets/bib/works.bib") + +--- issue-3481-cite-location --- +// The locator was cloned in the wrong location, leading to inconsistent +// citation group locations in the second footnote attempt. +#set page(height: 60pt) + +// First page shouldn't be empty because otherwise we won't skip the first +// region which causes the bug in the first place. +#v(10pt) + +// Everything moves to the second page because we want to keep the line and +// its footnotes together. +#footnote[@netwok] +#footnote[A] + +#show bibliography: none +#bibliography("/assets/bib/works.bib") diff --git a/tests/suite/model/footnote.typ b/tests/suite/model/footnote.typ index c41db5779..34450ca4c 100644 --- a/tests/suite/model/footnote.typ +++ b/tests/suite/model/footnote.typ @@ -180,3 +180,10 @@ B #footnote[b] - #footnote[1] - #footnote[2] + +--- issue-footnotes-skip-first-page --- +// In this issue, we would get an empty page at the beginning because footnote +// layout didn't properly check for in_last. +#set page(height: 50pt) +#footnote[A] +#footnote[B]