mirror of
https://github.com/bevyengine/bevy
synced 2024-11-26 22:50:19 +00:00
c2b332f98a
# Objective Update the `calculate_bounds` system to update `Aabb`s for entities who've either: - gotten a new mesh - had their mesh mutated Fixes https://github.com/bevyengine/bevy/issues/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:📷: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. |
||
|---|---|---|
| .. | ||
| macros | ||
| src | ||
| Cargo.toml | ||