diff --git a/crates/typst/src/layout/grid/layout.rs b/crates/typst/src/layout/grid/layout.rs index 469939891..6dbe151e6 100644 --- a/crates/typst/src/layout/grid/layout.rs +++ b/crates/typst/src/layout/grid/layout.rs @@ -427,7 +427,10 @@ impl CellGrid { // validity, since the amount of rows isn't known until all items were // analyzed in the for loop below. // We keep their spans so we can report errors later. - let mut pending_hlines: Vec<(Span, Line)> = vec![]; + // The additional boolean indicates whether the hline had an automatic + // 'y' index, and is used to change the index of hlines at the top of a + // header or footer. + let mut pending_hlines: Vec<(Span, Line, bool)> = vec![]; // For consistency, only push vertical lines later as well. let mut pending_vlines: Vec<(Span, Line)> = vec![]; @@ -494,7 +497,9 @@ impl CellGrid { let mut child_start = usize::MAX; let mut child_end = 0; let mut child_span = Span::detached(); - let mut min_auto_index = 0; + let mut start_new_row = false; + let mut first_index_of_top_hlines = usize::MAX; + let mut first_index_of_non_top_hlines = usize::MAX; let (header_footer_items, simple_item) = match child { ResolvableGridChild::Header { repeat, span, items, .. } => { @@ -512,7 +517,11 @@ impl CellGrid { // that row instead of starting a new one. // FIXME: Revise this approach when headers can start from // arbitrary rows. - min_auto_index = auto_index.next_multiple_of(c); + start_new_row = true; + + // Any hlines at the top of the header will start at this + // index. + first_index_of_top_hlines = pending_hlines.len(); (Some(items), None) } @@ -529,7 +538,11 @@ impl CellGrid { // have it skip to the next row. This is to avoid having a // footer after a partially filled row just add cells to // that row instead of starting a new one. - min_auto_index = auto_index.next_multiple_of(c); + start_new_row = true; + + // Any hlines at the top of the footer will start at this + // index. + first_index_of_top_hlines = pending_hlines.len(); (Some(items), None) } @@ -550,7 +563,18 @@ impl CellGrid { span, position, } => { + let has_auto_y = y.is_auto(); let y = y.unwrap_or_else(|| { + // Avoid placing the hline inside consecutive + // rowspans occupying all columns, as it'd just + // disappear, at least when there's no column + // gutter. + skip_auto_index_through_fully_merged_rows( + &resolved_cells, + &mut auto_index, + c, + ); + // When no 'y' is specified for the hline, we place // it under the latest automatically positioned // cell. @@ -572,7 +596,6 @@ impl CellGrid { // the start of a header will always appear above // that header's first row. Similarly for footers. auto_index - .max(min_auto_index) .checked_sub(1) .map_or(0, |last_auto_index| last_auto_index / c + 1) }); @@ -594,7 +617,7 @@ impl CellGrid { // one "wins" in case of conflict. Pushing the current // hline before we push pending hlines later would // change their order! - pending_hlines.push((span, line)); + pending_hlines.push((span, line, has_auto_y)); continue; } ResolvableGridItem::VLine { @@ -620,17 +643,15 @@ impl CellGrid { // to the left of the table. // // Exceptionally, a vline is also placed to the - // left of the table if the current auto index from - // past iterations is smaller than the minimum auto - // index. For example, this means that a vline at + // left of the table if we should start a new row + // for the next automatically positioned cell. + // For example, this means that a vline at // the beginning of a header will be placed to its // left rather than after the previous // automatically positioned cell. Same for footers. auto_index .checked_sub(1) - .filter(|last_auto_index| { - last_auto_index >= &min_auto_index - }) + .filter(|_| !start_new_row) .map_or(0, |last_auto_index| last_auto_index % c + 1) }); if end.is_some_and(|end| end.get() < start) { @@ -661,7 +682,7 @@ impl CellGrid { rowspan, &resolved_cells, &mut auto_index, - min_auto_index, + &mut start_new_row, c, ) .at(cell_span)? @@ -778,12 +799,28 @@ impl CellGrid { // contained within it. child_start = child_start.min(y); child_end = child_end.max(y + rowspan); + + if start_new_row && child_start <= auto_index.div_ceil(c) { + // No need to start a new row as we already include + // the row of the next automatically positioned cell in + // the header or footer. + start_new_row = false; + } + + if !start_new_row { + // From now on, upcoming hlines won't be at the top of + // the child, as the first automatically positioned + // cell was placed. + first_index_of_non_top_hlines = + first_index_of_non_top_hlines.min(pending_hlines.len()); + } } } if (is_header || is_footer) && child_start == usize::MAX { // Empty header/footer: consider the header/footer to be - // one row after the latest auto index. + // at the next empty row after the latest auto index. + auto_index = find_next_empty_row(&resolved_cells, auto_index, c); child_start = auto_index.div_ceil(c); child_end = child_start + 1; @@ -830,6 +867,22 @@ impl CellGrid { } if is_header || is_footer { + let amount_hlines = pending_hlines.len(); + for (_, top_hline, has_auto_y) in pending_hlines + .get_mut( + first_index_of_top_hlines + ..first_index_of_non_top_hlines.min(amount_hlines), + ) + .unwrap_or(&mut []) + { + if *has_auto_y { + // Move this hline to the top of the child, as it was + // placed before the first automatically positioned cell + // and had an automatic index. + top_hline.index = child_start; + } + } + // Next automatically positioned cell goes under this header. // FIXME: Consider only doing this if the header has any fully // automatically positioned cells. Otherwise, @@ -944,7 +997,7 @@ impl CellGrid { let mut hlines: Vec> = vec![]; let row_amount = resolved_cells.len().div_ceil(c); - for (line_span, line) in pending_hlines { + for (line_span, line, _) in pending_hlines { let y = line.index; if y > row_amount { bail!(line_span, "cannot place horizontal line at invalid row {y}"); @@ -1293,11 +1346,10 @@ impl CellGrid { /// `(auto, auto)` cell) and the amount of columns in the grid, returns the /// final index of this cell in the vector of resolved cells. /// -/// The `min_auto_index` parameter is used to bump the auto index to that value -/// if it is currently smaller than it and a cell requests fully automatic -/// 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. +/// The `start_new_row` parameter is used to ensure that, if this cell is +/// fully automatically positioned, it should start a new, empty row. This is +/// useful for headers and footers, which must start at their own rows, without +/// interference from previous cells. #[allow(clippy::too_many_arguments)] fn resolve_cell_position( cell_x: Smart, @@ -1306,7 +1358,7 @@ fn resolve_cell_position( rowspan: usize, resolved_cells: &[Option], auto_index: &mut usize, - min_auto_index: usize, + start_new_row: &mut bool, columns: usize, ) -> HintedStrResult { // Translates a (x, y) position to the equivalent index in the final cell vector. @@ -1322,13 +1374,22 @@ fn resolve_cell_position( (Smart::Auto, Smart::Auto) => { // Let's find the first available position starting from the // automatic position counter, searching in row-major order. - let mut resolved_index = min_auto_index.max(*auto_index); - while let Some(Some(_)) = resolved_cells.get(resolved_index) { - // Skip any non-absent cell positions (`Some(None)`) to - // determine where this cell will be placed. An out of bounds - // position (thus `None`) is also a valid new position (only - // requires expanding the vector). - resolved_index += 1; + let mut resolved_index = *auto_index; + if *start_new_row { + resolved_index = + find_next_empty_row(resolved_cells, resolved_index, columns); + + // Next cell won't have to start a new row if we just did that, + // in principle. + *start_new_row = false; + } else { + while let Some(Some(_)) = resolved_cells.get(resolved_index) { + // Skip any non-absent cell positions (`Some(None)`) to + // determine where this cell will be placed. An out of + // bounds position (thus `None`) is also a valid new + // position (only requires expanding the vector). + resolved_index += 1; + } } // Ensure the next cell with automatic position will be @@ -1401,6 +1462,53 @@ fn resolve_cell_position( } } +/// Computes the index of the first cell in the next empty row in the grid, +/// starting with the given initial index. +fn find_next_empty_row( + resolved_cells: &[Option], + initial_index: usize, + columns: usize, +) -> usize { + let mut resolved_index = initial_index.next_multiple_of(columns); + while resolved_cells + .get(resolved_index..resolved_index + columns) + .is_some_and(|row| row.iter().any(Option::is_some)) + { + // Skip non-empty rows. + resolved_index += columns; + } + + resolved_index +} + +/// Fully merged rows under the cell of latest auto index indicate rowspans +/// occupying all columns, so we skip the auto index until the shortest rowspan +/// ends, such that, in the resulting row, we will be able to place an +/// automatically positioned cell - and, in particular, hlines under it. The +/// idea is that an auto hline will be placed after the shortest such rowspan. +/// Otherwise, the hline would just be placed under the first row of those +/// rowspans and disappear (except at the presence of column gutter). +fn skip_auto_index_through_fully_merged_rows( + resolved_cells: &[Option], + auto_index: &mut usize, + columns: usize, +) { + // If the auto index isn't currently at the start of a row, that means + // there's still at least one auto position left in the row, ignoring + // cells with manual positions, so we wouldn't have a problem in placing + // further cells or, in this case, hlines here. + if *auto_index % columns == 0 { + while resolved_cells + .get(*auto_index..*auto_index + columns) + .is_some_and(|row| { + row.iter().all(|entry| matches!(entry, Some(Entry::Merged { .. }))) + }) + { + *auto_index += columns; + } + } +} + /// Performs grid layout. pub struct GridLayouter<'a> { /// The grid of cells. diff --git a/tests/ref/layout/grid-footers-5.png b/tests/ref/layout/grid-footers-5.png index 0cfd2d668..6cae5592c 100644 Binary files a/tests/ref/layout/grid-footers-5.png and b/tests/ref/layout/grid-footers-5.png differ diff --git a/tests/ref/layout/grid-stroke.png b/tests/ref/layout/grid-stroke.png index e9f9b0615..fbba379e8 100644 Binary files a/tests/ref/layout/grid-stroke.png and b/tests/ref/layout/grid-stroke.png differ diff --git a/tests/typ/layout/grid-footers-5.typ b/tests/typ/layout/grid-footers-5.typ index 874fcd2e4..16da940a0 100644 --- a/tests/typ/layout/grid-footers-5.typ +++ b/tests/typ/layout/grid-footers-5.typ @@ -39,3 +39,49 @@ [c], [d] ) ) + +--- +// Footer should appear at the bottom. Red line should be above the footer. +// Green line should be on the left border. +#set page(margin: 2pt) +#set text(6pt) +#table( + columns: 2, + inset: 1.5pt, + table.cell(y: 0)[a], + table.cell(x: 1, y: 1)[a], + table.cell(y: 2)[a], + table.footer( + table.hline(stroke: red), + table.vline(stroke: green), + [b], + ), + table.cell(x: 1, y: 3)[c] +) + +--- +// Table should be just one row. [c] appears at the third column. +#set page(margin: 2pt) +#set text(6pt) +#table( + columns: 3, + inset: 1.5pt, + table.cell(y: 0)[a], + table.footer( + table.hline(stroke: red), + table.hline(y: 1, stroke: aqua), + table.cell(y: 0)[b], + [c] + ) +) + +--- +// Footer should go below the rowspans. +#set page(margin: 2pt) +#set text(6pt) +#table( + columns: 2, + inset: 1.5pt, + table.cell(rowspan: 2)[a], table.cell(rowspan: 2)[b], + table.footer() +) diff --git a/tests/typ/layout/grid-stroke.typ b/tests/typ/layout/grid-stroke.typ index 7a01b52d6..35a65ae64 100644 --- a/tests/typ/layout/grid-stroke.typ +++ b/tests/typ/layout/grid-stroke.typ @@ -301,6 +301,18 @@ table.cell(colspan: 3, rowspan: 2)[a], table.vline(stroke: blue), table.hline(stroke: red) ) +--- +// Red line should be above [c] (hline skips the shortest rowspan). +#set text(6pt) +#table( + rows: 1em, + columns: 2, + inset: 1.5pt, + table.cell(rowspan: 3)[a], table.cell(rowspan: 2)[b], + table.hline(stroke: red), + [c] +) + --- // 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