Upgrade to wgpu 27 and associated crates

[JMS55]

Supersedes #21725.

[github-actions]

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

[IceSentry]

LGTM other than CI failing on dlss_wgpu

[kpreid]

Note that experimental features are not just features that may not work properly — they are features that can cause undefined behavior, which is why this is unsafe. Consider that a user of Bevy rendering might prefer not to enter that territory. Are you sure that you want to enable this unconditionally?

Regardless of the answer to that question, this // SAFETY comment could be improved.

[JMS55]

In Bevy, we enable all wgpu features by default.

Then, specific plugins can check if certain features are enabled or not, and either say the plugin is not supported on this platform or fallback to other options.

I don't think there's much of a use case for limiting experimental features at the device level.

If you don't want e.g. raytracing to be available, you just shouldn't use Bevy plugins that depend on raytracing.

[kpreid]

• The intent of this wgpu v27 API change is that the user of the API is opting in to testing experimental features that may cause undefined behavior, as marked by an unsafe operation.
• Bevy here serves as a (very complex) API wrapping wgpu API.
• Therefore, Bevy should not be exposing experimental features that may cause undefined behavior, without similarly marking it (which could consist of simply having the ExperimentalFeatures token be passed into the plugin configuration — there doesn't have to be any actual unsafe fn in Bevy).

This is a (somewhat atypical) case of the general Rust principle that it should not be possible to cause UB via calling safe functions. “The user can just not call the function” is not considered sufficient in Rust.

[cc @cwfitzgerald who implemented this requirement in #8163]

[JMS55]

Therefore, Bevy should not be exposing experimental features that may cause undefined behavior, without similarly marking it

Generally this is done via feature flags in Bevy. For instance bevy_solari is not part of DefaultPlugins - you need to specifically compile with the feature for Bevy's raytracing functionality.

But we don't gate it at the device level, and I feel strongly that this is a good design choice for us, as otherwise it requires specific plugin setup ordering.

[kpreid]

“Can cause undefined behavior from safe code” makes something not a good design choice. Avoiding that situation is the whole point of Rust. Please don't do this. I speak both as a contributor to wgpu and a user of Bevy.

[JMS55]

My perspective is that 99% of users are not using experimental wgpu features directly. The only real people using them are developers of rendering plugins.

Bevy users should be able to trust that plugin authors used GPU features correctly, and not have to deal with enabling experimental flags themselves just to use a plugin.

We already disable stuff like checked shader code and indirect draw call validation in Bevy, it's trivial to misuse GPU APIs if you really want to.

[tychedelia]

Even if we used some kind of runtime check instead of feature flag a plugin could still set it without a user knowing.

Not advocating this necessarily, but it could be a flag you have to set on app, which plugins don't control. We could also have an unsafe plugin API which doesn't solve your plugins adding plugins but could help.

Bevy users should be able to trust that plugin authors used GPU features correctly, and not have to deal with enabling experimental flags themselves just to use a plugin.

IMO this is dangerously close to the "git gud" argument used by C++ partisans against safety in general. If anything, the fact that GPU APIs are easy to misuse makes safety more important. I also think it's important to distinguish UB on the GPU which sucks but is mostly limited to DoS and UB within the driver which is much more classically UB scary.

I take your point that the use of these features is already behind a flag and explicitly noted as experimental, but I think there's a tension here between having experimental features in tree (which inherently get a bit less review and testing, fewer developers working on them, etc) and safety.

I think an additional flag is a good practical middle ground. But minimally, whatever rationale we choose should be documented here more explicitly even if we are just okay blanket enabling this.

[cwfitzgerald]

I think this is an important discussion to have, but I don't think this is the place to have it. This PR is not-worse than the state of v26, wgpu has just improved its ability to properly differentiate features that are potentially hazardous. I think bevy should land this as-is, then have the discussion about how to properly handle it after this lands. Getting wgpu v27 in bevy 0.18 is more important.

[kpreid]

That seems a reasonable pragmatic approach, but then the comment should reflect that. Right now the text of the safety comment (that this review thread is attached to) does not acknowledge that there are safety (or, more precisely, soundness) considerations here, other than literally starting with “SAFETY:”. It should be updated to say that there is more work to be done — perhaps linking to an issue where the proper safety discussion can be had.

[alice-i-cecile]

I've committed the suggestion by @mockersf to replace this with a TODO.

I agree that this is no worse than the status quo.

[tychedelia]

LGTM provided we resolve the safety question.

[mockersf]

crashes in CI and on my mac with:

wgpu error: Validation Error

Caused by:
  In a CommandEncoder
    A debug group was not popped before the encoder was finished

possibly related to #22003, @IceSentry any idea? It stops crashing if I revert that PR

[cwfitzgerald]

I had this too on my codebase - something was fixed about debug group enforcement in 27. Afaik it's correct, so there are incorrect debug groups somewhere.

[IceSentry]

possibly related to #22003, @IceSentry any idea? It stops crashing if I revert that PR

I can confirm it's related to my PR. I have no idea why it crashes though. My guess is I'm not handling an exit condition so the pop_debug_group never gets called. The issue is, this code hasn't changed so I'm not sure why it exits early.

[cwfitzgerald]

The issue is, this code hasn't changed so I'm not sure why it exits early.

I don't think we were properly enforcing push/pop matching in v26, so I suspect this was always a problem.

[IceSentry]

Here's a temporary fix for the debug_group issue #22079

It seems to me like the wgpu validation is now too strict considering it worked fine before but this PR uses debug_markers for now until I find an alternative way to set a debug_group around multiple command encoders.