Add a new #[data] attribute to AsBindGroup that allows packing data for multiple materials into a single array.
#17965
Conversation
for multiple materials into a single array. Currently, the structure-level `#[uniform]` attribute of `AsBindGroup` creates a binding array of individual buffers, each of which contains data for a single material. A more efficient approach would be to provide a single buffer with an array containing all of the data for all materials in the bind group. Because `StandardMaterial` uses `#[uniform]`, this can be notably inefficient with large numbers of materials. This patch introduces a new attribute on `AsBindGroup`, `#[data]`, which works identically to `#[uniform]` except that it concatenates all the data into a single buffer that the material bind group allocator itself manages. It also converts `StandardMaterial` to use this new feature. This effectively provides the "material data in arrays" feature.
pcwalton
added
A-Rendering
PR bevyengine#17898 regressed this, causing much of bevyengine#17970. This commit fixes the issue by freeing and reallocating materials in the `MaterialBindGroupAllocator` on change. Note that more efficiency is possible, but I opted for the simple approach because (1) we should fix this bug ASAP; (2) I'd like bevyengine#17965 to land first, because that unlocks the biggest potential optimization, which is not recreating the bind group if it isn't necessary to do so.
PR #17898 regressed this, causing much of #17970. This commit fixes the issue by freeing and reallocating materials in the `MaterialBindGroupAllocator` on change. Note that more efficiency is possible, but I opted for the simple approach because (1) we should fix this bug ASAP; (2) I'd like #17965 to land first, because that unlocks the biggest potential optimization, which is not recreating the bind group if it isn't necessary to do so.
| } | ||
|
|
||
| OwnedBindingResource::Data(_) => { | ||
| // The size of a data buffer is unlimited. |
There was a problem hiding this comment.
Storage and uniform buffers have maximum sizes, so perhaps that is the limit here. Uniform buffer limits are likely to be hit in some cases, but storage buffer ones practically not.
There was a problem hiding this comment.
Ah but this is only for bindless? Hrm. I was hoping flat buffers would be done for all cases. Do you see a way to do that? And also support uniform buffers? Not necessarily blocking the PR depending on difficulty.
There was a problem hiding this comment.
Non-bindless support wouldn't be useful until the non-bindless allocator has the ability to merge allocations that have identical resources other than buffers, like the bindless allocator can. In order to not have allocation be O(n), this would probably require implementing some kind of hashing scheme. So I'd like to have non-bindless support, but it'd probably be better to put in its own PR so as not to have too much unrelated stuff in this one.
| /// This will produce a binding matching the following WGSL declaration: | ||
| /// | ||
| /// ```wgsl | ||
| /// @group(2) @binding(10) var<storage> material_array: binding_array<StandardMaterial>; |
There was a problem hiding this comment.
It seems strange that annotating with uniform gives a storage buffer. Hmm.
There was a problem hiding this comment.
It is weird, yeah, but it's convenient, because there are a lot of cases in which you want a uniform in the non-bindless case (because UBOs are often faster) but want a storage buffer in the bindless case.
| /// Then Bevy produces a binding that matches this WGSL declaration instead: | ||
| /// | ||
| /// ```wgsl | ||
| /// @group(2) @binding(10) var<storage> material_array: array<StandardMaterial>; |
There was a problem hiding this comment.
In the bindless case only, right? We perhaps need to illustrate both here so people can copy and paste?
There was a problem hiding this comment.
Good point. I clarified that this example is only for bindless mode and added a new example illustrating how it looks in non-bindless mode.
tychedelia
left a comment
tychedelia
left a comment
There was a problem hiding this comment.
Given the amount of functionality in the macro, it might be nice to have an example covering some of these use cases outside of StandardMaterial in the engine code, but otherwise lgtm!
tychedelia
added
S-Ready-For-Final-Review
resource descriptors again. PR bevyengine#17965 mistakenly made the `AsBindGroup` macro no longer emit a bind group layout entry and a resource descriptor for buffers. This commit adds that functionality back, fixing the `shader_material_bindless` example. Closes bevyengine#18124.
4 participants
Currently, the structure-level
#[uniform]attribute ofAsBindGroupcreates a binding array of individual buffers, each of which contains data for a single material. A more efficient approach would be to provide a single buffer with an array containing all of the data for all materials in the bind group. BecauseStandardMaterialuses#[uniform], this can be notably inefficient with large numbers of materials.This patch introduces a new attribute on
AsBindGroup,#[data], which works identically to#[uniform]except that it concatenates all the data into a single buffer that the material bind group allocator itself manages. It also convertsStandardMaterialto use this new functionality. This effectively provides the "material data in arrays" feature.