Skip to content

Commit

Permalink
Resolve UI outlines using the correct target's viewport size (#14947)
Browse files Browse the repository at this point in the history
`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

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>
  • Loading branch information
2 people authored and mockersf committed Sep 5, 2024
1 parent a4ea708 commit 489c1e1
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 49 deletions.
1 change: 0 additions & 1 deletion crates/bevy_ui/src/accessibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
57 changes: 26 additions & 31 deletions crates/bevy_ui/src/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub fn ui_layout_system(
children_query: Query<(Entity, Ref<Children>), With<Node>>,
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>)>,
) {
struct CameraLayoutInfo {
size: UVec2,
Expand Down Expand Up @@ -232,6 +232,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,
Expand All @@ -244,13 +245,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<Vec2>,
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;
};
Expand All @@ -272,14 +274,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,
Expand All @@ -292,34 +315,6 @@ pub fn ui_layout_system(
}
}

/// Resolve and update the widths of Node outlines
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| 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 round_ties_up(value: f32) -> f32 {
Expand Down
17 changes: 1 addition & 16 deletions crates/bevy_ui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ pub enum UiSystem {
Focus,
/// After this label, the [`UiStack`] resource has been updated
Stack,
/// After this label, node outline widths have been updated
Outlines,
}

/// The current scale of the UI.
Expand Down Expand Up @@ -132,13 +130,7 @@ impl Plugin for UiPlugin {
.register_type::<Outline>()
.configure_sets(
PostUpdate,
(
CameraUpdateSystem,
UiSystem::Stack,
UiSystem::Layout,
UiSystem::Outlines,
)
.chain(),
(CameraUpdateSystem, UiSystem::Stack, UiSystem::Layout).chain(),
)
.add_systems(
PreUpdate,
Expand All @@ -156,17 +148,10 @@ impl Plugin for UiPlugin {
ui_layout_system
.in_set(UiSystem::Layout)
.before(TransformSystem::TransformPropagate),
resolve_outlines_system
.in_set(UiSystem::Outlines)
.after(UiSystem::Layout)
// 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),
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_ui/src/ui_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down

0 comments on commit 489c1e1

Please sign in to comment.