Skip to content

Commit

Permalink
Make cardinal splines include endpoints (bevyengine#12574)
Browse files Browse the repository at this point in the history
# Objective

- Fixes bevyengine#12570 

## Solution

Previously, cardinal splines constructed by `CubicCardinalSpline` would
leave out their endpoints when constructing the cubic curve segments
connecting their points. (See the linked issue for details.)

Now, cardinal splines include the endpoints. For instance, the provided
usage example
```rust
let points = [
    vec2(-1.0, -20.0),
    vec2(3.0, 2.0),
    vec2(5.0, 3.0),
    vec2(9.0, 8.0),
];
let cardinal = CubicCardinalSpline::new(0.3, points).to_curve();
let positions: Vec<_> = cardinal.iter_positions(100).collect();
```
will actually produce a spline that connects all four of these points
instead of just the middle two "interior" points.

Internally, this is achieved by duplicating the endpoints of the vector
of control points before performing the construction of the associated
`CubicCurve`. This amounts to specifying that the tangents at the
endpoints `P_0` and `P_n` (say) should be parallel to `P_1 - P_0` and
`P_n - P_{n-1}`.

---

## Migration Guide

Any users relying on the old behavior of `CubicCardinalSpline` will have
to truncate any parametrizations they used in order to access a curve
identical to the one they had previously. This would be done by chopping
off a unit-distance segment from each end of the parametrizing interval.
For instance, if a user's existing code looks as follows
```rust
fn interpolate(t: f32) -> Vec2 {
    let points = [
        vec2(-1.0, -20.0),
        vec2(3.0, 2.0),
        vec2(5.0, 3.0),
        vec2(9.0, 8.0),
    ];
    let my_curve = CubicCardinalSpline::new(0.3, points).to_curve();
    my_curve.position(t)
}
```

then in order to obtain similar behavior, `t` will need to be shifted up
by 1, since the output of `CubicCardinalSpline::to_curve` has introduced
a new segment in the interval [0,1], displacing the old segment from
[0,1] to [1,2]:

```rust
fn interpolate(t: f32) -> Vec2 {
    let points = [
        vec2(-1.0, -20.0),
        vec2(3.0, 2.0),
        vec2(5.0, 3.0),
        vec2(9.0, 8.0),
    ];
    let my_curve = CubicCardinalSpline::new(0.3, points).to_curve();
    my_curve.position(t+1)
}
```

(Note that this does not provide identical output for values of `t`
outside of the interval [0,1].)

On the other hand, any user who was specifying additional endpoint
tangents simply to get the curve to pass through the right points (i.e.
not requiring exactly the same output) can simply omit the endpoints
that were being supplied only for control purposes.

---

## Discussion

### Design considerations

This is one of the two approaches outlined in bevyengine#12570 — in this PR, we
are basically declaring that the docs are right and the implementation
was flawed.

One semi-interesting question is how the endpoint tangents actually
ought to be defined when we include them, and another option considered
was mirroring the control points adjacent to the endpoints instead of
duplicating them, which would have had the advantage that the expected
length of the corresponding difference should be more similar to that of
the other difference-tangents, provided that the points are equally
spaced.

In this PR, the duplication method (which produces smaller tangents) was
chosen for a couple reasons:
- It seems to be more standard
- It is exceptionally simple to implement
- I was a little concerned that the aforementioned alternative would
result in some over-extrapolation

### An annoyance

If you look at the code, you'll see I was unable to find a satisfactory
way of doing this without allocating a new vector. This doesn't seem
like a big problem given the context, but it does bother me. In
particular, if there is some easy parallel to `slice::windows` for
iterators that doesn't pull in an external dependency, I would love to
know about it.
  • Loading branch information
mweatherley authored Mar 21, 2024
1 parent a5d0265 commit 93c17d1
Showing 1 changed file with 57 additions and 6 deletions.
63 changes: 57 additions & 6 deletions crates/bevy_math/src/cubic_splines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use std::{
fmt::Debug,
iter::once,
ops::{Add, Div, Mul, Sub},
};

Expand Down Expand Up @@ -172,7 +173,8 @@ impl<P: Point> CubicGenerator<P> for CubicHermite<P> {
}

/// A spline interpolated continuously across the nearest four control points, with the position of
/// the curve specified at every control point and the tangents computed automatically.
/// the curve specified at every control point and the tangents computed automatically. The associated [`CubicCurve`]
/// has one segment between each pair of adjacent control points.
///
/// **Note** the Catmull-Rom spline is a special case of Cardinal spline where the tension is 0.5.
///
Expand All @@ -183,8 +185,8 @@ impl<P: Point> CubicGenerator<P> for CubicHermite<P> {
/// Tangents are automatically computed based on the positions of control points.
///
/// ### Continuity
/// The curve is at minimum C0 continuous, meaning it has no holes or jumps. It is also C1, meaning the
/// tangent vector has no sudden jumps.
/// The curve is at minimum C1, meaning that it is continuous (it has no holes or jumps), and its tangent
/// vector is also well-defined everywhere, without sudden jumps.
///
/// ### Usage
///
Expand Down Expand Up @@ -232,10 +234,28 @@ impl<P: Point> CubicGenerator<P> for CubicCardinalSpline<P> {
[-s, 2. - s, s - 2., s],
];

let segments = self
.control_points
let length = self.control_points.len();

// Early return to avoid accessing an invalid index
if length < 2 {
return CubicCurve { segments: vec![] };
}

// Extend the list of control points by mirroring the last second-to-last control points on each end;
// this allows tangents for the endpoints to be provided, and the overall effect is that the tangent
// at an endpoint is proportional to twice the vector between it and its adjacent control point.
//
// The expression used here is P_{-1} := P_0 - (P_1 - P_0) = 2P_0 - P_1. (Analogously at the other end.)
let mirrored_first = self.control_points[0] * 2. - self.control_points[1];
let mirrored_last = self.control_points[length - 1] * 2. - self.control_points[length - 2];
let extended_control_points = once(&mirrored_first)
.chain(self.control_points.iter())
.chain(once(&mirrored_last))
.collect::<Vec<_>>();

let segments = extended_control_points
.windows(4)
.map(|p| CubicSegment::coefficients([p[0], p[1], p[2], p[3]], char_matrix))
.map(|p| CubicSegment::coefficients([*p[0], *p[1], *p[2], *p[3]], char_matrix))
.collect();

CubicCurve { segments }
Expand Down Expand Up @@ -1275,6 +1295,37 @@ mod tests {
assert_eq!(bezier.ease(1.0), 1.0);
}

/// Test that a simple cardinal spline passes through all of its control points with
/// the correct tangents.
#[test]
fn cardinal_control_pts() {
use super::CubicCardinalSpline;

let tension = 0.2;
let [p0, p1, p2, p3] = [vec2(-1., -2.), vec2(0., 1.), vec2(1., 2.), vec2(-2., 1.)];
let curve = CubicCardinalSpline::new(tension, [p0, p1, p2, p3]).to_curve();

// Positions at segment endpoints
assert!(curve.position(0.).abs_diff_eq(p0, FLOAT_EQ));
assert!(curve.position(1.).abs_diff_eq(p1, FLOAT_EQ));
assert!(curve.position(2.).abs_diff_eq(p2, FLOAT_EQ));
assert!(curve.position(3.).abs_diff_eq(p3, FLOAT_EQ));

// Tangents at segment endpoints
assert!(curve
.velocity(0.)
.abs_diff_eq((p1 - p0) * tension * 2., FLOAT_EQ));
assert!(curve
.velocity(1.)
.abs_diff_eq((p2 - p0) * tension, FLOAT_EQ));
assert!(curve
.velocity(2.)
.abs_diff_eq((p3 - p1) * tension, FLOAT_EQ));
assert!(curve
.velocity(3.)
.abs_diff_eq((p3 - p2) * tension * 2., FLOAT_EQ));
}

/// Test that [`RationalCurve`] properly generalizes [`CubicCurve`]. A Cubic upgraded to a rational
/// should produce pretty much the same output.
#[test]
Expand Down

0 comments on commit 93c17d1

Please sign in to comment.