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

Cubic splines overhaul #10701

Merged

Conversation

JohnTheCoolingFan
Copy link
Contributor

@JohnTheCoolingFan JohnTheCoolingFan commented Nov 22, 2023

Objective

Improve the bevy::math::cubic_splines module by making it more flexible and adding new curve types.
Closes #10220

Solution

Added new spline types and improved existing


Changelog

Added

  • CubicNurbs rational cubic curve generator, allows setting the knot vector and weights associated with every point
  • LinearSpline curve generator, allows generating a linearly interpolated curve segment
  • Ability to push additional cubic segments to CubicCurve
  • IntoIterator and Extend implementations for CubicCurve

Changed

  • Point trait has been implemented for more types: Quat and Vec4.
  • CubicCurve::coefficients was moved to CubicSegment::coefficients because the function returns CubicSegment, so it seems logical to be associated with CubicSegment instead. The method is still not public.

Fixed

  • CubicBSpline::new was referencing Cardinal spline instead of B-Spline

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Math Fundamental domain-agnostic mathematical operations labels Nov 22, 2023
@JohnTheCoolingFan JohnTheCoolingFan changed the title Added Nurbs spline Cubic splines overhaul Nov 22, 2023
@JohnTheCoolingFan
Copy link
Contributor Author

The only thing left is documentation for the new types and methods, I think

@JohnTheCoolingFan
Copy link
Contributor Author

Ok, the Point trait is no more a blanket implementation and more types implement it, specifically Vec4 and Quat. There are reasons why I didn't implement it on more types:

  1. glam's MatX do not implement Div<f32>, only Mut<f32>, which I reported here: impl Div<f32>/Div<f64> for [D]MatX bitshifter/glam-rs#476
  2. Color from bevy_color and other color types also do not implement all the math operation traits required. I'm not sure whether implementing them wouldn't produce anything undesirable, so I would like to get more opinions, and implementing this would probably be better as a different PR
  3. f64 and types based on that don't interoperate with f32. So unless Point gets a generic argument or associated type specifying the type (f32, f64, potentially even more if it makes sense). Same for integers.

@alice-i-cecile
Copy link
Member

Like I said on Discord, I'd like to leave the Color implementations for follow-up :) Doing a final review pass now.

@alice-i-cecile alice-i-cecile added the M-Needs-Release-Note Work that should be called out in the blog due to impact label Feb 28, 2024
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'm happy with this now. Excellent work, and thanks for your patience. I'd like to see more learning materials / examples for this area of the engine, but that can definitely be done in follow-up.

@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 Feb 28, 2024
crates/bevy_math/src/cubic_splines.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/cubic_splines.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/cubic_splines.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/cubic_splines.rs Outdated Show resolved Hide resolved
Co-authored-by: Joona Aalto <[email protected]>
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 28, 2024
Merged via the queue into bevyengine:main with commit a543536 Feb 28, 2024
27 of 29 checks passed
JohnTheCoolingFan added a commit to JohnTheCoolingFan/bevy-website that referenced this pull request Jun 4, 2024
Initial release notes for the Cubic Splines overhaul PR.
JohnTheCoolingFan added a commit to JohnTheCoolingFan/bevy-website that referenced this pull request Jun 4, 2024
Initial release notes for the Cubic Splines overhaul PR.
@alice-i-cecile alice-i-cecile removed the M-Needs-Release-Note Work that should be called out in the blog due to impact label Jun 4, 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-Feature A new feature, making something new possible D-Complex Quite challenging from either a design or technical perspective. Ask for help! 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.

NURBS spline for generating CubicCurve
7 participants