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

[Merged by Bors] - Add generic cubic splines to bevy_math #7683

Closed
wants to merge 7 commits into from

Conversation

aevyrie
Copy link
Member

@aevyrie aevyrie commented Feb 15, 2023

Objective

  • Make cubic splines more flexible and more performant
  • Remove the existing spline implementation that is generic over many degrees
    • This is a potential performance footgun and adds type complexity for negligible gain.
  • Add implementations of:
    • Bezier splines
    • Cardinal splines (inc. Catmull-Rom)
    • B-Splines
    • Hermite splines
2023-02-27.22-10-31.mp4
2023-02-27.22-11-29.mp4
2023-02-27.22-24-01.mp4

Solution

  • Implements the concept that splines are curve generators (e.g. https://youtu.be/jvPPXbo87ds?t=3488) via the CubicGenerator trait.
  • Common splines are bespoke data types that implement this trait. This gives us flexibility to add custom spline-specific methods on these types, while ultimately all generating a CubicCurve.
  • All splines generate CubicCurves, which are a chain of precomputed polynomial coefficients. This means that all splines have the same evaluation cost, as the calculations for determining position, velocity, and acceleration are all identical. In addition, CubicCurves are simply a list of CubicSegments, which are evaluated from t=0 to t=1. This also means cubic splines of different type can be chained together, as ultimately they all are simply a collection of CubicSegments.
  • Because easing is an operation on a singe segment of a Bezier curve, we can simply implement easing on Beziers that use the Vec2 type for points. Higher level crates such as bevy_ui can wrap this in a more ergonomic interface as needed.

Performance

Measured on a desktop i5 8600K (6-year-old CPU):

  • easing: 2.7x faster (19ns)
  • cubic vec2 position sample: 1.5x faster (1.8ns)
  • cubic vec3 position sample: 1.5x faster (2.6ns)
  • cubic vec3a position sample: 1.9x faster (1.4ns)

On a laptop i7 11800H:

  • easing: 16ns
  • cubic vec2 position sample: 1.6ns
  • cubic vec3 position sample: 2.3ns
  • cubic vec3a position sample: 1.2ns

Changelog

  • Added a generic cubic curve trait, and implementation for Cardinal splines (including Catmull-Rom), B-Splines, Beziers, and Hermite Splines. 2D cubic curve segments also implement easing functionality for animation.

@alice-i-cecile alice-i-cecile added C-Enhancement A new feature A-Math Fundamental domain-agnostic mathematical operations labels Feb 15, 2023
@aevyrie
Copy link
Member Author

aevyrie commented Feb 15, 2023

@alice-i-cecile based on the work in #7653, I'm wondering if the sampling, position, velocity, and acceleration should be part of a spline trait? Also means to_positions and friends could be implemented for all splines automatically, to remove the boilerplate.

@alice-i-cecile
Copy link
Member

@alice-i-cecile based on the work in #7653, I'm wondering if the sampling, position, velocity, and acceleration should be part of a spline trait? Also means to_positions and friends could be implemented for all splines automatically, to remove the boilerplate.

Yes, this is my preference. There seems to be a general class of objects here that it would be very nice to work generically over.

@aevyrie aevyrie changed the title Add Catmull-Rom splines to bevy_math Add generic cubic splines to bevy_math Feb 25, 2023
Copy link
Contributor

@maniwani maniwani left a comment

Choose a reason for hiding this comment

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

I like the generalization to CubicCurve and CubicSegment. My only suggestion is to consider linking to an explanation of continuity wherever you mention it in the docs, maybe this one. Not everyone will know what C_k means.

Comment on lines +310 to +311
let [a, b, c, d] = self.coeff;
a + b * t + c * t.powi(2) + d * t.powi(3)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let [a, b, c, d] = self.coeff;
a + b * t + c * t.powi(2) + d * t.powi(3)
let [a, b, c, d] = self.coeff;
a + t * (b + t * (c + t * d))

You may be able to save a tiny bit more time by doing it like this. https://rust.godbolt.org/z/TvEMqY8Ps

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for these suggestions. I'll investigate this optimization and link to wikipedia in the docs.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I like the generalization too. This is well-made and documented, and Blazing Fast.

@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 Mar 3, 2023
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Mar 3, 2023
# Objective

- Make cubic splines more flexible and more performant
- Remove the existing spline implementation that is generic over many degrees
  - This is a potential performance footgun and adds type complexity for negligible gain.
- Add implementations of:
  - Bezier splines
  - Cardinal splines (inc. Catmull-Rom)
  - B-Splines
  - Hermite splines

https://user-images.githubusercontent.com/2632925/221780519-495d1b20-ab46-45b4-92a3-32c46da66034.mp4


https://user-images.githubusercontent.com/2632925/221780524-2b154016-699f-404f-9c18-02092f589b04.mp4


https://user-images.githubusercontent.com/2632925/221780525-f934f99d-9ad4-4999-bae2-75d675f5644f.mp4


## Solution

- Implements the concept that splines are curve generators (e.g. https://youtu.be/jvPPXbo87ds?t=3488) via the `CubicGenerator` trait.
- Common splines are bespoke data types that implement this trait. This gives us flexibility to add custom spline-specific methods on these types, while ultimately all generating a `CubicCurve`.
- All splines generate `CubicCurve`s, which are a chain of precomputed polynomial coefficients. This means that all splines have the same evaluation cost, as the calculations for determining position, velocity, and acceleration are all identical. In addition, `CubicCurve`s are simply a list of `CubicSegment`s, which are evaluated from t=0 to t=1. This also means cubic splines of different type can be chained together, as ultimately they all are simply a collection of `CubicSegment`s.
- Because easing is an operation on a singe segment of a Bezier curve, we can simply implement easing on `Beziers` that use the `Vec2` type for points. Higher level crates such as `bevy_ui` can wrap this in a more ergonomic interface as needed.

### Performance
Measured on a desktop i5 8600K (6-year-old CPU):
- easing: 2.7x faster (19ns)
- cubic vec2 position sample: 1.5x faster (1.8ns)
- cubic vec3 position sample: 1.5x faster (2.6ns)
- cubic vec3a position sample: 1.9x faster (1.4ns)

On a laptop i7 11800H:
- easing: 16ns
- cubic vec2 position sample: 1.6ns
- cubic vec3 position sample: 2.3ns
- cubic vec3a position sample: 1.2ns

---

## Changelog

- Added a generic cubic curve trait, and implementation for Cardinal splines (including Catmull-Rom), B-Splines, Beziers, and Hermite Splines. 2D cubic curve segments also implement easing functionality for animation.
@bors bors bot changed the title Add generic cubic splines to bevy_math [Merged by Bors] - Add generic cubic splines to bevy_math Mar 3, 2023
@bors bors bot closed this Mar 3, 2023
@james7132 james7132 modified the milestones: 0.11, 0.10 Mar 4, 2023
Shfty pushed a commit to shfty-rust/bevy that referenced this pull request Mar 19, 2023
# Objective

- Make cubic splines more flexible and more performant
- Remove the existing spline implementation that is generic over many degrees
  - This is a potential performance footgun and adds type complexity for negligible gain.
- Add implementations of:
  - Bezier splines
  - Cardinal splines (inc. Catmull-Rom)
  - B-Splines
  - Hermite splines

https://user-images.githubusercontent.com/2632925/221780519-495d1b20-ab46-45b4-92a3-32c46da66034.mp4


https://user-images.githubusercontent.com/2632925/221780524-2b154016-699f-404f-9c18-02092f589b04.mp4


https://user-images.githubusercontent.com/2632925/221780525-f934f99d-9ad4-4999-bae2-75d675f5644f.mp4


## Solution

- Implements the concept that splines are curve generators (e.g. https://youtu.be/jvPPXbo87ds?t=3488) via the `CubicGenerator` trait.
- Common splines are bespoke data types that implement this trait. This gives us flexibility to add custom spline-specific methods on these types, while ultimately all generating a `CubicCurve`.
- All splines generate `CubicCurve`s, which are a chain of precomputed polynomial coefficients. This means that all splines have the same evaluation cost, as the calculations for determining position, velocity, and acceleration are all identical. In addition, `CubicCurve`s are simply a list of `CubicSegment`s, which are evaluated from t=0 to t=1. This also means cubic splines of different type can be chained together, as ultimately they all are simply a collection of `CubicSegment`s.
- Because easing is an operation on a singe segment of a Bezier curve, we can simply implement easing on `Beziers` that use the `Vec2` type for points. Higher level crates such as `bevy_ui` can wrap this in a more ergonomic interface as needed.

### Performance
Measured on a desktop i5 8600K (6-year-old CPU):
- easing: 2.7x faster (19ns)
- cubic vec2 position sample: 1.5x faster (1.8ns)
- cubic vec3 position sample: 1.5x faster (2.6ns)
- cubic vec3a position sample: 1.9x faster (1.4ns)

On a laptop i7 11800H:
- easing: 16ns
- cubic vec2 position sample: 1.6ns
- cubic vec3 position sample: 2.3ns
- cubic vec3a position sample: 1.2ns

---

## Changelog

- Added a generic cubic curve trait, and implementation for Cardinal splines (including Catmull-Rom), B-Splines, Beziers, and Hermite Splines. 2D cubic curve segments also implement easing functionality for animation.
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-Enhancement A new feature 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