Skip to content

vello_sparse_tests: Add support for multi-frame tests #1501

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

Open
LaurenzV wants to merge 6 commits into
base: main
Choose a base branch
from
Open

vello_sparse_tests: Add support for multi-frame tests #1501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
23a0de9
vello_sparse_tests: Add support for multi-frame tests
laurenz-canva Mar 12, 2026
c4798c2
fix
laurenz-canva Mar 12, 2026
2632353
Update sparse_strips/vello_dev_macros/src/test.rs
LaurenzV Mar 16, 2026
90b6a44
Reformat
laurenz-canva Mar 16, 2026
eb301c1
Undo format change
laurenz-canva Mar 16, 2026
5aca476
Reformat
laurenz-canva Mar 16, 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
77 changes: 65 additions & 12 deletions sparse_strips/vello_dev_macros/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ struct Arguments {
no_ref: bool,
/// A reason for ignoring a test.
ignore_reason: Option<String>,
/// The number of frames to render. When greater than 1, the test function receives an
/// additional `u32` argument indicating the current frame index. The generated test will
/// loop `frame_count` times, resetting the context before each frame. Only the final
/// frame is checked against the reference image.
frame_count: u32,

@grebmeg grebmeg Mar 16, 2026

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.

What about snapshotting all frames into a single long strip and then comparing the whole thing? I think this way we could verify that all frames in the sequence have not changed.

@LaurenzV LaurenzV Mar 16, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, I guess this would be not ideal if we have some tests that rely on many frames to trigger a bug, but I wouldn't mind changing it. @taj-p What do you think?

@taj-p taj-p Mar 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Snapshotting across multiple frames increases our testing coverage in that we are also asserting on intermediate states. Perhaps it can be an additional flag to specify? Depending on what tests you want to add at this stage, we can decide on whether to default on or default off.

The existing test suite seems like it doesn't need it but I can imagine it might being wanted in the future (if we're unsure, we could leave it till then).

@grebmeg grebmeg Mar 17, 2026

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.

I don’t mind adding an extra flag for this. As for test coverage, I don’t think we’ll have many tests that check using multiple frames, but I’d definitely prefer to keep snapshotting all frames behind a flag. My thinking is that the final output might be correct even if intermediate frames are not. Anyway, that’s just an assumption.

}

impl Default for Arguments {
Expand All @@ -61,6 +66,7 @@ impl Default for Arguments {
no_ref: false,
diff_pixels: 0,
ignore_reason: None,
frame_count: 1,
}
}
}
Expand Down Expand Up @@ -158,8 +164,28 @@ pub(crate) fn vello_test_inner(attr: TokenStream, item: TokenStream) -> TokenStr
ignore_reason,
no_ref,
diff_pixels,
frame_count,
} = parse_args(&attrs);

let has_frame_count = frame_count > 1;

let test_fn_call = if has_frame_count {
quote! { #input_fn_name(&mut ctx, frame); }
} else {
quote! { #input_fn_name(&mut ctx); }
};

let width_f64 = width as f64;
let height_f64 = height as f64;
let draw_bg_snippet = if !transparent {
quote! {
ctx.set_paint(vello_common::color::palette::css::WHITE);
ctx.fill_rect(&vello_common::kurbo::Rect::new(0.0, 0.0, #width_f64, #height_f64));
}
} else {
quote! {}
};

// Wasm doesn't have access to the filesystem. For wasm, inline the snapshot bytes into the
// binary.
let reference_image_name = Ident::new(
Expand Down Expand Up @@ -276,9 +302,15 @@ pub(crate) fn vello_test_inner(attr: TokenStream, item: TokenStream) -> TokenStr
};
use vello_cpu::{RenderContext, RenderMode};

let mut ctx = get_ctx::<RenderContext>(#width, #height, #transparent, #num_threads, #level, #render_mode, false);
#input_fn_name(&mut ctx);
ctx.flush();
let mut ctx = get_ctx::<RenderContext>(#width, #height, #num_threads, #level, #render_mode, false);
for frame in 0..#frame_count {
if frame > 0 {
ctx.reset();
}
#draw_bg_snippet
#test_fn_call
ctx.flush();
}
if !#no_ref {
check_ref(&ctx, #input_fn_name_str, #fn_name_str, #tolerance, #diff_pixels, #is_reference, #reference_image_name);
}
Expand Down Expand Up @@ -462,9 +494,15 @@ pub(crate) fn vello_test_inner(attr: TokenStream, item: TokenStream) -> TokenStr
use crate::renderer::HybridRenderer;
use vello_cpu::RenderMode;

let mut ctx = get_ctx::<HybridRenderer>(#width, #height, #transparent, 0, "fallback", RenderMode::OptimizeSpeed, false);
#input_fn_name(&mut ctx);
ctx.flush();
let mut ctx = get_ctx::<HybridRenderer>(#width, #height, 0, "fallback", RenderMode::OptimizeSpeed, false);
for frame in 0..#frame_count {
if frame > 0 {
ctx.reset()
}
#draw_bg_snippet
#test_fn_call
ctx.flush();
}
if !#no_ref {
check_ref(&ctx, #input_fn_name_str, #hybrid_fn_name_str, #hybrid_tolerance, #diff_pixels, false, #reference_image_name);
}
Expand All @@ -480,9 +518,15 @@ pub(crate) fn vello_test_inner(attr: TokenStream, item: TokenStream) -> TokenStr
use crate::renderer::HybridRenderer;
use vello_cpu::RenderMode;

let mut ctx = get_ctx::<HybridRenderer>(#width, #height, #transparent, 0, "fallback", RenderMode::OptimizeSpeed, true);
#input_fn_name(&mut ctx);
ctx.flush();
let mut ctx = get_ctx::<HybridRenderer>(#width, #height, 0, "fallback", RenderMode::OptimizeSpeed, true);
for frame in 0..#frame_count {
if frame > 0 {
ctx.reset()
}
#draw_bg_snippet
#test_fn_call
ctx.flush();
}
if !#no_ref {
check_ref(&ctx, #input_fn_name_str, #hybrid_constrained_fn_name_str, #hybrid_tolerance, #diff_pixels, false, #reference_image_name);
}
Expand All @@ -498,9 +542,15 @@ pub(crate) fn vello_test_inner(attr: TokenStream, item: TokenStream) -> TokenStr
use crate::renderer::HybridRenderer;
use vello_cpu::RenderMode;

let mut ctx = get_ctx::<HybridRenderer>(#width, #height, #transparent, 0, "fallback", RenderMode::OptimizeSpeed, false);
#input_fn_name(&mut ctx);
ctx.flush();
let mut ctx = get_ctx::<HybridRenderer>(#width, #height, 0, "fallback", RenderMode::OptimizeSpeed, false);
for frame in 0..#frame_count {
if frame > 0 {
ctx.reset()
}
#draw_bg_snippet
#test_fn_call
ctx.flush();
}
if !#no_ref {
check_ref(&ctx, #input_fn_name_str, #webgl_fn_name_str, #hybrid_tolerance, #diff_pixels, false, #reference_image_name);
}
Expand Down Expand Up @@ -533,6 +583,9 @@ fn parse_args(attribute_input: &AttributeInput) -> Arguments {
"hybrid_tolerance" => {
args.hybrid_tolerance = parse_int_lit::<u8>(expr, "hybrid_tolerance");
}
"frame_count" => {
args.frame_count = parse_int_lit::<u32>(expr, "frame_count");
}
_ => panic!("unknown pair attribute {key_str}"),
}
}
Expand Down
3 changes: 3 additions & 0 deletions sparse_strips/vello_sparse_tests/snapshots/multi_frame_basic.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions sparse_strips/vello_sparse_tests/snapshots/multi_frame_with_clip_layer.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
31 changes: 31 additions & 0 deletions sparse_strips/vello_sparse_tests/tests/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,3 +547,34 @@ fn composite_to_pixmap_at_offset() {
"composite_to_pixmap_at_offset result should match direct rendering"
);
}

/// Ensure that fast path strips from the last frame don't leak into the next one.
#[vello_test(transparent, frame_count = 2)]
fn multi_frame_basic(ctx: &mut impl Renderer, frame: u32) {
let rect = Rect::new(15.0, 15.0, 85.0, 85.0);

if frame == 0 {
ctx.set_paint(RED.with_alpha(0.5));
} else {
ctx.set_paint(BLUE.with_alpha(0.5));
}

ctx.fill_rect(&rect);
}

/// Ensure that wide tiles are properly reset between two frames.
#[vello_test(transparent, frame_count = 2)]
fn multi_frame_with_clip_layer(ctx: &mut impl Renderer, frame: u32) {
let rect = Rect::new(15.0, 15.0, 85.0, 85.0);

ctx.push_clip_layer(&rect.to_path(0.1));

if frame == 0 {
ctx.set_paint(RED.with_alpha(0.5));
} else {
ctx.set_paint(BLUE.with_alpha(0.5));
}

ctx.fill_rect(&rect);
ctx.pop_layer();
}

@grebmeg grebmeg Mar 16, 2026

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.

What about adding more tests to check for state leaks, or even better a single test (or unify above tests) where you'd have clip, opacity layer, blends, etc.?

13 changes: 13 additions & 0 deletions sparse_strips/vello_sparse_tests/tests/renderer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pub(crate) trait Renderer: Sized {
fn set_blend_mode(&mut self, blend_mode: BlendMode);
fn set_filter_effect(&mut self, filter: Filter);
fn reset_filter_effect(&mut self);
fn reset(&mut self);
fn render_to_pixmap(&self, pixmap: &mut Pixmap);
fn width(&self) -> u16;
fn height(&self) -> u16;
Expand Down Expand Up @@ -207,6 +208,10 @@ impl Renderer for RenderContext {
Self::reset_filter_effect(self);
}

fn reset(&mut self) {
Self::reset(self);
}

fn render_to_pixmap(&self, pixmap: &mut Pixmap) {
Self::render_to_pixmap(self, pixmap);
}
Expand Down Expand Up @@ -448,6 +453,10 @@ impl Renderer for HybridRenderer {
self.scene.reset_filter_effect();
}

fn reset(&mut self) {
self.scene.reset();
}

// This method creates device resources every time it is called. This does not matter much for
// testing, but should not be used as a basis for implementing something real. This would be a
// very bad example for that.
Expand Down Expand Up @@ -793,6 +802,10 @@ impl Renderer for HybridRenderer {
self.scene.reset_filter_effect();
}

fn reset(&mut self) {
self.scene.reset();
}

// vello_hybrid WebGL renderer backend.
fn render_to_pixmap(&self, pixmap: &mut Pixmap) {
use web_sys::WebGl2RenderingContext;
Expand Down
18 changes: 4 additions & 14 deletions sparse_strips/vello_sparse_tests/tests/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ use smallvec::smallvec;
use std::cmp::max;
use std::sync::Arc;
use vello_common::color::DynamicColor;
use vello_common::color::palette::css::{BLUE, GREEN, RED, WHITE, YELLOW};
use vello_common::color::palette::css::{BLUE, GREEN, RED, YELLOW};
use vello_common::glyph::Glyph;
use vello_common::kurbo::{BezPath, Join, Point, Rect, Shape, Stroke, Vec2};
use vello_common::kurbo::{BezPath, Join, Point, Stroke, Vec2};
use vello_common::peniko::{Blob, ColorStop, ColorStops, FontData};
use vello_common::pixmap::Pixmap;
use vello_cpu::{Level, RenderMode};
Expand Down Expand Up @@ -100,7 +100,6 @@ macro_rules! load_image {
pub(crate) fn get_ctx<T: Renderer>(
width: u16,
height: u16,
transparent: bool,
num_threads: u16,
level: &str,
render_mode: RenderMode,
Expand Down Expand Up @@ -143,23 +142,14 @@ pub(crate) fn get_ctx<T: Renderer>(
_ => panic!("unknown level: {level}"),
};

let mut ctx = T::new(
T::new(
width,
height,
num_threads,
level,
render_mode,
default_blending_only,
);

if !transparent {
let path = Rect::new(0.0, 0.0, width as f64, height as f64).to_path(0.1);

ctx.set_paint(WHITE);
ctx.fill_path(&path);
}

ctx
)
}

pub(crate) fn render_pixmap(ctx: &impl Renderer) -> Pixmap {
Expand Down
Loading