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

Fix floating point math #15239

Merged

Conversation

BenjaminBrienen
Copy link
Contributor

Objective

Solution

  • Use bevy_math::ops instead of std floating point operations.

Testing

  • Did you test these changes? If so, how?
    Unit tests and cargo run -p ci -- test

  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
    Execute cargo run -p ci -- test on Windows.

  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?
    Windows

Migration Guide

  • Not a breaking change
  • Projects should use bevy math where applicable

@BenjaminBrienen
Copy link
Contributor Author

This is incomplete because the test still fails :)

Copy link
Contributor

@IQuick143 IQuick143 left a comment

Choose a reason for hiding this comment

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

The -ln needs fixing, I left some other comments, but those aren't all that blocking.

Someone else please take a thorough look, cause there's a lot of code touched so I may have missed smth.

@IQuick143 IQuick143 added C-Code-Quality A section of code that is hard to understand or change A-Math Fundamental domain-agnostic mathematical operations A-Cross-Cutting Impacts the entire engine D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Sep 16, 2024
BenjaminBrienen and others added 6 commits September 16, 2024 21:42
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
@BenjaminBrienen
Copy link
Contributor Author

This is in a good state for now, but I'm waiting for conclusive feedback on exactly how explicit we should make the calls to bevy_math trig functions, especially in documentation and examples.

@BenjaminBrienen
Copy link
Contributor Author

If anyone would like to suggest a different name for bevy_math::ops before it is made public, now is a good time. To me, ops kind of implies the traits like Add and Mul (for bevy, maybe instead for matrices and vector operations).

@alice-i-cecile
Copy link
Member

ops is fine IMO :)

@alice-i-cecile
Copy link
Member

This is in a good state for now, but I'm waiting for conclusive feedback on exactly how explicit we should make the calls to bevy_math trig functions, especially in documentation and examples.

Stick it in the prelude, but use only the method names please.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 16, 2024
Merged via the queue into bevyengine:main with commit 29508f0 Sep 16, 2024
26 checks passed
@BenjaminBrienen BenjaminBrienen deleted the fix-floating-point-math branch September 17, 2024 00:15
@IQuick143
Copy link
Contributor

Oh wow, this ended up being quite an undertaking, props for the work.

@BenjaminBrienen BenjaminBrienen self-assigned this Nov 10, 2024
@BenjaminBrienen BenjaminBrienen removed 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 Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Cross-Cutting Impacts the entire engine A-Math Fundamental domain-agnostic mathematical operations C-Code-Quality A section of code that is hard to understand or change C-Machine-Specific This bug is isolated to specific hardware or driver configurations D-Straightforward Simple bug fixes and API improvements, docs, test and examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bevy_color's operations are not stable across all hardware due to use of trigonometric float methods
5 participants