From 443cf60ae21d7d638a3279b64e22f6beb3a5f1b0 Mon Sep 17 00:00:00 2001
From: PgBiel <9021226+PgBiel@users.noreply.github.com>
Date: Mon, 11 Mar 2024 07:32:44 -0300
Subject: [PATCH] Ensure hline/vline positioning respects colspan/rowspan
 (#3610)

---
 crates/typst/src/layout/grid/layout.rs |  22 +++++++++++++++++++---
 tests/ref/layout/grid-stroke.png       | Bin 55633 -> 57339 bytes
 tests/typ/layout/grid-stroke.typ       |  13 +++++++++++++
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/crates/typst/src/layout/grid/layout.rs b/crates/typst/src/layout/grid/layout.rs
index 772c107e8..20b2ddc8f 100644
--- a/crates/typst/src/layout/grid/layout.rs
+++ b/crates/typst/src/layout/grid/layout.rs
@@ -646,6 +646,8 @@ impl CellGrid {
                     ResolvableGridItem::Cell(cell) => cell,
                 };
                 let cell_span = cell.span();
+                let colspan = cell.colspan(styles).get();
+                let rowspan = cell.rowspan(styles).get();
                 // Let's calculate the cell's final position based on its
                 // requested position.
                 let resolved_index = {
@@ -654,6 +656,8 @@ impl CellGrid {
                     resolve_cell_position(
                         cell_x,
                         cell_y,
+                        colspan,
+                        rowspan,
                         &resolved_cells,
                         &mut auto_index,
                         min_auto_index,
@@ -663,8 +667,6 @@ impl CellGrid {
                 };
                 let x = resolved_index % c;
                 let y = resolved_index / c;
-                let colspan = cell.colspan(styles).get();
-                let rowspan = cell.rowspan(styles).get();
 
                 if colspan > c - x {
                     bail!(
@@ -1284,9 +1286,12 @@ impl CellGrid {
 /// positioning. Useful with headers: if a cell in a header has automatic
 /// positioning, it should start at the header's first row, and not at the end
 /// of the previous row.
+#[allow(clippy::too_many_arguments)]
 fn resolve_cell_position(
     cell_x: Smart<usize>,
     cell_y: Smart<usize>,
+    colspan: usize,
+    rowspan: usize,
     resolved_cells: &[Option<Entry>],
     auto_index: &mut usize,
     min_auto_index: usize,
@@ -1316,7 +1321,18 @@ fn resolve_cell_position(
 
             // Ensure the next cell with automatic position will be
             // placed after this one (maybe not immediately after).
-            *auto_index = resolved_index + 1;
+            //
+            // The calculation below also affects the position of the upcoming
+            // automatically-positioned lines.
+            *auto_index = if colspan == columns {
+                // The cell occupies all columns, so no cells can be placed
+                // after it until all of its rows have been spanned.
+                resolved_index + colspan * rowspan
+            } else {
+                // The next cell will have to be placed at least after its
+                // spanned columns.
+                resolved_index + colspan
+            };
 
             Ok(resolved_index)
         }
diff --git a/tests/ref/layout/grid-stroke.png b/tests/ref/layout/grid-stroke.png
index 409d10f10ec964692e8c0a4b254895b684ef27ff..e9f9b0615065d4ae08bf872f24e8be162dc4ea32 100644
GIT binary patch
delta 2051
zcmV+e2>kcavIG0S1CSRErw9T7000?6)M)?!-@>saMb>{m_U@QP$v6(+_;Q1T_yXEQ
zTyn5Y$3Tc2dKFEPP_6BP<V`IN-4-XvIf>p-!%Z%E;ShL%;b6E3($u6#KRI}C_u+`^
z^W~3tAPUFxIsg8Aj;GyD5yNmeOis|qWHM<@lj`+)JbACzOFsEDo6X}fluD)fe4hN+
zXf)zgVy=I1xI>FyQGA|;;l5$mZnu?6C7aF0JTay*tyU|S%jNU=7$fd>yCX4lyWQA;
z6$*veTEvK^TrP))w7zFpE|>8>b8>PLQ~z467AF*~*Xv_2#G7QiEyRHy-?)uNqr_OT
zSUi*vuN9A}R;w{)fZ_gNNY2I?jYfmPASU;>JEc0fpBz}nV2I;221julTm12OyxDBh
z{RTR?PN&m;zn}aqKL$f=ieiu&PZ+nW)#`A%klc?QR7{y}ACsZiH(7c=$-e79I6Hg(
z<8_Mb>GiX~@apPG9Hw`%P9N?uOgb>QSc!|3xLC=*hT)>KUz6+9*T?g(v95mpNqt@1
z{CYdSJpKM9_Dd<Q%SHFoP5axd6I++NSd;UUG1&iU{bk?&S<?vP0FE~aWXX{^WJ-pR
zWJ<C$WC*lm$r3^!sJN)ORHrIV4i*Ga5ZtVbf;cFMB8Uim0fNI}bu1!?h$#M>L=^S<
zz`cM%v_#|ggUEBsE#4QOpWXog000000001h+cAL6LX(l89Ft(64S%q--?H-cv-16$
z@-IJr^*%ZNh<84C5Cbt_J7O4*$H`>U?RJ|?CSt%=$1s^pLZQ&X!NK|YIWd53l5%%<
zcXf3|3?K&O{{H^@`kEL(48(v<j-k<Lc)ebm%@&KrMx)V6VraM9KA+FwaO841V!&p^
zkWQyBE-uRDa<y6&*MITj<KuijUor+sk_-kzDwXnhJp23mMx&7!umLgL+}vn18g-q|
z=XZ8?N~O}0F&rHo&1N(8Q1r07yPHTPhym*!L#0ybcDw4jSS*Sd#Aa!QGef7-5i#iX
zdSbu^%nbE<-Q{w<yu4_&T5;h9gTd7f(2^uQJUkEs);)%3G=F-0d>oI*#ky9jtvrV5
zbh@{<XSG_10c#yYtJM+%u6m{Zrx;{e7U#@lGQ@zjjzP@IVn}d0o$6sY92SRuzkkUX
zE-x>|LM5j2r>7@=O~SgzFq_S8Z*L_@vREueqtWN{9UdN@o}RwGzAhGvx5Xe%cye;0
z)9E6S$n*2F*?(*vjYh<P4VW1wlS!da$mMdqUQgXAiFLo<fAcqkEX(3uoHLuvHk(cU
zfd<bra61Nwffzsx!~kL-1`q==fEb7Y#6S!n24cX6j6qzH@A&%iv%v=sVju>5tZh<0
zpAQ5A;`xW+aL6_ZTOC6>ojyA|6E|iL1_NTiwhz#DyMJB8FdmPI0gCdsy}Jp8Eer!V
z-g{zLOt#8KC<&V>-IAoc!%m&d)`Bt%ixcvZkyx1Q?!r!tk7gwiw(bT^<YN}pL`+F6
z#=^(g(9C`6<TUrX%<V4joipeE&tfPnoZp=D9`ioqYzzYf1A;+b@d^e40~jbrF>G#b
z4h|0X_kZ`VtgL)AH|go=N%rd)h9T2mWY^9o7!rxZ;^N}S$cRjciO1t$Alq#2(RIDE
zvojWpt*x!)=jU5nqSHG#IH<0!o}HbQ-9N!#(Cv1AgdrM@dOV(`rKOXT6PZn4Sy_2=
za{~sl0S0;gE-x=nCX?2t&8DWN)X&L6S*FZSO@B?L-tzf;zr`SDmAbmRv9U4h8}st=
z{C+<eDBCf-yu1_^7WVY?q~3abd^8X7^ycU1HBFOe`iyG^+3Km3>gwte55i1tI?1Vq
zfdFI!403!cr6wjO{^Ulg4BpF#L6#Frsnylhy}i9~IJ~>NyS=@Ad3gy2%61HqNJJ@R
zo_`rWNuKF7O}n_b$cSNRXh<m~0Jxh(HpB4z{4AF-H8nM<PYVPB)*vu5!!YD44-XH&
zV2~BIVCe1b1q0a(gCzHk(g%lKU0wC{^}T-shk}BF`uch~6`12>N=r)xgRCWF#cW|P
zmmI~##j+0B+S;;)_Ju+rFpv!}+~42px__=|njGt`t*vq|NRIW?yXNNR<Pfi|t@V1n
z-QC^e<KuFhsG*@@d3iY<gS5ZDUr|xva=B#Ka7#;zOcRq|0S2-G2D9_?^Y!)h)6>(`
z$KBrE%Ij65(WpF@e;rsimn;nF!-kKJj^qbkUte>XMC^29%|KuP1Azey1O_k=7?TOQ
z5`Tf=Zvz8?0Sp8NFc28PKwtm^<unG_$Tm4SX&A=B!a^dE00TJ`gN)R!s;W9ZK0Z4;
zYiw+6Zf=&HoM0d)V(@r8?d|Q>8!srO4i68(Ku*LEiA1b{Na1i;Fv!Ic7|4kjq*yFA
zIy&0c*5>tkHBFnIo(2Os5koK-EGa1og<?YH3>YwwLovuuBbUo%j@<?WIU0kkm6cM%
z!^7sMGcz;hKrt|o<1oCwzRC=Gx7+RW`8ql}0)aqLQBhf0nXc=}WD*SIxHl`=+1c6H
z*oepD=9Q8>zPr2Q%s{*xgO_6vlaaa<lMlKKlMcEKlMcEKlMcEE2!VkJ3`Af61Cs%|
z8W({943t9{zJB?SAK(4o_`?$zz(8Ck5t5u2lm3whlQ6mnlaN~lDOrwzh=_=Yh=_=Y
hh=_=Yh=}MP^aI2z4Hb-@8<GG3002ovPDHLkV1hE6><j<^

delta 485
zcmV<B0UG}MzXQ>-1CSRE5eNbR007m|Bmn>b*%+}UMb>}6_U@QP$v6(+_~PIozJN9n
zmmF-<F%Tk$UPV(RRBO8+c~eV6x5Wu^PJ%boaFa`3I0Rl`I2bO1G&L#GPYxd3eK^8C
zU;c;(qHsK)^Y72+csiXFF${;p<OGduHk;NosnKY}llS}m<daXU)jA$Sxm=#l=gE)F
zW;0$T<_dp@JGA%}#ph`lP7T9$yRBBMxm+&hi7}07x7+!AzECK{7;(4T9f_gW>%|7F
zSS-fYB1SZoN+mp`^*zIKxs3OjOePak|9ZV1ClsyM>tis)n`FE##DN~)xQ#}m#8|0R
zI+PHv6_2UaYB6Sj;dC%0XXA`Uqd1)|CiizcrMQ!6*BdIqV37PRKL$f=ieiu&PZ+nW
z)#`A%klc?QR7{!f9y6QGU~oEy|7UOtH<NI~8k5M^5oCTp$-NsqI6r^><8_Mb>CLmi
z@ap<W9H#fN&K~YDOu8_*Sc!|3xLC=*hT*ciUz3~E*T;*mv95pqNqt@3{(3vUI{W@5
z_Dd<Qt3~hAZRgvp8(WwASd)vrgx?DoT;NbHa0rvo*dLQ`#3+-5*sUQ+k|arzBuSDa
b=_L9KZ)EUe9IjZ)00000NkvXXu0mjfILYkM

diff --git a/tests/typ/layout/grid-stroke.typ b/tests/typ/layout/grid-stroke.typ
index 9d01e1cd1..7a01b52d6 100644
--- a/tests/typ/layout/grid-stroke.typ
+++ b/tests/typ/layout/grid-stroke.typ
@@ -288,6 +288,19 @@
   table.hline(stroke: blue, start: 1, end: 2),
 )
 
+---
+// - Vline should be placed after the colspan.
+// - Hline should be placed under the full-width rowspan.
+#table(
+  columns: 3,
+  rows: 1.25em,
+  inset: 1pt,
+  stroke: none,
+  table.cell(colspan: 2)[a], table.vline(stroke: red), table.hline(stroke: blue), [b],
+  [c], [d], [e],
+  table.cell(colspan: 3, rowspan: 2)[a], table.vline(stroke: blue), table.hline(stroke: red)
+)
+
 ---
 // Error: 8:3-8:32 cannot place horizontal line at the 'bottom' position of the bottom border (y = 2)
 // Hint: 8:3-8:32 set the line's position to 'top' or place it at a smaller 'y' index