bevyengine
Ui Node Borders
Objective
Implement borders for UI nodes.
Relevant discussion: #7785 Related: #5924, #3991
Solution
Add an extraction function to draw the borders.
Can only do one colour rectangular borders due to the limitations of the Bevy UI renderer.
Maybe it can be combined with #3991 eventually to add curved border support.
Changelog
- Added a component
BorderColor. - Added the
extract_uinode_borderssystem to the UI Render App. - Added the UI example
borders
I'm not sure what the overflow behaviour for borders should be. I have it clamp the border thickness to the size of the node at the moment, which seems reasonable enough.
I'm not sure what the overflow behaviour for borders should be. I have it clamp the border thickness to the size of the node at the moment, which seems reasonable enough.
As far as I can see, Chrome will always expand nodes to fit borders. And it seems that Taffy also implements this. So from Bevy's perspective, I think this will be a case of "never happens" (or at least shouldn't).
Update: Taffy does not in fact do this. We'll probably want to fix Taffy here.
Would it be possible to combine this PR with creating a new type specific for borders instead of UiRect (#7710) to have the width/color by border side in this PR?
Would it be possible to combine this PR with creating a new type specific for borders instead of
UiRect(#7710) to have the width/color by border side in this PR?
I think I'd prefer to keep this PR just focused on the implementation if we can.
I'm not sure what the overflow behaviour for borders should be. I have it clamp the border thickness to the size of the node at the moment, which seems reasonable enough.
If this refers to what happens when border overflows I suppose it depends on the parent node "overflow" property? Related though, is how the border affects the node size (CSS box-sizing), I would say that the most sane way is to grow the border inwards, see box-sizing: border-box: https://developer.mozilla.org/en-US/docs/Web/CSS/box-sizing
I mean, if the node has 200px width, and has a 10px border, which is the size of the node? 200px? 220px?
(ignore me if this unrelated 😅 )
I mean, if the node has 200px width, and has a 10px border, which is the size of the node? 200px? 220px?
It is indeed 200px (the border grows inwards) because we use box-sizing: border-box (historical note: it used to be 200px in internet explorer and 220px in Firefox - now you know why web dev was such a nightmare in the past). However, on the web if the node is 0px and has a 10px border, then the node will end up 20px wide. I'm currently implementing this in Taffy here: https://github.com/DioxusLabs/taffy/pull/372, after which I don't think bevy will need to worry about this as borders will always fit in the computed layout.
I'm old enough to have suffered those CSS quirks… 😅 Anyway, I was asking because ickshonpe was asking about overflow behaviour; for a moment I though Bevy wasn't doing border-box.
Although, I totally forgot that the sum of borders could be bigger than width… nice that it's being fixed. The PR you link means that size will be computed as: max(size, border + padding)?
Btw, unrelated, but just wanted to mention that you're doing an amazing job with Taffy. ❤️
for a moment I though Bevy wasn't doing
border-box
We're definitely doing border-box! Making that the default (well, only) style is our one (intentional) deviation from the web. We have had content-box support requested, but I'm pretty reluctant to add it. Yoga, from which Taffy was originally derived mentions that it's model is (unintentionally I think) a bit of a hybrid between border-box and content-box, so it's possible we're not completely compliant with the border-box model. But I think we're pretty close now if not.
The PR you link means that size will be computed as: max(size, border + padding)?
Yes, although the computation of size in that formula is itself quite complex, involving the size, min_size,max_size and aspect_ratio styles. As well as the node's content based size and it's flexbox alignment.
Btw, unrelated, but just wanted to mention that you're doing an amazing job with Taffy. ❤️
Thanks!
Taffy v0.3.7 has been released with changes that ensure that there is always enough space in the computed layout for borders (and padding).
Taffy
v0.3.7has been released with changes that ensure that there is always enough space in the computed layout for borders (and padding).
Thats brilliant. And from what I've looked up it's my understanding there's no meaning to negative borders so any negative values should just be clamped to 0, is that right?
I think maybe it's not worth precalculating the borders and having the ugly CalculatedBorder component and the extra system. I'll rewrite the extraction function and see what the performance is like without them.
it's my understanding there's no meaning to negative borders so any negative values should just be clamped to 0, is that right
Correct
Removed CalculatedBorders and the borders modules. This PR consists only of an extra component and the extract_uinode_borders function now.
The performance difference is really negligible without CalculatedBorders which definitely seems not worth the extra complexity, especially considering that this should only be a placeholder to be replaced with a shader-based implementation eventually.
running example many_buttons without text:
- without borders, around 250 fps
- with borders on each button: around 120 fps
I think it's probably OK but there's definitely a perf impact
running example
many_buttonswithout text:
- without borders, around 250 fps
- with borders on each button: around 120 fps
I think it's probably OK but there's definitely a perf impact
Yep considering that by adding borders to many_buttons you draw an extra 50000 quads it's not that bad. If performance is really crucial you can always fall back to the old method of drawing a border by using margins or padding to draw a node within another node.
Approach seems good, code is generally good. Examples are fine, if still pretty artificial. There are a few tricky bits of math that would really benefit from a bit more explanation.
@natasria could we get your opinion on this PR vs 8381? They're both trying to do the same thing: we should pick one and merge it for the upcoming release :)
@natasria could we get your opinion on this PR vs 8381? They're both trying to do the same thing: we should pick one and merge it for the upcoming release :)
Well, feature-wise #8381 has border radiuses, but it forces the borders to be the same for all sides, this PR allows specifying for different borders in the sides.
#8381 still have rough edges to polish on border radiuses (it's just aliased borders so the name is true? <3 ) and sharp transition issues too, but I don't feel it's a blocker.
