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

Give UI nodes with Display::None an empty clipping rect #10942

Merged
merged 3 commits into from Dec 23, 2023

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Dec 11, 2023

Objective

Outlines are drawn for UI nodes with Display::None set and their descendants. They should not be visible.

Solution

Make all Nodes with Display::None inherit an empty clipping rect, ensuring that the outlines are not visible.

Fixes #10940


Changelog

  • In update_clipping_system if a node has Display::None set, clip the entire node and all its descendants by replacing the inherited clip with a default rect (which is empty)

…he entire node and all its descendants by replacing the inherited clip with a default rect (which is empty)
@ickshonpe ickshonpe added A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior labels Dec 12, 2023
@ickshonpe ickshonpe changed the title Clip the outlines of UI nodes with Display::None set Give UI nodes with Display::None an empty clipping rect Dec 12, 2023
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Good comment, thanks!

@nicoburns
Copy link
Contributor

Are there any performance implications to this "rendering and then clipping" as opposed to special casing Display::None eliding rendering entirely?

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Dec 22, 2023

Are there any performance implications to this "rendering and then clipping" as opposed to special casing Display::None eliding rendering entirely?

Basically what this PR does is special case Display::None in the clipping system to ellide rendering (if the intersection of an item's bounding rect and its clipping rect is empty then rendering is skipped).

Normally this is handled automatically because nodes with Display::None are given a zero size in the layout, but in the case of outlines they are drawn outside of the node's bounds so it doesn't help.

@nicoburns nicoburns added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Dec 22, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 23, 2023
Merged via the queue into bevyengine:main with commit efb4fa5 Dec 23, 2023
25 checks passed
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-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Outlines are drawn for nodes with Display::None set and their children
4 participants