Skip to content

Fix Catmull curve tangent calculation and segment indexing - #127

Closed
cmyui wants to merge 2 commits into
llllllllll:masterfrom
cmyui:fix/catmull-tangent-calculation
Closed

cmyui wants to merge 2 commits into
llllllllll:masterfrom
cmyui:fix/catmull-tangent-calculation

Conversation

@cmyui

@cmyui cmyui commented Mar 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes three bugs in the Catmull curve implementation that caused it to produce incorrect curve shapes compared to both osu-stable and osu-lazer:

  • np.roll without axis=0: Flattens the 2D points array before rolling, scrambling x/y coordinates when computing tangents. All Catmull curves with 3+ points had incorrect tangent values.
  • Swapped roll directions: np.roll(points, 1) (shift right) was assigned to p_aheads instead of p_behinds, producing negated tangents.
  • Wrong segment at t=0: math.ceil(0 * N) - 1 = -1 indexed the last curve segment instead of the first. Also, the global parameter t was used as the Hermite parameter s instead of being remapped to the local segment range [0, 1].
  • Wrong endpoint boundary condition: The last control point's tangent was clamped (duplicated), but osu-stable and osu-lazer both extrapolate (v4 = v3 + (v3 - v2)), producing a tangent twice as large.

Verified against osu-stable (SliderOsu.cs + Vector2.CatmullRom) and osu-lazer (PathApproximator.CatmullToPiecewiseLinear + catmullFindPoint), which use the same 4-point formula and boundary logic.

Test plan

  • Added slider/tests/test_curve.py with tests that verify against a reference implementation of osu's exact Catmull-Rom formula and boundary handling
  • Tests cover 1-point (degenerate), 2-point, 3-point, and 5-point cases
  • Each test evaluates multiple t values per segment and compares against osu's 4-point formula
  • All 27 tests pass (20 existing + 7 new)

🤖 Generated with Claude Code

Three bugs in the Catmull curve implementation:

1. np.roll without axis=0 flattens the 2D points array before rolling,
   scrambling x/y coordinates when computing tangents.

2. Roll directions were swapped relative to variable names: np.roll(points, 1)
   shifts right (giving predecessors), but was assigned to p_aheads instead
   of p_behinds, producing negated tangents.

3. curve(0) returned the wrong position: math.ceil(0 * N) - 1 = -1 indexed
   the last segment instead of the first. Also, the global parameter t was
   used directly as the Hermite parameter s instead of being remapped to
   the local segment range [0, 1].

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cmyui cmyui closed this Mar 1, 2026
…clamp

Tests now verify against a reference implementation of osu's 4-point
Catmull-Rom formula with the exact boundary logic from both osu-stable
(SliderOsu.cs) and osu-lazer (PathApproximator.cs).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cmyui cmyui reopened this Mar 1, 2026
@cmyui cmyui closed this Mar 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant