vello_sparse_tests: Add support for multi-frame tests#1501
Conversation
laurenz-canva
force-pushed
the
laurenz/multi-frame
branch
from
614a2c1 to
23a0de9
Compare
| 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() |
There was a problem hiding this comment.
| ctx.reset() | |
| ctx.reset(); |
There was a problem hiding this comment.
Not sure I understand the reasoning here?
There was a problem hiding this comment.
Should there be semicolons after resetting? Sorry for removing the spaces (now fixed).
| /// 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, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
|
|
||
| ctx.fill_rect(&rect); | ||
| ctx.pop_layer(); | ||
| } |
There was a problem hiding this comment.
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.?
Co-authored-by: Alex Gemberg <gemberg@canva.com>
4 participants
The main intention behind this is to have infrastructure in place to better test glyph caching (where there are a range of bugs that only become evident when rendering more than one frame), to detect issues like the one in #1500.