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.

Sign up for GitHub

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

Jump to bottom

Have a separate implicit viewport node per root node + make viewport node Display::Grid #9637

Merged
merged 9 commits into from
Sep 19, 2023
Merged

Have a separate implicit viewport node per root node + make viewport node Display::Grid #9637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d086324
Remove implicit root nodes from bevy_ui
nicoburns Aug 29, 2023
16501f2
Fix a bunch of examples
nicoburns Aug 30, 2023
196d539
Reinstate implicit viewport nodes, except:
nicoburns Sep 15, 2023
1a0986e
Make borders example render identically to before
nicoburns Sep 15, 2023
bb6284e
Review feedback
nicoburns Sep 18, 2023
09490e4
Dummy commit to retry spuriously failing CI job
nicoburns Sep 18, 2023
a5f92cd
Dummy commit to retry spuriously failing CI job
nicoburns Sep 18, 2023
7b742eb
Fix multiple_windows example
nicoburns Sep 18, 2023
3c91bdb
Merge branch 'main' into no-implicit-ui-root-node
nicoburns Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions crates/bevy_ui/src/layout/debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,19 @@ pub fn print_ui_layout_tree(ui_surface: &UiSurface) {
.iter()
.map(|(entity, node)| (*node, *entity))
.collect();
for (&entity, &node) in &ui_surface.window_nodes {
for (&entity, roots) in &ui_surface.window_roots {
let mut out = String::new();
print_node(
ui_surface,
&taffy_to_entity,
entity,
node,
false,
String::new(),
&mut out,
);
for root in roots {
print_node(
ui_surface,
&taffy_to_entity,
entity,
root.implicit_viewport_node,
false,
String::new(),
&mut out,
);
}
bevy_log::info!("Layout tree for window entity: {entity:?}\n{out}");
}
}
Expand Down
111 changes: 65 additions & 46 deletions crates/bevy_ui/src/layout/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ use bevy_hierarchy::{Children, Parent};
use bevy_log::warn;
use bevy_math::Vec2;
use bevy_transform::components::Transform;
use bevy_utils::HashMap;
use bevy_utils::{default, HashMap};
use bevy_window::{PrimaryWindow, Window, WindowResolution, WindowScaleFactorChanged};
use std::fmt;
use taffy::{prelude::Size, style_helpers::TaffyMaxContent, Taffy};
use taffy::Taffy;

pub struct LayoutContext {
pub scale_factor: f64,
Expand All @@ -39,10 +39,18 @@ impl LayoutContext {
}
}

#[derive(Debug, Clone, PartialEq, Eq)]
struct RootNodePair {
// The implicit "viewport" node created by Bevy
implicit_viewport_node: taffy::node::Node,
// The root (parentless) node specified by the user
user_root_node: taffy::node::Node,
}

#[derive(Resource)]
pub struct UiSurface {
entity_to_taffy: HashMap<Entity, taffy::node::Node>,
window_nodes: HashMap<Entity, taffy::node::Node>,
window_roots: HashMap<Entity, Vec<RootNodePair>>,
taffy: Taffy,
}

Expand All @@ -57,7 +65,7 @@ impl fmt::Debug for UiSurface {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.debug_struct("UiSurface")
.field("entity_to_taffy", &self.entity_to_taffy)
.field("window_nodes", &self.window_nodes)
.field("window_nodes", &self.window_roots)
.finish()
}
}
Expand All @@ -68,7 +76,7 @@ impl Default for UiSurface {
taffy.disable_rounding();
Self {
entity_to_taffy: Default::default(),
window_nodes: Default::default(),
window_roots: Default::default(),
taffy,
}
}
Expand Down Expand Up @@ -132,50 +140,64 @@ without UI components as a child of an entity with UI components, results may be
}
}

/// Retrieve or insert the root layout node and update its size to match the size of the window.
pub fn update_window(&mut self, window: Entity, window_resolution: &WindowResolution) {
let taffy = &mut self.taffy;
let node = self
.window_nodes
.entry(window)
.or_insert_with(|| taffy.new_leaf(taffy::style::Style::default()).unwrap());

taffy
.set_style(
*node,
taffy::style::Style {
size: taffy::geometry::Size {
width: taffy::style::Dimension::Points(
window_resolution.physical_width() as f32
),
height: taffy::style::Dimension::Points(
window_resolution.physical_height() as f32,
),
},
..Default::default()
},
)
.unwrap();
}

/// Set the ui node entities without a [`Parent`] as children to the root node in the taffy layout.
pub fn set_window_children(
&mut self,
parent_window: Entity,
window_id: Entity,
children: impl Iterator<Item = Entity>,
) {
let taffy_node = self.window_nodes.get(&parent_window).unwrap();
let child_nodes = children
.map(|e| *self.entity_to_taffy.get(&e).unwrap())
.collect::<Vec<taffy::node::Node>>();
self.taffy.set_children(*taffy_node, &child_nodes).unwrap();
let viewport_style = taffy::style::Style {
display: taffy::style::Display::Grid,
// Note: Taffy percentages are floats ranging from 0.0 to 1.0.
// So this is setting width:100% and height:100%
size: taffy::geometry::Size {
nicoburns marked this conversation as resolved.
Show resolved Hide resolved
width: taffy::style::Dimension::Percent(1.0),
height: taffy::style::Dimension::Percent(1.0),
},
align_items: Some(taffy::style::AlignItems::Start),
justify_items: Some(taffy::style::JustifyItems::Start),
..default()
};

let existing_roots = self.window_roots.entry(window_id).or_default();
let mut new_roots = Vec::new();
for entity in children {
let node = *self.entity_to_taffy.get(&entity).unwrap();
let root_node = existing_roots
.iter()
.find(|n| n.user_root_node == node)
.cloned()
.unwrap_or_else(|| RootNodePair {
implicit_viewport_node: self
.taffy
.new_with_children(viewport_style.clone(), &[node])
.unwrap(),
user_root_node: node,
});
new_roots.push(root_node);
}

// Cleanup the implicit root nodes of any user root nodes that have been removed
for old_root in existing_roots {
if !new_roots.contains(old_root) {
self.taffy.remove(old_root.implicit_viewport_node).unwrap();
}
}

nicoburns marked this conversation as resolved.
Show resolved Hide resolved
self.window_roots.insert(window_id, new_roots);
}

/// Compute the layout for each window entity's corresponding root node in the layout.
pub fn compute_window_layouts(&mut self) {
for window_node in self.window_nodes.values() {
pub fn compute_window_layout(&mut self, window: Entity, window_resolution: &WindowResolution) {
let available_space = taffy::geometry::Size {
width: taffy::style::AvailableSpace::Definite(window_resolution.physical_width() as f32),
height: taffy::style::AvailableSpace::Definite(
window_resolution.physical_height() as f32
),
};
for root_nodes in self.window_roots.entry(window).or_default() {
self.taffy
.compute_layout(*window_node, Size::MAX_CONTENT)
.compute_layout(root_nodes.implicit_viewport_node, available_space)
nicoburns marked this conversation as resolved.
Show resolved Hide resolved
.unwrap();
}
}
Expand Down Expand Up @@ -251,11 +273,6 @@ pub fn ui_layout_system(
.read()
.any(|resized_window| resized_window.window == primary_window_entity);

// update window root nodes
for (entity, window) in windows.iter() {
ui_surface.update_window(entity, &window.resolution);
}

let scale_factor = logical_to_physical_factor * ui_scale.0;

let layout_context = LayoutContext::new(scale_factor, physical_size);
Expand Down Expand Up @@ -302,7 +319,9 @@ pub fn ui_layout_system(
}

// compute layouts
ui_surface.compute_window_layouts();
for (entity, window) in windows.iter() {
ui_surface.compute_window_layout(entity, &window.resolution);
}

let inverse_target_scale_factor = 1. / scale_factor;

Expand Down
1 change: 1 addition & 0 deletions examples/ecs/apply_deferred.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ fn setup(mut commands: Commands) {
.spawn(NodeBundle {
style: Style {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
flex_direction: FlexDirection::Column,
Expand Down
1 change: 1 addition & 0 deletions examples/ecs/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ fn setup_menu(mut commands: Commands) {
style: Style {
// center button
width: Val::Percent(100.),
height: Val::Percent(100.),
justify_content: JustifyContent::Center,
align_items: AlignItems::Center,
..default()
Expand Down
6 changes: 6 additions & 0 deletions examples/games/game_menu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ mod splash {
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
width: Val::Percent(100.0),
height: Val::Percent(100.0),
..default()
},
..default()
Expand Down Expand Up @@ -151,6 +152,7 @@ mod game {
NodeBundle {
style: Style {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
Copy link
Contributor

@ickshonpe ickshonpe Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The margins no longer display correctly for the new game section here.
Not sure what is wrong? Might not be something introduced by this PR but an already existing bug. I seem to remember someone describing a similar issue on discord but they didn't provide a code sample, only a screenshot, so not sure if it is related.

Copy link
Contributor Author

@nicoburns nicoburns Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, that's bizarre. I don't understand what is going on here. Not only is the black node taller in main, but the second line of text is rendered higher up the screen!

Main:

Screenshot 2023-09-15 at 23 12 33

This PR:

Screenshot 2023-09-15 at 23 12 04

Copy link
Contributor Author

@nicoburns nicoburns Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@ickshonpe I have a feeling this may have to do with non-deterministic text measurement. Because by removing the root node (and putting in some borders for debugging the layout), I was able to make it do this:

Screenshot 2023-09-16 at 00 10 15

Which it definitely shouldn't do. I suspect the bug we are seeing in the actual game_menu example is the text measurement returning a single-line height for preliminary measurements. And then a 2-line height we re-measured with the dimensions returned from the first measurement. FYI, cosmic-text recently fixed a very similar issue pop-os/cosmic-text#175.

Code for above screenshot:

//! This example will display a simple menu using Bevy UI where you can start a new game,
//! change some settings or quit. There is no actual game, it will just display the current
//! settings for 5 seconds before going back to the menu.

use bevy::prelude::*;

const TEXT_COLOR: Color = Color::rgb(0.9, 0.9, 0.9);

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

fn setup(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());
    // commands
    //     .spawn(
    //         NodeBundle {
    //             style: Style {
    //                 width: Val::Percent(100.0),
    //                 height: Val::Percent(100.0),
    //                 // center children`
    //                 align_items: AlignItems::Center,
    //                 justify_content: JustifyContent::Center,
    //                 border: UiRect::all(Val::Px(4.0)),
    //                 ..default()
    //             },
    //             border_color: Color::RED.into(),
    //             ..default()
    //         },
    //     )
    //     .with_children(|parent| {
    // First create a `NodeBundle` for centering what we want to display
    commands
        .spawn(NodeBundle {
            style: Style {
                // This will display its children in a column, from top to bottom
                flex_direction: FlexDirection::Column,
                // `align_items` will align children on the cross axis. Here the main axis is
                // vertical (column), so the cross axis is horizontal. This will center the
                // children
                align_items: AlignItems::Center,
                // margin: UiRect::all(Val::Auto),
                border: UiRect::all(Val::Px(4.0)),
                ..default()
            },
            border_color: Color::BLUE.into(),
            background_color: Color::BLACK.into(),
            ..default()
        })
        .with_children(|parent| {
            // Display two lines of text, the second one with the current settings
            parent
                .spawn(NodeBundle {
                    style: Style {
                        align_items: AlignItems::Start,
                        justify_content: JustifyContent::Start,
                        border: UiRect::all(Val::Px(4.0)),
                        margin: UiRect::all(Val::Px(50.0)),
                        ..default()
                    },
                    border_color: Color::RED.into(),
                    background_color: Color::GRAY.into(),
                    ..default()
                })
                .with_children(|parent| {
                    parent.spawn(
                        TextBundle::from_section(
                            "Will be back to the menu shortly...",
                            TextStyle {
                                font_size: 80.0,
                                color: TEXT_COLOR,
                                ..default()
                            },
                        )
                    );
                });

            parent
                .spawn(NodeBundle {
                    style: Style {
                        align_items: AlignItems::Start,
                        justify_content: JustifyContent::Start,
                        border: UiRect::all(Val::Px(4.0)),
                        margin: UiRect::all(Val::Px(50.0)),
                        ..default()
                    },
                    border_color: Color::RED.into(),
                    background_color: Color::GRAY.into(),
                    ..default()
                })
                .with_children(|parent| {
                    parent.spawn(
                        TextBundle::from_sections([
                            TextSection::new(
                                "quality: Medium",
                                TextStyle {
                                    font_size: 60.0,
                                    color: Color::BLUE,
                                    ..default()
                                },
                            ),
                            TextSection::new(
                                " - ",
                                TextStyle {
                                    font_size: 60.0,
                                    color: TEXT_COLOR,
                                    ..default()
                                },
                            ),
                            TextSection::new(
                                "volume: Volume(7)",
                                TextStyle {
                                    font_size: 60.0,
                                    color: Color::GREEN,
                                    ..default()
                                },
                            ),
                        ])
                    );
                });
        });
    // });
}

// center children
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
Expand Down Expand Up @@ -421,6 +423,7 @@ mod menu {
NodeBundle {
style: Style {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
..default()
Expand Down Expand Up @@ -546,6 +549,7 @@ mod menu {
NodeBundle {
style: Style {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
..default()
Expand Down Expand Up @@ -611,6 +615,7 @@ mod menu {
NodeBundle {
style: Style {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
..default()
Expand Down Expand Up @@ -714,6 +719,7 @@ mod menu {
NodeBundle {
style: Style {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
..default()
Expand Down
1 change: 1 addition & 0 deletions examples/stress_tests/many_buttons.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ fn setup_flex(mut commands: Commands, asset_server: Res<AssetServer>, args: Res<
justify_content: JustifyContent::Center,
align_items: AlignItems::Center,
width: Val::Percent(100.),
height: Val::Percent(100.),
..default()
},
..default()
Expand Down
2 changes: 1 addition & 1 deletion examples/stress_tests/many_glyphs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ fn setup(mut commands: Commands) {
commands
.spawn(NodeBundle {
style: Style {
flex_basis: Val::Percent(100.),
width: Val::Percent(100.),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
..default()
Expand Down
3 changes: 2 additions & 1 deletion examples/ui/borders.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ fn setup(mut commands: Commands) {
let root = commands
.spawn(NodeBundle {
style: Style {
flex_basis: Val::Percent(100.0),
margin: UiRect::all(Val::Px(25.0)),
align_self: AlignSelf::Stretch,
justify_self: JustifySelf::Stretch,
flex_wrap: FlexWrap::Wrap,
justify_content: JustifyContent::FlexStart,
align_items: AlignItems::FlexStart,
Expand Down
1 change: 1 addition & 0 deletions examples/ui/button.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
.spawn(NodeBundle {
style: Style {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
..default()
Expand Down
6 changes: 2 additions & 4 deletions examples/ui/display_and_visibility.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
commands.spawn(Camera2dBundle::default());
commands.spawn(NodeBundle {
style: Style {
width: Val::Percent(100.),
height: Val::Percent(100.),
flex_direction: FlexDirection::Column,
flex_basis: Val::Percent(100.),
align_items: AlignItems::Center,
justify_content: JustifyContent::SpaceEvenly,
..Default::default()
Expand Down Expand Up @@ -189,9 +190,6 @@ fn spawn_left_panel(builder: &mut ChildBuilder, palette: &[Color; 4]) -> Vec<Ent
.with_children(|parent| {
parent
.spawn(NodeBundle {
style: Style {
..Default::default()
},
background_color: BackgroundColor(Color::BLACK),
..Default::default()
})
Expand Down
3 changes: 2 additions & 1 deletion examples/ui/overflow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
commands
.spawn(NodeBundle {
style: Style {
width: Val::Percent(100.),
height: Val::Percent(100.),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
width: Val::Percent(100.),
..Default::default()
},
background_color: Color::ANTIQUE_WHITE.into(),
Expand Down
1 change: 1 addition & 0 deletions examples/ui/relative_cursor_position.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
.spawn(NodeBundle {
style: Style {
width: Val::Percent(100.),
height: Val::Percent(100.0),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
flex_direction: FlexDirection::Column,
Expand Down
3 changes: 2 additions & 1 deletion examples/ui/size_constraints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
commands
.spawn(NodeBundle {
style: Style {
flex_basis: Val::Percent(100.0),
width: Val::Percent(100.0),
height: Val::Percent(100.0),
justify_content: JustifyContent::Center,
align_items: AlignItems::Center,
..Default::default()
Expand Down
3 changes: 2 additions & 1 deletion examples/ui/text_wrap_debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ fn spawn(mut commands: Commands, asset_server: Res<AssetServer>) {
let root = commands
.spawn(NodeBundle {
style: Style {
flex_direction: FlexDirection::Column,
width: Val::Percent(100.),
height: Val::Percent(100.),
flex_direction: FlexDirection::Column,
..Default::default()
},
background_color: Color::BLACK.into(),
Expand Down
1 change: 1 addition & 0 deletions examples/ui/transparency_ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
.spawn(NodeBundle {
style: Style {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
align_items: AlignItems::Center,
justify_content: JustifyContent::SpaceAround,
..default()
Expand Down
1 change: 1 addition & 0 deletions examples/ui/ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
.spawn(NodeBundle {
style: Style {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
justify_content: JustifyContent::SpaceBetween,
..default()
},
Expand Down
3 changes: 2 additions & 1 deletion examples/ui/ui_texture_atlas.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ fn setup(
commands
.spawn(NodeBundle {
style: Style {
flex_direction: FlexDirection::Column,
width: Val::Percent(100.0),
height: Val::Percent(100.0),
flex_direction: FlexDirection::Column,
justify_content: JustifyContent::Center,
align_items: AlignItems::Center,
row_gap: Val::Px(text_style.font_size * 2.),
Expand Down
Loading