Make cardinal splines include endpoints (#12574)

# Objective

- Fixes #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 #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.
This commit is contained in:
Matty 2024-03-21 14:58:51 -04:00 committed by GitHub
parent a5d0265554
commit 93c17d105a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -2,6 +2,7 @@
use std::{
fmt::Debug,
iter::once,
ops::{Add, Div, Mul, Sub},
};
@ -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.
///
@ -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
///
@ -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 }
@ -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]