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 <alice.i.cecile@gmail.com>
This commit is contained in:
ickshonpe 2024-09-02 17:56:58 +01:00 committed by GitHub
parent f1414cba23
commit be100b8760
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 33 additions and 45 deletions

View file

@ -154,7 +154,6 @@ impl Plugin for AccessibilityPlugin {
.after(bevy_transform::TransformSystem::TransformPropagate) .after(bevy_transform::TransformSystem::TransformPropagate)
.after(CameraUpdateSystem) .after(CameraUpdateSystem)
// the listed systems do not affect calculated size // the listed systems do not affect calculated size
.ambiguous_with(crate::resolve_outlines_system)
.ambiguous_with(crate::ui_stack_system), .ambiguous_with(crate::ui_stack_system),
button_changed, button_changed,
image_changed, image_changed,

View file

@ -94,7 +94,7 @@ pub fn ui_layout_system(
children_query: Query<(Entity, Ref<Children>), With<Node>>, children_query: Query<(Entity, Ref<Children>), With<Node>>,
just_children_query: Query<&Children>, just_children_query: Query<&Children>,
mut removed_components: UiLayoutSystemRemovedComponentParam, 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 buffer_query: Query<&mut CosmicBuffer>,
#[cfg(feature = "bevy_text")] mut text_pipeline: ResMut<TextPipeline>, #[cfg(feature = "bevy_text")] mut text_pipeline: ResMut<TextPipeline>,
) { ) {
@ -247,6 +247,7 @@ pub fn ui_layout_system(
update_uinode_geometry_recursive( update_uinode_geometry_recursive(
*root, *root,
&ui_surface, &ui_surface,
None,
&mut node_transform_query, &mut node_transform_query,
&just_children_query, &just_children_query,
inverse_target_scale_factor, inverse_target_scale_factor,
@ -259,13 +260,14 @@ pub fn ui_layout_system(
fn update_uinode_geometry_recursive( fn update_uinode_geometry_recursive(
entity: Entity, entity: Entity,
ui_surface: &UiSurface, ui_surface: &UiSurface,
node_transform_query: &mut Query<(&mut Node, &mut Transform)>, root_size: Option<Vec2>,
node_transform_query: &mut Query<(&mut Node, &mut Transform, Option<&Outline>)>,
children_query: &Query<&Children>, children_query: &Query<&Children>,
inverse_target_scale_factor: f32, inverse_target_scale_factor: f32,
parent_size: Vec2, parent_size: Vec2,
mut absolute_location: 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 { let Ok(layout) = ui_surface.get_layout(entity) else {
return; return;
}; };
@ -287,39 +289,11 @@ pub fn ui_layout_system(
node.calculated_size = rounded_size; node.calculated_size = rounded_size;
node.unrounded_size = layout_size; node.unrounded_size = layout_size;
} }
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,
node_transform_query,
children_query,
inverse_target_scale_factor,
rounded_size,
absolute_location,
);
}
}
}
}
}
/// Resolve and update the widths of Node outlines let viewport_size = root_size.unwrap_or(node.calculated_size);
pub fn resolve_outlines_system(
primary_window: Query<&Window, With<PrimaryWindow>>,
ui_scale: Res<UiScale>,
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() { if let Some(outline) = outline {
// don't trigger change detection when only outlines are changed
let node = node.bypass_change_detection(); let node = node.bypass_change_detection();
node.outline_width = outline node.outline_width = outline
.width .width
@ -333,6 +307,27 @@ pub fn resolve_outlines_system(
.unwrap_or(0.) .unwrap_or(0.)
.max(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,
rounded_size,
absolute_location,
);
}
}
}
}
} }
#[inline] #[inline]

View file

@ -92,10 +92,6 @@ pub enum UiSystem {
/// ///
/// Runs in [`PostUpdate`]. /// Runs in [`PostUpdate`].
Stack, Stack,
/// After this label, node outline widths have been updated.
///
/// Runs in [`PostUpdate`].
Outlines,
} }
/// The current scale of the UI. /// The current scale of the UI.
@ -153,7 +149,7 @@ impl Plugin for UiPlugin {
CameraUpdateSystem, CameraUpdateSystem,
UiSystem::Prepare.before(UiSystem::Stack), UiSystem::Prepare.before(UiSystem::Stack),
UiSystem::Layout, UiSystem::Layout,
(UiSystem::PostLayout, UiSystem::Outlines), UiSystem::PostLayout,
) )
.chain(), .chain(),
) )
@ -170,16 +166,10 @@ impl Plugin for UiPlugin {
ui_layout_system ui_layout_system
.in_set(UiSystem::Layout) .in_set(UiSystem::Layout)
.before(TransformSystem::TransformPropagate), .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 ui_stack_system
.in_set(UiSystem::Stack) .in_set(UiSystem::Stack)
// the systems don't care about stack index // the systems don't care about stack index
.ambiguous_with(update_clipping_system) .ambiguous_with(update_clipping_system)
.ambiguous_with(resolve_outlines_system)
.ambiguous_with(ui_layout_system) .ambiguous_with(ui_layout_system)
.in_set(AmbiguousWithTextSystem), .in_set(AmbiguousWithTextSystem),
update_clipping_system.after(TransformSystem::TransformPropagate), update_clipping_system.after(TransformSystem::TransformPropagate),

View file

@ -35,10 +35,14 @@ pub struct Node {
pub(crate) calculated_size: Vec2, pub(crate) calculated_size: Vec2,
/// The width of this node's outline. /// The width of this node's outline.
/// If this value is `Auto`, negative or `0.` then no outline will be rendered. /// 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, pub(crate) outline_width: f32,
/// The amount of space between the outline and the edge of the node. /// 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, pub(crate) outline_offset: f32,
/// The unrounded size of the node as width and height in logical pixels. /// The unrounded size of the node as width and height in logical pixels.
/// ///

View file

@ -33,7 +33,7 @@ fn main() {
// Over time, we are working to reduce the number: your PR should not increase it. // 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. // If you decrease this by fixing an ambiguity, reduce the number to prevent regressions.
// See https://github.com/bevyengine/bevy/issues/7386 for progress. // See https://github.com/bevyengine/bevy/issues/7386 for progress.
48, 46,
"Main app has unexpected ambiguities among the following schedules: \n{:#?}.", "Main app has unexpected ambiguities among the following schedules: \n{:#?}.",
main_app_ambiguities, main_app_ambiguities,
); );