bevy: Mesh AABBs are never updated

Bevy version: 0.6 OS & graphic stack: ArchLinux kernel 5.16.2

Problem

When using https://github.com/aevyrie/bevy_mod_raycast which relies on bevy’s Aabb, and I update an entity’s mesh, either by (1) changing it’s Handle<Mesh> or (2) updating the mesh itself in the Assets<Mesh>, the plugin doesn’t account for the change in mesh value.

This is because bevy never updates the Aabb of an entity after the initial spawn of a mesh.

Solution

It is legitimate to expect the Aabbs would be updated with the meshes or mesh handles of entities (certainly the documentation doesn’t mention that). Either this should be documented as expected behavior or the code amended to update Aabbs based on the current mesh.

Technical solution

It seems the problematic system is bevy_render::view::visibility::calculate_bounds . ( crates/bevy_render/src/view/visibility/mod.rs line 112 on 024d98457c80d25f9).

https://github.com/bevyengine/bevy/pull/5489 has a few discussions around potential fixes.

Current Workaround

Manually update the Aabb after updating the mesh.

About this issue

Original URL
State: open
Created 2 years ago
Reactions: 11
Comments: 17 (15 by maintainers)

Commits related to this issue

Recalculate entity aabbs when meshes change (#4944) # Objective Update the `calculate_bounds` system to update `Aabb`s for entities who've either: - gotten a new mesh - had their mesh mutated ... — committed to bevyengine/bevy by mlodato517 2 years ago
Recalculate entity aabbs when meshes change (#4944) # Objective Update the `calculate_bounds` system to update `Aabb`s for entities who've either: - gotten a new mesh - had their mesh mutated ... — committed to IyesGames/bevy by mlodato517 2 years ago
Recalculate entity aabbs when meshes change (#4944) # Objective Update the `calculate_bounds` system to update `Aabb`s for entities who've either: - gotten a new mesh - had their mesh mutated ... — committed to james7132/bevy by mlodato517 2 years ago
Recalculate entity aabbs when meshes change (#4944) # Objective Update the `calculate_bounds` system to update `Aabb`s for entities who've either: - gotten a new mesh - had their mesh mutated ... — committed to ItsDoot/bevy by mlodato517 2 years ago

Most upvoted comments

If anyone is here looking for a quick workaround in the meantime, manually removing the AABB from the entity, after the mesh has been changed, seem to do the trick:

commands
    .entity(entity_id)
    .remove::<bevy::render::primitives::Aabb>();

splashdust on Jun 26, 2023

This is super annoying. One can easily circumvent this, but it’s still a trap. Can we get this merged in 0.11?

nerdachse on Jun 12, 2023

Another way would be to consider that the aabb of a mesh should be an asset rather than a component. That way each entity would have the handle to the same aabb linked to their mesh. Not sure how much that would add when querying though…

mockersf on May 31, 2022

I had in mind pretty much what you wrote down yeah. I was thinking of the entities_with_mesh argument as follow though:

mut entities_with_mesh: Local<HashMap<Handle<Mesh>, Vec<Entity>>>

(and updating the hashmap when necessary).

A hint: you could maybe chain the filter_map in the iterator to reduce rightward drift, or even use the ? operator in a closure that returns an option.

The idea described in the if not section seems indeed not the best.

nicopap on May 31, 2022

Reproduced in Bevy 0.9: great video of the problem here.

alice-i-cecile on Mar 6, 2023

@mlodato517 this is correct. I think you have everything you need now.

nicopap on May 31, 2022

Hmmm maybe I am a bit confused. I thought originally you were suggesting roughly this pseudocode:

pub fn calculate_bounds(
    mut commands: Commands,
    meshes: Res<Assets<Mesh>>,
    without_aabb: Query<(Entity, &Handle<Mesh>), (Without<Aabb>, Without<NoFrustumCulling>)>,
    mesh_events: EventReader<AssetEvent<Mesh>>,              // new
    entities_with_mesh: &HashMap<Handle<Mesh>, Vec<Entity>>, // new
) {
    // same old code plus...

    let updated_meshes = mesh_events.iter().filter_map(|&event| match event {
        Modified { handle } => Some(handle),
        _ => None,
    });
    for mesh_handle in updated_meshes {
        if let Some(mesh) = meshes.get(mesh_handle) {
            if let Some(aabb) = mesh.compute_aabb() {
                if let Some(entities) = entities_with_mesh(mesh_handle) {
                    commands.entity(entity).insert(aabb); // or do we need to remove + insert?
                // lots of closing curlies...

Was I way off there? (I also wonder if this should be a different system from calculate_bounds since that system would be getting cluttered 🤔 But it’s still under the idea of “calculating bounds” so ⚖️)

If not

I think this makes perfect sense. I was just saying that, if we wanted to, instead of storing HashMap<Handle<Mesh>, Vec<Entity>> we could store HashMap<Handle<Mesh>, Aabb> and we mutate that when there’s an event and then instead of having Query<&Aabb> we have like Query<Handle<Mesh>> and then they do a lookup in the HashMap for the Aabb.

I think that’s undesirable because it impacts the performance of everyone checking for Aabb (and it leaks the abstraction for how these Aabbs are managed) but, if we expect frequent mesh updates (which I doubt) then this would avoid the big loop in calculate_bounds.

Not that I’m saying we should go this way, just wanted to make sure my suggestion was clear 😃

mlodato517 on May 31, 2022