Skip to content
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

Upgrade to Taffy 0.6 #15844

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Upgrade to Taffy 0.6 #15844

wants to merge 2 commits into from

Conversation

nicoburns
Copy link
Contributor

@nicoburns nicoburns commented Oct 10, 2024

Objective

  • Keep Taffy version up to date

Taffy 0.6 doesn't include a huge amount relevant to Bevy. But it does:

  • Add the box_sizing style
  • Expose the computed margin in layout
  • Traitifies the Style struct, which opens up the possibility of using Bevy's Style struct directly (although Bevy currently does some style resolution at conversion time which would no longer be cached if it was used directly).
  • Have a few bug fixes in the layout algorithms

Solution

  • Upgrade Taffy to 0.6.0

Testing

  • I've run the grid example. All looks good.
  • More testing is probably warranted. We have had regressions from Taffy upgrades before
  • Having said that, most of the algorithm changes this cycle were driven by fixing WPT tests run through the new Servo integration. So they're possibly less likely than usual to cause regressions.

Breaking changes

The only "breaking" change is adding a field to Style. Probably doesn't bear mentioning?

@nicoburns nicoburns added the A-UI Graphical user interfaces, styles, layouts, and widgets label Oct 10, 2024
@nicoburns nicoburns added this to the 0.16 milestone Oct 10, 2024
@alice-i-cecile alice-i-cecile added C-Dependencies A change to the crates that Bevy depends on D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Oct 10, 2024
Copy link
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

crates/bevy_ui/src/ui_node.rs Outdated Show resolved Hide resolved
crates/bevy_ui/src/ui_node.rs Outdated Show resolved Hide resolved
crates/bevy_ui/src/ui_node.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@ickshonpe ickshonpe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good besides the comments nits.

Adding the computed margin values would let me add support for margin-box to #15561.

@nicoburns
Copy link
Contributor Author

Nits addressed

@rparrett rparrett added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Dependencies A change to the crates that Bevy depends on D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants