use precomputed border values#15163
Merged
alice-i-cecile merged 35 commits intobevyengine:mainfrom Sep 26, 2024
Merged
Conversation
…ourselves in each extraction function which is really complicated and fragile and has lead to a number of bugs. Adds an `Inset` type, a `border: Inset` field to `Node`, updates the border field in `ui_layout_system`, and removes the border calculations and extra queries from the extraction functions.
…der values instead.
ickshonpe
added
A-UI
UkoeHB
suggested changes
Sep 22, 2024
Contributor
UkoeHB
left a comment
UkoeHB
left a comment
There was a problem hiding this comment.
Looking good. Can you update the example to include varied combinations of border width and border radius? Looks like it's all uniform right now.
Contributor
Author
|
Okay this should be ready now. |
akimakinai
reviewed
Sep 25, 2024
Co-authored-by: akimakinai <[email protected]>
UkoeHB
approved these changes
Sep 25, 2024
kristoff3r
approved these changes
Sep 26, 2024
alice-i-cecile
added
S-Ready-For-Final-Review
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 15, 2024
# Objective Change UI clipping to respect borders and padding. Fixes #15335 ## Solution Based on #15163 1. Add a `padding` field to `Node`. 2. In `ui_layout_size` copy the padding values from taffy to `Node::padding`. 4. Determine the node's content box (The innermost part of the node excluding the padding and border). 5. In `update_clipping` perform the clipping intersection with the node's content box. ## Notes * `Rect` probably needs some helper methods for working with insets but because `Rect` and `BorderRect` are in different crates it's awkward to add them. Left for a follow up. * We could have another `Overflow` variant (probably called `Overflow::Hidden`) to that clips inside of the border box instead of the content box. Left it out here as I'm not certain about the naming or behaviour though. If this PR is adopted, it would be trivial to add a `Hidden` variant in a follow up. * Depending on UI scaling there are sometimes gaps in the layout: <img width="532" alt="rounding-bug" src="https://github.com/user-attachments/assets/cc29aa0d-44fe-403f-8f0e-cd28a8b1d1b3"> This is caused by existing bugs in `ui_layout_system`'s coordinates rounding and not anything to do with the changes in this PR. ## Testing This PR also changes the `overflow` example to display borders on the overflow nodes so you can see how this works: #### main (The image is clipped at the edges of the node, overwriting the border). <img width="722" alt="main_overflow" src="https://github.com/user-attachments/assets/eb316cd0-fff8-46ee-b481-e0cd6bab3f5c"> #### this PR (The image is clipped at the edges of the node's border). <img width="711" alt="content-box-clip" src="https://github.com/user-attachments/assets/fb302e56-9302-47b9-9a29-ec3e15fe9a9f"> ## Migration Guide Migration guide is on #15561 --------- Co-authored-by: UkoeHB <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
Fixes #15142
Solution
ui_layout_system.border: BorderRectfield toNodeto store the border size computed byui_layout_system.logical_rectandphysical_rectmethods fromNodethe descriptions and namings are deceptive, it's better to create the rects manually instead.outline_radiustoNodethat calculates the border radius of outlines.ExtractedUiNodetakesBorderRectandResolvedBorderRadiusnow instead of raw[f32; 4]values and converts them inprepare_uinodes.BorderRect::ZEROconstant.outlined_node_sizemethod toNode.Testing
Added some non-uniform borders to the border example. Everything seems to be in order:
Migration Guide
The
logical_rectandphysical_rectmethods have been removed fromNode. UseRect::from_center_sizewith the translation and node size instead.The types of the fields border and border_radius of
ExtractedUiNodehave been changed toBorderRectandResolvedBorderRadiusrespectively.