From bbbafdef6b68ff5c0a702a56a16d5895387b1531 Mon Sep 17 00:00:00 2001 From: arhammusheer Date: Sun, 11 Oct 2026 23:28:11 +0530 Subject: [PATCH] #1244: undo history bounded by bytes, without lookup tables or repeated selections in its steps History was limited by step count only; a whole-selection move of 50k paths keeps ~40 MB per step. Steps now carry a size estimate (only unshared subtrees are visited) and the oldest are dropped beyond a byte budget, keeping the last 8. Documents entering history drop their id index, selections are shared between steps, the command journal is bounded (held while an Action records), and document resources are copy-on-write. Saved files are byte-identical. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VmMmc1J8XeNLxhRihWv9cc --- crates/doc/src/id_index.rs | 13 + crates/doc/src/lib.rs | 70 ++-- crates/doc/src/mem.rs | 318 ++++++++++++++++++ crates/doc/src/shared.rs | 102 ++++++ crates/engine/src/cmd/clipboard.rs | 4 +- crates/engine/src/cmd/distortcmds.rs | 6 +- crates/engine/src/cmd/docmenu.rs | 4 +- crates/engine/src/cmd/edit.rs | 28 +- crates/engine/src/cmd/menucmds.rs | 1 + crates/engine/src/cmd/prefscmds.rs | 3 +- crates/engine/src/cmd/swatchlib.rs | 2 +- crates/engine/src/cmd/textedit.rs | 2 +- crates/engine/src/journal.rs | 197 +++++++++++ crates/engine/src/lib.rs | 152 +++++++-- crates/engine/src/tests_history_mem.rs | 126 +++++++ crates/engine/src/tests_journal.rs | 6 +- crates/engine/tests/pen_endpoint_picking.rs | 10 +- crates/eps/src/import/ai/read.rs | 3 +- crates/eps/src/import/mod.rs | 3 +- crates/pdf/src/import.rs | 3 +- .../src/dialogs/tests_rectangular_grid.rs | 2 +- crates/ui-egui/src/panels/actions.rs | 6 +- crates/ui-egui/src/panels/swatches.rs | 4 +- 23 files changed, 989 insertions(+), 76 deletions(-) create mode 100644 crates/doc/src/mem.rs create mode 100644 crates/engine/src/journal.rs create mode 100644 crates/engine/src/tests_history_mem.rs diff --git a/crates/doc/src/id_index.rs b/crates/doc/src/id_index.rs index cdb32c32..582e4c25 100644 --- a/crates/doc/src/id_index.rs +++ b/crates/doc/src/id_index.rs @@ -93,6 +93,19 @@ impl IdIndex { self.0.read().ok().and_then(|g| g.clone()) } + /// Let go of the cache (a document kept in the undo history): the next lookup builds it again. + pub(crate) fn clear(&self) { + if let Ok(mut g) = self.0.write() { + *g = None; + } + self.1.store(0, std::sync::atomic::Ordering::Relaxed); + } + + /// Whether a cache was built (and not cleared since). + pub(crate) fn is_built(&self) -> bool { + self.0.read().is_ok_and(|g| g.is_some()) + } + /// Rebuild the cache from `layers` (one walk) and return it. pub(crate) fn rebuild(&self, layers: &[Arc]) -> Arc { fn walk(nodes: &[Arc], parent: u32, out: &mut Paths) { diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index d1742933..26351869 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -22,6 +22,7 @@ pub use shared::Shared; pub mod links; pub mod live; pub mod marks; +mod mem; pub mod metadata; pub mod node; pub mod orient; @@ -60,6 +61,12 @@ pub(crate) mod skip { pub fn is_one(v: &f32) -> bool { *v == 1.0 } + pub fn is_empty_vec(v: &[T]) -> bool { + v.is_empty() + } + pub fn is_empty_map(v: &serde_json::Map) -> bool { + v.is_empty() + } } pub use appearance::StrokeGradientMode; @@ -549,18 +556,21 @@ pub struct Document { pub artboards: Vec, /// Top-level layers, bottom first. pub layers: Vec>, + // The document-wide resources below are [`Shared`]: an edit copies the document (copy-on-write) + // and the undo history keeps every version, so what an edit doesn't change stays one copy. + // Reads, writes and the saved form are those of the value itself. #[serde(default)] - pub swatches: Vec, + pub swatches: Shared>, #[serde(default)] - pub swatch_groups: Vec, + pub swatch_groups: Shared>, #[serde(default)] - pub graphic_styles: Vec, + pub graphic_styles: Shared>, /// Character styles (besides the built-in [Normal Character Style], which may be redefined here). - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub char_styles: Vec, + #[serde(default, skip_serializing_if = "skip::is_empty_vec")] + pub char_styles: Shared>, /// Paragraph styles (besides the built-in [Normal Paragraph Style]). - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub para_styles: Vec, + #[serde(default, skip_serializing_if = "skip::is_empty_vec")] + pub para_styles: Shared>, /// Threaded text: area-type frames one story flows through, in order. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub text_threads: Vec>, @@ -575,11 +585,11 @@ pub struct Document { #[serde(default, skip_serializing_if = "Vec::is_empty")] pub views: Vec, /// Select → Save Selection… (at most [`SavedSelection::MAX`]); saved with the document. - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub saved_selections: Vec, + #[serde(default, skip_serializing_if = "skip::is_empty_vec")] + pub saved_selections: Shared>, /// Variables (data merge): named values bound to objects, applied per dataset. #[serde(default, skip_serializing_if = "crate::skip::is_default")] - pub variables: Variables, + pub variables: Shared, #[serde(default)] pub grid: GridPrefs, #[serde(default = "ppi72")] @@ -598,8 +608,8 @@ pub struct Document { pub mask_edit: Option, next_id: u64, /// Foreign data preserved on round-trip. - #[serde(default, skip_serializing_if = "serde_json::Map::is_empty")] - pub unknown: serde_json::Map, + #[serde(default, skip_serializing_if = "skip::is_empty_map")] + pub unknown: Shared>, /// Transparency panel → Page Isolated Blending: the page is an isolated transparency group, /// so top-level blend modes don't blend with what lies under the page. #[serde(default, skip_serializing_if = "std::ops::Not::not")] @@ -705,18 +715,18 @@ impl Document { show_cross_hairs: false, }], layers: vec![], - swatches, - swatch_groups, - graphic_styles: default_graphic_styles(), - char_styles: vec![], - para_styles: vec![], + swatches: swatches.into(), + swatch_groups: swatch_groups.into(), + graphic_styles: default_graphic_styles().into(), + char_styles: Default::default(), + para_styles: Default::default(), text_threads: vec![], symbols: vec![], graph_designs: vec![], guides: vec![], views: vec![], - saved_selections: vec![], - variables: Variables::default(), + saved_selections: Default::default(), + variables: Default::default(), grid: GridPrefs::default(), raster_effects_ppi: 72.0, images: BTreeMap::new(), @@ -775,6 +785,26 @@ impl Document { self.index_path(id).and_then(|p| self.node_at(&p)) } + /// [`Document::node`] without building the lookup cache when there is none: for a look at a + /// document kept in the undo history ([`Document::drop_lookup_cache`]). + pub fn node_uncached(&self, id: NodeId) -> Option<&Node> { + match self.index.is_built() { + true => self.node(id), + false => self.walk_index_path(id).and_then(|p| self.node_at(&p)), + } + } + + /// Let go of the lookup cache ([`id_index`]), as a document kept in the undo history does: a + /// table per step would add up. Lookups still work; the first one builds the cache again. + pub fn drop_lookup_cache(&self) { + self.index.clear(); + } + + /// Whether the lookup cache ([`id_index`]) is built (see [`Document::drop_lookup_cache`]). + pub fn has_lookup_cache(&self) -> bool { + self.index.is_built() + } + /// Index path from the top-level layer list to `id` (e.g. `[0, 3, 1]`): the first place in /// tree order where `id` appears. pub fn index_path(&self, id: NodeId) -> Option> { @@ -1249,7 +1279,7 @@ impl Document { let objects = s.objects.into_iter().filter(|id| ids.contains(id) && seen.insert(*id)).collect(); kept.push(SavedSelection { name, objects }); } - self.saved_selections = kept; + self.saved_selections = kept.into(); } /// Leave opacity-mask editing: drop the temporary editing layer, a working copy of art the diff --git a/crates/doc/src/mem.rs b/crates/doc/src/mem.rs new file mode 100644 index 00000000..1162e8a6 --- /dev/null +++ b/crates/doc/src/mem.rs @@ -0,0 +1,318 @@ +//! How much memory a document keeps that another doesn't share: what an undo step costs. +//! +//! Documents share structure (`Arc` children, [`crate::Shared`] resources), so an old +//! version kept in the undo history holds only what the edits after it replaced. The estimate +//! walks the old version beside the newer one and visits only what isn't shared: a subtree whose +//! `Arc` the newer document also holds costs nothing and isn't entered, so the walk is about as +//! long as the edit was (plus a pass over each changed child list). It counts the heap the values +//! own (struct sizes, vector capacities, strings, JSON) plus a typical allocator overhead per +//! allocation, without following every last field: an estimate in the right ballpark, for a +//! memory budget, not an exact figure. + +use std::collections::HashSet; +use std::mem::size_of; +use std::sync::Arc; + +use serde_json::Value; +use vectorcraft_color::{Paint, Swatch, SwatchGroup}; +use vectorcraft_geom::{Anchor, PathData, SubPath}; + +use crate::text::{ParaStyle, TextObject, TextRun, TextStyleDef}; +use crate::{Appearance, AppearanceItem, Document, Effect, GraphicStyle, Node, NodeKind, SavedSelection, Shared}; + +/// Bookkeeping and rounding a typical allocator adds to each allocation. +const ALLOC: usize = 16; +/// An `Arc`'s two reference counts. +const ARC: usize = 16; +/// The kinds of node this doesn't break down (meshes, envelopes' settings, placed documents…): +/// a typical size of what they own besides the node. +const OTHER_KIND: usize = 256; + +impl Document { + /// Roughly how many bytes of heap this document keeps that `newer` doesn't share with it: what + /// keeping this version in the undo history costs while `newer` (a later version) is kept too. + /// Visits only the parts that aren't shared (see the [module docs](self)). + pub fn retained_bytes(&self, newer: &Document) -> usize { + if std::ptr::eq(self, newer) { + return 0; + } + let mut w = Walk { newer, seen: None, bytes: size_of::() + ALLOC + ARC }; + w.list(&self.layers, Some(&newer.layers)); + for (i, s) in self.symbols.iter().enumerate() { + w.bytes += s.name.capacity(); + w.tree(&s.art, newer.symbols.get(i).map(|n| &n.art)); + } + w.bytes += vec_bytes(&self.symbols); + for (i, p) in self.patterns.iter().enumerate() { + w.bytes += p.name.capacity(); + w.list(&p.art, newer.patterns.get(i).map(|n| &n.art)); + } + w.bytes += vec_bytes(&self.patterns); + w.bytes += self.resources_bytes(newer); + w.bytes + } + + /// The document-wide lists and settings: a copy of the document copies those that aren't + /// [`Shared`]; the shared ones count only when `newer` has its own. + fn resources_bytes(&self, newer: &Document) -> usize { + fn shared(a: &Shared, b: &Shared, f: impl FnOnce(&T) -> usize) -> usize { + if Shared::ptr_eq(a, b) { 0 } else { ALLOC + ARC + f(a) } + } + let mut n = 0; + n += vec_bytes(&self.artboards) + self.artboards.iter().map(|a| a.name.capacity()).sum::(); + n += vec_bytes(&self.guides) + vec_bytes(&self.views) + vec_bytes(&self.slices) + vec_bytes(&self.assets); + n += vec_bytes(&self.text_threads) + self.text_threads.iter().map(vec_bytes).sum::(); + n += self.images.iter().map(|(k, b)| 4 * size_of::() + k.capacity() + b.mime.capacity() + size_of_val(b)).sum::(); + n += map_bytes(&self.export_settings) + map_bytes(&self.extra) + self.print_setup.as_ref().map_or(0, json_bytes); + n += self.title.capacity(); + n += shared(&self.swatches, &newer.swatches, |s| swatches_bytes(s)); + n += shared(&self.swatch_groups, &newer.swatch_groups, |g| { + vec_bytes(g) + g.iter().map(|g: &SwatchGroup| g.name.capacity() + swatches_bytes(&g.swatches)).sum::() + }); + n += shared(&self.graphic_styles, &newer.graphic_styles, |g| { + vec_bytes(g) + g.iter().map(|g: &GraphicStyle| g.name.capacity() + appearance_heap(&g.appearance)).sum::() + }); + let styles = |s: &Vec| vec_bytes(s) + s.iter().map(|s| s.name.capacity() + map_bytes(&s.attrs)).sum::(); + n += shared(&self.char_styles, &newer.char_styles, styles); + n += shared(&self.para_styles, &newer.para_styles, styles); + n += shared(&self.saved_selections, &newer.saved_selections, |s| { + vec_bytes(s) + s.iter().map(|s: &SavedSelection| s.name.capacity() + vec_bytes(&s.objects)).sum::() + }); + n += shared(&self.variables, &newer.variables, |v| { + // Map entries: the key and value plus a share of the tree's nodes. + let entry = 2 * size_of::(); + vec_bytes(&v.variables) + + v.variables.iter().map(|v| v.name.capacity()).sum::() + + vec_bytes(&v.datasets) + + v.datasets.iter().map(|d| d.name.capacity() + d.values.keys().map(|k| entry + k.capacity() + 32).sum::()).sum::() + + v.bindings.values().map(|b| entry + 8 + b.iter().map(|s| entry + s.capacity()).sum::()).sum::() + }); + n += shared(&self.unknown, &newer.unknown, map_bytes); + n + } +} + +/// One estimate in progress. +struct Walk<'a> { + newer: &'a Document, + /// Every node the newer document holds (by address), built the first time a node has no + /// counterpart in place: then a node moved elsewhere (grouped, say) is still found shared. + seen: Option>, + bytes: usize, +} + +impl Walk<'_> { + /// The nodes of `list` that `newer` (the counterpart list, if any) doesn't share. + fn list(&mut self, list: &[Arc], newer: Option<&Vec>>) { + for (i, n) in list.iter().enumerate() { + self.tree(n, newer.and_then(|l| l.get(i))); + } + } + + /// Node `n` and what under it isn't shared; `newer` is the node in its place in the newer + /// document, if any. + fn tree(&mut self, n: &Arc, newer: Option<&Arc>) { + if newer.is_some_and(|m| Arc::ptr_eq(n, m)) { + return; + } + // In its place is another version of the same object (edited, moved): compare their + // children in place. Else it may sit elsewhere in the newer document. + let counterpart = newer.filter(|m| m.id == n.id); + if counterpart.is_none() && self.newer_holds(n) { + return; + } + self.bytes += node_bytes(n, counterpart.map(|m| &**m)); + if let Some(ch) = n.children() { + self.list(ch, counterpart.and_then(|m| m.children())); + } + if let Some(mask) = &n.mask { + self.tree(&mask.art, counterpart.and_then(|m| m.mask.as_ref()).map(|m| &m.art)); + } + } + + fn newer_holds(&mut self, n: &Arc) -> bool { + let newer = self.newer; + let seen = self.seen.get_or_insert_with(|| { + fn add(n: &Arc, out: &mut HashSet) { + // A node already seen was entered already (a node shared within the document). + if !out.insert(Arc::as_ptr(n) as usize) { + return; + } + for c in n.children().into_iter().flatten() { + add(c, out); + } + if let Some(m) = &n.mask { + add(&m.art, out); + } + } + let mut out = HashSet::new(); + let roots = newer.layers.iter().chain(newer.symbols.iter().map(|s| &s.art)).chain(newer.patterns.iter().flat_map(|p| &p.art)); + for n in roots { + add(n, &mut out); + } + out + }); + seen.contains(&(Arc::as_ptr(n) as usize)) + } +} + +/// One node's own heap: the node, and what it owns besides its children and mask art (counted +/// as nodes of their own). Its appearance counts unless `newer`, its newer version, shares it. +fn node_bytes(n: &Node, newer: Option<&Node>) -> usize { + fn boxed(b: &Option>) -> usize { + b.as_ref().map_or(0, |_| ALLOC + size_of::()) + } + let mut b = ALLOC + ARC + size_of::(); + b += n.name.as_ref().map_or(0, String::capacity); + if !newer.is_some_and(|m| Shared::ptr_eq(&m.appearance, &n.appearance)) { + b += ALLOC + ARC + appearance_heap(&n.appearance); + } + b += boxed(&n.mask) + boxed(&n.graph) + boxed(&n.shaper) + boxed(&n.attrs) + boxed(&n.slice) + boxed(&n.perspective); + b += n.trace.as_ref().map_or(0, |t| ALLOC + json_bytes(t)); + b += match &n.kind { + NodeKind::Path { path, .. } => path_bytes(path), + NodeKind::Layer { children, .. } + | NodeKind::Group { children, .. } + | NodeKind::Compound { children, .. } + | NodeKind::Blend { children, .. } + | NodeKind::Envelope { content: children, .. } => vec_bytes(children), + NodeKind::Repeat(r) => vec_bytes(&r.source) + OTHER_KIND, + NodeKind::Text(t) => ALLOC + text_bytes(t), + NodeKind::Image(im) => im.key.capacity(), + NodeKind::SymbolInstance { symbol, .. } => symbol.capacity(), + _ => OTHER_KIND, + }; + b +} + +fn path_bytes(p: &PathData) -> usize { + let anchors: usize = + p.subpaths.iter().map(|s| if s.anchors.capacity() == 0 { 0 } else { ALLOC + s.anchors.capacity() * size_of::() }).sum(); + anchors + if p.subpaths.capacity() == 0 { 0 } else { ALLOC + p.subpaths.capacity() * size_of::() } +} + +fn text_bytes(t: &TextObject) -> usize { + size_of::() + + vec_bytes(&t.runs) + + t.runs.iter().map(|r: &TextRun| r.text.capacity()).sum::() + + t.paras.capacity() * size_of::() + + vec_bytes(&t.wrap) + + vec_bytes(&t.cached_baselines) +} + +/// The heap an appearance owns (not the appearance itself). +fn appearance_heap(a: &Appearance) -> usize { + size_of::() + + vec_bytes(&a.items) + + a.items + .iter() + .map(|i| match i { + AppearanceItem::Fill(f) => paint_heap(&f.paint), + AppearanceItem::Stroke(s) => paint_heap(&s.paint), + }) + .sum::() + + vec_bytes(&a.effects) + + a.effects.iter().map(|e: &Effect| e.id.capacity() + json_bytes(&e.params)).sum::() +} + +fn paint_heap(p: &Paint) -> usize { + match p { + Paint::Solid { swatch, .. } => swatch.as_ref().map_or(0, String::capacity), + Paint::Gradient(g) => ALLOC + size_of_val(&**g) + vec_bytes(&g.gradient.stops), + Paint::Pattern { pattern, .. } => pattern.capacity(), + Paint::None => 0, + } +} + +fn swatches_bytes(s: &[Swatch]) -> usize { + ALLOC + std::mem::size_of_val(s) + s.iter().map(|w| w.name.capacity() + paint_heap(&w.paint)).sum::() +} + +/// A vector's buffer (not what its elements own). +fn vec_bytes(v: &Vec) -> usize { + if v.capacity() == 0 { 0 } else { ALLOC + v.capacity() * size_of::() } +} + +fn map_bytes(m: &serde_json::Map) -> usize { + m.iter().map(|(k, v)| ALLOC + 4 * size_of::() + k.capacity() + size_of::() + json_bytes(v)).sum() +} + +/// The heap a JSON value owns. +fn json_bytes(v: &Value) -> usize { + match v { + Value::String(s) => s.capacity(), + Value::Array(a) => vec_bytes(a) + a.iter().map(json_bytes).sum::(), + Value::Object(m) => map_bytes(m), + _ => 0, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use vectorcraft_geom::shapes; + + fn doc_with_rects(n: usize) -> Document { + let mut d = Document::new(1000.0, 1000.0); + let layer = d.layers[0].id; + for i in 0..n { + let id = d.alloc_id(); + let r = shapes::rectangle(kurbo::Rect::new(i as f64, 0.0, i as f64 + 10.0, 10.0)); + d.insert(Some(layer), usize::MAX, Node::path(id, r, Appearance::default_art())).unwrap(); + } + d + } + + #[test] + fn an_unchanged_copy_keeps_nothing_of_its_own() { + let d = doc_with_rects(10); + let copy = d.clone(); + // Only the document itself and its own lists: no node, no shared resource. + assert!(d.retained_bytes(©) < 2048, "{}", d.retained_bytes(©)); + assert_eq!(d.retained_bytes(&d), 0); + } + + /// Moving every rectangle: each keeps its old node and anchors (its appearance stays shared). + /// The process grew by about 775 bytes per moved rectangle when measured (#1244). + #[test] + fn moving_rectangles_costs_their_nodes() { + let n = 1000; + let before = doc_with_rects(n); + let mut after = before.clone(); + let ids: Vec<_> = after.layers[0].children().unwrap().iter().map(|c| c.id).collect(); + for id in ids { + let node = after.node_mut(id).unwrap(); + if let NodeKind::Path { path, .. } = &mut node.kind { + for a in path.subpaths.iter_mut().flat_map(|s| s.anchors.iter_mut()) { + a.p.x += 5.0; + } + } + } + let per = before.retained_bytes(&after) / n; + assert!((400..1200).contains(&per), "{per} bytes per moved rectangle"); + } + + #[test] + fn a_grouped_object_is_found_shared() { + let before = doc_with_rects(100); + let mut after = before.clone(); + let ids: Vec<_> = after.layers[0].children().unwrap().iter().map(|c| c.id).collect(); + // Put the rectangles in a group (the same Arcs, elsewhere). + let gid = after.alloc_id(); + let layer = Arc::make_mut(&mut after.layers[0]); + let kids = std::mem::take(layer.children_mut().unwrap()); + layer.children_mut().unwrap().push(Arc::new(Node::group(gid, kids))); + assert_eq!(ids.len(), 100); + let bytes = before.retained_bytes(&after); + assert!(bytes < 8192, "only the old layer and its child list: {bytes}"); + } + + #[test] + fn a_resource_counts_only_when_changed() { + let before = doc_with_rects(1); + let mut after = before.clone(); + let same = before.retained_bytes(&after); + after.swatches.push(before.swatches[1].clone()); + assert!(before.retained_bytes(&after) > same + 1000, "the old swatch list is the old version's own"); + } +} diff --git a/crates/doc/src/shared.rs b/crates/doc/src/shared.rs index 208ccd4c..0a7eefc3 100644 --- a/crates/doc/src/shared.rs +++ b/crates/doc/src/shared.rs @@ -34,6 +34,46 @@ impl std::ops::DerefMut for Shared { } } +impl Shared { + /// The value, taken out (copied only when another copy still shares it). + pub fn into_inner(self) -> T { + Arc::unwrap_or_clone(self.0) + } +} + +/// `for x in std::mem::take(&mut doc.swatches)` takes the value, as it would the value itself. +impl IntoIterator for Shared { + type Item = T::Item; + type IntoIter = T::IntoIter; + fn into_iter(self) -> Self::IntoIter { + self.into_inner().into_iter() + } +} + +/// `for x in &doc.swatches` iterates the shared value, as it would the value itself. +impl<'a, T> IntoIterator for &'a Shared +where + &'a T: IntoIterator, +{ + type Item = <&'a T as IntoIterator>::Item; + type IntoIter = <&'a T as IntoIterator>::IntoIter; + fn into_iter(self) -> Self::IntoIter { + (&*self.0).into_iter() + } +} + +/// `for x in &mut doc.swatches` makes the value this copy's own first, as any change does. +impl<'a, T: Clone> IntoIterator for &'a mut Shared +where + &'a mut T: IntoIterator, +{ + type Item = <&'a mut T as IntoIterator>::Item; + type IntoIter = <&'a mut T as IntoIterator>::IntoIter; + fn into_iter(self) -> Self::IntoIter { + Arc::make_mut(&mut self.0).into_iter() + } +} + impl From for Shared { fn from(v: T) -> Self { Shared::new(v) @@ -70,3 +110,65 @@ impl<'de, T: Deserialize<'de>> Deserialize<'de> for Shared { T::deserialize(d).map(Shared::new) } } + +#[cfg(test)] +mod tests { + use serde_json::json; + + use crate::text::TextStyleDef; + use crate::variables::{Variable, VariableKind}; + use crate::{Document, NodeId, SavedSelection}; + + /// A document with every shared resource set, changed only through `push`/`insert` (as code + /// written for the plain values does). + fn doc_with_resources() -> Document { + let mut d = Document::new(100.0, 100.0); + let swatch = d.swatches[1].clone(); + d.swatches.push(swatch); + d.swatch_groups.truncate(1); + d.graphic_styles.truncate(2); + d.char_styles.push(TextStyleDef { name: "Emphasis".into(), attrs: json!({"size": 14}).as_object().cloned().unwrap_or_default() }); + d.para_styles.push(TextStyleDef { name: "Body".into(), attrs: Default::default() }); + d.saved_selections.push(SavedSelection { name: "Saved".into(), objects: vec![NodeId(1)] }); + d.variables.variables.push(Variable { name: "Name".into(), kind: VariableKind::Text }); + d.variables.bindings.entry(NodeId(1)).or_default().insert("Name".into()); + d.unknown.insert("foreign".into(), json!({"keep": [1, 2]})); + d + } + + /// Shared document resources save as the values themselves: the saved form is unchanged. + #[test] + fn shared_resources_save_and_load_as_their_values() { + let d = doc_with_resources(); + let v = serde_json::to_value(&d).unwrap(); + assert_eq!(v["swatches"], serde_json::to_value(&*d.swatches).unwrap()); + assert_eq!(v["swatch_groups"], serde_json::to_value(&*d.swatch_groups).unwrap()); + assert_eq!(v["graphic_styles"], serde_json::to_value(&*d.graphic_styles).unwrap()); + assert_eq!(v["char_styles"], json!([{"name": "Emphasis", "attrs": {"size": 14}}])); + assert_eq!(v["para_styles"], json!([{"name": "Body", "attrs": {}}])); + assert_eq!(v["saved_selections"], json!([{"name": "Saved", "objects": [1]}])); + assert_eq!(v["variables"], json!({"variables": [{"name": "Name", "kind": "text"}], "bindings": {"1": ["Name"]}})); + assert_eq!(v["unknown"], json!({"foreign": {"keep": [1, 2]}})); + let text = serde_json::to_string(&d).unwrap(); + let back: Document = serde_json::from_str(&text).unwrap(); + assert_eq!(back, d); + assert_eq!(serde_json::to_string(&back).unwrap(), text, "saving again gives the same bytes"); + // Empty ones are left out, as before. + let v = serde_json::to_value(Document::new(10.0, 10.0)).unwrap(); + for k in ["char_styles", "para_styles", "saved_selections", "variables", "unknown"] { + assert!(v.get(k).is_none(), "{k} is left out when empty"); + } + } + + /// A copy of a document shares its resources until one copy changes one of them. + #[test] + fn copies_share_resources_until_changed() { + let a = doc_with_resources(); + let mut b = a.clone(); + assert!(super::Shared::ptr_eq(&a.swatches, &b.swatches) && super::Shared::ptr_eq(&a.unknown, &b.unknown)); + b.swatches.pop(); + assert!(!super::Shared::ptr_eq(&a.swatches, &b.swatches)); + assert_eq!(a.swatches.len(), b.swatches.len() + 1, "the other copy keeps its own"); + assert!(super::Shared::ptr_eq(&a.graphic_styles, &b.graphic_styles), "the rest stays shared"); + } +} diff --git a/crates/engine/src/cmd/clipboard.rs b/crates/engine/src/cmd/clipboard.rs index 84569f88..faaa1245 100644 --- a/crates/engine/src/cmd/clipboard.rs +++ b/crates/engine/src/cmd/clipboard.rs @@ -142,8 +142,8 @@ impl Clipboard { d.patterns = self.patterns.clone(); d.swatches.extend(self.swatches.iter().cloned()); d.graphic_styles.extend(self.graphic_styles.iter().cloned()); - d.char_styles = self.char_styles.clone(); - d.para_styles = self.para_styles.clone(); + d.char_styles = self.char_styles.clone().into(); + d.para_styles = self.para_styles.clone().into(); if !self.brushes.is_empty() { let mut lib = vectorcraft_brush::library(&d); for b in &self.brushes { diff --git a/crates/engine/src/cmd/distortcmds.rs b/crates/engine/src/cmd/distortcmds.rs index 55f80371..e4aeea52 100644 --- a/crates/engine/src/cmd/distortcmds.rs +++ b/crates/engine/src/cmd/distortcmds.rs @@ -1198,6 +1198,7 @@ fn persp_draw(s: &mut Session, p: &Value) -> Result { if let Some(e) = st.history.undo.last_mut() { e.label = "Draw in Perspective".into(); } + st.history.recost_last(&st.doc); } res?; Ok(json!({ "id": id.0, "plane": plane.id() })) @@ -1268,7 +1269,9 @@ fn edit_text(s: &mut Session, p: &Value) -> Result { /// in every undo state (the flat view is never an undo step of its own). pub(crate) fn finish_edit_text(st: &mut crate::DocState, id: NodeId) { let clear = |doc: &mut Arc| { - if doc.node(id).and_then(|n| n.perspective.as_deref()).is_some_and(|p| p.editing) + // Undo steps keep no lookup cache: look without building one, and drop the one an edit + // builds (the document now keeps its own). + if doc.node_uncached(id).and_then(|n| n.perspective.as_deref()).is_some_and(|p| p.editing) && let Some(p) = Arc::make_mut(doc).node_mut(id).and_then(|n| n.perspective.as_deref_mut()) { p.editing = false; @@ -1281,6 +1284,7 @@ pub(crate) fn finish_edit_text(st: &mut crate::DocState, id: NodeId) { } for e in st.history.undo.iter_mut().chain(st.history.redo.iter_mut()) { clear(&mut e.doc); + e.doc.drop_lookup_cache(); } if let Some(it) = &mut st.interaction { clear(&mut it.doc); diff --git a/crates/engine/src/cmd/docmenu.rs b/crates/engine/src/cmd/docmenu.rs index eef34b40..20de95d5 100644 --- a/crates/engine/src/cmd/docmenu.rs +++ b/crates/engine/src/cmd/docmenu.rs @@ -402,9 +402,9 @@ fn edit_saved(s: &mut Session, p: &Value) -> Result { return Err(bad(C, format!("two saved selections would be named `{}`", x.name))); } } - if after != before { + if *before != after { s.edit("Edit Selection", |d, _| { - d.saved_selections = after; + d.saved_selections = after.into(); Ok(()) })?; } diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 7adb48b6..7a7f8f4e 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -215,11 +215,11 @@ fn undo(s: &mut Session, _: &Value) -> Result { let e = st.history.undo.pop().ok_or_else(|| EngineError::Other("nothing to undo".into()))?; let label = e.label.clone(); let editing = st.doc.mask_edit.map(|m| m.layer); - st.history.redo.push(crate::HistoryEntry { - label: e.label, - doc: std::mem::replace(&mut st.doc, e.doc), - selection: std::mem::replace(&mut st.selection, e.selection), - }); + let doc = std::mem::replace(&mut st.doc, e.doc); + let selection = std::mem::replace(&mut st.selection, Arc::unwrap_or_clone(e.selection)); + let selection = shared_selection(selection, st.history.redo.last()); + st.history.redo.push(crate::HistoryEntry::kept(e.label, doc, selection, &st.doc)); + st.history.trim(); st.revision += 1; st.selection.prune(&st.doc); super::maskedit::follow_history(st, editing); @@ -231,17 +231,25 @@ fn redo(s: &mut Session, _: &Value) -> Result { let e = st.history.redo.pop().ok_or_else(|| EngineError::Other("nothing to redo".into()))?; let label = e.label.clone(); let editing = st.doc.mask_edit.map(|m| m.layer); - st.history.undo.push(crate::HistoryEntry { - label: e.label, - doc: std::mem::replace(&mut st.doc, e.doc), - selection: std::mem::replace(&mut st.selection, e.selection), - }); + let doc = std::mem::replace(&mut st.doc, e.doc); + let selection = std::mem::replace(&mut st.selection, Arc::unwrap_or_clone(e.selection)); + let selection = shared_selection(selection, st.history.undo.last()); + st.history.undo.push(crate::HistoryEntry::kept(e.label, doc, selection, &st.doc)); + st.history.trim(); st.revision += 1; st.selection.prune(&st.doc); super::maskedit::follow_history(st, editing); Ok(json!({ "redone": label })) } +/// `selection` for an undo (redo) step, sharing that of `like`, the step beside it, when the same. +fn shared_selection(selection: vectorcraft_doc::Selection, like: Option<&crate::HistoryEntry>) -> Arc { + match like { + Some(l) if *l.selection == selection => l.selection.clone(), + _ => Arc::new(selection), + } +} + /// Selected top-level objects in paint order, dropping any whose ancestor is also selected. pub(crate) fn selected_roots(s: &Session) -> Result> { let st = s.doc()?; diff --git a/crates/engine/src/cmd/menucmds.rs b/crates/engine/src/cmd/menucmds.rs index a6ebd243..7b6ad172 100644 --- a/crates/engine/src/cmd/menucmds.rs +++ b/crates/engine/src/cmd/menucmds.rs @@ -180,6 +180,7 @@ pub(crate) fn squash(s: &mut Session, from: usize, label: &str) { if h.len() > from { h.truncate(from + 1); h[from].label = label.to_string(); + st.history.recost_last(&st.doc); } } } diff --git a/crates/engine/src/cmd/prefscmds.rs b/crates/engine/src/cmd/prefscmds.rs index 59e5ae77..1a30edbf 100644 --- a/crates/engine/src/cmd/prefscmds.rs +++ b/crates/engine/src/cmd/prefscmds.rs @@ -543,8 +543,7 @@ impl Session { for st in &mut self.docs { if history_changed { st.history.limit = self.prefs.history_states as usize; - let over = st.history.undo.len().saturating_sub(st.history.limit); - st.history.undo.drain(..over); + st.history.trim(); } // Documents in pattern editing mode redraw their tile edge. if tile_edge_changed && st.doc.pattern_edit.is_some() { diff --git a/crates/engine/src/cmd/swatchlib.rs b/crates/engine/src/cmd/swatchlib.rs index 33adf24f..ddf8ccd9 100644 --- a/crates/engine/src/cmd/swatchlib.rs +++ b/crates/engine/src/cmd/swatchlib.rs @@ -606,7 +606,7 @@ fn reset_defaults(s: &mut Session, p: &Value) -> Result { let mut added = vec![]; if replace { let before: Vec = d.swatches_iter().map(|w| w.name.clone()).collect(); - (d.swatches, d.swatch_groups) = (defaults, groups); + (d.swatches, d.swatch_groups) = (defaults.into(), groups.into()); added.extend(d.swatches_iter().filter(|w| !before.contains(&w.name)).map(|w| w.name.clone())); live.extend(d.swatches_iter().filter(|w| w.global).map(|w| w.name.clone())); d.map_solid_paints(&mut |_, l, _| unlink_dead(&live, l)); diff --git a/crates/engine/src/cmd/textedit.rs b/crates/engine/src/cmd/textedit.rs index d830d824..d783cd59 100644 --- a/crates/engine/src/cmd/textedit.rs +++ b/crates/engine/src/cmd/textedit.rs @@ -755,7 +755,7 @@ fn discard_empty(s: &mut Session, p: &Value) -> Result { // plus the text (and its id), roll back to it without a trace. let st = s.doc_mut()?; if st.interaction.is_none() - && let Some(k) = st.history.undo.iter().rposition(|e| e.doc.node(id).is_none()) + && let Some(k) = st.history.undo.iter().rposition(|e| e.doc.node_uncached(id).is_none()) && let Some(created) = st.history.undo.get(k) { let mut now = (*st.doc).clone(); diff --git a/crates/engine/src/journal.rs b/crates/engine/src/journal.rs new file mode 100644 index 00000000..96cfe7ae --- /dev/null +++ b/crates/engine/src/journal.rs @@ -0,0 +1,197 @@ +//! The session's journal of executed commands: what the Actions panel records an action from +//! (and what a replay runs), kept within a memory bound. +//! +//! Entries are numbered from the session's first command on, and a number keeps naming the same +//! entry when older ones are let go: [`Journal::len`] counts every entry ever journaled, so a +//! recording that remembers where it started finds its steps by that number. Only the most recent +//! entries are kept (at most [`Journal::MAX_ENTRIES`] and about [`Journal::MAX_BYTES`], always the +//! last [`Journal::MIN_ENTRIES`]): a long session of select-alls on a large document no longer +//! grows the journal by a copy of every id list forever. A recording that must keep everything +//! from its start holds the journal there ([`Journal::hold`]). + +use serde_json::Value; + +/// One journaled command: its id and params (with the values it noted, see +/// [`crate::Session::note_journal`]). +pub type JournalEntry = (String, Value); + +#[derive(Clone, Debug, Default)] +pub struct Journal { + /// How many entries were let go from the front; the first kept one has this number. + dropped: usize, + entries: Vec, + /// Each kept entry's estimated size, and their sum. + sizes: Vec, + bytes: usize, + /// Keep every entry from this number on ([`Journal::hold`]). + held: Option, +} + +/// Journals with the same entries (by number) are equal, whatever their history. +impl PartialEq for Journal { + fn eq(&self, other: &Self) -> bool { + self.dropped == other.dropped && self.entries == other.entries + } +} + +impl Journal { + /// Most entries kept (the oldest go first). + pub const MAX_ENTRIES: usize = 10_000; + /// About the most memory the kept entries take. + pub const MAX_BYTES: usize = 32 << 20; + /// The most recent entries are kept whatever they take. + pub const MIN_ENTRIES: usize = 64; + + /// Every entry ever journaled, kept or let go: the number the next one gets. + pub fn len(&self) -> usize { + self.dropped + self.entries.len() + } + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + /// The number of the oldest entry kept. + pub fn first_kept(&self) -> usize { + self.dropped + } + /// The kept entries, oldest first. + pub fn iter(&self) -> std::slice::Iter<'_, JournalEntry> { + self.entries.iter() + } + /// The kept entries from number `start` on (all kept ones when older ones were let go). + pub fn since(&self, start: usize) -> &[JournalEntry] { + self.entries.get(start.saturating_sub(self.dropped)..).unwrap_or_default() + } + /// Entry number `i`, if kept. + pub fn get(&self, i: usize) -> Option<&JournalEntry> { + self.entries.get(i.checked_sub(self.dropped)?) + } + pub fn get_mut(&mut self, i: usize) -> Option<&mut JournalEntry> { + self.entries.get_mut(i.checked_sub(self.dropped)?) + } + pub fn last(&self) -> Option<&JournalEntry> { + self.entries.last() + } + pub fn last_mut(&mut self) -> Option<&mut JournalEntry> { + self.entries.last_mut() + } + pub fn push(&mut self, e: JournalEntry) { + let size = entry_bytes(&e); + self.entries.push(e); + self.sizes.push(size); + self.bytes += size; + self.trim(); + } + /// Drop the entries from number `len` on (a cancelled undo group's). + pub fn truncate(&mut self, len: usize) { + let keep = len.saturating_sub(self.dropped); + self.entries.truncate(keep); + self.sizes.truncate(keep); + self.bytes = self.sizes.iter().sum(); + self.dropped = self.dropped.min(len); + } + /// 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(); + } + + /// 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). + fn trim(&mut self) { + let over_count = self.entries.len() > Self::MAX_ENTRIES + Self::MAX_ENTRIES / 4; + if !over_count && self.bytes <= Self::MAX_BYTES { + return; + } + // The entries that may go: not the last few, nor any a hold keeps. + let mut may = self.entries.len().saturating_sub(Self::MIN_ENTRIES); + if let Some(h) = self.held { + may = may.min(h.saturating_sub(self.dropped)); + } + let mut n = 0; + let mut bytes = self.bytes; + while n < may && (self.entries.len() - n > Self::MAX_ENTRIES || bytes > Self::MAX_BYTES) { + bytes -= self.sizes.get(n).copied().unwrap_or(0); + n += 1; + } + if n > 0 { + self.entries.drain(..n); + self.sizes.drain(..n); + self.bytes = bytes; + self.dropped += n; + } + } +} + +impl<'a> IntoIterator for &'a Journal { + type Item = &'a JournalEntry; + type IntoIter = std::slice::Iter<'a, JournalEntry>; + fn into_iter(self) -> Self::IntoIter { + self.entries.iter() + } +} + +/// Roughly the heap an entry takes: its id, and its params as `serde_json` holds them. +fn entry_bytes((id, p): &JournalEntry) -> usize { + fn json(v: &Value) -> usize { + const VALUE: usize = std::mem::size_of::(); + match v { + Value::String(s) => s.capacity(), + Value::Array(a) => 16 + a.capacity() * VALUE + a.iter().map(json).sum::(), + Value::Object(m) => m.iter().map(|(k, v)| 48 + k.capacity() + VALUE + json(v)).sum(), + _ => 0, + } + } + std::mem::size_of::() + id.capacity() + json(p) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn big(i: usize) -> JournalEntry { + ("select.set".into(), json!({"ids": (0..50_000u64).collect::>(), "n": i})) + } + + #[test] + fn numbers_survive_letting_entries_go() { + let mut j = Journal::default(); + for i in 0..200 { + j.push(big(i)); + } + assert_eq!(j.len(), 200, "every entry counts"); + assert!(j.first_kept() > 0, "big entries were let go"); + assert!(j.iter().count() >= Journal::MIN_ENTRIES); + assert_eq!(j.get(199).map(|e| e.1["n"].clone()), Some(json!(199))); + assert_eq!(j.since(150).len(), 50); + assert!(j.get(0).is_none()); + } + + #[test] + fn a_hold_keeps_a_recording_whole() { + let mut j = Journal::default(); + j.push(("file.new".into(), json!({}))); + let start = j.len(); + j.hold(Some(start)); + for i in 0..200 { + j.push(big(i)); + } + assert_eq!(j.since(start).len(), 200, "nothing recorded was let go"); + assert_eq!(j.first_kept(), 1, "only what came before the recording may go"); + j.hold(None); + j.push(big(200)); + assert!(j.first_kept() > start); + } + + #[test] + fn many_small_entries_are_bounded_too() { + let mut j = Journal::default(); + for i in 0..30_000 { + j.push(("object.move".into(), json!({"dx": i}))); + } + assert_eq!(j.len(), 30_000); + assert!(j.iter().count() <= Journal::MAX_ENTRIES + Journal::MAX_ENTRIES / 4); + j.truncate(29_990); + assert_eq!((j.len(), j.last().map(|e| e.1["dx"].clone())), (29_990, Some(json!(29_989)))); + } +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 307c4084..58b89bd9 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -10,6 +10,7 @@ pub mod cmd; pub mod file_access; pub mod guard; pub mod inspect; +pub mod journal; pub mod link_watch; pub mod steps; mod tooling; @@ -28,6 +29,7 @@ pub use cmd::clipboard::Clipboard; pub use cmd::distortcmds::perspective_click; pub use cmd::rasterfx::{export_pdf, flatten_raster_effects}; pub use cmd::{CommandInfo, CommandSpec, command_specs, find_command}; +pub use journal::Journal; pub use tooling::{UiRequest, ViewInfo}; pub use vectorcraft_doc as doc; pub use vectorcraft_tools as tools; @@ -63,16 +65,106 @@ pub type Result = std::result::Result; #[derive(Clone, Debug)] pub struct HistoryEntry { pub label: String, + /// The document as it was. It keeps no lookup cache ([`Document::drop_lookup_cache`]): a + /// table per step would add up, and undoing builds it again once. pub doc: Arc, - pub selection: Selection, + /// The selection as it was, shared with the neighbouring steps that have the same one. + pub selection: Arc, + /// Roughly the memory this step keeps of its own: what of `doc` the step after it (or the + /// document now) doesn't share ([`Document::retained_bytes`]). + pub bytes: usize, } -#[derive(Clone, Debug, Default)] +impl HistoryEntry { + /// A step keeping `doc` and `selection`, as they were before the edit that made `next`. + pub(crate) fn kept(label: String, doc: Arc, selection: Arc, next: &Document) -> Self { + doc.drop_lookup_cache(); + let bytes = doc.retained_bytes(next); + HistoryEntry { label, doc, selection, bytes } + } +} + +/// The undo history of a document: bounded by a number of steps (Preferences › Performance › +/// History States) and by the memory the steps keep ([`History::byte_budget`]). +#[derive(Clone, Debug)] pub struct History { pub undo: Vec, pub redo: Vec, /// Maximum undo depth (Illustrator has no hard limit; snapshots are cheap here). pub limit: usize, + /// About the most memory the steps may keep ([`HistoryEntry::bytes`]): past it the oldest + /// steps go, but never the last [`History::MIN_STEPS`] undo (and redo) steps. + pub byte_budget: usize, +} + +impl Default for History { + fn default() -> Self { + History { undo: vec![], redo: vec![], limit: 0, byte_budget: default_history_budget() } + } +} + +impl History { + /// Undo (and redo) steps kept whatever memory they take. + pub const MIN_STEPS: usize = 8; + + /// Roughly the memory the steps keep of their own ([`HistoryEntry::bytes`]). + pub fn bytes(&self) -> usize { + self.undo.iter().chain(&self.redo).map(|e| e.bytes).sum() + } + + /// 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) { + let over = self.undo.len().saturating_sub(self.limit); + if over > 0 { + self.undo.drain(..over); + } + let mut total = self.bytes(); + if total <= self.byte_budget { + return; + } + let mut n = 0; + while total > self.byte_budget && self.undo.len() - n > Self::MIN_STEPS { + total -= self.undo.get(n).map_or(0, |e| e.bytes); + n += 1; + } + self.undo.drain(..n); + let mut n = 0; + while total > self.byte_budget && self.redo.len() - n > Self::MIN_STEPS { + total -= self.redo.get(n).map_or(0, |e| e.bytes); + n += 1; + } + self.redo.drain(..n); + } + + /// 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). + pub(crate) fn recost_last(&mut self, now: &Document) { + if let Some(e) = self.undo.last_mut() { + e.bytes = e.doc.retained_bytes(now); + } + } +} + +/// The default [`History::byte_budget`]: an eighth of the machine's memory where it is known +/// (Linux), at most 1 GiB (the figure elsewhere); 256 MiB on the web, whose memory is small. +pub fn default_history_budget() -> usize { + const MAX: usize = 1 << 30; + if cfg!(target_arch = "wasm32") { + return 256 << 20; + } + static BUDGET: std::sync::OnceLock = std::sync::OnceLock::new(); + *BUDGET.get_or_init(|| physical_memory().map_or(MAX, |m| (m / 8).clamp(64 << 20, MAX))) +} + +/// The machine's memory, where cheap to learn without platform calls. +fn physical_memory() -> Option { + if !cfg!(target_os = "linux") { + return None; + } + let info = std::fs::read_to_string("/proc/meminfo").ok()?; + let kb: usize = info.lines().find_map(|l| l.strip_prefix("MemTotal:"))?.trim().trim_end_matches("kB").trim().parse().ok()?; + kb.checked_mul(1024) } /// An in-progress drag: the snapshot the previews are applied to. @@ -80,7 +172,8 @@ pub struct History { pub struct Interaction { pub label: String, pub doc: Arc, - pub selection: Selection, + /// The selection the previews start from (shared, so a preview frame copies nothing). + pub selection: Arc, pub preview: Option<(String, Value)>, /// Per-document state restored on cancel (current layer, highlighted Layers panel rows, /// isolation). @@ -284,34 +377,40 @@ impl DocState { /// Keep what interaction `it` (taken from this document) changed, as one undo step. pub(crate) fn keep_interaction(&mut self, it: Interaction) { if !Arc::ptr_eq(&it.doc, &self.doc) { - self.push_undo(HistoryEntry { label: it.label, doc: it.doc, selection: it.selection }); + let selection = match self.history.undo.last() { + Some(l) if l.selection == it.selection => l.selection.clone(), + _ => it.selection, + }; + self.push_undo(it.label, it.doc, selection); self.revision += 1; } } - /// Record undo step `e` (the document before an edit). In an undo group only the group's first - /// edit records one: the edits after it extend that step. - fn push_undo(&mut self, mut e: HistoryEntry) { + /// Record an undo step: `doc` and `selection` as they were before an edit (the document now is + /// the one after it). In an undo group only the group's first edit records one: the edits + /// after it extend that step. + fn push_undo(&mut self, mut label: String, doc: Arc, selection: Arc) { if let Some(g) = &mut self.undo_group { let recorded = |first: &Arc| self.history.undo.last().is_some_and(|l| Arc::ptr_eq(&l.doc, first)); if g.first.as_ref().is_some_and(recorded) { + self.history.recost_last(&self.doc); + self.history.trim(); return; } - g.first = Some(e.doc.clone()); + g.first = Some(doc.clone()); if let Some(l) = &g.label { - e.label = l.clone(); + label = l.clone(); } } + let e = HistoryEntry::kept(label, doc, selection, &self.doc); self.history.undo.push(e); - if self.history.undo.len() > self.history.limit { - self.history.undo.remove(0); - } self.history.redo.clear(); + self.history.trim(); } /// End the interaction in progress, undoing what it changed. pub(crate) fn undo_interaction(&mut self) { if let Some(it) = self.interaction.take() { self.doc = it.doc; - self.selection = it.selection; + self.selection = Arc::unwrap_or_clone(it.selection); self.active_layer = it.active_layer; self.layer_rows = it.layer_rows; self.variables_highlight = it.variables_highlight; @@ -840,8 +939,8 @@ pub struct Session { pub fill_active: bool, /// Internal clipboard: the copied objects and the document resources they use. pub clipboard: Clipboard, - /// Executed commands (for actions and debugging). - pub journal: Vec<(String, Value)>, + /// Executed commands (for actions and debugging), the most recent ones kept ([`Journal`]). + pub journal: Journal, pub(crate) tool: Box, /// 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`]). @@ -952,7 +1051,7 @@ impl Session { paint: PaintDefaults::default(), fill_active: true, clipboard: Clipboard::default(), - journal: vec![], + journal: Journal::default(), tool: vectorcraft_tools::create("selection"), lender: None, last_selection_tool: None, @@ -1194,8 +1293,8 @@ impl Session { /// pin a replay to the original run ([`REPLAY_ONLY`]), so playing the action later acts now. pub fn journal_for_action(&self, start: usize) -> Vec<(String, Value)> { self.journal + .since(start) .iter() - .skip(start) .cloned() .map(|(id, mut p)| { strip_replay_only(&id, &mut p); @@ -1296,7 +1395,11 @@ impl Session { st.selection.prune(&st.doc); st.revision += 1; if st.interaction.is_none() { - st.push_undo(HistoryEntry { label: label.to_string(), doc: before, selection: before_sel }); + let selection = match st.history.undo.last() { + Some(l) if *l.selection == before_sel => l.selection.clone(), + _ => Arc::new(before_sel), + }; + st.push_undo(label.to_string(), before, selection); } Ok(v) } @@ -1342,7 +1445,7 @@ impl Session { st.interaction = Some(Interaction { label: label.to_string(), doc: st.doc.clone(), - selection: st.selection.clone(), + selection: Arc::new(st.selection.clone()), preview: None, active_layer: st.active_layer, layer_rows: st.layer_rows.clone(), @@ -1360,11 +1463,14 @@ impl Session { let Some(it) = &st.interaction else { return Err(EngineError::Other("no interaction in progress".into())) }; // Undoing the previous preview counts as a change, so views redraw even when the command // below returns without an edit. - if !Arc::ptr_eq(&st.doc, &it.doc) || st.selection != it.selection { + let selection_changed = st.selection != *it.selection; + if !Arc::ptr_eq(&st.doc, &it.doc) || selection_changed { st.revision += 1; } st.doc = it.doc.clone(); - st.selection = it.selection.clone(); + if selection_changed { + st.selection.clone_from(&it.selection); + } } let rw = cmd::distortcmds::perspective_rewrite(self, cmd, params); let (cmd, params) = rw.as_ref().map_or((cmd, params), |(c, p)| (c.as_str(), p)); @@ -1463,7 +1569,7 @@ impl Session { && let Some(e) = st.history.undo.pop() { st.doc = e.doc; - st.selection = e.selection; + st.selection = Arc::unwrap_or_clone(e.selection); st.revision += 1; journal = Some(g.journal); } @@ -1580,6 +1686,8 @@ mod tests_gradpanel; #[cfg(test)] mod tests_halftone; #[cfg(test)] +mod tests_history_mem; +#[cfg(test)] mod tests_inline; #[cfg(test)] mod tests_journal; diff --git a/crates/engine/src/tests_history_mem.rs b/crates/engine/src/tests_history_mem.rs new file mode 100644 index 00000000..dd190700 --- /dev/null +++ b/crates/engine/src/tests_history_mem.rs @@ -0,0 +1,126 @@ +//! The undo history's memory (#1244): steps keep no lookup tables, unchanged selections are +//! shared between steps, and past a memory budget the oldest steps go (never the last few). + +use serde_json::json; +use vectorcraft_doc::{Appearance, Node}; +use vectorcraft_geom::shapes; + +use super::*; + +/// A session with one document holding `n` rectangles, all selected. +fn session_with_rects(n: usize) -> Session { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 2000, "height": 2000})).unwrap(); + s.edit("Add", |d, _| { + let layer = d.layers[0].id; + for i in 0..n { + let id = d.alloc_id(); + let r = shapes::rectangle(kurbo::Rect::new(i as f64, 0.0, i as f64 + 10.0, 10.0)); + d.insert(Some(layer), usize::MAX, Node::path(id, r, Appearance::default_art()))?; + } + Ok(()) + }) + .unwrap(); + s.execute("select.all", &json!({})).unwrap(); + s +} + +fn first_x(s: &Session) -> f64 { + let d = &s.doc().unwrap().doc; + let id = d.layers[0].children().unwrap()[0].id; + d.node(id).unwrap().geometric_bounds().unwrap().x0 +} + +#[test] +fn past_the_budget_the_oldest_steps_go_but_the_last_few_stay() { + let n = 500; + let mut s = session_with_rects(n); + s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); + let step = s.doc().unwrap().history.undo.last().unwrap().bytes; + // Each move keeps every rectangle's old node: about 700 bytes each (measured: ~775). + assert!((300 * n..1500 * n).contains(&step), "{step} bytes for moving {n} rectangles"); + // A budget of about three moves. + s.active_mut().unwrap().history.byte_budget = 3 * step; + for _ in 0..20 { + s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); + } + let st = s.doc().unwrap(); + assert_eq!(st.history.undo.len(), History::MIN_STEPS, "only the last few steps are kept"); + assert!(st.history.undo.iter().all(|e| e.label == "Move"), "the newest ones: {:?}", st.history.undo.iter().map(|e| &e.label).collect::>()); + // The kept steps undo as before. + let x = first_x(&s); + for _ in 0..History::MIN_STEPS { + s.execute("edit.undo", &json!({})).unwrap(); + } + assert_eq!(first_x(&s), x - History::MIN_STEPS as f64); + assert!(s.execute("edit.undo", &json!({})).is_err(), "the older steps went"); + // Redo too stays within the budget, keeping the next few steps. + let st = s.doc().unwrap(); + assert_eq!(st.history.redo.len(), History::MIN_STEPS); + for _ in 0..History::MIN_STEPS { + s.execute("edit.redo", &json!({})).unwrap(); + } + assert_eq!(first_x(&s), x); +} + +#[test] +fn within_the_budget_the_step_limit_still_counts() { + let mut s = session_with_rects(10); + s.active_mut().unwrap().history.limit = 12; + for _ in 0..30 { + s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); + } + let st = s.doc().unwrap(); + assert_eq!(st.history.undo.len(), 12); + assert!(st.history.bytes() < st.history.byte_budget); +} + +#[test] +fn undo_steps_keep_no_lookup_tables() { + let mut s = session_with_rects(200); + for _ in 0..5 { + s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); + // Lookups build the current document's table. + first_x(&s); + assert!(s.doc().unwrap().doc.has_lookup_cache()); + } + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + let st = s.doc().unwrap(); + assert!(!st.history.undo.is_empty() && !st.history.redo.is_empty()); + for e in st.history.undo.iter().chain(&st.history.redo) { + assert!(!e.doc.has_lookup_cache(), "step {} keeps a lookup table", e.label); + } + // The document undone to builds its table again on its first lookup. + first_x(&s); + assert!(s.doc().unwrap().doc.has_lookup_cache()); + // A drag's snapshot goes into the history without its table too. + s.begin_interaction("Move").unwrap(); + s.preview("object.move", &json!({"dx": 3, "dy": 0})).unwrap(); + first_x(&s); + s.commit_interaction().unwrap(); + assert!(!s.doc().unwrap().history.undo.last().unwrap().doc.has_lookup_cache()); +} + +#[test] +fn steps_share_an_unchanged_selection() { + let mut s = session_with_rects(100); + for _ in 0..4 { + s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); + } + s.begin_interaction("Move").unwrap(); + s.preview("object.move", &json!({"dx": 3, "dy": 0})).unwrap(); + s.preview("object.move", &json!({"dx": 4, "dy": 0})).unwrap(); + s.commit_interaction().unwrap(); + let st = s.doc().unwrap(); + let moves: Vec<_> = st.history.undo.iter().filter(|e| e.label == "Move").collect(); + assert_eq!(moves.len(), 5); + assert_eq!(moves[0].selection.objects.len(), 100); + assert!(moves.windows(2).all(|w| Arc::ptr_eq(&w[0].selection, &w[1].selection)), "one selection for all the moves"); + // Undo and redo keep it shared, and restore it. + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + let st = s.doc().unwrap(); + assert!(Arc::ptr_eq(&st.history.redo[0].selection, &st.history.redo[1].selection)); + assert_eq!(st.selection.objects.len(), 100); +} diff --git a/crates/engine/src/tests_journal.rs b/crates/engine/src/tests_journal.rs index 832e1722..810be573 100644 --- a/crates/engine/src/tests_journal.rs +++ b/crates/engine/src/tests_journal.rs @@ -41,7 +41,7 @@ fn replay_reproduces_the_created_date_of_an_earlier_run() { s.execute("file.new", &json!({"width": 300, "height": 200})).unwrap(); s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 40})).unwrap(); // As if the original run happened long ago: a replay now must still land on its date. - s.journal[0].1["created"] = json!(1_234_567_890); + s.journal.get_mut(0).unwrap().1["created"] = json!(1_234_567_890); let mut r = Session::new(); for (id, p) in &s.journal { r.execute(id, p).unwrap(); @@ -63,7 +63,7 @@ fn recorded_actions_leave_out_replay_only_values() { // …while values resolved from the preferences stay recorded. let scale = steps.iter().find(|(id, _)| id == "object.scale").unwrap(); assert!(scale.1.get("strokes").is_some(), "{scale:?}"); - assert_eq!(s.journal_for_action(1), s.journal[1..].to_vec()); + assert_eq!(s.journal_for_action(1), s.journal.since(1).to_vec()); } /// The artboard the app passes to Paste in Place, in Front and in Back, to Align to Artboard, to @@ -87,7 +87,7 @@ fn recorded_actions_leave_out_the_artboard_the_app_passes() { for (id, p) in &steps { s.execute(id, p).unwrap(); } - assert!(s.journal[start..].iter().all(|(_, p)| p["artboard"] == json!(1)), "the journal keeps it: {:?}", &s.journal[start..]); + assert!(s.journal.since(start).iter().all(|(_, p)| p["artboard"] == json!(1)), "the journal keeps it: {:?}", s.journal.since(start)); let action = s.journal_for_action(start); assert_eq!(action.len(), steps.len()); for ((id, mut p), (step, recorded)) in steps.into_iter().zip(action) { diff --git a/crates/engine/tests/pen_endpoint_picking.rs b/crates/engine/tests/pen_endpoint_picking.rs index b6f29353..0d372a64 100644 --- a/crates/engine/tests/pen_endpoint_picking.rs +++ b/crates/engine/tests/pen_endpoint_picking.rs @@ -148,7 +148,7 @@ fn grid_continue(preselected: bool, snap_to_grid: bool) { assert_eq!(path_ids(&s), vec![id], "clicking the advertised endpoint must continue it, not create a snapped path"); assert_eq!(node(&s, id), &original); assert_eq!(s.doc().unwrap().selection.objects, vec![id]); - assert!(!s.journal[journal_start..].iter().any(|(command, _)| command == "path.create")); + assert!(!s.journal.since(journal_start).iter().any(|(command, _)| command == "path.create")); let before_append = s.doc().unwrap().history.undo.len(); gesture(&mut s, view, &[(200.0, 300.0)], Mods::default()); assert_eq!(path_ids(&s), vec![id]); @@ -328,7 +328,7 @@ fn snapped_only_continue(preselected: bool) { assert_eq!(s.doc().unwrap().selection.objects, vec![id]); let expected = if first { [(200.0, 300.0), (100.0, 300.0)] } else { [(100.0, 300.0), (200.0, 300.0)] }; assert_eq!(points(&s, id), expected, "continue from the snapped end: selected={preselected}, first={first}"); - assert!(!s.journal[journal_start..].iter().any(|(command, _)| command == "path.create")); + assert!(!s.journal.since(journal_start).iter().any(|(command, _)| command == "path.create")); assert!(!s.in_interaction() && !s.tool_busy()); assert_eq!(s.doc().unwrap().history.undo.len(), before_continue + usize::from(first)); let continued = node(&s, id).clone(); @@ -342,7 +342,7 @@ fn snapped_only_continue(preselected: bool) { assert_eq!(path_ids(&s), vec![id]); assert_eq!(s.doc().unwrap().selection.objects, vec![id]); assert_eq!(points(&s, id), [expected[0], expected[1], next]); - assert!(!s.journal[journal_start..].iter().any(|(command, _)| command == "path.create")); + assert!(!s.journal.since(journal_start).iter().any(|(command, _)| command == "path.create")); assert!(!s.in_interaction() && !s.tool_busy()); assert_eq!(s.doc().unwrap().history.undo.len(), before_append + 1); let extended = node(&s, id).clone(); @@ -393,7 +393,7 @@ fn raw_unselected_endpoint_precedes_snapped_selected_endpoint() { assert_eq!(path_ids(&s), vec![selected, raw]); assert_eq!(node(&s, selected), &original_selected); assert_eq!(node(&s, raw), &original_raw); - assert!(!s.journal[journal_start..].iter().any(|(command, _)| command == "path.create")); + assert!(!s.journal.since(journal_start).iter().any(|(command, _)| command == "path.create")); assert!(!s.in_interaction() && !s.tool_busy()); assert_eq!(s.doc().unwrap().history.undo.len(), before_append); @@ -402,7 +402,7 @@ fn raw_unselected_endpoint_precedes_snapped_selected_endpoint() { assert_eq!(s.doc().unwrap().selection.objects, vec![raw]); assert_eq!(node(&s, selected), &original_selected); assert_eq!(points(&s, raw), [(13.0, 303.0), (63.0, 303.0), (200.0, 400.0)]); - assert!(!s.journal[journal_start..].iter().any(|(command, _)| command == "path.create")); + assert!(!s.journal.since(journal_start).iter().any(|(command, _)| command == "path.create")); assert!(!s.in_interaction() && !s.tool_busy()); assert_eq!(s.doc().unwrap().history.undo.len(), before_append + 1); let extended = node(&s, raw).clone(); diff --git a/crates/eps/src/import/ai/read.rs b/crates/eps/src/import/ai/read.rs index 2a3204aa..f61825a5 100644 --- a/crates/eps/src/import/ai/read.rs +++ b/crates/eps/src/import/ai/read.rs @@ -1528,7 +1528,8 @@ impl<'a> Reader<'a> { } if self.color_model.unwrap_or(self.cmyk_colors > self.rgb_colors) { doc.color_mode = ColorMode::Cmyk; - (doc.swatches, doc.swatch_groups) = vectorcraft_color::default_swatches(ColorMode::Cmyk.model()); + let (swatches, groups) = vectorcraft_color::default_swatches(ColorMode::Cmyk.model()); + (doc.swatches, doc.swatch_groups) = (swatches.into(), groups.into()); } for n in &self.spots { if doc.swatch(&n.name).is_none() { diff --git a/crates/eps/src/import/mod.rs b/crates/eps/src/import/mod.rs index 02906f66..d6e9f92a 100644 --- a/crates/eps/src/import/mod.rs +++ b/crates/eps/src/import/mod.rs @@ -250,7 +250,8 @@ fn finish_with(out: Out, layers: Vec>) -> Imported { let mut doc = out.doc; if out.cmyk > out.rgb { doc.color_mode = ColorMode::Cmyk; - (doc.swatches, doc.swatch_groups) = vectorcraft_color::default_swatches(ColorMode::Cmyk.model()); + let (swatches, groups) = vectorcraft_color::default_swatches(ColorMode::Cmyk.model()); + (doc.swatches, doc.swatch_groups) = (swatches.into(), groups.into()); } for (name, color) in out.spots { if doc.swatch(&name).is_none() { diff --git a/crates/pdf/src/import.rs b/crates/pdf/src/import.rs index a8a39eba..d61e06f8 100644 --- a/crates/pdf/src/import.rs +++ b/crates/pdf/src/import.rs @@ -336,7 +336,8 @@ pub fn import_with_report(bytes: &[u8], opts: &ImportOptions) -> Result, + journal: vectorcraft_engine::Journal, dirty: bool, } diff --git a/crates/ui-egui/src/panels/actions.rs b/crates/ui-egui/src/panels/actions.rs index b211a5cf..543da8ac 100644 --- a/crates/ui-egui/src/panels/actions.rs +++ b/crates/ui-egui/src/panels/actions.rs @@ -98,6 +98,7 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut egui::Ui) { && let Some((set, name, start)) = app.ui.recording.take() { let steps = app.session.journal_for_action(start); + app.session.journal.hold(None); if let Some(s) = app.ui.action_sets.get_mut(set) { s.actions.push(Action { name, steps }); } @@ -110,7 +111,10 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut egui::Ui) { app.ui.action_sets.push(ActionSet { name: "Set 1".into(), actions: vec![] }); } let set = selected.map(|s| s.0).unwrap_or(0); - app.ui.recording = Some((set, format!("Action {n}"), app.session.journal.len())); + let start = app.session.journal.len(); + // Keep every step from here on until recording stops, however long it runs. + app.session.journal.hold(Some(start)); + app.ui.recording = Some((set, format!("Action {n}"), start)); } if widgets::icon_button(ui, "dc-actions", tl!("Play Current Selection"), false, 24.0).clicked() && let Some(s) = selected diff --git a/crates/ui-egui/src/panels/swatches.rs b/crates/ui-egui/src/panels/swatches.rs index 518d5e5e..122a4001 100644 --- a/crates/ui-egui/src/panels/swatches.rs +++ b/crates/ui-egui/src/panels/swatches.rs @@ -514,7 +514,7 @@ fn move_params(d: &Document, names: &[String], target: Option<&str>, after: bool Some(t) => { let (group, list) = match d.swatch_group_of(t) { Some(g) => (Some(d.swatch_groups[g].name.as_str()), &d.swatch_groups[g].swatches), - None => (None, &d.swatches), + None => (None, &*d.swatches), }; (group, list.iter().position(|w| w.name == t).map(|i| i + usize::from(after))) } @@ -1415,7 +1415,7 @@ mod tests { let d = &app.session.active().unwrap().doc; let list = match group { Some(g) => &d.swatch_groups.iter().find(|x| x.name == g).unwrap().swatches, - None => &d.swatches, + None => &*d.swatches, }; list.iter().map(|w| w.name.clone()).collect() }