Skip to content

Commit

Permalink
Explicitly order CameraUpdateSystem before UiSystem::Prepare (bev…
Browse files Browse the repository at this point in the history
…yengine#14609)

# Objective

Fixes bevyengine#14277.

May also fix bevyengine#14255, needs
verification.

## Solution

Explicitly order `CameraUpdateSystem` before `UiSystem::Prepare`, so
that when the window resizes, `camera_system` will update the `Camera`'s
viewport size before `ui_layout_system` also reacts to the window resize
and tries to read the new `Camera` viewport size to set UI node sizes
accordingly.

## Testing

I tested that explicitly ordering `CameraUpdateSystem` _after_ triggers
the buggy behavior, and explicitly ordering it _before_ does not trigger
the buggy behavior or crash the app (which also demonstrates that the
system sets are ambiguous).

---

## Migration Guide

`CameraUpdateSystem` is now explicitly ordered before
`UiSystem::Prepare` instead of being ambiguous with it.
  • Loading branch information
benfrankel authored Aug 4, 2024
1 parent 4b20d82 commit 9b254aa
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 6 deletions.
7 changes: 2 additions & 5 deletions crates/bevy_ui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ use bevy_app::prelude::*;
use bevy_ecs::prelude::*;
use bevy_input::InputSystem;
use bevy_render::{
camera::CameraUpdateSystem,
view::{check_visibility, VisibilitySystems},
RenderApp,
};
Expand Down Expand Up @@ -146,6 +147,7 @@ impl Plugin for UiPlugin {
.configure_sets(
PostUpdate,
(
CameraUpdateSystem,
UiSystem::Prepare.before(UiSystem::Stack),
UiSystem::Layout,
(UiSystem::PostLayout, UiSystem::Outlines),
Expand Down Expand Up @@ -226,11 +228,6 @@ fn build_text_interop(app: &mut App) {
widget::measure_text_system
.in_set(UiSystem::Prepare)
// Potential conflict: `Assets<Image>`
// In practice, they run independently since `bevy_render::camera_update_system`
// will only ever observe its own render target, and `widget::measure_text_system`
// will never modify a pre-existing `Image` asset.
.ambiguous_with(bevy_render::camera::CameraUpdateSystem)
// Potential conflict: `Assets<Image>`
// Since both systems will only ever insert new [`Image`] assets,
// they will never observe each other's effects.
.ambiguous_with(bevy_text::update_text2d_layout)
Expand Down
2 changes: 1 addition & 1 deletion tests/ecs/ambiguity_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ pub fn main() {

let total_ambiguities = ambiguities.total();
assert_eq!(
total_ambiguities, 81,
total_ambiguities, 72,
"Main app does not have an expected conflicting systems count, \
you might consider verifying if it's normal, or change the expected number.\n\
Details:\n{:#?}",
Expand Down

0 comments on commit 9b254aa

Please sign in to comment.