From 197639988fb988a5124216ea1dd94b7e87c6b11f Mon Sep 17 00:00:00 2001 From: ReReddo <249398616+ReReddo@users.noreply.github.com> Date: Sat, 10 Oct 2026 11:29:33 +0300 Subject: [PATCH] Escape cancels a drag-and-drop in progress (#580) egui ends a drag when Escape is pressed, and FilmCraft treated that stop as a release: the drag was committed wherever the pointer was. Escape now abandons it: nothing moves, nothing is dropped, nothing goes on the undo stack. - Timeline: clip moves, trims, rolls, slips, slides and stretches, caption blocks and automation keyframes. The clips stay put and stay selected. - Items carried between panels (a clip or sequence from the Project panel, Source monitor drags, effects, presets, graphics templates): the payload is cleared in drag_ghost. panels::cancel_drag / drag_cancelled keep a cancelled drag from restarting until the button comes up. - Freeform card moves go back, and the next drag works. - Media Browser drags, which import on drag start, take the import back even when released over a panel. Tests: escape_drag_ui (5 tests: Timeline clip, caption, Project panel item, Freeform card, effect; each with the same drag without Escape as a control) and escape_takes_back_a_media_browser_drag. Automation keyframes have no test fixture. The ui-egui suite passes except the two Windows CRLF failures fixed in #488. --- crates/ui-egui/src/panels/media_browser.rs | 5 +- crates/ui-egui/src/panels/mod.rs | 25 ++- crates/ui-egui/src/panels/project_views.rs | 10 +- crates/ui-egui/src/panels/timeline.rs | 5 + .../ui-egui/src/panels/timeline_automation.rs | 9 +- .../ui-egui/src/panels/timeline_captions.rs | 3 +- crates/ui-egui/tests/escape_drag_ui.rs | 176 ++++++++++++++++++ crates/ui-egui/tests/project_panel_ui.rs | 36 ++++ 8 files changed, 260 insertions(+), 9 deletions(-) create mode 100644 crates/ui-egui/tests/escape_drag_ui.rs diff --git a/crates/ui-egui/src/panels/media_browser.rs b/crates/ui-egui/src/panels/media_browser.rs index c19ed5ce..592abb20 100644 --- a/crates/ui-egui/src/panels/media_browser.rs +++ b/crates/ui-egui/src/panels/media_browser.rs @@ -470,8 +470,9 @@ fn finish_drag(app: &mut FilmcraftApp, ui: &egui::Ui) { } let pos = with_cache(ui.ctx(), |c| c.drag.take().map(|d| d.2)).unwrap_or_default(); let targets = ["panel.Project", "panel.Timeline", "panel.Source", "panel.Program"]; - let landed = - targets.iter().any(|t| app.auto.find(t).is_some_and(|e| Rect::from_min_size(pos2(e.rect[0], e.rect[1]), vec2(e.rect[2], e.rect[3])).contains(pos))); + // a drag cancelled with Escape (#580) landed nowhere, wherever the pointer is + let landed = !crate::panels::drag_cancelled(ui) + && targets.iter().any(|t| app.auto.find(t).is_some_and(|e| Rect::from_min_size(pos2(e.rect[0], e.rect[1]), vec2(e.rect[2], e.rect[3])).contains(pos))); let used_elsewhere = app.session.history.undo.len() > hist; if !landed && !used_elsewhere && items.iter().all(|i| app.session.project.item(ItemId(*i)).is_some()) { // dropped nowhere: take the import back diff --git a/crates/ui-egui/src/panels/mod.rs b/crates/ui-egui/src/panels/mod.rs index 5b1b9f51..768a9474 100644 --- a/crates/ui-egui/src/panels/mod.rs +++ b/crates/ui-egui/src/panels/mod.rs @@ -188,10 +188,33 @@ pub fn clear_drag(ui: &egui::Ui) { ui.ctx().data_mut(|d| d.insert_temp::>(payload_id(), None)); } -/// Draw the drag ghost near the pointer and clear the payload after release. +fn cancelled_id() -> egui::Id { + egui::Id::new("filmcraft-drag-cancelled") +} + +/// Abandon the drag in progress (Escape, #580): nothing is dropped, and nothing restarts it until +/// the button comes up. +pub fn cancel_drag(ui: &egui::Ui) { + clear_drag(ui); + ui.ctx().data_mut(|d| d.insert_temp(cancelled_id(), true)); +} + +/// Whether the press in progress had its drag cancelled with Escape. +pub fn drag_cancelled(ui: &egui::Ui) -> bool { + ui.ctx().data(|d| d.get_temp::(cancelled_id())).unwrap_or(false) +} + +/// Draw the drag ghost near the pointer and clear the payload after release. Escape cancels it. pub fn drag_ghost(app: &FilmcraftApp, ui: &egui::Ui) { + if !ui.ctx().input(|i| i.pointer.any_down()) { + ui.ctx().data_mut(|d| d.remove::(cancelled_id())); + } let Some(pl) = payload(ui) else { return }; let ctx = ui.ctx(); + if ctx.input(|i| i.key_pressed(egui::Key::Escape)) { + cancel_drag(ui); + return; + } if let Some(p) = ctx.pointer_hover_pos() { let label = match &pl { DragPayload::Item(i) => app.session.project.item(*i).map(|x| x.name.clone()).unwrap_or_default(), diff --git a/crates/ui-egui/src/panels/project_views.rs b/crates/ui-egui/src/panels/project_views.rs index e48d98dd..8e47a487 100644 --- a/crates/ui-egui/src/panels/project_views.rs +++ b/crates/ui-egui/src/panels/project_views.rs @@ -1017,7 +1017,13 @@ pub fn freeform_view(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, v: & app.auto.add(&format!("{}.stack.{s}", v.prefix), br, &format!("Stack of {n}")); } item_interactions(app, ui, &resp, ItemId(c.item), &kind, actions, false, false); - if resp.dragged() { + // Escape puts the cards back and drops nothing until the button comes up (#580); egui + // also ends the drag on Escape, so it arrives as a stop as often as during the drag + let escape = ui.input(|i| i.key_pressed(egui::Key::Escape)); + if (resp.dragged() || resp.drag_stopped()) && (escape || crate::panels::drag_cancelled(ui)) { + ui.ctx().data_mut(|d| d.remove::<(u64, egui::Vec2)>(drag_key)); + crate::panels::cancel_drag(ui); + } else if resp.dragged() { let total = dragging.filter(|d| d.0 == c.item).map(|d| d.1).unwrap_or_default() + resp.drag_delta(); ui.ctx().data_mut(|d| d.insert_temp(drag_key, (c.item, total))); // leaving the panel turns the move into an item drag (to the Timeline, a monitor) @@ -1028,7 +1034,7 @@ pub fn freeform_view(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, v: & crate::panels::start_drag_item(ui, ItemId(c.item)); } } - if resp.drag_stopped() { + if resp.drag_stopped() && !crate::panels::drag_cancelled(ui) { released = dragging.filter(|d| d.0 == c.item).map(|d| (d.0, d.1 + resp.drag_delta())); } resp.context_menu(|ui| card_menu(app, ui, v, c.item, actions)); diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index d5766c0b..112740e8 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -2200,6 +2200,11 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & } } + // ---- Escape abandons the drag: the release commits nothing and the clips stay put (#580) + if app.tl.drag.is_some() && ctx.input(|i| i.key_pressed(egui::Key::Escape)) { + app.tl.drag = None; + } + // ---- drag if resp.dragged() && let Some(p) = resp.interact_pointer_pos() diff --git a/crates/ui-egui/src/panels/timeline_automation.rs b/crates/ui-egui/src/panels/timeline_automation.rs index 783dc5ba..4e4ecc08 100644 --- a/crates/ui-egui/src/panels/timeline_automation.rs +++ b/crates/ui-egui/src/panels/timeline_automation.rs @@ -211,9 +211,12 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & if resp.drag_stopped() && let Some(d) = dragging { - let nt = layout.tick_at(c.x).max(Tick::ZERO); - let nv = denorm(tr, &key, ((b.max.y - c.y) / b.height()).clamp(0.0, 1.0)); - acts.push(("mixer.moveKeyframe", json!({"strip": r.track.0, "lane": key, "time": d.orig.0, "newTime": nt.0, "value": nv}))); + // egui ends a drag on Escape: that stop abandons it (#580) + if !ui.input(|i| i.key_pressed(egui::Key::Escape)) { + let nt = layout.tick_at(c.x).max(Tick::ZERO); + let nv = denorm(tr, &key, ((b.max.y - c.y) / b.height()).clamp(0.0, 1.0)); + acts.push(("mixer.moveKeyframe", json!({"strip": r.track.0, "lane": key, "time": d.orig.0, "newTime": nt.0, "value": nv}))); + } ui.data_mut(|dd| dd.remove::(drag_id())); } if dragging.is_some() { diff --git a/crates/ui-egui/src/panels/timeline_captions.rs b/crates/ui-egui/src/panels/timeline_captions.rs index 781c7ade..fd123576 100644 --- a/crates/ui-egui/src/panels/timeline_captions.rs +++ b/crates/ui-egui/src/panels/timeline_captions.rs @@ -259,7 +259,8 @@ fn interact_block( && id == c.id.0 { ctx.data_mut(|dd| dd.remove::<(u64, u8, i64)>(drag_id())); - if d != 0 { + // egui ends a drag on Escape: that stop abandons it (#580) + if d != 0 && !ctx.input(|i| i.key_pressed(egui::Key::Escape)) { match g { 0 => { let sel: Vec = if app.session.state.caption_selection.contains(&ClipId(id)) { diff --git a/crates/ui-egui/tests/escape_drag_ui.rs b/crates/ui-egui/tests/escape_drag_ui.rs new file mode 100644 index 00000000..51c74b6f --- /dev/null +++ b/crates/ui-egui/tests/escape_drag_ui.rs @@ -0,0 +1,176 @@ +//! Escape during a drag-and-drop abandons it (#580): moving clips and captions on the Timeline, +//! dragging an item out of the Project panel, moving a Freeform card and dragging an effect onto a +//! clip. Each test +//! also makes the same drag without Escape, so it shows the drag would otherwise have done something. + +use std::sync::mpsc::{Sender, channel}; + +use egui_kittest::Harness; +use filmcraft_engine::Session; +use filmcraft_engine::project_panel::FREEFORM_POS; +use filmcraft_project::{BinEntry, ItemId}; +use filmcraft_ui_egui::FilmcraftApp; +use filmcraft_ui_egui::control::ControlRequest; +use serde_json::{Value, json}; + +struct Driver { + harness: Harness<'static, FilmcraftApp>, + tx: Sender, +} + +impl Driver { + fn demo() -> Self { + let mut session = Session::default(); + session.execute("file.openDemoProject", json!({})).expect("demo project"); + let (tx, rx) = channel(); + let app = FilmcraftApp::new(session).with_control(rx); + let harness = Harness::builder().with_size(egui::vec2(1600.0, 980.0)).with_max_steps(10_000).build_eframe(move |_cc| app); + let mut d = Driver { harness, tx }; + d.frames(4); + d + } + fn frames(&mut self, n: usize) { + for _ in 0..n { + let ctx = self.harness.ctx.clone(); + let mut raw = std::mem::take(self.harness.input_mut()); + eframe::App::raw_input_hook(self.harness.state_mut(), &ctx, &mut raw); + *self.harness.input_mut() = raw; + self.harness.step(); + } + } + fn ok(&mut self, method: &str, params: Value) -> Value { + let (req, reply) = ControlRequest::new(method, params.clone()); + self.tx.send(req).unwrap(); + for _ in 0..600 { + self.frames(1); + if let Ok(r) = reply.try_recv() { + assert_eq!(r["ok"], true, "{method} {params}: {r}"); + self.frames(2); + return r["result"].clone(); + } + } + panic!("no reply to {method} {params}"); + } + fn rect(&mut self, id: &str) -> [f32; 4] { + let v = self.ok("ui.elements", json!({"prefix": id})); + let e = v.as_array().unwrap().iter().find(|e| e["id"] == id).unwrap_or_else(|| panic!("no element {id}")).clone(); + let r: Vec = e["rect"].as_array().unwrap().iter().map(|x| x.as_f64().unwrap() as f32).collect(); + [r[0], r[1], r[2], r[3]] + } + fn send(&mut self, e: egui::Event) { + self.harness.input_mut().events.push(e); + self.frames(1); + } + /// A left-button drag in 12 steps, with Escape pressed half way when `escape`. + fn drag(&mut self, from: egui::Pos2, to: egui::Pos2, escape: bool) { + let button = |pos, pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }; + self.send(egui::Event::PointerMoved(from)); + self.send(button(from, true)); + for k in 1..=12 { + self.send(egui::Event::PointerMoved(from.lerp(to, k as f32 / 12.0))); + if escape && k == 6 { + self.send(egui::Event::Key { key: egui::Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }); + } + } + self.send(button(to, false)); + self.frames(4); + } + fn project(&self) -> String { + self.harness.state().session.project.to_json().to_string() + } + fn undo_len(&self) -> usize { + self.harness.state().session.history.undo.len() + } + /// The demo's Footage bin open in place in the Project panel in `view`; its items. + fn footage_in(&mut self, view: &str) -> Vec { + let p = &self.harness.state().session.project; + let bin = p.root.children.iter().find_map(|e| if let BinEntry::Bin(b) = e { (b.name == "Footage").then(|| b.clone()) } else { None }).expect("Footage"); + let items: Vec = bin.children.iter().filter_map(|e| if let BinEntry::Item(i) = e { Some(i.0) } else { None }).collect(); + self.ok("ui.panel.show", json!({"panel": "Project"})); + self.ok("engine.execute", json!({"command": "project.view.set", "params": {"view": view}})); + self.ok("ui.menu.invoke", json!({"id": "projectPanel.openBin", "params": {"bin": bin.id.0, "how": "inPlace"}})); + self.frames(4); + items + } +} + +fn centre(r: [f32; 4]) -> egui::Pos2 { + egui::pos2(r[0] + r[2] / 2.0, r[1] + r[3] / 2.0) +} + +#[test] +fn escape_puts_a_moved_timeline_clip_back() { + let mut d = Driver::demo(); + let clip = d.harness.state().session.active_sequence().unwrap().video_tracks[0].items[0].id; + d.ok("engine.execute", json!({"command": "timeline.select", "params": {"clips": [clip.0]}})); + let (before, undo) = (d.project(), d.undo_len()); + let from = centre(d.rect(&format!("timeline.clip.{}", clip.0))); + let to = from + egui::vec2(120.0, 0.0); + d.drag(from, to, true); + assert_eq!(d.project(), before, "the clip stayed where it was"); + assert_eq!(d.undo_len(), undo, "nothing to undo"); + assert!(d.harness.state().session.state.selection.contains(&clip), "the clip is still selected (with its linked audio)"); + d.drag(from, to, false); + assert_ne!(d.project(), before, "without Escape the same drag moves the clip"); +} + +#[test] +fn escape_puts_a_moved_caption_back() { + let mut d = Driver::demo(); + let id = + d.ok("engine.execute", json!({"command": "captions.add", "params": {"seconds": 2.0, "durationSeconds": 2.0}}))["caption"].as_u64().expect("caption id"); + d.frames(4); + let from = centre(d.rect(&format!("timeline.caption.{id}"))); + let to = from + egui::vec2(60.0, 0.0); + let (before, undo) = (d.project(), d.undo_len()); + d.drag(from, to, true); + assert_eq!(d.project(), before, "the caption stayed where it was"); + assert_eq!(d.undo_len(), undo); + d.drag(from, to, false); + assert_ne!(d.project(), before, "without Escape the same drag moves the caption"); +} + +#[test] +fn escape_drops_nothing_from_the_project_panel() { + let mut d = Driver::demo(); + let items = d.footage_in("list"); + let from = centre(d.rect(&format!("project.item.{}", items[0]))); + let tl = d.rect("panel.Timeline"); + let to = egui::pos2(tl[0] + tl[2] * 0.6, tl[1] + tl[3] * 0.75); + let (before, undo) = (d.project(), d.undo_len()); + d.drag(from, to, true); + assert_eq!(d.project(), before, "nothing was placed on the Timeline"); + assert_eq!(d.undo_len(), undo); + d.drag(from, to, false); + assert_ne!(d.project(), before, "without Escape the same drag places the clip"); +} + +#[test] +fn escape_puts_a_freeform_card_back_and_the_next_drag_works() { + let mut d = Driver::demo(); + let items = d.footage_in("freeform"); + let from = centre(d.rect(&format!("project.item.{}", items[0]))); + let to = from + egui::vec2(90.0, 60.0); + d.drag(from, to, true); + let meta = |d: &Driver| d.harness.state().session.project.item(ItemId(items[0])).unwrap().metadata.contains_key(FREEFORM_POS); + assert!(!meta(&d), "the card was not placed"); + d.drag(from, to, false); + assert!(meta(&d), "after a cancel the next drag places the card"); +} + +#[test] +fn escape_cancels_dragging_an_effect_onto_a_clip() { + let mut d = Driver::demo(); + d.ok("ui.panel.show", json!({"panel": "Effects"})); + let effects = d.ok("ui.elements", json!({"prefix": "effects.item."})); + let id = effects.as_array().unwrap().iter().filter_map(|e| e["id"].as_str()).find(|i| i.matches('.').count() == 2).expect("an effect").to_string(); + let from = centre(d.rect(&id)); + let clip = d.harness.state().session.active_sequence().unwrap().video_tracks[0].items[0].id; + let to = centre(d.rect(&format!("timeline.clip.{}", clip.0))); + let (before, undo) = (d.project(), d.undo_len()); + d.drag(from, to, true); + assert_eq!(d.project(), before, "no effect was applied"); + assert_eq!(d.undo_len(), undo); + d.drag(from, to, false); + assert_ne!(d.project(), before, "without Escape the same drag applies {id}"); +} diff --git a/crates/ui-egui/tests/project_panel_ui.rs b/crates/ui-egui/tests/project_panel_ui.rs index 19187508..3f414228 100644 --- a/crates/ui-egui/tests/project_panel_ui.rs +++ b/crates/ui-egui/tests/project_panel_ui.rs @@ -1002,3 +1002,39 @@ fn marquee_selects_cards_in_freeform_view() { assert_eq!(selection(&mut d), sorted(vec![i0, i1])); assert!(!d.app().session.project.item(ItemId(i0)).unwrap().metadata.contains_key(FREEFORM_POS), "the marquee moved no card"); } + +/// Escape during a Media Browser drag (#580) takes the import back, even over the Timeline. +#[test] +fn escape_takes_back_a_media_browser_drag() { + let dir = media_dir("drag-escape"); + let mut session = Session::default(); + session.execute("file.openDemoProject", json!({})).unwrap(); + let mut d = Driver::with(session); + d.ok("ui.set", json!({"workspace": "Editing"})); + d.ok("ui.panel.show", json!({"panel": "MediaBrowser"})); + d.exec("mediaBrowser.navigate", json!({"path": dir.to_string_lossy()})); + d.frames(3); + d.wait_for("mediaBrowser.entry.b-roll.wav"); + let (n0, before) = (d.app().session.project.items.len(), d.app().session.project.to_json()); + let e = d.rect("mediaBrowser.entry.b-roll.wav"); + let tl = d.rect("panel.Timeline"); + let (from, to) = (egui::pos2(e[0] + 40.0, e[1] + e[3] / 2.0), egui::pos2(tl[0] + tl[2] * 0.6, tl[1] + tl[3] * 0.75)); + let mut send = |e: egui::Event| { + d.harness.input_mut().events.push(e); + d.frames(1); + }; + let button = |pos, pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }; + send(egui::Event::PointerMoved(from)); + send(button(from, true)); + for k in 1..=20 { + send(egui::Event::PointerMoved(from.lerp(to, k as f32 / 20.0))); + if k == 10 { + send(egui::Event::Key { key: egui::Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }); + } + } + send(button(to, false)); + d.frames(6); + assert_eq!(d.app().session.project.items.len(), n0, "the import was taken back"); + assert_eq!(d.app().session.project.to_json(), before, "nothing was placed on the Timeline"); + let _ = std::fs::remove_dir_all(&dir); +}