From a1b0a418837dd5fc017e98e2c9cc200a87c11e2d Mon Sep 17 00:00:00 2001 From: Laurenz Date: Thu, 8 Jun 2023 11:51:20 +0200 Subject: [PATCH] Fix a bug with footnotes in blocks/lists/etc. This still not gives the "footnote and entry" are on same page invariant for blocks, but at least we do not get random extraneous page breaks anymore. Relevant issues: #1275, #1433 --- library/src/layout/flow.rs | 87 ++++++++++++++++++++----------- tests/ref/bugs/footnote-list.png | Bin 0 -> 1410 bytes tests/typ/bugs/footnote-list.typ | 11 ++++ 3 files changed, 69 insertions(+), 29 deletions(-) create mode 100644 tests/ref/bugs/footnote-list.png create mode 100644 tests/typ/bugs/footnote-list.typ diff --git a/library/src/layout/flow.rs b/library/src/layout/flow.rs index 6940b55ce..3e524c112 100644 --- a/library/src/layout/flow.rs +++ b/library/src/layout/flow.rs @@ -60,11 +60,12 @@ impl Layout for FlowElem { } else if child.is::() { let mut frame = Frame::new(Size::zero()); frame.meta(styles, true); - layouter.items.push(FlowItem::Frame( + layouter.items.push(FlowItem::Frame { frame, - Axes::new(Align::Top, Align::Left), - true, - )); + aligns: Axes::new(Align::Top, Align::Left), + sticky: true, + movable: false, + }); } else if child.can::() { layouter.layout_multiple(vt, child, styles)?; } else if child.is::() { @@ -120,14 +121,27 @@ enum FlowItem { Absolute(Abs, bool), /// Fractional spacing between other items. Fractional(Fr), - /// A frame for a layouted block, how to align it, and whether it is sticky. - Frame(Frame, Axes, bool), + /// A frame for a layouted block, how to align it, whether it sticks to the + /// item after it (for orphan preventation), and whether it is movable + /// (to keep it together with its footnotes). + Frame { frame: Frame, aligns: Axes, sticky: bool, movable: bool }, /// An absolutely placed frame. Placed(Frame), /// A footnote frame (can also be the separator). Footnote(Frame), } +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(), + } + } +} + impl<'a> FlowLayouter<'a> { /// Create a new flow layouter. fn new(mut regions: Regions<'a>, styles: StyleChain<'a>) -> Self { @@ -194,7 +208,7 @@ impl<'a> FlowLayouter<'a> { for (i, item) in self.items.iter().enumerate().rev() { match *item { FlowItem::Absolute(_, _) => {} - FlowItem::Frame(.., true) => sticky = i, + FlowItem::Frame { sticky: true, .. } => sticky = i, _ => break, } } @@ -214,7 +228,10 @@ impl<'a> FlowLayouter<'a> { self.layout_item(vt, FlowItem::Absolute(leading, true))?; } - self.layout_item(vt, FlowItem::Frame(frame, aligns, false))?; + self.layout_item( + vt, + FlowItem::Frame { frame, aligns, sticky: false, movable: true }, + )?; } self.last_was_par = true; @@ -233,7 +250,7 @@ impl<'a> FlowLayouter<'a> { let sticky = BlockElem::sticky_in(styles); let pod = Regions::one(self.regions.base(), Axes::splat(false)); let frame = content.layout(vt, styles, pod)?.into_frame(); - self.layout_item(vt, FlowItem::Frame(frame, aligns, sticky))?; + self.layout_item(vt, FlowItem::Frame { frame, aligns, sticky, movable: true })?; self.last_was_par = false; Ok(()) } @@ -276,11 +293,16 @@ impl<'a> FlowLayouter<'a> { self.regions.root = self.root && is_columns; for (i, frame) in fragment.into_iter().enumerate() { - if i > 0 { + if i > 0 + && !self.items.iter().all(|item| matches!(item, FlowItem::Footnote(_))) + { self.finish_region()?; } - self.layout_item(vt, FlowItem::Frame(frame, aligns, sticky))?; + self.layout_item( + vt, + FlowItem::Frame { frame, aligns, sticky, movable: false }, + )?; } self.regions.root = false; @@ -295,14 +317,17 @@ impl<'a> FlowLayouter<'a> { match item { FlowItem::Absolute(v, weak) => { if weak - && !self.items.iter().any(|item| matches!(item, FlowItem::Frame(..))) + && !self + .items + .iter() + .any(|item| matches!(item, FlowItem::Frame { .. })) { return Ok(()); } self.regions.size.y -= v } FlowItem::Fractional(_) => {} - FlowItem::Frame(ref frame, ..) => { + FlowItem::Frame { ref frame, .. } => { let size = frame.size(); if !self.regions.size.y.fits(size.y) && !self.regions.in_last() { self.finish_region()?; @@ -310,7 +335,7 @@ impl<'a> FlowLayouter<'a> { self.regions.size.y -= size.y; if self.root { - return self.handle_footnotes(vt, item, size.y); + return self.handle_footnotes(vt, item); } } FlowItem::Placed(_) => {} @@ -341,7 +366,7 @@ impl<'a> FlowLayouter<'a> { match item { FlowItem::Absolute(v, _) => used.y += *v, FlowItem::Fractional(v) => fr += *v, - FlowItem::Frame(frame, ..) => { + FlowItem::Frame { frame, .. } => { let size = frame.size(); used.y += size.y; used.x.set_max(size.x); @@ -383,7 +408,7 @@ impl<'a> FlowLayouter<'a> { let remaining = self.initial.y - used.y; offset += v.share(fr, remaining); } - FlowItem::Frame(frame, aligns, _) => { + FlowItem::Frame { frame, aligns, .. } => { ruler = ruler.max(aligns.y); let x = aligns.x.position(size.x - frame.width()); let y = offset + ruler.position(size.y - used.y); @@ -426,16 +451,14 @@ impl<'a> FlowLayouter<'a> { impl FlowLayouter<'_> { /// Processes all footnotes in the frame. #[tracing::instrument(skip_all)] - fn handle_footnotes( - &mut self, - vt: &mut Vt, - item: FlowItem, - height: Abs, - ) -> SourceResult<()> { + fn handle_footnotes(&mut self, vt: &mut Vt, item: FlowItem) -> SourceResult<()> { // Find footnotes in the frame. let mut notes = Vec::new(); - if let FlowItem::Frame(frame, ..) = &item { + + let mut is_movable = true; + if let FlowItem::Frame { frame, movable, .. } = &item { find_footnotes(&mut notes, frame); + is_movable = *movable; } let prev_len = self.items.len(); @@ -474,12 +497,18 @@ impl FlowLayouter<'_> { if !had_footnotes { self.items.pop(); } - let moved: Vec<_> = self.items.drain(prev_len..).collect(); - self.finish_region()?; - self.has_footnotes = - moved.iter().any(|item| matches!(item, FlowItem::Footnote(_))); - self.regions.size.y -= height; - self.items.extend(moved); + + if is_movable { + let moved: Vec<_> = self.items.drain(prev_len..).collect(); + self.finish_region()?; + self.has_footnotes = + moved.iter().any(|item| matches!(item, FlowItem::Footnote(_))); + self.regions.size.y -= moved.iter().map(FlowItem::height).sum(); + self.items.extend(moved); + } else { + self.finish_region()?; + } + can_skip = false; continue 'outer; } diff --git a/tests/ref/bugs/footnote-list.png b/tests/ref/bugs/footnote-list.png new file mode 100644 index 0000000000000000000000000000000000000000..1b56f2274fb51ef25c95cf5806adaeb851b9384d GIT binary patch literal 1410 zcmeAS@N?(olHy`uVBq!ia0y~yU}OQ}bsWq<5y!xe4M5J$0G|-o|NsC0{rmUNpFh8U z1JUoFKY#uF`SZt*AK$*RNkcefsp_!>4!eKfizf{>6(I&z`@1_Uzfyr%xY0 ze*Easqr3N?-@kwV&b?=M@7}$2=gEy*k8a$!aqZf*%UADSy?XWXl^a*CTse2)#@X}N z&!0bk?%cVPr!OBrdGW}xbBB+fJ#yqo{q6sI_aEQ4Z{O~{NB8X6vuoF`E!*~O+qP}X zmMt4MZd|!~^Qu*=map8nbou(_%a<=(wrt6gB?}j?Ua(-%?72&4&z?PN)~u=1=TDh7 zZ}QZ+Q>RXyJY~k@$&)8eoH${^gzny{T|HAeIy&0hJKNja+uGWiT6$VqTAG`i8yg#I z>)Y$=>#J&7s;a8WD;p~+s>;gBii(Qz3o8o?3-bypvU5vwb8~ZYax$}vGqQ>@GILYY z^HWn(6Oyvx5;9`r)8pgg!y*!bLt{fjLxVzM0)nEvd_sJDd_28_y}iAiot+(=yquhz z9335P?cMF|?X9h?%`6-&EG$gT?aj^2P0Z}f%*;$oObm>y3=IwS3@i)`3^cWk)iewg z6%}PeA#l*yT_{4a5dAWE*czAfYxw+Xm1UNW2n3$M=kr>Y!wU>c`xy94PF{I+w z+q>cZqJbi87nm2tacjN0A+2>|_o5pczsCh^6o127{XWTcZhX!=(^&#NiiX$Ozx|SX zmKztg?5*+1@*urRCk93)77hUghlUHfEh0J~23!Qj(6B#rMotK<2quK7i0UFgx{mJIOr0m16dC$uiE5AAWd5@)))BI=Z$A7uHzxdS!)H%aRQ}mkf8g(oofX8zh z+)j58I$p>%olnCSQm1+hYW}~|yePOye6y6DANTz;Gq2XJyS}&V*9xXx(_(MFc)`)$ zm+rf|Nx7${^z*0LRk?d_Rc-y6^_sV5!P@WV<-G+IGG1?;hbuU+MH?PRlO***&*``Q zlH&?IzuR}n+6Y6eYN_D3Dtta?#XC_M#pvqN`{#92#97=<@J$hTcJIHKT;io!!m<+u z&uyQQ*M3aDQSjQ*!WasAs!{?7%#+!x-Wi|2wVu>DxdxNj;e6F8=*4SaD zk|HcHcP-iQ>BF~1(Sw3n`l;8qiv+#0uD