EmiOnGit / warbler_grass

A bevy plugin for creating 3d grass in your game
Apache License 2.0
120 stars 11 forks source link

Optimize prepare stage for frames that did not change #15

Closed janhohenheim closed 1 year ago

janhohenheim commented 1 year ago

Disclaimer: I don't know how much impact this actually has on the performance.

EmiOnGit commented 1 year ago

Actually this is a pretty good change :) If you actually want you can optimize it even more by splitting the function into two functions. One that is just preparing the uniform buffers prepare_uniform_buffers and one just for the instance buffer prepare_instance_buffer They should be independent, but beside that it would be also more organized. However, you don't have to ofc.

janhohenheim commented 1 year ago

I see what you mean, but wouldn't they both have mut access to GrassCache and thus not run in parallel? Edit: ah, but they could have other early return guards, I see!

EmiOnGit commented 1 year ago

ah, but they could have other early return guards, I see!

The other return guards doesn't matter as much because these 2 functions are the only two that could run in parallel (The only in the RenderStage::Prepare at least currently) This means we'd need to split the buffers in separate resources or use unsafe code. I'm against the second option and unsure about the first

EmiOnGit commented 1 year ago

It is definitely better readable though

janhohenheim commented 1 year ago

How about this for the moment? This way, we at least distinguish the cases of config changed vs grass changed

EmiOnGit commented 1 year ago

Ah also the functions are named the wrong way around. prepare_instance_buffer is currently preparing all uniform buffers . Happens easily if you copy paste to fast :smile:

janhohenheim commented 1 year ago

hehehe, good catch. Fixed.

EmiOnGit commented 1 year ago

Nice, looks almost good now. If you simplify the for loop mentioned above I'll merge. Have you looked into the issues btw?

janhohenheim commented 1 year ago

Which loop did you mention? Do you mean like this? Checking the issues now 👍

EmiOnGit commented 1 year ago

Yes now it's perfect