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

Fix UI borders #10078

Merged
merged 1 commit into from
Oct 13, 2023
Merged

Fix UI borders #10078

merged 1 commit into from
Oct 13, 2023

Conversation

rparrett
Copy link
Contributor

@rparrett rparrett commented Oct 10, 2023

Objective

Fixes #10069

Solution

Extracted UI nodes were previously stored in a SparseSet and had a predictable iteration order. UI borders and outlines relied on this. Now they are stored in a HashMap and that is no longer true.

This adds entity.index() to the sort key for TransparentUi so that the iteration order is predictable and the "border entities" that get spawned during extraction are guaranteed to get drawn after their respective container nodes again.

I think that everything still works for overlapping ui nodes etc, because the z value / primary sort is still controlled by the "ui stack."

Text above is just my current understanding. A rendering expert should check this out.

I will do some more testing when I can.

@rparrett rparrett added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets labels Oct 10, 2023
@alice-i-cecile alice-i-cecile added this to the 0.12 milestone Oct 10, 2023
@rparrett rparrett changed the title Fix borders Fix UI borders and outlines Oct 10, 2023
@rparrett rparrett changed the title Fix UI borders and outlines Fix UI borders Oct 10, 2023
Copy link
Member

@mockersf mockersf left a comment

Choose a reason for hiding this comment

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

This should work.

I think we could do smaller than adding an u32 to the sort key, but not sure it's worth it. System extract_atlas_uinodes could extract nodes with a UiNodeType::Content, extract_uinode_borders with UiNodeType::Border, extract_uinode_outlines with UiNodeType::Outline and then this enum could be ordered

@superdump superdump added this pull request to the merge queue Oct 13, 2023
Merged via the queue into bevyengine:main with commit 26ecfcf Oct 13, 2023
21 checks passed
ameknite pushed a commit to ameknite/bevy that referenced this pull request Nov 6, 2023
# Objective

Fixes bevyengine#10069

## Solution

Extracted UI nodes were previously stored in a `SparseSet` and had a
predictable iteration order. UI borders and outlines relied on this. Now
they are stored in a HashMap and that is no longer true.

This adds `entity.index()` to the sort key for `TransparentUi` so that
the iteration order is predictable and the "border entities" that get
spawned during extraction are guaranteed to get drawn after their
respective container nodes again.

I **think** that everything still works for overlapping ui nodes etc,
because the z value / primary sort is still controlled by the "ui
stack."

Text above is just my current understanding. A rendering expert should
check this out.

I will do some more testing when I can.
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

Fixes bevyengine#10069

## Solution

Extracted UI nodes were previously stored in a `SparseSet` and had a
predictable iteration order. UI borders and outlines relied on this. Now
they are stored in a HashMap and that is no longer true.

This adds `entity.index()` to the sort key for `TransparentUi` so that
the iteration order is predictable and the "border entities" that get
spawned during extraction are guaranteed to get drawn after their
respective container nodes again.

I **think** that everything still works for overlapping ui nodes etc,
because the z value / primary sort is still controlled by the "ui
stack."

Text above is just my current understanding. A rendering expert should
check this out.

I will do some more testing when I can.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI borders "randomly" do not render
4 participants