bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
35.35k stars 3.49k forks source link

Be smarter about bind group usage #19

Closed cart closed 2 years ago

cart commented 4 years ago

Currently bind groups are re-created each frame to prevent "garbage" from stacking up from removed resources. @kvark has advised me that this approach isn't optimal :smile:

As a short term fix, he suggested just retaining them for X frames before dropping them. We could also make WgpuResources aware of BindGroup resource dependencies. The information is all there, it would just be a matter of storing a RenderResource->Vec<BindGroupId> mapping, then cleaning up bind groups for removed resources.

cart commented 2 years ago

We are now much smarter about this. There are a few very scoped / cheap cases that still create bind groups every frame, but for pretty much all of the important stuff we now cache them.

superdump commented 2 years ago

Hmm. Is that true? This made me want to look through to see what we're doing. I'm going to call out ones that are not being cached:

Which is relatively few, but still. :)

HugoPeters1024 commented 12 months ago

We also teach our users to recreate bind groups each frame in our examples. e.g.:

https://github.com/HugoPeters1024/bevy/blob/badfa2015e22d93bc40dce0928673601191e580a/examples/shader/compute_shader_game_of_life.rs#L77-L80

It's also not entirely clear to me if there is a canonical solution to that available right now.

cart commented 11 months ago

Hmm. Is that true? This made me want to look through to see what we're doing. I'm going to call out ones that are not being cached:

From the perspective I was speaking from I think it is true. Iirc this issue was written in the context of per-entity / per-asset bind groups being created each frame. We are being way smarter about bind group usage now. Mission accomplished.

I don't think we have any reason to believe that creating a small number of bind groups each frame is a cause for concern. And if it is, I believe that deserves to be a separate discussion.

davids91 commented 3 months ago

It is a big concern in case the bindgroups have significant data stored in them, e.g. here: https://github.com/davids91/shocovox/blob/2f5e583ba1465d33fae4be9bb11fad3cded6e383/src/octree/raytracing/bevy/types.rs#L79

The view I implemented stores the Viewport and the renderable data in one bind group. While the renderable data ( octree ) does not change very often, Viewport does. This essentially means that the whole structure is copied to the GPU at every frame which is unnecessary, and is a big performance hit.

I1ve been suggested to set the RenderAssetUsage to RENDER_WORLD only, but that would mean I am not able to change the Viewport .

Is there any best practice here I am missing? Should I open up another issue for this? This seemed relevant to this one..