Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
28 changes: 12 additions & 16 deletions sparse_strips/vello_common/src/coarse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@ impl<const MODE: u8> Wide<MODE> {
}

// Calculate the width of the strip in columns
let mut col = strip.alpha_idx / u32::from(Tile::HEIGHT);
let next_col = next_strip.alpha_idx / u32::from(Tile::HEIGHT);
let mut col = strip.alpha_idx() / u32::from(Tile::HEIGHT);
let next_col = next_strip.alpha_idx() / u32::from(Tile::HEIGHT);
// Can potentially be 0 if strip only changes winding without covering pixels
let strip_width = next_col.saturating_sub(col) as u16;
let x1 = x0 + strip_width;
Expand Down Expand Up @@ -314,7 +314,7 @@ impl<const MODE: u8> Wide<MODE> {
}

// Determine if the region between this strip and the next should be filled.
let active_fill = next_strip.fill_gap;
let active_fill = next_strip.fill_gap();

// If region should be filled and both strips are on the same row,
// generate fill commands for the region between them
Expand Down Expand Up @@ -456,7 +456,7 @@ impl<const MODE: u8> Wide<MODE> {
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;
((next_strip.alpha_idx() - strip.alpha_idx()) / u32::from(Tile::HEIGHT)) as u16;
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 @@ -505,7 +505,7 @@ impl<const MODE: u8> Wide<MODE> {
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
let is_inside = strip.fill_gap;
let is_inside = strip.fill_gap();
if !is_inside {
for wtile_x in cur_wtile_x..wtile_x_clamped {
self.get_mut(wtile_x, cur_wtile_y).push_zero_clip();
Expand All @@ -518,7 +518,8 @@ impl<const MODE: u8> Wide<MODE> {

// Process wide tiles covered by the strip - these need actual clipping
let next_strip = &strips[i + 1];
let width = ((next_strip.alpha_idx - strip.alpha_idx) / u32::from(Tile::HEIGHT)) as u16;
let width =
((next_strip.alpha_idx() - strip.alpha_idx()) / u32::from(Tile::HEIGHT)) as u16;
let wtile_x1 = (x + width).div_ceil(WideTile::WIDTH).min(clip_bbox.x1());
if cur_wtile_x < wtile_x1 {
for wtile_x in cur_wtile_x..wtile_x1 {
Expand Down Expand Up @@ -631,7 +632,7 @@ impl<const MODE: u8> Wide<MODE> {
// Pop zero clips for tiles that had zero winding or didn't match fill rule
// TODO: The winding check is probably not needed; if there was a fill,
// the logic below should have advanced wtile_x.
let is_inside = strip.fill_gap;
let is_inside = strip.fill_gap();
if !is_inside {
for wtile_x in cur_wtile_x..wtile_x_clamped {
self.get_mut(wtile_x, cur_wtile_y).pop_zero_clip();
Expand All @@ -643,14 +644,14 @@ impl<const MODE: u8> Wide<MODE> {
// Process tiles covered by the strip - render clip content and pop
let next_strip = &strips[i + 1];
let strip_width =
((next_strip.alpha_idx - strip.alpha_idx) / u32::from(Tile::HEIGHT)) as u16;
((next_strip.alpha_idx() - strip.alpha_idx()) / u32::from(Tile::HEIGHT)) as u16;
let mut clipped_x1 = x0 + strip_width;
let wtile_x0 = (x0 / WideTile::WIDTH).max(clip_bbox.x0());
let wtile_x1 = clipped_x1.div_ceil(WideTile::WIDTH).min(clip_bbox.x1());

// Calculate starting position and column for alpha mask
let mut x = x0;
let mut col = strip.alpha_idx / u32::from(Tile::HEIGHT);
let mut col = strip.alpha_idx() / u32::from(Tile::HEIGHT);
let clip_x = clip_bbox.x0() * WideTile::WIDTH;
if clip_x > x {
col += u32::from(clip_x - x);
Expand Down Expand Up @@ -691,7 +692,7 @@ impl<const MODE: u8> Wide<MODE> {
}

// Handle fill regions between strips based on fill rule
let is_inside = next_strip.fill_gap;
let is_inside = next_strip.fill_gap();
if is_inside && strip_y == next_strip.strip_y() {
if cur_wtile_x >= clip_bbox.x1() {
continue;
Expand Down Expand Up @@ -1164,12 +1165,7 @@ mod tests {
assert_eq!(wide.layer_stack.len(), 1);
assert_eq!(wide.clip_stack.len(), 0);

let strip = Strip {
x: 2,
y: 2,
alpha_idx: 0,
fill_gap: true,
};
let strip = Strip::new(2, 2, 0, true);
let clip_path = Some(vec![strip].into_boxed_slice());
wide.push_layer(clip_path, BlendMode::default(), None, 0.09, 0);

Expand Down
104 changes: 79 additions & 25 deletions sparse_strips/vello_common/src/strip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,72 @@ pub struct Strip {
pub x: u16,
/// The y coordinate of the strip, in user coordinates.
pub y: u16,
/// The index into the alpha buffer.
pub alpha_idx: u32,
/// Whether the gap that lies between this strip and the previous in the _same_
/// row should be filled.
pub fill_gap: bool,
/// Packed alpha index and fill gap flag.
///
/// Bit layout (u32):
/// - bit 31: `fill_gap` (See `Strip::fill_gap()`).
/// - bits 0..=30: `alpha_idx` (See `Strip::alpha_idx()`).
packed_alpha_idx_fill_gap: u32,
}

impl Strip {
/// Return the y coordinate of the strip, in strip units.
/// The bit mask for `fill_gap` packed into `packed_alpha_idx_fill_gap`.
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 {
// Ensure `alpha_idx` does not collide with the fill flag bit.
assert!(
alpha_idx & Self::FILL_GAP_MASK == 0,
"`alpha_idx` too large"
);
let fill_gap = u32::from(fill_gap) << 31;
Self {
x,
y,
packed_alpha_idx_fill_gap: alpha_idx | fill_gap,
}
}

/// Returns the y coordinate of the strip, in strip units.
pub fn strip_y(&self) -> u16 {
self.y / Tile::HEIGHT
}

/// Returns the alpha index.
#[inline(always)]
pub fn alpha_idx(&self) -> u32 {
self.packed_alpha_idx_fill_gap & !Self::FILL_GAP_MASK
}

/// Sets the alpha index.
///
/// Note that the largest value that can be stored in the alpha index is `u32::MAX - 1`, as the

@LaurenzV LaurenzV Sep 21, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Isn't it u32::MAX / 2 (or u32::MAX << 1)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Whoops - fixed in bd97a68

Comment thread
taj-p marked this conversation as resolved.
Outdated
/// highest bit is reserved for `fill_gap`.
#[inline(always)]
pub fn set_alpha_idx(&mut self, alpha_idx: u32) {
// Ensure `alpha_idx` does not collide with the fill flag bit.
assert!(
alpha_idx & Self::FILL_GAP_MASK == 0,
"`alpha_idx` too large"
);
let fill_gap = self.packed_alpha_idx_fill_gap & Self::FILL_GAP_MASK;
self.packed_alpha_idx_fill_gap = alpha_idx | fill_gap;
}

/// Returns whether the gap that lies between this strip and the previous in the same row should be filled.
#[inline(always)]
pub fn fill_gap(&self) -> bool {
(self.packed_alpha_idx_fill_gap & Self::FILL_GAP_MASK) != 0
}

/// Sets whether the gap that lies between this strip and the previous in the same row should be filled.
#[inline(always)]
pub fn set_fill_gap(&mut self, fill: bool) {
let fill = u32::from(fill) << 31;
self.packed_alpha_idx_fill_gap =
(self.packed_alpha_idx_fill_gap & !Self::FILL_GAP_MASK) | fill;
}
}

/// Render the tiles stored in `tiles` into the strip and alpha buffer.
Expand Down Expand Up @@ -100,12 +154,12 @@ fn render_impl<S: Simd>(
const SENTINEL: Tile = Tile::new(u16::MAX, u16::MAX, 0, false);

// The strip we're building.
let mut strip = Strip {
x: prev_tile.x * Tile::WIDTH,
y: prev_tile.y * Tile::HEIGHT,
alpha_idx: alpha_buf.len() as u32,
fill_gap: false,
};
let mut strip = Strip::new(
prev_tile.x * Tile::WIDTH,
prev_tile.y * Tile::HEIGHT,
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];
Expand Down Expand Up @@ -179,7 +233,7 @@ fn render_impl<S: Simd>(
if !prev_tile.same_loc(&tile) && !prev_tile.prev_loc(&tile) {
debug_assert_eq!(
(prev_tile.x + 1) * Tile::WIDTH - strip.x,
((alpha_buf.len() - strip.alpha_idx as usize) / usize::from(Tile::HEIGHT)) as u16,
((alpha_buf.len() - strip.alpha_idx() as usize) / usize::from(Tile::HEIGHT)) as u16,
"The number of columns written to the alpha buffer should equal the number of columns spanned by this strip."
);
strip_buf.push(strip);
Expand All @@ -190,12 +244,12 @@ fn render_impl<S: Simd>(
// or unconditionally if we've reached the sentinel tile to end the path (the
// `alpha_idx` field is used for width calculations).
if winding_delta != 0 || is_sentinel {
strip_buf.push(Strip {
x: u16::MAX,
y: prev_tile.y * Tile::HEIGHT,
alpha_idx: alpha_buf.len() as u32,
fill_gap: should_fill(winding_delta),
});
strip_buf.push(Strip::new(
u16::MAX,
prev_tile.y * Tile::HEIGHT,
alpha_buf.len() as u32,
should_fill(winding_delta),
));
}

winding_delta = 0;
Expand All @@ -211,12 +265,12 @@ fn render_impl<S: Simd>(
break;
}

strip = Strip {
x: tile.x * Tile::WIDTH,
y: tile.y * Tile::HEIGHT,
alpha_idx: alpha_buf.len() as u32,
fill_gap: should_fill(winding_delta),
};
strip = Strip::new(
tile.x * Tile::WIDTH,
tile.y * Tile::HEIGHT,
alpha_buf.len() as u32,
should_fill(winding_delta),
);
// Note: this fill is mathematically not necessary. It provides a way to reduce
// accumulation of float rounding errors.
accumulated_winding = f32x4::splat(s, winding_delta as f32);
Expand Down
2 changes: 1 addition & 1 deletion sparse_strips/vello_cpu/src/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ impl RenderContext {
.iter()
.map(move |strip| {
let mut adjusted_strip = *strip;
adjusted_strip.alpha_idx += alpha_offset;
adjusted_strip.set_alpha_idx(adjusted_strip.alpha_idx().wrapping_add(alpha_offset));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we really want a wrapping add here? Wouldn't it be better to just use normal add so that at least in debug mode it panics?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right. Thank you - nice catch! Fixed in a217c0c

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

And fixed in hybrid in c0947d3

adjusted_strip
})
.collect()
Expand Down
2 changes: 1 addition & 1 deletion sparse_strips/vello_hybrid/src/scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ impl Scene {
.iter()
.map(move |strip| {
let mut adjusted_strip = *strip;
adjusted_strip.alpha_idx += alpha_offset;
adjusted_strip.set_alpha_idx(adjusted_strip.alpha_idx().wrapping_add(alpha_offset));
adjusted_strip
})
.collect()
Expand Down
14 changes: 7 additions & 7 deletions sparse_strips/vello_toy/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,13 +225,13 @@ fn draw_strip_areas(document: &mut Document, strips: &[Strip], alphas: &[u8]) {

let end = strips
.get(i + 1)
.map(|s| s.alpha_idx / u32::from(Tile::HEIGHT))
.map(|s| s.alpha_idx() / u32::from(Tile::HEIGHT))
.unwrap_or(alphas.len() as u32);

let width = end - strip.alpha_idx / u32::from(Tile::HEIGHT);
let width = end - strip.alpha_idx() / u32::from(Tile::HEIGHT);

// TODO: Account for even-odd?
let color = if strip.fill_gap { "red" } else { "limegreen" };
let color = if strip.fill_gap() { "red" } else { "limegreen" };

let rect = Rectangle::new()
.set("x", x)
Expand All @@ -254,17 +254,17 @@ fn draw_strips(document: &mut Document, strips: &[Strip], alphas: &[u8]) {

let end = strips
.get(s + 1)
.map(|st| st.alpha_idx / u32::from(Tile::HEIGHT))
.map(|st| st.alpha_idx() / u32::from(Tile::HEIGHT))
.unwrap_or(alphas.len() as u32);

let width = u16::try_from(end - strip.alpha_idx / u32::from(Tile::HEIGHT)).unwrap();
let width = u16::try_from(end - strip.alpha_idx() / u32::from(Tile::HEIGHT)).unwrap();

// TODO: Account for even-odd?
let color = if strip.fill_gap { "red" } else { "limegreen" };
let color = if strip.fill_gap() { "red" } else { "limegreen" };

for x in 0..width {
for y in 0..Tile::HEIGHT {
let alpha = alphas[strip.alpha_idx as usize
let alpha = alphas[strip.alpha_idx() as usize
+ usize::from(x) * usize::from(Tile::HEIGHT)
+ usize::from(y)];
let rect = Rectangle::new()
Expand Down
Loading