From 693edb475dad7ea60a1b1c5d2868f85adce0af06 Mon Sep 17 00:00:00 2001 From: Eric Biedert Date: Mon, 26 May 2025 19:44:51 +0200 Subject: [PATCH] Don't break blocks after empty frame Instead, spill the whole child into the next region to prevent small leftovers to influence layout. This is not done when all frames are empty (e.g. for an explicitly sized block without content or fill). This helps with the following cases: - Previously, if a sticky block was followed by a leftover frame, the stickiness would be ignored, as the leftover was in fact sticking. This is not currently a problem, as sticky blocks aren't really breakable at the moment, but probably will be in the future. - When ignoring stroke and fill for a first empty frame, a nested broken block would previously make the first frame not be considered empty anymore, which would lead to the leftover frame being filled. - Similarly, when the fill of an explicitly sized block is ignored in the first empty frame, the leftover part would still be considered as laid out, making the actually visible block too small. --- crates/typst-layout/src/flow/collect.rs | 3 +++ crates/typst-layout/src/flow/distribute.rs | 7 +++++++ tests/ref/issue-2914-block-fill-skip-nested.png | Bin 499 -> 452 bytes tests/ref/issue-2914-block-height-cut-off.png | Bin 409 -> 411 bytes .../issue-6125-block-place-width-limited.png | Bin 147 -> 144 bytes 5 files changed, 10 insertions(+) diff --git a/crates/typst-layout/src/flow/collect.rs b/crates/typst-layout/src/flow/collect.rs index 2c14f7a37..fb0a8086e 100644 --- a/crates/typst-layout/src/flow/collect.rs +++ b/crates/typst-layout/src/flow/collect.rs @@ -457,6 +457,7 @@ impl<'a> MultiChild<'a> { regions: Regions, ) -> SourceResult<(Frame, Option>)> { let fragment = self.layout_full(engine, regions)?; + let exist_non_empty_frame = fragment.iter().any(|f| !f.is_empty()); // Extract the first frame. let mut frames = fragment.into_iter(); @@ -466,6 +467,7 @@ impl<'a> MultiChild<'a> { let mut spill = None; if frames.next().is_some() { spill = Some(MultiSpill { + exist_non_empty_frame, multi: self, full: regions.full, first: regions.size.y, @@ -537,6 +539,7 @@ fn layout_multi_impl( /// The spilled remains of a `MultiChild` that broke across two regions. #[derive(Debug, Clone)] pub struct MultiSpill<'a, 'b> { + pub(super) exist_non_empty_frame: bool, multi: &'b MultiChild<'a>, first: Abs, full: Abs, diff --git a/crates/typst-layout/src/flow/distribute.rs b/crates/typst-layout/src/flow/distribute.rs index f504d22e7..d12b1ff68 100644 --- a/crates/typst-layout/src/flow/distribute.rs +++ b/crates/typst-layout/src/flow/distribute.rs @@ -283,6 +283,13 @@ impl<'a, 'b> Distributor<'a, 'b, '_, '_, '_> { // Lay out the block. let (frame, spill) = multi.layout(self.composer.engine, self.regions)?; + if frame.is_empty() && spill.as_ref().is_some_and(|s| s.exist_non_empty_frame) { + // If the first frame is empty, but there are non-empty frames in + // the spill, the whole child should be put in the next region to + // avoid any invisible orphans at the end of this region. + return Err(Stop::Finish(false)); + } + self.frame(frame, multi.align, multi.sticky, true)?; // If the block didn't fully fit into the current region, save it into diff --git a/tests/ref/issue-2914-block-fill-skip-nested.png b/tests/ref/issue-2914-block-fill-skip-nested.png index d7b1aa7d7a8e5b0cc1489a689620ba3a2ddf0af1..0dcbd6d85fd2383e58a8606c645b27b7b4f57374 100644 GIT binary patch delta 426 zcmV;b0agC<1H=Q6B!ACPOjJex|Nnrx>_DgNf7|~GeBA5w{0Ms6w%7Ev*7Se5>_DjO zAdcc7jpA@{aP{@|l$4Zwe0=Nc>y3?#@$vD&!NJwl)yBrgXlQ6aaDrT)xpT+pO@^Fy z%k4yViCdkw7%4DHfR!pqReIC*QjVowqrNg%WoNR`Q;(-gfq#}ZU2Id3sD0S{3ltX= zBPbCaA`>Ad5+5WIASF+TpUcb3X=!PblauP|>PSdPIbw1?K0Y=!Hrm?Setv$ImX`SV z_{hk}ZEbC!prHHv`z|gn0001c$YAXN006K_L_t(|+U?d=jsrmq08wvx!_3Ug%nbKG zQ!X%{nX#hOyMI8*EwyCGQEEh-Ypy=aJMQ-`CrrB!A>kOjJex|Nras{0e;Bf7}0c)BF^F-0SP>QjVn%ecQ_7_sQY* zJEZD6qw0Fo_FkjDGFN3eVsdA)&{L16N`aP7h@TQ5B@!Pb6Coy3kf=6YY<<}L3ltX= zBPbCaB3qreM0SZ7DKK`+?Mi@^DoIsbpSg3#=uL*4KyZSVmVcHuHa5Y*!PV8(XlQ80 z#>U#(+J1h1E-o(k`1r`k$Zc(H@$vCKK0cF^lWA#bNJvQP>gs%ae3X=waBy(-_4S~j zp!@s#%gf7+jg26U;vkOVwbt~u*YpT_+d!!7f4S^Hr|f{b>;M1&mp08c0002WNklwLGeZ*zZT-QCJ5yf)S? z2@@u4H9TKjTDO>qg9{h%G66VgTP5r~fJ4~0z_B}7TT(aWCvnbU@5g&Ept9mm6Ar0_ zzv79AO4uJuMjtf7|HeFf6~e!N#ymYLguj2@USGIC2s3VC+ynpsfa9|iK1g)Q z>MBW95+5WIASF+TpG0U=~Xf|GLW2?v)C@y=|_-wn{KYwg~XR^>!kEcq3mNs2% z6eB1L6cg(mFJmy)ovTd&TFu)p1S7eTmO zG&>#h(T6J~$7>Gz0t+apIED*wk2@@vF#~c6v000m(KWv#LSaTpf Q)c^nh07*qoM6N<$f|t3UWdHyG delta 368 zcmV-$0gwKh1DOMmGJi&Sjd;)TV5Pxox7A{*#xz-IBtA%V$?8ddlQddsWvGXC<_!9ec1d{kf=6YY&l|bIACyk()CGyls;;FKyZRwpSg3#=uL*4 zcFXNVc8Ob^w-_lfN`RFrNmY8&_EL_eUZcJ;S7m3i&{L16N`HZtd)4@3tH?H9ZBL1y zeb@UJC@w#2er&ti8!t8=H9i0U0LqUKLz38 z!0i0EM;{)I9cjB~~F?yu_W- xH=21r=ZJ`!7R=RC0iPI0?juZui7)|pX96@Mi!Pu45w!pS002ovPDHLkV1ly%BsKs5 delta 94 zcmV-k0HObo0h0lcJXb&wkgW$r;|v zRS0sOha|Zr$j(y%XkuS+pPyk8)X) At^fc4