Skip to content

Revert "Build/Canvas: Vello or Vello CPU feature flags (#41686)"#41698

Merged
mrobinson merged 1 commit intoservo:mainfrom
mrobinson:revert-canvas-change
Jan 6, 2026
Merged

Revert "Build/Canvas: Vello or Vello CPU feature flags (#41686)"#41698
mrobinson merged 1 commit intoservo:mainfrom
mrobinson:revert-canvas-change

Conversation

@mrobinson
Copy link
Copy Markdown
Member

@mrobinson mrobinson commented Jan 5, 2026 •
edited
Loading

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.

@mrobinson mrobinson requested a review from sagudev as a code owner January 5, 2026 22:43
@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 6, 2026
TimvdLippe
TimvdLippe reviewed Jan 6, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

Maybe it should only be a compile error if you specify neither of the two options?

@mrobinson
Revert "Build/Canvas: Vello or Vello CPU feature flags (servo#41686)"
ed73544 
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]>
@mrobinson mrobinson force-pushed the revert-canvas-change branch from 0ab762e to ed73544 Compare January 6, 2026 08:46
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 6, 2026
@mrobinson mrobinson enabled auto-merge January 6, 2026 08:46
@mrobinson mrobinson added this pull request to the merge queue Jan 6, 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 6, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 6, 2026
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 6, 2026
@mrobinson mrobinson added this pull request to the merge queue Jan 6, 2026
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 6, 2026
@TimvdLippe
Copy link
Copy Markdown
Contributor

TimvdLippe commented Jan 6, 2026

@mrobinson did you see #41686 (comment) as well? I think this PR can be updated to only check for the "none selected" case to fix your issue as well as theirs.

@Narfinger
Copy link
Copy Markdown
Contributor

Narfinger commented Jan 6, 2026

Sorry I missed this. I can make a new PR for only if no one is enabled (it is basically modifying the cfg slightly). Would that be acceptable?

Merged via the queue into servo:main with commit 9ace4a1 Jan 6, 2026
39 checks passed
@mrobinson mrobinson deleted the revert-canvas-change branch January 6, 2026 10:04
@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 6, 2026
@mrobinson
Copy link
Copy Markdown
Member Author

mrobinson commented Jan 6, 2026

Currently I believe you need at least a single canvas implementation enabled, so we could make this situation impossible by having only a single compilation option that enables vello_gpu (disabled by default). That will make it easier to compile Servo with cargo, which is a long-term goal that we have. Otherwise, folks trying to compile with only cargo (currently unsupported, but hopefully possible one day) will need to provide a feature when compiling to avoid a build failures.

@mrobinson mrobinson mentioned this pull request Jan 6, 2026
Merged
@atouchet atouchet mentioned this pull request Jan 6, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TimvdLippe TimvdLippe TimvdLippe left review comments

@sagudev sagudev sagudev approved these changes

Assignees

No one assigned

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mrobinson @TimvdLippe @Narfinger @sagudev @servo-highfive