You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Recalculate entity aabbs when meshes change (bevyengine#4944)
# Objective
Update the `calculate_bounds` system to update `Aabb`s
for entities who've either:
- gotten a new mesh
- had their mesh mutated
Fixesbevyengine#4294.
## Solution
There are two commits here to address the two issues above:
### Commit 1
**This Commit**
Updates the `calculate_bounds` system to operate not only on entities
without `Aabb`s but also on entities whose `Handle<Mesh>` has changed.
**Why?**
So if an entity gets a new mesh, its associated `Aabb` is properly
recalculated.
**Questions**
- This type is getting pretty gnarly - should I extract some types?
- This system is public - should I add some quick docs while I'm here?
### Commit 2
**This Commit**
Updates `calculate_bounds` to update `Aabb`s of entities whose meshes
have been directly mutated.
**Why?**
So if an entity's mesh gets updated, its associated `Aabb` is properly
recalculated.
**Questions**
- I think we should be using `ahash`. Do we want to do that with a
direct `hashbrown` dependency or an `ahash` dependency that we
configure the `HashMap` with?
- There is an edge case of duplicates with `Vec<Entity>` in the
`HashMap`. If an entity gets its mesh handle changed and changed back
again it'll be added to the list twice. Do we want to use a `HashSet`
to avoid that? Or do a check in the list first (assuming iterating
over the `Vec` is faster and this edge case is rare)?
- There is an edge case where, if an entity gets a new mesh handle and
then its old mesh is updated, we'll update the entity's `Aabb` to the
new geometry of the _old_ mesh. Do we want to remove items from the
`Local<HashMap>` when handles change? Does the `Changed` event give us
the old mesh handle? If not we might need to have a
`HashMap<Entity, Handle<Mesh>>` or something so we can unlink entities
from mesh handles when the handle changes.
- I did the `zip()` with the two `HashMap` gets assuming those would
be faster than calculating the Aabb of the mesh (otherwise we could do
`meshes.get(mesh_handle).and_then(Mesh::compute_aabb).zip(entity_mesh_map...)`
or something). Is that assumption way off?
## Testing
I originally tried testing this with `bevy_mod_raycast` as mentioned in the
original issue but it seemed to work (maybe they are currently manually
updating the Aabbs?). I then tried doing it in 2D but it looks like
`Handle<Mesh>` is just for 3D. So I took [this example](https://github.com/bevyengine/bevy/blob/main/examples/3d/pbr.rs)
and added some systems to mutate/assign meshes:
<details>
<summary>Test Script</summary>
```rust
use bevy::prelude::*;
use bevy::render::camera::ScalingMode;
use bevy::render::primitives::Aabb;
/// Make sure we only mutate one mesh once.
#[derive(Eq, PartialEq, Clone, Debug, Default)]
struct MutateMeshState(bool);
/// Let's have a few global meshes that we can cycle between.
/// This way we can be assigned a new mesh, mutate the old one, and then get the old one assigned.
#[derive(Eq, PartialEq, Clone, Debug, Default)]
struct Meshes(Vec<Handle<Mesh>>);
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.init_resource::<MutateMeshState>()
.init_resource::<Meshes>()
.add_startup_system(setup)
.add_system(assign_new_mesh)
.add_system(show_aabbs.after(assign_new_mesh))
.add_system(mutate_meshes.after(show_aabbs))
.run();
}
fn setup(
mut commands: Commands,
mut meshes: ResMut<Assets<Mesh>>,
mut global_meshes: ResMut<Meshes>,
mut materials: ResMut<Assets<StandardMaterial>>,
) {
let m1 = meshes.add(Mesh::from(shape::Icosphere::default()));
let m2 = meshes.add(Mesh::from(shape::Icosphere {
radius: 0.90,
..Default::default()
}));
let m3 = meshes.add(Mesh::from(shape::Icosphere {
radius: 0.80,
..Default::default()
}));
global_meshes.0.push(m1.clone());
global_meshes.0.push(m2);
global_meshes.0.push(m3);
// add entities to the world
// sphere
commands.spawn_bundle(PbrBundle {
mesh: m1,
material: materials.add(StandardMaterial {
base_color: Color::hex("ffd891").unwrap(),
..default()
}),
..default()
});
// new 3d camera
commands.spawn_bundle(Camera3dBundle {
projection: OrthographicProjection {
scale: 3.0,
scaling_mode: ScalingMode::FixedVertical(1.0),
..default()
}
.into(),
..default()
});
// old 3d camera
// commands.spawn_bundle(OrthographicCameraBundle {
// transform: Transform::from_xyz(0.0, 0.0, 8.0).looking_at(Vec3::default(), Vec3::Y),
// orthographic_projection: OrthographicProjection {
// scale: 0.01,
// ..default()
// },
// ..OrthographicCameraBundle::new_3d()
// });
}
fn show_aabbs(query: Query<(Entity, &Handle<Mesh>, &Aabb)>) {
for thing in query.iter() {
println!("{thing:?}");
}
}
/// For testing the second part - mutating a mesh.
///
/// Without the fix we should see this mutate an old mesh and it affects the new mesh that the
/// entity currently has.
/// With the fix, the mutation doesn't affect anything until the entity is reassigned the old mesh.
fn mutate_meshes(
mut meshes: ResMut<Assets<Mesh>>,
time: Res<Time>,
global_meshes: Res<Meshes>,
mut mutate_mesh_state: ResMut<MutateMeshState>,
) {
let mutated = mutate_mesh_state.0;
if time.seconds_since_startup() > 4.5 && !mutated {
println!("Mutating {:?}", global_meshes.0[0]);
let m = meshes.get_mut(&global_meshes.0[0]).unwrap();
let mut p = m.attribute(Mesh::ATTRIBUTE_POSITION).unwrap().clone();
use bevy::render::mesh::VertexAttributeValues;
match &mut p {
VertexAttributeValues::Float32x3(v) => {
v[0] = [10.0, 10.0, 10.0];
}
_ => unreachable!(),
}
m.insert_attribute(Mesh::ATTRIBUTE_POSITION, p);
mutate_mesh_state.0 = true;
}
}
/// For testing the first part - assigning a new handle.
fn assign_new_mesh(
mut query: Query<&mut Handle<Mesh>, With<Aabb>>,
time: Res<Time>,
global_meshes: Res<Meshes>,
) {
let s = time.seconds_since_startup() as usize;
let idx = s % global_meshes.0.len();
for mut handle in query.iter_mut() {
*handle = global_meshes.0[idx].clone_weak();
}
}
```
</details>
## Changelog
### Fixed
Entity `Aabb`s not updating when meshes are mutated or re-assigned.
0 commit comments