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

Update glam version requirement from 0.25 to 0.27 #12757

Merged
merged 4 commits into from May 2, 2024

Conversation

mnmaita
Copy link
Member

@mnmaita mnmaita commented Mar 28, 2024

Objective

  • Update glam version requirement to latest version.

Solution

  • Updated glam version requirement from 0.25 to 0.27.
  • Updated encase and encase_derive_impl version requirement from 0.7 to 0.8.
  • Updated hexasphere version requirement from 10.0 to 12.0.
  • Breaking changes from glam changelog:
    • [0.26.0] Minimum Supported Rust Version bumped to 1.68.2 for impl From for {f32,f64} support.
    • [0.27.0] Changed implementation of vector fract method to match the Rust implementation instead of the GLSL implementation, that is self - self.trunc() instead of self - self.floor().

Migration Guide

  • When using glam exports, keep in mind that vector fract() method now matches Rust implementation (that is self - self.trunc() instead of self - self.floor()). If you want to use the GLSL implementation you should now use fract_gl().

@mnmaita
Copy link
Member Author

mnmaita commented Mar 28, 2024

Bevy does re-export structs from glam, right? If so, I'm wondering if I should add a migration guide along the lines of "keep in mind that vector fract method now matches Rust implementation. If you want to use GLSL implementation use fract_gl()".

I also see several errors popping up in CI so I'll check them later. encase seems to need a version bump before we can merge this. See teoxoy/encase#70.

@matiqo15 matiqo15 added the C-Dependencies A change to the crates that Bevy depends on label Mar 28, 2024
@mockersf
Copy link
Member

mockersf commented Mar 28, 2024

this PR should also update hexasphere to 0.12, and is blocked on encase releasing an updated version using glam 0.27

@mockersf mockersf added the S-Blocked This cannot move forward until something else changes label Mar 28, 2024
@mnmaita
Copy link
Member Author

mnmaita commented Mar 29, 2024

this PR should also update hexasphere to 0.12, and is blocked on encase releasing an updated version using glam 0.27

Pushed an update to bump hexaspere version requirement. We'll wait for a new encase release then. Thanks!

@teoxoy
Copy link
Contributor

teoxoy commented Apr 24, 2024

Just published encase v0.8.

@mnmaita
Copy link
Member Author

mnmaita commented Apr 25, 2024

Thanks a lot for the release and the ping @teoxoy! This should be ready to be reviewed now @mockersf. Would you also be able to clarify if there will be any potential breaking changes that require migration? Please check my first comment for context.

@mockersf
Copy link
Member

Would you also be able to clarify if there will be any potential breaking changes that require migration? Please check my first comment for context.

I think it's very niche, but it certainly doesn't hurt to mention it

@mockersf mockersf added A-Math Fundamental domain-agnostic mathematical operations and removed S-Blocked This cannot move forward until something else changes labels Apr 27, 2024
@mnmaita
Copy link
Member Author

mnmaita commented Apr 30, 2024

@mockersf rebased and updated the Migration Guide with some details on glam breaking changes. Should be good to merge now.

@superdump superdump added this pull request to the merge queue May 2, 2024
Copy link
Contributor

@NthTensor NthTensor 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 but I had a question about the linear rgb change.

crates/bevy_color/src/linear_rgba.rs Outdated Show resolved Hide resolved
@superdump superdump removed this pull request from the merge queue due to a manual request May 2, 2024
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 2, 2024
@NthTensor
Copy link
Contributor

NthTensor commented May 2, 2024

@alice-i-cecile Can we just commit that suggested change and add this to the queue? It's a one liner, and this is blocking some other stuff.

@mnmaita
Copy link
Member Author

mnmaita commented May 2, 2024

@alice-i-cecile Can we just commit that suggested change and add this to the queue? It's a one liner, and this is blocking some other stuff.

Commited the change and rebased. @alice-i-cecile @mockersf we should be able to merge now.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 2, 2024
Merged via the queue into bevyengine:main with commit 32cd0c5 May 2, 2024
31 checks passed
@mnmaita mnmaita deleted the mnmaita/glam-0.27 branch May 4, 2024 14:08
nzhao95 added a commit to nzhao95/bevy that referenced this pull request May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Dependencies A change to the crates that Bevy depends on 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.

None yet

8 participants