Skip to content

Port some tests to the new version of Vello Api#1369

Open
DJMcNab wants to merge 7 commits intolinebender:mainfrom
DJMcNab:api-tests
Open

Port some tests to the new version of Vello Api#1369
DJMcNab wants to merge 7 commits intolinebender:mainfrom
DJMcNab:api-tests

Conversation

@DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented Jan 20, 2026

This uses the version of Vello API which landed in #1360.

This code was mostly adapted from #1299, but changed to reflect the lack of Renderer trait.

These tests are duplicates of some existing tests, rather than new tests; indeed, they use the existing reference image for these tests. This is because we expect Vello API's API to still change substantially (especially around how brushes are handled), so keeping that localised/in a low merge-conflict way is the goal.

@nicoburns
Copy link
Contributor

Some thoughts:

  • Would it not make sense to move the tests rather than duplicate them? Duplicate tests don't seem like a fun thing to maintain, and isn't the purpose of moving the tests to the abstraction to reduce duplication (one set of tests covers all renderers). If the abstraction isn't ready for that yet, then maybe we should just wait?
  • This seems like where we could really do with a Vello Classic backend. Presumably we'd also like to run the same tests there.

@DJMcNab
Copy link
Member Author

DJMcNab commented Jan 20, 2026

Would it not make sense to move the tests rather than duplicate them? Duplicate tests don't seem like a fun thing to maintain, and isn't the purpose of moving the tests to the abstraction to reduce duplication (one set of tests covers all renderers). If the abstraction isn't ready for that yet, then maybe we should just wait?

It's possible. I still do lean slightly towards duplicating them. The reasoning for this approach was to provide a testbed for the new Vello API work, rather than for actual test use. That is, I didn't think these tests don't need to be maintainable as such, because they'll eventually need to be blasted away and re-synced when we port the rest of the tests.
Essentially, this is recognising that Vello API is still experimental (and underpowered, e.g. no images), so is currently only viable for porting a subset of tests.

With that said, I don't strongly have a position one way or the other. I certainly wouldn't block a follow-up PR to delete the duplicated tests. This feels like something to potentially discuss in office hours.

This seems like where we could really do with a Vello Classic backend. Presumably we'd also like to run the same tests there.

This PR seems an odd place for this concern; I do agree that running many of the same tests in Vello Classic would be nice, but it's a tradeoff around potentially making Vello API less fit-for-purpose (e.g. we're not likely to ever support "global" layers in Vello Classic, but the current Vello API does support them).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments