diff --git a/crates/engine/src/history.rs b/crates/engine/src/history.rs index ec53acd5b..ee6adccc4 100644 --- a/crates/engine/src/history.rs +++ b/crates/engine/src/history.rs @@ -19,6 +19,14 @@ 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, +} + +/// 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 @@ -45,7 +53,7 @@ impl HistoryEntry { pub(crate) fn kept(label: String, doc: Arc, selection: Arc, next: &Document) -> Self { doc.drop_lookup_cache(); let (bytes, next_bytes) = doc.retained_bytes_each(next); - HistoryEntry { label, doc, selection, bytes, next_bytes, next: std::ptr::from_ref(next) as usize, named: None } + HistoryEntry { label, doc, selection, bytes, next_bytes, next: std::ptr::from_ref(next) as usize, named: None, serial: next_serial() } } /// The template and name the step's label was made from, while the label is still the one @@ -66,7 +74,16 @@ impl HistoryEntry { return Self::kept(label, doc, selection, now); } doc.drop_lookup_cache(); - HistoryEntry { label, doc, selection, bytes: costs.next_bytes, next_bytes: costs.bytes, next: Arc::as_ptr(now) as usize, named: None } + HistoryEntry { + label, + doc, + selection, + bytes: costs.next_bytes, + next_bytes: costs.bytes, + next: Arc::as_ptr(now) as usize, + named: None, + serial: next_serial(), + } } /// The figures [`HistoryEntry::turned`] reuses. @@ -111,6 +128,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 and Redo changes it; an edit merged into + /// the newest step (an undo group) and a selection change do 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)) + } + /// Let the oldest steps go while over the step limit or the memory budget: undo steps first /// (the oldest), then redo steps (the furthest ahead). pub fn trim(&mut self) { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 7f32ca994..226c391d3 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -60,6 +60,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 tool chosen last (Selection, Direct Selection or Group Selection): the one Cmd /// lends the other tools. None until one is chosen. pub(crate) last_selection_tool: Option<&'static str>, diff --git a/crates/engine/src/session/documents.rs b/crates/engine/src/session/documents.rs index 89d846330..c073369aa 100644 --- a/crates/engine/src/session/documents.rs +++ b/crates/engine/src/session/documents.rs @@ -31,6 +31,7 @@ impl Session { journal: Journal::default(), tool: vectorcraft_tools::create("selection"), lender: None, + tool_step: None, last_selection_tool: None, last_view: ViewInfo::default(), preview_memo: Default::default(), diff --git a/crates/engine/src/tests/blendopts.rs b/crates/engine/src/tests/blendopts.rs index 97f356078..ec05893f8 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}; @@ -167,6 +167,78 @@ 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_or_undo_made_outside_the_blend_tool_drops_its_first_pick() { + for undo 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)); + 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 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()); + if undo { + s.execute("edit.undo", &json!({})).unwrap(); + } else { + s.execute("paint.setFill", &json!({"ids": [c.0], "color": "#ff0000"})).unwrap(); + } + assert!(s.overlays(ViewInfo::default()).is_empty(), "undo {undo}: 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], "undo {undo}: b is picked first"); + assert_eq!((&s.doc().unwrap().doc.layers, s.doc().unwrap().history.undo.len()), (&layers, depth), "undo {undo}: 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] }, "undo {undo}"); + } +} + +#[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/tooling.rs b/crates/engine/src/tooling.rs index 81f8616a8..94d2d6742 100644 --- a/crates/engine/src/tooling.rs +++ b/crates/engine/src/tooling.rs @@ -328,6 +328,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.last_view = view; @@ -341,7 +360,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 acts = self.with_tool_cx(view, |t, cx| t.pointer(cx, ev)); let r = self.take_tool_panic().and_then(|()| { // A gesture may change options (Alt-drag sizes a Liquify brush): keep them. @@ -350,6 +371,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(); + } if ev.kind == PointerKind::Up { self.give_back_tool(view)?; } @@ -553,6 +579,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 e55bb4069..686e00408 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -224,6 +224,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 d6a46faf0..e881d54c4 100644 --- a/crates/tools/src/meshblend.rs +++ b/crates/tools/src/meshblend.rs @@ -148,6 +148,11 @@ impl Tool for BlendTool { (self.first, self.chained) = (None, false); vec![] } + /// 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)]