From c02e2a4f05111844a48fa64dd5fa51f2b319a49a Mon Sep 17 00:00:00 2001 From: Akshar Patel Date: Sun, 18 Oct 2026 20:10:51 -0400 Subject: [PATCH] perf(desktop-gpui): keep long clip lists responsive --- apps/desktop-gpui/src/editor_clips.rs | 122 +++++++++++++++++++------- 1 file changed, 90 insertions(+), 32 deletions(-) diff --git a/apps/desktop-gpui/src/editor_clips.rs b/apps/desktop-gpui/src/editor_clips.rs index a69618ab8ef..ac3a273155e 100644 --- a/apps/desktop-gpui/src/editor_clips.rs +++ b/apps/desktop-gpui/src/editor_clips.rs @@ -34,9 +34,9 @@ use cap_project::{ use gpui::{ AnyElement, App, AppContext as _, Bounds, Context, CursorStyle, Entity, FontWeight, Hsla, InteractiveElement, IntoElement, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, - ParentElement, Pixels, Point, RenderImage, ScrollHandle, SharedString, - StatefulInteractiveElement as _, Styled, StyledImage as _, Window, WindowHandle, div, img, - prelude::FluentBuilder, px, svg, + ParentElement, Pixels, Point, RenderImage, SharedString, StatefulInteractiveElement as _, + Styled, StyledImage as _, UniformListScrollHandle, Window, WindowHandle, div, img, + prelude::FluentBuilder, px, svg, uniform_list, }; use crate::{ @@ -283,12 +283,11 @@ pub(crate) fn move_clip( /// `computeDropIndex` (`ClipsSidebar.tsx:692-703`): the insertion point is /// after every card whose vertical midpoint the pointer has passed. -pub(crate) fn compute_drop_index(pointer_y: Pixels, cards: &[Option>]) -> usize { - let mut insertion = 0; - for (index, bounds) in cards.iter().enumerate() { - let Some(bounds) = bounds else { continue }; - if pointer_y > bounds.origin.y + bounds.size.height / 2. { - insertion = index + 1; +fn compute_drop_index(pointer_y: Pixels, cards: &[ClipCardBounds]) -> usize { + let mut insertion = cards.first().map_or(0, |card| card.index); + for card in cards { + if pointer_y > card.bounds.origin.y + card.bounds.size.height / 2. { + insertion = card.index + 1; } } insertion @@ -296,6 +295,7 @@ pub(crate) fn compute_drop_index(pointer_y: Pixels, cards: &[Option, +} + /// `MAX_THUMBNAIL_LOADS` (`:93`). const MAX_THUMBNAIL_LOADS: usize = 2; @@ -345,10 +350,9 @@ pub(crate) struct ClipsState { drag: Option, /// `dropIndex`. drop_index: Option, - /// Every card's painted bounds, written by a per-card canvas probe -- - /// `getBoundingClientRect` for `computeDropIndex`. - card_bounds: Rc>>>>, - scroll: ScrollHandle, + /// Visible card bounds retain timeline indices for scrolled drop targets. + card_bounds: Rc>>, + scroll: UniformListScrollHandle, thumbs: HashMap, thumbs_inflight: usize, } @@ -367,7 +371,7 @@ impl Default for ClipsState { drag: None, drop_index: None, card_bounds: Rc::new(RefCell::new(Vec::new())), - scroll: ScrollHandle::new(), + scroll: UniformListScrollHandle::new(), thumbs: HashMap::new(), thumbs_inflight: 0, } @@ -927,12 +931,16 @@ impl EditorWindow { .min_h_0() .mx(px(-4.)) .px(px(4.)) - .overflow_y_scroll() - .track_scroll(&self.clips.scroll) + .flex() + .flex_col() + .overflow_hidden() + .when(self.clip_segments().is_empty(), |this| { + this.overflow_y_scroll() + }) .child(if self.clip_segments().is_empty() { self.render_clips_empty().into_any_element() } else { - self.render_clip_list(cx).into_any_element() + self.render_clip_list(cx) }), ) } @@ -981,15 +989,49 @@ impl EditorWindow { } /// The card list (`:852-964`). - fn render_clip_list(&mut self, cx: &mut Context) -> impl IntoElement { + fn render_clip_list(&mut self, cx: &mut Context) -> AnyElement { let count = self.clip_segments().len(); - self.clips.card_bounds.borrow_mut().resize(count, None); + self.clips.card_bounds.borrow_mut().clear(); - div() - .flex() - .flex_col() - .gap(px(8.)) - .children((0..count).map(|index| self.render_clip_card(index, cx))) + // Offscreen virtual rows lose their key handlers. Keep the rename input + // mounted until editing ends so Enter and Escape still reach it. + if self.clips.editing.is_some() { + let scroll = self.clips.scroll.0.borrow().base_handle.clone(); + return div() + .id("clips-list") + .flex_1() + .min_h_0() + .overflow_y_scroll() + .track_scroll(&scroll) + .child( + div() + .flex() + .flex_col() + .gap(px(CLIP_CARD_GAP)) + .children((0..count).map(|index| self.render_clip_card(index, cx))), + ) + .into_any_element(); + } + + uniform_list( + "clips-list", + count, + cx.processor(|this, range: std::ops::Range, _, cx| { + range + .map(|index| { + div() + .pb(px(CLIP_CARD_GAP)) + .child(this.render_clip_card(index, cx)) + }) + .collect() + }), + ) + .track_scroll(&self.clips.scroll) + .flex_1() + .min_h_0() + // Clip the last row's gap so the scroll range matches the original list. + .mb(px(-CLIP_CARD_GAP)) + .into_any_element() } fn render_clip_card(&self, index: usize, cx: &mut Context) -> AnyElement { @@ -1024,10 +1066,7 @@ impl EditorWindow { let cell = self.clips.card_bounds.clone(); gpui::canvas( move |bounds, _window, _cx| { - let mut slots = cell.borrow_mut(); - if index < slots.len() { - slots[index] = Some(bounds); - } + cell.borrow_mut().push(ClipCardBounds { index, bounds }); }, |_, _, _, _| {}, ) @@ -4076,13 +4115,14 @@ mod tests { /// card whose midpoint the pointer has passed. #[test] fn drop_index_follows_the_card_midpoints() { - let card = |top: f32| { - Some(Bounds { + let card = |index, top: f32| ClipCardBounds { + index, + bounds: Bounds { origin: point(px(0.), px(top)), size: size(px(100.), px(72.)), - }) + }, }; - let cards = vec![card(0.), card(80.), card(160.)]; + let cards = vec![card(0, 0.), card(1, 80.), card(2, 160.)]; assert_eq!(compute_drop_index(px(-10.), &cards), 0); assert_eq!(compute_drop_index(px(35.), &cards), 0); assert_eq!(compute_drop_index(px(37.), &cards), 1); @@ -4090,6 +4130,24 @@ mod tests { assert_eq!(compute_drop_index(px(500.), &cards), 3); } + #[test] + fn scrolled_drop_targets_keep_the_visible_clips_timeline_indices() { + let cards = [46, 47, 48].map(|index| ClipCardBounds { + index, + bounds: Bounds { + origin: point(px(0.), px((index - 46) as f32 * 80. - 20.)), + size: size(px(100.), px(72.)), + }, + }); + assert_eq!(compute_drop_index(px(0.), &cards), 46); + assert_eq!(compute_drop_index(px(16.), &cards), 46); + assert_eq!(compute_drop_index(px(17.), &cards), 47); + assert_eq!(compute_drop_index(px(96.), &cards), 47); + assert_eq!(compute_drop_index(px(97.), &cards), 48); + assert_eq!(compute_drop_index(px(300.), &cards), 49); + assert_eq!(compute_drop_index(px(0.), &[]), 0); + } + #[test] fn relative_paths_are_validated_before_they_resolve() { assert!(normalized_relative("content/segments/segment-0/display.mp4", "video").is_ok());