From be100b8760ba01d3119df0864838e94a725de5b5 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 2 Sep 2024 17:56:58 +0100 Subject: [PATCH] Resolve UI outlines using the correct target's viewport size (#14947) # Objective `resolve_outlines_system` wasn't updated when multi-window support was added and it always uses the size of the primary window when resolving viewport coords, regardless of the layout's camera target. Fixes #14945 ## Solution It's awkward to get the viewport size of the target for an individual node without walking the tree or adding extra fields to `Node`, so I removed `resolve_outlines_system` and instead the outline values are updated in `ui_layout_system`. --------- Co-authored-by: Alice Cecile --- crates/bevy_ui/src/accessibility.rs | 1 - crates/bevy_ui/src/layout/mod.rs | 57 +++++++++++++---------------- crates/bevy_ui/src/lib.rs | 12 +----- crates/bevy_ui/src/ui_node.rs | 6 ++- tests/ecs/ambiguity_detection.rs | 2 +- 5 files changed, 33 insertions(+), 45 deletions(-) diff --git a/crates/bevy_ui/src/accessibility.rs b/crates/bevy_ui/src/accessibility.rs index cbcf8e82d3..8ca46e124e 100644 --- a/crates/bevy_ui/src/accessibility.rs +++ b/crates/bevy_ui/src/accessibility.rs @@ -154,7 +154,6 @@ impl Plugin for AccessibilityPlugin { .after(bevy_transform::TransformSystem::TransformPropagate) .after(CameraUpdateSystem) // the listed systems do not affect calculated size - .ambiguous_with(crate::resolve_outlines_system) .ambiguous_with(crate::ui_stack_system), button_changed, image_changed, diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 53ac334c60..7fbdbf5df1 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -94,7 +94,7 @@ pub fn ui_layout_system( children_query: Query<(Entity, Ref), With>, just_children_query: Query<&Children>, mut removed_components: UiLayoutSystemRemovedComponentParam, - mut node_transform_query: Query<(&mut Node, &mut Transform)>, + mut node_transform_query: Query<(&mut Node, &mut Transform, Option<&Outline>)>, #[cfg(feature = "bevy_text")] mut buffer_query: Query<&mut CosmicBuffer>, #[cfg(feature = "bevy_text")] mut text_pipeline: ResMut, ) { @@ -247,6 +247,7 @@ pub fn ui_layout_system( update_uinode_geometry_recursive( *root, &ui_surface, + None, &mut node_transform_query, &just_children_query, inverse_target_scale_factor, @@ -259,13 +260,14 @@ pub fn ui_layout_system( fn update_uinode_geometry_recursive( entity: Entity, ui_surface: &UiSurface, - node_transform_query: &mut Query<(&mut Node, &mut Transform)>, + root_size: Option, + node_transform_query: &mut Query<(&mut Node, &mut Transform, Option<&Outline>)>, children_query: &Query<&Children>, inverse_target_scale_factor: f32, parent_size: Vec2, mut absolute_location: Vec2, ) { - if let Ok((mut node, mut transform)) = node_transform_query.get_mut(entity) { + if let Ok((mut node, mut transform, outline)) = node_transform_query.get_mut(entity) { let Ok(layout) = ui_surface.get_layout(entity) else { return; }; @@ -287,14 +289,35 @@ pub fn ui_layout_system( node.calculated_size = rounded_size; node.unrounded_size = layout_size; } + + let viewport_size = root_size.unwrap_or(node.calculated_size); + + if let Some(outline) = outline { + // don't trigger change detection when only outlines are changed + let node = node.bypass_change_detection(); + node.outline_width = outline + .width + .resolve(node.size().x, viewport_size) + .unwrap_or(0.) + .max(0.); + + node.outline_offset = outline + .offset + .resolve(node.size().x, viewport_size) + .unwrap_or(0.) + .max(0.); + } + if transform.translation.truncate() != rounded_location { transform.translation = rounded_location.extend(0.); } + if let Ok(children) = children_query.get(entity) { for &child_uinode in children { update_uinode_geometry_recursive( child_uinode, ui_surface, + Some(viewport_size), node_transform_query, children_query, inverse_target_scale_factor, @@ -307,34 +330,6 @@ pub fn ui_layout_system( } } -/// Resolve and update the widths of Node outlines -pub fn resolve_outlines_system( - primary_window: Query<&Window, With>, - ui_scale: Res, - mut outlines_query: Query<(&Outline, &mut Node)>, -) { - let viewport_size = primary_window - .get_single() - .map(Window::size) - .unwrap_or(Vec2::ZERO) - / ui_scale.0; - - for (outline, mut node) in outlines_query.iter_mut() { - let node = node.bypass_change_detection(); - node.outline_width = outline - .width - .resolve(node.size().x, viewport_size) - .unwrap_or(0.) - .max(0.); - - node.outline_offset = outline - .offset - .resolve(node.size().x, viewport_size) - .unwrap_or(0.) - .max(0.); - } -} - #[inline] /// Round `value` to the nearest whole integer, with ties (values with a fractional part equal to 0.5) rounded towards positive infinity. fn approx_round_ties_up(value: f32) -> f32 { diff --git a/crates/bevy_ui/src/lib.rs b/crates/bevy_ui/src/lib.rs index 44eacb51af..7d1c804bf9 100644 --- a/crates/bevy_ui/src/lib.rs +++ b/crates/bevy_ui/src/lib.rs @@ -92,10 +92,6 @@ pub enum UiSystem { /// /// Runs in [`PostUpdate`]. Stack, - /// After this label, node outline widths have been updated. - /// - /// Runs in [`PostUpdate`]. - Outlines, } /// The current scale of the UI. @@ -153,7 +149,7 @@ impl Plugin for UiPlugin { CameraUpdateSystem, UiSystem::Prepare.before(UiSystem::Stack), UiSystem::Layout, - (UiSystem::PostLayout, UiSystem::Outlines), + UiSystem::PostLayout, ) .chain(), ) @@ -170,16 +166,10 @@ impl Plugin for UiPlugin { ui_layout_system .in_set(UiSystem::Layout) .before(TransformSystem::TransformPropagate), - resolve_outlines_system - .in_set(UiSystem::Outlines) - // clipping doesn't care about outlines - .ambiguous_with(update_clipping_system) - .in_set(AmbiguousWithTextSystem), ui_stack_system .in_set(UiSystem::Stack) // the systems don't care about stack index .ambiguous_with(update_clipping_system) - .ambiguous_with(resolve_outlines_system) .ambiguous_with(ui_layout_system) .in_set(AmbiguousWithTextSystem), update_clipping_system.after(TransformSystem::TransformPropagate), diff --git a/crates/bevy_ui/src/ui_node.rs b/crates/bevy_ui/src/ui_node.rs index 3a46e952b3..549eb796ce 100644 --- a/crates/bevy_ui/src/ui_node.rs +++ b/crates/bevy_ui/src/ui_node.rs @@ -35,10 +35,14 @@ pub struct Node { pub(crate) calculated_size: Vec2, /// The width of this node's outline. /// If this value is `Auto`, negative or `0.` then no outline will be rendered. + /// Outline updates bypass change detection. /// - /// Automatically calculated by [`super::layout::resolve_outlines_system`]. + /// Automatically calculated by [`super::layout::ui_layout_system`]. pub(crate) outline_width: f32, /// The amount of space between the outline and the edge of the node. + /// Outline updates bypass change detection. + /// + /// Automatically calculated by [`super::layout::ui_layout_system`]. pub(crate) outline_offset: f32, /// The unrounded size of the node as width and height in logical pixels. /// diff --git a/tests/ecs/ambiguity_detection.rs b/tests/ecs/ambiguity_detection.rs index 57adee3174..ca2139b32b 100644 --- a/tests/ecs/ambiguity_detection.rs +++ b/tests/ecs/ambiguity_detection.rs @@ -33,7 +33,7 @@ fn main() { // Over time, we are working to reduce the number: your PR should not increase it. // If you decrease this by fixing an ambiguity, reduce the number to prevent regressions. // See https://github.com/bevyengine/bevy/issues/7386 for progress. - 48, + 46, "Main app has unexpected ambiguities among the following schedules: \n{:#?}.", main_app_ambiguities, );