Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The doc for DynamicStorageBuffer doesn't really explain the difference with StorageBuffer #15052

Open
djeedai opened this issue Sep 5, 2024 · 9 comments · May be fixed by #17228
Open

The doc for DynamicStorageBuffer doesn't really explain the difference with StorageBuffer #15052

djeedai opened this issue Sep 5, 2024 · 9 comments · May be fixed by #17228
Assignees
Labels
A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@djeedai
Copy link
Contributor

djeedai commented Sep 5, 2024

https://docs.rs/bevy/latest/bevy/render/render_resource/struct.DynamicStorageBuffer.html

Each item pushed into this structure will additionally be aligned to meet dynamic offset alignment requirements.

Compared with https://docs.rs/bevy/latest/bevy/render/render_resource/struct.StorageBuffer.html#method.write_buffer

must conform to std430 alignment/padding requirements, which is automatically enforced by this structure.

Those sound like the same thing phrased in two different ways. The type T also has the same ShadeType bound, so there's no telling from that what a difference might be.

A table explaining the differences and pros/cons of all buffers, probably under the module's page, would be quite useful.

@djeedai djeedai added C-Docs An addition or correction to our documentation A-Rendering Drawing game state to the screen labels Sep 5, 2024
@djeedai djeedai changed the title The doc for DynamicStorageBuffer dlesn't really explain the difference with StorageBuffer The doc for DynamicStorageBuffer doesn't really explain the difference with StorageBuffer Sep 5, 2024
@JMS55
Copy link
Contributor

JMS55 commented Sep 5, 2024

I don't know that DynamicStorageBuffer really makes sense tbh, I've thought that we should remove it. Typically you don't need dynamic offsets if you're binding a storage buffer anyways.

@djeedai
Copy link
Contributor Author

djeedai commented Sep 6, 2024

Ok I didn't realize the "dynamic" was for the dynamic bind offset. From there I'm guessing that this buffer was added for the same (niche) reason I added mine in Hanabi, which is to be able to bind both the entire buffer but also individual elements. If that's the case this should be documented because this implies all elements are individually aligned, which has the potential for a lot of wasted padding, so should be carefully considered.

@djeedai
Copy link
Contributor Author

djeedai commented Sep 7, 2024

(individual elements or any other subset view if the array, the binding doesn't need to be a single element in size, what's important is the start of the bound view)

@djeedai
Copy link
Contributor Author

djeedai commented Sep 8, 2024

Ping @JMS55 before this gets into limbo, can you please clarify? I'm not asking about whether it makes sense or not, just that we at least document what this is (and at the very least on this issue, so someone can pick up the work to write the doc PR). Thanks!

@JMS55
Copy link
Contributor

JMS55 commented Sep 8, 2024

Yes, DynamicStorageBuffer is just a StorageBuffer, but also allows you to set dynamic offsets. Tbh I don't actually know how dynamic offsets work with storage buffers (what's the point even? You have the whole buffer bound at once anyways, no need to slice it up per binding...). I've never used it.

@djeedai
Copy link
Contributor Author

djeedai commented Sep 8, 2024

The point is say you have a buffer that stores an array of struct. This is especially common if you batch several items. In one compute pass you bind it as a whole and work on the entire array. But in other passes you bind just one element, or a chunk (= view = sub-array) of it. In Hanabi this is "work on all particles" vs. "work on a single group" (which is stored alongside all the other groups for the effect, inside the same buffer). Why is that helpful compared to binding the whole buffer? Because your compute pass doesn't need to know how to address the chunk; it's implicit by the binding, so saves on extra data to pass. Now, I believe the key aspect here is not about the usage of dynamic offsets, but the fact that if you want to bind a sub-array, then the first element of that sub-array needs to be aligned according to min_storage_buffer_offset_alignment (independently of whether the offset in the storage buffer was specified at layout time or dynamically at bind time), which can be up to 256 bytes on some platforms (macOS silicon in particular). And that means all elements of the array need to be aligned like this, unless you know exactly where the chunks you bind are in advance. And aligning all array elements to e.g. 256 bytes takes a lot more wasted padding than only aligning the start of the array. But sometimes it's still worth it (better have 30 elements and waste 500 bytes, but run only one compute pass, than having to handle 30 separate tiny buffers and dispatch 30 separate compute passes to avoid the padding).

Is DynamicStorageBuffer for this usage? That's what I'm trying to confirm.

@JMS55
Copy link
Contributor

JMS55 commented Sep 8, 2024

Sounds right to me, but idk for sure.

@djeedai
Copy link
Contributor Author

djeedai commented Sep 8, 2024

Ok thanks, I'll dig into the code if I have time, or if someone else knows...

@djeedai
Copy link
Contributor Author

djeedai commented Sep 8, 2024

Ok that's actually just a wrapper around encase's own DynamicStorageBuffer, which clearly points at the default 256 bytes and the others per-device limit >= 32 bytes mentioned above.

@BenjaminBrienen BenjaminBrienen added D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! labels Jan 7, 2025
@BenjaminBrienen BenjaminBrienen self-assigned this Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants