Add optional shared bind group to custom materials

[marlyx]

Objective

Add the possibility for all instances of a custom material to have a shared data set (Uniforms, Textures, Samplers, etc.) or multiple custom materials to share a common data set.

This can be used for things like:

• Material instance common data. textures, world information, etc. (e.g. Environment global wind direction can be used in multiple instances of a custom shader)
• Global shared render information like decal positions and decal textures.
• Engine/App variables like Time can be uploaded only once per frame and then used by multiple materials. (Even though time probably should be addressed by something like: [Merged by Bors] - add globals to mesh view bind group #5409)

In my project the need arose when trying to visualize my world grid in multiple shaders.

Solution

• Add a secondary generic parameter to MaterialPlugin<M, G> that specify a resource that can be turned into a shared bind group (must impl AsBindGroup). This resource can either use the derive(AsBindGroup) to create a group of shared static data (e.g. shared textures, samplers) or manually implemented for user managed buffers etc.

• Update MaterialPlugin to bind the SharedGroup bind group to slot 3 if present. (If no shared group is specified the code path should be identical to pre- this patch)

• Add a convenience plugin that will fetch a resource that impl AsBindGroup and prepares it for use as a shared bind group in the material plugin.

Alternate Solution:

• Use an associated type in the Material trait (instead of the extra generic parameter G) to bind the material to a shared group resource. Pros: Connects the material to its shared group in a slightly more logical place (subjective). Cons: Forces an extra line of boilerplate on all custom materials no matter if you use the feature or not.

Example

Added a new example showing this feature in action. Here two materials are created that both are aware of the position and parameters of a "force field" to create different effects.

---

Changelog

• Changed MaterialPlugin to support an optional user-specified shared bind group parameter
• Added SharedBindGroupPlugin to help prepare shared bind groups

Migration Guide

• None

[marlyx]

Hi, First time pull-request here :). Hope the description makes sense. Happy for any feedback.

[IceSentry]

First of all, the PR description is well made and I like the feature, unfortunately I personally dislike some things about the implementation.

I'm not a fan of using an associated type for that. It adds complexity to all uses of Material even if most Material don't need shared data. I also don't like the need for a new branch everywhere you want to add a draw command that could potentially make use of the shared data.

I don't think this should be an alternative to adding a time/globals bind group. This is something that is useful in enough situations to justify a permanent bind group and most other engines also offer similar features. Especially if people want to have more things other than time, it would be annoying that they would now need to merge them somehow.

[IceSentry]

This needs a lot more documentation to explain what it is and how to use it.

[IceSentry]

See previous comment

[marlyx]

Fully agree with you on basically all accounts.

I'm not a fan of using an associated type for that. It adds complexity to all uses of Material even if most Material don't need shared data.

Me neither, it is unfortunate that it forces boilerplate on all implementations. I will try an implementation of the alternate (in the description) and see how that goes.

I also don't like the need for a new branch everywhere you want to add a draw command that could potentially make use of the shared data.

Are you referring to the two sets of draw commands (DrawWithoutSharedGroup and DrawWithSharedGroup)? Honestly I thought these where only meant for internal use within the MaterialPlugin, but maybe I have missunderstood their full purpose.

I don't think this should be an alternative to adding a time/globals bind group. This is something that is useful in enough situations to justify a permanent bind group and most other engines also offer similar features. Especially if people want to have more things other than time, it would be annoying that they would now need to merge them somehow.

Agreed, personally I would like both a standard set of often used globals (time, etc.) and the option to add your own. In my case some map data for my grid.

[IceSentry]

Are you referring to the two sets of draw commands (DrawWithoutSharedGroup and DrawWithSharedGroup)? Honestly I thought these where only meant for internal use within the MaterialPlugin, but maybe I have missunderstood their full purpose.

Yes, but reading the code again, it's not as bad as I thought. It's still a bit unfortunate that it needs to separate sets of draw commands.

[marlyx]

Changed the implementation to use a optional (defaulted to ()) generic parameter to the MaterialPlugin instead of the associated type on the Material trait. This changes the feature to be purely additive rather than requiring changes in existing code. No more added boilerplate when not using a shared bind group.

[superdump]

I definitely appreciate the need that you put forth in the description and you clarify your approach well.

A couple of things come to mind:

• When bind groups are bound, all bind groups with higher indices also have to be rebound. Binding has a cost so this performs less well if the bind rate is not from least (e.g. view or global bindings in group 0) to most (e.g. per-draw mesh bindings in group 2, currently.)
• The approach I have been thinking about is to make it possible to configure the different bind groups. The bind group layouts can be modified through pipeline specialisation but there is no easy way to create the bind groups. AsBindGroup only supports a type including an entire binding, and that doesn’t help with, say, if you want to add some global or low-bind-rate data to bind group 0. So I’m trying to think about a flexible and efficient way of supporting composable bind group layouts and perhaps an easier and more flexible way to create bind groups. AsBindGroup also creates a separate buffer for each instance of that type. That maybe works fine for one material in our current one Mesh means one draw setup, but we want to move toward batching draw calls and ultimately bindless textures which then means trying to move toward having all instance data in an array in one buffer, storing indices in appropriate ways to look up that data.

So, I want to peel back AsBindGroup a bit to ideally have methods that return bind group layout entries and bind group entries or something like that such that one can say ‘I want the view uniform binding in bind group 0 binding 0, and this noise texture and sampler in bind group 0 bindings 1 and 2’ etc. I don’t know what @cart thinks about this exactly. I think they’ll be on board with the flexibility as long as the ergonomics and performance are there. :) And if we take a plain data only material asset (ie no textures/samplers) as an example, I want those to be written into arrays in single buffers and the array index to be stored in a component on the render world entity, kind of like the mesh uniform dynamic offset stuff, but array indices instead of buffer offsets.

I started work on all this but unfortunately I don’t currently have the time to continue.

[marlyx]

Alright, this is very interesting.

• When bind groups are bound, all bind groups with higher indices also have to be rebound. Binding has a cost so this performs less well if the bind rate is not from least (e.g. view or global bindings in group 0) to most (e.g. per-draw mesh bindings in group 2, currently.)

Ah, that makes some things a lot clearer. I knew that the best/common practice was to group the shader resources by frequency to minimize binds/rebinds, but not that the actual index mattered. That., of course, makes my proposed implementation less than ideal to say the least.

So, I want to peel back AsBindGroup a bit to ideally have methods that return bind group layout entries and bind group entries or something like that such ...

Yes, I was thinking the same thing while diggin' through the code for this implementation. For me it was simply the fact that after the .bind_group_layout(...) method is called there is no way for the caller to know what is actually in the layout. For instance in this feature, being able to know if the layout is empty would be valuable to opt for a code path with no binding at all.

• The approach I have been thinking about is to make it possible to configure the different bind groups. ...

This would be fantastic. While working on this I was thinking of something similar where the material system have couple of named contexts or "buckets" for shader resources defined. E.g. GLOBAL, SHADER, MATERIAL, MESH. Where each correspond to a update frequency or basically nesting depth in the render loop.
Then the AsBindGroup becomes something akin to a AsResourceSet instead that only return a collection of resource descriptors that are named and assigned to contexts. The material system would then be in charge of creating the actual layouts and groups enabling some optimizations like removing empty groups, sampler deduplication etc.
On the shader side some preprocessor magic creates a set of constants that enable the shader to access resources via name and not needing to know the actual group and binding id that have been assigned.