Make wgpu-hal crate features target aware

[daxpedda]

I noticed that the way wgpu-hals crate features are designed is target independent.

The problem is of course that using --all-features or any other non-compatible crate feature will break compilation. See #3463 for example.

I propose that activating crate features on wgpu-hal that only work on specific targets should be no-op on non-compatible targets.
For example activating the vulkan crate feature on wasm32-unknown-unknown should do nothing.

Additionally I think removing

wgpu/wgpu-hal/src/lib.rs

Lines 54 to 61 in 5b8c55c

	#[cfg(not(any(
	feature = "dx11",
	feature = "dx12",
	feature = "gles",
	feature = "metal",
	feature = "vulkan"
	)))]
	compile_error!("No back ends enabled in `wgpu-hal`. Enable at least one backend feature.");

would also remove some unnecessary barriers.

I think the only downside this change would introduce is that you could have activated crate features that you don't really need for your target, which shouldn't really have any downside if they are no-op.

[grovesNL]

Thanks for raising this issue!

I think we'd like to be able to rely on the metal feature to link to Apple libraries at compile-time, so trying to hide the backends feels like it would be complicated.

Similarly we don't allow cross compilation of some backends (e.g., cross compiling the Metal backend on Linux) so I think it would be better to fail at compile-time to let the person know instead of runtime with a no-op.

Is there a reason we need to support --all-features? Usually we can specify which features are supported based on the host/target.

[daxpedda]

Is there a reason we need to support --all-features? Usually we can specify which features are supported based on the host/target.

It is not a "need", there are plenty of ways to work around that. Mainly I was motivated by making the docs.rs build work, because right now it is using --all-features. We can of course just not use that, but docs.rs will just not have full documentation for all the targets then. It is unfortunate that there is no support for target specific features on docs.rs.

It is also beneficial for downstream users, specifically reducing the amount of cfgs in Cargo.toml would improve the user experience and alleviate any potential pitfalls with it (although it introduces new ones, even if minor).

I think we'd like to be able to rely on the metal feature to link to Apple libraries at compile-time, so trying to hide the backends feels like it would be complicated.

Could you elaborate on that? How does relying on the metal feature help you with linking at compile-time?

Similarly we don't allow cross compilation of some backends (e.g., cross compiling the Metal backend on Linux) so I think it would be better to fail at compile-time to let the person know instead of runtime with a no-op.

I'm not following here, I can't see that the current crate feature system disallows cross compilation anywhere. Could you point me into the right direction?

---

Lots of crates do it like this, think of winit or wasm_bindgen for example.

I was under the impression that such a change really would have no downsides except the fact that it is a change and people have to be aware that using these target features is no-op on certain systems. I would like to better understand any potential issues here.

[grovesNL]

We can of course just not use that, but docs.rs will just not have full documentation for all the targets then. It is unfortunate that there is no support for target specific features on docs.rs.

I'm not sure if this is possible because we can't certain build dependencies of the metal backend on docs.rs for the same reason (docs.rs build agent/Linux host trying to cross-compile macOS), e.g., https://docs.rs/crate/metal/0.24.0

I'm not following here, I can't see that the current crate feature system disallows cross compilation anywhere. Could you point me into the right direction?

I mean being able to disable/enable features conditionally based on the target by default. e.g.,

wgpu/wgpu/Cargo.toml

Lines 115 to 117 in 9908f3e

	[target.'cfg(any(target_os = "macos", target_os = "ios"))'.dependencies.wgc]
	workspace = true
	features = ["metal"]

enables Metal for target_os = "macos" which is convenient because some dependencies for that backend are gated in the same way

wgpu/wgpu-hal/Cargo.toml

Lines 98 to 101 in 9908f3e

	[target.'cfg(any(target_os="macos", target_os="ios"))'.dependencies]
	mtl = { package = "metal", version = "0.24.0" }
	objc = "0.2.5"
	core-graphics-types = "0.1"

Ideally we'd like to say that certain features can only be enabled on specific targets but as far as I know there isn't a way to express target-specific features in Cargo.toml yet.

It is also beneficial for downstream users, specifically reducing the amount of cfgs in Cargo.toml would improve the user experience and alleviate any potential pitfalls with it (although it introduces new ones, even if minor).

I don't think cfgs should be needed for crates downstream of wgpu in general, except when using wgpu-core or wgpu-hal directly (to do target-specific things). That seems pretty reasonable though, because the raw backend types might not be available depending on the target.

In case it's helpful, the discussion at #1221 is also relevant and talks about the issue with target-specific features.

[daxpedda]

I mean being able to disable/enable features conditionally based on the target by default. e.g.,

That ability doesn't go away with what I'm proposing, see #3466.

I don't think cfgs should be needed for crates downstream of wgpu in general, except when using wgpu-core or wgpu-hal directly (to do target-specific things).

Indeed, I was specifically talking about wgpu-core and wgpu-hal. I never use them though, so probably shouldn't talk about it if I don't have a clue.

In case it's helpful, the discussion at #1221 is also relevant and talks about the issue with target-specific features.

That is really interesting, I think my proposal here would address this issue nicely!
Because now wgpu can just activate all features by default: default-features = false would still work while leaving the default the same as it is now.

[teoxoy]

I left a comment on #1221 (comment). I think that one is more about disabling the GLES and DX11 backends.