Skip to content

Build/Canvas: Vello or Vello CPU feature flags#41686

Merged
TimvdLippe merged 1 commit intoservo:mainfrom
Narfinger:vello_or_vello_cpu
Jan 5, 2026
Merged

Build/Canvas: Vello or Vello CPU feature flags#41686
TimvdLippe merged 1 commit intoservo:mainfrom
Narfinger:vello_or_vello_cpu

Conversation

@Narfinger
Copy link
Copy Markdown
Contributor

@Narfinger Narfinger commented Jan 5, 2026

Simple compile time check for only allowing one backend to be enabled. The error messages are sadly still full of other random errors but at least if people scroll up enough they are going to see the compile error.

Signed-off-by: Narfinger [email protected]

Testing: Tested with both flags and without both flags and both time we get an error.
Fixes: #40779

Signed-off-by: Narfinger <[email protected]>
@Narfinger Narfinger marked this pull request as ready for review January 5, 2026 14:05
@Narfinger Narfinger requested a review from sagudev as a code owner January 5, 2026 14:05
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 5, 2026
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jan 5, 2026
@TimvdLippe TimvdLippe added this pull request to the merge queue Jan 5, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 5, 2026
Merged via the queue into servo:main with commit fb239a1 Jan 5, 2026
33 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 5, 2026
@Narfinger Narfinger deleted the vello_or_vello_cpu branch January 5, 2026 16:19
@mrobinson
Copy link
Copy Markdown
Member

I think this change should be reverted. It is not an error to enable both canvas backends and they are configurable via a runtime option. With the change the runtime option no longer makes sense. It is a problem to not enable either.

One potential fix would to make it so that the compile-time option only configures whether vello_gpu is enabled.

mrobinson added a commit to mrobinson/servo that referenced this pull request Jan 5, 2026
This reverts commit fb239a1.

This change makes it impossible to enable both canvas backends at the
same time which isn't a problem at all.
@Narfinger
Copy link
Copy Markdown
Contributor Author

The above PR can be easily changed to have the error only when no backend is selected. Is this preferable?

mrobinson added a commit to mrobinson/servo that referenced this pull request Jan 6, 2026
This reverts commit fb239a1.

This change makes it impossible to enable both canvas backends at the
same time which isn't a problem at all.

Signed-off-by: Martin Robinson <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jan 6, 2026
)

This reverts commit fb239a1 (#41686).

This change makes it impossible to enable both canvas backends at the
same time which isn't a problem at all.

Signed-off-by: Martin Robinson <[email protected]>
@mrobinson
Copy link
Copy Markdown
Member

I left a comment on #41698 about this, but essentially if we are working toward being able to build Servo with just cargo, it would be preferable to remove the compilation option for vello_cpu.

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.

Better error message if libservo is compiled without vello and vello_cpu

4 participants