Divide by UiScale when converting UI coordinates from physical to l…

…ogical (bevyengine#8720)

# Objective

After the UI layout is computed when the coordinates are converted back
from physical coordinates to logical coordinates the `UiScale` is
ignored. This results in a confusing situation where we have two
different systems of logical coordinates.

Example:

```rust
use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, update)
        .run();
}

fn setup(mut commands: Commands, mut ui_scale: ResMut<UiScale>) {
    ui_scale.scale = 4.;

    commands.spawn(Camera2dBundle::default());
    commands.spawn(NodeBundle {
        style: Style {
            align_items: AlignItems::Center,
            justify_content: JustifyContent::Center,
            width: Val::Percent(100.),
            ..Default::default()
        },
        ..Default::default()
    })
    .with_children(|builder| {
        builder.spawn(NodeBundle {
            style: Style {
                width: Val::Px(100.),
                height: Val::Px(100.),
                ..Default::default()
            },
            background_color: Color::MAROON.into(),
            ..Default::default()
        }).with_children(|builder| {
            builder.spawn(TextBundle::from_section("", TextStyle::default());
        });
    });
}

fn update(
    mut text_query: Query<(&mut Text, &Parent)>,
    node_query: Query<Ref<Node>>,
) {
    for (mut text, parent) in text_query.iter_mut() {
        let node = node_query.get(parent.get()).unwrap();
        if node.is_changed() {
            text.sections[0].value = format!("size: {}", node.size());
        }
    }
}
```
result:

![Bevy App 30_05_2023
16_54_32](https://github.com/bevyengine/bevy/assets/27962798/a5ecbf31-0a12-4669-87df-b0c32f058732)

We asked for a 100x100 UI node but the Node's size is multiplied by the
value of `UiScale` to give a logical size of 400x400.

## Solution

Divide the output physical coordinates by `UiScale` in
`ui_layout_system` and multiply the logical viewport size by `UiScale`
when creating the projection matrix for the UI's `ExtractedView` in
`extract_default_ui_camera_view`.

---

## Changelog
* The UI layout's physical coordinates are divided by both the window
scale factor and `UiScale` when converting them back to logical
coordinates. The logical size of Ui nodes now matches the values given
to their size constraints.
* Multiply the logical viewport size by `UiScale` before creating the
projection matrix for the UI's `ExtractedView` in
`extract_default_ui_camera_view`.
* In `ui_focus_system` the cursor position returned from `Window` is
divided by `UiScale`.
* Added a scale factor parameter to `Node::physical_size` and
`Node::physical_rect`.
* The example `viewport_debug` now uses a `UiScale` of 2. to ensure that
viewport coordinates are working correctly with a non-unit `UiScale`.

## Migration Guide

Physical UI coordinates are now divided by both the `UiScale` and the
window's scale factor to compute the logical sizes and positions of UI
nodes.

This ensures that UI Node size and position values, held by the `Node`
and `GlobalTransform` components, conform to the same logical coordinate
system as the style constraints from which they are derived,
irrespective of the current `scale_factor` and `UiScale`.

---------

Co-authored-by: Carter Anderson <[email protected]>

crates/bevy_ui/src/focus.rs

@@ -1,4 +1,4 @@
-use crate::{camera_config::UiCameraConfig, CalculatedClip, Node, UiStack};
+use crate::{camera_config::UiCameraConfig, CalculatedClip, Node, UiScale, UiStack};
 use bevy_derive::{Deref, DerefMut};
 use bevy_ecs::{
     change_detection::DetectChangesMut,
@@ -138,6 +138,7 @@ pub fn ui_focus_system(
     windows: Query<&Window>,
     mouse_button_input: Res<Input<MouseButton>>,
     touches_input: Res<Touches>,
+    ui_scale: Res<UiScale>,
     ui_stack: Res<UiStack>,
     mut node_query: Query<NodeQuery>,
     primary_window: Query<Entity, With<PrimaryWindow>>,
@@ -187,7 +188,10 @@ pub fn ui_focus_system(
                 .ok()
                 .and_then(|window| window.cursor_position())
         })
-        .or_else(|| touches_input.first_pressed_position());
+        .or_else(|| touches_input.first_pressed_position())
+        // The cursor position returned by `Window` only takes into account the window scale factor and not `UiScale`.
+        // To convert the cursor position to logical UI viewport coordinates we have to divide it by `UiScale`.
+        .map(|cursor_position| cursor_position / ui_scale.scale as f32);
 
     // prepare an iterator that contains all the nodes that have the cursor in their rect,
     // from the top node to the bottom one. this will also reset the interaction to `None`

crates/bevy_ui/src/layout/mod.rs

@@ -301,7 +301,7 @@ pub fn ui_layout_system(
     // compute layouts
     ui_surface.compute_window_layouts();
 
-    let physical_to_logical_factor = 1. / logical_to_physical_factor;
+    let physical_to_logical_factor = 1. / scale_factor;
 
     let to_logical = |v| (physical_to_logical_factor * v as f64) as f32;
 

crates/bevy_ui/src/render/mod.rs

@@ -8,11 +8,11 @@ use bevy_window::{PrimaryWindow, Window};
 pub use pipeline::*;
 pub use render_pass::*;
 
-use crate::UiTextureAtlasImage;
 use crate::{
-    prelude::UiCameraConfig, BackgroundColor, BorderColor, CalculatedClip, Node, UiImage, UiStack,
+    prelude::UiCameraConfig, BackgroundColor, BorderColor, CalculatedClip, ContentSize, Node,
+    Style, UiImage, UiScale, UiStack, UiTextureAtlasImage, Val,
 };
-use crate::{ContentSize, Style, Val};
+
 use bevy_app::prelude::*;
 use bevy_asset::{load_internal_asset, AssetEvent, Assets, Handle, HandleUntyped};
 use bevy_ecs::prelude::*;
@@ -170,7 +170,6 @@ pub fn extract_atlas_uinodes(
     mut extracted_uinodes: ResMut<ExtractedUiNodes>,
     images: Extract<Res<Assets<Image>>>,
     texture_atlases: Extract<Res<Assets<TextureAtlas>>>,
-
     ui_stack: Extract<Res<UiStack>>,
     uinode_query: Extract<
         Query<
@@ -258,6 +257,7 @@ fn resolve_border_thickness(value: Val, parent_width: f32, viewport_size: Vec2)
 pub fn extract_uinode_borders(
     mut extracted_uinodes: ResMut<ExtractedUiNodes>,
     windows: Extract<Query<&Window, With<PrimaryWindow>>>,
+    ui_scale: Extract<Res<UiScale>>,
     ui_stack: Extract<Res<UiStack>>,
     uinode_query: Extract<
         Query<
@@ -277,10 +277,13 @@ pub fn extract_uinode_borders(
 ) {
     let image = bevy_render::texture::DEFAULT_IMAGE_HANDLE.typed();
 
-    let viewport_size = windows
+    let ui_logical_viewport_size = windows
         .get_single()
         .map(|window| Vec2::new(window.resolution.width(), window.resolution.height()))
-        .unwrap_or(Vec2::ZERO);
+        .unwrap_or(Vec2::ZERO)
+        // The logical window resolutin returned by `Window` only takes into account the window scale factor and not `UiScale`,
+        // so we have to divide by `UiScale` to get the size of the UI viewport.
+        / ui_scale.scale as f32;
 
     for (stack_index, entity) in ui_stack.uinodes.iter().enumerate() {
         if let Ok((node, global_transform, style, border_color, parent, visibility, clip)) =
@@ -300,11 +303,21 @@ pub fn extract_uinode_borders(
             let parent_width = parent
                 .and_then(|parent| parent_node_query.get(parent.get()).ok())
                 .map(|parent_node| parent_node.size().x)
-                .unwrap_or(viewport_size.x);
-            let left = resolve_border_thickness(style.border.left, parent_width, viewport_size);
-            let right = resolve_border_thickness(style.border.right, parent_width, viewport_size);
-            let top = resolve_border_thickness(style.border.top, parent_width, viewport_size);
-            let bottom = resolve_border_thickness(style.border.bottom, parent_width, viewport_size);
+                .unwrap_or(ui_logical_viewport_size.x);
+            let left =
+                resolve_border_thickness(style.border.left, parent_width, ui_logical_viewport_size);
+            let right = resolve_border_thickness(
+                style.border.right,
+                parent_width,
+                ui_logical_viewport_size,
+            );
+            let top =
+                resolve_border_thickness(style.border.top, parent_width, ui_logical_viewport_size);
+            let bottom = resolve_border_thickness(
+                style.border.bottom,
+                parent_width,
+                ui_logical_viewport_size,
+            );
 
             // Calculate the border rects, ensuring no overlap.
             // The border occupies the space between the node's bounding rect and the node's bounding rect inset in each direction by the node's corresponding border value.
@@ -433,8 +446,10 @@ pub struct DefaultCameraView(pub Entity);
 
 pub fn extract_default_ui_camera_view<T: Component>(
     mut commands: Commands,
+    ui_scale: Extract<Res<UiScale>>,
     query: Extract<Query<(Entity, &Camera, Option<&UiCameraConfig>), With<T>>>,
 ) {
+    let scale = (ui_scale.scale as f32).recip();
     for (entity, camera, camera_ui) in &query {
         // ignore cameras with disabled ui
         if matches!(camera_ui, Some(&UiCameraConfig { show_ui: false, .. })) {
@@ -446,8 +461,14 @@ pub fn extract_default_ui_camera_view<T: Component>(
             camera.physical_viewport_size(),
         ) {
             // use a projection matrix with the origin in the top left instead of the bottom left that comes with OrthographicProjection
-            let projection_matrix =
-                Mat4::orthographic_rh(0.0, logical_size.x, logical_size.y, 0.0, 0.0, UI_CAMERA_FAR);
+            let projection_matrix = Mat4::orthographic_rh(
+                0.0,
+                logical_size.x * scale,
+                logical_size.y * scale,
+                0.0,
+                0.0,
+                UI_CAMERA_FAR,
+            );
             let default_camera_view = commands
                 .spawn(ExtractedView {
                     projection: projection_matrix,
@@ -481,6 +502,7 @@ pub fn extract_text_uinodes(
     texture_atlases: Extract<Res<Assets<TextureAtlas>>>,
     windows: Extract<Query<&Window, With<PrimaryWindow>>>,
     ui_stack: Extract<Res<UiStack>>,
+    ui_scale: Extract<Res<UiScale>>,
     uinode_query: Extract<
         Query<(
             &Node,
@@ -495,10 +517,11 @@ pub fn extract_text_uinodes(
     // TODO: Support window-independent UI scale: https://github.com/bevyengine/bevy/issues/5621
     let scale_factor = windows
         .get_single()
-        .map(|window| window.resolution.scale_factor() as f32)
-        .unwrap_or(1.0);
+        .map(|window| window.resolution.scale_factor())
+        .unwrap_or(1.0)
+        * ui_scale.scale;
 
-    let inverse_scale_factor = scale_factor.recip();
+    let inverse_scale_factor = (scale_factor as f32).recip();
 
     for (stack_index, entity) in ui_stack.uinodes.iter().enumerate() {
         if let Ok((uinode, global_transform, text, text_layout_info, visibility, clip)) =

crates/bevy_ui/src/ui_node.rs

@@ -29,12 +29,12 @@ impl Node {
         self.calculated_size
     }
 
-    /// Returns the size of the node in physical pixels based on the given scale factor.
+    /// Returns the size of the node in physical pixels based on the given scale factor and `UiScale`.
     #[inline]
-    pub fn physical_size(&self, scale_factor: f64) -> Vec2 {
+    pub fn physical_size(&self, scale_factor: f64, ui_scale: f64) -> Vec2 {
         Vec2::new(
-            (self.calculated_size.x as f64 * scale_factor) as f32,
-            (self.calculated_size.y as f64 * scale_factor) as f32,
+            (self.calculated_size.x as f64 * scale_factor * ui_scale) as f32,
+            (self.calculated_size.y as f64 * scale_factor * ui_scale) as f32,
         )
     }
 
@@ -46,16 +46,21 @@ impl Node {
 
     /// Returns the physical pixel coordinates of the UI node, based on its [`GlobalTransform`] and the scale factor.
     #[inline]
-    pub fn physical_rect(&self, transform: &GlobalTransform, scale_factor: f64) -> Rect {
+    pub fn physical_rect(
+        &self,
+        transform: &GlobalTransform,
+        scale_factor: f64,
+        ui_scale: f64,
+    ) -> Rect {
         let rect = self.logical_rect(transform);
         Rect {
             min: Vec2::new(
-                (rect.min.x as f64 * scale_factor) as f32,
-                (rect.min.y as f64 * scale_factor) as f32,
+                (rect.min.x as f64 * scale_factor * ui_scale) as f32,
+                (rect.min.y as f64 * scale_factor * ui_scale) as f32,
             ),
             max: Vec2::new(
-                (rect.max.x as f64 * scale_factor) as f32,
-                (rect.max.y as f64 * scale_factor) as f32,
+                (rect.max.x as f64 * scale_factor * ui_scale) as f32,
+                (rect.max.y as f64 * scale_factor * ui_scale) as f32,
             ),
         }
     }

crates/bevy_ui/src/widget/text.rs

@@ -184,7 +184,8 @@ fn queue_text(
             // With `NoWrap` set, no constraints are placed on the width of the text.
             Vec2::splat(f32::INFINITY)
         } else {
-            node.physical_size(scale_factor)
+            // `scale_factor` is already multiplied by `UiScale`
+            node.physical_size(scale_factor, 1.)
         };
 
         match text_pipeline.queue_text(

examples/ui/viewport_debug.rs

@@ -1,10 +1,14 @@
-//! An example for debugging viewport coordinates
-
+//! A simple example for debugging viewport coordinates
+//!
+//! This example creates two uinode trees, one using viewport coordinates and one using pixel coordinates,
+//! and then switches between them once per second using the `Display` style property.
+//! If there are no problems both layouts should be identical, except for the color of the margin changing which is used to signal that the displayed uinode tree has changed
+//! (red for viewport, yellow for pixel).
 use bevy::prelude::*;
 
 const PALETTE: [Color; 10] = [
-    Color::ORANGE,
-    Color::BLUE,
+    Color::RED,
+    Color::YELLOW,
     Color::WHITE,
     Color::BEIGE,
     Color::CYAN,
@@ -15,7 +19,7 @@ const PALETTE: [Color; 10] = [
     Color::BLACK,
 ];
 
-#[derive(Default, Debug, Hash, Eq, PartialEq, Clone, States)]
+#[derive(Component, Default, PartialEq)]
 enum Coords {
     #[default]
     Viewport,
@@ -24,66 +28,66 @@ enum Coords {
 
 fn main() {
     App::new()
+        .insert_resource(UiScale { scale: 2.0 })
         .add_plugins(DefaultPlugins.set(WindowPlugin {
             primary_window: Some(Window {
-                resolution: [800., 600.].into(),
+                resolution: [1600., 1200.].into(),
                 title: "Viewport Coordinates Debug".to_string(),
                 resizable: false,
                 ..Default::default()
             }),
             ..Default::default()
         }))
-        .add_state::<Coords>()
         .add_systems(Startup, setup)
-        .add_systems(OnEnter(Coords::Viewport), spawn_with_viewport_coords)
-        .add_systems(OnEnter(Coords::Pixel), spawn_with_pixel_coords)
-        .add_systems(OnExit(Coords::Viewport), despawn_nodes)
-        .add_systems(OnExit(Coords::Pixel), despawn_nodes)
         .add_systems(Update, update)
         .run();
 }
 
-fn despawn_nodes(mut commands: Commands, query: Query<Entity, With<Node>>) {
-    for entity in query.iter() {
-        commands.entity(entity).despawn();
-    }
-}
-
 fn update(
     mut timer: Local<f32>,
+    mut visible_tree: Local<Coords>,
     time: Res<Time>,
-    state: Res<State<Coords>>,
-    mut next_state: ResMut<NextState<Coords>>,
+    mut coords_style_query: Query<(&Coords, &mut Style)>,
 ) {
-    *timer += time.delta_seconds();
-    if 1. <= *timer {
-        *timer = 0.;
-        next_state.set(if *state.get() == Coords::Viewport {
-            Coords::Pixel
-        } else {
-            Coords::Viewport
-        });
+    *timer -= time.delta_seconds();
+    if *timer <= 0. {
+        *timer = 1.;
+        *visible_tree = match *visible_tree {
+            Coords::Viewport => Coords::Pixel,
+            Coords::Pixel => Coords::Viewport,
+        };
+        for (coords, mut style) in coords_style_query.iter_mut() {
+            style.display = if *coords == *visible_tree {
+                Display::Flex
+            } else {
+                Display::None
+            };
+        }
     }
 }
 
 fn setup(mut commands: Commands) {
     commands.spawn(Camera2dBundle::default());
+    spawn_with_viewport_coords(&mut commands);
+    spawn_with_pixel_coords(&mut commands);
 }
 
-fn spawn_with_viewport_coords(mut commands: Commands) {
+fn spawn_with_viewport_coords(commands: &mut Commands) {
     commands
-        .spawn(NodeBundle {
-            style: Style {
-                width: Val::Vw(100.),
-                height: Val::Vh(100.),
-                border: UiRect::axes(Val::Vw(5.), Val::Vh(5.)),
-                flex_wrap: FlexWrap::Wrap,
+        .spawn((
+            NodeBundle {
+                style: Style {
+                    width: Val::Vw(100.),
+                    height: Val::Vh(100.),
+                    border: UiRect::axes(Val::Vw(5.), Val::Vh(5.)),
+                    flex_wrap: FlexWrap::Wrap,
+                    ..default()
+                },
+                border_color: PALETTE[0].into(),
                 ..default()
             },
-            background_color: PALETTE[0].into(),
-            border_color: PALETTE[1].into(),
-            ..default()
-        })
+            Coords::Viewport,
+        ))
         .with_children(|builder| {
             builder.spawn(NodeBundle {
                 style: Style {
@@ -155,20 +159,22 @@ fn spawn_with_viewport_coords(mut commands: Commands) {
         });
 }
 
-fn spawn_with_pixel_coords(mut commands: Commands) {
+fn spawn_with_pixel_coords(commands: &mut Commands) {
     commands
-        .spawn(NodeBundle {
-            style: Style {
-                width: Val::Px(800.),
-                height: Val::Px(600.),
-                border: UiRect::axes(Val::Px(40.), Val::Px(30.)),
-                flex_wrap: FlexWrap::Wrap,
+        .spawn((
+            NodeBundle {
+                style: Style {
+                    width: Val::Px(800.),
+                    height: Val::Px(600.),
+                    border: UiRect::axes(Val::Px(40.), Val::Px(30.)),
+                    flex_wrap: FlexWrap::Wrap,
+                    ..default()
+                },
+                border_color: PALETTE[1].into(),
                 ..default()
             },
-            background_color: PALETTE[1].into(),
-            border_color: PALETTE[0].into(),
-            ..default()
-        })
+            Coords::Pixel,
+        ))
         .with_children(|builder| {
             builder.spawn(NodeBundle {
                 style: Style {