From 5846b00da3ad7f09ffe3b07630c6b06320759e78 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 20 Oct 2026 21:38:15 -0700 Subject: [PATCH] Actions: a recording keeps its steps when a canceled undo group drops earlier entries When a cancel removes journal entries numbered below a recording's start, the entries after them are numbered down. The recording's start and the journal's hold kept the old number, so Stop Recording left out as many of the recorded steps as there were entries removed, and the hold no longer covered them. `Journal::remove` now moves the hold down with the entries, and Stop Recording reads the start from the hold. `Journal::replace` puts an entry in place of another and counts it at its new size. A cancel uses it for the `document.activate` that takes the place of a canceled group's entry that switched documents, so the journal's size estimate does not keep the old entry's size. --- crates/engine/src/journal.rs | 32 ++++++++++++++++++++++----- crates/engine/src/lib.rs | 6 ++--- crates/ui-egui/src/panels/actions.rs | 33 ++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/journal.rs b/crates/engine/src/journal.rs index b57697d69..230bcfba0 100644 --- a/crates/engine/src/journal.rs +++ b/crates/engine/src/journal.rs @@ -105,16 +105,33 @@ impl Journal { !gone(i - 1) }); self.bytes = self.sizes.iter().sum(); - // Each entry that was let go already counts once. - let mut last = None; - let let_go = numbers.iter().take_while(|&&n| n < dropped).filter(|&&n| last.replace(n) != Some(n)).count(); - self.dropped = dropped.saturating_sub(let_go); + // How many of the numbers are below `n`, each counted once. + let below = |n: usize| { + let mut last = None; + numbers.iter().take_while(|&&m| m < n).filter(|&&m| last.replace(m) != Some(m)).count() + }; + self.held = self.held.map(|h| h.saturating_sub(below(h))); + self.dropped = dropped.saturating_sub(below(dropped)); + } + /// Put `e` in place of entry number `i`, if kept. + pub fn replace(&mut self, i: usize, e: JournalEntry) { + let Some(k) = i.checked_sub(self.dropped) else { return }; + if let (Some(old), Some(size)) = (self.entries.get_mut(k), self.sizes.get_mut(k)) { + *size = entry_bytes(&e); + *old = e; + self.bytes = self.sizes.iter().sum(); + } } /// Keep every entry from number `from` on (a recording in progress), or stop (`None`). pub fn hold(&mut self, from: Option) { self.held = from; self.trim(); } + /// The number the entries are kept from ([`Journal::hold`]). [`Journal::remove`] moves it down + /// with the entries it numbers down. + pub fn held(&self) -> Option { + self.held + } /// Let the oldest entries go while over the bounds (in batches: a quarter of the entry bound /// at a time, so a push doesn't shift the whole list every time). @@ -223,12 +240,17 @@ mod tests { j.push(("object.move".into(), json!({"dx": i}))); } let first = j.first_kept(); + // A recording from entry 29,500 on: the entries removed before it move its start down. + j.hold(Some(29_500)); // One entry that was let go already and two kept ones, two of them given twice. j.remove(&[first - 1, first - 1, 29_000, 29_000, 29_998]); - assert_eq!((j.len(), j.first_kept()), (29_997, first - 1)); + assert_eq!((j.len(), j.first_kept(), j.held()), (29_997, first - 1, Some(29_498))); let dx = |j: &Journal, i: usize| j.get(i).map(|e| e.1["dx"].clone()); assert_eq!((dx(&j, 28_998), dx(&j, 28_999)), (Some(json!(28_999)), Some(json!(29_001)))); assert_eq!(j.last().map(|e| e.1["dx"].clone()), Some(json!(29_999))); assert_eq!(j.bytes, j.sizes.iter().sum::()); + // A replaced entry is counted at its new size. + j.replace(29_000, big(0)); + assert_eq!(j.bytes, j.iter().map(entry_bytes).sum::()); } } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 76ecc1ba0..34a9c86da 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -2046,10 +2046,8 @@ impl Session { // An entry that changed the active document becomes that change, so the entries after it run // in the same document when the journal is replayed. for &(n, index) in &journal { - if let Some(index) = index - && let Some(e) = self.journal.get_mut(n) - { - *e = ("document.activate".to_string(), serde_json::json!({ "index": index })); + if let Some(index) = index { + self.journal.replace(n, ("document.activate".to_string(), serde_json::json!({ "index": index }))); } } let gone: Vec = journal.into_iter().filter(|(_, index)| index.is_none()).map(|(n, _)| n).collect(); diff --git a/crates/ui-egui/src/panels/actions.rs b/crates/ui-egui/src/panels/actions.rs index 5eb641217..c7f6974ae 100644 --- a/crates/ui-egui/src/panels/actions.rs +++ b/crates/ui-egui/src/panels/actions.rs @@ -118,6 +118,8 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut egui::Ui) { if widgets::icon_button(ui, "square", tl!("Stop Playing/Recording"), false, 24.0).clicked() && let Some((set, name, start)) = app.ui.recording.take() { + // A canceled undo group numbers the steps down when it drops entries made before them. + let start = app.session.journal.held().unwrap_or(start); let steps = app.session.journal_for_action(start); app.session.journal.hold(None); if let Some(s) = app.ui.action_sets.get_mut(set) { @@ -211,6 +213,37 @@ mod tests { assert_eq!(names(&app), before[1..], "nothing selected after a delete: nothing more goes"); } + /// A recording keeps its steps when a canceled undo group drops journal entries made before the + /// recording began. + #[test] + fn a_recording_keeps_its_steps_when_a_canceled_undo_group_drops_earlier_entries() { + use super::super::tests_appearance::{click, frame_events}; + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + let rect = |x: i32| json!({"x": x, "y": 10, "width": 20, "height": 20}); + for (id, p) in [ + ("file.new", json!({"width": 200, "height": 100})), + ("file.new", json!({"width": 200, "height": 100})), + ("document.activate", json!({"index": 0})), + ("edit.undoGroup.begin", json!({})), + ("shape.rectangle", rect(10)), + ] { + app.run(id, p).unwrap(); + } + let ctx = egui::Context::default(); + frame_events(&ctx, &mut app, vec![], show); + // The bottom bar's 24-point buttons, left to right: stop, record, play, delete. + let mut bar = crate::toolbar::tests::widget_rects(&ctx, egui::vec2(24.0, 24.0)); + bar.sort_by(|a, b| a.left().total_cmp(&b.left())); + let [stop, record, _, _] = bar[..] else { panic!("the bottom bar: {bar:?}") }; + click(&ctx, &mut app, record.center(), show); + for (id, p) in [("document.activate", json!({"index": 1})), ("shape.rectangle", rect(50)), ("edit.undoGroup.end", json!({"cancel": true}))] { + app.run(id, p).unwrap(); + } + click(&ctx, &mut app, stop.center(), show); + let steps = app.ui.action_sets[0].actions.last().map(|a| a.steps.iter().map(|(c, _)| c.clone()).collect::>()); + assert_eq!(steps, Some(vec!["document.activate".to_string(), "shape.rectangle".into(), "edit.undoGroup.end".into()])); + } + /// Actions recorded with Align to Artboard and Paste in Place, and the default Center on /// Artboard, play on the artboard active when they play, not on the one active when they were /// recorded.