From 3ec5d442d7be4700aa02d015420ddd5cefa78b95 Mon Sep 17 00:00:00 2001
From: Laurenz <laurmaedje@gmail.com>
Date: Tue, 1 Oct 2024 15:32:02 +0200
Subject: [PATCH] Fix panic in spilled block layout (#5085)

---
 crates/typst/src/layout/flow/collect.rs |  25 ++++++++++++++++++------
 tests/ref/issue-5024-spill-backlog.png  | Bin 0 -> 270 bytes
 tests/suite/layout/flow/flow.typ        |   4 ++++
 3 files changed, 23 insertions(+), 6 deletions(-)
 create mode 100644 tests/ref/issue-5024-spill-backlog.png

diff --git a/crates/typst/src/layout/flow/collect.rs b/crates/typst/src/layout/flow/collect.rs
index 9cd9bdc4e..9fc64f067 100644
--- a/crates/typst/src/layout/flow/collect.rs
+++ b/crates/typst/src/layout/flow/collect.rs
@@ -403,6 +403,7 @@ impl<'a> MultiChild<'a> {
                 full: regions.full,
                 first: regions.size.y,
                 backlog: vec![],
+                min_backlog_len: regions.backlog.len(),
             });
         }
 
@@ -474,6 +475,7 @@ pub struct MultiSpill<'a, 'b> {
     first: Abs,
     full: Abs,
     backlog: Vec<Abs>,
+    min_backlog_len: usize,
 }
 
 impl MultiSpill<'_, '_> {
@@ -483,17 +485,18 @@ impl MultiSpill<'_, '_> {
         engine: &mut Engine,
         regions: Regions,
     ) -> SourceResult<(Frame, Option<Self>)> {
-        // We build regions for the whole `MultiChild` with the sizes passed to
-        // earlier parts of it plus the new regions. Then, we layout the
-        // complete block, but extract only the suffix that interests us.
+        // The first region becomes unchangable and committed to our backlog.
         self.backlog.push(regions.size.y);
 
+        // The remaining regions are ephemeral and may be replaced.
         let mut backlog: Vec<_> =
             self.backlog.iter().chain(regions.backlog).copied().collect();
 
-        // Remove unnecessary backlog items (also to prevent it from growing
-        // unnecessarily, which would change the region's hash).
-        while !backlog.is_empty() && backlog.last().copied() == regions.last {
+        // Remove unnecessary backlog items to prevent it from growing
+        // unnecessarily, changing the region's hash.
+        while backlog.len() > self.min_backlog_len
+            && backlog.last().copied() == regions.last
+        {
             backlog.pop();
         }
 
@@ -513,6 +516,16 @@ impl MultiSpill<'_, '_> {
             .into_iter()
             .skip(self.backlog.len());
 
+        // Ensure that the backlog never shrinks, so that unwrapping below is at
+        // least fairly safe. Note that the whole region juggling here is
+        // fundamentally not ideal: It is a compatibility layer between the old
+        // (all regions provided upfront) & new (each region provided on-demand,
+        // like an iterator) layout model. This approach is not 100% correct, as
+        // in the old model later regions could have an effect on earlier
+        // frames, but it's the best we can do for now, until the multi
+        // layouters are refactored to the new model.
+        self.min_backlog_len = self.min_backlog_len.max(backlog.len());
+
         // Save the first frame.
         let frame = frames.next().unwrap();
 
diff --git a/tests/ref/issue-5024-spill-backlog.png b/tests/ref/issue-5024-spill-backlog.png
new file mode 100644
index 0000000000000000000000000000000000000000..76fd9fbbfe8a74c32b772d7c15245218984f7b6a
GIT binary patch
literal 270
zcmeAS@N?(olHy`uVBq!ia0vp^6+mpn0VEhUopE3RQdd1)978H@CI7H@Xh^*BD6rdI
zS8koye4GE3x7n*A_k?7A{eSt{(RznB+ivxTnZJ0J?5e<L#h><W&2RbY?mV&8^*=wU
ze)#m?R$J{~YlKbdw<Y_(Y>244xXAU{^!+#9f4|MMt-|~He@|JUD09bQJ^7BRUq94u
zJX%)yzyIjPgOBW!91gwyq5iesJ}zJ5uY6Hq`HA~8H@{5y*fnSCe}D0^e>bBxJ&CWr
zI&)Iw|LtFOf0#!V^+wrbeOz&Lu~<KLhQSsI27L$pZE=(CF9Lbg)78&qol`;+0CSsy
Ae*gdg

literal 0
HcmV?d00001

diff --git a/tests/suite/layout/flow/flow.typ b/tests/suite/layout/flow/flow.typ
index fcbc005b6..5a5e84044 100644
--- a/tests/suite/layout/flow/flow.typ
+++ b/tests/suite/layout/flow/flow.typ
@@ -76,3 +76,7 @@ Hi
   #block(rect(width: 80%, height: 80pt), breakable: false)
   #lines(6)
 ]
+
+--- issue-5024-spill-backlog ---
+#set page(columns: 2, height: 50pt)
+#columns(2)[Hello]