Add `wgpu` crate features for backends by daxpedda · Pull Request #4815 · gfx-rs/wgpu

daxpedda · 2023-12-02T00:01:54Z

This adds crate features dx12 and metal to wgpu, which are enabled by default.
dx12 is no-op unless used on Windows.
metal is no-op unless used on MacOS/iOS.

Vulkan and OpenGL can currently not guarded by such a crate feature because it would mess up our defaults, e.g. a gles crate feature can not be enabled by default without also enabling if for MacOS and Windows, which is currently not what we want. See #3514 (comment) for more details.

Cc #3514.

daxpedda · 2023-12-02T00:04:15Z

I will add some documentation upon some initial feedback if we can proceed like this.

CI already tests --no-default-features, but doesn't test all sorts of crate feature combinations. Let me know if you want me to change/add something there.

daxpedda · 2023-12-02T00:10:38Z

I believe CI failure is unrelated.

Wumpf

I like where this is going (having an explicit feature for every platform backend) but I feel like we should first figure out what to do with Vulkan and Gl, otherwise this gets even more confusing for anyone trying to use wgpu and make a choice of feature flag.
Let's try finishing the discussion on #3514 first

wgpu/Cargo.toml

Wumpf · 2023-12-02T09:09:07Z

Hmm maybe we could leverage https://docs.rs/cfg_aliases/latest/cfg_aliases/ to detangle things a bit further. I wouldn't mind having something like gles-linux, gles-windows, gles-mac (which implies angle iff we're on mac) and then we use config aliases in the code to distill the real gles feature.

What do you think?

wgpu/src/backend/mod.rs

daxpedda · 2023-12-02T10:04:43Z

Hmm maybe we could leverage https://docs.rs/cfg_aliases/latest/cfg_aliases/ to detangle things a bit further. I wouldn't mind having something like gles-linux, gles-windows, gles-mac (which implies angle iff we're on mac) and then we use config aliases in the code to distill the real gles feature.

What do you think?

This would be nice if we can solve the crate feature issue. Unfortunately, e.g. gles-linux = ["wgc/gles"] would still pull in all OpenGL dependencies on MacOS as well.

I described this problem in more detail in #3514.

daxpedda · 2023-12-02T10:12:26Z

I believe the CI failure is unrelated, but it's consistent, so I assume it's broken on trunk as well if we rerun it there.
See #4813 for the same CI failure.

daxpedda · 2023-12-08T13:23:06Z

Rebased after #4828.
Which also fixed the CI.

Wumpf

we talked about this PR (and the general feature story) on our weekly wgpu maintainers call. Key takeaways:

reaffirming that we want to go the route of detailed opt-in features
this PR is as-is an improvement despite unsolved vulkan/gles stuff
going forward we want to rely on cfg_alias to reduce the complicated cfg expressions in the code (this also affects other stuff like the notoriously length cfg checks on send/sync on wasm)
if possible we don't want the dummy.rs way of not having a backend, but the rationale makes sense so if we don't find another way let's go for it
- maybe it's possible to fail instance creation before even instantiating any instance since everything already goes through DynContext 🤔 . I want to experiment with it before we commit to dummy.rs
we don't have a great out-of-the-box solution for the vulkan/gles problematic; handing more things down to wgc is definitely on the table

-> Action points for this PR before landing:

experiment with default-feature builds that don't rely on dummy.rs @Wumpf
changelog update

-> Next steps after this PR

iterate on GLES/Vulkan issue
start cfg_alias'ing things to make cfg configs nicer

daxpedda · 2023-12-14T15:34:22Z

if possible we don't want the dummy.rs way of not having a backend, but the rationale makes sense so if we don't find another way let's go for it

maybe it's possible to fail instance creation before even instantiating any instance since everything already goes through DynContext 🤔 . I want to experiment with it before we commit to dummy.rs

So I did remove the Dummy backend, instead I'm relying on hal::api::Empty to let everything compile. I believe right now it's not going to error on Instance::new(), but I'm not entirely sure where to implement returning the error.

Instance::new()
direct::Context::init()
wgc::Global::new()
wgc::Instance::new()

Let me know how you want me to proceed.

daxpedda · 2023-12-14T15:52:37Z

-> Action points for this PR before landing:

experiment with default-feature builds that don't rely on dummy.rs @Wumpf

changelog update

I covered both points now.
Whats still missing is figuring out where exactly we want to error out when no backend is available.

Otherwise the PR should be ready!

Wumpf · 2023-12-15T08:50:56Z

Ah nice, the empty feature makes actually a lot of sense, didn't think of that. What I actually meant is something along those lines
Wumpf@9c7f77068

Haven't tried it out yet, but this also gives a suggestion of when to panic in the stack: Right on wgpu::Instance. I.e. compared to the dummy solution we fail before the first call into the context. Also, I sketched this out to be information that's easily accessible to the user at compile time.
What do you think of that direction? (also my attempt to doc status quo in there surely has some mistakes?)

daxpedda · 2023-12-15T11:13:46Z

Panicking at wgpu::Instance looks fine to me. I don't think Instance::any_backend_feature_enabled() will actually be used by anybody? But it's a nice place to document all that, alternatively we can make the function private and put the documentation on Instance itself.

The whole hal::api::Empty is still necessary to make things compile, unless we want to put guards in all the functions in wgpu that use gfx_select!. E.g. your current solution in Wumpf@9c7f77068 won't compile with cargo build --target x86_64-apple-darwin -p wgpu --no-default-features.

(also my attempt to doc status quo in there surely has some mistakes?)

I left a comment here: Wumpf@9c7f77068#r135074861.
Except the crate features names it's great!

Let me know how you want to proceed, I'm happy to pull in your commit here.

EDIT: I made a commit with what I'm proposing. Happy to change it of course.

Wumpf · 2023-12-15T14:41:27Z

ah, thanks for fixing up the feature names and docs! Needing the hal::api::Empty makes a lot of sense, by basing off that version I meant to imply that it's included 😄

I'd really like any_backend_feature_enabled to be pub - I already can see that some library write wants to run all their tests in no-default-features and then wants a convenient runtime check to opt out of creating a wgpu instance, so I think it would be really useful to expose it. Unless you see some good reason better not to?

But I think that's all I have left to note. I'd would do a final pass over it tomorrow (or maybe later today) and then finally merge it in :)

…tures`

Co-Authored-By: Andreas Reich <1220815+Wumpf@users.noreply.github.com>

daxpedda · 2023-12-15T15:19:00Z

Needing the hal::api::Empty makes a lot of sense, by basing off that version I meant to imply that it's included 😄

Ah, my bad! I didn't even check 😅

I'd really like any_backend_feature_enabled to be pub - I already can see that some library write wants to run all their tests in no-default-features and then wants a convenient runtime check to opt out of creating a wgpu instance, so I think it would be really useful to expose it. Unless you see some good reason better not to?

No, that sounds absolutely fine to me! I've done that now.

Wumpf

Confirming that all looks good now. Great step forward, thanks @daxpedda for being patient and going through these iterations!

…225e1744fd7. r=webgpu-reviewers,supply-chain-reviewers,teoxoy Changelog: * #4865 fix present mode for wgl By xiaopengli89 in gfx-rs/wgpu#4865 * #4873 Bump ctor from 0.2.5 to 0.2.6 By dependabot[bot] in gfx-rs/wgpu#4873 * #4874 Bump syn from 2.0.40 to 2.0.41 By dependabot[bot] in gfx-rs/wgpu#4874 * #4869 [naga wgsl-out] Include the `f` suffix on `f32` literals. By jimblandy in gfx-rs/wgpu#4869 * #4850 [naga wgsl-in] Support abstract operands to binary operators. By jimblandy in gfx-rs/wgpu#4850 * #4850 [naga wgsl-in] Support abstract operands to binary operators. By jimblandy in gfx-rs/wgpu#4850 * #4850 [naga wgsl-in] Support abstract operands to binary operators. By jimblandy in gfx-rs/wgpu#4850 * #4850 [naga wgsl-in] Support abstract operands to binary operators. By jimblandy in gfx-rs/wgpu#4850 * #4850 [naga wgsl-in] Support abstract operands to binary operators. By jimblandy in gfx-rs/wgpu#4850 * #4870 [naga wgsl] Let unary operators accept and produce abstract types. By jimblandy in gfx-rs/wgpu#4870 * #4867 Reintroduce buffer snatching Part 1 By nical in gfx-rs/wgpu#4867 * #4882 Bump zerocopy from 0.7.26 to 0.7.31 By dependabot[bot] in gfx-rs/wgpu#4882 * #4878 Buffer snatching part 2 - Refactor create_buffer By nical in gfx-rs/wgpu#4878 * #4815 Add `wgpu` crate features for backends By daxpedda in gfx-rs/wgpu#4815 * #4887 Allow clippy::pattern_type_mismatch By nical in gfx-rs/wgpu#4887 * #4886 Document wgpu & wgpu-core features By Wumpf in gfx-rs/wgpu#4886 * #4826 validation: More detailed on incompatible BGL By scoopr in gfx-rs/wgpu#4826 * #4888 Web: add support for more `RawWindowHandle` variants By daxpedda in gfx-rs/wgpu#4888 * #4890 Bump thiserror from 1.0.50 to 1.0.51 By dependabot[bot] in gfx-rs/wgpu#4890 * #4880 Simplify `ResourceMaps` By nical in gfx-rs/wgpu#4880 * #4891 Make the naga version in trunk as high as the latest published one By nical in gfx-rs/wgpu#4891 * #4893 Avoid allocating memory every time we might log a label By nical in gfx-rs/wgpu#4893 * #4894 Remove some locks in BindGroup By nical in gfx-rs/wgpu#4894 * #4862 Ensure that DeviceLostCallbackC is always called exactly once By bradwerth in gfx-rs/wgpu#4862 * #4900 Support Device fence sharing with dx12 on Windows By sotaroikeda in gfx-rs/wgpu#4900 * #4903 Bump tokio from 1.35.0 to 1.35.1 By dependabot[bot] in gfx-rs/wgpu#4903 * #4895 Check that raw buffers and raw bind groups are valid By nical in gfx-rs/wgpu#4895 * #4901 fix: docs By miaobuao in gfx-rs/wgpu#4901 * #4892 Simplify some code around buffer unmapping By nical in gfx-rs/wgpu#4892 * #4896 Buffer snatching By nical in gfx-rs/wgpu#4896 * #4851 Eagerly release GPU resources when we lose the device. By bradwerth in gfx-rs/wgpu#4851 * #4906 Use nightly for docs By cwfitzgerald in gfx-rs/wgpu#4906 * #4908 Bump syn from 2.0.41 to 2.0.42 By dependabot[bot] in gfx-rs/wgpu#4908 * #4909 Bump profiling from 1.0.12 to 1.0.13 By dependabot[bot] in gfx-rs/wgpu#4909 * #4910 Bump anyhow from 1.0.75 to 1.0.76 By dependabot[bot] in gfx-rs/wgpu#4910 * #4913 Remove id32 Feature By cwfitzgerald in gfx-rs/wgpu#4913 * #4914 Add BGL Deduplication Index Test By cwfitzgerald in gfx-rs/wgpu#4914 * #4921 Fix typo "layout pipeline layout" -> "pipeline layout" By HactarCE in gfx-rs/wgpu#4921 * #4922 Bump winit from 0.29.4 to 0.29.5 By dependabot[bot] in gfx-rs/wgpu#4922 * #4924 Inline `document-features` usage, remove dep. By ErichDonGubler in gfx-rs/wgpu#4924 * #4871 Speed up Naga's `cargo xtask validate wgsl` from 12s to 0.8s By jimblandy in gfx-rs/wgpu#4871 * #4871 Speed up Naga's `cargo xtask validate wgsl` from 12s to 0.8s By jimblandy in gfx-rs/wgpu#4871 * #4871 Speed up Naga's `cargo xtask validate wgsl` from 12s to 0.8s By jimblandy in gfx-rs/wgpu#4871 * #4930 Bump winit from 0.29.5 to 0.29.6 By dependabot[bot] in gfx-rs/wgpu#4930 * #4929 Bump web-time from 0.2.3 to 0.2.4 By dependabot[bot] in gfx-rs/wgpu#4929 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver. By jimblandy in gfx-rs/wgpu#4902 * #4940 Align `wgpu_types::CompositeAlphaMode` serde serializations to spec By littledivy in gfx-rs/wgpu#4940 * #4936 Bump anyhow from 1.0.76 to 1.0.77 By dependabot[bot] in gfx-rs/wgpu#4936 * #4933 Bump thiserror from 1.0.51 to 1.0.52 By dependabot[bot] in gfx-rs/wgpu#4933 * #4932 Bump syn from 2.0.42 to 2.0.43 By dependabot[bot] in gfx-rs/wgpu#4932 Differential Revision: https://phabricator.services.mozilla.com/D197519

daxpedda requested a review from a team as a code owner December 2, 2023 00:01

Wumpf requested changes Dec 2, 2023

View reviewed changes

wgpu/Cargo.toml Show resolved Hide resolved

wgpu/Cargo.toml Outdated Show resolved Hide resolved

Wumpf reviewed Dec 2, 2023

View reviewed changes

wgpu/src/backend/mod.rs Outdated Show resolved Hide resolved

Wumpf mentioned this pull request Dec 3, 2023

Add a default feature for wgpu that enables Vulkan & GLES on Windows & Linux #3514

Closed

daxpedda force-pushed the wgpu-backend-features branch from 999372c to 4fc2d9f Compare December 8, 2023 13:14

cwfitzgerald self-requested a review December 12, 2023 18:41

Wumpf self-requested a review December 13, 2023 19:29

Wumpf reviewed Dec 13, 2023

View reviewed changes

daxpedda force-pushed the wgpu-backend-features branch from 8a6ca6d to aea4457 Compare December 14, 2023 15:36

daxpedda requested a review from Wumpf December 14, 2023 15:51

daxpedda and others added 6 commits December 15, 2023 16:18

Introduce dx12 and metal crate features to wgpu

7839bd5

Implement dummy Context to allow compilation with `--no-default-fea…

635c721

…tures`

Address review

d10f7d0

Remove dummy::Context in favor of hal::api::Empty

6b53f80

Add changelog entry

0989706

Panic early in Instance::new() if no backend is enabled

831e7fa

Co-Authored-By: Andreas Reich <1220815+Wumpf@users.noreply.github.com>

daxpedda force-pushed the wgpu-backend-features branch from 76c81b4 to 831e7fa Compare December 15, 2023 15:18

Wumpf approved these changes Dec 16, 2023

View reviewed changes

Wumpf merged commit 2053358 into gfx-rs:trunk Dec 16, 2023

daxpedda mentioned this pull request Dec 26, 2023

Add cfg_aliases to wgpu #4935

Merged

Wumpf mentioned this pull request Jan 18, 2025

Fix building wgpu with --no-default-feautures on web (wasm32-unknown-unknown) #6946

Merged

7 tasks

Conversation

daxpedda commented Dec 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daxpedda commented Dec 2, 2023

Uh oh!

daxpedda commented Dec 2, 2023

Uh oh!

Wumpf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Wumpf commented Dec 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

daxpedda commented Dec 2, 2023

Uh oh!

daxpedda commented Dec 2, 2023

Uh oh!

daxpedda commented Dec 8, 2023

Uh oh!

Wumpf left a comment

Choose a reason for hiding this comment

Uh oh!

daxpedda commented Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daxpedda commented Dec 14, 2023

Uh oh!

Wumpf commented Dec 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daxpedda commented Dec 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Wumpf commented Dec 15, 2023

Uh oh!

daxpedda commented Dec 15, 2023

Uh oh!

Wumpf left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

daxpedda commented Dec 2, 2023 •

edited

Loading

Wumpf commented Dec 2, 2023 •

edited

Loading

daxpedda commented Dec 14, 2023 •

edited

Loading

Wumpf commented Dec 15, 2023 •

edited

Loading

daxpedda commented Dec 15, 2023 •

edited

Loading