-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Merged by Bors] - Upgrade to Taffy 0.3.3 #7859
Conversation
* Removed the `Undefined` variant of the `Val` enum. * Changed `UiRect::default()` to set all fields to `Val::Px(0.0)`. * Added the `Inset` type that is a copy of `UiRect` but its defaults are all `Val::Auto`. * Renamed the `position` field of `Style` to `inset` and changed its type to `Inset`. * Updated the UI examples to remove or replace any use of `Val::Undefined`. * Updated the UI examples to use `Inset`.
Added a type `Breadth`, that is similar to `Val` but with only evaluatable variants. Gave UiRect a type parameter, so it can take `Breadth` or `Val` values. Added tests for `Breadth` and `UiRect`. Changed style properties to use `Breadth` instead of `Val` for the padding and border properties. Changed `bevy_ui::flex::convert::from_rect` to take an `UiRect<T: Into<Val>>` instead of a `UiRect`. Made minimal necessary changes to the examples.
…padding and border values in `bevy_ui::flex::convert::from_style`
* Added a new trait `MeasureNode`. * Added new structs `ImageMeasure` and `BasicMeasure` that implement `MeasureNode`. * Add a field to `CalculatedSize` called `measure` that takes a boxed `MeasureNode`. * `upsert_leaf` uses the `measure` of `CalculatedSize` to create a `MeasureFunc` for the node.
I got carried away again. There's no need to include the |
Good, I was wondering about that :) |
@ickshonpe I think I'd like to leave the grid support to a seperate PR and merge this more quickly. It's significantly more involved, and there's no reason to block the performance improvements and bug fixes. |
Agreed, grid should be implemented in another PR. This seems fine as it is now. |
crates/bevy_ui/src/flex/convert.rs
Outdated
bottom: from_val(scale_factor, rect.bottom), | ||
fn from_breadth(scale_factor: f64, breadth: Breadth) -> LengthPercentage { | ||
match breadth { | ||
Breadth::Percent(value) => LengthPercentage::Percent(value / 100.0), |
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.
Yeah, I was thinking something like PercentAsFloat
or PercentAsDecimal
. Val::Percent(1.0)
is definitely confusing! Taffy currently has the same issue.
crates/bevy_ui/src/geometry.rs
Outdated
/// }; | ||
/// ``` | ||
#[derive(Copy, Clone, PartialEq, Debug, Reflect)] | ||
#[reflect(PartialEq)] | ||
pub struct UiRect { | ||
pub struct UiRect<T: Default + Copy + Clone + PartialEq = Val> { |
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.
You could also just remove the defaults
crates/bevy_ui/src/ui_node.rs
Outdated
#[derive(Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Reflect)] | ||
#[reflect(PartialEq, Serialize, Deserialize)] | ||
pub enum JustifySelf { | ||
Auto, |
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.
I'm not quite sure what JustifySelf does. I thought it was a grid property but it's not part of Taffy's grid feature.
Correct. It not being feature flagged by grid
in Taffy is a bug (unnoticed because having an extra style property doesn't break anything).
@@ -412,26 +418,26 @@ impl Default for AlignSelf { | |||
#[derive(Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Reflect)] | |||
#[reflect(PartialEq, Serialize, Deserialize)] | |||
pub enum AlignContent { | |||
/// Items are packed toward the start of the axis | |||
/// Each line moves towards the start of the cross axis. |
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.
"cross axis" is true for Flexbox. But Grid has no "main" or "cross" axis. It only has "block" (vertical with left-to-right writing mode) and "inline" (horizontal with left-to-right writing mode). For grid, the align*
properties apply to the block axis and the justify*
properties apply to the inline axis.
Removed the extra enum variants and comments that were grid specific. |
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.
I'm still not 100% familiar with bevy's style setup, but this looks good to me.
I've taken a thorough look through the code and run the examples. This is ready: I'm glad the migration was so easy. Going to merge now: in its current form it's an uncontroversial perf + bug fix PR due to things resolved in taffy. For the We can add CSS grid support and remove bors r+ |
@alice-i-cecile Bors doesn't seem to be doing anything with this. Could it perhaps be the milestone label which is preventing it from being merged? |
Milestone shouldn't be having an effect. Sometimes bors just gets sleepy (the scanning process isn't fully reliable IME) ;) bors r+ |
# Objective Upgrade to Taffy 0.3.3 Fixes: #7712 ## Solution Upgrade to Taffy 0.3.3 with the `grid` feature disabled. --- ## Changelog * Changed Taffy version to 0.3.3 and disabled its `grid` feature. * Added the `Start` and `End` variants to `AlignItems`, `AlignSelf`, `AlignContent` and `JustifyContent`. * Added the `SpaceEvenly` variant to `AlignContent`. * Updated `from_style` for Taffy 0.3.3.
# Objective Upgrade to Taffy 0.3.3 Fixes: bevyengine#7712 ## Solution Upgrade to Taffy 0.3.3 with the `grid` feature disabled. --- ## Changelog * Changed Taffy version to 0.3.3 and disabled its `grid` feature. * Added the `Start` and `End` variants to `AlignItems`, `AlignSelf`, `AlignContent` and `JustifyContent`. * Added the `SpaceEvenly` variant to `AlignContent`. * Updated `from_style` for Taffy 0.3.3.
Objective
Upgrade to Taffy 0.3.3
Fixes: #7712
Solution
Upgrade to Taffy 0.3.3 with the
grid
feature disabled.Changelog
grid
feature.Start
andEnd
variants toAlignItems
,AlignSelf
,AlignContent
andJustifyContent
.SpaceEvenly
variant toAlignContent
.from_style
for Taffy 0.3.3.