Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
| Some((start, end)) if previous_playhead > start && previous_playhead < end => { | ||
| (previous_playhead - start).clamp(0.0, new_total.max(0.0)) |
There was a problem hiding this comment.
Playhead jumps behind retained titles
keep_only_playhead subtracts the old clip start but misses fullscreen titles that remain before the clip. For two 10-second clips with a fullscreen title at [10, 12], keeping the second clip moves the title to [0, 2]. The old clip starts at 12, so a playhead at 15 moves to 3; it should move to 5 to show the same recording frame.
Account for the retained clip’s new output start when rebasing, and add a boundary-title test.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/editor_window.rs
Line: 305-306
Comment:
**Playhead jumps behind retained titles**
`keep_only_playhead` subtracts the old clip start but misses fullscreen titles that remain before the clip. For two 10-second clips with a fullscreen title at `[10, 12]`, keeping the second clip moves the title to `[0, 2]`. The old clip starts at `12`, so a playhead at `15` moves to `3`; it should move to `5` to show the same recording frame.
Account for the retained clip’s new output start when rebasing, and add a boundary-title test.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed in commit 940fd90. keep_only_playhead now rebases against
ew_clip_start (the retained clip's output start) so playhead positions within clips that follow preceding fullscreen titles or holds stay frame-accurate, and added boundary-title unit test coverage.
|
Thanks! Can you get Greptile to 5/5? Then we'll take a look. |
|
@greptileai review Re-requesting review on latest commit 0754857 (addresses playhead rebasing with preceding fullscreen titles/holds via new_clip_start, with boundary-title unit tests and GPUI fixtures passing). |
Summary
Closes #3628
Adds a Keep only this clip action to the desktop GPUI video editor timeline's clip context menu right beside Delete clip, allowing users to isolate a single retained clip in one step after splitting a recording instead of deleting every surrounding clip individually.
Changes
apps/desktop-gpui/src/editor_edits.rs):keep_only_clip_segment(timeline, index)with upfront bounds and multi-clip guards (index < timeline.segments.len()andtimeline.segments.len() >= 2).delete_clip_segments, preserving the retained clip's trim boundaries and settings (timescale,name,speed_audio_mode,hide_cursor,volume), stripping removed clip transitions, and ripple-trimming/rebasing all associated output tracks (zoom,scene,camera3d,mask,text& fullscreen holds,caption,keyboard,audiowithtrim_start,style, andimage) cleanly to0.0without leading or trailing gaps.apps/desktop-gpui/src/editor_window.rs):EditorWindow::keep_only_clip, committing the mutation as a single undoable history entry (including source-timed caption re-projection viasynchronize_caption_track) so a single Undo / Redo restores all removed clips, transitions, and track timings.keep_only_playheadandtransform_after_keep_onlyso an interior playhead stays on the same relative frame within the kept clip while an exterior/boundary playhead resets to0.0, and zoomed-in timeline viewports re-anchor cleanly over[0.0, total]instead of leaving the retained clip off-screen."clip-menu-keep-only"row above"clip-menu-delete"inrender_clip_speed_popover(disabled with a hint when the timeline already has only one clip) and adjustedCLIP_MENU_HEIGHT_ESTIMATE.apps/desktop-gpui/locales/*.toml,src/i18n/editor_shell_surface_tests.rs):editor.window.clip_menu.keep_onlyandeditor.window.clip_menu.keep_only_single_clipacross all 7 locale catalogs (en,de,es,fr,hi,ja,zh-CN) and updatededitor_shell_surface_tests.

Confidence Score: 5/5
The PR appears safe to merge; the previous playhead issue is fixed.Summary
Adds “Keep only this clip” to the clip menu, with translations, one-step undo, and track timing tests.
Reviews (2) · Last reviewed commit: "test(desktop-gpui): fix transition type ..." · Reviewed by Greptile