Skip to content

vello_common: Store strip data in tile units #1379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
LaurenzV wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

vello_common: Store strip data in tile units #1379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b3f24b8
Store strip data in tile space
LaurenzV Jan 25, 2026
3d3060a
Update sparse_strips/vello_common/src/strip.rs
LaurenzV Jan 27, 2026
cf84249
Inline some methods
LaurenzV Jan 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 30 additions & 28 deletions sparse_strips/vello_common/src/clip.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,17 +162,20 @@ fn intersect_impl<S: Simd>(
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;
}

// 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;
Expand Down Expand Up @@ -277,12 +280,9 @@ fn intersect_impl<S: Simd>(
}

// 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.
Expand Down Expand Up @@ -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.
Expand All @@ -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,
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();

Expand All @@ -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<StripState>, strips: &mut Vec<Strip>, cur_y: u16) {
fn flush_strip(strip_state: &mut Option<StripState>, strips: &mut Vec<Strip>, 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,
));
Expand All @@ -515,7 +515,7 @@ fn flush_strip(strip_state: &mut Option<StripState>, strips: &mut Vec<Strip>, cu
#[inline(always)]
fn start_strip(strip_data: &mut Option<StripState>, 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,
});
Expand All @@ -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
})
Expand Down Expand Up @@ -674,6 +674,8 @@ mod tests {
}
}

// TODO: Rewrite tests so we work with tile_x instead of x.

Comment on lines +677 to +678
Copy link
Collaborator

@grebmeg grebmeg Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// TODO: Rewrite tests so we work with tile_x instead of x.
// TODO: Rewrite tests so we work with tile_x instead of x.

Copy link
Collaborator Author

@LaurenzV LaurenzV Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the suggestion here? 🤔 Or is it buggy on my screen?
image

Copy link
Collaborator

@grebmeg grebmeg Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or it's just an empty line 😅

fn add_strip(self, x: u16, strip_y: u16, end: u16, fill_gap: bool) -> Self {
let width = end - x;
self.add_strip_with(
Expand All @@ -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],
Expand All @@ -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
}
Expand Down
34 changes: 18 additions & 16 deletions sparse_strips/vello_common/src/coarse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,8 @@ impl Wide<MODE_HYBRID> {
}
}

// TODO: Ensure that all of the calculations below are compatible with i16.

Comment on lines +288 to +289
Copy link
Collaborator

@grebmeg grebmeg Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// TODO: Ensure that all of the calculations below are compatible with i16.
// TODO: Ensure that all of the calculations below are compatible with i16.

Copy link
Collaborator Author

@LaurenzV LaurenzV Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

impl<const MODE: u8> Wide<MODE> {
/// Create a new container for wide tiles.
fn new_internal(width: u16, height: u16) -> Self {
Expand Down Expand Up @@ -438,18 +440,18 @@ impl<const MODE: u8> Wide<MODE> {
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() {
Expand Down Expand Up @@ -507,10 +509,10 @@ impl<const MODE: u8> Wide<MODE> {

// 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),
Expand Down Expand Up @@ -808,18 +810,18 @@ impl<const MODE: u8> Wide<MODE> {
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));
}
Expand Down Expand Up @@ -848,7 +850,7 @@ impl<const MODE: u8> Wide<MODE> {
// 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 {
Expand All @@ -875,7 +877,7 @@ impl<const MODE: u8> Wide<MODE> {
}

// 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
Expand Down Expand Up @@ -978,7 +980,7 @@ impl<const MODE: u8> Wide<MODE> {
// 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 {
Expand Down Expand Up @@ -1010,7 +1012,7 @@ impl<const MODE: u8> Wide<MODE> {
}

// 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
Expand Down Expand Up @@ -1082,12 +1084,12 @@ impl<const MODE: u8> Wide<MODE> {

// 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);

Expand Down
Loading