Divide by UiScale when converting UI coordinates from physical to logical (#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 <mcanders1@gmail.com>
This commit is contained in:
ickshonpe 2023-07-06 21:27:54 +01:00 committed by GitHub
parent 048e00fc32
commit 9655acebb6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 117 additions and 78 deletions

View File

@ -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_derive::{Deref, DerefMut};
use bevy_ecs::{ use bevy_ecs::{
change_detection::DetectChangesMut, change_detection::DetectChangesMut,
@ -138,6 +138,7 @@ pub fn ui_focus_system(
windows: Query<&Window>, windows: Query<&Window>,
mouse_button_input: Res<Input<MouseButton>>, mouse_button_input: Res<Input<MouseButton>>,
touches_input: Res<Touches>, touches_input: Res<Touches>,
ui_scale: Res<UiScale>,
ui_stack: Res<UiStack>, ui_stack: Res<UiStack>,
mut node_query: Query<NodeQuery>, mut node_query: Query<NodeQuery>,
primary_window: Query<Entity, With<PrimaryWindow>>, primary_window: Query<Entity, With<PrimaryWindow>>,
@ -187,7 +188,10 @@ pub fn ui_focus_system(
.ok() .ok()
.and_then(|window| window.cursor_position()) .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, // 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` // from the top node to the bottom one. this will also reset the interaction to `None`

View File

@ -301,7 +301,7 @@ pub fn ui_layout_system(
// compute layouts // compute layouts
ui_surface.compute_window_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; let to_logical = |v| (physical_to_logical_factor * v as f64) as f32;

View File

@ -8,11 +8,11 @@ use bevy_window::{PrimaryWindow, Window};
pub use pipeline::*; pub use pipeline::*;
pub use render_pass::*; pub use render_pass::*;
use crate::UiTextureAtlasImage;
use crate::{ 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_app::prelude::*;
use bevy_asset::{load_internal_asset, AssetEvent, Assets, Handle, HandleUntyped}; use bevy_asset::{load_internal_asset, AssetEvent, Assets, Handle, HandleUntyped};
use bevy_ecs::prelude::*; use bevy_ecs::prelude::*;
@ -170,7 +170,6 @@ pub fn extract_atlas_uinodes(
mut extracted_uinodes: ResMut<ExtractedUiNodes>, mut extracted_uinodes: ResMut<ExtractedUiNodes>,
images: Extract<Res<Assets<Image>>>, images: Extract<Res<Assets<Image>>>,
texture_atlases: Extract<Res<Assets<TextureAtlas>>>, texture_atlases: Extract<Res<Assets<TextureAtlas>>>,
ui_stack: Extract<Res<UiStack>>, ui_stack: Extract<Res<UiStack>>,
uinode_query: Extract< uinode_query: Extract<
Query< Query<
@ -258,6 +257,7 @@ fn resolve_border_thickness(value: Val, parent_width: f32, viewport_size: Vec2)
pub fn extract_uinode_borders( pub fn extract_uinode_borders(
mut extracted_uinodes: ResMut<ExtractedUiNodes>, mut extracted_uinodes: ResMut<ExtractedUiNodes>,
windows: Extract<Query<&Window, With<PrimaryWindow>>>, windows: Extract<Query<&Window, With<PrimaryWindow>>>,
ui_scale: Extract<Res<UiScale>>,
ui_stack: Extract<Res<UiStack>>, ui_stack: Extract<Res<UiStack>>,
uinode_query: Extract< uinode_query: Extract<
Query< Query<
@ -277,10 +277,13 @@ pub fn extract_uinode_borders(
) { ) {
let image = bevy_render::texture::DEFAULT_IMAGE_HANDLE.typed(); let image = bevy_render::texture::DEFAULT_IMAGE_HANDLE.typed();
let viewport_size = windows let ui_logical_viewport_size = windows
.get_single() .get_single()
.map(|window| Vec2::new(window.resolution.width(), window.resolution.height())) .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() { for (stack_index, entity) in ui_stack.uinodes.iter().enumerate() {
if let Ok((node, global_transform, style, border_color, parent, visibility, clip)) = 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 let parent_width = parent
.and_then(|parent| parent_node_query.get(parent.get()).ok()) .and_then(|parent| parent_node_query.get(parent.get()).ok())
.map(|parent_node| parent_node.size().x) .map(|parent_node| parent_node.size().x)
.unwrap_or(viewport_size.x); .unwrap_or(ui_logical_viewport_size.x);
let left = resolve_border_thickness(style.border.left, parent_width, viewport_size); let left =
let right = resolve_border_thickness(style.border.right, parent_width, viewport_size); resolve_border_thickness(style.border.left, parent_width, ui_logical_viewport_size);
let top = resolve_border_thickness(style.border.top, parent_width, viewport_size); let right = resolve_border_thickness(
let bottom = resolve_border_thickness(style.border.bottom, parent_width, viewport_size); 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. // 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. // 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>( pub fn extract_default_ui_camera_view<T: Component>(
mut commands: Commands, mut commands: Commands,
ui_scale: Extract<Res<UiScale>>,
query: Extract<Query<(Entity, &Camera, Option<&UiCameraConfig>), With<T>>>, query: Extract<Query<(Entity, &Camera, Option<&UiCameraConfig>), With<T>>>,
) { ) {
let scale = (ui_scale.scale as f32).recip();
for (entity, camera, camera_ui) in &query { for (entity, camera, camera_ui) in &query {
// ignore cameras with disabled ui // ignore cameras with disabled ui
if matches!(camera_ui, Some(&UiCameraConfig { show_ui: false, .. })) { 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(), camera.physical_viewport_size(),
) { ) {
// use a projection matrix with the origin in the top left instead of the bottom left that comes with OrthographicProjection // use a projection matrix with the origin in the top left instead of the bottom left that comes with OrthographicProjection
let projection_matrix = let projection_matrix = Mat4::orthographic_rh(
Mat4::orthographic_rh(0.0, logical_size.x, logical_size.y, 0.0, 0.0, UI_CAMERA_FAR); 0.0,
logical_size.x * scale,
logical_size.y * scale,
0.0,
0.0,
UI_CAMERA_FAR,
);
let default_camera_view = commands let default_camera_view = commands
.spawn(ExtractedView { .spawn(ExtractedView {
projection: projection_matrix, projection: projection_matrix,
@ -481,6 +502,7 @@ pub fn extract_text_uinodes(
texture_atlases: Extract<Res<Assets<TextureAtlas>>>, texture_atlases: Extract<Res<Assets<TextureAtlas>>>,
windows: Extract<Query<&Window, With<PrimaryWindow>>>, windows: Extract<Query<&Window, With<PrimaryWindow>>>,
ui_stack: Extract<Res<UiStack>>, ui_stack: Extract<Res<UiStack>>,
ui_scale: Extract<Res<UiScale>>,
uinode_query: Extract< uinode_query: Extract<
Query<( Query<(
&Node, &Node,
@ -495,10 +517,11 @@ pub fn extract_text_uinodes(
// TODO: Support window-independent UI scale: https://github.com/bevyengine/bevy/issues/5621 // TODO: Support window-independent UI scale: https://github.com/bevyengine/bevy/issues/5621
let scale_factor = windows let scale_factor = windows
.get_single() .get_single()
.map(|window| window.resolution.scale_factor() as f32) .map(|window| window.resolution.scale_factor())
.unwrap_or(1.0); .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() { for (stack_index, entity) in ui_stack.uinodes.iter().enumerate() {
if let Ok((uinode, global_transform, text, text_layout_info, visibility, clip)) = if let Ok((uinode, global_transform, text, text_layout_info, visibility, clip)) =

View File

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

View File

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

View File

@ -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::*; use bevy::prelude::*;
const PALETTE: [Color; 10] = [ const PALETTE: [Color; 10] = [
Color::ORANGE, Color::RED,
Color::BLUE, Color::YELLOW,
Color::WHITE, Color::WHITE,
Color::BEIGE, Color::BEIGE,
Color::CYAN, Color::CYAN,
@ -15,7 +19,7 @@ const PALETTE: [Color; 10] = [
Color::BLACK, Color::BLACK,
]; ];
#[derive(Default, Debug, Hash, Eq, PartialEq, Clone, States)] #[derive(Component, Default, PartialEq)]
enum Coords { enum Coords {
#[default] #[default]
Viewport, Viewport,
@ -24,55 +28,54 @@ enum Coords {
fn main() { fn main() {
App::new() App::new()
.insert_resource(UiScale { scale: 2.0 })
.add_plugins(DefaultPlugins.set(WindowPlugin { .add_plugins(DefaultPlugins.set(WindowPlugin {
primary_window: Some(Window { primary_window: Some(Window {
resolution: [800., 600.].into(), resolution: [1600., 1200.].into(),
title: "Viewport Coordinates Debug".to_string(), title: "Viewport Coordinates Debug".to_string(),
resizable: false, resizable: false,
..Default::default() ..Default::default()
}), }),
..Default::default() ..Default::default()
})) }))
.add_state::<Coords>()
.add_systems(Startup, setup) .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) .add_systems(Update, update)
.run(); .run();
} }
fn despawn_nodes(mut commands: Commands, query: Query<Entity, With<Node>>) {
for entity in query.iter() {
commands.entity(entity).despawn();
}
}
fn update( fn update(
mut timer: Local<f32>, mut timer: Local<f32>,
mut visible_tree: Local<Coords>,
time: Res<Time>, time: Res<Time>,
state: Res<State<Coords>>, mut coords_style_query: Query<(&Coords, &mut Style)>,
mut next_state: ResMut<NextState<Coords>>,
) { ) {
*timer += time.delta_seconds(); *timer -= time.delta_seconds();
if 1. <= *timer { if *timer <= 0. {
*timer = 0.; *timer = 1.;
next_state.set(if *state.get() == Coords::Viewport { *visible_tree = match *visible_tree {
Coords::Pixel 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 { } else {
Coords::Viewport Display::None
}); };
}
} }
} }
fn setup(mut commands: Commands) { fn setup(mut commands: Commands) {
commands.spawn(Camera2dBundle::default()); 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 commands
.spawn(NodeBundle { .spawn((
NodeBundle {
style: Style { style: Style {
width: Val::Vw(100.), width: Val::Vw(100.),
height: Val::Vh(100.), height: Val::Vh(100.),
@ -80,10 +83,11 @@ fn spawn_with_viewport_coords(mut commands: Commands) {
flex_wrap: FlexWrap::Wrap, flex_wrap: FlexWrap::Wrap,
..default() ..default()
}, },
background_color: PALETTE[0].into(), border_color: PALETTE[0].into(),
border_color: PALETTE[1].into(),
..default() ..default()
}) },
Coords::Viewport,
))
.with_children(|builder| { .with_children(|builder| {
builder.spawn(NodeBundle { builder.spawn(NodeBundle {
style: Style { style: Style {
@ -155,9 +159,10 @@ 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 commands
.spawn(NodeBundle { .spawn((
NodeBundle {
style: Style { style: Style {
width: Val::Px(800.), width: Val::Px(800.),
height: Val::Px(600.), height: Val::Px(600.),
@ -165,10 +170,11 @@ fn spawn_with_pixel_coords(mut commands: Commands) {
flex_wrap: FlexWrap::Wrap, flex_wrap: FlexWrap::Wrap,
..default() ..default()
}, },
background_color: PALETTE[1].into(), border_color: PALETTE[1].into(),
border_color: PALETTE[0].into(),
..default() ..default()
}) },
Coords::Pixel,
))
.with_children(|builder| { .with_children(|builder| {
builder.spawn(NodeBundle { builder.spawn(NodeBundle {
style: Style { style: Style {