fix(video-editor): correct cursor drift after cropping

[SakuraiSatoru]

Summary

Fixes #64. After cropping a video, the synthetic cursor overlay drifted and no longer lined up with the real mouse position in the frame.

Cursor telemetry (cx/cy) is normalized to the full, uncropped video, but PixiCursorOverlay.update() projected those coordinates directly onto the cropped viewport (maskRect) without re-normalizing against the crop rect, so the cursor shifted. The native-cursor path (getCroppedCursorPosition / projectNativeCursorToLocal) already handled this correctly — only the synthetic overlay introduced in #61 was missing the transform.

Changes

• Add a pure helper mapCursorToCroppedViewport() that re-normalizes the cursor position against the crop rect before projecting it onto the viewport, and returns null when the cursor falls outside the visible crop (so it is hidden, matching the native-cursor path).
• PixiCursorOverlay.update() takes an optional cropRegion and uses the helper.
• VideoPlayback.tsx passes the current cropRegion.

Test plan

• New unit tests in cursorRenderer.test.ts (5 cases, including a regression test for [Bug]: Mouse drifting after cropping the video #64).
• tsc --noEmit and biome check pass.
• All video-editor and cursor tests pass (92 tests).

Summary by CodeRabbit

• Bug Fixes
  • Improved cursor alignment during video playback by mapping cursor position to the cropped viewport.
  • Fixed cursor drift so the cursor no longer appears offset during playback.
  • The cursor overlay now hides when the cursor moves outside the visible cropped region.
• Tests
  • Added automated coverage for cursor-to-cropped-viewport mapping, including drift prevention and handling of degenerate/invalid crop areas.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0de90100-385d-4de2-ad27-48bf42c5539f

📥 Commits

Reviewing files that changed from the base of the PR and between 15a1a5a and 94c8bf0.

📒 Files selected for processing (2)

• src/components/video-editor/VideoPlayback.tsx
• src/components/video-editor/videoPlayback/cursorRenderer.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/components/video-editor/videoPlayback/cursorRenderer.ts

---

📝 Walkthrough

Walkthrough

This PR adds crop-aware cursor positioning to the video editor. Cursor coordinates are remapped into cropped viewport pixels, the overlay hides cursors outside the crop, playback passes the active crop region, and tests cover mapping and drift regression behavior.

Changes

Crop-aware Cursor Mapping

Layer / File(s)	Summary
Cursor-to-cropped-viewport mapping and validation src/components/video-editor/videoPlayback/cursorRenderer.ts, src/components/video-editor/videoPlayback/cursorRenderer.test.ts	Adds crop-aware normalized-to-pixel mapping, returns null outside or for degenerate crops, and tests direct mapping, re-normalization, drift regression, and invalid positions.
Pixi overlay integration src/components/video-editor/videoPlayback/cursorRenderer.ts	Updates the overlay API to accept a crop region, remaps cursor positions, and hides the overlay when the cursor is outside the crop.
Playback crop wiring src/components/video-editor/VideoPlayback.tsx	Passes the current crop region and uses the shared full-frame default for native cursor projection and sizing.

Estimated code review effort: 2 (Simple) | ~15 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main fix: cursor drift after video cropping.
Description check	✅ Passed	The description covers the bug, fix, and testing, with only some non-critical template sections left blank.
Linked Issues check	✅ Passed	The changes address #64 by re-normalizing cursor coordinates after cropping and adding regression tests.
Out of Scope Changes check	✅ Passed	The PR appears scoped to the cursor-drift fix and its tests, with no unrelated changes shown.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/components/video-editor/videoPlayback/cursorRenderer.test.ts (1)

50-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Non-null assertion could be avoided.

mapped!.px relies on a non-null assertion right after a toEqual on the previous line already implicitly proved mapped is non-null. Could use expect(mapped).not.toBeNull() first to narrow the type without !, but this is purely stylistic.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/video-editor/videoPlayback/cursorRenderer.test.ts` at line 50,
The test in cursorRenderer.test.ts uses a non-null assertion on mapped after
already asserting its value, so update the cursor mapping test to narrow mapped
without using !. In the relevant test around the mapped and buggyPx assertions,
replace the direct mapped!.px access by first asserting mapped is not null (or
otherwise narrowing its type) and then compare its px value, keeping the same
expectation semantics while removing the assertion operator.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/components/video-editor/videoPlayback/cursorRenderer.test.ts`:
- Line 50: The test in cursorRenderer.test.ts uses a non-null assertion on
mapped after already asserting its value, so update the cursor mapping test to
narrow mapped without using !. In the relevant test around the mapped and
buggyPx assertions, replace the direct mapped!.px access by first asserting
mapped is not null (or otherwise narrowing its type) and then compare its px
value, keeping the same expectation semantics while removing the assertion
operator.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 73b61c1f-898e-4500-abc9-43ad080f105f

📥 Commits

Reviewing files that changed from the base of the PR and between 2fc116b and 9fd9eea.

📒 Files selected for processing (3)

• src/components/video-editor/VideoPlayback.tsx
• src/components/video-editor/videoPlayback/cursorRenderer.test.ts
• src/components/video-editor/videoPlayback/cursorRenderer.ts