Fix two edge cases in multi-select placement

A selection that holds a bin beside items (one id space) made the whole drop
fail with "no such item"; only items are placed now.

Session::edit_group raised the history limit while a group ran and did not
restore it if the group panicked. It now restores it and rolls the group back
before the panic goes on.
This commit is contained in:
awe 2026-10-11 11:12:24 +07:00
parent fb4546e9d6
commit a8dac81d97
4 changed files with 56 additions and 11 deletions

View File

@ -840,11 +840,20 @@ impl Session {
(self.project.clone(), self.state.clone(), self.history.undo.len(), self.history.redo.clone(), self.history.limit);
// keep every step the group makes, however many, so the first one can be found afterwards
self.history.limit = usize::MAX;
let r = f(self);
// a panic inside the group must not leave the history unlimited: restore, then let it go on
let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(self)));
self.history.limit = limit;
let first = self.history.undo.get(undo).map(|e| e.1.clone());
let rolled_back = |s: &mut Session| {
s.history.undo.truncate(undo);
s.history.redo = redo.clone();
s.history.merge_key = None;
s.project = project.clone();
s.state = state.clone();
s.bump();
};
match (r, first) {
(Ok(v), Some(first)) => {
(Ok(Ok(v)), Some(first)) => {
self.history.undo.truncate(undo);
self.history.undo.push((label.to_string(), first));
if self.history.undo.len() > limit {
@ -853,16 +862,15 @@ impl Session {
self.history.merge_key = None;
Ok(v)
}
(Ok(v), None) => Ok(v),
(Err(e), _) => {
self.history.undo.truncate(undo);
self.history.redo = redo;
self.history.merge_key = None;
self.project = project;
self.state = state;
self.bump();
(Ok(Ok(v)), None) => Ok(v),
(Ok(Err(e)), _) => {
rolled_back(self);
Err(e)
}
(Err(panic), _) => {
rolled_back(self);
std::panic::resume_unwind(panic)
}
}
}

View File

@ -123,3 +123,22 @@ fn items_on_a_new_track_share_that_one_track() {
s.undo();
assert_eq!(s.active_sequence().unwrap().video_tracks.len(), tracks, "undo removes the track too");
}
/// A panic inside a group leaves the project, the history and its limit as they were.
#[test]
fn a_panic_in_a_group_rolls_back_and_keeps_the_history_limit() {
let (mut s, items) = three();
let (steps, limit) = (s.history.undo.len(), s.history.limit);
let before = s.project.to_json();
let one = items[0];
let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
let _ = s.edit_group::<()>("Boom", |s| {
s.execute("timeline.place", json!({"item": one.0, "time": 0}))?;
panic!("inside the group");
});
}));
assert!(outcome.is_err(), "the panic goes on");
assert_eq!(s.history.limit, limit, "the history is not left unlimited");
assert_eq!(s.history.undo.len(), steps);
assert_eq!(s.project.to_json(), before);
}

View File

@ -182,7 +182,8 @@ pub fn dragged_project_item(ui: &egui::Ui) -> Option<ItemId> {
/// one of several selected, otherwise just the dragged item.
pub fn dragged_selection(app: &FilmcraftApp, item: ItemId) -> Vec<ItemId> {
let sel = &app.session.state.project_selection;
if sel.len() > 1 && sel.contains(&item) { sel.clone() } else { vec![item] }
// the selection can hold bins beside items (one id space): only items can be placed
if sel.len() > 1 && sel.contains(&item) { sel.iter().copied().filter(|i| app.session.project.item(*i).is_some()).collect() } else { vec![item] }
}
pub fn dragged_effect(ui: &egui::Ui) -> Option<String> {
match payload(ui) {

View File

@ -1039,3 +1039,20 @@ 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");
}
/// The Project selection can hold a bin beside items (one id space). Dragging the items still
/// places them: the bin is left out instead of failing the whole drop.
#[test]
fn a_bin_in_the_selection_does_not_stop_the_items_being_placed() {
let mut d = Driver::demo();
let (bin, items) = d.footage();
d.exec("file.newSequence", json!({"name": "Drop", "video": 2, "audio": 2}));
d.ok("ui.menu.invoke", json!({"id": "projectPanel.openBin", "params": {"bin": bin, "how": "inPlace"}}));
d.wait_for(&format!("project.item.{}", items[0]));
d.exec("project.select", json!({"items": [items[0], bin, items[1]]}));
d.frames(2);
d.ok("ui.drag", json!({"from": {"id": format!("project.item.{}", items[0])}, "to": {"id": "timeline.tracks", "fx": 0.3, "fy": 0.1}, "steps": 16}));
d.frames(4);
let clips = d.app().session.active_sequence().map(|q| q.video_tracks.iter().map(|t| t.items.len()).sum::<usize>()).unwrap_or(0);
assert_eq!(clips, 2, "status: {}", d.app().ui.status);
}