Fix timeline drop targets and deleted drag layers (#2492)

Resolve viewport-edge insertion through the full Timeline outline, skipping
moving layers even below the viewport. Cancel a gesture if any captured
layer disappears before its ID can be interpreted as a layer index.

Cover intermediate drops, selected groups, deletion of the owner or another
group member, Undo/Redo, and later independent drags.
This commit is contained in:
WillPastor committed 2026-10-28 19:32:33 -04:00
1 parent 4fccf2a3b7
commit 559958ea62
3 files changed
+144 -2

No files matched your search

+20 -2
View File
@@ -1509,6 +1509,20 @@ fn insertion(layer_rows: &[(Rect, LayerId)], py: f32, top: f32) -> (Option<usize
(at, y)
}
/// Resolve a visible insertion boundary through the full outline, including offscreen layers.
fn layer_drop_target(rows: &[Row], visible: &[(Rect, LayerId)], at: Option<usize>, moving: &[u64]) -> Option<LayerId> {
let (anchor, after) = match at {
Some(i) => (visible.get(i)?.1, false),
None => (visible.last()?.1, true),
};
rows.iter()
.filter(|row| matches!(row.kind, RowKind::Layer))
.skip_while(|row| row.layer != anchor)
.skip(usize::from(after))
.find(|row| !moving.contains(&row.layer.0))
.map(|row| row.layer)
}
/// Whether a Project item can replace a layer's source (the same set `layer.replaceSource`
/// accepts): compositions, footage and solids; not folders, data files or 3D models.
fn replaceable_item(it: &effectcraft_engine::project::Item) -> bool {
@@ -4365,7 +4379,11 @@ fn layer_reorder_drag(ui: &mut egui::Ui, g: &mut Rw) {
let (down, released, escape) =
ctx.input(|i| (i.pointer.primary_down(), i.pointer.button_released(egui::PointerButton::Primary), i.key_pressed(egui::Key::Escape)));
let dropped = ctx.drag_stopped_id() == Some(drag.owner) && released;
let valid = ui.is_enabled() && drag.comp == cid && !escape && (dropped || (down && ctx.dragged_id() == Some(drag.owner)));
let valid = ui.is_enabled()
&& drag.comp == cid
&& drag.layers.iter().all(|id| comp.layer(LayerId(*id)).is_some())
&& !escape
&& (dropped || (down && ctx.dragged_id() == Some(drag.owner)));
if dropped || !valid {
ctx.data_mut(|d| d.remove::<LayerDrag>(layer_drag_id()));
}
@@ -4376,7 +4394,7 @@ fn layer_reorder_drag(ui: &mut egui::Ui, g: &mut Rw) {
&& !layer_rows.is_empty()
{
let (at, line_y) = insertion(&layer_rows, py, rows_rect.min.y);
let target = at.and_then(|i| layer_rows[i..].iter().find(|(_, l)| !moving.contains(&l.0)).map(|(_, l)| *l));
let target = layer_drop_target(&g.tl.rows, &layer_rows, at, moving);
if dropped {
// Only when the order changes (no empty undo step for a drop in place).
let mut order: Vec<u64> = comp.layers.iter().map(|l| l.id.0).filter(|id| !moving.contains(id)).collect();
@@ -535,3 +535,13 @@ fn disk_bar_range_covers_partial_frames() {
let above_limit = TMap { start: 100_001.0, ..tm };
assert_eq!(disk_bar_range(&above_limit, clip, 1.0, i64::MAX), 100_000..100_000);
}
#[test]
fn layer_drop_target_skips_moving_layers_beyond_the_viewport() {
let mut rows: Vec<Row> = (1..=6).map(|id| Row { layer: LayerId(id), depth: 0, kind: RowKind::Layer }).collect();
rows.insert(4, Row { layer: LayerId(4), depth: 1, kind: RowKind::Prop { uid: 0 } });
let visible: Vec<_> = (2..=4).map(|id| (Rect::NOTHING, LayerId(id))).collect();
for at in [Some(2), None] {
assert_eq!(layer_drop_target(&rows, &visible, at, &[1, 4, 5]), Some(LayerId(6)), "skip moving headers and their properties across the viewport edge");
assert_eq!(layer_drop_target(&rows, &visible, at, &[1, 4, 5, 6]), None, "the stack ends when no stationary layer follows");
}
}
+114
View File
@@ -1835,6 +1835,120 @@ fn timeline_rows_drag_to_top_after_scrolling() {
}
}
/// An intermediate scroll must not turn the viewport's lower edge into the stack's end.
fn timeline_reorder_at_viewport_edge(below: bool, multiple: bool) {
let mut h = harness();
for i in 0..38 {
h.state_mut().session.execute("layer.newSolid", json!({"name": format!("Layer {i}"), "color": "#406080", "width": 80, "height": 80})).unwrap();
}
h.state_mut().session.execute("edit.deselectAll", json!({})).unwrap();
h.run_steps(3);
let before = timeline_layer_order(&h);
let moving = before[..if multiple { 2 } else { 1 }].to_vec();
h.state_mut().session.execute("layer.select", json!({"layers": moving})).unwrap();
h.run_steps(2);
h.state_mut().session.history.clear();
let from = timeline_layer_row(&h, moving[0]).center();
start_timeline_row_drag(&mut h, from);
scroll_timeline(&mut h, -200.0);
let visible = |h: &Harness<'_, EffectcraftApp>| {
before.iter().copied().filter(|id| h.state().auto.find(&format!("timeline.layer.{id}.row")).is_some()).collect::<Vec<_>>()
};
// Leave space inside the viewport below the last rendered layer's midpoint.
for _ in 0..20 {
let last = timeline_layer_row(&h, *visible(&h).last().unwrap());
if last.center().y < rect(&h, "timeline.layerMarquee").max.y - 3.0 {
break;
}
scroll_timeline(&mut h, -10.0);
}
let shown = visible(&h);
assert!(!shown.contains(&moving[0]), "the drag owner is culled");
let last = *shown.last().unwrap();
assert_ne!(last, *before.last().unwrap(), "layers remain below the viewport");
let row = timeline_layer_row(&h, last);
let viewport = rect(&h, "timeline.layerMarquee");
let y = if below { (row.center().y + row.height() * 0.4).min(viewport.max.y - 1.0) } else { row.min.y + row.height() * 0.2 };
let to = pos2(row.center().x, y);
assert!(viewport.contains(to));
assert_eq!(to.y > row.center().y, below);
let mut expected: Vec<u64> = before.iter().copied().filter(|id| !moving.contains(id)).collect();
let at = expected.iter().position(|id| *id == last).unwrap() + usize::from(below);
expected.splice(at..at, moving);
release_timeline_row_drag(&mut h, to);
assert_eq!(timeline_layer_order(&h), expected, "the drop follows the indicated row boundary");
assert_eq!(h.state().session.history.undo.len(), 1);
assert_ne!(h.output().platform_output.cursor_icon, egui::CursorIcon::Grabbing);
h.state_mut().session.execute("edit.undo", json!({})).unwrap();
assert_eq!(timeline_layer_order(&h), before);
h.state_mut().session.execute("edit.redo", json!({})).unwrap();
assert_eq!(timeline_layer_order(&h), expected);
}
#[test]
fn timeline_rows_drag_after_last_visible_row() {
for multiple in [false, true] {
timeline_reorder_at_viewport_edge(true, multiple);
}
}
#[test]
fn timeline_rows_drag_before_last_visible_row() {
for multiple in [false, true] {
timeline_reorder_at_viewport_edge(false, multiple);
}
}
/// A removed ID must not be reinterpreted as a one-based index by layer.arrange.
fn timeline_reorder_after_deletion(delete_owner: bool) {
let mut h = harness();
for i in 0..6 {
h.state_mut().session.execute("layer.newSolid", json!({"name": format!("Layer {i}"), "color": "#406080", "width": 80, "height": 80})).unwrap();
}
let before = timeline_layer_order(&h);
let removed = *before.last().unwrap();
assert!(removed > 0 && removed < before.len() as u64, "the missing ID would also resolve as a surviving layer index");
let owner = if delete_owner { removed } else { before[0] };
let moving = if delete_owner { vec![removed] } else { vec![owner, removed] };
h.state_mut().session.execute("layer.select", json!({"layers": moving})).unwrap();
h.run_steps(3);
h.state_mut().session.history.clear();
let from = timeline_layer_row(&h, owner).center();
start_timeline_row_drag(&mut h, from);
if !delete_owner {
// Changing the selection does not change the group captured by the held drag.
h.state_mut().session.execute("layer.select", json!({"layers": [removed]})).unwrap();
h.run_steps(2);
}
key(&mut h, egui::Key::Delete, Default::default());
assert!(h.ctx.input(|i| i.pointer.primary_down()), "Delete happened during the held drag");
let survivors: Vec<u64> = before.iter().copied().filter(|id| *id != removed).collect();
assert_eq!(timeline_layer_order(&h), survivors, "Delete removed only the selected layer");
let steps = h.state().session.history.undo.len();
let row = timeline_layer_row(&h, survivors[0]);
release_timeline_row_drag(&mut h, row.center() - vec2(0.0, row.height() * 0.4));
assert_eq!(timeline_layer_order(&h), survivors, "a stale drag cannot move any surviving layer");
assert_eq!(h.state().session.history.undo.len(), steps, "release adds no undo step");
assert_ne!(h.output().platform_output.cursor_icon, egui::CursorIcon::Grabbing);
let from = timeline_layer_row(&h, survivors[0]).center();
let row = timeline_layer_row(&h, survivors[1]);
drag(&mut h, from, row.center() + vec2(0.0, row.height() * 0.4));
let mut expected = survivors;
expected.swap(0, 1);
assert_eq!(timeline_layer_order(&h), expected, "a later independent drag still works");
assert_eq!(h.state().session.history.undo.len(), steps + 1);
}
#[test]
fn timeline_rows_drag_cancels_when_the_owner_is_deleted() {
timeline_reorder_after_deletion(true);
}
#[test]
fn timeline_rows_drag_cancels_when_a_group_member_is_deleted() {
timeline_reorder_after_deletion(false);
}
#[test]
fn timeline_rows_drag_cancels_without_a_primary_release() {
for escape in [false, true] {