Ugrade to wgpu version 25.0

[tychedelia]

Objective

Upgrade to wgpu version 25.0.

Depends on bevyengine/naga_oil#121

Solution

Problem

The biggest issue we face upgrading is the following requirement:

To facilitate this change, there was an additional validation rule put in place: if there is a binding array in a bind group, you may not use dynamic offset buffers or uniform buffers in that bind group. This requirement comes from vulkan rules on UpdateAfterBind descriptors.

This is a major difficulty for us, as there are a number of binding arrays that are used in the view bind group. Note, this requirement does not affect merely uniform buffors that use dynamic offset but the use of any uniform in a bind group that also has a binding array.

Attempted fixes

The easiest fix would be to change uniforms to be storage buffers whenever binding arrays are in use:

#ifdef BINDING_ARRAYS_ARE_USED
@group(0) @binding(0) var<uniform> view: View;
@group(0) @binding(1) var<uniform> lights: types::Lights;
#else
@group(0) @binding(0) var<storage> view: array<View>;
@group(0) @binding(1) var<storage> lights: array<types::Lights>;
#endif

This requires passing the view index to the shader so that we know where to index into the buffer:

struct PushConstants {
    view_index: u32,
}

var<push_constant> push_constants: PushConstants;

Using push constants is no problem because binding arrays are only usable on native anyway.

However, this greatly complicates the ability to access view in shaders. For example:

#ifdef BINDING_ARRAYS_ARE_USED
mesh_view_bindings::view.view_from_world[0].z
#else
mesh_view_bindings::view[mesh_view_bindings::view_index].view_from_world[0].z
#endif

Using this approach would work but would have the effect of polluting our shaders with ifdef spam basically everywhere.

Why not use a function? Unfortunately, the following is not valid wgsl as it returns a binding directly from a function in the uniform path.

fn get_view() -> View {
#if BINDING_ARRAYS_ARE_USED
    let view_index = push_constants.view_index;
    let view = views[view_index];
#endif
    return view;
}

This also poses problems for things like lights where we want to return a ptr to the light data. Returning ptrs from wgsl functions isn't allowed even if both bindings were buffers.

The next attempt was to simply use indexed buffers everywhere, in both the binding array and non binding array path. This would be viable if push constants were available everywhere to pass the view index, but unfortunately they are not available on webgpu. This means either passing the view index in a storage buffer (not ideal for such a small amount of state) or using push constants sometimes and uniform buffers only on webgpu. However, this kind of conditional layout infects absolutely everything.

Even if we were to accept just using storage buffer for the view index, there's also the additional problem that some dynamic offsets aren't actually per-view but per-use of a setting on a camera, which would require passing that uniform data on every camera regardless of whether that rendering feature is being used, which is also gross.

As such, although it's gross, the simplest solution just to bump binding arrays into @group(1) and all other bindings up one bind group. This should still bring us under the device limit of 4 for most users.

Next steps / looking towards the future

I'd like to avoid needing split our view bind group into multiple parts. In the future, if wgpu were to add @builtin(draw_index), we could build a list of draw state in gpu processing and avoid the need for any kind of state change at all (see gfx-rs/wgpu#6823). This would also provide significantly more flexibility to handle things like offsets into other arrays that may not be per-view.

Testing

Tested a number of examples, there are probably more that are still broken.

[github-actions]

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

[IceSentry]

What's the purpose of this empty_layout?

[tychedelia]

Wgpu gets upset if you have a gap in bind groups i guess. should double check that this doesn't have any perf implications but it seems to work.

[mockersf]

could you also update codespan-reporting? #18508

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19563

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19563

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19563

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

[Elabajaba]

Need to add the vulkan and webgl features now.

Maybe gles as well, but in my experience it hasn't worked in years and just spits out confusing error messages.

[tychedelia]

we have a separate top level feature webgl so i don't htink we want it here? i included gles just because we still have code for it.

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19563

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

[hukasu]

instance_id and instance_index are both valid fields? are they built-in or are they bevy defined?

[mockersf]

Example run is good apart from taking way too long on DX12 which can't be reproduced locally 👍

[JMS55]

@tychedelia need to run rustfmt.

[tychedelia]

This should be good now sans the naga_oil release.

[Elabajaba]

@blend_src(0) doesn't work if the dual_source_blending feature isn't enabled, so the atmosphere shader needs a bit more #ifdef spaghetti. (sorry for splitting it up into 2 suggestions, but github won't let me do it as a single suggestion due to the deleted lines in the middle...).

[Elabajaba]

If we're telling users to use group 3 for their material uniforms, doesn't that mean that we can't put our bindless stuff in group 3? (eg. crates/bevy_render/src/bindless.wgsl or assets/shaders/bindless_material.wgsl)

[tychedelia]

I was being a bit too loose so just pushed some clarifying language. I don't think our existing bindless mechanism via AsBindGroup lets users bind uniform buffers in combination with binding arrays anyway, despite the attr being named #[uniform] it puts everything in storage afaict.