bevy
bevy copied to clipboard
Recompute Aabbs for updated Meshes
Objective
Partially address #4294. This was attempted in #4944 and #5423., but reverted in #5489. Aabb components are not updated when the underlying Mesh has changed in some way.
Solution
Recompute their Aabb when it's been updated or newly created.
Note: this only addresses when the Mesh is updated itself, not when the Handle<Mesh> has changed. I attempted to include a Ref<Handle<Mesh>>::is_changed check, but that easily added 1+ms on many_cubes to do what is basically zero work in that common case. I attempted to parallelize it with QueryParIter, which shrunk it down to 360us, but it still was an excessive amount of work for basically nothing. This current implementation only incurs a cost when a Mesh is created or modified.
I view this as a stop gap until we have a form of asset preprocessing that can be used to compute the Aabb offline, or force meshes to be immutable after construction and precompute it's Aabb ahead of time.
Changelog
TODO
Migration Guide
TODO
I'm not sure I understand your comment about asset preprocessing. Isn't the point of this to be able to recompute AABBs on runtime meshes? Spawning a mesh will already create the correct AABB. Asset preprocessing will help of course, but this will still need to exist for people making dynamic runtime meshes.
I still think all of this state-syncing is a nightmare and we should try removing it entirely: https://github.com/bevyengine/bevy/pull/5423#issuecomment-1199995825
Needs more benchmarking though
I just put together a "store aabb directly on meshes" impl: https://github.com/cart/bevy/commit/a465c1da355fa337f5b8659ae3789cf5b8e58d76
(still needs benchmarking ... haven't been able to compile Tracy on my new debian install yet)
I just put together a "store aabb directly on meshes" impl: cart@a465c1d
(still needs benchmarking ... haven't been able to compile Tracy on my new debian install yet)
Tested a few different stress tests.
On many_cubes with --sphere, this was faster for overall frametimes but the par_for_each{query="(bevy_ecs::entity::Entity, &mut bevy_render::view::visibility::ComputedVisibility, ... query took longer.
(this is cart's "store aabb directly on meshes" cherrypicked onto bevy main (10f5c9206847ae01b8dc833c2680562e7bd46664), while ext is bevy main)
I tried testing many_lights, but it doesn't render any of the lights on cart's PR, and many_foxes panics with:
thread 'main' panicked at 'scene contains the unregistered type bevy_render::primitives::AabbSource. consider registering the type using app.register_type::<T>()', crates\bevy_scene\src\scene_spawner.rs:358:35
Hmm thats certainly a big perf difference (likely prohibitively so). Worth trying to see what we can optimize though.
Sadly punting this to 0.12. We've run out of time and this needs more of it :)
This is a pretty high impact bug: I'm in favor of getting any fix in rather than delaying another cycle.
I think this could be updated to use the new AssetId. And there was some feedback. It’s a shame it has to do so many lookups but if the benchmarking for this looks ok then it seems reasonable.