Repository navigation
Storage Support in AsBindGroup #5499
Description
What problem does this solve or what need does it fill?
Currently I use the storage binding to pass a vector of elements to a shader. Storage should be supported on the new AsBindGroup derive, otherwise it isn't really a replacement.
What solution would you like?
Support writing#[storage(0)] the same way #[uniform(0)] is supported.
What alternative(s) have you considered?
None, this is a pretty small change that fits in to the rest of the AsBindGroup macro, just seems like a strange missing piece.
Activity
alice-i-cecile commentedon Jul 30, 2022
Seems sensible. Could you link to the code where the uniform macro annotation is implemented? I think this is feasible for new contributors but I'd like to scope out the work a little more.
jafioti commentedon Jul 31, 2022
All of the AsBindGroup macro code is located here: https://github.com/bevyengine/bevy/blob/main/crates/bevy_render/macros/src/as_bind_group.rs
alice-i-cecile commentedon Jul 31, 2022
So not too bad, but macro code is always a bit intimidating 🤔 I think this should be pretty feasible, but not quite Good-First-Issue.
jafioti commentedon Aug 1, 2022
@alice-i-cecile Is there any way to get a vector of data to a shader currently without this? My manual implementation of the shader worked in 0.7 but broke in 0.8, so I'd like to switch methods if possible since I can't figure out what went wrong.
alice-i-cecile commentedon Aug 1, 2022
I'm the wrong person to ask; I'd check on Discord.
wrapperup commentedon Aug 1, 2022
The reason this wasn't included was because @superdump wanted to collect materials together into an array in a later version of Bevy for instancing/batching, and that only works sensibly if materials are uniform buffers.
You can get around it by implementing the AsBindGroup trait manually and using a storage buffer, but I suspect this won't be a supported case for materials soonish.
I recommend using something like ArrayVec from the tinyvec or arrayvec crates and making ShaderType implementations for them. Then you can use an array binding while treating it like a vec, and use the derive macro as usual.
jafioti commentedon Aug 1, 2022
@wrapperup Thanks for the response! ShaderType is not implemented for ArrayVec. I did however just use a normal rust array. This isn't optimal of course since a variable amount of data can't be used, so a lot of wasted data is being sent for padding. For now it's an acceptable solution, but if manual implementation is going away soon, what's the replacement? Surely this is a pretty important use case that can't be cut off.
wrapperup commentedon Aug 1, 2022
It mostly depends on what you are trying to do. Also ask in the Discord, we can help you out here! There may be a better way to do what you want.
Having a storage buffer that all of your material instances can access would be an interesting supported case, but there isn't a high level way to do that (yet?). But no, there isn't a replacement for using an unsized array in each material instance. It's an intended limitation of materials that will be useful for a future optimization (instancing/batching meshes automatically).
superdump commentedon Aug 2, 2022
That isn’t quite true. I want storage buffer support for ergonomic data bindings similar to AsBindGroup just as much as anyone else. :) I also want to group data into single buffers and update those buffers rather than having a separate buffer per material as AsBindGroup currently does. Indeed if the material is modified AsBindGroup currently allocates a new uniform buffer for that one material’s data. What I want to do is store all material data of the same material type in an array of some sort in the same buffer and index into it.
My intention is to work toward an abstraction over uniform/storage/data texture/whatever else to enable simple storage of per-instance, per-material, and per-view data.
I also ideally want something more flexible than AsBindGroup so there is some way of combining separate rust types into one bind group rather than having to define an entire bind group in one single type.
So my intention is to modify AsBindGroup a bit to meet these kinds of goals. But my desire for these goals isn’t why there’s no support for storage buffers in AsBindGroup. It just hasn’t been implemented yet. :)
I’d review a PR. I’m just looking into batching and instancing with the time I have, which is the more long-term solution.
ShaderType supports a trailing Vec for storage buffers as I recall.
blaind commentedon Nov 17, 2022
Just created a pull request (#6669), but turns out there was already an unlinked one: #6129