From b3f24b86290945c48d598da33a5ebfc00f9c482b Mon Sep 17 00:00:00 2001 From: Laurenz Stampfl Date: Sun, 25 Jan 2026 21:48:03 +0100 Subject: [PATCH 1/3] Store strip data in tile space --- sparse_strips/vello_common/src/clip.rs | 58 ++++++++++----------- sparse_strips/vello_common/src/coarse.rs | 34 +++++++------ sparse_strips/vello_common/src/strip.rs | 64 ++++++++++++++++-------- sparse_strips/vello_toy/src/debug.rs | 8 +-- 4 files changed, 95 insertions(+), 69 deletions(-) diff --git a/sparse_strips/vello_common/src/clip.rs b/sparse_strips/vello_common/src/clip.rs index 56fb82fd6..e3dd934ee 100644 --- a/sparse_strips/vello_common/src/clip.rs +++ b/sparse_strips/vello_common/src/clip.rs @@ -162,6 +162,9 @@ fn intersect_impl( path_2: PathDataRef<'_>, target: &mut StripStorage, ) { + // TODO: Rewrite the implementation (and the tests further below) + // so it can be made compatible with i16. + // In case either path is empty, the clip path should be empty. if path_1.strips.is_empty() || path_2.strips.is_empty() { return; @@ -169,10 +172,10 @@ fn intersect_impl( // Ignore any y values that are outside the bounding box of either of the two paths, as // those are guaranteed to have neither fill nor strip regions. - let mut cur_y = path_1.strips[0].strip_y().min(path_2.strips[0].strip_y()); + let mut cur_y = path_1.strips[0].tile_y().min(path_2.strips[0].tile_y()); let end_y = path_1.strips[path_1.strips.len() - 1] - .strip_y() - .min(path_2.strips[path_2.strips.len() - 1].strip_y()); + .tile_y() + .min(path_2.strips[path_2.strips.len() - 1].tile_y()); let mut path_1_idx = 0; let mut path_2_idx = 0; @@ -277,12 +280,9 @@ fn intersect_impl( } // Push the sentinel strip. - target.strips.push(Strip::new( - u16::MAX, - end_y * Tile::HEIGHT, - target.alphas.len() as u32, - false, - )); + target + .strips + .push(Strip::new_sentinel(end_y, target.alphas.len() as u32)); } /// An overlap between two regions. @@ -383,8 +383,8 @@ impl Region<'_> { struct RowIterator<'a> { /// The path in question. input: PathDataRef<'a>, - /// The strip row we want to iterate over. - strip_y: u16, + /// The strip row we want to iterate over, in tile coordinates. + tile_y: u16, /// The index of the current strip. cur_idx: &'a mut usize, /// Whether the iterator should yield a strip next or not. @@ -395,16 +395,16 @@ struct RowIterator<'a> { } impl<'a> RowIterator<'a> { - fn new(input: PathDataRef<'a>, cur_idx: &'a mut usize, strip_y: u16) -> Self { + fn new(input: PathDataRef<'a>, cur_idx: &'a mut usize, tile_y: u16) -> Self { // Forward the index until we have found the right strip. - while input.strips[*cur_idx].strip_y() < strip_y { + while input.strips[*cur_idx].tile_y() < tile_y { *cur_idx += 1; } Self { input, cur_idx, - strip_y, + tile_y, on_strip: true, } } @@ -440,8 +440,8 @@ impl<'a> RowIterator<'a> { // zero winding so we don't need to special case this. if next.fill_gap() { let cur = self.cur_strip(); - let x = cur.x + self.cur_strip_width(); - let width = next.x - x; + let x = cur.x() + self.cur_strip_width(); + let width = next.x() - x; Some(FillRegion { start: x, width }) } else { @@ -477,12 +477,12 @@ impl<'a> Iterator for RowIterator<'a> { self.on_strip = false; // If the current strip is sentinel or not within our target row, terminate. - if self.cur_strip().is_sentinel() || self.cur_strip().strip_y() != self.strip_y { + if self.cur_strip().is_sentinel() || self.cur_strip().tile_y() != self.tile_y { return None; } // Calculate the dimensions of the strip and yield it. - let x = self.cur_strip().x; + let x = self.cur_strip().x(); let width = self.cur_strip_width(); let alphas = self.cur_strip_alphas(); @@ -496,16 +496,16 @@ impl<'a> Iterator for RowIterator<'a> { /// The data of the current strip we are building. struct StripState { - x: u16, + tile_x: u16, alpha_idx: u32, fill_gap: bool, } -fn flush_strip(strip_state: &mut Option, strips: &mut Vec, cur_y: u16) { +fn flush_strip(strip_state: &mut Option, strips: &mut Vec, tile_y: u16) { if let Some(state) = core::mem::take(strip_state) { strips.push(Strip::new( - state.x, - cur_y * Tile::HEIGHT, + state.tile_x, + tile_y, state.alpha_idx, state.fill_gap, )); @@ -515,7 +515,7 @@ fn flush_strip(strip_state: &mut Option, strips: &mut Vec, cu #[inline(always)] fn start_strip(strip_data: &mut Option, alphas: &[u8], x: u16, fill_gap: bool) { *strip_data = Some(StripState { - x, + tile_x: x / Tile::WIDTH, alpha_idx: alphas.len() as u32, fill_gap, }); @@ -529,7 +529,7 @@ fn should_create_new_strip( // Returns false in case we can append to the currently built strip. strip_state.as_ref().is_none_or(|state| { let width = ((alphas.len() as u32 - state.alpha_idx) / Tile::HEIGHT as u32) as u16; - let strip_end = state.x + width; + let strip_end = state.tile_x * Tile::WIDTH + width; strip_end < overlap_start - 1 }) @@ -674,6 +674,8 @@ mod tests { } } + // TODO: Rewrite tests so we work with tile_x instead of x. + fn add_strip(self, x: u16, strip_y: u16, end: u16, fill_gap: bool) -> Self { let width = end - x; self.add_strip_with( @@ -688,7 +690,7 @@ mod tests { fn add_strip_with( mut self, x: u16, - strip_y: u16, + tile_y: u16, end: u16, fill_gap: bool, alphas: &[u8], @@ -698,19 +700,19 @@ mod tests { let idx = self.storage.alphas.len(); self.storage .strips - .push(Strip::new(x, strip_y * Tile::HEIGHT, idx as u32, fill_gap)); + .push(Strip::new(x / Tile::WIDTH, tile_y, idx as u32, fill_gap)); self.storage.alphas.extend_from_slice(alphas); self } fn finish(mut self) -> StripStorage { - let last_y = self.storage.strips.last().unwrap().y; + let last_tile_y = self.storage.strips.last().unwrap().tile_y(); let idx = self.storage.alphas.len(); self.storage .strips - .push(Strip::new(u16::MAX, last_y, idx as u32, false)); + .push(Strip::new_sentinel(last_tile_y, idx as u32)); self.storage } diff --git a/sparse_strips/vello_common/src/coarse.rs b/sparse_strips/vello_common/src/coarse.rs index 4892055e8..1bfbf3f6a 100644 --- a/sparse_strips/vello_common/src/coarse.rs +++ b/sparse_strips/vello_common/src/coarse.rs @@ -285,6 +285,8 @@ impl Wide { } } +// TODO: Ensure that all of the calculations below are compatible with i16. + impl Wide { /// Create a new container for wide tiles. fn new_internal(width: u16, height: u16) -> Self { @@ -438,18 +440,18 @@ impl Wide { let strip = &strip_buf[i]; debug_assert!( - strip.y < self.height, + strip.y() < self.height, "Strips below the viewport should have been culled prior to this stage." ); // Don't render strips that are outside the viewport width - if strip.x >= self.width { + if strip.x() >= self.width { continue; } let next_strip = &strip_buf[i + 1]; - let x0 = strip.x; - let strip_y = strip.strip_y(); + let x0 = strip.x(); + let strip_y = strip.tile_y(); // Skip strips outside the current clip bounding box if strip_y < bbox.y0() { @@ -507,10 +509,10 @@ impl Wide { // If region should be filled and both strips are on the same row, // generate fill commands for the region between them - if active_fill && strip_y == next_strip.strip_y() { + if active_fill && strip_y == next_strip.tile_y() { // Clamp the fill to the clip bounding box x = x1.max(bbox.x0() * WideTile::WIDTH); - let x2 = next_strip.x.min( + let x2 = next_strip.x().min( self.width .checked_next_multiple_of(WideTile::WIDTH) .unwrap_or(u16::MAX), @@ -808,18 +810,18 @@ impl Wide { WideTilesBbox::empty() } else { // Calculate the y range from first to last strip in wide tile coordinates - let wtile_y0 = strips[0].strip_y(); - let wtile_y1 = strips[n_strips.saturating_sub(1)].strip_y() + 1; + let wtile_y0 = strips[0].tile_y(); + let wtile_y1 = strips[n_strips.saturating_sub(1)].tile_y() + 1; // Calculate the x range by examining all strips in wide tile coordinates - let mut wtile_x0 = strips[0].x / WideTile::WIDTH; + let mut wtile_x0 = strips[0].x() / WideTile::WIDTH; let mut wtile_x1 = wtile_x0; for i in 0..n_strips.saturating_sub(1) { let strip = &strips[i]; let next_strip = &strips[i + 1]; let width = ((next_strip.alpha_idx() - strip.alpha_idx()) / u32::from(Tile::HEIGHT)) as u16; - let x = strip.x; + let x = strip.x(); wtile_x0 = wtile_x0.min(x / WideTile::WIDTH); wtile_x1 = wtile_x1.max((x + width).div_ceil(WideTile::WIDTH)); } @@ -848,7 +850,7 @@ impl Wide { // Process strips to determine the clipping state for each wide tile for i in 0..n_strips.saturating_sub(1) { let strip = &strips[i]; - let strip_y = strip.strip_y(); + let strip_y = strip.tile_y(); // Skip strips before current wide tile row if strip_y < cur_wtile_y { @@ -875,7 +877,7 @@ impl Wide { } // Process wide tiles to the left of this strip in the same row - let x = strip.x; + let x = strip.x(); let wtile_x_clamped = (x / WideTile::WIDTH).min(clip_bbox.x1()); if cur_wtile_x < wtile_x_clamped { // If winding is zero or doesn't match fill rule, these wide tiles are outside the path @@ -978,7 +980,7 @@ impl Wide { // Process each strip to determine the clipping state for each tile for i in 0..n_strips.saturating_sub(1) { let strip = &strips[i]; - let strip_y = strip.strip_y(); + let strip_y = strip.tile_y(); // Skip strips before current tile row if strip_y < cur_wtile_y { @@ -1010,7 +1012,7 @@ impl Wide { } // Process tiles to the left of this strip in the same row - let x0 = strip.x; + let x0 = strip.x(); let wtile_x_clamped = (x0 / WideTile::WIDTH).min(clip_bbox.x1()); if cur_wtile_x < wtile_x_clamped { // Handle any pending clip pop from previous iteration @@ -1082,12 +1084,12 @@ impl Wide { // Handle fill regions between strips based on fill rule let is_inside = next_strip.fill_gap(); - if is_inside && strip_y == next_strip.strip_y() { + if is_inside && strip_y == next_strip.tile_y() { if cur_wtile_x >= clip_bbox.x1() { continue; } - let x2 = next_strip.x; + let x2 = next_strip.x(); let clipped_x2 = x2.min((cur_wtile_x + 1) * WideTile::WIDTH); let width = clipped_x2.saturating_sub(clipped_x1); diff --git a/sparse_strips/vello_common/src/strip.rs b/sparse_strips/vello_common/src/strip.rs index 731b58d38..3b26a244e 100644 --- a/sparse_strips/vello_common/src/strip.rs +++ b/sparse_strips/vello_common/src/strip.rs @@ -13,10 +13,13 @@ use fearless_simd::*; /// A strip. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Strip { - /// The x coordinate of the strip, in user coordinates. - pub x: u16, - /// The y coordinate of the strip, in user coordinates. - pub y: u16, + /// The x coordinate of the strip, in tile units. + tile_x: u16, + /// The y coordinate of the strip, in tile units. + tile_y: u16, + // TODO: Change it so that alpha_idx is stored divided by Tile::WIDTH * TIlE::HEIGHT. + // This will free up 4 more bits we can use in the future (or just increase the + // number of permissible alpha values). /// Packed alpha index and fill gap flag. /// /// Bit layout (u32): @@ -30,7 +33,7 @@ impl Strip { const FILL_GAP_MASK: u32 = 1 << 31; /// Creates a new strip. - pub fn new(x: u16, y: u16, alpha_idx: u32, fill_gap: bool) -> Self { + pub fn new(tile_x: u16, tile_y: u16, alpha_idx: u32, fill_gap: bool) -> Self { // Ensure `alpha_idx` does not collide with the fill flag bit. assert!( alpha_idx & Self::FILL_GAP_MASK == 0, @@ -38,20 +41,44 @@ impl Strip { ); let fill_gap = u32::from(fill_gap) << 31; Self { - x, - y, + tile_x, + tile_y, packed_alpha_idx_fill_gap: alpha_idx | fill_gap, } } + /// Creates a new sentinel strip. + pub fn new_sentinel(tile_y: u16, alpha_idx: u32) -> Self { + Self::new(u16::MAX, tile_y, alpha_idx, false) + } + /// Return whether the strip is a sentinel strip. pub fn is_sentinel(&self) -> bool { - self.x == u16::MAX + self.tile_x == u16::MAX + } + + /// Return the x coordinate of the strip, in user coordinates. + pub fn x(&self) -> u16 { + if self.is_sentinel() { + u16::MAX + } else { + self.tile_x * Tile::WIDTH + } + } + + /// Return the y coordinate of the strip, in user coordinates. + pub fn y(&self) -> u16 { + self.tile_y * Tile::HEIGHT + } + + /// Return the x coordinate of the strip, in tile units. + pub fn tile_x(&self) -> u16 { + self.tile_x } - /// Return the y coordinate of the strip, in strip units. - pub fn strip_y(&self) -> u16 { - self.y / Tile::HEIGHT + /// Return the y coordinate of the strip, in tile units. + pub fn tile_y(&self) -> u16 { + self.tile_y } /// Returns the alpha index. @@ -141,12 +168,7 @@ fn render_impl( const SENTINEL: Tile = Tile::new(u16::MAX, u16::MAX, 0, 0); // The strip we're building. - let mut strip = Strip::new( - prev_tile.x * Tile::WIDTH, - prev_tile.y * Tile::HEIGHT, - alpha_buf.len() as u32, - false, - ); + let mut strip = Strip::new(prev_tile.x, prev_tile.y, alpha_buf.len() as u32, false); for (tile_idx, tile) in tiles.iter().copied().chain([SENTINEL]).enumerate() { let line = lines[tile.line_idx() as usize]; @@ -219,7 +241,7 @@ fn render_impl( // Push out the strip if we're moving to a next strip. if !prev_tile.same_loc(&tile) && !prev_tile.prev_loc(&tile) { debug_assert_eq!( - (prev_tile.x as u32 + 1) * Tile::WIDTH as u32 - strip.x as u32, + (prev_tile.x as u32 + 1) * Tile::WIDTH as u32 - strip.x() as u32, ((alpha_buf.len() - strip.alpha_idx() as usize) / usize::from(Tile::HEIGHT)) as u32, "The number of columns written to the alpha buffer should equal the number of columns spanned by this strip." ); @@ -233,7 +255,7 @@ fn render_impl( if winding_delta != 0 || is_sentinel { strip_buf.push(Strip::new( u16::MAX, - prev_tile.y * Tile::HEIGHT, + prev_tile.y, alpha_buf.len() as u32, should_fill(winding_delta), )); @@ -253,8 +275,8 @@ fn render_impl( } strip = Strip::new( - tile.x * Tile::WIDTH, - tile.y * Tile::HEIGHT, + tile.x, + tile.y, alpha_buf.len() as u32, should_fill(winding_delta), ); diff --git a/sparse_strips/vello_toy/src/debug.rs b/sparse_strips/vello_toy/src/debug.rs index ac1df2643..bb4dd0bef 100644 --- a/sparse_strips/vello_toy/src/debug.rs +++ b/sparse_strips/vello_toy/src/debug.rs @@ -228,8 +228,8 @@ fn draw_tile_areas(document: &mut Document, tiles: &Tiles) { fn draw_strip_areas(document: &mut Document, strips: &[Strip], alphas: &[u8]) { for i in 0..strips.len() { let strip = &strips[i]; - let x = strip.x; - let y = strip.strip_y(); + let x = strip.x(); + let y = strip.tile_y(); let end = strips .get(i + 1) @@ -276,8 +276,8 @@ fn draw_strips(document: &mut Document, strips: &[Strip], alphas: &[u8]) { + usize::from(x) * usize::from(Tile::HEIGHT) + usize::from(y)]; let rect = Rectangle::new() - .set("x", strip.x + x) - .set("y", strip.y + y) + .set("x", strip.x() + x) + .set("y", strip.y() + y) .set("width", 1) .set("height", 1) .set("fill", color) From 3d3060ae140bbff4419619fe400c727ad3c12ce1 Mon Sep 17 00:00:00 2001 From: Laurenz Stampfl <47084093+LaurenzV@users.noreply.github.com> Date: Tue, 27 Jan 2026 08:00:03 +0100 Subject: [PATCH 2/3] Update sparse_strips/vello_common/src/strip.rs Co-authored-by: Alex Gemberg --- sparse_strips/vello_common/src/strip.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sparse_strips/vello_common/src/strip.rs b/sparse_strips/vello_common/src/strip.rs index 3b26a244e..2a0ca041c 100644 --- a/sparse_strips/vello_common/src/strip.rs +++ b/sparse_strips/vello_common/src/strip.rs @@ -17,7 +17,7 @@ pub struct Strip { tile_x: u16, /// The y coordinate of the strip, in tile units. tile_y: u16, - // TODO: Change it so that alpha_idx is stored divided by Tile::WIDTH * TIlE::HEIGHT. + // TODO: Change it so that alpha_idx is stored divided by Tile::WIDTH * Tile::HEIGHT. // This will free up 4 more bits we can use in the future (or just increase the // number of permissible alpha values). /// Packed alpha index and fill gap flag. From cf84249dc9d71a347d7263da7652686385f1be03 Mon Sep 17 00:00:00 2001 From: Laurenz Stampfl Date: Tue, 27 Jan 2026 08:01:26 +0100 Subject: [PATCH 3/3] Inline some methods --- sparse_strips/vello_common/src/strip.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sparse_strips/vello_common/src/strip.rs b/sparse_strips/vello_common/src/strip.rs index 2a0ca041c..19e59502c 100644 --- a/sparse_strips/vello_common/src/strip.rs +++ b/sparse_strips/vello_common/src/strip.rs @@ -33,6 +33,7 @@ impl Strip { const FILL_GAP_MASK: u32 = 1 << 31; /// Creates a new strip. + #[inline] pub fn new(tile_x: u16, tile_y: u16, alpha_idx: u32, fill_gap: bool) -> Self { // Ensure `alpha_idx` does not collide with the fill flag bit. assert!( @@ -48,16 +49,19 @@ impl Strip { } /// Creates a new sentinel strip. + #[inline] pub fn new_sentinel(tile_y: u16, alpha_idx: u32) -> Self { Self::new(u16::MAX, tile_y, alpha_idx, false) } /// Return whether the strip is a sentinel strip. + #[inline] pub fn is_sentinel(&self) -> bool { self.tile_x == u16::MAX } /// Return the x coordinate of the strip, in user coordinates. + #[inline] pub fn x(&self) -> u16 { if self.is_sentinel() { u16::MAX @@ -67,16 +71,19 @@ impl Strip { } /// Return the y coordinate of the strip, in user coordinates. + #[inline] pub fn y(&self) -> u16 { self.tile_y * Tile::HEIGHT } /// Return the x coordinate of the strip, in tile units. + #[inline] pub fn tile_x(&self) -> u16 { self.tile_x } /// Return the y coordinate of the strip, in tile units. + #[inline] pub fn tile_y(&self) -> u16 { self.tile_y }