Skip to content

@remotion/paths: Fix geometry of chained T commands after a non-quadratic command - #12821

Merged
JonnyBurger merged 1 commit into
remotion-dev:mainfrom
b84330808:claude/funny-galileo-b8glbo
Oct 30, 2026
Merged

JonnyBurger merged 1 commit into
remotion-dev:mainfrom
b84330808:claude/funny-galileo-b8glbo

Conversation

@b84330808

Copy link
Copy Markdown
Contributor

Per the SVG spec, a T/t that does not follow a Q/q/T/t uses the current point as its control point, and the next T reflects that control point. Both code paths in @remotion/paths got the second T wrong:

  • construct() (used by getLength(), getPointAtLength(), getTangentAtLength(), getInstructionIndexAtLength(), evolvePath()) reflected a stale control point instead: the one from an earlier Q, or [0, 0]. Because that point is absolute, the result also changed when the path was translated.
  • iterateOverSegments() (used by reduceInstructions(), and through it getBoundingBox(), cutPath(), reversePath(), scalePath(), warpPath(), interpolatePath()) left the tracked control point null after H/L/V/A/Z/M, which was then read as 0. After C/S, it reflected the cubic's second control point.
Path Chromium getTotalLength() getLength() before after
M 0 0 H 10 T 20 10 T 30 0 45.925 44.206 45.925
same after translatePath(path, 100, 100) 45.925 160.758 45.925
M 0 0 C 0 10 10 10 10 0 T 20 10 T 30 0 55.925 54.206 55.925
M 0 0 Q 5 5 10 0 L 20 0 T 30 10 T 40 0 57.403 60.765 57.403

I also compared 2000 random paths mixing every command against Chromium's getTotalLength(). Before: getLength() disagreed on 300 and getLength(serializeInstructions(reduceInstructions(...))) on 296. After: 0 and 0.

The added tests compare each T chain with the equivalent explicit Q path.

I used Claude Code to write this change.

…uadratic command

Per the SVG spec, a `T`/`t` that does not follow a quadratic command uses
the current point as its control point, and the next `T` reflects that.

- `getLength()` and the other `construct()`-based helpers reflected a
  stale control point (from an earlier `Q`, or the origin) instead, so the
  result also changed when the path was translated.
- `reduceInstructions()` left the tracked control point `null` after
  `H`/`L`/`V`/`A`/`Z`/`M`, and reflected the cubic's second control point
  after `C`/`S`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018KWRbA1vtGSbDKWjrU4XXq
@vercel

vercel Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
bugs Ready Ready Preview Oct 30, 2026 9:04am UTC
remotion Ready Ready Preview Oct 30, 2026 9:04am UTC

Request Review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • construct.ts — after processing a T/t, prev_point is now set to the current point when the predecessor is not Q/q/T/t, instead of reflecting a stale control point. This matches the spec and makes chained T lengths translation-invariant.
  • iterate.ts — T segment state now only reflects the tracked control point when the previous segment is actually Q or T; otherwise it sets the control point to the current point. Previously a T following C/S reflected cp2x/cpx, corrupting the next T.
  • get-length.test.ts — new test asserting a T chain after H equals the explicit Q equivalent, including a translated copy.
  • reduce-instructions.test.ts — new test asserting T chains after H and after C reduce to the same instructions as the explicit Q equivalents.

I verified the reflection logic in both code paths against the SVG spec by tracing H 10 T 20 10 T 30 0 and C ... T 20 10 T 30 0, and ran the full @remotion/paths suite: 104 pass, 0 fail.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash | 𝕏

@JonnyBurger
JonnyBurger merged commit 90dac5b into remotion-dev:main Oct 30, 2026
20 checks passed
@JonnyBurger

JonnyBurger commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Thanks!

This branch was successfully deployed

2 active deployments
Preview – remotion — 0d8564a9 Deployed Oct 30, 2026 by vercel[bot]
Preview – bugs — 0d8564a9 Deployed Oct 30, 2026 by vercel[bot]
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.

2 participants