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() }