From 9b697d59ae4d06a0ee0b1bc6b3879d3b9c67a8d4 Mon Sep 17 00:00:00 2001 From: PgBiel <9021226+PgBiel@users.noreply.github.com> Date: Wed, 18 Dec 2024 13:54:14 -0300 Subject: [PATCH] Fix infinite loop with footnote which never fits (#5498) --- crates/typst-layout/src/flow/compose.rs | 38 ++++++++++++++-- ...ssue-5496-footnote-in-float-never-fits.png | Bin 0 -> 399 bytes ...ssue-5496-footnote-never-fits-multiple.png | Bin 0 -> 1211 bytes tests/ref/issue-5496-footnote-never-fits.png | Bin 0 -> 399 bytes ...sue-5496-footnote-separator-never-fits.png | Bin 0 -> 226 bytes tests/suite/layout/flow/footnote.typ | 43 ++++++++++++++++++ 6 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 tests/ref/issue-5496-footnote-in-float-never-fits.png create mode 100644 tests/ref/issue-5496-footnote-never-fits-multiple.png create mode 100644 tests/ref/issue-5496-footnote-never-fits.png create mode 100644 tests/ref/issue-5496-footnote-separator-never-fits.png diff --git a/crates/typst-layout/src/flow/compose.rs b/crates/typst-layout/src/flow/compose.rs index 326456752..3cf66f9e3 100644 --- a/crates/typst-layout/src/flow/compose.rs +++ b/crates/typst-layout/src/flow/compose.rs @@ -15,7 +15,7 @@ use typst_library::model::{ FootnoteElem, FootnoteEntry, LineNumberingScope, Numbering, ParLineMarker, }; use typst_syntax::Span; -use typst_utils::NonZeroExt; +use typst_utils::{NonZeroExt, Numeric}; use super::{distribute, Config, FlowResult, LineNumberConfig, PlacedChild, Stop, Work}; @@ -374,7 +374,11 @@ impl<'a, 'b> Composer<'a, 'b, '_, '_> { let mut relayout = false; let mut regions = *regions; - let mut migratable = migratable && !breakable && regions.may_progress(); + + // The first footnote's origin frame should be migratable if the region + // may progress (already checked by the footnote function) and if the + // origin frame isn't breakable (checked here). + let mut migratable = migratable && !breakable; for (y, elem) in notes { // The amount of space used by the in-flow content that contains the @@ -464,11 +468,35 @@ impl<'a, 'b> Composer<'a, 'b, '_, '_> { // If the first frame is empty, then none of its content fit. If // possible, we then migrate the origin frame to the next region to // uphold the footnote invariant (that marker and entry are on the same - // page). If not, we just queue the footnote for the next page. + // page). If not, we just queue the footnote for the next page, but + // only if that would actually make a difference (that is, if the + // footnote isn't alone in the page after not fitting in any previous + // pages, as it probably won't ever fit then). + // + // Note that a non-zero flow need also indicates that queueing would + // make a difference, because the flow need is subtracted from the + // available height in the entry's pod even if what caused that need + // wasn't considered for the input `regions`. For example, floats just + // pass the `regions` they received along to their footnotes, which + // don't take into account the space occupied by the floats themselves, + // but they do indicate their footnotes have a non-zero flow need, so + // queueing them can matter as, in the following pages, the flow need + // will be set to zero and the footnote will be alone in the page. + // Then, `may_progress()` will also be false (this time, correctly) and + // the footnote is laid out, as queueing wouldn't improve the lack of + // space anymore and would result in an infinite loop. + // + // However, it is worth noting that migration does take into account + // the original region, before inserting what prompted the flow need. + // Logically, if moving the original frame can't improve the lack of + // space, then migration should be inhibited. The space occupied by the + // original frame is not relevant for that check. Therefore, + // `regions.may_progress()` must still be checked separately for + // migration, regardless of the presence of flow need. if first.is_empty() && exist_non_empty_frame { - if migratable { + if migratable && regions.may_progress() { return Err(Stop::Finish(false)); - } else { + } else if regions.may_progress() || !flow_need.is_zero() { self.footnote_queue.push(elem); return Ok(()); } diff --git a/tests/ref/issue-5496-footnote-in-float-never-fits.png b/tests/ref/issue-5496-footnote-in-float-never-fits.png new file mode 100644 index 0000000000000000000000000000000000000000..4ae5903d808d8fe616b9e3b7d484e59e508b0ca1 GIT binary patch literal 399 zcmV;A0dW3_P)No|9L@C6`7jPU%ctz6|`d2CNgb? zprNJt-TTiFG^~;*C15l^k7^#(JgRxL02~eNQO)FQe){w&nVOFtKcAYO4{Vc=*&3>- zY65B|v+dc?)b;qu3-a4xK%42R8PSOQ{{81@@*FJyM>UUX9x%<19zB{jZ(dbZ)y$bQ z;euDMUY#{-76HvafBw|g)_(u~{qNtu6%`dvojL^+eEIUFuCDH%KYs{l{{H>@{Q2`C z?7+alwzf8i=Cy0r66NfB_wJRJme$qPSzB8d6cj**1J0k%&(9~I`QgKdl9G})Z{7s5 z)6&u)nvWkpuB@yKw4Z?H1q&7^C@26KzkdC4adAmcPY1HAtE+)*!p?s2;)TAxzO%D) tMn*<$ZLNfa#I$MCE?>S33_L$SKLA0=QVT-knP30_002ovPDHLkV1k12!Gr(+ literal 0 HcmV?d00001 diff --git a/tests/ref/issue-5496-footnote-never-fits-multiple.png b/tests/ref/issue-5496-footnote-never-fits-multiple.png new file mode 100644 index 0000000000000000000000000000000000000000..24fcf7098028adda21b1861bd6f4ab35d0147cb9 GIT binary patch literal 1211 zcmV;s1VsCZP)+9>jzP_iYr{(44-{9xU%+!*Sl0iX1jg5`&?(WOW%S1#(WMpKerKO;t zplxk!$jHc|qN0f++!`}_Q9X=(BC@xj5tK0ZEZXlPYcRr>n+ zQc_a(_V!a#Q%Fci#KgpKaB!}!u1!r%M@L7Ik&$O-XBZe5eSLk9kdQn)JQ5NTf`Wqe z_4SmLlze=Aa&mHIWo6LN(4C!~s;a73SXl7z@HRF!+S=M?W@bA(JGr^JA|fItCMF~# zBqb#!ii(PJb90A>hvw$y!otGI$;r;n&Vhk};o;#94i2TKuZ@qF!o$jGYja&+XQ`{R zg@=!rnxehF#&vgpLPA1dU|@rTgIru(+1lRQ+~Aa!o~Nm?kdm6l$k1kJaJjp~l9Zgl z!phXu+3oJ~Y;JbFzQVAwx`v35p`)wo?C`FzxTvbKNJ>(0a(Y5UOxW7u-QC?FARrkT z8QIy{dwYAw$H&yv)X&k`BO@cv&(DB@igSSGmetkO?d|Qpz{vOa_xbtx zC@3hy!^8ai{F<7Y}0D*FRssI226-h)vRCwC$)n!j3 zK^({N-!5&DD->F24>+uM$6e3e-Ca0$cj26KcX!9#-Aa+tLc6>wNO>|lGa*O_1or!6 zlTZGW>}E2v7~+i!V=W-U$f7nfnlV*O)Y4_lU~Qc>jk4JmpeChEj&tU!5Nyz+@3nEl zQ0cZLZH|-z{7Lh@m2mCc>78*qya*I;CWvRx`0UwD;-eea<8~d`zi+Q>(vy!U_6`M4 zd4e!X*|;7Sv^M+qFGknE_K(5Ul~w;4T(xqRc`S`hz`P*Jn)8AAY6_DJteORYa^kwq zn`1AY3oMGyIOzNI17}hcx`~3|1%ArL4MgFl@(KWT4#FBgeJ(34hO4!iz=mw#^&7Z8 zf-EOBZG{DR@}975ckf}j1rHv(2-M%fXx$Xt*MX?AFgIq}RBLr<$uQiwEn6K0nUsaa zx&Z@)(-#7eC^RerTT&_qZQpC*o7pXgNh74mjZrW~#-%NV>kwYEJ zX*_(8gAp5O{0UoOI3~;9Q}?61s2I>rRQ_@OnR4QKSHnXN)?d89q8NXPGkN8za#aqV z9Fz1}ZVl39=nO;-{M{v?Bzo(r9OKQd=qMQmB12jXGss@P0vi8dj0`p3oUkXVD%Ab* Z8SPk-9L29Y7$yJ!002ovPDHLkV1jc1cp3lz literal 0 HcmV?d00001 diff --git a/tests/ref/issue-5496-footnote-never-fits.png b/tests/ref/issue-5496-footnote-never-fits.png new file mode 100644 index 0000000000000000000000000000000000000000..4ae5903d808d8fe616b9e3b7d484e59e508b0ca1 GIT binary patch literal 399 zcmV;A0dW3_P)No|9L@C6`7jPU%ctz6|`d2CNgb? zprNJt-TTiFG^~;*C15l^k7^#(JgRxL02~eNQO)FQe){w&nVOFtKcAYO4{Vc=*&3>- zY65B|v+dc?)b;qu3-a4xK%42R8PSOQ{{81@@*FJyM>UUX9x%<19zB{jZ(dbZ)y$bQ z;euDMUY#{-76HvafBw|g)_(u~{qNtu6%`dvojL^+eEIUFuCDH%KYs{l{{H>@{Q2`C z?7+alwzf8i=Cy0r66NfB_wJRJme$qPSzB8d6cj**1J0k%&(9~I`QgKdl9G})Z{7s5 z)6&u)nvWkpuB@yKw4Z?H1q&7^C@26KzkdC4adAmcPY1HAtE+)*!p?s2;)TAxzO%D) tMn*<$ZLNfa#I$MCE?>S33_L$SKLA0=QVT-knP30_002ovPDHLkV1k12!Gr(+ literal 0 HcmV?d00001 diff --git a/tests/ref/issue-5496-footnote-separator-never-fits.png b/tests/ref/issue-5496-footnote-separator-never-fits.png new file mode 100644 index 0000000000000000000000000000000000000000..4fe4fdee344279a099818e3252c031754aab592f GIT binary patch literal 226 zcmV<803H8{P)(us, C @fn, D @fn, E @fn. float: true, footnote[b] ) + +--- issue-5496-footnote-never-fits --- +// Test whether a footnote which is always too large would cause an infinite +// loop. +#set page(width: 20pt, height: 20pt) +#set footnote.entry(indent: 0pt) + +#footnote(text(size: 15pt)[a] * 100) + +--- issue-5496-footnote-in-float-never-fits --- +// Test whether an overlarge footnote in a float also does not cause an +// infinite loop. +#set page(width: 20pt, height: 20pt) + +#place( + top, + float: true, + footnote(text(size: 15pt)[a] * 100) +) + +--- issue-5496-footnote-never-fits-multiple --- +// Test whether multiple overlarge footnotes are properly split up across +// pages. +#set page(width: 20pt, height: 20pt) +#set footnote.entry(indent: 0pt) + +A + +#footnote(text(size: 15pt)[a] * 100) +#footnote(text(size: 15pt)[b] * 100) +#footnote[Fit] + +B + +C + +--- issue-5496-footnote-separator-never-fits --- +// Test whether an overlarge footnote separator does not cause an infinite +// loop and compiles. +#set page(height: 2em) +#set footnote.entry(separator: v(5em)) + +#footnote[]