Refactor `Style` component into many smaller pieces

[alice-i-cecile]

Note for release notes: this PR is co-authored with @Weibye <3

Objective

• The Style component is monolithic and poorly named (it only controls layout). This makes it hard to work with and reason about.
• Fixes #5511.
• Fixes #6286.

Changelog

• broke the Style UI component into many small components
• added the FlexboxLayoutQuery, FlexboxLayoutQueryItem and FlexboxLayoutChanged helper types to make recomputing layouts easier

Migration Guide

The Style component has been removed. In its place, there are several new components.

Style field	New location
display: Display	LayoutStrategy
position_type: PositionType	PositionType
direction: Direction	TextDirection
flex_direction: FlexDirection	FlexboxLayout.flex_direction
flex_wrap: FlexWrap	FlexboxLayout.wrap
align_items: AlignItems	FlexboxLayout.align_items
align_self: AlignSelf	FlexboxLayout.align_self
align_content: AlignContent	FlexboxLayout.align_content
justify_content: JustifyContent	FlexboxLayout.justify_content
position: UiRect	UiPosition
margin: UiRect	Spacing.margin
padding: UiRect	Spacing.padding
border: UiRect	Spacing.border
flex_grow: f32	FlexboxLayout.flex_grow
flex_shrink: f32	FlexboxLayout.flex_shrink
flex_basis: Val	FlexboxLayout.flex_basis
size: Size	SizeConstraints.suggested
min_size: Size	SizeConstraints.min
max_size: Size	SizeConstraints.max
aspect_ratio: Option	SizeConstraints.aspect_ratio
overflow: Overflow	Overflow

TODO

• [x] Reach consensus on design goals
• [ ] Reach consensus on exact split
• [x] Migrate examples
• [ ] Decided how we're doing the builder pattern, see https://github.com/bevyengine/bevy/pull/3694#issuecomment-1200546820

[IceSentry]

I'd suggest only migrating the ui example for now. It's complex enough to show the use of this new api without being too complex.

[IceSentry]

This should probably be done in a future PR just to not let this one grow too much, but doing this for TextStyle should probably be done too.

[Nilirad]

I'm a bit concerned that it will reduce discoverability of all the style parameters.

We can group the components under a module, so they are listed in rendered documentation.

[Weibye]

Been thinking a bit about this recently, so here's some deeper reasoning and context:

Let's consider the use case of us implementing borders: (#3991)

• We want to support creating borders on rects.
• This should support border width, border radius, color / material and a few other properties.
• Rendering of the border and layouting of the border should be the same. I.e there should only be one source of truth for setting these properties.

Option 1

• To set border width, user sets the border property in Style
• To set border color, border radius, the user sets the properties on some completely different component BorderComponent

This has the downside of splitting concerns across multiple components without providing consistency.

Option 2

• All border-related properties are moved into BorderComponent and out of Style
• Adding a border to a node is now simple as only a single component is concerned.
• During layouting, we query the BorderComponent.rect to feed the data into the layout calculation

The end-user experience is better in this situation, but it does fragment Style.

Option 3

• All Style-properties are fragmented fully.
• BorderWidthComponent, BorderColorComponent, BorderRadiusComponent are introduced.
• In order to setup borders on nodes, some or all of these components must be present

End user experience is much more verbose, but this is technically closer to how CSS actually operates.

---

I think option 2 or 3 is the best way to go as they provide much more consistency, and they pave the way to unlock more style functionality that isn't related to layouting.

Option 3 is probably the best if, and only if we manage to build higher level ergonomics on top, but that is something that has to come later. We can start with path 2, then go to 3 later.

My conclusion is that we need this merged to unlock the way forward.

[StarArawn]

How does one handle reusable styles when its fragmented into multiple components like this? Perhaps a separate monolithic style bundle? 🤔Internally I'm still debating if styles should be attached to entities or if they should be treated more like materials that live inside of the asset system.

[Weibye]

How does one handle reusable styles when its fragmented into multiple components like this? Perhaps a separate monolithic style bundle? 🤔Internally I'm still debating if styles should be attached to entities or if they should be treated more like materials that live inside of the asset system.

Here's how I'm picturing it at a high level:

1. The style-components are completely fragmented. Their job is only to tell layouting and rending of what these nodes should look like right now. They just store the runtime data.
2. We have some asset that defines a theme. Likely using scenes, but behaves like css. This asset stores targets and the styling data to be applied to them.
3. A theming-plugin / system is responsible for getting the theme-asset as input, queries for the nodes specified, then inject new data to the various style-components.
4. Rendering and layouting is run, in which they simply query for the style-components they need to look at.

The heavy lifting will be done by the theming-plugin, and we should try to aim for a design that separates theme from style.

[TimJentzsch]

I would suggest to reorder the migration table a bit to have all flexbox layout stuff together.

[alice-i-cecile]

How does one handle reusable styles when its fragmented into multiple components like this? Perhaps a separate monolithic style bundle?

The "store each style as an entity" model in https://github.com/bevyengine/rfcs/pull/1 would work well with more scattered styling components. It's also fully extensible to user-defined style properties.

We already have issues with this; other things you might want to share (such as color) live outside of the Style component.

[Weibye]

This can be considered ready for review :)

[Weibye]

Rebased on top of current main. Will start addressing feedback.

[Weibye]

@alice-i-cecile Please update the description with the following:

Migration Guide

The Style component has been removed. In its place, there are several new components:

• Layout
• FlexLayout
• PositionType
• Offset
• SizeConstraints
• Spacing
• Overflow

Previous location	New location
Style.display: Display	Layout
Style.position_type: PositionType	PositionType
Style.direction: Direction	:exclamation: Removed. Bevy does not currently support multiple text-directions ❗
Style.flex_direction: FlexDirection	FlexboxLayout.direction
Style.flex_wrap: FlexWrap	FlexboxLayout.wrap
Style.align_items: AlignItems	FlexboxLayout.align_items
Style.align_self: AlignSelf	FlexboxLayout.align_self
Style.align_content: AlignContent	FlexboxLayout.align_content
Style.justify_content: JustifyContent	FlexboxLayout.justify_content
Style.flex_grow: f32	FlexboxLayout.flex_grow
Style.flex_shrink: f32	FlexboxLayout.flex_shrink
Style.flex_basis: Val	FlexboxLayout.flex_basis
Style.position: UiRect	Offset
Style.margin: UiRect	Spacing.margin
Style.padding: UiRect	Spacing.padding
Style.border: UiRect	Spacing.border
Style.size: Size	SizeConstraints.suggested
Style.min_size: Size	SizeConstraints.min
Style.max_size: Size	SizeConstraints.max
Style.aspect_ratio: Option	SizeConstraints.aspect_ratio
Style.overflow: Overflow	Overflow

[nicoburns]

I would suggest that FlexboxLayout is split into FlexContainerLayout and FlexChildLayout:

• FlexContainerLayout would have flex_direction, flex_wrap, align_items, align_content, justify_content (and gap from Taffy 0.2 if you add support for that).
• FlexChildLayout would have flex-basis, flex-grow, flex-shrink and align-self.

There will likely be lots of nodes that use only 1 set of these but not the other. Especially in the future if more layout modes get added.

[Weibye]

Edit:❗ Outdated ❗

This is what the migration guide looks like with these changes:

Migration Guide

The Style component has been removed. In its place, there are several new components:

• FlexContainerLayout
• FlexChildLayout
• PositionType
• Offset
• SizeConstraints
• Spacing
• Overflow

Previous location	New location
Style.direction: Direction	:exclamation: Removed. Bevy does not currently support multiple text-directions ❗
Style.display: Display	FlexContainerLayout.hide_from_layout: bool
Style.flex_direction: FlexDirection	FlexContainerLayout.direction: Direction
Style.flex_wrap: FlexWrap	FlexContainerLayout.wrap: Wrap
Style.align_items: AlignItems	FlexContainerLayout.align_items: AlignItems
Style.align_content: AlignContent	FlexContainerLayout.align_content: AlignContent
Style.justify_content: JustifyContent	FlexContainerLayout.justify_content: JustifyContent
Style.align_self: AlignSelf	FlexChildLayout.align_self: AlignSelf
Style.flex_grow: f32	FlexChildLayout.flex_grow: f32
Style.flex_shrink: f32	FlexChildLayout.flex_shrink: f32
Style.flex_basis: Val	FlexChildLayout.flex_basis: Val
Style.position_type: PositionType	PositionType
Style.position: UiRect	Offset(UiRect)
Style.margin: UiRect	Spacing.margin: UiRect
Style.padding: UiRect	Spacing.padding: UiRect
Style.border: UiRect	Spacing.border: UiRect

[nicoburns]

@Weibye Your latest grouping looks pretty good to me. I have a few comments:

• I think Display (or whatever it ends up being called) should be separate from FlexContainerProps, as it applies to leaf nodes too.
• I think Display should be an enum rather than a boolean. This aids readability and it self-documenting. You could always have builder/setter methods that accept a boolean if that's desirable.
• I think you should consider grouping Display with Position. As I see it, these are really the two most fundamental properties that apply to every single node. I don't know if bevy_ui supports scrolling (Overflow in CSS), but it would also make sense to me to group the property for that with these two if it does.
• The alignment properties apply to Grid too, so long term it might make sense to split them into their own components (container and child). But probably that's not worth worrying about for now?

I think we're going to need to do something similar upstream in Taffy in order to support custom layout modes, so watching this keenly.

[mockersf]

FlexboxLayout could be split between containers properties and item properties:

• https://www.w3schools.com/css/css3_flexbox_container.asp
• https://www.w3schools.com/css/css3_flexbox_items.asp

[mockersf]

Ha I was basing my comment above on the PR description, but it seems it changed since? @Weibye if that's the case I would prefer to replace your "child" notion in flex box by "item" which is more standard @alice-i-cecile could you update the description to reflect the latest state? thanks!

[alice-i-cecile]

Yep! I will. Moving this to draft until I do so. May be a bit: I'm without a desktop for two weeks, and this isn't the sort of work I really want to tackle while travelling.

[Weibye]

• I think you should consider grouping Display with Position. As I see it, these are really the two most fundamental properties that apply to every single node. I don't know if bevy_ui supports scrolling (Overflow in CSS), but it would also make sense to me to group the property for that with these two if it does.

What should this groping be called?

pub struct ???
{
    pub UiRect offset; // alternatively revert back to `pub UiRect position;`
    pub Display display;
    pub Overflow overflow; // potentially, at a later time
}

[Weibye]

Does it make sense to do the following renames?

• FlexContainerLayout -> FlexContainer
• FlexChildLayout -> FlexItem

[Weibye]

1. Brought this up to speed on main
   • ~~Gap is now a standalone component which I'm not sure if is the correct approach.~~
   • @nicoburns Do you think this instead belongs within either FlexContainer or FlexItem?
     • Gap has been moved into FlexContainer
2. Renamed FlexContainerLayout -> FlexContainer
3. Renamed FlexChildLayout -> FlexItem
4. Renamed PositionType -> Position to adhere to CSS specification.
   • Specs: https://developer.mozilla.org/en-US/docs/Web/CSS/position
5. Renamed Offset -> Inset to adhere to CSS specification.
   • It more correctly describes the behaviour
   • Specs: https://developer.mozilla.org/en-US/docs/Web/CSS/inset
   • The same change was done in taffy (but not yet released): https://github.com/DioxusLabs/taffy/issues/271

[nicoburns]

@Weibye Would you be able to make another table (like the one in the first post) representing the latest state of this PR?

I'd suggest that Gap probably belongs with FlexContainer for now. It also applies to the future GridContainer, but that is also true of the alignment properties in FlexContainer.

[nicoburns]

For context, the styles for CSS Grid will look like (new properties marked in bold):

GridContainer

Property	Explanation
grid-template-columns	The track sizing functions of the grid's explicit columns
grid-template-rows	The track sizing functions of the grid's explicit rows
grid-auto-rows	Track sizing functions for the grid's implicitly generated rows
grid-auto-columns	Track sizing functions for the grid's implicitly generated columns
grid-auto-flow	Whether auto-placed items are placed row-wise or column-wise. And sparsely or densely.
gap	The size of the vertical and horizontal gaps between grid rows/columns
align-content	Align grid tracks within the container in the inline (horizontal) axis
justify-content	Align grid tracks within the container in the block (vertical) axis
align-items	Align the child items within their grid areas in the inline (horizontal) axis
justify-items	Align the child items within their grid areas in the block (vertical) axis

GridItem

Property	Explanation
grid-row	The (row) grid line the item starts at (or a span)
grid-column	The (column) grid line the item end at (or a span)
align-self	Align the item within it's grid area in the inline (horizontal) axis. Overrides align-items.
justify-self	Align the item within it's grid area in the block (vertical) axis. Overrides justify-items.

[Weibye]

@Weibye Would you be able to make another table (like the one in the first post) representing the latest state of this PR?

Yes, will do but I'd like to get rid of the "display as boolean" (FlexContainer.hide_from_layout) first.

[Weibye]

Migration Guide

Up to date as of 76557be5f80473547d0bfcb4de857d0d78f77046

The Style-component has been removed. In its place, there are several new components:

• LayoutControl
• FlexContainer
• FlexItem
• SizeConstraints
• Spacing

Previous location	New location
Style.direction: Direction	:exclamation: Removed. Bevy does not currently support multiple text-directions ❗
	LayoutControl
Style.position_type: PositionType	LayoutControl.position: Position
Style.position: UiRect	LayoutControl.inset: Inset(UiRect)
Style.overflow: Overflow	LayoutControl.overflow: Overflow
Style.display: Display	LayoutControl.display: Display
	FlexContainer
Style.flex_direction: FlexDirection	FlexContainer.direction: Direction
Style.flex_wrap: FlexWrap	FlexContainer.wrap: Wrap
Style.align_items: AlignItems	FlexContainer.align_items: AlignItems
Style.align_content: AlignContent	FlexContainer.align_content: AlignContent
Style.justify_content: JustifyContent	FlexContainer.justify_content: JustifyContent
Style.gap: Gap(UiRect)	FlexContainer.gap: Gap(UiRect)
	FlexItem
Style.align_self: AlignSelf	FlexItem.align_self: AlignSelf
Style.flex_grow: f32	FlexItem.flex_grow: f32
Style.flex_shrink: f32	FlexItem.flex_shrink: f32
Style.flex_basis: Val	FlexItem.flex_basis: Val
	Spacing
Style.margin: UiRect	Spacing.margin: UiRect
Style.padding: UiRect	Spacing.padding: UiRect
Style.border: UiRect	Spacing.border: UiRect
	SizeConstraints
Style.size: Size	SizeConstraints.suggested: Size
Style.min_size: Size	SizeConstraints.min: Size
Style.max_size: Size	SizeConstraints.max: Size
Style.aspect_ratio: Option<f32>	SizeConstraints.aspect_ratio: Option<f32>

Queries

Any queries that previously queried all of Style should now instead use the FlexLayoutQuery.

// old
fn my_system(q: Query<&Style>) { .. }
// new
fn my_system(q: Query<FlexlayoutQuery>) { .. }

Most of the time however, you'd likely want to query for the more specific layout components you need.

Change detection

If you previously relied on detecting when something on Style changed you can now instead use the FlexLayoutChanged type.

// old
fn my_system(q: Query<.., Changed<Style>>) { .. }
// new
fn my_system(q: Query<.., Changed<FlexLayoutChanged>>) { .. }

[nicoburns]

@Weibye Your table is missing gap :)

[Weibye]

@Weibye Your table is missing gap :)

Fixed!

[ickshonpe]

The way the constraints data is structured seems wrong to me. In Flex you usually access the constraints one axis at a time but the current API makes that awkward and fragile. For example, look at the text_constraint function in widget/text.rs:

pub fn text_constraint(min_size: Val, size: Val, max_size: Val, scale_factor: f64) -> f32 {
    match (min_size, size, max_size) {
       (_, _, Val::Px(max)) => scale_value(max, scale_factor),
       // .. 
    }
}

called with:

text_constraint(
      layout.size_constraints.min.width,
      layout.size_constraints.suggested.width,
      layout.size_constraints.max.width,
      scale_factor,
 )

With the constraints data stored per-axis:

pub struct LengthConstraint {
    pub min: Val,
    pub max: Val,
    pub suggested: Val,
} 

it's much more natural:

pub fn text_constraint(length_constraint: LengthConstraint, scale_factor: f64) -> f32 {
    match length_constraint {
        LengthConstraint { max: Val::Px(max), .. } => scale_value(max, scale_factor),
        // .. 
    }
}

text_constraint(layout.size_constraints.width, scale_factor)

In Bevy that is the worst it gets at the moment, but in Taffy there are lots of tangled sections like:

 if constants.node_inner_size.main(constants.dir).is_none() && constants.is_row {
            child.target_size.set_main(
                constants.dir,
                child.size.main(constants.dir).unwrap_or(0.0).maybe_clamp(
                    child.resolved_minimum_size.main(constants.dir).into(),
                    child.max_size.main(constants.dir),
                ),
            );
        } else {
            child.target_size.set_main(constants.dir, child.hypothetical_inner_size.main(constants.dir));
        }

where most of the complexity would vanish with some sort of per-axis interface.

[alice-i-cecile]

I agree with that. @Weibye, do you think we should implement that now, or fix it upstream first?

[Weibye]

I agree with that. @Weibye, do you think we should implement that now, or fix it upstream first?

If possible I'd prefer getting this merged in its current state, then tackle that in a separate PR so that discussion can take place more focused, and we can get some movement on these refactors.

I do think we should fix the axis separation upstream first, then fix it here after.

[nicoburns]

I do think we should fix the axis separation upstream first, then fix it here after.

I'm not even completely convinced that this would an improvement for Taffy. It's true that accessing lots of values in a single axis is awkward atm. But equally it makes accessing both components of the same value easy, and that does happen a lot too. For example, applying aspect ratio is a single method call on Size<T> atm and would be pretty awkward to do if the two components were separated.