Add `no_std` Support to `bevy_math` by bushrat011899 · Pull Request #15810 · bevyengine/bevy

bushrat011899 · 2024-10-10T04:28:39Z

Objective

Contributes to Tracking Issue: MVP no_std Bevy #15460

Solution

Added two new features, std (default) and alloc, gating std and alloc behind them respectively.
Added missing f32 functions to std_ops as required. These f32 methods have been added to the clippy.toml deny list to aid in no_std development.

Testing

CI
cargo clippy -p bevy_math --no-default-features --features libm --target "x86_64-unknown-none"
cargo test -p bevy_math --no-default-features --features libm
cargo test -p bevy_math --no-default-features --features "libm, alloc"
cargo test -p bevy_math --no-default-features --features "libm, alloc, std"
cargo test -p bevy_math --no-default-features --features "std"

Notes

The following items require the alloc feature to be enabled:

CubicBSpline
CubicBezier
CubicCardinalSpline
CubicCurve
CubicGenerator
CubicHermite
CubicNurbs
CyclicCubicGenerator
RationalCurve
RationalGenerator
BoxedPolygon
BoxedPolyline2d
BoxedPolyline3d
SampleCurve
SampleAutoCurve
UnevenSampleCurve
UnevenSampleAutoCurve
EvenCore
UnevenCore
ChunkedUnevenCore

This requirement could be relaxed in certain cases, but I had erred on the side of gating rather than modifying. Since no_std is a new set of platforms we are adding support to, and the alloc feature is enabled by default, this is not a breaking change.

crates/bevy_math/Cargo.toml

crates/bevy_math/src/ops.rs

Co-Authored-By: Benjamin Brienen <[email protected]>

crates/bevy_math/src/ops.rs

mweatherley

A few notes:

The purpose of the ops module is (or perhaps was) to guarantee that alternatives to std floating-point functions with unspecified precision were available, in the interest of cross-platform determinism. Since libm is no_std-compatible, I think this extension of that is pretty reasonable, but this is a break from its original purpose.
I have concerns about the ongoing maintenance burden of these changes. For example, we lint against std math operations with unspecified precision for the aforementioned determinism reasons; right now, as far as I'm aware, this doesn't extend to the new methods added, so I'm not sure if breaks to no_std compatibility are actually discoverable.
cubic_splines is feature-gated behind alloc, but plenty of other areas use alloc as well. What is the logic behind this?

bushrat011899 · 2024-10-10T15:54:39Z

The purpose of the ops module is (or perhaps was) to guarantee that alternatives to std floating-point functions with unspecified precision were available, in the interest of cross-platform determinism. Since libm is no_std-compatible, I think this extension of that is pretty reasonable, but this is a break from its original purpose.

That makes sense, thanks for clarifying the original intent of the module for me! I think to highlight this change in intent I'll update the module documentation to say there are two reasons you'd want to use std_ops: determinism, and no_std compatibility.

I have concerns about the ongoing maintenance burden of these changes. For example, we lint against std math operations with unspecified precision for the aforementioned determinism reasons; right now, as far as I'm aware, this doesn't extend to the new methods added, so I'm not sure if breaks to no_std compatibility are actually discoverable.

I'll look into adding these new methods to the lint as well, with some documentation explaining that it's for no_std compatibility. It's not included in this PR, but my intention is to add no_std testing to the CI as well. Right now it's sufficient to attempt compilation for a no_std target (e.g., x86_64-unknown-none), but this is still an open question (which no_std platforms are people actually using Bevy on in this hypothetical future? Bare metal x86 is an example, but perhaps not realistic)

cubic_splines is feature-gated behind alloc, but plenty of other areas use alloc as well. What is the logic behind this?

My reasoning here was I saw a large amount of alloc usage contained within this module, so I feature gated the whole lot. I will look into it more closely to see if there are some elements which can still be exported without alloc, and if so, move the feature gate inside the module.

- Only referenced new `ops` methods via `ops` namespace (no direct import) - Moved the new `no_std` ops into their own module within `ops` to clarify intent - Updated documentation for `ops` module clarifying it can also be used to assist with `no_std` compatibility - Moved `alloc` feature gate to within `cubic_spline` to clarify which items require its inclusion - Updated tests to improve compatibility with `no_std` environments - Added `f32` methods which are not available in `no_std` to the `clippy.toml` deny list with an explanation around the `no_std` compatibility. Co-Authored-By: Matty <[email protected]> Co-Authored-By: Joona Aalto <[email protected]>

bushrat011899 · 2024-10-10T22:19:39Z

Added to the 0.16 milestone to indicate this is not intended to be merged during the 0.15 release cycle.

Also fixed issues with `serialize` being enabled without `alloc`

Jondolf

This does increase maintenance burden somewhat / make some things slightly uglier, but I would say it's worth it for no_std support, and to move the no_std efforts as a whole forward. Changes look good to me, didn't notice anything wrong.

crates/bevy_math/src/bounding/raycast2d.rs

mweatherley

Just one thing that I found kind of confusing at first glance.

My main concern with this is just making sure that we have enough tools (in CI or otherwise) to maintain this matrix of features without breaking compilation in some combination.

crates/bevy_math/src/ops.rs

Reduces line-count, and generally preferred.

Idea is to provide a standard set of names for maths functions which are normally provided by `std::f32` but might not be on `no_std` platforms.

bushrat011899 · 2024-11-30T21:09:47Z

My main concern with this is just making sure that we have enough tools (in CI or otherwise) to maintain this matrix of features without breaking compilation in some combination.

Agreed. I've added bevy_math to compile-check-no-std in CI, so the specific feature combination of --no-default-features and --feature libm will be checked, along with the standard set of features in the rest of CI. I do agree that there is a flag combinatorics problem here that no_std exacerbates.

# Objective - Contributes to #15460 ## Solution - Added two new features, `std` (default) and `alloc`, gating `std` and `alloc` behind them respectively. - Added missing `f32` functions to `std_ops` as required. These `f32` methods have been added to the `clippy.toml` deny list to aid in `no_std` development. ## Testing - CI - `cargo clippy -p bevy_math --no-default-features --features libm --target "x86_64-unknown-none"` - `cargo test -p bevy_math --no-default-features --features libm` - `cargo test -p bevy_math --no-default-features --features "libm, alloc"` - `cargo test -p bevy_math --no-default-features --features "libm, alloc, std"` - `cargo test -p bevy_math --no-default-features --features "std"` ## Notes The following items require the `alloc` feature to be enabled: - `CubicBSpline` - `CubicBezier` - `CubicCardinalSpline` - `CubicCurve` - `CubicGenerator` - `CubicHermite` - `CubicNurbs` - `CyclicCubicGenerator` - `RationalCurve` - `RationalGenerator` - `BoxedPolygon` - `BoxedPolyline2d` - `BoxedPolyline3d` - `SampleCurve` - `SampleAutoCurve` - `UnevenSampleCurve` - `UnevenSampleAutoCurve` - `EvenCore` - `UnevenCore` - `ChunkedUnevenCore` This requirement could be relaxed in certain cases, but I had erred on the side of gating rather than modifying. Since `no_std` is a new set of platforms we are adding support to, and the `alloc` feature is enabled by default, this is not a breaking change. --------- Co-authored-by: Benjamin Brienen <[email protected]> Co-authored-by: Matty <[email protected]> Co-authored-by: Joona Aalto <[email protected]>

# Objective - Contributes to bevyengine#15460 ## Solution - Added two new features, `std` (default) and `alloc`, gating `std` and `alloc` behind them respectively. - Added missing `f32` functions to `std_ops` as required. These `f32` methods have been added to the `clippy.toml` deny list to aid in `no_std` development. ## Testing - CI - `cargo clippy -p bevy_math --no-default-features --features libm --target "x86_64-unknown-none"` - `cargo test -p bevy_math --no-default-features --features libm` - `cargo test -p bevy_math --no-default-features --features "libm, alloc"` - `cargo test -p bevy_math --no-default-features --features "libm, alloc, std"` - `cargo test -p bevy_math --no-default-features --features "std"` ## Notes The following items require the `alloc` feature to be enabled: - `CubicBSpline` - `CubicBezier` - `CubicCardinalSpline` - `CubicCurve` - `CubicGenerator` - `CubicHermite` - `CubicNurbs` - `CyclicCubicGenerator` - `RationalCurve` - `RationalGenerator` - `BoxedPolygon` - `BoxedPolyline2d` - `BoxedPolyline3d` - `SampleCurve` - `SampleAutoCurve` - `UnevenSampleCurve` - `UnevenSampleAutoCurve` - `EvenCore` - `UnevenCore` - `ChunkedUnevenCore` This requirement could be relaxed in certain cases, but I had erred on the side of gating rather than modifying. Since `no_std` is a new set of platforms we are adding support to, and the `alloc` feature is enabled by default, this is not a breaking change. --------- Co-authored-by: Benjamin Brienen <[email protected]> Co-authored-by: Matty <[email protected]> Co-authored-by: Joona Aalto <[email protected]>

Add no_std Support to bevy_math

b7164b6

bushrat011899 mentioned this pull request Oct 10, 2024

Tracking Issue: MVP no_std Bevy #15460

Closed

Run taplo fmt

b854d75

BenjaminBrienen suggested changes Oct 10, 2024

View reviewed changes

crates/bevy_math/Cargo.toml Show resolved Hide resolved

crates/bevy_math/src/ops.rs Outdated Show resolved Hide resolved

Addressing Review Comments

02d3c20

Co-Authored-By: Benjamin Brienen <[email protected]>

BenjaminBrienen approved these changes Oct 10, 2024

View reviewed changes

Fixed issue with curve feature and rem_euclid

bf2e366

bushrat011899 commented Oct 10, 2024

View reviewed changes

crates/bevy_math/src/ops.rs Show resolved Hide resolved

mweatherley suggested changes Oct 10, 2024

View reviewed changes

mweatherley added X-Contentious There are nontrivial implications that should be thought through and removed X-Uncontroversial This work is generally agreed upon labels Oct 10, 2024

bushrat011899 added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 10, 2024

bushrat011899 added this to the 0.16 milestone Oct 10, 2024

bushrat011899 added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Oct 10, 2024

bushrat011899 added 2 commits October 11, 2024 20:12

Allow parts of the curve feature to be accessible without alloc

9ee2c75

Also fixed issues with `serialize` being enabled without `alloc`

Clippy

0da406f

BenjaminBrienen approved these changes Oct 12, 2024

View reviewed changes

Jondolf approved these changes Oct 12, 2024

View reviewed changes

bushrat011899 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

mweatherley self-requested a review October 13, 2024 14:22

tim-blackbird approved these changes Nov 30, 2024

View reviewed changes

crates/bevy_math/src/bounding/raycast2d.rs Outdated Show resolved Hide resolved

mweatherley approved these changes Nov 30, 2024

View reviewed changes

crates/bevy_math/src/ops.rs Show resolved Hide resolved

bushrat011899 added 4 commits December 1, 2024 07:40

Inline assert format args

1c187fd

Reduces line-count, and generally preferred.

Added documentation explaining the purpose of std_ops_for_no_std

4acd030

Idea is to provide a standard set of names for maths functions which are normally provided by `std::f32` but might not be on `no_std` platforms.

Merge remote-tracking branch 'upstream/main' into NoSTDBevyMath

4eb501a

Add bevy_math to compile-check-no-std

7e6777c

Fix typo

17b8626

alice-i-cecile added this pull request to the merge queue Dec 3, 2024

Merged via the queue into bevyengine:main with commit a8b9c94 Dec 3, 2024

TimJentzsch added the O-Embedded Weird hardware and no_std platforms label Dec 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `no_std` Support to `bevy_math`#15810

Add `no_std` Support to `bevy_math`#15810
alice-i-cecile merged 12 commits intobevyengine:mainfrom
bushrat011899:NoSTDBevyMath

bushrat011899 commented Oct 10, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mweatherley left a comment

Uh oh!

bushrat011899 commented Oct 10, 2024

Uh oh!

bushrat011899 commented Oct 10, 2024

Uh oh!

Jondolf left a comment

Uh oh!

Uh oh!

mweatherley left a comment

Uh oh!

Uh oh!

bushrat011899 commented Nov 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

bushrat011899 commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Testing

Notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mweatherley left a comment

Choose a reason for hiding this comment

Uh oh!

bushrat011899 commented Oct 10, 2024

Uh oh!

bushrat011899 commented Oct 10, 2024

Uh oh!

Jondolf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mweatherley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bushrat011899 commented Nov 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

bushrat011899 commented Oct 10, 2024 •

edited

Loading