Skip to content

Commit

Permalink
Update AABB when Sprite component changes in calculate_bounds_2d() (#…
Browse files Browse the repository at this point in the history
…11016)

# Objective

- Fixes #10587, where the `Aabb` component of entities with `Sprite` and
`Handle<Image>` components was not automatically updated when
`Sprite::custom_size` changed.

## Solution

- In the query for entities with `Sprite` components in
`calculate_bounds_2d`, use the `Changed` filter to detect for `Sprites`
that changed as well as sprites that do not have `Aabb` components. As
noted in the issue, this will cause the `Aabb` to be recalculated when
other fields of the `Sprite` component change, but calculating the
`Aabb` for sprites is trivial.

---

## Changelog
- Modified query for entities with `Sprite` components in
`calculate_bounds_2d`, so that entities with `Sprite` components that
changed will also have their AABB recalculated.
  • Loading branch information
matthew-gries authored Dec 18, 2023
1 parent 2c7eab1 commit d99053c
Showing 1 changed file with 118 additions and 3 deletions.
121 changes: 118 additions & 3 deletions crates/bevy_sprite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,12 @@ pub fn calculate_bounds_2d(
images: Res<Assets<Image>>,
atlases: Res<Assets<TextureAtlas>>,
meshes_without_aabb: Query<(Entity, &Mesh2dHandle), (Without<Aabb>, Without<NoFrustumCulling>)>,
sprites_without_aabb: Query<
sprites_to_recalculate_aabb: Query<
(Entity, &Sprite, &Handle<Image>),
(Without<Aabb>, Without<NoFrustumCulling>),
(
Or<(Without<Aabb>, Changed<Sprite>)>,
Without<NoFrustumCulling>,
),
>,
atlases_without_aabb: Query<
(Entity, &TextureAtlasSprite, &Handle<TextureAtlas>),
Expand All @@ -136,7 +139,7 @@ pub fn calculate_bounds_2d(
}
}
}
for (entity, sprite, texture_handle) in &sprites_without_aabb {
for (entity, sprite, texture_handle) in &sprites_to_recalculate_aabb {
if let Some(size) = sprite
.custom_size
.or_else(|| images.get(texture_handle).map(|image| image.size_f32()))
Expand All @@ -163,3 +166,115 @@ pub fn calculate_bounds_2d(
}
}
}

#[cfg(test)]
mod test {

use bevy_math::Vec2;
use bevy_utils::default;

use super::*;

#[test]
fn calculate_bounds_2d_create_aabb_for_image_sprite_entity() {
// Setup app
let mut app = App::new();

// Add resources and get handle to image
let mut image_assets = Assets::<Image>::default();
let image_handle = image_assets.add(Image::default());
app.insert_resource(image_assets);
let mesh_assets = Assets::<Mesh>::default();
app.insert_resource(mesh_assets);
let texture_atlas_assets = Assets::<TextureAtlas>::default();
app.insert_resource(texture_atlas_assets);

// Add system
app.add_systems(Update, calculate_bounds_2d);

// Add entites
let entity = app.world.spawn((Sprite::default(), image_handle)).id();

// Verify that the entity does not have an AABB
assert!(!app
.world
.get_entity(entity)
.expect("Could not find entity")
.contains::<Aabb>());

// Run system
app.update();

// Verify the AABB exists
assert!(app
.world
.get_entity(entity)
.expect("Could not find entity")
.contains::<Aabb>());
}

#[test]
fn calculate_bounds_2d_update_aabb_when_sprite_custom_size_changes_to_some() {
// Setup app
let mut app = App::new();

// Add resources and get handle to image
let mut image_assets = Assets::<Image>::default();
let image_handle = image_assets.add(Image::default());
app.insert_resource(image_assets);
let mesh_assets = Assets::<Mesh>::default();
app.insert_resource(mesh_assets);
let texture_atlas_assets = Assets::<TextureAtlas>::default();
app.insert_resource(texture_atlas_assets);

// Add system
app.add_systems(Update, calculate_bounds_2d);

// Add entites
let entity = app
.world
.spawn((
Sprite {
custom_size: Some(Vec2::ZERO),
..default()
},
image_handle,
))
.id();

// Create initial AABB
app.update();

// Get the initial AABB
let first_aabb = *app
.world
.get_entity(entity)
.expect("Could not find entity")
.get::<Aabb>()
.expect("Could not find initial AABB");

// Change `custom_size` of sprite
let mut binding = app
.world
.get_entity_mut(entity)
.expect("Could not find entity");
let mut sprite = binding
.get_mut::<Sprite>()
.expect("Could not find sprite component of entity");
sprite.custom_size = Some(Vec2::ONE);

// Re-run the `calculate_bounds_2d` system to get the new AABB
app.update();

// Get the re-calculated AABB
let second_aabb = *app
.world
.get_entity(entity)
.expect("Could not find entity")
.get::<Aabb>()
.expect("Could not find second AABB");

// Check that the AABBs are not equal
assert_ne!(first_aabb, second_aabb);
}
}

0 comments on commit d99053c

Please sign in to comment.