Fix grid-cell misalignment bug (#963) (#978)

Having a table span multiple pages can cause an alignment bug in the
table itself. If the first region in a cell in a grid row is empty, all
other cells in this row will skip the first region. A misalignment bug
can occur, since the calculation of all region sizes happen before the
skip. The overall size allocated for the content of a cell with multiple
regions and content in the first region thus is too little.
Fixes #963

library/src/layout/grid.rs

@@ -450,33 +450,15 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
     /// Layout a row with automatic height. Such a row may break across multiple
     /// regions.
     fn layout_auto_row(&mut self, y: usize) -> SourceResult<()> {
-        let mut resolved: Vec<Abs> = vec![];
-        let mut skip = false;
-
-        // Determine the size for each region of the row.
-        for (x, &rcol) in self.rcols.iter().enumerate() {
-            if let Some(cell) = self.cell(x, y) {
-                let mut pod = self.regions;
-                pod.size.x = rcol;
-
-                let frames = cell.measure(self.vt, self.styles, pod)?.into_frames();
-                if let [first, rest @ ..] = frames.as_slice() {
-                    skip |=
-                        first.is_empty() && rest.iter().any(|frame| !frame.is_empty());
-                }
-
-                // For each region, we want to know the maximum height any
-                // column requires.
-                let mut sizes = frames.iter().map(|frame| frame.height());
-                for (target, size) in resolved.iter_mut().zip(&mut sizes) {
-                    target.set_max(size);
-                }
-
-                // New heights are maximal by virtue of being new. Note that
-                // this extend only uses the rest of the sizes iterator.
-                resolved.extend(sizes);
+        // Determine the size for each region of the row. If the first region
+        // ends up empty for some column, skip the region and remeasure.
+        let mut resolved = match self.measure_auto_row(y, true)? {
+            Some(resolved) => resolved,
+            None => {
+                self.finish_region()?;
+                self.measure_auto_row(y, false)?.unwrap()
             }
-        }
+        };
 
         // Nothing to layout.
         if resolved.is_empty() {
@@ -490,12 +472,6 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
             return Ok(());
         }
 
-        // Skip the first region if it's empty for some cell.
-        if skip && !self.regions.in_last() {
-            self.finish_region()?;
-            resolved.remove(0);
-        }
-
         // Expand all but the last region.
         // Skip the first region if the space is eaten up by an fr row.
         let len = resolved.len();
@@ -521,6 +497,47 @@ impl<'a, 'v> GridLayouter<'a, 'v> {
         Ok(())
     }
 
+    /// Measure the regions sizes of an auto row. The option is always `Some(_)`
+    /// if `can_skip` is false.
+    fn measure_auto_row(
+        &mut self,
+        y: usize,
+        can_skip: bool,
+    ) -> SourceResult<Option<Vec<Abs>>> {
+        let mut resolved: Vec<Abs> = vec![];
+
+        for (x, &rcol) in self.rcols.iter().enumerate() {
+            if let Some(cell) = self.cell(x, y) {
+                let mut pod = self.regions;
+                pod.size.x = rcol;
+
+                let frames = cell.measure(self.vt, self.styles, pod)?.into_frames();
+
+                // Skip the first region if one cell in it is empty. Then,
+                // remeasure.
+                if let [first, rest @ ..] = frames.as_slice() {
+                    if can_skip
+                        && first.is_empty()
+                        && rest.iter().any(|frame| !frame.is_empty())
+                    {
+                        return Ok(None);
+                    }
+                }
+
+                let mut sizes = frames.iter().map(|frame| frame.height());
+                for (target, size) in resolved.iter_mut().zip(&mut sizes) {
+                    target.set_max(size);
+                }
+
+                // New heights are maximal by virtue of being new. Note that
+                // this extend only uses the rest of the sizes iterator.
+                resolved.extend(sizes);
+            }
+        }
+
+        Ok(Some(resolved))
+    }
+
     /// Layout a row with relative height. Such a row cannot break across
     /// multiple regions, but it may force a region break.
     fn layout_relative_row(&mut self, v: Rel<Length>, y: usize) -> SourceResult<()> {

tests/typ/bugs/grid-2.typ

@@ -9,7 +9,9 @@
   rect(width: 100%, fill: red),
   rect(width: 100%, fill: blue),
   rect(width: 100%, height: 80%, fill: green),
-  [Hello],
+  [hello \ darkness #parbreak my \ old \ friend \ I],
+  rect(width: 100%, height: 20%, fill: blue),
+  polygon(fill: red, (0%, 0%), (100%, 0%), (100%, 20%))
 )
 
 ---