allow user-defined mesh-view bindings

[robtfm]

Objective

allow user-defined mesh-view bindings to be added for use in custom materials or user-overridden pbr functions.

Solution

• add resources UserViewBindingsLayouts and UserViewBindingsEntries, add the layouts to MeshPipeline's view_layout at build time and then add entries to the bind group each frame
• add a bevy_pbr::mesh_view_user_bindings wgsl import which can be overridden to pull the user bindings into the shaders
• add an example custom_view_bindings to show how to use it

the layouts are keyed by a string, so that a plugin doesn't need to know what other plugins are adding view bindings.
unfortunately if multiple plugins with view bindings are being used, the app author will need to make their own composite bevy_pbr::mesh_view_user_bindings containing the bindings from all the plugins (and they will need to have no overlap in names).

this is pretty ugly so i welcome any suggestions to improve it, and i won't be upset if it's not accepted - i think the functionality is needed but this may be too hacky.

i can't think of a clean way to specify the bindings per-plugin without extending the preprocessor, and i don't want to do that without feedback on the approach first.

[IceSentry]

I agree that the general concept would be really nice to have, but asking users to compose file goes way too much against the easy modularity goals of bevy.

I haven't looked at the code yet, but I assume the reason for that is that the bind group index needs to be specified in the shader. My suggestion for that is to inject the index with a shader def. This way, plugin authors could simply add a #import BIND_GROUP_INDEX where the index would normally go in the binding() annotation. That way authors don't need to know or care about the index and bevy can set it in whatever order the plugin is added.

[robtfm]

the bind group index needs to be specified in the shader

yes exactly.

inject the index with a shader def

shader defs (in VertexState) are currently strings which are defined or not, not key/value pairs, so that won't work as it stands. as well, multiple plugins trying to replace the same #import name will just overwrite each other currently.

so i guess i would add another vec to the UserViewBindingsLayouts resource into which plugins can put handles to their binding shaders (and count of bindings they use), and do a custom preprocess search/replace of the binding offset to build the final user bindings shader import.

[IceSentry]

Yeah, to be clear I don't think shader defs allows that right now. I meant that it could be extended and use to do something like that. I had in mind something like you are suggesting with a search and replace yes.

[robtfm]

added a search/replace on user defined binding strings (which must be WGSL) so that users don't need to worry about binding index with multiple plugins, updated the example.

still pretty ugly but much less error prone.

(as an aside, running the clippy warnings locally always fails for me in crates\bevy_tasks\src\task_pool.rs:153:56 so i have to wait for ci to tell me what i got wrong, is there a workaround for that?)

[aevyrie]

(as an aside, running the clippy warnings locally always fails for me in crates\bevy_tasks\src\task_pool.rs:153:56 so i have to wait for ci to tell me what i got wrong, is there a workaround for that?)

Have you tried running cargo run -p ci? https://github.com/bevyengine/bevy/blob/b399a374cb407ac94aaeb6755c06b9a3a559ec6a/docs/linters.md#code-linting-with-clippy

[robtfm]

Have you tried running cargo run -p ci?

yes, it gives the same output:

error: deref which would be done by auto-deref
   --> crates\bevy_tasks\src\task_pool.rs:153:56
    |
153 |             let executor: &async_executor::Executor = &*self.executor;
    |                                                        ^^^^^^^^^^^^^^ help: try this: `self.executor`
    |
    = note: `-D clippy::explicit-auto-deref` implied by `-D warnings`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref

[robtfm]

closing in favour of #6738