Merge pull request #2649 from zitann2020/ui/2589-self-drop

ui: a drop that holds the target composition lands the rest of the selection (#2589)

crates/ui-egui/src/panels.rs

@@ -124,8 +124,10 @@ impl PresetRef {
 
 /// Layers carried by a Project item drag. Dragging a selected item uses the
 /// complete selection, while dragging an unselected item uses only that item.
-/// Folders and data files are not composition layers.
-pub(crate) fn project_drop_items(app: &EffectcraftApp, dragged: u64) -> Vec<u64> {
+/// Folders and data files are not composition layers, and the composition the
+/// drop lands in is left out of a selection: it cannot contain itself, and one
+/// such item would make the engine refuse the whole drop (#2589).
+pub(crate) fn project_drop_items(app: &EffectcraftApp, dragged: u64, target: Option<effectcraft_engine::project::ItemId>) -> Vec<u64> {
     use effectcraft_engine::project::{FootageKind, ItemId, ItemKind};
 
     let selected = &app.session.state.project_selection;
@@ -139,6 +141,9 @@ pub(crate) fn project_drop_items(app: &EffectcraftApp, dragged: u64) -> Vec<u64>
     candidates
         .into_iter()
         .filter(|id| {
+            if Some(ItemId(*id)) == target {
+                return false;
+            }
             app.session.project.item(ItemId(*id)).is_some_and(|item| match &item.kind {
                 ItemKind::Comp(_) | ItemKind::Solid(_) => true,
                 ItemKind::Footage(f) => f.kind != FootageKind::Data,

crates/ui-egui/src/panels/project.rs

@@ -618,9 +618,14 @@ fn item_menu(
         actions.push(("project.move".into(), json!({"items": targets(app, id, selected), "folder": null})));
         ui.close();
     }
-    if !it.is_folder() && ui.button(crate::i18n::tr("Add to Composition")).clicked() {
-        actions.push(("layer.addItem".into(), json!({"item": id.0})));
-        ui.close();
+    // A composition cannot contain itself, so its own row offers nothing to add to (#2589).
+    if !it.is_folder() {
+        let add = ui.add_enabled(app.session.state.active_comp != Some(id), egui::Button::new(crate::i18n::tr("Add to Composition")));
+        app.auto.add("project.context.addToComposition", add.rect, "Add to Composition");
+        if add.clicked() {
+            actions.push(("layer.addItem".into(), json!({"item": id.0})));
+            ui.close();
+        }
     }
     // File ▸ New Comp from Selection on the selection (the right-clicked row joins it
     // above): several items ask how in its dialog, as in After Effects (#360).

crates/ui-egui/src/panels/timeline.rs

@@ -1778,7 +1778,7 @@ fn empty_timeline(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
     if let Some(crate::panels::DragPayload::Item(dragged)) = egui::DragAndDrop::payload::<crate::panels::DragPayload>(&ctx).as_deref()
         && ui.rect_contains_pointer(rect)
     {
-        let items: Vec<ItemId> = crate::panels::project_drop_items(app, *dragged)
+        let items: Vec<ItemId> = crate::panels::project_drop_items(app, *dragged, None)
             .into_iter()
             .map(ItemId)
             .filter(|id| {
@@ -4964,7 +4964,7 @@ fn drop_insert_layers(app: &mut EffectcraftApp, g: &mut Rw, payload: &std::sync:
         match payload.as_ref() {
             crate::panels::DragPayload::Item(id) => {
                 // Insert each selected item at the drop point, in selection order.
-                for (offset, item) in crate::panels::project_drop_items(app, *id).into_iter().enumerate() {
+                for (offset, item) in crate::panels::project_drop_items(app, *id, Some(g.tl.cid)).into_iter().enumerate() {
                     actions.push(("layer.addItem".into(), json!({"item": item, "index": index.saturating_add(offset), "time": time.clone()})));
                 }
             }

crates/ui-egui/src/panels/viewer/frame.rs

@@ -25,7 +25,7 @@ pub(super) fn empty_state(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rec
     if let Some(crate::panels::DragPayload::Item(dragged)) = egui::DragAndDrop::payload::<crate::panels::DragPayload>(&ctx).as_deref()
         && ui.rect_contains_pointer(rect)
     {
-        let items: Vec<effectcraft_engine::project::ItemId> = crate::panels::project_drop_items(app, *dragged)
+        let items: Vec<effectcraft_engine::project::ItemId> = crate::panels::project_drop_items(app, *dragged, None)
             .into_iter()
             .map(effectcraft_engine::project::ItemId)
             .filter(|id| {

crates/ui-egui/src/panels/viewer_drop.rs

@@ -54,7 +54,11 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &egui::Ui, painter: &egui::Pain
             }
             // New layers go above the current selection, so insert in reverse
             // to preserve the order of the dragged Project items.
-            crate::panels::project_drop_items(app, *item).into_iter().rev().map(|item| ("layer.addItem", json!({"item": item, "position": at}))).collect()
+            crate::panels::project_drop_items(app, *item, app.session.state.active_comp)
+                .into_iter()
+                .rev()
+                .map(|item| ("layer.addItem", json!({"item": item, "position": at})))
+                .collect()
         }
         DragPayload::Files(paths) => vec![("mediaBrowser.import", json!({"paths": paths, "addToComp": true, "position": at}))],
         DragPayload::Property { .. } => vec![],

crates/ui-egui/tests/it/main.rs

@@ -69,6 +69,7 @@ mod ui_render_queue;
 mod ui_script_console;
 mod ui_scriptui;
 mod ui_scrolling;
+mod ui_self_drop;
 mod ui_settings_info;
 mod ui_tabs;
 mod ui_tail;

crates/ui-egui/tests/it/ui_self_drop.rs

@@ -0,0 +1,75 @@
+//! #2589: a Project selection that contains the composition being dropped into must not fail the
+//! whole drop. Rendering writes a file named after the composition, so the Project panel holds
+//! both and selects both; the composition itself is not a layer it can hold.
+
+use effectcraft_engine::Session;
+use effectcraft_ui_egui::EffectcraftApp;
+use egui::{Event, Modifiers, Pos2, Rect, pos2, vec2};
+use egui_kittest::Harness;
+use serde_json::json;
+
+/// Compositions "Main" (active, 4 s at 30 fps, one solid) and "Clip".
+fn harness() -> (Harness<'static, EffectcraftApp>, u64, u64) {
+    let mut s = Session::default();
+    let clip = s.execute("comp.new", json!({"name": "Clip", "width": 160, "height": 90, "frameRate": 30, "duration": 1})).unwrap()["comp"].as_u64().unwrap();
+    let main = s.execute("comp.new", json!({"name": "Main", "width": 320, "height": 180, "frameRate": 30, "duration": 4})).unwrap()["comp"].as_u64().unwrap();
+    s.execute("layer.newSolid", json!({"name": "Bottom", "color": "#406080"})).unwrap();
+    let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s));
+    h.run_steps(3);
+    (h, main, clip)
+}
+
+fn rect(h: &Harness<'_, EffectcraftApp>, id: &str) -> Rect {
+    let e = h.state().auto.find(id).unwrap_or_else(|| panic!("no {id}")).clone();
+    Rect::from_min_size(pos2(e.rect[0], e.rect[1]), vec2(e.rect[2], e.rect[3]))
+}
+
+/// Press on `from`, move to `to` in steps, release there.
+fn drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2) {
+    h.event(Event::PointerMoved(from));
+    h.step();
+    h.event(Event::PointerButton { pos: from, button: egui::PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE });
+    h.step();
+    for k in 1..=10 {
+        h.event(Event::PointerMoved(from + (to - from) * (k as f32 / 10.0)));
+        h.step();
+    }
+    h.event(Event::PointerButton { pos: to, button: egui::PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE });
+    h.run_steps(3);
+}
+
+fn stack(h: &Harness<'_, EffectcraftApp>) -> Vec<String> {
+    h.state().session.active_comp().unwrap().layers.iter().map(|l| l.name.clone()).collect()
+}
+
+fn layer_id(h: &Harness<'_, EffectcraftApp>, name: &str) -> u64 {
+    h.state().session.active_comp().unwrap().layers.iter().find(|l| l.name == name).unwrap().id.0
+}
+
+/// Both the composition and the render output are selected: dragging the file onto the Timeline
+/// still lands the file, and the composition is not carried along into itself.
+#[test]
+fn a_selection_holding_the_drop_target_does_not_fail_the_drop() {
+    let (mut h, main, clip) = harness();
+    h.state_mut().session.execute("project.select", json!({"items": [main, clip]})).unwrap();
+    h.run_steps(2);
+    let from = rect(&h, &format!("project.item.{clip}.name")).center();
+    let row = rect(&h, &format!("timeline.layer.{}.row", layer_id(&h, "Bottom")));
+    drag(&mut h, from, row.center());
+    assert_eq!(stack(&h), ["Bottom", "Clip"], "the composition the drop lands in is left out");
+    assert!(!h.state().ui.status.contains("contain itself"), "the drop did not fail: {}", h.state().ui.status);
+}
+
+/// Dragging the composition's own row onto its Timeline adds nothing and reports nothing.
+#[test]
+fn the_composition_dropped_on_itself_adds_nothing() {
+    let (mut h, main, _clip) = harness();
+    h.state_mut().session.execute("project.select", json!({"items": [main]})).unwrap();
+    h.run_steps(2);
+    let before = stack(&h);
+    let from = rect(&h, &format!("project.item.{main}.name")).center();
+    let row = rect(&h, &format!("timeline.layer.{}.row", layer_id(&h, "Bottom")));
+    drag(&mut h, from, row.center());
+    assert_eq!(stack(&h), before, "nothing is added");
+    assert!(!h.state().ui.status.contains("contain itself"), "no error: {}", h.state().ui.status);
+}