-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
rem units support #21187
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
base: main
Are you sure you want to change the base?
rem units support #21187
Conversation
* Removed `TextFont` and `TextColor` from the `Text`, `Text2d`, and `TextSpan` requires, replaced with `ComputedTextStyle`. * `update_text_styles` updates the `ComputedTextStyle`s each frame from the text entities nearest ancestors with `TextFont` or `TextColor` components.
crates/bevy_ui/src/geometry.rs
Outdated
/// * For `margin`, `padding`, and `border` values: the percentage is relative to the parent node's width. | ||
/// * For positions, `left` and `right` are relative to the parent's width, while `bottom` and `top` are relative to the parent's height. | ||
Percent(f32), | ||
/// Value relative to the font size of the root element |
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.
Is this truly the root element of the hierarchy, or the default size in the resource? Normally the "r" in "rem" stands for "root" and everyone knows what it means, but perhaps in our case it should stand for "resource".
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.
It uses the value of the font size from the root UI node (if it has a TextFont
component), or the DefaultTextStyle
resource (if not).
I'm wondering if "rem" is the right name to use. I think it probably is, but "root element' is more web terminology again, which I want to avoid. I can only come up with ugly or silly alternatives though, like "rent" for "root entity".
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.
Unfortunately, I think we are dealing with conflicting needs here: a tradeoff between simplicity and flexibility.
Unlike the web where there's a single root element (html
), in Bevy you have multiple roots. This means that if font size is tied to the UI root, then globally adjusting the font size requires updating every root instead of adjusting a single parameter in a resource.
There's also a concern about cost: for something like an editor or app with nested panels, UI hierarchies can get fairly deep, with a depth of 10-15 levels not being uncommon. Calculating the effective font size for a leaf node could require looking up components on every ancestor.
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.
Unfortunately, I think we are dealing with conflicting needs here: a tradeoff between simplicity and flexibility.
Unlike the web where there's a single root element (
html
), in Bevy you have multiple roots. This means that if font size is tied to the UI root, then globally adjusting the font size requires updating every root instead of adjusting a single parameter in a resource.
Unless the root node is also a leaf text node, most of the time the only reason for setting a font size on the root would be to override the default font size from the resource. It feels to me like it would be very unintuitive if "rem" values weren't based on the font size of the node's root ancestor. Maybe we could have a seperate "gem" (or something) unit based on the global font size?
There's also a concern about cost: for something like an editor or app with nested panels, UI hierarchies can get fairly deep, with a depth of 10-15 levels not being uncommon. Calculating the effective font size for a leaf node could require looking up components on every ancestor.
The final implementation will do propagation on changes. When no font size is set, there won't be any need to walk up the tree. It will just access the default font resource directly.
Objective
Add rem units support.
Fixes #21208
Solution
TextFont
component'sfont_size
value, or, if not present, the size of the default font.Rem
variant toVal
and arem
helper construction function.rem
field toComputedUiRenderTargetInfo
.Val
resolve functions to support rem units.Based on #21181 for the default text style implementation.
Testing