Skip to content

Commit

Permalink
Fix the double leaf node updates in flex_node_system (#8264)
Browse files Browse the repository at this point in the history
# Objective

If a UI node has a changed `CalculatedSize` component and either the UI
does a full update or the node also has a changed `Style` component, the
node's corresponding Taffy node will be updated twice by
`flex_node_system`.

## Solution

Add a `Without<Calculated>` query filter so that the two changed node
queries in `flex_node_system` are mutually exclusive and move the
`CalculatedSize` node updater into the else block of the full-update if
conditional.
  • Loading branch information
ickshonpe authored Apr 21, 2023
1 parent e900bd9 commit 0cf3000
Showing 1 changed file with 21 additions and 24 deletions.
45 changes: 21 additions & 24 deletions crates/bevy_ui/src/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use bevy_ecs::{
change_detection::DetectChanges,
entity::Entity,
event::EventReader,
query::{Changed, ReadOnlyWorldQuery, With, Without},
query::{Changed, Or, With, Without},
removal_detection::RemovedComponents,
system::{Query, Res, ResMut, Resource},
};
Expand Down Expand Up @@ -244,11 +244,14 @@ pub fn ui_layout_system(
mut resize_events: EventReader<bevy_window::WindowResized>,
mut ui_surface: ResMut<UiSurface>,
root_node_query: Query<Entity, (With<Node>, Without<Parent>)>,
node_query: Query<(Entity, &Style, Option<&CalculatedSize>), (With<Node>, Changed<Style>)>,
full_node_query: Query<(Entity, &Style, Option<&CalculatedSize>), With<Node>>,
changed_style_query: Query<
(Entity, &Style),
(With<Node>, Without<CalculatedSize>, Changed<Style>),
>,
changed_size_query: Query<
(Entity, &Style, &CalculatedSize),
(With<Node>, Changed<CalculatedSize>),
(With<Node>, Or<(Changed<CalculatedSize>, Changed<Style>)>),
>,
children_query: Query<(Entity, &Children), (With<Node>, Changed<Children>)>,
mut removed_children: RemovedComponents<Children>,
Expand Down Expand Up @@ -282,34 +285,28 @@ pub fn ui_layout_system(
}

let scale_factor = logical_to_physical_factor * ui_scale.scale;
let layout_context = LayoutContext::new(scale_factor, physical_size);

let viewport_values = LayoutContext::new(scale_factor, physical_size);

fn update_changed<F: ReadOnlyWorldQuery>(
ui_surface: &mut UiSurface,
viewport_values: &LayoutContext,
query: Query<(Entity, &Style, Option<&CalculatedSize>), F>,
) {
// update changed nodes
for (entity, style, calculated_size) in &query {
// TODO: remove node from old hierarchy if its root has changed
if !scale_factor_events.is_empty() || ui_scale.is_changed() || resized {
scale_factor_events.clear();
// update all nodes
for (entity, style, calculated_size) in &full_node_query {
if let Some(calculated_size) = calculated_size {
ui_surface.upsert_leaf(entity, style, calculated_size, viewport_values);
ui_surface.upsert_leaf(entity, style, calculated_size, &layout_context);
} else {
ui_surface.upsert_node(entity, style, viewport_values);
ui_surface.upsert_node(entity, style, &layout_context);
}
}
}

if !scale_factor_events.is_empty() || ui_scale.is_changed() || resized {
scale_factor_events.clear();
update_changed(&mut ui_surface, &viewport_values, full_node_query);
} else {
update_changed(&mut ui_surface, &viewport_values, node_query);
}
// update changed nodes without a calculated size
for (entity, style) in changed_style_query.iter() {
ui_surface.upsert_node(entity, style, &layout_context);
}

for (entity, style, calculated_size) in &changed_size_query {
ui_surface.upsert_leaf(entity, style, calculated_size, &viewport_values);
// update changed nodes with a calculated size
for (entity, style, calculated_size) in changed_size_query.iter() {
ui_surface.upsert_leaf(entity, style, calculated_size, &layout_context);
}
}

// clean up removed nodes
Expand Down

0 comments on commit 0cf3000

Please sign in to comment.