Allow UI layout updates without a window by ickshonpe · Pull Request #8672 · bevyengine/bevy

ickshonpe · 2023-05-24T20:10:32Z

Objective

Allow UI layout updates without a window present.

Solution

Make LayoutContext into a component that is used to mark an entity as a UI root entity with an associated Taffy root node and
add a LayoutContext component to the primary window entity in UiPlugin::build.

ui_layout_system now picks the first entity containing a LayoutContext component and introduces a corresponding taffy node to the Taffy layout tree (given that it isn't present already). This node is then set as the root of the entire tree. Then the UI layout update proceeds as normal.

Changelog

changes:

LayoutContext derives Component. It has a new field layout_to_logical_factor, scale_factor is renamed to combined_scale_factor, physical_size is renamed to root_node_size and the min_size and max_size are removed.
UiPlugin::build adds a LayoutContext component to the primary window entity.
For the first LayoutContext entity, an associated Taffy node is added to the Taffy layout tree, which is then set as the root of the entire tree.
Added a simple test that adds and then removes a UI node.

Migration Guide

LayoutContext is now a component and its fields have been changed:

min and max have been removed
scale_factor has been renamed to combined_scale_factor
layout_to_logical_factor is a new field. The coordinates used in the internal UI layout tree are multiplied by layout_to_logical_factor after each UI update before being written to the Node and Transform components.

* Added `physical_to_logical_factor` field to `LayoutContext`. * Created `ui_windows_system` to track rack's window and scale factor changes and set the `LayoutContext`. * Removed window queries and event reader params from `ui_layout_system`, it only queries for a `LayoutContext` and the UI tree data now.

…` import in `bevy_ui::node_bundles`.

…ribute-for-TextFlags-import' into ui-windows-system

alice-i-cecile · 2023-05-24T22:11:45Z

crates/bevy_ui/src/layout/mod.rs

 use taffy::{prelude::Size, style_helpers::TaffyMaxContent, Taffy};

+#[derive(Component, Debug, Copy, Clone)]
 pub struct LayoutContext {


Basic doc strings while we're here please.

Added doc comments, they might need some more work. I changed the names of the fields as well.
I'm not sure about the name LayoutContext either but I guess it's okay for now.

alice-i-cecile · 2023-05-24T22:15:06Z

crates/bevy_ui/src/lib.rs

-                ui_focus_system.in_set(UiSystem::Focus).after(InputSystem),
+                (
+                    ui_focus_system.in_set(UiSystem::Focus).after(InputSystem),
+                    ui_windows_system,


Suggested change

ui_windows_system,

ui_windows_system.in_set(UiSystem::Window),

I'm not sure if there are other missing ordering constraints, but this is definitely implied by the doc strings above.

Ah yeah, I wasn't sure whether to add a label to UiSet or not. ui_window_system runs in PreUpdate so there shouldn't be any need for ordering constraints.

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

* Renamed the fields of `LayoutContext` to match their purpose and added doc comments. * Added more comments to `ui_layout_system`. * Created a new module `window` and moved `ui_windows_system` to it.

djeedai · 2023-05-27T07:12:37Z

Make ui_layout_system easier to test by moving the window logic into a separate system.

I disagree with that approach. Testing calls for a modular block of code, which means a function. Adding a separate system is a more heavyweight process that adds to the pile of things the scheduler has to do, and should only be done for runtime reasons. Can we move the logic into a function called from the same system it's currently in? That should solve the testability issue without requiring an additional system.

ickshonpe · 2023-05-27T11:10:18Z

Make ui_layout_system easier to test by moving the window logic into a separate system.

I disagree with that approach. Testing calls for a modular block of code, which means a function. Adding a separate system is a more heavyweight process that adds to the pile of things the scheduler has to do, and should only be done for runtime reasons. Can we move the logic into a function called from the same system it's currently in? That should solve the testability issue without requiring an additional system.

Ah, that's a good point. There is no need for a new system just for this change. The situation is a bit muddled and confused because it's not clear yet how multi ui root nodes and multi-window ui support will be implemented. I'll refactor this a bit.

djeedai · 2023-05-27T14:01:45Z

To be fair I'm on a phone so didn't look in details at the PR sorry. Just wanted to raise a general point. If that's too complicated for this case I'm fine using a separate system. It was just a suggestion in general, from experience I at least tend to put everything in a system function, and I think Bevy does that too, and that as you noted makes testing a lot harder. Separate non-system functions are nicer for testing but a pain to pass arguments to unfortunately.

* deleted the `window` module * `ui_layout_system` only runs if an entity with a `LayoutContext` exists. * `UiPlugin::build` adds a `LayoutContext` component to the primary window entity.

…tem` requires it now again.

ickshonpe · 2023-05-28T03:40:28Z

It was really helpful, I was making things over complicated. The implementation is much better now and realised I could easily hack together multiple window UI on top of this as well:

multiple_window_ui.mp4

djeedai

Couple of things I'm not sure about, but fine to get overridden by any UI expert if they feel we can merge as is.

crates/bevy_ui/src/layout/mod.rs

Co-authored-by: Jerome Humbert <djeedai@gmail.com>

alice-i-cecile · 2023-05-30T23:47:41Z

Can you add a simple migration guide for the renames?

ickshonpe · 2023-05-31T10:14:59Z

Can you add a simple migration guide for the renames?

done

I didn't make one before because LayoutContext was added after the Bevy 10 release, do we need them for changes to features new to the next version?

alice-i-cecile · 2023-05-31T14:56:30Z

I didn't make one before because LayoutContext was added after the Bevy 10 release, do we need them for changes to features new to the next version?

Ah I'd missed that. These are nice to include in the PR description still in case folks are working off main or the PR gets delayed. Ideally we'll exclude it from the migration guide notes but a false positive like that does no harm.

ickshonpe added 8 commits May 24, 2023 10:54

Renamed LayoutContext to WindowContext

ba55608

Removed unnecessary as f32 casts

348fa67

Added the #[cfg(feature = "bevy_text")] attribute to the `TextFlags…

5116ac7

…` import in `bevy_ui::node_bundles`.

cargo fmt --all

b64b76f

Added a test for ui_layout_system.

101be11

Merge remote-tracking branch 'origin/dd-missing-bevy_text_feature_att…

8c3bac2

…ribute-for-TextFlags-import' into ui-windows-system

comments

1ef4725

alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Code-Quality A section of code that is hard to understand or change labels May 24, 2023

alice-i-cecile reviewed May 24, 2023

View reviewed changes

ickshonpe and others added 2 commits May 25, 2023 00:28

Update crates/bevy_ui/src/lib.rs

486c456

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

changes:

2cd2975

* Renamed the fields of `LayoutContext` to match their purpose and added doc comments. * Added more comments to `ui_layout_system`. * Created a new module `window` and moved `ui_windows_system` to it.

ickshonpe changed the title ~~Make ui_layout_system easier to test by moving the window logic into a separate system.~~ Move UI window logic into a separate system May 25, 2023

ickshonpe added 4 commits May 25, 2023 13:54

Fixed some mistakes in doc comments.

1bbc048

Fixed some broken intra-doc links

22ebf1d

More comments

30cfa06

added back tick quotes to a comment

72ca0e9

ickshonpe added 2 commits May 27, 2023 15:18

changes:

d897981

* deleted the `window` module * `ui_layout_system` only runs if an entity with a `LayoutContext` exists. * `UiPlugin::build` adds a `LayoutContext` component to the primary window entity.

Delete the Taffy root node from root_nodes not entity_to_taffy.

54b55fb

ickshonpe changed the title ~~Move UI window logic into a separate system~~ Allow UI layout updates without a window May 27, 2023

alice-i-cecile approved these changes May 27, 2023

View reviewed changes

Added an init_resource for UiScale to the test, as `ui_layout_sys…

97506b6

…tem` requires it now again.

djeedai suggested changes May 28, 2023

View reviewed changes

Update crates/bevy_ui/src/layout/mod.rs

5e36283

Co-authored-by: Jerome Humbert <djeedai@gmail.com>

ickshonpe and others added 6 commits May 28, 2023 11:15

replaced LayoutContext's ne() function with a PartialEq derive.

a6b2d79

Replaced Default impl for LayoutContext with a derive.

4cc681a

Update crates/bevy_ui/src/layout/mod.rs

a390faa

Co-authored-by: Jerome Humbert <djeedai@gmail.com>

Clarified LayoutContext doc comments.

ec6bf90

cargo fmt

76855f0

fixed doc comment

0bdd9e9

alice-i-cecile added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label May 30, 2023

ickshonpe added 2 commits May 31, 2023 18:25

Removed debug layout printout from test.

1780625

Removed unused import

64c5829

ickshonpe mentioned this pull request Aug 29, 2023

Adding just a Node component to an empty entity causes a panic #9615

Closed

	ui_windows_system,
	ui_windows_system.in_set(UiSystem::Window),

Uh oh!

Comments

Conversation

ickshonpe commented May 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Changelog

Migration Guide

Uh oh!

alice-i-cecile May 24, 2023

Choose a reason for hiding this comment

Uh oh!

ickshonpe May 25, 2023

Choose a reason for hiding this comment

Uh oh!

alice-i-cecile May 24, 2023

Choose a reason for hiding this comment

Uh oh!

ickshonpe May 25, 2023

Choose a reason for hiding this comment

Uh oh!

djeedai commented May 27, 2023

Uh oh!

ickshonpe commented May 27, 2023

Uh oh!

djeedai commented May 27, 2023

Uh oh!

ickshonpe commented May 28, 2023

Uh oh!

djeedai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alice-i-cecile commented May 30, 2023

Uh oh!

ickshonpe commented May 31, 2023

Uh oh!

alice-i-cecile commented May 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ickshonpe commented May 24, 2023 •

edited

Loading