From 4103be5138e341a9b7875e57379f56602f338528 Mon Sep 17 00:00:00 2001 From: PgBiel <9021226+PgBiel@users.noreply.github.com> Date: Thu, 20 Feb 2025 02:52:11 -0300 Subject: [PATCH] check for conflict between row-pos cells and headers/footers --- .../typst-library/src/layout/grid/resolve.rs | 94 ++++++------------ tests/ref/grid-footer-expand.png | Bin 365 -> 0 bytes ...rid-footer-moved-to-bottom-of-rowspans.png | Bin 0 -> 1182 bytes tests/ref/grid-header-expand.png | Bin 2005 -> 0 bytes tests/suite/layout/grid/footers.typ | 30 ++++-- tests/suite/layout/grid/headers.typ | 5 +- 6 files changed, 54 insertions(+), 75 deletions(-) delete mode 100644 tests/ref/grid-footer-expand.png create mode 100644 tests/ref/grid-footer-moved-to-bottom-of-rowspans.png delete mode 100644 tests/ref/grid-header-expand.png diff --git a/crates/typst-library/src/layout/grid/resolve.rs b/crates/typst-library/src/layout/grid/resolve.rs index 5d9c788eb..a742898f8 100644 --- a/crates/typst-library/src/layout/grid/resolve.rs +++ b/crates/typst-library/src/layout/grid/resolve.rs @@ -1134,6 +1134,34 @@ impl<'a> CellGrid<'a> { child_start = new_child_start; child_end = new_child_end; + } else { + // TODO: Perhaps do this during cell resolving? + // This is unnecessary for non-row-positioned cells, as + // they skip headers and footers already. + if let Some(header) = &header { + // TODO: check start (right now zero, always satisfied) + if y < header.end { + bail!( + cell_span, + "cell would conflict with header spanning the same position"; + hint: "try moving the cell or the header" + ); + } + } + if let Some((footer_end, _, footer)) = &footer { + // NOTE: y + rowspan >, not >=, footer.start, to + // check if the rowspan enters the footer. For example, + // rowspan of 1: y + 1 = footer.start means + // `y < footer.start`, and it only occupies one row + // (`y`), so the cell is safe. + if y < *footer_end && y + rowspan > footer.start { + bail!( + cell_span, + "cell would conflict with footer spanning the same position"; + hint: "try reducing the cell's rowspan or moving the footer" + ); + } + } } if largest_index >= resolved_cells.len() { @@ -1278,23 +1306,6 @@ impl<'a> CellGrid<'a> { )); } - if is_header || is_footer { - // Next automatically positioned cell goes under this header. - // FIXME: Consider only doing this if the header has any fully - // automatically positioned cells. Otherwise, - // `resolve_cell_position` should be smart enough to skip - // upcoming headers. - // Additionally, consider that cells with just an 'x' override - // could end up going too far back and making previous - // non-header rows into header rows (maybe they should be - // placed at the first row that is fully empty or something). - // Nothing we can do when both 'x' and 'y' were overridden, of - // course. - // None of the above are concerns for now, as headers must - // start at the first row. - local_auto_index = local_auto_index.max(c * child_end); - } - if !is_row_group { // The child was a single cell outside headers or footers. // Therefore, 'local_auto_index' for this table child was @@ -1326,51 +1337,6 @@ impl<'a> CellGrid<'a> { .enumerate() .map(|(i, cell)| { if let Some(cell) = cell { - if let Some(parent_cell) = cell.as_cell() { - if let Some(header) = &mut header - { - let y = i / c; - if y < header.end { - // Ensure the header expands enough such that - // all cells inside it, even those added later, - // are fully contained within the header. - // FIXME: check if start < y < end when start can - // be != 0. - // FIXME: when start can be != 0, decide what - // happens when a cell after the header placed - // above it tries to span the header (either - // error or expand upwards). - header.end = header.end.max(y + parent_cell.rowspan.get()); - } - } - - if let Some((end, footer_span, footer)) = &mut footer { - let x = i % c; - let y = i / c; - let cell_end = y + parent_cell.rowspan.get(); - if y < footer.start && cell_end > footer.start { - // Don't allow a cell before the footer to span - // it. Surely, we could move the footer to - // start at where this cell starts, so this is - // more of a design choice, as it's unlikely - // for the user to intentionally include a cell - // before the footer spanning it but not - // being repeated with it. - bail!( - *footer_span, - "footer would conflict with a cell placed before it at column {x} row {y}"; - hint: "try reducing that cell's rowspan or moving the footer" - ); - } - if y >= footer.start && y < *end { - // Expand the footer to include all rows - // spanned by this cell, as it is inside the - // footer. - *end = (*end).max(cell_end); - } - } - } - Ok(cell) } else { let x = i % c; @@ -1533,10 +1499,6 @@ impl<'a> CellGrid<'a> { } } - if header_end.is_some_and(|header_end| header_end > footer.start) { - bail!(footer_span, "header and footer must not have common rows"); - } - Ok(footer) }) .transpose()? diff --git a/tests/ref/grid-footer-expand.png b/tests/ref/grid-footer-expand.png deleted file mode 100644 index 6b173b0da98c6b54bcf934846505bd3948b575b1..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 365 zcmV-z0h0cSP)-VsVN4&Z3(G zVez5-0ZP>;@}K=47>ga(yS*3~AOD};KR^~wSTzt9A9FgC`s2$(`dj?z&82_;dtwKu zlDM8ge~bTa?s@Y6hUd)1|L7gZr_Sg7zv1`4XfvHH{;_bL&;J_<|2J3C+2Z!?uRK8( zw;Z6e#rsR=s&BiKI(5zfr0UGW1JmaERWcwJf2&xkGawfCulrv-AQl%d|35H6y~FFg zQ~7`-u9MrY+#gw8n9Hl54T#0p>VE4Fh{d&4i>CY?2#Y7R4^SMxatNs&n8cO+b^d@@ z{LgKGf_k&Ys)4cio8$ixX7NkY0kU{Q+p*Dcu2G9eEgrRaC|C>t-y~FZIbppJ00000 LNkvXXu0mjfBuBJ8 diff --git a/tests/ref/grid-footer-moved-to-bottom-of-rowspans.png b/tests/ref/grid-footer-moved-to-bottom-of-rowspans.png new file mode 100644 index 0000000000000000000000000000000000000000..d8a9c74f82b100a9ec4147e1b738f2517ac8bf6f GIT binary patch literal 1182 zcmV;P1Y!G$P)fzPkT{d;i?p|72VLw6On+h1uEJjEs!N#>W4Uipa>wZ*OlwK|#aA z!`9Z;!otG%`1q=-s%2$m=H}+VzrTlvhyRw4|6W!9dUF4Te*b%OkdTmcbaX;OLhS79 z^78W3)YQ__(tdt^`}_NPdV22e?rCXh|7BeI`ue4%rSZ$qN1Yz zfO!9adH-r*m6es0l$0+oFRQDo)6>)cii7`|lmCr}uCA{CkBFR{od1l5|8r~qT2O?9 zgr=sZ|42RX@bGJEYyY8}|4BdJ-`{t4cfi2Fd3kyNgMI&IUH?={|8Z%jr>FmQZU1az z|Av46i-iA|lK**f|5Hi-j)=RvyMls(xw*OB-QBRTu!e?)H8nNq>FNK9g8!9||9o}- zSWZVrM@vgfWMpL3)zyH2fYH&>OiWB}Zf>5Qp2^9{wzjrPN=nbq&&tZm|B8hFuBoA+ zq5p7b|A>PBtf>EQXScVv|4Baon3DgTm$gwv@;o)6fU8AF;nVFgN^z=?nPDx2g|6*AGfqJj6uP7)e=;-K1 zMn;#Hm;ZKd|8s2ra%yvPbB~XY|A>MAVOalucmIQY|B{T?*Vo+K++bi}*x1-|a&q?e z_WyKk{r&y3v$Ox9od24Y|DT%wotOWWj{l~i{{H^|Ze{<{&;LX@|4>H%@bCXdJpcLm z|DBlsqMaL+IwSx90uxC@K~#9!?c3E;8&MR;@r8Puq);4Mpil}_pziMO?#12R-QC@t zAVGt}f5yNI_wv9D1G_(1&iC!?%V)B8?wRBv0!2~O#*1&$Vf+4#8{48qgX3a#*}i<` z#)K%*U>#j<+_jq<=NAYjc)P;RP62@s_`KTR4+7g-$M`~E`#=AEAh6xve|?AOIuX3N z4J1voy?VxtIoYDYDakWzFIC)Vkws@L7g(VH!G?Mu_^_U-0|wTwsjtI)VxgfG1X~*; zV5Nhc~o%VXTw(IpyOUA33(%u#M^@1!#(nBaB7gBnF91Xe~y z1Vdn>n2@05@s8-5i-*k5G6&;UB;Ev02=g3jXkB&!c6S58F0M-2T(X5nM(5bZ$GCBLNHn;4YL;zk w)i^+BOfbO&Z@tacwD#s|r2j`z6tz`a1W(L`M#F}Q_5c6?07*qoM6N<$f_BrFEdT%j literal 0 HcmV?d00001 diff --git a/tests/ref/grid-header-expand.png b/tests/ref/grid-header-expand.png deleted file mode 100644 index d0fbd72ed23d726d44eef215fabe6c874199d52e..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 2005 zcmV;`2P*i9P)?)!Uu-tr7rRN#HrqX7>#EyhYZt|-%C_FFS{v(y1&gGJXKhswYEe#+3JQXv zj3^A3TwVkL5flLx5G?n|DJTPS4>Jrf!yMn$kkkpmMqq@sneUrPCjaDfIu&*2B-Z~XrT zcV0qEwl@AMbm?ABDGcp zc9oL|v67m(hE5Cl;pm89?~AAg!&XJOo|P;DLDV6UY(5dkF@FT?%x_&rvnjJfH3%sG z8v6`pQfWOR*x?img6~+|;OH62YWgPvvc*IlXMNoO{5gtQiVnwbFy$$njex4JMg*5k zkBxl>_ikC4xz96qS&pSF_FqkGvr8klYGW`kk1cjol>^vN{M@}E6ZY>Cs+K} zziGmQn2)c(Di zb(^@QPONzO15!J0oR%y0(7ji9GPkdZ(eL-IV+F7Kz+AuIHjeYZQ|fL2wEGenl;Y*W z>gfP*>J)>zT*oK7$N}0^s1#}w1%R>*x{yLAAcsf~<*4EK z-~ba6-9zo4!UrPlk{YLa>;_z~t?K?yQpw~vB_u^`&25}|!hNC8>Jl!=mW|(UxHW2c zHVBxZWkIDVsM4IS53tM!u<@4SoQeRTvEAJn%W_P~aFWc6+?ILXhta~izvvJ!sTcDi zQBc7OwC`hC3n2CT^k+Es)J^-RUkk`|%v%W<0LJ`8_@DB8c2%#@I_~aYcIlj-V$OMh z?a~eXU;1KlSq0|lo@*b?Noo4CEO@Q@!}Dp)X&bVWuZ|K{-)t|22kHI&nVCx6ogyA* zWdqzz?v!QTq_hg?%;?K%&MK-`u@^YtB|T+3bw`f4cb?4xU3#*Y><8VAU#O0(2&Kq{S;DRu-+3(A2)1f|U`U zm;1(%co6$+-s{C+u=Xt-tjWRuWYt$(I;^c*?RDW>z@!15z|nTV6Zne9Q&@oEVAgsU zmtQvTSTD&~qC5F*NqEaP*_3;?WgfN_w|iz}bxbL|)h%=`DPLN=z_I5|sR3bC;&rHr zmXLa(E#X3I{AC4o5fzYeQA<(x{u8;pShxHcvU0Z( z!FYz^sRICaU?8n046b_`wNZ%W3`#q7WN?s(U#cRp%A( z#xUV5rA6MgLj;QWSeInSIVViB_bO(F#pCAQnNO?^}@|K?c9TRFpmNslgy(%u#p-MlAEN%`Vz4!4C_ zH$|DKW6r2kh55m!yM!W{t>Y$dKp_lMUKidC5jK~eQHSP(EI77bYTLL!@|9EHM(s1x1oz|53eV8^u z*^;P_%A$RltyY;~d!-uZ5I*d!x6qo=`;UQ3gTcDDLBKm&Ebl_7n@&TVFJ@}B8Lb=F z;g*7c$6vHhhmRC*8}?BP^7H@JY<-ReeNZ<|{-`I!oz>-;lUn^T@7^2F`^uWd%=$^T zc;M1-aD!Ktu)aSms1D$oTe{lUNA}=w-pPirh^#K}(_+u81h0qEptGF=m&WBX!oNoN zaU1~LPaPfHC*v2(xz4i=0Ct+41D&^L$B$>g%wL3isUB( z3kxg~Hpd0pN+b6A@2n%;&!@h1ZL|&G5dPq^l&yiteU7AOtVbFU@FpkSC=YU)C)@aw nc)2#n)9cHJmG7Kw8*Ka!kjmohl`Q2d00000NkvXXu0mjfvAN$l diff --git a/tests/suite/layout/grid/footers.typ b/tests/suite/layout/grid/footers.typ index 2d4ce31b9..0cfbd0f0d 100644 --- a/tests/suite/layout/grid/footers.typ +++ b/tests/suite/layout/grid/footers.typ @@ -95,12 +95,28 @@ grid.cell(x: 1)[c] ) ---- grid-footer-expand --- -// Ensure footer properly expands +--- grid-footer-no-expand --- #grid( columns: 2, [a], [], [b], [], + fill: (_, y) => if calc.odd(y) { blue } else { red }, + inset: 5pt, + grid.cell(x: 1, y: 3, rowspan: 4)[b], + grid.cell(y: 2, rowspan: 2)[a], + grid.footer(), + // Error: 3-33 cell would conflict with footer spanning the same position + // Hint: 3-33 try reducing the cell's rowspan or moving the footer + grid.cell(y: 6, rowspan: 2)[d], +) + +--- grid-footer-moved-to-bottom-of-rowspans --- +#grid( + columns: 2, + [a], [], + [b], [], + stroke: red, + inset: 5pt, grid.cell(x: 1, y: 3, rowspan: 4)[b], grid.cell(y: 2, rowspan: 2)[a], grid.footer(), @@ -125,13 +141,13 @@ ) --- grid-footer-overlap --- -// Error: 4:3-4:19 footer would conflict with a cell placed before it at column 1 row 0 -// Hint: 4:3-4:19 try reducing that cell's rowspan or moving the footer #grid( columns: 2, grid.header(), - grid.footer([a]), - grid.cell(x: 1, y: 0, rowspan: 2)[a], + grid.footer(grid.cell(y: 2)[a]), + // Error: 3-39 cell would conflict with footer spanning the same position + // Hint: 3-39 try reducing the cell's rowspan or moving the footer + grid.cell(x: 1, y: 1, rowspan: 2)[a], ) --- grid-footer-multiple --- @@ -386,8 +402,8 @@ table.hline(stroke: red), table.vline(stroke: green), [b], + [c] ), - table.cell(x: 1, y: 3)[c] ) --- grid-footer-hline-and-vline-2 --- diff --git a/tests/suite/layout/grid/headers.typ b/tests/suite/layout/grid/headers.typ index eb329cb71..b83ec8b11 100644 --- a/tests/suite/layout/grid/headers.typ +++ b/tests/suite/layout/grid/headers.typ @@ -284,8 +284,7 @@ ) #context count.display() ---- grid-header-expand --- -// Ensure header expands to fit cell placed in it after its declaration +--- grid-header-no-expand --- #set page(height: 10em) #table( columns: 2, @@ -293,6 +292,8 @@ [a], [b], [c], ), + // Error: 3-48 cell would conflict with header spanning the same position + // Hint: 3-48 try moving the cell or the header table.cell(x: 1, y: 1, rowspan: 2, lorem(80)) )