search

ecurtiss / CatRom

Creates Catmull-Rom splines for Roblox
https://ecurtiss.github.io/CatRom/
Mozilla Public License 2.0
47 stars 11 forks source link

Certain values of tension cause the spline to not be immersed #27

Open ecurtiss opened 4 months ago

ecurtiss commented 4 months ago

An immersed curve is a curve whose speed is always positive. CatRom assumes that its splines are immersed anywhere that Segment:SolveVelocity and Segment:SolveTangent are used internally (and really, in my head, I assume this to always be true). However, a spline with tension 1 has a speed of zero at times t = 0 and t = 1. Moreover, completely straight splines are susceptible to zeros in speed when the spline backtracks on itself, which happens with tensions outside of [0, 1].

This leaves us with three solutions:

  1. Gracefully handle non-immersed curves
  2. Disallow certain values of tension
  3. Do nothing and warn the user that bad things could happen when tension is outside of [0, 1) (seems like a footgun waiting to happen)

For option 1, a graceful way to handle non-immersion with Segment:SolveTangent is to introduce an internal "safe" version that computes a discrete approximation of the tangent. There's not a great way to make Segment:SolveVelocity safe, but given how it's used, I don't think it should be. However, "fixing" these problems feels wrong since the parametrization should always be immersed to begin with. The only exception would be unit-speed parametrizations, which are by definition immersed. But if we choose to gracefully handle unit-speed curves, then I suppose we should also gracefully handle non-unit-speed cases as well.

Bookkeeping

Note that we use Segment:SolveVelocity in

  1. Segment:SolveTangent
  2. Segment:SolveNormal
  3. Segment:SolveCurvature
  4. Segment:SolveTorsion
  5. Segment:SolveCFrameLookAlong
  6. getLengthIntegrand
  7. Segment:_ReparametrizeNewtonBisection

and we use Segment:SolveTangent in

  1. Segment:SolveNormal (on Vector2 splines)
  2. Segment:SolveBinormal
  3. Segment:SolveCFrameFrenet
  4. Segment:SolveCFrameRMF
  5. Segment:PrecomputeRMFs
  6. Catrom:PrecomputeRMFs
ecurtiss commented 4 months ago

I would bet that >90% of users have never changed alpha or tension. Of the 10% who have, I would bet that >90% have kept within the suggested bounds of [0, 1]. Therefore, I don't think it's a big deal to eliminate values of tension outside of [0, 1]. And when tension is exactly 1, the spline is just a chain of straight lines, so there is no point in using CatRom. Therefore, I think it's not unreasonable to take option 2 and just restrict tension to [0, 1). If users really want straight lines, they can set tension to 0.999.

In addition to the above heuristic argument, I'll add that

  1. I don't want the extra code complexity of gracefully handling non-immersion.
  2. I think gracefully handling non-immersion in the non-unit speed case is disingenuous.
  3. If I simply do nothing, I don't want users to trip on this landmine and assume CatRom is broken.