Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Transform in the UI with NodeTransform #8240

Open
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Mar 28, 2023

Objective

Bevy UI uses Transform and GlobalTransform to manage node positions. There are lots of problems:

  • Users can modify Transform breaking the UI layout.
  • Transform and GlobalTransform are big, 112 bytes together in total.
  • The UI logic is much more complex and fragile than it needs to be. Look at Fix size of clipped text glyphs. #8197 which has persisted despite being fixed multiple times.
  • Users struggle to understand how to locate UI elements within the layout using GlobalTransform.
  • Users naturally expect to be able to control the scaling and rotation of a UI element by modifying its Transform.
  • propagate_transforms has to be run after flex_layout_system
  • The UI node tree is walked multiple times.

Solution

Remove the Transform and GlobalTransform components and queries from the UI bundles and systems. Replace them with a a tuple struct component wrapping an Affine2 called NodeTransform.

Bevy.App.2023-03-30.14-12-09.mp4

Changelog

  • Added NodeTransform which is a tuple struct component wrapping an Affine2 called NodeTransform.
  • Added NodeTransform to the UI node bundles.
  • Removed Transform and GlobalTransform` from the UI node bundles.
  • Removed TransformSystem after and before conditions from UiPlugin
  • Modified the UI clipping, focus, extraction and flex layout systems to use NodePosition instead of Transform/GlobalTransform.
  • Fixed ui_focus_system to respond correctly to interactions with rotated and scaled elements.
  • Added an example node_transform that demos interactions with translated, rotated and scaled buttons.
  • Added NodeTranslation, NodeRotation and NodeScale components.

Migration Guide

The Transform and GlobalTransform components have been replaced in Bevy UI's node bundles by the component NodeTransform.

The components NodeTranslation, NodeRotation and NodeScale can be added to UI node entities to allow translation, rotation and scaling of a node and its children.

* Added `NodePosition` component
* Added `update_node_positions` system that walks the UI node tree and updates the `NodePosition` component
* Added the `NodePosition` to the relevant Ui node bundles.
* Added the variant `Positions` to the `UiSystem` enum.
* Changed `Node` helper functions to calculate the node's rect using `NodePosition` instead of `GlobalTransform`.
* Changed `ExtractUiNodes` to use `NodePosition` instead of `GlobalTransform`.
* Removed `Transform` and `GlobalTransform` from ui node bundles.
* Changed `update_clipping_system` to use the `NodePosition` instead of the `GlobalTransform` to calculate the clipping bounds.
* Changed `flex_node_system` to no longer query for or update `Transform`.
* Updated focus and accessibility systems to use `NodePosition` instead of `GlobalTransform`.
* Moved position calculations into `flex_node_system`
* Removed `UiSystem::Positions`
* Removed the `update_node_positions_recursive` function
@ickshonpe ickshonpe changed the title NodePosition Replace Transform and GlobalTransform in the UI with NodePosition Mar 28, 2023
@alice-i-cecile alice-i-cecile added C-Performance A change motivated by improving speed, memory usage or compile times A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Mar 28, 2023
@alice-i-cecile alice-i-cecile added this to the 0.11 milestone Mar 28, 2023
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Mar 28, 2023

This partially fixes #7876.

Strongly in favor of this direction. Transform is the wrong abstraction for what we're representing here, with serious performance overhead and usability problems.

@james7132
Copy link
Member

Haven't been able to give this a thorough review just yet, but I'm also in favor of this change, if not only to avoid transform propagation from wasting time on UI hierarchies when the layoutting algorithm already does that natively.

@james7132 james7132 added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Mar 28, 2023
@mockersf
Copy link
Member

mockersf commented Mar 28, 2023

This is a regression, as we lose the ability to rotate and scale independently of the layout.

Having a Transform in UI allows us to mimic part of the transform functions in CSS.

I would prefer to add support for (even limited to rotation and scale) transform functions to our UI before removing the actual Transform

@mockersf mockersf added the X-Controversial There is active debate or serious implications around merging this PR label Mar 28, 2023
* `NodePosition` becomes `NodeTransform` and now holds a `Mat4` instead of a `Vec2`.
* Added new components `NodeTranslation`, `NodeRotation` and `NodeScale`.
* Changed node geometry calculations to work with `NodeTransform`.
* Fixed focus to work with rotated and scaled ui nodes.
* Renamed `calculated` field of `NodeTransform` to `matrix`.
* Added an example `node_rotation`.
* Added `NodeTransform` helper functions.
* `NodeScale` default inner value changed to `Vec2::ONE`
* Renamed the example to `node_transform`
* Added scaling and translation to the example
@github-actions
Copy link
Contributor

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

ode_transform example.
@github-actions
Copy link
Contributor

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

@ickshonpe ickshonpe changed the title Replace Transform and GlobalTransform in the UI with NodeTransform Replace Transform in the UI with NodeTransform Mar 30, 2023
@mockersf
Copy link
Member

mockersf commented Apr 6, 2023

on rotations, this does not behave like I expected it to:

use bevy::prelude::*;

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

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
    commands.spawn(Camera2dBundle::default());

    commands
        .spawn(NodeBundle { ..default() })
        .with_children(|parent| {
            parent.spawn((TextBundle::from_section(
                "Text Example 1",
                TextStyle {
                    font: asset_server.load("fonts/FiraSans-Bold.ttf"),
                    font_size: 30.0,
                    color: Color::WHITE,
                },
            ),));
            parent.spawn((
                TextBundle::from_section(
                    "Text Example 2",
                    TextStyle {
                        font: asset_server.load("fonts/FiraSans-Bold.ttf"),
                        font_size: 30.0,
                        color: Color::WHITE,
                    },
                ),
                NodeRotation::default(),
            ));
        });
}

fn rotate(mut rots: Query<&mut NodeRotation>, time: Res<Time>) {
    for mut rot in &mut rots {
        rot.0 += time.delta_seconds();
    }
}
text-rotation.mp4

background_color: Color::WHITE.into(),
..default()
})
.insert(RotateButton(-std::f32::consts::PI / 8.))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.insert(RotateButton(-std::f32::consts::PI / 8.))
.insert(RotateButton(-std::f32::consts::FRAC_PI_8))

You should use the appropriate constants instead of dividing PI
see https://doc.rust-lang.org/std/f32/consts/index.html

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also do the other PI fractions

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Apr 6, 2023

on rotations, this does not behave like I expected it to:

 ...

This is because it's rotating the Ui Node, not only the text. If you add background colors it becomes obvious what is happening:

text-rotation-example

By default items stretch to fill the cross-axis, you can get the text to rotate on its center by setting AlignItems::Start on the outer node:

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
    commands.spawn(Camera2dBundle::default());

    commands
        .spawn(NodeBundle { 
            style: Style {
                align_items: AlignItems::Start,
                ..Default::default()
            },
            ..default() })
        .with_children(|parent| {
            parent.spawn(
                (TextBundle::from_section(
                    "Text Example 1",
                    TextStyle {
                        font: asset_server.load("fonts/FiraSans-Bold.ttf"),
                        font_size: 30.0,
                        color: Color::WHITE,
                    },
                )
                .with_background_color(Color::NAVY)),
            );
            parent.spawn((
                TextBundle::from_section(
                    "Text Example 2",
                    TextStyle {
                        font: asset_server.load("fonts/FiraSans-Bold.ttf"),
                        font_size: 30.0,
                        color: Color::WHITE,
                    },
                )
                .with_background_color(Color::MAROON),
                NodeRotation::default(),
            ));
        });
}

I thought about adding some sort of anchors, but I think it goes beyond the scope of this PR probably.

@alice-i-cecile
Copy link
Member

Agreed: I'd like anchors, but not in this PR. IMO rotating the entire node is the correct behavior here.

@mockersf
Copy link
Member

mockersf commented Apr 6, 2023

Still I was expecting it to behave like this html:

text-rotation-html.mp4

EDIT: this PR does behave the same as in Bevy 0.10

use bevy::prelude::*;

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

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
    commands.spawn(Camera2dBundle::default());

    commands
        .spawn(NodeBundle { ..default() })
        .with_children(|parent| {
            parent.spawn((TextBundle::from_section(
                "Text Example 1",
                TextStyle {
                    font: asset_server.load("fonts/FiraSans-Bold.ttf"),
                    font_size: 30.0,
                    color: Color::WHITE,
                },
            ),));
            parent.spawn((
                TextBundle::from_section(
                    "Text Example 2",
                    TextStyle {
                        font: asset_server.load("fonts/FiraSans-Bold.ttf"),
                        font_size: 30.0,
                        color: Color::WHITE,
                    },
                ),
                NodeRotation,
            ));
        });
}

#[derive(Component)]
struct NodeRotation;

fn rotate(mut rots: Query<&mut Transform, With<NodeRotation>>, time: Res<Time>) {
    for mut rot in &mut rots {
        rot.rotate_local_z(time.delta_seconds());
    }
}

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Apr 11, 2023

Still I was expecting it to behave like this HTML:

...

Rotating content like this text is really easy to implement at the widget level. We would just need to add a rotation value to TextBundle or Text. Then in extract_text_uinodes multiply the transform by the rotation.

@alice-i-cecile alice-i-cecile modified the milestones: 0.11, 0.12 Jun 19, 2023
@alice-i-cecile alice-i-cecile modified the milestones: 0.12, 0.13 Sep 29, 2023
@alice-i-cecile alice-i-cecile removed this from the 0.13 milestone Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide X-Controversial There is active debate or serious implications around merging this PR
Projects
Status: Candidate
Development

Successfully merging this pull request may close these issues.

5 participants