-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Update mesh AABB when software skinning is used #53144
Conversation
Not updating the mesh properly can cause rendering issues in some cases, like shadows not updating properly.
Thanks! |
Just to note, it's usually better to update AABB in a high performance setting something like this:
The Godot AABB structure is not good for fast building, for this working with min max works better, than convert to position + size in a final step. Also for skinning, often the approach used is to use a conservative oversize large bound, and not recalculate it. So being able to set a custom AABB, and if this is set, to not do the AABB calcs might be good. Still this works to be going on with. |
It's amazing, I just made a test based on an equivalent of the portal code, and it works very well! This gets rid of most of the drop in performance, I'm going to open another PR.
Makes sense too, although in this case, even if the AABB wasn't modified we would have to call an instance update because of displaced vertices, so from what I see it doesn't look like it would make a lot of difference. Might be useful in specific cases though, it's good to keep in mind there's more room for improvements. |
Yeah it will reduce to just a couple of SIMD instructions per vertex I expect, whereas the default Godot mechanism for expanding will have all kinds of branching and extra work that only needs to be done once off. This is a bit of a trade off and why some engines store AABBs / RECTs as min / maxes rather than position + size. The windows The other great trick BTW (although can't be used here I don't think) is to store the max values of the AABB as negative max (or min as negative min). This means you can do intersection tests with all comparisons of the same sign, which means they can be combined easier in SIMD instructions. I do this in the BVH. |
Not updating the mesh properly can cause rendering issues in some cases, like shadows not updating properly.
This is 3.x only since software skinning is not implemented on master for now.
This change seems to have a limited impact on performance. In
release_debug
, I've only noticed a drop of around 4% when updating +100 models at once (mostly due to calculating the AABB itself, not dynamic shadows).Fixes #53018