Skip to content

Commit 71e0877

Browse files
Merge pull request #2705 from AksharP5/perf/virtual-clip-sidebar
perf(desktop-gpui): keep long clip lists responsive
2 parents d77886d + ae95299 commit 71e0877

1 file changed

Lines changed: 90 additions & 32 deletions

File tree

‎apps/desktop-gpui/src/editor_clips.rs‎

Lines changed: 90 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ use cap_project::{
3434
use gpui::{
3535
AnyElement, App, AppContext as _, Bounds, Context, CursorStyle, Entity, FontWeight, Hsla,
3636
InteractiveElement, IntoElement, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent,
37-
ParentElement, Pixels, Point, RenderImage, ScrollHandle, SharedString,
38-
StatefulInteractiveElement as _, Styled, StyledImage as _, Window, WindowHandle, div, img,
39-
prelude::FluentBuilder, px, svg,
37+
ParentElement, Pixels, Point, RenderImage, SharedString, StatefulInteractiveElement as _,
38+
Styled, StyledImage as _, UniformListScrollHandle, Window, WindowHandle, div, img,
39+
prelude::FluentBuilder, px, svg, uniform_list,
4040
};
4141

4242
use crate::{
@@ -283,19 +283,19 @@ pub(crate) fn move_clip(
283283

284284
/// `computeDropIndex` (`ClipsSidebar.tsx:692-703`): the insertion point is
285285
/// after every card whose vertical midpoint the pointer has passed.
286-
pub(crate) fn compute_drop_index(pointer_y: Pixels, cards: &[Option<Bounds<Pixels>>]) -> usize {
287-
let mut insertion = 0;
288-
for (index, bounds) in cards.iter().enumerate() {
289-
let Some(bounds) = bounds else { continue };
290-
if pointer_y > bounds.origin.y + bounds.size.height / 2. {
291-
insertion = index + 1;
286+
fn compute_drop_index(pointer_y: Pixels, cards: &[ClipCardBounds]) -> usize {
287+
let mut insertion = cards.first().map_or(0, |card| card.index);
288+
for card in cards {
289+
if pointer_y > card.bounds.origin.y + card.bounds.size.height / 2. {
290+
insertion = card.index + 1;
292291
}
293292
}
294293
insertion
295294
}
296295

297296
/// `startClipDrag`'s activation threshold (`:729`).
298297
const DRAG_THRESHOLD_PX: f32 = 5.;
298+
const CLIP_CARD_GAP: f32 = 8.;
299299

300300
// ---------------------------------------------------------------------------
301301
// State
@@ -312,6 +312,11 @@ enum ClipThumb {
312312
Failed,
313313
}
314314

315+
struct ClipCardBounds {
316+
index: usize,
317+
bounds: Bounds<Pixels>,
318+
}
319+
315320
/// `MAX_THUMBNAIL_LOADS` (`:93`).
316321
const MAX_THUMBNAIL_LOADS: usize = 2;
317322

@@ -345,10 +350,9 @@ pub(crate) struct ClipsState {
345350
drag: Option<ClipDrag>,
346351
/// `dropIndex`.
347352
drop_index: Option<usize>,
348-
/// Every card's painted bounds, written by a per-card canvas probe --
349-
/// `getBoundingClientRect` for `computeDropIndex`.
350-
card_bounds: Rc<RefCell<Vec<Option<Bounds<Pixels>>>>>,
351-
scroll: ScrollHandle,
353+
/// Visible card bounds retain timeline indices for scrolled drop targets.
354+
card_bounds: Rc<RefCell<Vec<ClipCardBounds>>>,
355+
scroll: UniformListScrollHandle,
352356
thumbs: HashMap<ThumbKey, ClipThumb>,
353357
thumbs_inflight: usize,
354358
}
@@ -367,7 +371,7 @@ impl Default for ClipsState {
367371
drag: None,
368372
drop_index: None,
369373
card_bounds: Rc::new(RefCell::new(Vec::new())),
370-
scroll: ScrollHandle::new(),
374+
scroll: UniformListScrollHandle::new(),
371375
thumbs: HashMap::new(),
372376
thumbs_inflight: 0,
373377
}
@@ -927,12 +931,16 @@ impl EditorWindow {
927931
.min_h_0()
928932
.mx(px(-4.))
929933
.px(px(4.))
930-
.overflow_y_scroll()
931-
.track_scroll(&self.clips.scroll)
934+
.flex()
935+
.flex_col()
936+
.overflow_hidden()
937+
.when(self.clip_segments().is_empty(), |this| {
938+
this.overflow_y_scroll()
939+
})
932940
.child(if self.clip_segments().is_empty() {
933941
self.render_clips_empty().into_any_element()
934942
} else {
935-
self.render_clip_list(cx).into_any_element()
943+
self.render_clip_list(cx)
936944
}),
937945
)
938946
}
@@ -981,15 +989,49 @@ impl EditorWindow {
981989
}
982990

983991
/// The card list (`:852-964`).
984-
fn render_clip_list(&mut self, cx: &mut Context<Self>) -> impl IntoElement {
992+
fn render_clip_list(&mut self, cx: &mut Context<Self>) -> AnyElement {
985993
let count = self.clip_segments().len();
986-
self.clips.card_bounds.borrow_mut().resize(count, None);
994+
self.clips.card_bounds.borrow_mut().clear();
987995

988-
div()
989-
.flex()
990-
.flex_col()
991-
.gap(px(8.))
992-
.children((0..count).map(|index| self.render_clip_card(index, cx)))
996+
// Offscreen virtual rows lose their key handlers. Keep the rename input
997+
// mounted until editing ends so Enter and Escape still reach it.
998+
if self.clips.editing.is_some() {
999+
let scroll = self.clips.scroll.0.borrow().base_handle.clone();
1000+
return div()
1001+
.id("clips-list")
1002+
.flex_1()
1003+
.min_h_0()
1004+
.overflow_y_scroll()
1005+
.track_scroll(&scroll)
1006+
.child(
1007+
div()
1008+
.flex()
1009+
.flex_col()
1010+
.gap(px(CLIP_CARD_GAP))
1011+
.children((0..count).map(|index| self.render_clip_card(index, cx))),
1012+
)
1013+
.into_any_element();
1014+
}
1015+
1016+
uniform_list(
1017+
"clips-list",
1018+
count,
1019+
cx.processor(|this, range: std::ops::Range<usize>, _, cx| {
1020+
range
1021+
.map(|index| {
1022+
div()
1023+
.pb(px(CLIP_CARD_GAP))
1024+
.child(this.render_clip_card(index, cx))
1025+
})
1026+
.collect()
1027+
}),
1028+
)
1029+
.track_scroll(&self.clips.scroll)
1030+
.flex_1()
1031+
.min_h_0()
1032+
// Clip the last row's gap so the scroll range matches the original list.
1033+
.mb(px(-CLIP_CARD_GAP))
1034+
.into_any_element()
9931035
}
9941036

9951037
fn render_clip_card(&self, index: usize, cx: &mut Context<Self>) -> AnyElement {
@@ -1024,10 +1066,7 @@ impl EditorWindow {
10241066
let cell = self.clips.card_bounds.clone();
10251067
gpui::canvas(
10261068
move |bounds, _window, _cx| {
1027-
let mut slots = cell.borrow_mut();
1028-
if index < slots.len() {
1029-
slots[index] = Some(bounds);
1030-
}
1069+
cell.borrow_mut().push(ClipCardBounds { index, bounds });
10311070
},
10321071
|_, _, _, _| {},
10331072
)
@@ -4076,20 +4115,39 @@ mod tests {
40764115
/// card whose midpoint the pointer has passed.
40774116
#[test]
40784117
fn drop_index_follows_the_card_midpoints() {
4079-
let card = |top: f32| {
4080-
Some(Bounds {
4118+
let card = |index, top: f32| ClipCardBounds {
4119+
index,
4120+
bounds: Bounds {
40814121
origin: point(px(0.), px(top)),
40824122
size: size(px(100.), px(72.)),
4083-
})
4123+
},
40844124
};
4085-
let cards = vec![card(0.), card(80.), card(160.)];
4125+
let cards = vec![card(0, 0.), card(1, 80.), card(2, 160.)];
40864126
assert_eq!(compute_drop_index(px(-10.), &cards), 0);
40874127
assert_eq!(compute_drop_index(px(35.), &cards), 0);
40884128
assert_eq!(compute_drop_index(px(37.), &cards), 1);
40894129
assert_eq!(compute_drop_index(px(120.), &cards), 2);
40904130
assert_eq!(compute_drop_index(px(500.), &cards), 3);
40914131
}
40924132

4133+
#[test]
4134+
fn scrolled_drop_targets_keep_the_visible_clips_timeline_indices() {
4135+
let cards = [46, 47, 48].map(|index| ClipCardBounds {
4136+
index,
4137+
bounds: Bounds {
4138+
origin: point(px(0.), px((index - 46) as f32 * 80. - 20.)),
4139+
size: size(px(100.), px(72.)),
4140+
},
4141+
});
4142+
assert_eq!(compute_drop_index(px(0.), &cards), 46);
4143+
assert_eq!(compute_drop_index(px(16.), &cards), 46);
4144+
assert_eq!(compute_drop_index(px(17.), &cards), 47);
4145+
assert_eq!(compute_drop_index(px(96.), &cards), 47);
4146+
assert_eq!(compute_drop_index(px(97.), &cards), 48);
4147+
assert_eq!(compute_drop_index(px(300.), &cards), 49);
4148+
assert_eq!(compute_drop_index(px(0.), &[]), 0);
4149+
}
4150+
40934151
#[test]
40944152
fn relative_paths_are_validated_before_they_resolve() {
40954153
assert!(normalized_relative("content/segments/segment-0/display.mp4", "video").is_ok());

0 commit comments

Comments
 (0)