diff --git a/crates/engine/src/history.rs b/crates/engine/src/history.rs index 86c46fcf21..8df6060e02 100644 --- a/crates/engine/src/history.rs +++ b/crates/engine/src/history.rs @@ -19,12 +19,20 @@ pub struct HistoryEntry { next: usize, /// The template and name `label` was made from ([`Session::edit_named`]). pub(crate) named: Option, + /// A number no other step takes ([`History::step`]). + serial: u64, /// Original and converted new-art colors, only for steps that change document color mode. pub(crate) color_mode_paints: Option<(vectorcraft_tools::PaintDefaults, vectorcraft_tools::PaintDefaults)>, /// Owned new-art snapshots included in both directional estimates. color_mode_bytes: usize, } +/// The next [`HistoryEntry::serial`]: 1 for the first step made, so 0 never names a step. +fn next_serial() -> u64 { + static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); + NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed) +} + /// A step name made by filling the `{name}` placeholder of a template with a name /// ([`Session::edit_named`]). #[derive(Clone, Debug, PartialEq, Eq)] @@ -57,6 +65,7 @@ impl HistoryEntry { next_bytes, next: std::ptr::from_ref(next) as usize, named: None, + serial: next_serial(), color_mode_paints: None, color_mode_bytes: 0, } @@ -88,6 +97,7 @@ impl HistoryEntry { next_bytes: costs.bytes, next: Arc::as_ptr(now) as usize, named: None, + serial: next_serial(), color_mode_paints: None, color_mode_bytes: 0, } @@ -155,6 +165,13 @@ impl History { self.undo.iter().chain(&self.redo).map(|e| e.bytes).sum() } + /// Which step the document is at: the newest undo step and the next redo step, by numbers no + /// other step takes (0 for none). Every new step, Undo, Redo and edit merged into the newest + /// step (an undo group) changes it; a selection change does not. + pub fn step(&self) -> (u64, u64) { + (self.undo.last().map_or(0, |e| e.serial), self.redo.last().map_or(0, |e| e.serial)) + } + /// The document at row `row` of the History panel, `now` being the document now: the rows before /// it are the undo steps, oldest first, and the rows after it the redo steps, nearest first. pub(crate) fn state<'a>(&'a self, now: &'a Arc, row: usize) -> Option<&'a Arc> { @@ -192,13 +209,14 @@ impl History { } /// Estimate the newest undo step again, its next state now being `now` (the steps after it were - /// merged into it, or an undo group extended it). + /// merged into it, or an undo group extended it). It takes a new number ([`History::step`]). pub(crate) fn recost_last(&mut self, now: &Document) { if let Some(e) = self.undo.last_mut() { let (before, after) = e.doc.retained_bytes_each(now); e.bytes = before.saturating_add(e.color_mode_bytes); e.next_bytes = after.saturating_add(e.color_mode_bytes); e.next = std::ptr::from_ref(now) as usize; + e.serial = next_serial(); } } } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 03082e7f57..3dc6414009 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -62,6 +62,10 @@ pub struct Session { /// While Cmd lends `tool` (a selection tool) for a drag: the tool it was lent to, which comes /// back as it was at the release ([`Session::pointer`]). pub(crate) lender: Option>, + /// The active document (its uid) and its [`History::step`] when the tool last handled a pointer + /// event or drew its overlays: when they differ at the next one, the tool is told first + /// ([`vectorcraft_tools::Tool::history_moved`]). None until the first. + pub(crate) tool_step: Option<(u64, (u64, u64))>, /// The selection of the active document (its uid) as Cmd lent `tool`: what the lent tool's /// gesture found, so that one that leaves it as it was picks nothing. pub(crate) selection_at_lend: Option<(u64, vectorcraft_doc::Selection)>, diff --git a/crates/engine/src/session/documents.rs b/crates/engine/src/session/documents.rs index c3c112bd3f..832a229610 100644 --- a/crates/engine/src/session/documents.rs +++ b/crates/engine/src/session/documents.rs @@ -46,6 +46,7 @@ impl Session { journal: Journal::default(), tool: vectorcraft_tools::create("selection"), lender: None, + tool_step: None, selection_at_lend: None, lent_selection: None, last_selection_tool: None, diff --git a/crates/engine/src/tests/blendopts.rs b/crates/engine/src/tests/blendopts.rs index e4d19be3bc..059357ddb3 100644 --- a/crates/engine/src/tests/blendopts.rs +++ b/crates/engine/src/tests/blendopts.rs @@ -1,6 +1,6 @@ //! Blend Options (defaults with nothing selected, `object.blend.info`) and the Blend tool: //! start points from clicked anchors, adding keys to a blend instead of nesting it (M8.18), and -//! Escape dropping the first pick. +//! Escape, or an edit made outside the tool, dropping the first pick. use serde_json::{Value, json}; use vectorcraft_doc::live::{BlendOrientation, BlendSpacing, BlendSpec}; @@ -233,6 +233,102 @@ fn escape_drops_the_blend_tools_first_pick() { assert_eq!(spec(&s, g).0, vec![b, c], "a was not added"); } +#[test] +fn an_edit_undo_or_redo_made_outside_the_blend_tool_drops_its_first_pick() { + for case in ["edit", "undo", "redo", "group"] { + let undo = case == "undo"; + let mut s = session(); + let (a, b, c) = (rect(&mut s, 0.0, 0.0, 40.0), rect(&mut s, 200.0, 0.0, 40.0), rect(&mut s, 400.0, 0.0, 40.0)); + if case == "redo" { + s.execute("paint.setFill", &json!({"ids": [c.0], "color": "#ff0000"})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + } + // In an open undo group, the edit after the pick is merged into the group's step. + if case == "group" { + s.begin_undo_group(); + s.execute("paint.setFill", &json!({"ids": [c.0], "color": "#00ff00"})).unwrap(); + } + s.select_tool("blend", ViewInfo::default()).unwrap(); + let click = |s: &mut Session, x: f64, y: f64| s.pointer(&PointerEvent::new(PointerKind::Down, x, y), ViewInfo::default()).unwrap(); + // Undo takes away c, the object picked first; an edit or a redo recolors c, which is not picked. + click(&mut s, if undo { 420.0 } else { 20.0 }, 20.0); + assert!(!s.overlays(ViewInfo::default()).is_empty()); + match case { + "undo" => s.execute("edit.undo", &json!({})).unwrap(), + "redo" => s.execute("edit.redo", &json!({})).unwrap(), + _ => s.execute("paint.setFill", &json!({"ids": [c.0], "color": "#ff0000"})).unwrap(), + }; + assert!(s.overlays(ViewInfo::default()).is_empty(), "{case}: the pick is not marked"); + let (layers, depth) = (s.doc().unwrap().doc.layers.clone(), s.doc().unwrap().history.undo.len()); + click(&mut s, 220.0, 20.0); + assert_eq!(s.doc().unwrap().selection.objects, vec![b], "{case}: b is picked first"); + assert_eq!((&s.doc().unwrap().doc.layers, s.doc().unwrap().history.undo.len()), (&layers, depth), "{case}: the click changes no art"); + click(&mut s, if undo { 20.0 } else { 420.0 }, 20.0); + let g = s.doc().unwrap().selection.objects[0]; + assert_eq!(spec(&s, g).0, if undo { vec![a, b] } else { vec![b, c] }, "{case}"); + } +} + +#[test] +fn two_picks_with_no_edit_between_them_blend() { + let mut s = session(); + let (a, b) = (rect(&mut s, 0.0, 0.0, 40.0), rect(&mut s, 200.0, 0.0, 40.0)); + s.select_tool("blend", ViewInfo::default()).unwrap(); + let click = |s: &mut Session, x: f64, y: f64| s.pointer(&PointerEvent::new(PointerKind::Down, x, y), ViewInfo::default()).unwrap(); + click(&mut s, 20.0, 20.0); + // Drawing the overlays between the picks does not drop the first one. + assert!(!s.overlays(ViewInfo::default()).is_empty()); + click(&mut s, 220.0, 20.0); + let g = s.doc().unwrap().selection.objects[0]; + assert_eq!(spec(&s, g).0, vec![a, b]); +} + +#[test] +fn removing_the_blend_the_blend_tool_adds_to_ends_adding() { + let mut s = session(); + let (a, b, c) = (rect(&mut s, 0.0, 0.0, 40.0), rect(&mut s, 200.0, 0.0, 40.0), rect(&mut s, 400.0, 0.0, 40.0)); + let (d, e) = (rect(&mut s, 0.0, 300.0, 40.0), rect(&mut s, 200.0, 300.0, 40.0)); + let other = id_of(&s.execute("object.blend.make", &json!({"ids": [d.0, e.0]})).unwrap()); + s.select_tool("blend", ViewInfo::default()).unwrap(); + let click = |s: &mut Session, x: f64, y: f64| s.pointer(&PointerEvent::new(PointerKind::Down, x, y), ViewInfo::default()).unwrap(); + click(&mut s, 20.0, 20.0); + click(&mut s, 220.0, 20.0); + let g = s.doc().unwrap().selection.objects[0]; + s.execute("edit.clear", &json!({"ids": [g.0]})).unwrap(); + sel(&mut s, &[other]); + click(&mut s, 420.0, 20.0); + assert_eq!(s.doc().unwrap().selection.objects, vec![c], "c is picked first"); + assert_eq!(spec(&s, other).0, vec![d, e], "c is not added to the selected blend"); + assert!([a, b, g].iter().all(|id| s.doc().unwrap().doc.node(*id).is_none()), "the blend went with its keys"); +} + +#[test] +fn a_selection_change_keeps_the_blend_tools_first_pick() { + for drag in [false, true] { + let mut s = session(); + let (a, b, c) = (rect(&mut s, 0.0, 0.0, 40.0), rect(&mut s, 200.0, 0.0, 40.0), rect(&mut s, 400.0, 0.0, 40.0)); + let view = ViewInfo::default(); + s.select_tool("blend", view).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Down, 20.0, 20.0), view).unwrap(); + sel(&mut s, &[c]); + // Cmd lends the selection tool: a click on c selects it, a drag moves it (an edit). + let cmd = Mods { cmd: true, ..Mods::default() }; + let to = if drag { 440.0 } else { 420.0 }; + s.pointer(&PointerEvent::new(PointerKind::Down, 420.0, 20.0).with_mods(cmd), view).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Drag, to, 20.0).with_mods(cmd), view).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Up, to, 20.0).with_mods(cmd), view).unwrap(); + assert_eq!(s.tool_id(), "blend"); + assert_eq!(s.overlays(view).is_empty(), drag, "drag {drag}"); + s.pointer(&PointerEvent::new(PointerKind::Down, 220.0, 20.0), view).unwrap(); + if drag { + assert_eq!(s.doc().unwrap().selection.objects, vec![b], "the drag dropped the pick"); + } else { + let g = s.doc().unwrap().selection.objects[0]; + assert_eq!(spec(&s, g).0, vec![a, b]); + } + } +} + #[test] fn start_points_round_trip_and_old_blends_load() { let mut s = session(); diff --git a/crates/engine/src/tests/history_mem.rs b/crates/engine/src/tests/history_mem.rs index d3cbbda95f..d21d9ec971 100644 --- a/crates/engine/src/tests/history_mem.rs +++ b/crates/engine/src/tests/history_mem.rs @@ -180,6 +180,47 @@ fn redo_during_an_interaction_keeps_the_history_and_the_document_in_step() { assert_eq!(first_x(&s), start); } +/// `History::step` changes with every new step, Undo and Redo, also when an undo and a new edit +/// leave as many steps as before, and with an edit merged into an undo group's step. It stays the +/// same for a selection change. +#[test] +fn the_history_step_changes_with_each_step_undo_and_redo() { + let mut s = session_with_rects(2); + let step = |s: &Session| s.doc().unwrap().history.step(); + let start = step(&s); + assert!(start.0 != 0 && start.1 == 0, "the Add step, and nothing to redo: {start:?}"); + s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); + let moved = step(&s); + assert!(moved.0 != start.0 && moved.1 == 0, "{moved:?}"); + s.execute("edit.undo", &json!({})).unwrap(); + let undone = step(&s); + assert!(undone.0 == start.0 && undone.1 != 0, "{undone:?}"); + s.execute("edit.redo", &json!({})).unwrap(); + let redone = step(&s); + assert!(![start, moved, undone].contains(&redone) && redone.1 == 0, "{redone:?}"); + // An undo and a new edit: as many undo steps as after the move, at a different step. + s.execute("edit.undo", &json!({})).unwrap(); + let depth = s.doc().unwrap().history.undo.len(); + s.execute("object.move", &json!({"dx": 0, "dy": 1})).unwrap(); + let other = step(&s); + assert_eq!(s.doc().unwrap().history.undo.len(), depth + 1); + assert!(![start, moved, undone, redone].contains(&other), "{other:?}"); + s.execute("select.none", &json!({})).unwrap(); + assert_eq!(step(&s), other, "a selection change is no step"); + s.execute("select.all", &json!({})).unwrap(); + s.begin_undo_group(); + s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); + let (grouped, depth) = (step(&s), s.doc().unwrap().history.undo.len()); + assert_ne!(grouped, other); + s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); + assert_eq!(s.doc().unwrap().history.undo.len(), depth, "the second move is merged into the first"); + let merged = step(&s); + assert!(merged != grouped && merged.1 == 0, "a merged edit moves the step: {merged:?}"); + s.execute("select.none", &json!({})).unwrap(); + assert_eq!(step(&s), merged); + s.end_undo_group(false); +} + #[test] fn color_mode_paint_snapshots_count_toward_the_history_budget() { let mut s = Session::new(); diff --git a/crates/engine/src/tooling.rs b/crates/engine/src/tooling.rs index 28b014ee54..d195f74f9b 100644 --- a/crates/engine/src/tooling.rs +++ b/crates/engine/src/tooling.rs @@ -402,6 +402,25 @@ impl Session { self.tool_panic.take().map_or(Ok(()), Err) } + /// The active document (its uid) and its [`crate::History::step`]. + fn history_step(&self) -> Option<(u64, (u64, u64))> { + self.active().map(|d| (d.uid, d.history.step())) + } + + /// Tell the active tool when the history moved, or another document became active, since it + /// last handled a pointer event or was last told ([`vectorcraft_tools::Tool::history_moved`]). + /// A lent selection tool is not told. + fn tell_tool_history_moved(&mut self) { + if self.lender.is_some() { + return; + } + let now = self.history_step(); + if self.tool_step.is_some_and(|seen| Some(seen) != now) { + self.tool.history_moved(); + } + self.tool_step = now; + } + /// Feed a pointer event to the active tool. Returns requests for the UI. pub fn pointer(&mut self, ev: &PointerEvent, view: ViewInfo) -> Result> { self.pointer_event(ev, view, true) @@ -495,7 +514,9 @@ impl Session { return r; } self.leave_draw_inside(ev, view)?; + self.tell_tool_history_moved(); let ev = &self.lend_selection_tool(ev); + let lent = self.lender.is_some(); let mut acts = self.with_tool_cx(view, |t, cx| t.pointer(cx, ev)); if !keep { acts.retain(|a| matches!(a, Action::Cancel | Action::Notify(_))); @@ -507,6 +528,11 @@ impl Session { } self.apply_actions(acts) }); + // The tool's own edits do not move the history for it. A selection tool that Cmd lends + // edits for the lender, which hears of the edit at its next event. + if !lent { + self.tool_step = self.history_step(); + } match ev.kind { PointerKind::Down => self.press_point = Some(self.freeform_point), PointerKind::Up => self.press_point = None, @@ -764,6 +790,8 @@ impl Session { } pub fn overlays(&mut self, view: ViewInfo) -> Vec { + // The Blend tool's first pick is not marked after an edit that dropped it. + self.tell_tool_history_moved(); let mut v = self.with_tool_cx(view, |t, cx| t.overlays(cx)); // The Type tools draw the ports of selected type themselves. if !matches!(self.tool.id(), "type" | "areaType" | "typeOnPath" | "verticalType" | "verticalAreaType" | "verticalTypeOnPath") { diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index b9f88c03b7..8fd4462e25 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -239,6 +239,10 @@ pub trait Tool: Send { fn after_command(&mut self, _cx: &ToolContext) -> Vec { vec![] } + /// The undo history moved outside the tool (an edit, Undo or Redo the tool did not make), or + /// another document became active. The Blend tool drops its first pick and stops adding to the + /// blend it just made. + fn history_moved(&mut self) {} /// Time passes while the pointer button is held (`dt` seconds since the last tick, whether or /// not the pointer moved): tools that keep working while the brush holds still (Twirl, Pucker, /// Bloat) act on it. The host supplies the time, so tests and agents drive it exactly. diff --git a/crates/tools/src/meshblend.rs b/crates/tools/src/meshblend.rs index eaa13cf422..5edc14ad42 100644 --- a/crates/tools/src/meshblend.rs +++ b/crates/tools/src/meshblend.rs @@ -180,6 +180,11 @@ impl Tool for BlendTool { fn chosen_again(&mut self, cx: &ToolContext) -> Vec { self.deactivate(cx) } + /// An edit, Undo or Redo made outside the tool drops the object picked first and stops adding to + /// the blend just made, so the next click picks a new first object. + fn history_moved(&mut self) { + (self.first, self.chained) = (None, false); + } } #[derive(Default)]