Skip to content
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.

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

Replace vello's default wgpu feature with an optional wgpu feature #17

Merged
merged 1 commit into from
May 5, 2024

Conversation

MarijnS95
Copy link
Contributor

@MarijnS95 MarijnS95 commented Apr 29, 2024

linebender/vello#359 made wgpu an optional but default dependency/feature, but any crate using velato will be forced to opt in to wgpu due to not disabling the default here.

Since velato does not need any rendering backend at all, this feature should be disabled by default. As the vello crate is reexported from the root, a vello-wgpu passthrough feature is provided so that users can turn it back on without explicitly depending on vello in their crate dependencies.


Note that with_winit runs briefly (with wgpu), but if I don't manage to immediately fullscreen it, it panics:

thread 'main' panicked at examples/with_winit/src/lib.rs:436:30:
failed to get surface texture: Timeout

This is on a Wayland compositor with AMD GPU. I'm not a WGPU expert and don't dare to immediately say where it is being used wrong.

@MarijnS95
Copy link
Contributor Author

Looks like the same fix needs to be applied to vello_svg.

@simbleau
Copy link
Member

Can you explain why we wouldn't want the default feature for wgpu?

@MarijnS95
Copy link
Contributor Author

So that crates using vello with their own rendering backend - without wgpu - can use the Lottie loader/render implementation provided here, without pulling that dependency in? Same reasoning as linebender/vello#359 that did it for the vello crate.

@simbleau
Copy link
Member

Please update the CHANGELOG and I can merge this

@MarijnS95
Copy link
Contributor Author

Before pushing an updated changelog: since this crate reexports vello from the root, does it make sense to forward the wgpu feature by adding wgpu = ["vello/wgpu"]?

@simbleau
Copy link
Member

Yeah, let's do that.

@MarijnS95 MarijnS95 force-pushed the vello-no-default branch from 90c3063 to a9f2971 Compare May 3, 2024 09:25
@MarijnS95
Copy link
Contributor Author

@simbleau done.

@MarijnS95 MarijnS95 changed the title Disable default features on vello to remove wgpu Replace vello's default wgpu feature with an optional vello-wgpu feature May 3, 2024
Copy link
Member

@simbleau simbleau left a comment

Choose a reason for hiding this comment

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

Let's change that feature name to just wgpu.

…ature

linebender/vello#359 made `wgpu` an optional
but default dependency/feature, but any crate using `velato` will be
forced to opt in to `wgpu` due to not disabling the default here.

Since `velato` does not need any rendering backend at all, this feature
should be disabled by default.  As the `vello` crate is reexported from
the root, a `wgpu` passthrough feature is provided so that users can
turn it back on without explicitly depending on `vello` in their crate
dependencies.
@MarijnS95 MarijnS95 force-pushed the vello-no-default branch from a9f2971 to 0119c07 Compare May 4, 2024 19:09
github-merge-queue bot pushed a commit to linebender/vello_svg that referenced this pull request May 5, 2024
…feature (#10)

linebender/vello#359 made `wgpu` an optional but
default dependency/feature, but any crate using `vello_svg` will be
forced to opt in to `wgpu` due to not disabling the default here.

Since `vello_svg` does not need any rendering backend at all, this
feature should be disabled by default. As the `vello` crate is
reexported from the root, a `vello-wgpu` passthrough feature is provided
so that users can turn it back on without explicitly depending on
`vello` in their crate dependencies.

Same change as linebender/velato#17.
Copy link
Member

@simbleau simbleau left a comment

Choose a reason for hiding this comment

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

Thanks!

github-merge-queue bot pushed a commit to linebender/vello_svg that referenced this pull request May 5, 2024
#10)

linebender/vello#359 made `wgpu` an optional but
default dependency/feature, but any crate using `vello_svg` will be
forced to opt in to `wgpu` due to not disabling the default here.

Since `vello_svg` does not need any rendering backend at all, this
feature should be disabled by default. As the `vello` crate is
reexported from the root, a `vello-wgpu` passthrough feature is provided
so that users can turn it back on without explicitly depending on
`vello` in their crate dependencies.

Same change as linebender/velato#17.
@simbleau simbleau merged commit 0283f38 into linebender:main May 5, 2024
9 checks passed
@MarijnS95 MarijnS95 changed the title Replace vello's default wgpu feature with an optional vello-wgpu feature Replace vello's default wgpu feature with an optional wgpu feature May 5, 2024
@MarijnS95 MarijnS95 deleted the vello-no-default branch May 5, 2024 16:35
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