Repository navigation
fix(compositor): no Full Camera trail on a clip's first frame when its start is inexact in f32 - #1294
Conversation
…s start is inexact in f32 The previous frame's time is held at the clip's start (297dfea), but the test compared the frame time, an f32, with the clip's start widened to f64. The first frame of the clip after a cut lands exactly on that start (SpeedSegment::source_time clamps it there, and set_timeline_time hands it over as f32), and a start f32 rounds down, 11.388 among them, read back as 11.38799953 < 11.388: the hold was skipped, the previous frame fell in the trim's hole with the camera in its PiP corner, while the Full Camera phase, compared in f32, already had it full frame. One ghosted frame at the cut in the export, and in getFrames (macOS pass of v2.1.0-rc.3, frame 600). A start that rounds up, like 15.788090425531916, was unaffected, which is why the same check looked clean on another cut. Compare in f32, as the region bounds are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly identifies the compositor bug and the specific fix for an inexact f32 clip start causing a Full Camera trail on the first frame. |
| Description check | |
The description follows the repository template. It explains the bug, cause, fix, related issue, change type, release impact, platform impact, evidence, and testing status. It clearly states that loca… |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
-
Explanation
The description follows the repository template. It explains the bug, cause, fix, related issue, change type, release impact, platform impact, evidence, and testing status. It clearly states that local tests were not run and identifies the CI validation required.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 Clippy (1.98.1)
-
Clippy execution failed
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.
Comment @coderabbitai help to get the list of available commands.
Summary
The macOS e2e pass of v2.1.0-rc.3 found a ghost frame at a trim cut inside a Full Camera region. It shows in the export and in
getFrames, but not in the preview. The fix for #1266 (297dfea) didn't catch it. On the first frame of the clip after the cut, the full-frame camera is smeared into several copies that slide toward its PiP corner.Cause: 297dfea clamps the previous frame's time to the clip's start, but the test that guards that clamp compares the frame time, an
f32, with the clip's start widened tof64.SpeedSegment::source_timeclamps it there, andset_timeline_timepasses it on asf32.f32rounds down, such as 11.388 (which reads back as 11.38799953 inf64), the clamp was skipped.f32, already had the camera full frame, which gives the trail.A start that rounds up, such as 15.788090425531916, was unaffected, which is why the same check looked clean on another cut.
The fix compares in
f32, as the region bounds already are. A new test reproduces the rc.3 case: a clip starting at 11.388, with its first frame exactly on that start.Related issue
Refs #1266
Type of change
Release impact
Desktop impact
Screenshots / video
Evidence from the rc.3 export (Full Camera 8.006–14.061 s, trim 10.001–11.388 s, 60 fps):
getFramesat 10.000, 10.001, 10.017 and 10.033 s returned the same ghosted frame.Testing
the_first_frame_of_a_clip_starting_inside_full_camera_has_no_trailincrates/compositor/src/frame_geometry.rs. It fails without the change, withw_dst_prevat the PiP rect.cargo test -p openscreen-compositor --lib --tests(macOS arm64 and x64, Windows, Linux).🤖 Generated with Claude Code
Summary by CodeRabbit