Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Aabb on Sprite::custom_size changed. #10587

Closed
Selene-Amanita opened this issue Nov 16, 2023 · 0 comments · Fixed by #11016
Closed

Update Aabb on Sprite::custom_size changed. #10587

Selene-Amanita opened this issue Nov 16, 2023 · 0 comments · Fixed by #11016
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@Selene-Amanita
Copy link
Member

Selene-Amanita commented Nov 16, 2023

Motivation

This PR is a subset of #4294 : the Aabb component is never automatically updated after it is added to an entity, even when it should be (for example the Mesh vertices positions are updated).

To fix #4294 integrally, #5069 needs to be solved, as we need to rely on asset change detection for most of the cases.

From Bevy 0.11.0, frustum culling was added for 2D entities, and an Aabb component is automatically added to entities with a Sprite and Handle<Image> component (by calculate_bounds_2d), and never updated. To update it automatically, in the case where Sprite::custom_size is None, we would require the image asset change detection. However, when Sprite is changed and custom_size is set to Some, we could update the Aabb in the current state of Bevy.

Note

Simply detecting Sprite change could trigger false positives, like when another field than custom_size or anchor is changed, the Aabb would still be needlessly recalculated in this case. I don't think it is a big problem as Aabb calculation is trivial in that case. If it is, custom_size could be isolated in a separate component.

@Selene-Amanita Selene-Amanita added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen D-Trivial Nice and easy! A great choice to get started with Bevy and removed S-Needs-Triage This issue needs to be labelled C-Feature A new feature, making something new possible labels Nov 16, 2023
github-merge-queue bot pushed a commit that referenced this issue Dec 18, 2023
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant