Skip to content

perf(gpui): avoid whole-timeline copies during clip trim previews #3439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
himanshu748 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

perf(gpui): avoid whole-timeline copies during clip trim previews #3439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
463134c
perf(gpui): avoid whole-timeline copies during clip trim previews
himanshu748 Oct 27, 2026
3a5df9c
test(gpui): render clip preview smoke test through a view
himanshu748 Oct 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions apps/desktop-gpui/src/editor_timeline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1292,9 +1292,12 @@ pub struct TimelineModel {
pub mic_waveforms: Vec<Arc<Vec<f32>>>,
pub system_waveforms: Vec<Arc<Vec<f32>>>,
pub waveform_paths: Arc<WaveformPaths>,
/// The span a live ghost trim is removing, in output time. Drawn as a gap
/// with a red duration badge, the way Blip's ghost resize marks the cut.
pub clip_ghost_gap: Option<(f64, f64)>,
}

#[derive(Debug, Clone)]
pub struct ClipPreview {
pub clips: Vec<Segment>,
pub ghost_gap: Option<(f64, f64)>,
}

impl TimelineModel {
Expand Down Expand Up @@ -1522,7 +1525,6 @@ impl TimelineModel {
mic_waveforms: Vec::new(),
system_waveforms: Vec::new(),
waveform_paths: Arc::default(),
clip_ghost_gap: None,
};
model.rows = build_rows(
config,
Expand Down Expand Up @@ -1800,6 +1802,7 @@ impl Default for TimelineView {
/// is a `Vec` and the view is `Copy` on the playback path.
#[derive(Debug, Clone, Copy, Default)]
pub struct SegmentUi<'a> {
pub clip_preview: Option<&'a ClipPreview>,
/// `editorState.timeline.selection`.
pub selection: Option<&'a Selection>,
/// `editorState.timeline.interactMode === "split"`, which swaps the cursor
Expand All @@ -1826,7 +1829,16 @@ pub struct SegmentUi<'a> {
pub camera3d_editing_end: bool,
}

impl SegmentUi<'_> {
impl<'a> SegmentUi<'a> {
pub fn segments_for(&self, model: &'a TimelineModel, row: TrackRow) -> &'a [Segment] {
if row.kind == TrackKind::Clip
&& let Some(preview) = self.clip_preview
{
return &preview.clips;
}
model.segments_for(row)
}

fn is_selected(&self, kind: TrackKind, index: usize) -> bool {
self.selection
.is_some_and(|selection| selection.contains(kind, index))
Expand All @@ -1839,8 +1851,8 @@ impl SegmentUi<'_> {
fn row_selected(&self, model: &TimelineModel, row: TrackRow) -> bool {
self.selection.is_some_and(|selection| {
selection.track == row.kind
&& model
.segments_for(row)
&& self
.segments_for(model, row)
Comment on lines +1854 to +1855

@greptile-apps greptile-apps Bot Oct 27, 2026

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.

P1 Native app cannot build

row_selected accepts model: &TimelineModel, but the new call to self.segments_for requires model: &'a TimelineModel. The input does not promise that lifetime, so this fails to compile with an explicit-lifetime-required error. Change the parameter to model: &'a TimelineModel, or let segments_for borrow both inputs for a shorter shared lifetime.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop-gpui/src/editor_timeline.rs
Line: 1854-1855

Comment:
**Native app cannot build**

`row_selected` accepts `model: &TimelineModel`, but the new call to `self.segments_for` requires `model: &'a TimelineModel`. The input does not promise that lifetime, so this fails to compile with an explicit-lifetime-required error. Change the parameter to `model: &'a TimelineModel`, or let `segments_for` borrow both inputs for a shorter shared lifetime.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

.iter()
.enumerate()
.any(|(index, segment)| {
Expand Down Expand Up @@ -2224,7 +2236,7 @@ fn render_track_content(
) -> AnyElement {
let width = content_width(viewport_width);
let secs_per_pixel = view.transform.secs_per_pixel(width);
let segments = model.segments_for(row);
let segments = ui.segments_for(model, row);

let mut content = div().relative().size_full();

Expand Down Expand Up @@ -2341,7 +2353,7 @@ fn render_track_content(
}

if row.kind == TrackKind::Clip
&& let Some((gap_start, gap_end)) = model.clip_ghost_gap
&& let Some((gap_start, gap_end)) = ui.clip_preview.and_then(|preview| preview.ghost_gap)
{
let x = ((gap_start - view.transform.position) / secs_per_pixel) as f32;
let width = ((gap_end - gap_start) / secs_per_pixel) as f32;
Expand Down
Loading
Loading