-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[Merged by Bors] - Add z-index support with a predictable UI stack #5877
Conversation
The local and global z-index solution in this PR is an attempt to bring flexibility without adding too much complexity. Local z-index allows a node to change its depth compared to its siblings regardless of the order they were added to their parent, while the Global z-index allows nodes to "escape" the parent and be ordered relative to the entire UI. Those two use-cases are the most common in my opinion, but I can see some benefits in other more complex solutions. For example, having the |
The sentence cut off. |
a48628a
to
321de09
Compare
c5b64e1
to
12cc199
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this PR. I think it not only provides a relatively simple and intuitive solution to a design limitation which currently blocks entire classes of use cases, but it's also a step in what I'd argue is the right direction for UI hierarchies:
- support for multiple UI roots, which this PR helps greatly with its
StackingContext
- getting rid of the use of
Transform
in UI hierarchies, which unlike other use cases is for UI not user controlled but auto-generated (the source of truth isStyle
for UI), which makes it very unintuitive for users. Unity has the same approach; for UI their Transform is replaced with a RectTransform.
@alice-i-cecile what are the arguments for making this PR as controversial? Can we link a discussion or post some rationale?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, while z-index ordering might not be perfect for all cases, it's definitely a good basis to build on (and familiar to a lot of people).
I think this system works great with how Bevy UI works at the moment, because it assumes one window and viewport. However, the UiStack will need to change slightly after or before #5892 gets merged. The UiStack struct will be even more useful if it contains one For example, the UI interaction system will be able to filter on the root's target information to only update UI that's getting rendered on the current active window. This will also be useful during UI rendering when multiple render targets are supported. |
I think that this PR could be improved by
|
The included example serves as a good demo, but putting myself in the shoes of a user who is trying to learn how |
This was what motivated me to add the Controversial label. So long as we can avoid the problems there, I'm happy to move forward on this. I think we have consensus that this needs to happen, and this seems like a good step forward. Much of the related camera-driven rendering + hierarchy mess that plagued #1211 has since been dramatically improved. |
I made a summary of some important points from that PR and how they're improved upon with this one.
The UiStack concept helps with this since it's a sorted vector built from the hierarchies (and the z-index escape hatch). You can only iterate over it in one way or the other depending on what you need to do.
The global and local options helps giving some flexibility here and the result is somewhat similar to CSS with less rules and exceptions.
There's nothing root-centric in this PR, so that helps. As for the
The z precision issues discussed on there are gone with this solution because we don't move ui nodes on the z axis anymore.
This PR doesn't change anything in that regard. My PR also doesn't address the fact that multiple root nodes share the same internal taffy root and will share screen space. However, I have a separate PR #5892 which does. |
I just realized that adding |
7b439e4
to
2d2d335
Compare
bors try |
@alice-i-cecile I would still call this controversial as this is a pretty foundational piece of UI that will be hard to change once people start depending on its behaviors. I'd like to give this a look, but I also don't want to hold back progress. Lets "start the merge clock" on this, but with the agreement that it can be merged for Bevy 0.9 if I don't get to it in time (so slightly accelerated). |
Sounds good! |
tryMerge conflict. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm on board for this! The sorting algorithm is usable and roughly in line with the web spec. There may be potential here for optimization in various places, but I'm cool with moving forward on this as-is (once merge conflicts are resolved).
crates/bevy_ui/src/stack.rs
Outdated
} | ||
|
||
*ui_stack = UiStack { | ||
uinodes: Vec::<Entity>::with_capacity(total_entry_count), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should consider reusing the vector here to avoid allocations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just made changes to do that. Would just need a confirmation that it makes sense as-is.
extracted_uinodes | ||
.uinodes | ||
.sort_by(|a, b| FloatOrd(a.transform.w_axis[2]).cmp(&FloatOrd(b.transform.w_axis[2]))); | ||
.sort_by_key(|node| node.stack_index); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its a bit unfortunate that we already have an absolute order in UiStack, which we're then re-establishing here in uinodes. The sort is redundant. That being said, if we want a "distributed" extract (ex: multiple systems with node-type-specific extraction logic), then the only real alternative is extracting nodes as entities (instead of pushing them onto an unsorted ExtractedUiNodes list), then using an extracted UiStack value to query/iterate over them in prepare_uinodes
. The extra cost of "per-entity extraction" might outweigh the time saved on sorting.
Worth measuring, but not something we need to block on here, especially given that this is following the pre-existing pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge looks correct.
3b27b67
to
32766b3
Compare
@alice-i-cecile @oceantume does this address #1275? |
IMO no, not fully. It doesn't work for 2D, and doesn't allow a direct and automatic translation from an ordered enum. |
Just calling out that the recent merge broke this code / it needs fixing. |
bors r+ |
# Objective Add consistent UI rendering and interaction where deep nodes inside two different hierarchies will never render on top of one-another by default and offer an escape hatch (z-index) for nodes to change their depth. ## The problem with current implementation The current implementation of UI rendering is broken in that regard, mainly because [it sets the Z value of the `Transform` component based on a "global Z" space](https://github.com/bevyengine/bevy/blob/main/crates/bevy_ui/src/update.rs#L43) shared by all nodes in the UI. This doesn't account for the fact that each node's final `GlobalTransform` value will be relative to its parent. This effectively makes the depth unpredictable when two deep trees are rendered on top of one-another. At the moment, it's also up to each part of the UI code to sort all of the UI nodes. The solution that's offered here does the full sorting of UI node entities once and offers the result through a resource so that all systems can use it. ## Solution ### New ZIndex component This adds a new optional `ZIndex` enum component for nodes which offers two mechanism: - `ZIndex::Local(i32)`: Overrides the depth of the node relative to its siblings. - `ZIndex::Global(i32)`: Overrides the depth of the node relative to the UI root. This basically allows any node in the tree to "escape" the parent and be ordered relative to the entire UI. Note that in the current implementation, omitting `ZIndex` on a node has the same result as adding `ZIndex::Local(0)`. Additionally, the "global" stacking context is essentially a way to add your node to the root stacking context, so using `ZIndex::Local(n)` on a root node (one without parent) will share that space with all nodes using `Index::Global(n)`. ### New UiStack resource This adds a new `UiStack` resource which is calculated from both hierarchy and `ZIndex` during UI update and contains a vector of all node entities in the UI, ordered by depth (from farthest from camera to closest). This is exposed publicly by the bevy_ui crate with the hope that it can be used for consistent ordering and to reduce the amount of sorting that needs to be done by UI systems (i.e. instead of sorting everything by `global_transform.z` in every system, this array can be iterated over). ### New z_index example This also adds a new z_index example that showcases the new `ZIndex` component. It's also a good general demo of the new UI stack system, because making this kind of UI was very broken with the old system (e.g. nodes would render on top of each other, not respecting hierarchy or insert order at all). ![image](https://user-images.githubusercontent.com/1060971/189015985-8ea8f989-0e9d-4601-a7e0-4a27a43a53f9.png) --- ## Changelog - Added the `ZIndex` component to bevy_ui. - Added the `UiStack` resource to bevy_ui, and added implementation in a new `stack.rs` module. - Removed the previous Z updating system from bevy_ui, because it was replaced with the above. - Changed bevy_ui rendering to use UiStack instead of z ordering. - Changed bevy_ui focus/interaction system to use UiStack instead of z ordering. - Added a new z_index example. ## ZIndex demo Here's a demo I wrote to test these features https://user-images.githubusercontent.com/1060971/188329295-d7beebd6-9aee-43ab-821e-d437df5dbe8a.mp4 Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Thanks for fixing it @cart. I was hoping to look into this today, but didn't find the time. |
# Objective Add consistent UI rendering and interaction where deep nodes inside two different hierarchies will never render on top of one-another by default and offer an escape hatch (z-index) for nodes to change their depth. ## The problem with current implementation The current implementation of UI rendering is broken in that regard, mainly because [it sets the Z value of the `Transform` component based on a "global Z" space](https://github.com/bevyengine/bevy/blob/main/crates/bevy_ui/src/update.rs#L43) shared by all nodes in the UI. This doesn't account for the fact that each node's final `GlobalTransform` value will be relative to its parent. This effectively makes the depth unpredictable when two deep trees are rendered on top of one-another. At the moment, it's also up to each part of the UI code to sort all of the UI nodes. The solution that's offered here does the full sorting of UI node entities once and offers the result through a resource so that all systems can use it. ## Solution ### New ZIndex component This adds a new optional `ZIndex` enum component for nodes which offers two mechanism: - `ZIndex::Local(i32)`: Overrides the depth of the node relative to its siblings. - `ZIndex::Global(i32)`: Overrides the depth of the node relative to the UI root. This basically allows any node in the tree to "escape" the parent and be ordered relative to the entire UI. Note that in the current implementation, omitting `ZIndex` on a node has the same result as adding `ZIndex::Local(0)`. Additionally, the "global" stacking context is essentially a way to add your node to the root stacking context, so using `ZIndex::Local(n)` on a root node (one without parent) will share that space with all nodes using `Index::Global(n)`. ### New UiStack resource This adds a new `UiStack` resource which is calculated from both hierarchy and `ZIndex` during UI update and contains a vector of all node entities in the UI, ordered by depth (from farthest from camera to closest). This is exposed publicly by the bevy_ui crate with the hope that it can be used for consistent ordering and to reduce the amount of sorting that needs to be done by UI systems (i.e. instead of sorting everything by `global_transform.z` in every system, this array can be iterated over). ### New z_index example This also adds a new z_index example that showcases the new `ZIndex` component. It's also a good general demo of the new UI stack system, because making this kind of UI was very broken with the old system (e.g. nodes would render on top of each other, not respecting hierarchy or insert order at all). ![image](https://user-images.githubusercontent.com/1060971/189015985-8ea8f989-0e9d-4601-a7e0-4a27a43a53f9.png) --- ## Changelog - Added the `ZIndex` component to bevy_ui. - Added the `UiStack` resource to bevy_ui, and added implementation in a new `stack.rs` module. - Removed the previous Z updating system from bevy_ui, because it was replaced with the above. - Changed bevy_ui rendering to use UiStack instead of z ordering. - Changed bevy_ui focus/interaction system to use UiStack instead of z ordering. - Added a new z_index example. ## ZIndex demo Here's a demo I wrote to test these features https://user-images.githubusercontent.com/1060971/188329295-d7beebd6-9aee-43ab-821e-d437df5dbe8a.mp4 Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Objective
Add consistent UI rendering and interaction where deep nodes inside two different hierarchies will never render on top of one-another by default and offer an escape hatch (z-index) for nodes to change their depth.
The problem with current implementation
The current implementation of UI rendering is broken in that regard, mainly because it sets the Z value of the
Transform
component based on a "global Z" space shared by all nodes in the UI. This doesn't account for the fact that each node's finalGlobalTransform
value will be relative to its parent. This effectively makes the depth unpredictable when two deep trees are rendered on top of one-another.At the moment, it's also up to each part of the UI code to sort all of the UI nodes. The solution that's offered here does the full sorting of UI node entities once and offers the result through a resource so that all systems can use it.
Solution
New ZIndex component
This adds a new optional
ZIndex
enum component for nodes which offers two mechanism:ZIndex::Local(i32)
: Overrides the depth of the node relative to its siblings.ZIndex::Global(i32)
: Overrides the depth of the node relative to the UI root. This basically allows any node in the tree to "escape" the parent and be ordered relative to the entire UI.Note that in the current implementation, omitting
ZIndex
on a node has the same result as addingZIndex::Local(0)
. Additionally, the "global" stacking context is essentially a way to add your node to the root stacking context, so usingZIndex::Local(n)
on a root node (one without parent) will share that space with all nodes usingIndex::Global(n)
.New UiStack resource
This adds a new
UiStack
resource which is calculated from both hierarchy andZIndex
during UI update and contains a vector of all node entities in the UI, ordered by depth (from farthest from camera to closest). This is exposed publicly by the bevy_ui crate with the hope that it can be used for consistent ordering and to reduce the amount of sorting that needs to be done by UI systems (i.e. instead of sorting everything byglobal_transform.z
in every system, this array can be iterated over).New z_index example
This also adds a new z_index example that showcases the new
ZIndex
component. It's also a good general demo of the new UI stack system, because making this kind of UI was very broken with the old system (e.g. nodes would render on top of each other, not respecting hierarchy or insert order at all).Changelog
ZIndex
component to bevy_ui.UiStack
resource to bevy_ui, and added implementation in a newstack.rs
module.ZIndex demo
Here's a demo I wrote to test these features
https://user-images.githubusercontent.com/1060971/188329295-d7beebd6-9aee-43ab-821e-d437df5dbe8a.mp4