From b322da930fe35ee3d19896de6ab653e2f321e301 Mon Sep 17 00:00:00 2001 From: Tobias Schmitz Date: Tue, 6 May 2025 10:26:55 +0200 Subject: [PATCH] Respect RTL cell layouting order in grid layout (#6232) Co-authored-by: PgBiel <9021226+PgBiel@users.noreply.github.com> --- crates/typst-layout/src/grid/layouter.rs | 60 +++----- crates/typst-layout/src/grid/rowspans.rs | 16 +- tests/ref/grid-rtl-counter.png | Bin 0 -> 272 bytes tests/ref/grid-rtl-rowspan-counter-equal.png | Bin 0 -> 272 bytes .../ref/grid-rtl-rowspan-counter-mixed-1.png | Bin 0 -> 360 bytes .../ref/grid-rtl-rowspan-counter-mixed-2.png | Bin 0 -> 361 bytes .../grid-rtl-rowspan-counter-unequal-1.png | Bin 0 -> 361 bytes .../grid-rtl-rowspan-counter-unequal-2.png | Bin 0 -> 360 bytes tests/suite/layout/grid/rtl.typ | 140 ++++++++++++++++++ 9 files changed, 173 insertions(+), 43 deletions(-) create mode 100644 tests/ref/grid-rtl-counter.png create mode 100644 tests/ref/grid-rtl-rowspan-counter-equal.png create mode 100644 tests/ref/grid-rtl-rowspan-counter-mixed-1.png create mode 100644 tests/ref/grid-rtl-rowspan-counter-mixed-2.png create mode 100644 tests/ref/grid-rtl-rowspan-counter-unequal-1.png create mode 100644 tests/ref/grid-rtl-rowspan-counter-unequal-2.png diff --git a/crates/typst-layout/src/grid/layouter.rs b/crates/typst-layout/src/grid/layouter.rs index dc9e2238d..99b85eddb 100644 --- a/crates/typst-layout/src/grid/layouter.rs +++ b/crates/typst-layout/src/grid/layouter.rs @@ -11,7 +11,7 @@ use typst_library::layout::{ use typst_library::text::TextElem; use typst_library::visualize::Geometry; use typst_syntax::Span; -use typst_utils::{MaybeReverseIter, Numeric}; +use typst_utils::Numeric; use super::{ generate_line_segments, hline_stroke_at_column, layout_cell, vline_stroke_at_row, @@ -574,7 +574,7 @@ impl<'a> GridLayouter<'a> { // Reverse with RTL so that later columns start first. let mut dx = Abs::zero(); - for (x, &col) in self.rcols.iter().enumerate().rev_if(self.is_rtl) { + for (x, &col) in self.rcols.iter().enumerate() { let mut dy = Abs::zero(); for row in rows { // We want to only draw the fill starting at the parent @@ -643,18 +643,13 @@ impl<'a> GridLayouter<'a> { .sum() }; let width = self.cell_spanned_width(cell, x); - // In the grid, cell colspans expand to the right, - // so we're at the leftmost (lowest 'x') column - // spanned by the cell. However, in RTL, cells - // expand to the left. Therefore, without the - // offset below, cell fills would start at the - // rightmost visual position of a cell and extend - // over to unrelated columns to the right in RTL. - // We avoid this by ensuring the fill starts at the - // very left of the cell, even with colspan > 1. - let offset = - if self.is_rtl { -width + col } else { Abs::zero() }; - let pos = Point::new(dx + offset, dy); + let mut pos = Point::new(dx, dy); + if self.is_rtl { + // In RTL cells expand to the left, thus the + // position must additionally be offset by the + // cell's width. + pos.x = self.width - (dx + width); + } let size = Size::new(width, height); let rect = Geometry::Rect(size).filled(fill); fills.push((pos, FrameItem::Shape(rect, self.span))); @@ -1236,10 +1231,9 @@ impl<'a> GridLayouter<'a> { } let mut output = Frame::soft(Size::new(self.width, height)); - let mut pos = Point::zero(); + let mut offset = Point::zero(); - // Reverse the column order when using RTL. - for (x, &rcol) in self.rcols.iter().enumerate().rev_if(self.is_rtl) { + for (x, &rcol) in self.rcols.iter().enumerate() { if let Some(cell) = self.grid.cell(x, y) { // Rowspans have a separate layout step if cell.rowspan.get() == 1 { @@ -1257,25 +1251,17 @@ impl<'a> GridLayouter<'a> { let frame = layout_cell(cell, engine, disambiguator, self.styles, pod)? .into_frame(); - let mut pos = pos; + let mut pos = offset; if self.is_rtl { - // In the grid, cell colspans expand to the right, - // so we're at the leftmost (lowest 'x') column - // spanned by the cell. However, in RTL, cells - // expand to the left. Therefore, without the - // offset below, the cell's contents would be laid out - // starting at its rightmost visual position and extend - // over to unrelated cells to its right in RTL. - // We avoid this by ensuring the rendered cell starts at - // the very left of the cell, even with colspan > 1. - let offset = -width + rcol; - pos.x += offset; + // In RTL cells expand to the left, thus the position + // must additionally be offset by the cell's width. + pos.x = self.width - (pos.x + width); } output.push_frame(pos, frame); } } - pos.x += rcol; + offset.x += rcol; } Ok(output) @@ -1302,8 +1288,8 @@ impl<'a> GridLayouter<'a> { pod.backlog = &heights[1..]; // Layout the row. - let mut pos = Point::zero(); - for (x, &rcol) in self.rcols.iter().enumerate().rev_if(self.is_rtl) { + let mut offset = Point::zero(); + for (x, &rcol) in self.rcols.iter().enumerate() { if let Some(cell) = self.grid.cell(x, y) { // Rowspans have a separate layout step if cell.rowspan.get() == 1 { @@ -1314,17 +1300,19 @@ impl<'a> GridLayouter<'a> { let fragment = layout_cell(cell, engine, disambiguator, self.styles, pod)?; for (output, frame) in outputs.iter_mut().zip(fragment) { - let mut pos = pos; + let mut pos = offset; if self.is_rtl { - let offset = -width + rcol; - pos.x += offset; + // In RTL cells expand to the left, thus the + // position must additionally be offset by the + // cell's width. + pos.x = self.width - (offset.x + width); } output.push_frame(pos, frame); } } } - pos.x += rcol; + offset.x += rcol; } Ok(Fragment::frames(outputs)) diff --git a/crates/typst-layout/src/grid/rowspans.rs b/crates/typst-layout/src/grid/rowspans.rs index 21992ed02..5ab0417d8 100644 --- a/crates/typst-layout/src/grid/rowspans.rs +++ b/crates/typst-layout/src/grid/rowspans.rs @@ -3,7 +3,6 @@ use typst_library::engine::Engine; use typst_library::foundations::Resolve; use typst_library::layout::grid::resolve::Repeatable; use typst_library::layout::{Abs, Axes, Frame, Point, Region, Regions, Size, Sizing}; -use typst_utils::MaybeReverseIter; use super::layouter::{in_last_with_offset, points, Row, RowPiece}; use super::{layout_cell, Cell, GridLayouter}; @@ -23,6 +22,10 @@ pub struct Rowspan { /// specified for the parent cell's `breakable` field. pub is_effectively_unbreakable: bool, /// The horizontal offset of this rowspan in all regions. + /// + /// This is the offset from the text direction start, meaning that, on RTL + /// grids, this is the offset from the right of the grid, whereas, on LTR + /// grids, it is the offset from the left. pub dx: Abs, /// The vertical offset of this rowspan in the first region. pub dy: Abs, @@ -118,10 +121,11 @@ impl GridLayouter<'_> { // Nothing to layout. return Ok(()); }; - let first_column = self.rcols[x]; let cell = self.grid.cell(x, y).unwrap(); let width = self.cell_spanned_width(cell, x); - let dx = if self.is_rtl { dx - width + first_column } else { dx }; + // In RTL cells expand to the left, thus the position + // must additionally be offset by the cell's width. + let dx = if self.is_rtl { self.width - (dx + width) } else { dx }; // Prepare regions. let size = Size::new(width, *first_height); @@ -185,10 +189,8 @@ impl GridLayouter<'_> { /// Checks if a row contains the beginning of one or more rowspan cells. /// If so, adds them to the rowspans vector. pub fn check_for_rowspans(&mut self, disambiguator: usize, y: usize) { - // We will compute the horizontal offset of each rowspan in advance. - // For that reason, we must reverse the column order when using RTL. - let offsets = points(self.rcols.iter().copied().rev_if(self.is_rtl)); - for (x, dx) in (0..self.rcols.len()).rev_if(self.is_rtl).zip(offsets) { + let offsets = points(self.rcols.iter().copied()); + for (x, dx) in (0..self.rcols.len()).zip(offsets) { let Some(cell) = self.grid.cell(x, y) else { continue; }; diff --git a/tests/ref/grid-rtl-counter.png b/tests/ref/grid-rtl-counter.png new file mode 100644 index 0000000000000000000000000000000000000000..fb0df44ad40da59bfc8ee7d98b1445de8c70d3a3 GIT binary patch literal 272 zcmeAS@N?(olHy`uVBq!ia0vp^6+o=Y0VEjK$QP*tsq3CDjv*Ddl7HAcG$dYm6xi*q zE4Q@*#5lR}$N9$H%qfMZf+Rj4Ul(fcY4Y(nv{N3;QLEU8< zD>nBvMb>BE7Z5$5uXcuyIY=aW&y(;2^ACqNtodcEuP&Tic=DpR>E*9$tG7JM57(nv{N3;QLEU8< zD>nBvMb>BE7Z5$5uXcuyIY=aW&y(;2^ACqNtodcEuP&Tic=DpR>E*9$tG7JM57}9D*9cp;sUck_{3~S`;=krMT5lXeI?g1rAn75JqAY#E}hMh4fD= zC^+cO&ie+z?dM&b1i$a}Y@h83(4ztZ3oNj}0t@U0HjmUE!Qs3;_5Ce_?o#hv!=wuK zihQl6>8f)7fd@FckmckGXtEc_L7yA(I=im`bB7+_HGM<*t? z%Ra)5fn1g$4O)w}(6^5;`niq?aHm*uAOZ_4u)qTUGx!I#s*xWevRUx}0000{9D*9cp+_JMk_{3~S`;=krMT5lXeI?g1rAn75JqAY#E}hMh4fD= zC^+cO&U*)ew4R6jg8F`@_x9R8K7MIFfdv*=V1Wfz1KS6Bw_*L(6&%dA)7;&#x&Rk@ z_i(dBu3e#z<)ml$eJxsL`x#(n-yIyDOLKGq)Y*x8hL5i+;~l~)_#eMvn!z{E@Ka)G z1n30L5h?-DioM_&o+zCM*lYkqw%oz1&0+gc1Ex>vDbMg(yb#gzuYfigH*m4SLgI?@ zB`_Nv%gb{KEU>`;5q1{Wrja8>3}Eg9cC=%BY01DJ(1i=b`6LA)Ji; zbg-xpuJvWo3>eUuuLM37!kDKfHo%SM$bkqfu)qQf{O92>(vXoKDr-mq00000NkvXX Hu0mjfwc?u% literal 0 HcmV?d00001 diff --git a/tests/ref/grid-rtl-rowspan-counter-unequal-1.png b/tests/ref/grid-rtl-rowspan-counter-unequal-1.png new file mode 100644 index 0000000000000000000000000000000000000000..c091f3a806bb3bbd5cc37d2e5372c59005093466 GIT binary patch literal 361 zcmV-v0ha!WP){9D*9cp+_JMk_{3~S`;=krMT5lXeI?g1rAn75JqAY#E}hMh4fD= zC^+cO&U*)ew4R6jg8F`@_x9R8K7MIFfdv*=V1Wfz1KS6Bw_*L(6&%dA)7;&#x&Rk@ z_i(dBu3e#z<)ml$eJxsL`x#(n-yIyDOLKGq)Y*x8hL5i+;~l~)_#eMvn!z{E@Ka)G z1n30L5h?-DioM_&o+zCM*lYkqw%oz1&0+gc1Ex>vDbMg(yb#gzuYfigH*m4SLgI?@ zB`_Nv%gb{KEU>`;5q1{Wrja8>3}Eg9cC=%BY01DJ(1i=b`6LA)Ji; zbg-xpuJvWo3>eUuuLM37!kDKfHo%SM$bkqfu)qQf{O92>(vXoKDr-mq00000NkvXX Hu0mjfwc?u% literal 0 HcmV?d00001 diff --git a/tests/ref/grid-rtl-rowspan-counter-unequal-2.png b/tests/ref/grid-rtl-rowspan-counter-unequal-2.png new file mode 100644 index 0000000000000000000000000000000000000000..fffccc5664edfcd379a237268f14dd21e18fa39a GIT binary patch literal 360 zcmV-u0hj)XP)}9D*9cp;sUck_{3~S`;=krMT5lXeI?g1rAn75JqAY#E}hMh4fD= zC^+cO&ie+z?dM&b1i$a}Y@h83(4ztZ3oNj}0t@U0HjmUE!Qs3;_5Ce_?o#hv!=wuK zihQl6>8f)7fd@FckmckGXtEc_L7yA(I=im`bB7+_HGM<*t? z%Ra)5fn1g$4O)w}(6^5;`niq?aHm*uAOZ_4u)qTUGx!I#s*xWevRUx}0000 ([\##i], table.cell(stroke: green)[123], table.cell(stroke: blue)[456], [789], [?], table.hline(start: 4, end: 5, stroke: red))).flatten() ) + +--- grid-rtl-counter --- +// Test interaction between RTL and counters +#set text(dir: rtl) +#let test = counter("test") +#grid( + columns: (1fr, 1fr), + inset: 5pt, + align: center, + [ + a: // should produce 1 + #test.step() + #context test.get().first() + ], + [ + b: // should produce 2 + #test.step() + #context test.get().first() + ], +) + +--- grid-rtl-rowspan-counter-equal --- +// Test interaction between RTL and counters +#set text(dir: rtl) +#let test = counter("test") +#grid( + columns: (1fr, 1fr), + inset: 5pt, + align: center, + grid.cell(rowspan: 2, [ + a: // should produce 1 + #test.step() + #context test.get().first() + ]), + grid.cell(rowspan: 2, [ + b: // should produce 2 + #test.step() + #context test.get().first() + ]), +) + +--- grid-rtl-rowspan-counter-unequal-1 --- +// Test interaction between RTL and counters +#set text(dir: rtl) +#let test = counter("test") +#grid( + columns: (1fr, 1fr), + inset: 5pt, + align: center, + grid.cell(rowspan: 5, [ + b: // will produce 2 + #test.step() + #context test.get().first() + ]), + grid.cell(rowspan: 2, [ + a: // will produce 1 + #test.step() + #context test.get().first() + ]), + grid.cell(rowspan: 3, [ + c: // will produce 3 + #test.step() + #context test.get().first() + ]), +) + +--- grid-rtl-rowspan-counter-unequal-2 --- +// Test interaction between RTL and counters +#set text(dir: rtl) +#let test = counter("test") +#grid( + columns: (1fr, 1fr), + inset: 5pt, + align: center, + grid.cell(rowspan: 2, [ + a: // will produce 1 + #test.step() + #context test.get().first() + ]), + grid.cell(rowspan: 5, [ + b: // will produce 2 + #test.step() + #context test.get().first() + ]), + grid.cell(rowspan: 3, [ + c: // will produce 3 + #test.step() + #context test.get().first() + ]), +) + +--- grid-rtl-rowspan-counter-mixed-1 --- +// Test interaction between RTL and counters +#set text(dir: rtl) +#let test = counter("test") +#grid( + columns: (1fr, 1fr), + inset: 5pt, + align: center, + [ + a: // will produce 1 + #test.step() + #context test.get().first() + ], + grid.cell(rowspan: 2, [ + b: // will produce 2 + #test.step() + #context test.get().first() + ]), + [ + c: // will produce 3 + #test.step() + #context test.get().first() + ], +) + +--- grid-rtl-rowspan-counter-mixed-2 --- +// Test interaction between RTL and counters +#set text(dir: rtl) +#let test = counter("test") +#grid( + columns: (1fr, 1fr), + inset: 5pt, + align: center, + grid.cell(rowspan: 2, [ + b: // will produce 2 + #test.step() + #context test.get().first() + ]), + [ + a: // will produce 1 + #test.step() + #context test.get().first() + ], + [ + c: // will produce 3 + #test.step() + #context test.get().first() + ] +)