diff --git a/apps/vectorcraft-cli/src/perf.rs b/apps/vectorcraft-cli/src/perf.rs index 41da4abc..9652d4f7 100644 --- a/apps/vectorcraft-cli/src/perf.rs +++ b/apps/vectorcraft-cli/src/perf.rs @@ -113,11 +113,124 @@ fn edit_drags(n: usize) -> Result<(f64, f64), String> { Ok((handle, segment)) } +/// Resident memory of this process in MB (`ps`), if available. +fn rss_mb() -> Option { + let pid = std::process::id().to_string(); + let out = std::process::Command::new("ps").args(["-o", "rss=", "-p", &pid]).output().ok().filter(|o| o.status.success())?; + String::from_utf8_lossy(&out.stdout).trim().parse::().ok().map(|kb| kb / 1024.0) +} + +/// `--deep`: tracked rows (no budget yet) for work the budgets above don't reach: editing a whole +/// selection, live effects, every Pathfinder operation, typing in a long story, and memory. +fn deep(n: usize) -> Result<(), String> { + let err = |e: vectorcraft_engine::EngineError| e.to_string(); + let mut rows: Vec<(String, f64)> = vec![]; + let mut mem: Vec<(String, f64)> = vec![]; + let mut note = |what: &str| { + if let Some(m) = rss_mb() { + mem.push((what.to_string(), m)); + } + }; + note("start"); + // Editing every object of an n-path document. + let mut s = vectorcraft_engine::Session::new(); + s.add_document(synthetic(n)?, None); + note("document built"); + rows.push((format!("select all, {n} paths"), median_ms(3, || drop(s.execute("select.all", &json!({})))))); + rows.push((format!("nudge the selection ({n} paths)"), median_ms(5, || drop(s.execute("object.move", &json!({"dx": 1, "dy": 0})))))); + rows.push((format!("undo a nudge ({n} paths)"), median_ms(5, || drop(s.execute("edit.undo", &json!({})))))); + rows.push((format!("redo a nudge ({n} paths)"), median_ms(5, || drop(s.execute("edit.redo", &json!({})))))); + // Dragging the whole selection with the Selection tool: one pointer move (the preview). + { + use vectorcraft_engine::tools::{Mods, PointerEvent, PointerKind}; + use vectorcraft_geom::Vec2; + let view = vectorcraft_engine::ViewInfo { zoom: 1.0, ..Default::default() }; + let first = s.doc().map_err(err)?.selection.objects.first().copied(); + let at = first.and_then(|id| s.doc().ok()?.doc.node(id)?.geometric_bounds()).map(|b| b.center()); + if let Some(down) = at { + let ev = |kind, dx: f64| PointerEvent { kind, pos: down + Vec2::new(dx, 0.0), mods: Mods::default(), pressure: 1.0 }; + s.select_tool("selection", view).map_err(err)?; + s.pointer(&ev(PointerKind::Down, 0.0), view).map_err(err)?; + let mut dx = 0.0; + rows.push(( + format!("drag the selection, per move ({n} paths)"), + median_ms(5, || { + dx += 2.0; + let _ = s.pointer(&ev(PointerKind::Drag, dx), view); + }), + )); + s.pointer(&ev(PointerKind::Up, dx), view).map_err(err)?; + } + } + for _ in 0..40 { + s.execute("object.move", &json!({"dx": 1, "dy": 0})).map_err(err)?; + } + note("after 45 whole-selection edits"); + drop(s); + // Live effects on 2,000 paths: render a page whose every object has the effect. + let opts = RenderOptions::default(); + for (fx, params) in [("path.offsetPath", json!({"offset": 3})), ("distort.roughen", json!({"size": 5, "relative": true}))] { + let mut s = vectorcraft_engine::Session::new(); + s.add_document(synthetic(2000)?, None); + s.execute("select.all", &json!({})).map_err(err)?; + s.execute("effect.apply", &json!({"effect": fx, "params": params})).map_err(err)?; + let doc = s.doc().map_err(err)?.doc.clone(); + let mut r = Renderer::new(); + r.render(&doc, 2880, 1800, Affine::scale(1.6), &opts); + rows.push((format!("render, 2,000 paths with {fx} (warm)"), median_ms(5, || drop(r.render(&doc, 2880, 1800, Affine::scale(1.6), &opts))))); + } + note("after live effects"); + // Every Pathfinder operation on 300 overlapping paths. + for op in ["unite", "intersect", "exclude", "minusFront", "minusBack", "divide", "trim", "merge", "crop", "outline"] { + let ms = median_ms(1, || { + let mut s = vectorcraft_engine::Session::new(); + if let Ok(d) = synthetic(300) { + s.add_document(d, None); + let _ = s.execute("select.all", &json!({})).and_then(|_| s.execute(&format!("object.pathfinder.{op}"), &json!({}))); + } + }); + rows.push((format!("Pathfinder {op}, 300 paths"), ms)); + } + note("after Pathfinder"); + // Typing one character in the middle of a 20,000-character area-type story. + let mut s = vectorcraft_engine::Session::new(); + s.add_document(Document::new(1600.0, 1200.0), None); + let text: String = (0..2000).map(|i| if i % 12 == 11 { "paragraph.\n" } else { "lorem ips " }).collect(); + let id = s.execute("text.create", &json!({"x": 20, "y": 20, "text": text, "size": 10, "area": {"width": 1500, "height": 1100}})).map_err(err)?; + let id = id.get("id").cloned().unwrap_or(json!(0)); + let mut at = 10_000; + rows.push(( + "type one character, 20,000-character area text".into(), + median_ms(9, || { + at += 1; + let _ = s.execute("text.editRange", &json!({"id": id, "start": at, "end": at, "insert": "x"})); + }), + )); + note("after typing"); + // Output hashes, to compare builds: the SVG export and a save → open → save round trip. + let fnv = |b: &[u8]| b.iter().fold(0xcbf2_9ce4_8422_2325u64, |h, &x| (h ^ u64::from(x)).wrapping_mul(0x100_0000_01b3)); + let doc = synthetic(n)?; + let svg = vectorcraft_svg::export(&doc, &vectorcraft_svg::ExportOptions::default()); + let saved = vectorcraft_format::save_file(&doc); + let reopened = vectorcraft_format::load(&saved).map_err(|e| e.to_string())?; + outln!(" output hash: SVG export {:016x}, save→open→save {:016x}", fnv(svg.as_bytes()), fnv(&vectorcraft_format::save_file(&reopened))); + outln!(" tracked (no budget yet)"); + for (name, ms) in &rows { + outln!(" {name:<52} {ms:>10.2} ms"); + } + for (what, mb) in &mem { + outln!(" memory {what:<45} {mb:>8.0} MB"); + } + Ok(()) +} + pub fn run(args: &[String]) -> Result<(), String> { let mut n = 50_000usize; + let mut want_deep = false; let mut it = args.iter(); while let Some(a) = it.next() { match a.as_str() { + "--deep" => want_deep = true, "--paths" => n = it.next().and_then(|v| v.parse().ok()).ok_or("--paths needs a number")?, other => return Err(format!("unknown perf option `{other}`")), } @@ -200,6 +313,9 @@ pub fn run(args: &[String]) -> Result<(), String> { if let Some(e) = failed { return Err(e); } + if want_deep { + deep(n)?; + } let mut over = 0; outln!(" {:<40} {:>10} {:>10}", "", "measured", "budget"); diff --git a/crates/brush/src/defaults.rs b/crates/brush/src/defaults.rs index 0a9f4367..8cf74673 100644 --- a/crates/brush/src/defaults.rs +++ b/crates/brush/src/defaults.rs @@ -26,7 +26,7 @@ fn group(children: Vec) -> Node { fn compound(paths: Vec, c: Color) -> Node { let children = paths.into_iter().map(|p| Arc::new(Node::path(NodeId(0), p, Appearance::default()))).collect(); let mut n = Node::new(NodeId(0), NodeKind::Compound { children, rule: FillRule::EvenOdd }); - n.appearance = fill(c); + n.appearance = fill(c).into(); n } @@ -121,7 +121,7 @@ pub(crate) fn stitch_tile() -> Node { let dash = path(shapes::rounded_rectangle(Rect::new(1.5, -0.75, 8.5, 0.75), 0.75), Color::BLACK); // Invisible spacer that fixes the tile width. let mut spacer = path(shapes::rectangle(Rect::new(0.0, -0.75, 10.0, 0.75)), Color::BLACK); - spacer.appearance = Appearance::default(); + spacer.appearance = Appearance::default().into(); group(vec![spacer, dash]) } diff --git a/crates/brush/src/tests.rs b/crates/brush/src/tests.rs index 631be2d5..b012fb97 100644 --- a/crates/brush/src/tests.rs +++ b/crates/brush/src/tests.rs @@ -118,7 +118,7 @@ fn art_brush_arc_length_mapping_is_linear() { Node::path(NodeId(0), shapes::rectangle(Rect::new(5.0, -1.0, 6.0, 1.0)), Appearance::basic(Paint::solid(Color::BLACK), Paint::None, 0.0)); let frame = rect_art(10.0, 2.0); let mut spacer = frame.clone(); - spacer.appearance = Appearance::default(); + spacer.appearance = Appearance::default().into(); let art = Node::group(NodeId(0), vec![std::sync::Arc::new(spacer), std::sync::Arc::new(marker)]); let out = stroke_pieces(&art_brush(art, ArtScale::Stretch), &line(0.0, 0.0, 300.0, 0.0), &stroke(1.0)); let g = &out[0]; diff --git a/crates/cad/src/import/build.rs b/crates/cad/src/import/build.rs index 404b6a16..b729d800 100644 --- a/crates/cad/src/import/build.rs +++ b/crates/cad/src/import/build.rs @@ -479,7 +479,7 @@ impl<'d, 'a> Build<'d, 'a> { .into_iter() .map(|sp| { let mut c = Node::new(self.doc.alloc_id(), kind(PathData::single(sp))); - c.appearance = appearance.clone(); + c.appearance = appearance.clone().into(); Arc::new(c) }) .collect(); @@ -487,7 +487,7 @@ impl<'d, 'a> Build<'d, 'a> { } else { Node::new(self.doc.alloc_id(), kind(path)) }; - n.appearance = appearance; + n.appearance = appearance.into(); Some(n) } diff --git a/crates/doc/src/appearance.rs b/crates/doc/src/appearance.rs index e7ad78a8..5b1dd736 100644 --- a/crates/doc/src/appearance.rs +++ b/crates/doc/src/appearance.rs @@ -942,7 +942,7 @@ impl Appearance { }) } /// Gradient paints of the fills and strokes. - fn gradients(&self) -> impl Iterator { + pub(crate) fn gradients(&self) -> impl Iterator { self.items.iter().filter_map(|i| match i { AppearanceItem::Fill(FillLayer { paint: Paint::Gradient(g), .. }) | AppearanceItem::Stroke(StrokeLayer { paint: Paint::Gradient(g), .. }) => Some(&**g), diff --git a/crates/doc/src/assets.rs b/crates/doc/src/assets.rs index ec84333a..3c4451d5 100644 --- a/crates/doc/src/assets.rs +++ b/crates/doc/src/assets.rs @@ -50,7 +50,9 @@ impl Document { /// `ids` that are in the document, back to front. pub fn paint_order(&self, ids: impl IntoIterator) -> Vec { - let mut v: Vec<(Vec, NodeId)> = ids.into_iter().filter_map(|id| self.index_path(id).map(|p| (p, id))).collect(); + let ids: Vec = ids.into_iter().collect(); + let paths = self.index_paths(ids.iter().copied()); + let mut v: Vec<(Vec, NodeId)> = ids.into_iter().filter_map(|id| paths.get(&id).map(|p| (p.clone(), id))).collect(); v.sort(); v.into_iter().map(|(_, id)| id).collect() } diff --git a/crates/doc/src/blend.rs b/crates/doc/src/blend.rs index 5695834a..e52bd366 100644 --- a/crates/doc/src/blend.rs +++ b/crates/doc/src/blend.rs @@ -619,7 +619,7 @@ impl Lerp { } }; n.id = NodeId(0); - n.appearance = lerp_appearance(&a.appearance, &b.appearance, t); + n.appearance = lerp_appearance(&a.appearance, &b.appearance, t).into(); n.opacity = lerp_f32(a.opacity, b.opacity, t as f32); n.blend = base.blend; n.visible = true; diff --git a/crates/doc/src/hit.rs b/crates/doc/src/hit.rs index 7cc00f83..12c58bad 100644 --- a/crates/doc/src/hit.rs +++ b/crates/doc/src/hit.rs @@ -463,7 +463,7 @@ mod tests { d.insert(Some(l), 0, Node::path(a, shapes::rectangle(Rect::new(0.0, 0.0, 50.0, 50.0)), Appearance::default_art())).unwrap(); let inner = Node::path(d.alloc_id(), shapes::rectangle(Rect::new(100.0, 0.0, 150.0, 50.0)), Appearance::default()); let mut compound = Node::new(c, NodeKind::Compound { children: vec![Arc::new(inner)], rule: vectorcraft_geom::FillRule::NonZero }); - compound.appearance = Appearance::default_art(); + compound.appearance = Appearance::default_art().into(); d.insert(Some(l), 9, compound).unwrap(); let path_only = HitOptions { path_only: true, ..Default::default() }; for (id, inside, edge) in [(a, Point::new(25.0, 25.0), Point::new(0.5, 25.0)), (c, Point::new(125.0, 25.0), Point::new(100.5, 25.0))] { diff --git a/crates/doc/src/id_index.rs b/crates/doc/src/id_index.rs new file mode 100644 index 00000000..cdb32c32 --- /dev/null +++ b/crates/doc/src/id_index.rs @@ -0,0 +1,142 @@ +//! A cache of every node's index path, so a lookup by id checks one path instead of walking the +//! tree. +//! +//! The tree is edited in many places through public fields, so the cache can't be kept in step +//! with it. It is checked instead: a cached path is used only while the node at that path still +//! has the id (a check as deep as the tree). A path that no longer checks out rebuilds the cache, +//! and an id the cache doesn't have (deleted, or added since) is looked up by walking, as before. +//! An id that appears more than once (a damaged file) is never cached, so lookups keep returning +//! its first place in tree order. +//! +//! The one case a cached lookup can answer differently from a walk: a second node with an +//! existing id put in the tree *after* the cache was built, earlier in tree order than the first. +//! The lookup then returns the original node (also one with that id) instead of the new one. The +//! app never makes such a node (paste and duplicate give new ids, undo restores whole trees); +//! duplicated ids come from damaged files, which are cached afresh when opened. + +use std::collections::HashMap; +use std::hash::{BuildHasherDefault, Hasher}; +use std::sync::{Arc, RwLock}; + +use crate::{Node, NodeId}; + +/// Hasher for node ids: one multiply. Ids are small counters, not attacker-chosen keys, and the +/// default SipHash dominated the cost of building the cache. +#[derive(Default)] +pub struct IdHasher(u64); + +impl Hasher for IdHasher { + fn write(&mut self, bytes: &[u8]) { + for &b in bytes { + self.0 = (self.0 ^ u64::from(b)).wrapping_mul(0x100_0000_01b3); + } + } + fn write_u64(&mut self, v: u64) { + self.0 = (self.0 ^ v).wrapping_mul(0x9e37_79b9_7f4a_7c15); + } + fn finish(&self) -> u64 { + self.0 ^ (self.0 >> 29) + } +} + +/// A map keyed by node id with [`IdHasher`]. +pub type IdMap = HashMap>; +/// Where each node is: its parent's entry (`u32::MAX` for a top-level layer) and its index among +/// its siblings. One flat table, so building it allocates nothing per node. +#[derive(Default)] +pub(crate) struct Paths { + /// Each id's entry in `at`; `None` for an id that appears more than once. + ids: IdMap>, + at: Vec<(u32, u32)>, +} + +impl Paths { + /// The index path of `id`: `Some(None)` when the id appears more than once, `None` when it + /// isn't in the table. + pub(crate) fn get(&self, id: &NodeId) -> Option>> { + let e = (*self.ids.get(id)?)?; + let mut path = Vec::new(); + let mut cur = e; + // Each step goes to a parent entry, made before its children: at most `at.len()` steps. + for _ in 0..=self.at.len() { + let Some(&(parent, i)) = self.at.get(cur as usize) else { return Some(None) }; + path.push(i as usize); + if parent == u32::MAX { + path.reverse(); + return Some(Some(path)); + } + cur = parent; + } + Some(None) + } + pub(crate) fn contains_key(&self, id: &NodeId) -> bool { + self.ids.contains_key(id) + } + pub(crate) fn len(&self) -> usize { + self.ids.len() + } +} + +/// Lookups that had to walk (the cache didn't have the id, or its path no longer checks out) +/// before the cache is rebuilt: a rebuild costs a few walks, so rebuilding only after a few keeps +/// a lookup's cost within a small factor of a walk whatever the edit pattern (say, delete an +/// object and look the next one up, again and again). +const MISSES_PER_REBUILD: u32 = 4; + +/// The cache, shared by clones of a document (each checks every path it uses against its own tree). +#[derive(Default)] +pub struct IdIndex(RwLock>>, std::sync::atomic::AtomicU32); + +impl IdIndex { + /// The current cache, if one was built. + pub(crate) fn get(&self) -> Option> { + self.0.read().ok().and_then(|g| g.clone()) + } + + /// 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) { + for (i, n) in nodes.iter().enumerate() { + let e = out.at.len() as u32; + out.at.push((parent, i as u32)); + out.ids.entry(n.id).and_modify(|x| *x = None).or_insert(Some(e)); + if let Some(ch) = n.children() { + walk(ch, e, out); + } + } + } + let mut out = Paths::default(); + walk(layers, u32::MAX, &mut out); + let out = Arc::new(out); + if let Ok(mut g) = self.0.write() { + *g = Some(out.clone()); + } + self.1.store(0, std::sync::atomic::Ordering::Relaxed); + out + } + + /// Note a lookup that had to walk (an id the cache didn't have, or a path that no longer + /// checks out); true when it's time to rebuild. + pub(crate) fn missed(&self) -> bool { + self.1.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + 1 >= MISSES_PER_REBUILD + } +} + +impl Clone for IdIndex { + fn clone(&self) -> Self { + IdIndex(RwLock::new(self.get()), Default::default()) + } +} + +/// A cache, not document content: two documents are equal whatever their caches hold. +impl PartialEq for IdIndex { + fn eq(&self, _: &Self) -> bool { + true + } +} + +impl std::fmt::Debug for IdIndex { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("IdIndex") + } +} diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index 259512b8..ca7d744d 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -13,9 +13,12 @@ pub mod cmyk; pub mod corners; pub mod graph; pub mod hit; +mod id_index; pub mod inks; mod inline; +pub mod shared; pub use inline::{SYMBOL_HALF, SYMBOL_SIZES}; +pub use shared::Shared; pub mod links; pub mod live; pub mod marks; @@ -658,6 +661,9 @@ pub struct Document { /// state, never saved. #[serde(skip)] pub puppet: Option>, + /// Where each node is in the tree, for lookups by id ([`id_index`]): a cache, never saved. + #[serde(skip)] + index: id_index::IdIndex, } fn ppi72() -> f64 { @@ -729,6 +735,7 @@ impl Document { print_setup: None, assets: vec![], puppet: None, + index: Default::default(), }; let id = d.alloc_id(); d.layers.push(Arc::new(Node::layer(id, "Layer 1", LayerColor::Preset(0)))); @@ -758,24 +765,31 @@ impl Document { /// Find a node anywhere in the tree. pub fn node(&self, id: NodeId) -> Option<&Node> { - fn find(nodes: &[Arc], id: NodeId) -> Option<&Node> { - for n in nodes { - if n.id == id { - return Some(n); - } - if let Some(ch) = n.children() - && let Some(f) = find(ch, id) - { - return Some(f); - } - } - None - } - find(&self.layers, id) + self.index_path(id).and_then(|p| self.node_at(&p)) } - /// Index path from the top-level layer list to `id` (e.g. `[0, 3, 1]`). + /// 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> { + // The cached path, while the node there still has the id ([`id_index`]). + let cached = self.index.get(); + match cached.as_ref().and_then(|m| m.get(&id)) { + Some(Some(p)) if self.node_at(&p).is_some_and(|n| n.id == id) => return Some(p), + Some(None) => return self.walk_index_path(id), + _ => {} + } + // A stale path (the tree changed), an id the cache doesn't have (deleted or added since), + // or no cache yet: walk, and rebuild now and then. + let found = self.walk_index_path(id); + let stale = cached.as_ref().is_some_and(|m| m.contains_key(&id)); + if cached.is_none() || ((stale || found.is_some()) && self.index.missed()) { + self.index.rebuild(&self.layers); + } + found + } + + /// [`Document::index_path`] by walking the tree. + fn walk_index_path(&self, id: NodeId) -> Option> { fn find(nodes: &[Arc], id: NodeId, path: &mut Vec) -> bool { for (i, n) in nodes.iter().enumerate() { path.push(i); @@ -795,6 +809,85 @@ impl Document { find(&self.layers, id, &mut p).then_some(p) } + /// [`Document::index_path`] of every id in `ids` that is in the tree, in one walk of it: an + /// edit of many objects looks them all up for the cost of one search instead of one each. Like + /// `index_path`, an id that appears twice maps to its first place in tree order. + pub fn index_paths(&self, ids: impl IntoIterator) -> std::collections::HashMap> { + use std::collections::{HashMap, HashSet}; + fn walk(nodes: &[Arc], want: &HashSet, path: &mut Vec, out: &mut HashMap>) -> bool { + for (i, n) in nodes.iter().enumerate() { + path.push(i); + if want.contains(&n.id) && !out.contains_key(&n.id) { + out.insert(n.id, path.clone()); + if out.len() == want.len() { + return true; + } + } + if let Some(ch) = n.children() + && walk(ch, want, path, out) + { + return true; + } + path.pop(); + } + false + } + let mut want: HashSet = ids.into_iter().collect(); + let mut out = HashMap::with_capacity(want.len()); + // Cached paths that still check out ([`id_index`]); the rest take one walk. For a good + // share of the document, one walk is quicker than checking each cached path. + if let Some(cache) = self.index.get().filter(|c| want.len().saturating_mul(8) < c.len()) { + want.retain(|id| match cache.get(id) { + Some(Some(p)) if self.node_at(&p).is_some_and(|n| n.id == *id) => { + out.insert(*id, p); + false + } + _ => true, + }); + } + if !want.is_empty() { + let mut rest = HashMap::with_capacity(want.len()); + walk(&self.layers, &want, &mut Vec::new(), &mut rest); + out.extend(rest); + } + out + } + + /// The node at index path `path` (as [`Document::index_path`] gives it). + pub fn node_at(&self, path: &[usize]) -> Option<&Node> { + let (first, rest) = path.split_first()?; + let mut cur: &Node = self.layers.get(*first)?; + for &i in rest { + cur = cur.children()?.get(i)?; + } + Some(cur) + } + + /// Mutable access to the node at index path `path`; clones the Arc path (copy-on-write), as + /// [`Document::node_mut`] does. + pub fn node_mut_at(&mut self, path: &[usize]) -> Option<&mut Node> { + let (first, rest) = path.split_first()?; + let mut cur: &mut Arc = self.layers.get_mut(*first)?; + for &i in rest { + cur = Arc::make_mut(cur).children_mut()?.get_mut(i)?; + } + Some(Arc::make_mut(cur)) + } + + /// [`Document::ancestry`] of the node at index path `path`. + pub fn ancestry_at(&self, path: &[usize]) -> Option> { + let mut out = Vec::with_capacity(path.len()); + let mut nodes = &self.layers; + for &i in path { + let n = nodes.get(i)?; + out.push(n.id); + if let Some(ch) = n.children() { + nodes = ch; + } + } + Some(out) + } + /// Ids from the top-level layer down to `id` inclusive. pub fn ancestry(&self, id: NodeId) -> Option> { let path = self.index_path(id)?; @@ -1391,6 +1484,8 @@ mod tests { } } +#[cfg(test)] +mod tests_lookups; #[cfg(test)] mod tests_slices; #[cfg(test)] diff --git a/crates/doc/src/live.rs b/crates/doc/src/live.rs index 51641b14..0759e293 100644 --- a/crates/doc/src/live.rs +++ b/crates/doc/src/live.rs @@ -1524,7 +1524,7 @@ fn pattern_art(n: &Node, tiles: PatternTiles) -> Option { }; let plain = |items: Vec| { Arc::new(Node { - appearance: Appearance { items, ..Default::default() }, + appearance: Appearance { items, ..Default::default() }.into(), opacity: 1.0, blend: Default::default(), isolate: false, @@ -1560,7 +1560,7 @@ fn pattern_art(n: &Node, tiles: PatternTiles) -> Option { } Some(Node { kind: NodeKind::Group { children: pieces, clip: false }, - appearance: Appearance { effects: n.appearance.effects.clone(), ..Default::default() }, + appearance: Appearance { effects: n.appearance.effects.clone(), ..Default::default() }.into(), ..n.clone() }) } diff --git a/crates/doc/src/node.rs b/crates/doc/src/node.rs index 48d7fc7b..9eb4b0cd 100644 --- a/crates/doc/src/node.rs +++ b/crates/doc/src/node.rs @@ -514,7 +514,7 @@ pub struct Node { #[serde(default, skip_serializing_if = "crate::skip::is_default")] pub knockout_shape: bool, #[serde(default, skip_serializing_if = "crate::skip::is_default")] - pub appearance: Appearance, + pub appearance: crate::Shared, /// Opacity mask (Transparency panel). Its art lives here, outside the layer tree. #[serde(default, skip_serializing_if = "Option::is_none")] pub mask: Option>, @@ -594,7 +594,7 @@ impl Node { isolate: false, knockout: Knockout::Neutral, knockout_shape: false, - appearance: Appearance::default(), + appearance: Appearance::default().into(), mask: None, trace: None, wrap: None, @@ -611,7 +611,7 @@ impl Node { } pub fn path(id: NodeId, path: PathData, appearance: Appearance) -> Self { let mut n = Self::new(id, NodeKind::Path { path, rule: FillRule::NonZero, live: None, clipping: false, guide: false }); - n.appearance = appearance; + n.appearance = appearance.into(); n } pub fn group(id: NodeId, children: Vec>) -> Self { @@ -854,7 +854,11 @@ impl Node { self.appearance.scale_effects(k, f); } } - self.appearance.transform_gradients(a); + // Only through `DerefMut` when there is something to change: the appearance is shared + // copy-on-write (`Shared`), and a move must not copy what it leaves as it is. + if self.appearance.gradients().next().is_some() { + self.appearance.transform_gradients(a); + } if sc.patterns { crate::pattern::transform_pattern_paints(self, a); } diff --git a/crates/doc/src/pattern.rs b/crates/doc/src/pattern.rs index aa185815..39f4e11a 100644 --- a/crates/doc/src/pattern.rs +++ b/crates/doc/src/pattern.rs @@ -343,6 +343,14 @@ pub fn uses_pattern(n: &Node, name: Option<&str>) -> bool { /// Apply `m` to the pattern placement of every pattern paint of `n` (Transform Patterns). pub fn transform_pattern_paints(n: &mut Node, m: Affine) { + // Without pattern paints there is nothing to change: don't copy the shared appearance. + let patterned = |it: &AppearanceItem| { + matches!(it, AppearanceItem::Fill(f) if matches!(f.paint, Paint::Pattern { .. })) + || matches!(it, AppearanceItem::Stroke(s) if matches!(s.paint, Paint::Pattern { .. })) + }; + if !n.appearance.items.iter().any(patterned) { + return; + } for it in n.appearance.items.iter_mut() { let p = match it { AppearanceItem::Fill(f) => &mut f.paint, diff --git a/crates/doc/src/selection.rs b/crates/doc/src/selection.rs index 2726d149..d8914465 100644 --- a/crates/doc/src/selection.rs +++ b/crates/doc/src/selection.rs @@ -62,9 +62,9 @@ impl Selection { } pub fn set(&mut self, ids: impl IntoIterator) { self.clear(); - for id in ids { - self.add(id); - } + // What `add` per id does after a clear, without its linear duplicate check per id. + let mut seen = std::collections::HashSet::new(); + self.objects.extend(ids.into_iter().filter(|id| seen.insert(*id))); } pub fn add(&mut self, id: NodeId) { self.target = None; @@ -118,10 +118,13 @@ impl Selection { } /// Drop ids that no longer exist or are no longer editable. pub fn prune(&mut self, doc: &Document) { - self.objects.retain(|id| doc.node(*id).is_some()); - self.anchors.retain(|id, _| doc.node(*id).is_some()); + // Every id this looks up, found in one walk of the tree. + let ids = self.objects.iter().chain(self.anchors.keys()).copied().chain(self.key).chain(self.target); + let found = doc.index_paths(ids); + self.objects.retain(|id| found.contains_key(id)); + self.anchors.retain(|id, _| found.contains_key(id)); // The key object stays while another selected object lies outside it. - let keep = self.key.and_then(|k| doc.node(k)).is_some_and(|n| { + let keep = self.key.and_then(|k| found.get(&k)).and_then(|p| doc.node_at(p)).is_some_and(|n| { let mut inside = BTreeSet::new(); n.walk(&mut |c| { inside.insert(c.id); @@ -131,7 +134,7 @@ impl Selection { if !keep { self.key = None; } - if self.target.is_some_and(|t| doc.node(t).is_none()) { + if self.target.is_some_and(|t| !found.contains_key(&t)) { self.target = None; } self.slices.retain(|id| doc.is_slice(*id)); diff --git a/crates/doc/src/shared.rs b/crates/doc/src/shared.rs new file mode 100644 index 00000000..208ccd4c --- /dev/null +++ b/crates/doc/src/shared.rs @@ -0,0 +1,72 @@ +//! [`Shared`]: a value shared between copies of a node until one of them changes it. + +use std::sync::Arc; + +use serde::{Deserialize, Deserializer, Serialize, Serializer}; + +/// A copy-on-write value: cloning shares it, and the first change through `DerefMut` makes the +/// changed copy its own (`Arc::make_mut`). A node moved or recoloured keeps sharing what it didn't +/// change with the version the undo history holds. Reads, writes, equality and the saved form are +/// exactly those of the value itself. +#[derive(Clone, Default)] +pub struct Shared(Arc); + +impl Shared { + pub fn new(v: T) -> Self { + Shared(Arc::new(v)) + } + /// Whether `a` and `b` share one value (an unchanged copy). + pub fn ptr_eq(a: &Self, b: &Self) -> bool { + Arc::ptr_eq(&a.0, &b.0) + } +} + +impl std::ops::Deref for Shared { + type Target = T; + fn deref(&self) -> &T { + &self.0 + } +} + +impl std::ops::DerefMut for Shared { + fn deref_mut(&mut self) -> &mut T { + Arc::make_mut(&mut self.0) + } +} + +impl From for Shared { + fn from(v: T) -> Self { + Shared::new(v) + } +} + +impl PartialEq for Shared { + fn eq(&self, other: &Self) -> bool { + Arc::ptr_eq(&self.0, &other.0) || *self.0 == *other.0 + } +} + +/// A shared value equals the plain value it holds (`node.appearance == style.appearance`). +impl PartialEq for Shared { + fn eq(&self, other: &T) -> bool { + *self.0 == *other + } +} + +impl std::fmt::Debug for Shared { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + +impl Serialize for Shared { + fn serialize(&self, s: S) -> Result { + self.0.serialize(s) + } +} + +impl<'de, T: Deserialize<'de>> Deserialize<'de> for Shared { + fn deserialize>(d: D) -> Result { + T::deserialize(d).map(Shared::new) + } +} diff --git a/crates/doc/src/tests_lookups.rs b/crates/doc/src/tests_lookups.rs new file mode 100644 index 00000000..e42d2468 --- /dev/null +++ b/crates/doc/src/tests_lookups.rs @@ -0,0 +1,156 @@ +//! The batched lookups ([`Document::index_paths`], `node_at`, `node_mut_at`, `ancestry_at`) agree +//! with the one-id ones on nested trees with missing and duplicated ids. +use super::*; +use vectorcraft_geom::shapes; + +/// Layers with paths and groups nested `depth` deep; every 7th id is reused (a damaged file), so +/// first-in-tree-order matters. +fn tree(seed: u64) -> (Document, Vec) { + let mut d = Document::new(612.0, 792.0); + let mut r = seed | 1; + let mut next = |n: u64| { + r ^= r << 13; + r ^= r >> 7; + r ^= r << 17; + r % n + }; + let mut ids = vec![]; + let leaf = |d: &mut Document, ids: &mut Vec| { + let id = if ids.len() % 7 == 6 { ids[ids.len() / 2] } else { d.alloc_id() }; + ids.push(id); + Arc::new(Node::path(id, shapes::rectangle(Rect::new(0.0, 0.0, 5.0, 5.0)), Appearance::default_art())) + }; + for l in 0..3 { + let lid = d.alloc_id(); + let mut layer = Node::layer(lid, &format!("L{l}"), LayerColor::Preset(0)); + ids.push(lid); + for _ in 0..(5 + next(10)) { + let child = if next(3) == 0 { + let mut inner: Vec> = (0..(1 + next(4))).map(|_| leaf(&mut d, &mut ids)).collect(); + for _ in 0..next(3) { + let gid = d.alloc_id(); + ids.push(gid); + inner = vec![Arc::new(Node::group(gid, inner))]; + } + let gid = d.alloc_id(); + ids.push(gid); + Arc::new(Node::group(gid, inner)) + } else { + leaf(&mut d, &mut ids) + }; + if let Some(ch) = layer.children_mut() { + ch.push(child); + } + } + d.layers.push(Arc::new(layer)); + } + ids.push(NodeId(999_999)); // not in the tree + (d, ids) +} + +#[test] +fn batched_lookups_match_one_at_a_time() { + for seed in 1..40 { + let (d, ids) = tree(seed); + let paths = d.index_paths(ids.iter().copied()); + for id in &ids { + assert_eq!(paths.get(id).cloned(), d.index_path(*id), "seed {seed} id {id:?}"); + let p = paths.get(id); + assert_eq!(p.and_then(|p| d.node_at(p)).map(|n| n as *const Node), d.node(*id).map(|n| n as *const Node), "seed {seed} node {id:?}"); + assert_eq!(p.and_then(|p| d.ancestry_at(p)), d.ancestry(*id), "seed {seed} ancestry {id:?}"); + } + // A subset, and nothing. + let some: Vec = ids.iter().copied().step_by(3).collect(); + let sub = d.index_paths(some.iter().copied()); + assert!(some.iter().all(|id| sub.get(id).cloned() == d.index_path(*id))); + assert!(d.index_paths(std::iter::empty()).is_empty()); + // Mutable access reaches the same node as node_mut, copy-on-write. + let mut a = d.clone(); + let mut b = d.clone(); + for id in ids.iter().step_by(5) { + if let Some(n) = a.node_mut(*id) { + n.opacity = 0.25; + } + if let Some(n) = d.index_path(*id).and_then(|p| b.node_mut_at(&p)) { + n.opacity = 0.25; + } + } + assert_eq!(serde_json::to_string(&a).ok(), serde_json::to_string(&b).ok(), "seed {seed}"); + } +} + +/// What a walk of the tree gives for every id, checked against the cached lookups. +fn agree(d: &Document, ids: &[NodeId], what: &str) { + agree_except(d, ids, None, what); +} + +/// [`agree`], except for `dup`, an id given a second node after the cache was built: lookups of it +/// may return either of its nodes (see `id_index`), but always one with that id. +fn agree_except(d: &Document, ids: &[NodeId], dup: Option, what: &str) { + if let Some(dup) = dup { + assert_eq!(d.node(dup).map(|n| n.id), Some(dup), "{what}: node {dup:?} has its id"); + } + let ids: Vec = ids.iter().copied().filter(|id| Some(*id) != dup).collect(); + let ids = &ids[..]; + for id in ids { + let walked = d.walk_index_path(*id); + assert_eq!(d.index_path(*id), walked, "{what}: index_path {id:?}"); + let by_walk = walked.as_deref().and_then(|p| d.node_at(p)).map(|n| n as *const Node); + assert_eq!(d.node(*id).map(|n| n as *const Node), by_walk, "{what}: node {id:?}"); + } + let batched = d.index_paths(ids.iter().copied()); + for id in ids { + assert_eq!(batched.get(id).cloned(), d.walk_index_path(*id), "{what}: index_paths {id:?}"); + } +} + +/// The id cache never answers differently from a walk, however the tree is changed behind its +/// back (through the public fields), including in a clone that shares the cache. +#[test] +fn cached_lookups_follow_any_edit_of_the_tree() { + for seed in 1..30 { + let (mut d, ids) = tree(seed); + agree(&d, &ids, "fresh"); + let snapshot = d.clone(); // shares the cache + // Insert at the front of the first layer: every path in it shifts. + let extra = d.alloc_id(); + let leaf = Arc::new(Node::path(extra, shapes::rectangle(Rect::new(0.0, 0.0, 1.0, 1.0)), Appearance::default_art())); + if let Some(ch) = Arc::make_mut(&mut d.layers[0]).children_mut() { + ch.insert(0, leaf.clone()); + } + let mut all = ids.clone(); + all.push(extra); + agree(&d, &all, "inserted at the front"); + // Remove a child, swap two, and move one into another layer. + if let Some(ch) = Arc::make_mut(&mut d.layers[1]).children_mut() + && ch.len() > 3 + { + ch.remove(1); + ch.swap(0, 2); + } + let moved = Arc::make_mut(&mut d.layers[2]).children_mut().and_then(|ch| (!ch.is_empty()).then(|| ch.remove(0))); + if let Some(m) = moved + && let Some(ch) = Arc::make_mut(&mut d.layers[0]).children_mut() + { + ch.push(m); + } + agree(&d, &all, "removed, swapped and moved"); + // A new duplicate of an existing id, earlier in tree order than the original. + let dup = ids[ids.len() / 3]; + let copy = Arc::new(Node::path(dup, shapes::rectangle(Rect::new(0.0, 0.0, 2.0, 2.0)), Appearance::default_art())); + if let Some(ch) = Arc::make_mut(&mut d.layers[0]).children_mut() { + ch.insert(0, copy); + } + agree_except(&d, &all, Some(dup), "a duplicate inserted earlier"); + // Delete a whole layer, then look everything up again, missing ids included. + d.layers.remove(1); + agree_except(&d, &all, Some(dup), "a layer deleted"); + // A rebuilt cache (as after loading a file) knows the duplicate and walks for it. + let mut fresh = d.clone(); + fresh.layers = d.layers.clone(); + fresh.index = Default::default(); + agree(&fresh, &all, "a damaged tree, cached afresh"); + // The clone made before all this still answers for its own tree. + agree(&snapshot, &ids, "the earlier clone"); + } +} diff --git a/crates/effects/src/bake.rs b/crates/effects/src/bake.rs index 148f2c01..6538b2de 100644 --- a/crates/effects/src/bake.rs +++ b/crates/effects/src/bake.rs @@ -166,7 +166,7 @@ fn piece(d: &mut Document, n: &Node, path: PathData, rule: FillRule, item: Appea Node { id, name: None, - appearance: Appearance { items: vec![item], ..Default::default() }, + appearance: Appearance { items: vec![item], ..Default::default() }.into(), opacity: 1.0, blend: Default::default(), isolate: false, diff --git a/crates/effects/src/reshape.rs b/crates/effects/src/reshape.rs index bf405990..e37a0d20 100644 --- a/crates/effects/src/reshape.rs +++ b/crates/effects/src/reshape.rs @@ -141,7 +141,7 @@ pub(crate) fn as_art(n: &Node, symbol_art: Option<&Node>) -> Option { if let NodeKind::Path { clipping, .. } = &mut clip.kind { *clipping = true; } - let image = Node { appearance: Appearance::default(), ..Node::new(n.id, n.kind.clone()) }; + let image = Node { appearance: Appearance::default().into(), ..Node::new(n.id, n.kind.clone()) }; Node::new(n.id, NodeKind::Group { children: vec![Arc::new(clip), Arc::new(image)], clip: true }) } _ => outline_art(n, symbol_art)?, diff --git a/crates/effects/src/revolve.rs b/crates/effects/src/revolve.rs index 2d5fa3eb..5465c448 100644 --- a/crates/effects/src/revolve.rs +++ b/crates/effects/src/revolve.rs @@ -200,7 +200,7 @@ fn art(n: &Node, expand: bool) -> Option { } let mut art = n.clone(); art.kind = NodeKind::Group { children, clip: false }; - art.appearance = Appearance { effects: remaining, ..Default::default() }; + art.appearance = Appearance { effects: remaining, ..Default::default() }.into(); Some(art) } diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index ff48b838..b4c8ad42 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -443,7 +443,7 @@ fn clear_appearance(s: &mut Session, p: &Value) -> Result { /// Clear Appearance on `n`: no fill, no stroke, and the object's Opacity row back to Default. A /// group keeps no fill or stroke rows of its own. fn clear(n: &mut Node) { - n.appearance = if is_group(n) { Appearance::default() } else { Appearance::basic(Paint::None, Paint::None, 1.0) }; + n.appearance = if is_group(n) { Appearance::default().into() } else { Appearance::basic(Paint::None, Paint::None, 1.0).into() }; n.opacity = 1.0; n.blend = BlendMode::Normal; } @@ -496,7 +496,7 @@ fn reduce_basic(s: &mut Session, p: &Value) -> Result { // keeps the rows it had. let fill = (fill.is_some() || !is_group(n)).then(|| AppearanceItem::Fill(FillLayer::new(fill.map_or(Paint::None, |f| f.paint().clone())))); - n.appearance = Appearance { items: fill.into_iter().collect(), ..Default::default() }; + n.appearance = Appearance { items: fill.into_iter().collect(), ..Default::default() }.into(); if let Some(AppearanceItem::Stroke(mut st)) = stroke { st.effects.clear(); st.opacity = 1.0; @@ -758,7 +758,7 @@ impl Picked { let st = &t.runs[0].style; (st.basic_appearance(), Some(t.local_bounds()), Some((st.clone(), t.para.clone()))) } - _ => (n.appearance.clone(), n.geometric_bounds(), None), + _ => ((*n.appearance).clone(), n.geometric_bounds(), None), }; Self { appearance, bounds, opacity: n.opacity, blend: n.blend, text } } @@ -831,7 +831,7 @@ impl Picked { } let placed = self.placed(n.geometric_bounds()); if ap.fill == FillAttrs::ALL && ap.stroke == StrokeAttrs::ALL { - n.appearance = placed; + n.appearance = placed.into(); } else { focal_attrs(&mut n.appearance, &placed, ap); } diff --git a/crates/engine/src/cmd/buildcmds.rs b/crates/engine/src/cmd/buildcmds.rs index ef3a10fe..107d8996 100644 --- a/crates/engine/src/cmd/buildcmds.rs +++ b/crates/engine/src/cmd/buildcmds.rs @@ -478,7 +478,7 @@ fn lp_merge(s: &mut Session, p: &Value) -> Result { .filter_map(|e| { let pd = e.path_data()?; let sp = pd.subpaths.first().filter(|sp| sp.segment_count() > 0)?; - Some((kurbo::ParamCurve::eval(&sp.segment(0), 0.5), e.appearance.clone())) + Some((kurbo::ParamCurve::eval(&sp.segment(0), 0.5), (*e.appearance).clone())) }) .collect(); let mut sources = source_nodes(&old); @@ -505,7 +505,7 @@ fn lp_merge(s: &mut Session, p: &Value) -> Result { && let Some(ch) = g.children_mut() && let Some(e) = ch.iter_mut().find(|c| c.id == eid) { - Arc::make_mut(e).appearance = app; + Arc::make_mut(e).appearance = app.into(); } } for r in roots.iter().filter(|r| **r != target) { @@ -777,8 +777,9 @@ fn trace_make(s: &mut Session, p: &Value, expand: bool) -> Result { ..StrokeLayer::new(paint, width * unit) })], ..Default::default() - }, - None => fill_only(paint), + } + .into(), + None => fill_only(paint).into(), }; children.push(Arc::new(n)); } diff --git a/crates/engine/src/cmd/create.rs b/crates/engine/src/cmd/create.rs index 5b4dd8c1..84370f57 100644 --- a/crates/engine/src/cmd/create.rs +++ b/crates/engine/src/cmd/create.rs @@ -97,7 +97,7 @@ pub(crate) fn add_look(s: &mut Session, label: &str, kind: NodeKind, look: NewAr let (par, idx, _) = d.position(target).ok_or(crate::EngineError::NoNode(target))?; let mut mask = d.node(target).cloned().ok_or(crate::EngineError::NoNode(target))?; mask.id = d.alloc_id(); - mask.appearance = Appearance::basic(vectorcraft_color::Paint::None, vectorcraft_color::Paint::None, 0.0); + mask.appearance = Appearance::basic(vectorcraft_color::Paint::None, vectorcraft_color::Paint::None, 0.0).into(); if let NodeKind::Path { clipping, .. } = &mut mask.kind { *clipping = true; } diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index e5e94c10..c6201b7b 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -251,22 +251,29 @@ pub(crate) fn selected_roots(s: &Session) -> Result> { /// The top-level objects among `ids` (in paint order): compound-path members stand for their /// compound, and ids whose ancestor is also listed, and layers, are dropped. pub(crate) fn roots_of(doc: &Document, ids: Vec) -> Vec { + // Every id's place in the tree, found in one walk (a lookup per id is a walk per id). + let paths = doc.index_paths(ids.iter().copied()); // A compound-path member acts as its compound (compounds are one object). - let mut sel: Vec = ids + let mut sel: Vec<(NodeId, Option>)> = ids .into_iter() - .map(|id| match doc.parent_of(id).and_then(|p| doc.node(p).map(|n| (p, n))) { - Some((p, n)) if matches!(n.kind, vectorcraft_doc::NodeKind::Compound { .. }) => p, - _ => id, + .map(|id| { + let path = paths.get(&id); + let parent = path.and_then(|p| p.split_last()).map(|(_, up)| up).filter(|up| !up.is_empty()); + match parent.and_then(|up| doc.node_at(up).map(|n| (up, n))) { + Some((up, n)) if matches!(n.kind, vectorcraft_doc::NodeKind::Compound { .. }) => (n.id, Some(up.to_vec())), + _ => (id, path.cloned()), + } }) .collect(); - sel.dedup(); + sel.dedup_by_key(|(id, _)| *id); + let listed: std::collections::HashSet = sel.iter().map(|(id, _)| *id).collect(); sel.iter() - .copied() - .filter(|id| { - let anc = doc.ancestry(*id).unwrap_or_default(); - !anc[..anc.len().saturating_sub(1)].iter().any(|a| sel.contains(a)) + .filter(|(_, path)| { + let anc = path.as_deref().and_then(|p| doc.ancestry_at(p)).unwrap_or_default(); + !anc[..anc.len().saturating_sub(1)].iter().any(|a| listed.contains(a)) }) - .filter(|id| doc.node(*id).is_some_and(|n| !n.is_layer())) + .filter(|(_, path)| path.as_deref().and_then(|p| doc.node_at(p)).is_some_and(|n| !n.is_layer())) + .map(|(id, _)| *id) .collect() } diff --git a/crates/engine/src/cmd/fileio/affinity.rs b/crates/engine/src/cmd/fileio/affinity.rs index 92c8b3a4..70dff78f 100644 --- a/crates/engine/src/cmd/fileio/affinity.rs +++ b/crates/engine/src/cmd/fileio/affinity.rs @@ -236,13 +236,13 @@ impl Builder { let appearance = self.appearance(fills, strokes, to_doc); if children.is_empty() { let mut p = self.base(n, NodeKind::Path { path: data, rule, live: None, clipping: false, guide: false }); - p.appearance = appearance; + p.appearance = appearance.into(); p } else { // A shape clips its children: VectorCraft's clipping path keeps its paint, its // fill behind the clipped art and its stroke over it, as Affinity draws it. let mut clip = Node::new(self.doc.alloc_id(), NodeKind::Path { path: data, rule, live: None, clipping: true, guide: false }); - clip.appearance = appearance; + clip.appearance = appearance.into(); let mut kids = vec![Arc::new(clip)]; kids.extend(children); self.base(n, NodeKind::Group { children: kids, clip: true }) @@ -259,7 +259,7 @@ impl Builder { guide: false, }, ); - clip.appearance = self.appearance(background, strokes, to_doc); + clip.appearance = self.appearance(background, strokes, to_doc).into(); let mut kids = vec![Arc::new(clip)]; kids.extend(children); let mut g = self.base(n, NodeKind::Group { children: kids, clip: true }); diff --git a/crates/engine/src/cmd/flatten.rs b/crates/engine/src/cmd/flatten.rs index ad010c13..8d11365b 100644 --- a/crates/engine/src/cmd/flatten.rs +++ b/crates/engine/src/cmd/flatten.rs @@ -655,7 +655,7 @@ impl FlattenReport { art.iter_mut().for_each(opaque); for path in areas { let mut n = shape_node(&mut tmp, path.clone(), None); - n.appearance = Appearance::basic(Paint::solid(Color::rgb(0.0, 0.0, 0.0)), Paint::None, 0.0); + n.appearance = Appearance::basic(Paint::solid(Color::rgb(0.0, 0.0, 0.0)), Paint::None, 0.0).into(); art.push(n); } Some(isolated_doc(&tmp, art)) @@ -1054,7 +1054,8 @@ fn apply(d: &mut Document, sel: &mut vectorcraft_doc::Selection, plan: Plan, o: n.appearance = Appearance { items: vec![AppearanceItem::Fill(FillLayer { overprint: fill.overprint, ..FillLayer::new(fill.paint) })], ..Appearance::default() - }; + } + .into(); n.opacity = fill.alpha.min(1.0); children.push(Arc::new(n)); vector += 1; diff --git a/crates/engine/src/cmd/menucmds.rs b/crates/engine/src/cmd/menucmds.rs index 84082d02..7e013da8 100644 --- a/crates/engine/src/cmd/menucmds.rs +++ b/crates/engine/src/cmd/menucmds.rs @@ -575,7 +575,7 @@ fn crop_image(s: &mut Session, p: &Value) -> Result { let mut bare = node.clone(); bare.opacity = 1.0; bare.blend = Default::default(); - bare.appearance = Appearance::default(); + bare.appearance = Appearance::default().into(); bare.visible = true; let tmp = isolated_doc(&st.doc, vec![bare]); let region = Rect::new(r.x0, r.y0, r.x0 + (r.width() * scale).round().max(1.0) / scale, r.y0 + (r.height() * scale).round().max(1.0) / scale); diff --git a/crates/engine/src/cmd/newart.rs b/crates/engine/src/cmd/newart.rs index 1d7607a2..429ac840 100644 --- a/crates/engine/src/cmd/newart.rs +++ b/crates/engine/src/cmd/newart.rs @@ -94,7 +94,7 @@ impl NewArt { /// Give new object `n` (in `d`) this look: its placed gradients land relative to its bounds, and /// it links to the style (type keeps its own transparency). pub(crate) fn apply(&self, d: &mut vectorcraft_doc::Document, n: &mut Node) { - n.appearance = self.appearance.clone(); + n.appearance = self.appearance.clone().into(); if self.unit_box && n.appearance.has_placed_gradient() && let Some(b) = n.geometric_bounds() diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index ba3e580b..385d394e 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -242,7 +242,8 @@ pub(crate) fn transform_patterns(s: &mut Session, p: &Value, ids: &[NodeId]) -> let on = bool_or(p, "patterns", s.prefs.transform_pattern_tiles); let d = &s.doc()?.doc; let mut uses = false; - for n in ids.iter().filter_map(|id| d.node(*id)) { + let paths = d.index_paths(ids.iter().copied()); + for n in ids.iter().filter_map(|id| paths.get(id)).filter_map(|p| d.node_at(p)) { n.walk(&mut |c| uses |= vectorcraft_doc::pattern::uses_pattern(c, None)); } if uses { @@ -263,14 +264,16 @@ pub(crate) fn apply_transform(s: &mut Session, label: &str, ids: Vec, xf // nothing: the document and its undo history stay as they are, and Transform Again repeats it. // With a copy, the reply lists the copies, and there are none. let d = &s.doc()?.doc; - let none = ids.iter().all(|id| d.node(*id).is_none()); + let none = d.index_paths(ids.iter().copied()).is_empty(); let ids = if none || (!copy && xf == Affine::IDENTITY) { if copy { vec![] } else { ids } } else { s.edit(label, |d, sel| { let targets = if copy { duplicate_in(d, sel, &ids, Affine::IDENTITY)? } else { ids.clone() }; + // Transforms don't move nodes in the tree, so the paths found up front stay right. + let paths = d.index_paths(targets.iter().copied()); for id in &targets { - if let Some(n) = d.node_mut(*id) + if let Some(n) = paths.get(id).and_then(|p| d.node_mut_at(p)) && !(areas && resize_type_area(n, xf)) { n.transform(xf, sc); @@ -658,7 +661,7 @@ fn compound_make(s: &mut Session, _: &Value) -> Result { let back = paths.first().map_or(0.0, |p| direction(p)); for (i, mut p) in paths.into_iter().enumerate() { let pn = Arc::make_mut(&mut p); - pn.appearance = Appearance::default(); + pn.appearance = Appearance::default().into(); let flip = i > 0 && back != 0.0 && direction(pn) == back; if let NodeKind::Path { path, live, .. } = &mut pn.kind { *live = None; @@ -739,7 +742,7 @@ pub(super) fn as_clipping_path(c: &mut Node) -> Result<()> { if !matches!(c.kind, NodeKind::Path { guide: false, .. } | NodeKind::Compound { .. } | NodeKind::Text(_)) { return Err(EngineError::Other("the top object must be a path, compound path or text object to use as a clipping mask".into())); } - c.appearance = Appearance::basic(Paint::None, Paint::None, 0.0); + c.appearance = Appearance::basic(Paint::None, Paint::None, 0.0).into(); match &mut c.kind { NodeKind::Path { clipping, .. } => *clipping = true, NodeKind::Text(t) => { diff --git a/crates/engine/src/cmd/paint.rs b/crates/engine/src/cmd/paint.rs index 2bc62004..fc9424e1 100644 --- a/crates/engine/src/cmd/paint.rs +++ b/crates/engine/src/cmd/paint.rs @@ -524,7 +524,7 @@ fn default_paint(s: &mut Session, p: &Value) -> Result { r.style.stroke = Paint::None; } } - _ => n.appearance = Appearance::default_art(), + _ => n.appearance = Appearance::default_art().into(), } } Ok(()) diff --git a/crates/engine/src/cmd/pathops.rs b/crates/engine/src/cmd/pathops.rs index adf6afe7..89ab4b82 100644 --- a/crates/engine/src/cmd/pathops.rs +++ b/crates/engine/src/cmd/pathops.rs @@ -203,10 +203,10 @@ struct Style { impl Style { fn of(n: &Node) -> Self { - Style { appearance: n.appearance.clone(), opacity: n.opacity, blend: n.blend } + Style { appearance: (*n.appearance).clone(), opacity: n.opacity, blend: n.blend } } fn apply(&self, n: &mut Node) { - n.appearance = self.appearance.clone(); + n.appearance = self.appearance.clone().into(); n.opacity = self.opacity; n.blend = self.blend; } @@ -377,7 +377,7 @@ fn run_pf(s: &mut Session, op: PathfinderOp) -> Result { // Outline: open edges stroked with the face's former fill. let fill = st.appearance.fill_paint(); let paint = if fill.is_none() { st.appearance.stroke_paint() } else { fill }; - n.appearance = Appearance { items: vec![AppearanceItem::Stroke(StrokeLayer::new(paint, 1.0))], ..Default::default() }; + n.appearance = Appearance { items: vec![AppearanceItem::Stroke(StrokeLayer::new(paint, 1.0))], ..Default::default() }.into(); } children.push(Arc::new(n)); } @@ -519,7 +519,7 @@ pub(crate) fn outlined_stroke( overprint: st.overprint, ..vectorcraft_doc::appearance::FillLayer::new(st.paint.clone()) }; - n.appearance = Appearance { items: vec![AppearanceItem::Fill(fill)], ..Default::default() }; + n.appearance = Appearance { items: vec![AppearanceItem::Fill(fill)], ..Default::default() }.into(); Some(n) } @@ -554,7 +554,7 @@ pub(crate) fn outline_strokes(d: &mut Document, brushes: &[vectorcraft_brush::Br /// A plain copy of path `l` (new ids, no transparency, name or mask) painted with `items`. pub(crate) fn styled_copy(d: &mut Document, l: &Node, items: Vec) -> Node { let mut f = d.reid(l); - f.appearance = Appearance { items, ..Default::default() }; + f.appearance = Appearance { items, ..Default::default() }.into(); f.opacity = 1.0; f.blend = BlendMode::Normal; f.name = None; diff --git a/crates/engine/src/cmd/rasterfx.rs b/crates/engine/src/cmd/rasterfx.rs index 3339502b..013ed127 100644 --- a/crates/engine/src/cmd/rasterfx.rs +++ b/crates/engine/src/cmd/rasterfx.rs @@ -136,7 +136,7 @@ fn settings(s: &mut Session, p: &Value) -> Result { /// `art` in a clip group clipped by `path` filled by `rule` (a clipping path that paints nothing). pub(crate) fn clip_group(d: &mut Document, path: PathData, rule: FillRule, art: Node) -> Node { let mut clip = super::pathops::shape_node(d, path, None); - clip.appearance = Appearance::basic(Paint::None, Paint::None, 0.0); + clip.appearance = Appearance::basic(Paint::None, Paint::None, 0.0).into(); match &mut clip.kind { NodeKind::Path { clipping, rule: r, .. } => (*clipping, *r) = (true, rule), NodeKind::Compound { rule: r, .. } => *r = rule, @@ -363,7 +363,7 @@ fn split_items(out: &mut Document, n: &Node) -> Option { } let mut stroke_piece = |d: &mut Document, path: &PathData, rule: FillRule, st: &StrokeLayer| { let mut p = super::pathops::shape_node(d, path.clone(), None); - p.appearance = Appearance { items: vec![AppearanceItem::Stroke(st.clone())], ..Default::default() }; + p.appearance = Appearance { items: vec![AppearanceItem::Stroke(st.clone())], ..Default::default() }.into(); if let NodeKind::Path { rule: r, .. } | NodeKind::Compound { rule: r, .. } = &mut p.kind { *r = rule; } diff --git a/crates/engine/src/cmd/shaper.rs b/crates/engine/src/cmd/shaper.rs index b0966ccf..b7863217 100644 --- a/crates/engine/src/cmd/shaper.rs +++ b/crates/engine/src/cmd/shaper.rs @@ -334,7 +334,7 @@ fn generated(d: &mut Document, src: &[Arc], spec: &ShaperSpec, map: &Build let edit = edits.get(&i); let Some(source) = edit.and_then(|e| e.paint_source).or_else(|| fill_source(src, region)).and_then(|i| src.get(i)) else { continue }; let path = po::unite_all(&indices.iter().filter_map(|i| map.regions.get(*i).map(|r| (&r.0.path, FillRule::NonZero))).collect::>()); - let style = edit.and_then(|e| e.paint.clone()).unwrap_or_else(|| source.appearance.clone()); + let style = edit.and_then(|e| e.paint.clone()).unwrap_or_else(|| (*source.appearance).clone()); let mut n = Node::path(d.alloc_id(), path, fill_style(style)); n.opacity = source.opacity; n.blend = source.blend; @@ -358,7 +358,7 @@ fn generated(d: &mut Document, src: &[Arc], spec: &ShaperSpec, map: &Build continue; } let Some(source) = src.get(piece.key as usize) else { continue }; - let style = edge_edits.get(&i).and_then(|e| e.paint.clone()).unwrap_or_else(|| source.appearance.clone()); + let style = edge_edits.get(&i).and_then(|e| e.paint.clone()).unwrap_or_else(|| (*source.appearance).clone()); if style.stroke_paint().is_none() { continue; } @@ -448,7 +448,7 @@ pub(crate) fn refresh(before: &Document, d: &mut Document) -> Result<()> { for i in related { let Some(selector) = face_selector(&map, &src, i) else { continue }; if let Some(e) = spec.faces.iter_mut().rev().find(|e| e.selector == selector) { - e.paint = Some(n.appearance.clone()); + e.paint = Some((*n.appearance).clone()); } else { spec.faces.push(FaceEdit { selector, @@ -456,7 +456,7 @@ pub(crate) fn refresh(before: &Document, d: &mut Document) -> Result<()> { erase_edges: false, paint_source: None, merge, - paint: Some(n.appearance.clone()), + paint: Some((*n.appearance).clone()), }); } } @@ -472,7 +472,7 @@ pub(crate) fn refresh(before: &Document, d: &mut Document) -> Result<()> { && piece_point(&piece.path).is_some_and(|point| n.path_data().and_then(|p| p.nearest(point)).is_some_and(|p| p.4 < 1e-5)); if contained && let Some(mut e) = edge_selector(&map, &src, piece) { e.erase = false; - e.paint = Some(n.appearance.clone()); + e.paint = Some((*n.appearance).clone()); spec.edges.push(e); } } diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index 275a5364..c6f720fc 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -217,7 +217,7 @@ fn unitized(mut ap: Appearance, from: Option) -> Appearance { /// and stroke. Placed gradients come relative to the unit box (from the bounds of the object /// they're on). The pen pressure of strokes stays with their own path. pub(crate) fn captured(n: &Node) -> Appearance { - let mut ap = n.appearance.clone().without_pressure(); + let mut ap = (*n.appearance).clone().without_pressure(); if !ap.items.is_empty() { return unitized(ap, n.geometric_bounds()); } @@ -261,7 +261,7 @@ fn override_chars(n: &mut Node, ap: &Appearance) { /// Give `n` style `g`: its appearance (a group or layer too: its own, above its contents; type its /// characters' colour too with `chars`, Override Character Color) and transparency. fn style_node(n: &mut Node, g: &GraphicStyle, chars: bool) { - n.appearance = g.appearance_on(n).into_owned(); + n.appearance = g.appearance_on(n).into_owned().into(); if chars { override_chars(n, &g.appearance); } diff --git a/crates/engine/src/cmd/textedit.rs b/crates/engine/src/cmd/textedit.rs index 364693d4..d830d824 100644 --- a/crates/engine/src/cmd/textedit.rs +++ b/crates/engine/src/cmd/textedit.rs @@ -561,7 +561,7 @@ fn create_in_path(s: &mut Session, p: &Value) -> Result { d.remove(pid)?; let id = d.alloc_id(); let mut n = Node::new(id, NodeKind::Text(Box::new(t))); - n.appearance = Appearance::default(); + n.appearance = Appearance::default().into(); n.opacity = node.opacity; n.name = node.name.clone(); d.insert(par, idx, n)?; diff --git a/crates/engine/src/cmd/typecmd.rs b/crates/engine/src/cmd/typecmd.rs index 5230c5c5..99c960c9 100644 --- a/crates/engine/src/cmd/typecmd.rs +++ b/crates/engine/src/cmd/typecmd.rs @@ -248,14 +248,14 @@ fn create_outlines(s: &mut Session, _: &Value) -> Result { } let st = t.runs.get(g.run).map(|r| r.style.clone()).unwrap_or_else(|| t.first_style()); let mut node = shape_node(d, path, None); - node.appearance = st.appearance(); + node.appearance = st.appearance().into(); children.push(Arc::new(node)); } // Underline and strikethrough bars become paths of their run's paint too (#847). for (run, bar) in vectorcraft_text::decorations(&lay, vectorcraft_text::FontDb::global(), t) { let st = t.runs.get(run).map(|r| r.style.clone()).unwrap_or_else(|| t.first_style()); let mut node = shape_node(d, PathData::from_bezpath(&bar).transformed(t.xf), None); - node.appearance = st.appearance(); + node.appearance = st.appearance().into(); children.push(Arc::new(node)); } let (par, idx, _) = d.position(tid).ok_or(EngineError::NoNode(tid))?; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index f3041d6c..fc819f78 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -421,9 +421,10 @@ fn strip_replay_only(id: &str, p: &mut Value) { /// have finite, in-range geometry, so saved files always reload and renderers never see NaN/∞. fn doc_sane(d: &Document, sel: &Selection) -> bool { let ok = |r: vectorcraft_geom::Rect| [r.x0, r.y0, r.x1, r.y1].iter().all(|v| v.is_finite() && v.abs() <= MAX_COORD); - d.artboards.iter().all(|a| ok(a.rect)) - && d.guides.iter().all(|g| g.pos.is_finite() && g.pos.abs() <= MAX_COORD) - && sel.objects.iter().all(|id| d.node(*id).and_then(|n| n.geometric_bounds()).is_none_or(ok)) + d.artboards.iter().all(|a| ok(a.rect)) && d.guides.iter().all(|g| g.pos.is_finite() && g.pos.abs() <= MAX_COORD) && { + let paths = d.index_paths(sel.objects.iter().copied()); + sel.objects.iter().all(|id| paths.get(id).and_then(|p| d.node_at(p)).and_then(|n| n.geometric_bounds()).is_none_or(ok)) + } } /// Where new art goes (Illustrator's drawing modes, Shift+D cycles). diff --git a/crates/engine/src/tests_containers.rs b/crates/engine/src/tests_containers.rs index dc11f0c0..211bca2e 100644 --- a/crates/engine/src/tests_containers.rs +++ b/crates/engine/src/tests_containers.rs @@ -103,7 +103,7 @@ fn effects_and_fills_apply_to_layers_by_id() { // Two undo steps take both off again. run(&mut s, "edit.undo", json!({})); run(&mut s, "edit.undo", json!({})); - assert_eq!(node(&s, layer).appearance, Default::default()); + assert_eq!(node(&s, layer).appearance, vectorcraft_doc::Appearance::default()); // Nothing selected and no ids: an error, not a silent no-op. assert!(s.execute("appearance.addFill", &json!({})).is_err()); } diff --git a/crates/engine/src/tests_envelope_distort.rs b/crates/engine/src/tests_envelope_distort.rs index 246b13ef..300a6ec5 100644 --- a/crates/engine/src/tests_envelope_distort.rs +++ b/crates/engine/src/tests_envelope_distort.rs @@ -34,7 +34,7 @@ fn sel(s: &mut Session, ids: &[NodeId]) { fn rect(s: &mut Session) -> NodeId { let a = id_of(&s.execute("shape.rectangle", &json!({"x": 100, "y": 100, "width": 200, "height": 100})).unwrap()); s.edit("red", |d, _| { - d.node_mut(a).unwrap().appearance = vectorcraft_doc::Appearance::basic(Paint::solid(Color::rgb(1.0, 0.0, 0.0)), Paint::None, 0.0); + d.node_mut(a).unwrap().appearance = vectorcraft_doc::Appearance::basic(Paint::solid(Color::rgb(1.0, 0.0, 0.0)), Paint::None, 0.0).into(); Ok(()) }) .unwrap(); diff --git a/crates/engine/src/tests_eyedropper.rs b/crates/engine/src/tests_eyedropper.rs index 9dd0a9bd..d3f83a01 100644 --- a/crates/engine/src/tests_eyedropper.rs +++ b/crates/engine/src/tests_eyedropper.rs @@ -126,7 +126,7 @@ fn shift_alt_click_appends_the_appearance() { let mut s = session(); let (_, dst) = source_and_target(&mut s); click(&mut s, 325.0, 35.0, Mods { alt: true, shift: true, ..Default::default() }); - let items = node(&s, dst).appearance.items; + let items = node(&s, dst).appearance.items.clone(); assert_eq!(items.len(), 4, "the source's fill and stroke on top of the target's"); assert_eq!((items[0].paint().color().unwrap().to_hex(), items[2].paint().color().unwrap().to_hex()), ("#ffffff".into(), "#ff0000".into())); } diff --git a/crates/engine/src/tests_pathops.rs b/crates/engine/src/tests_pathops.rs index 8a2d9396..91b991fb 100644 --- a/crates/engine/src/tests_pathops.rs +++ b/crates/engine/src/tests_pathops.rs @@ -229,7 +229,7 @@ fn outline_stroke_of_line() { let r = s.execute("shape.line", &json!({"x1": 0, "y1": 50, "x2": 100, "y2": 50})).unwrap(); let l = NodeId(r["id"].as_u64().unwrap()); s.edit("t", |d, _| { - d.node_mut(l).unwrap().appearance = Appearance::basic(Paint::None, Paint::solid(Color::rgb(1.0, 0.0, 0.0)), 10.0); + d.node_mut(l).unwrap().appearance = Appearance::basic(Paint::None, Paint::solid(Color::rgb(1.0, 0.0, 0.0)), 10.0).into(); Ok(()) }) .unwrap(); @@ -304,7 +304,7 @@ fn clean_up_removes_junk() { let keep = rect(&mut s, 0.0, 0.0, 10.0, 10.0); let unpainted = rect(&mut s, 20.0, 0.0, 10.0, 10.0); s.edit("t", |d, _| { - d.node_mut(unpainted).unwrap().appearance = Appearance::basic(Paint::None, Paint::None, 1.0); + d.node_mut(unpainted).unwrap().appearance = Appearance::basic(Paint::None, Paint::None, 1.0).into(); Ok(()) }) .unwrap(); diff --git a/crates/eps/src/import/ai/read.rs b/crates/eps/src/import/ai/read.rs index 4ef288a8..2a3204aa 100644 --- a/crates/eps/src/import/ai/read.rs +++ b/crates/eps/src/import/ai/read.rs @@ -1330,7 +1330,7 @@ impl<'a> Reader<'a> { } let kind = NodeKind::Path { path, rule: gs.rule, live: None, clipping: clip, guide }; let mut node = self.new_node(kind, &gs); - node.appearance = Appearance { items, ..Appearance::default() }; + node.appearance = Appearance { items, ..Appearance::default() }.into(); node.opacity = gs.opacity; node.blend = gs.blend; node.knockout_shape = gs.knockout_shape; diff --git a/crates/eps/src/import/graphics.rs b/crates/eps/src/import/graphics.rs index 1bbf8337..10c462f6 100644 --- a/crates/eps/src/import/graphics.rs +++ b/crates/eps/src/import/graphics.rs @@ -1688,7 +1688,7 @@ pub(crate) fn collapse(nodes: &mut [Arc], shadings: &[NodeId]) { && shadings.contains(&fill.id) && let (NodeKind::Path { path, rule, clipping: true, .. }, NodeKind::Path { .. }) = (&clip.kind, &fill.kind) { - let mut p = Node::path(fill.id, path.clone(), fill.appearance.clone()); + let mut p = Node::path(fill.id, path.clone(), (*fill.appearance).clone()); if let NodeKind::Path { rule: r, .. } = &mut p.kind { *r = *rule; } diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 42ea6f3d..48e82f7c 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -145,6 +145,19 @@ fn preview_mime(bytes: &[u8]) -> &'static str { /// The JSON of a file and the data after it (v4), split at [`BLOB_MAGIC`] after the first JSON /// value. fn split_tail(text: &[u8]) -> (&[u8], &[u8]) { + // JSON holds no raw NUL, so the marker (which has one) starts at the first NUL, if the file + // has one. The writer puts it right after the JSON's last byte; found there, it is where the + // first JSON value ends, without parsing the JSON twice. Anything else takes the parse. + let Some(nul) = text.iter().position(|&b| b == 0) else { + // No NUL, so no marker: the parse below would find none either. + return (text, &[]); + }; + if let Some(at) = nul.checked_sub(1) + && text.get(at..).is_some_and(|r| r.starts_with(BLOB_MAGIC)) + && at.checked_sub(1).and_then(|k| text.get(k)).is_some_and(|b| matches!(b, b'}' | b']' | b'"')) + { + return (text.get(..at).unwrap_or(text), text.get(at + BLOB_MAGIC.len()..).unwrap_or_default()); + } let mut values = serde_json::Deserializer::from_slice(text).into_iter::(); if values.next().is_some_and(|v| v.is_ok()) { let end = values.byte_offset(); diff --git a/crates/pathops/src/boolean.rs b/crates/pathops/src/boolean.rs index d3143c8c..79f2e532 100644 --- a/crates/pathops/src/boolean.rs +++ b/crates/pathops/src/boolean.rs @@ -50,24 +50,22 @@ pub(crate) fn inside(rule: FillRule, w: i32) -> bool { } } -/// Winding numbers for an arbitrary number of input sets (tag = input index). -#[derive(Clone, Debug, Default)] -pub(crate) struct Multi(pub(crate) Vec); +/// Winding numbers for an arbitrary number of input sets (tag = input index): the non-zero ones, +/// as (tag, winding) sorted by tag. A face is covered by a few of the inputs, so this stays short +/// where a vector of every input's winding would make each Pathfinder test cost one step per input. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub(crate) struct Multi(Vec<(usize, i32)>); impl Multi { fn get(&self, i: usize) -> i32 { - self.0.get(i).copied().unwrap_or(0) + self.0.binary_search_by_key(&i, |e| e.0).ok().and_then(|k| self.0.get(k)).map_or(0, |e| e.1) + } + /// The non-zero windings, by input. + pub(crate) fn iter(&self) -> impl DoubleEndedIterator + '_ { + self.0.iter().copied() } } -impl PartialEq for Multi { - fn eq(&self, other: &Self) -> bool { - let n = self.0.len().max(other.0.len()); - (0..n).all(|i| self.get(i) == other.get(i)) - } -} -impl Eq for Multi {} - impl std::ops::Add for Multi { type Output = Multi; fn add(mut self, rhs: Self) -> Multi { @@ -78,11 +76,21 @@ impl std::ops::Add for Multi { impl std::ops::AddAssign for Multi { fn add_assign(&mut self, rhs: Self) { - if self.0.len() < rhs.0.len() { - self.0.resize(rhs.0.len(), 0); - } - for (a, b) in self.0.iter_mut().zip(rhs.0) { - *a += b; + // In place, input by input (the sweep mostly adds one input's ±1 to a short list): add to + // the input's winding, or insert it in tag order, and drop it if it reaches zero. + for (t, k) in rhs.0 { + match self.0.binary_search_by_key(&t, |e| e.0) { + Ok(i) => { + if let Some(e) = self.0.get_mut(i) { + e.1 += k; + if e.1 == 0 { + self.0.remove(i); + } + } + } + Err(i) if k != 0 => self.0.insert(i, (t, k)), + Err(_) => {} + } } } } @@ -90,14 +98,11 @@ impl std::ops::AddAssign for Multi { impl WindingNumber for Multi { type Tag = usize; fn single(tag: usize, positive: bool) -> Self { - let mut v = vec![0; tag + 1]; - v[tag] = if positive { 1 } else { -1 }; - Multi(v) + Multi(vec![(tag, if positive { 1 } else { -1 })]) } fn of_tag(&self, tag: usize) -> Self { - let mut v = vec![0; tag + 1]; - v[tag] = self.get(tag); - Multi(v) + let w = self.get(tag); + Multi(if w == 0 { vec![] } else { vec![(tag, w)] }) } } @@ -288,7 +293,24 @@ impl Arrangement { } pub(crate) fn mask(&self, w: &Multi) -> Vec { - self.rules.iter().enumerate().map(|(i, &r)| inside(r, w.get(i))).collect() + // A zero winding is outside under both rules, so only the non-zero ones can set a flag. + let mut m = vec![false; self.rules.len()]; + for (i, k) in w.iter() { + if let (Some(&r), Some(b)) = (self.rules.get(i), m.get_mut(i)) { + *b = inside(r, k); + } + } + m + } + + /// The inputs that cover a face of winding `w` (`mask(w)`'s true indices), back to front. + pub(crate) fn covered<'a>(&'a self, w: &'a Multi) -> impl DoubleEndedIterator + 'a { + w.iter().filter(|&(i, k)| self.rules.get(i).is_some_and(|&r| inside(r, k))).map(|(i, _)| i) + } + + /// [`Arrangement::contours`] with the test on the windings themselves (see [`Self::covered`]). + pub(crate) fn contours_where(&self, pred: impl Fn(&Multi) -> bool) -> Contours { + self.top.contours(pred) } pub(crate) fn contours(&self, pred: impl Fn(&[bool]) -> bool) -> Contours { diff --git a/crates/pathops/src/pathfinder.rs b/crates/pathops/src/pathfinder.rs index 123fb2c5..c9861266 100644 --- a/crates/pathops/src/pathfinder.rs +++ b/crates/pathops/src/pathfinder.rs @@ -75,8 +75,10 @@ fn arrangement(shapes: &[Shape]) -> Option { Arrangement::new(bps).ok() } -fn topmost(m: &[bool]) -> Option { - m.iter().rposition(|&b| b) +/// The one item of `it`, if it has exactly one. +fn only(mut it: impl Iterator) -> Option { + let first = it.next()?; + it.next().is_none().then_some(first) } fn one(path: PathData, key: u64) -> Vec { @@ -102,10 +104,16 @@ pub fn pathfinder(op: PathfinderOp, shapes: &[Shape]) -> Vec { let p = &arr.tidy; match op { PathfinderOp::Unite => unite(), - PathfinderOp::MinusFront => one(all_contours_to_path(&arr.contours(|m| m[0] && !m[1..].iter().any(|&b| b)), p), shapes[0].key), - PathfinderOp::MinusBack => one(all_contours_to_path(&arr.contours(|m| m[n - 1] && !m[..n - 1].iter().any(|&b| b)), p), front_key), - PathfinderOp::Intersect => one(all_contours_to_path(&arr.contours(|m| m.iter().all(|&b| b)), p), front_key), - PathfinderOp::Exclude => one(all_contours_to_path(&arr.contours(|m| m.iter().filter(|&&b| b).count() % 2 == 1), p), front_key), + // The tests read the covering inputs straight off the windings ([`Arrangement::covered`]); + // each is the coverage-mask test in its comment. + // m[0] && !m[1..].any() + PathfinderOp::MinusFront => one(all_contours_to_path(&arr.contours_where(|w| only(arr.covered(w)) == Some(0)), p), shapes[0].key), + // m[n - 1] && !m[..n - 1].any() + PathfinderOp::MinusBack => one(all_contours_to_path(&arr.contours_where(|w| only(arr.covered(w)) == Some(n - 1)), p), front_key), + // m.all() + PathfinderOp::Intersect => one(all_contours_to_path(&arr.contours_where(|w| arr.covered(w).count() == n), p), front_key), + // m.count() is odd + PathfinderOp::Exclude => one(all_contours_to_path(&arr.contours_where(|w| arr.covered(w).count() % 2 == 1), p), front_key), PathfinderOp::Divide => regions_of(&arr) .into_iter() .filter_map(|r| { @@ -113,7 +121,10 @@ pub fn pathfinder(op: PathfinderOp, shapes: &[Shape]) -> Vec { Some(Shape::new(r.path, FillRule::NonZero, key)) }) .collect(), - PathfinderOp::Trim => (0..n).flat_map(|i| one(all_contours_to_path(&arr.contours(|m| topmost(m) == Some(i)), p), shapes[i].key)).collect(), + // topmost(m) == Some(i) + PathfinderOp::Trim => { + (0..n).flat_map(|i| one(all_contours_to_path(&arr.contours_where(|w| arr.covered(w).next_back() == Some(i)), p), shapes[i].key)).collect() + } PathfinderOp::Merge => { let mut keys: Vec = Vec::new(); for s in shapes { @@ -123,7 +134,8 @@ pub fn pathfinder(op: PathfinderOp, shapes: &[Shape]) -> Vec { } let mut out = Vec::new(); for k in keys { - let c = arr.contours(|m| topmost(m).is_some_and(|t| shapes[t].key == k)); + // topmost(m) is a shape with key k + let c = arr.contours_where(|w| arr.covered(w).next_back().is_some_and(|t| shapes[t].key == k)); for g in c.grouped() { out.extend(one(contours_to_path(&c, g, p), k)); } @@ -136,7 +148,11 @@ pub fn pathfinder(op: PathfinderOp, shapes: &[Shape]) -> Vec { } (0..n - 1) .flat_map(|i| { - let c = arr.contours(|m| m[n - 1] && topmost(&m[..n - 1]) == Some(i)); + // m[n - 1] && topmost(&m[..n - 1]) == Some(i): the front shape, then i below it + let c = arr.contours_where(|w| { + let mut c = arr.covered(w).rev(); + c.next() == Some(n - 1) && c.next() == Some(i) + }); one(all_contours_to_path(&c, p), shapes[i].key) }) .collect() @@ -342,9 +358,11 @@ impl FaceMerger { /// Pathfinder Outline: split every shape's boundary at junctions of the arrangement. fn outline_edges(arr: &Arrangement, shapes: &[Shape]) -> Vec { + // Sorted by x (then y), so a segment needs only the junctions inside its control box. let junctions = arr.junctions(); let scale = shapes.iter().filter_map(|s| s.path.control_bounds()).map(|r| r.width().max(r.height())).fold(1.0, f64::max); let tol = scale * 1e-7; + let reach = tol.max(1e-6); // (chain, key, source index) let mut chains: Vec<(Vec, u64)> = Vec::new(); for s in shapes { @@ -359,11 +377,19 @@ fn outline_edges(arr: &Arrangement, shapes: &[Shape]) -> Vec { for i in 0..sp.segment_count() { let c = sp.segment(i); let line = sp.segment_is_line(i); + // A point within `reach` of the curve is within `reach` of its control box (the + // curve stays inside its control points' hull), so the other junctions can't match. + let b = kurbo::Shape::bounding_box(&c).union(kurbo::Rect::from_points(c.p1, c.p2)).inflate(reach, reach); + let lo = junctions.partition_point(|j| j.x < b.x0); let mut ts: Vec = junctions + .get(lo..) + .unwrap_or_default() .iter() + .take_while(|j| j.x <= b.x1) + .filter(|j| j.y >= b.y0 && j.y <= b.y1) .filter_map(|&j| { let nr = c.nearest(j, 1e-9); - (nr.distance_sq.sqrt() <= tol.max(1e-6)).then_some(nr.t) + (nr.distance_sq.sqrt() <= reach).then_some(nr.t) }) .collect(); ts.sort_by(f64::total_cmp); @@ -401,24 +427,41 @@ fn outline_edges(arr: &Arrangement, shapes: &[Shape]) -> Vec { } } } - // De-duplicate coincident chains, keeping the front-most key (later shapes overwrite). + // De-duplicate coincident chains, keeping the front-most key (later shapes overwrite). A + // match has its midpoint within `near` of this one's, so only the chains in the grid cells + // around it are candidates; the first of them in `out` order is the one a scan would find. let mut out: Vec<(Vec, u64)> = Vec::new(); + let mut sigs: Vec<(Point, Point, Point)> = Vec::new(); let sig = |c: &[Seg]| -> (Point, Point, Point) { let a = c[0].c.p0; let b = c[c.len() - 1].c.p3; let mid = mid_point(c); (a, b, mid) }; + let d = reach * 10.0; + let near = |p: Point, q: Point| p.distance(q) <= d; + let cell = |p: Point| ((p.x / d).floor() as i64, (p.y / d).floor() as i64); + let mut grid: HashMap<(i64, i64), Vec> = HashMap::new(); for (chain, key) in chains { let (a, b, m) = sig(&chain); - let near = |p: Point, q: Point| p.distance(q) <= tol.max(1e-6) * 10.0; - if let Some(e) = out.iter_mut().find(|(c, _)| { - let (a2, b2, m2) = sig(c); - near(m, m2) && ((near(a, a2) && near(b, b2)) || (near(a, b2) && near(b, a2))) - }) { - e.1 = key; - } else { - out.push((chain, key)); + let (cx, cy) = cell(m); + let found = (cx - 1..=cx + 1) + .flat_map(|x| (cy - 1..=cy + 1).map(move |y| (x, y))) + .filter_map(|k| grid.get(&k)) + .flatten() + .copied() + .filter(|&i| { + let (a2, b2, m2) = sigs[i]; + near(m, m2) && ((near(a, a2) && near(b, b2)) || (near(a, b2) && near(b, a2))) + }) + .min(); + match found { + Some(i) => out[i].1 = key, + None => { + grid.entry((cx, cy)).or_default().push(out.len()); + sigs.push((a, b, m)); + out.push((chain, key)); + } } } out.into_iter() diff --git a/crates/pathops/tests/pathfinder_pinned.rs b/crates/pathops/tests/pathfinder_pinned.rs new file mode 100644 index 00000000..b23c1499 --- /dev/null +++ b/crates/pathops/tests/pathfinder_pinned.rs @@ -0,0 +1,94 @@ +//! The exact output of every Pathfinder operation, pinned so that work on the windings and the +//! coverage tests can't change it: overlapping ellipses, rectangles and stars (even-odd and +//! non-zero, repeated keys for Merge) at a few counts. +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use vectorcraft_geom::{FillRule, Point, Rect, shapes}; +use vectorcraft_pathops::{PathfinderOp, Shape, pathfinder}; + +fn shapes_of(n: usize, seed: u64) -> Vec { + let mut r = seed | 1; + let mut next = || { + r ^= r << 13; + r ^= r >> 7; + r ^= r << 17; + (r % 1_000_000) as f64 / 1_000_000.0 + }; + (0..n) + .map(|i| { + let (x, y, s) = (next() * 200.0, next() * 150.0, 15.0 + next() * 40.0); + let path = match i % 3 { + 0 => shapes::ellipse(Rect::from_center_size(Point::new(x, y), (2.0 * s, 1.4 * s))), + 1 => shapes::rectangle(Rect::new(x, y, x + 1.5 * s, y + s)), + _ => shapes::star(Point::new(x, y), s, s / 2.0, 5, 0.3), + }; + let rule = if i % 4 == 3 { FillRule::EvenOdd } else { FillRule::NonZero }; + Shape::new(path, rule, (i % 5) as u64) + }) + .collect() +} + +fn fnv(s: &str) -> u64 { + s.bytes().fold(0xcbf2_9ce4_8422_2325, |h, b| (h ^ u64::from(b)).wrapping_mul(0x100_0000_01b3)) +} + +#[test] +fn every_pathfinder_operation_is_pinned() { + use PathfinderOp::*; + let ops = [Unite, MinusFront, MinusBack, Intersect, Exclude, Divide, Trim, Merge, Crop, Outline]; + let mut got = vec![]; + for (n, seed) in [(2, 3), (5, 7), (12, 11), (30, 13)] { + let input = shapes_of(n, seed); + for op in ops { + let out = pathfinder(op, &input); + let text: String = out.iter().map(|s| format!("{}|{:?}|{:?};", s.key, s.rule, s.path)).collect(); + got.push(fnv(&text)); + } + } + assert_eq!(got.len(), PINNED.len()); + assert_eq!(got, PINNED); +} + +/// Computed with the dense windings (main at b08482d6). +const PINNED: [u64; 40] = [ + 0x5e62_baae_d8b4_dba5, + 0x2bbc_e138_a97c_5a28, + 0xce78_992e_750b_1092, + 0x30cf_cc0b_861c_48b4, + 0xe6e4_2a29_1e16_5e86, + 0x390b_0633_5066_6292, + 0x06b7_3884_f961_c343, + 0x06b7_3884_f961_c343, + 0x0384_d599_5a03_dafd, + 0x3dcd_029a_dd3f_fa38, + 0x8249_71f8_fda4_2bb6, + 0xe7f2_8633_cef9_a76c, + 0x1a11_084e_721e_ac3b, + 0xcbf2_9ce4_8422_2325, + 0xa786_d973_a0d7_ca2a, + 0x389b_c89a_0ccf_578f, + 0xff9d_cf53_d872_681e, + 0xff9d_cf53_d872_681e, + 0xcbf2_9ce4_8422_2325, + 0x6be3_02a6_d9c9_e80e, + 0x3525_cabe_b235_aaef, + 0xf969_aa07_a44d_1770, + 0xed13_60c7_0aaa_7854, + 0xcbf2_9ce4_8422_2325, + 0x6550_a381_db6b_a35f, + 0x1ba9_6dfb_89ea_ac78, + 0x1344_b467_dd20_5f02, + 0x948f_e2a1_d657_0d4c, + 0x49a4_5c3c_2d96_14af, + 0x16a7_ce9e_4a3c_0974, + 0x0b12_c643_a446_da04, + 0xcbf2_9ce4_8422_2325, + 0xeb6a_6614_cea5_a508, + 0xcbf2_9ce4_8422_2325, + 0xb9c4_2803_7db9_bd4f, + 0x34b6_90d5_e443_b799, + 0xdfaf_ecbd_2d58_3227, + 0x6d5e_6e8d_b05b_8688, + 0x59dc_9268_a73f_e92e, + 0x5a83_5cbd_1f8c_5143, +]; diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index 447018e2..99c69e7d 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -469,7 +469,7 @@ fn import_invalid_dash_array_strokes_solid() { // And the exporter never writes an invalid array back. let mut d2 = doc(100.0, 100.0); let mut n = rect_node(Rect::new(10.0, 10.0, 90.0, 90.0), Color::rgb(1.0, 0.0, 0.0)); - n.appearance = Appearance::basic(Paint::None, Paint::solid(Color::BLACK), 2.0); + n.appearance = Appearance::basic(Paint::None, Paint::solid(Color::BLACK), 2.0).into(); if let Some(AppearanceItem::Stroke(st)) = n.appearance.items.last_mut() { st.dash = Some(Dash { pattern: vec![-5.0, 3.0], offset: 0.0, align_corners: false }); } diff --git a/crates/render/src/fx.rs b/crates/render/src/fx.rs index 266bfe0b..80ca5c52 100644 --- a/crates/render/src/fx.rs +++ b/crates/render/src/fx.rs @@ -261,6 +261,16 @@ pub(crate) struct ShadowEntry { pub(crate) stamp: u64, } +/// Live-effect geometry of a node, kept across frames: the object-level result and each +/// appearance item's own result on it (filled in as items are drawn). The node is kept alive, so +/// its address names it alone and a hit is exact. +pub(crate) struct FxGeom { + node: Arc, + g: Arc, + items: Vec>>, + pub(crate) stamp: u64, +} + impl Renderer { /// Drop shadow / outer glow `fx` (shadow-cache slot `i` of `n`) from a cached raster: /// the silhouette is rendered once into a small crop, blurred and tinted on the CPU, and reused @@ -340,12 +350,43 @@ impl Renderer { /// `draw_shape` for nodes with live effects. pub(crate) fn draw_shape_fx(&mut self, ctx: &mut RenderContext, f: &Frame, n: &Node, bp: &BezPath, rule: FillRule) { - let g = effected_path(n, bp); + // The node `draw_arc` is drawing gets its geometry from the cache: `bp` is its own path, + // made from the node alone, so the effects give the same result every frame. + let owner = self.fx_owner.as_ref().filter(|o| std::ptr::eq(&***o, n)).cloned(); + let mut cached = owner.map(|o| { + let key = Arc::as_ptr(&o) as usize; + let stamp = self.stamp; + let e = match self.fx_geom.remove(&key) { + Some(e) if Arc::ptr_eq(&e.node, &o) => e, + _ => FxGeom { g: Arc::new(effected_path(n, bp)), items: vec![None; n.appearance.items.len()], node: o, stamp }, + }; + (key, FxGeom { stamp, ..e }) + }); + let g = match &cached { + Some((_, e)) => e.g.clone(), + None => Arc::new(effected_path(n, bp)), + }; + self.draw_shape_fx_geom(ctx, f, n, &g, rule, cached.as_mut().map(|(_, e)| &mut e.items)); + if let Some((key, e)) = cached { + self.fx_geom.insert(key, e); + } + } + + /// [`Self::draw_shape_fx`] on the effected geometry `g`, with the items' cached geometry. + fn draw_shape_fx_geom( + &mut self, + ctx: &mut RenderContext, + f: &Frame, + n: &Node, + g: &BezPath, + rule: FillRule, + items: Option<&mut Vec>>>, + ) { if g.elements().is_empty() { return; } if f.opts.outline { - self.hairline(ctx, f, &g, [0, 0, 0, 255]); + self.hairline(ctx, f, g, [0, 0, 0, 255]); return; } let gctx = GeomContext::of(n); @@ -353,8 +394,9 @@ impl Renderer { // Document-space reach of the painted geometry (strokes, arrowheads…). let outset = n.appearance.outset(); let reach = g.bounding_box().inflate(outset, outset); - let content = Content { node: n, cache: Some(0), reach, outline: Some((&g, rule)) }; - self.raster_fx(ctx, f, &content, &rfx, &mut |r, c, fr| r.paint_items(c, fr, n, &g, rule, &gctx)); + let content = Content { node: n, cache: Some(0), reach, outline: Some((g, rule)) }; + let mut items = items; + self.raster_fx(ctx, f, &content, &rfx, &mut |r, c, fr| r.paint_items(c, fr, n, g, rule, &gctx, items.as_deref_mut())); ctx.set_transform(Affine::IDENTITY); } @@ -698,32 +740,49 @@ impl Renderer { /// Paint the fills and strokes of `n` on geometry `g`, applying per-item geometry effects and /// painting per-item raster effects around that item alone. - fn paint_items(&mut self, ctx: &mut RenderContext, f: &Frame, n: &Node, g: &BezPath, rule: FillRule, gctx: &GeomContext) { + #[allow(clippy::too_many_arguments)] + fn paint_items( + &mut self, + ctx: &mut RenderContext, + f: &Frame, + n: &Node, + g: &BezPath, + rule: FillRule, + gctx: &GeomContext, + mut cache: Option<&mut Vec>>>, + ) { let bounds = g.bounding_box(); for (j, item) in n.appearance.items.iter().enumerate() { if !item.visible() || item.paint().is_none() { continue; } - let ig = apply(item_effects(item), g, &gctx.item(item)); + // The item's own geometry effects on `g`, from the cache when there is one. + let slot = cache.as_deref_mut().and_then(|c| c.get_mut(j)); + let ig: Arc = match slot { + Some(Some(ig)) => ig.clone(), + Some(s) => s.insert(Arc::new(apply(item_effects(item), g, &gctx.item(item)))).clone(), + None => Arc::new(apply(item_effects(item), g, &gctx.item(item))), + }; + let ig = &*ig; let ib = if effects::has_geometry(item_effects(item)) && !ig.elements().is_empty() { ig.bounding_box() } else { bounds }; let rfx = effects::raster_effects(item_effects(item)); if rfx.is_empty() { - self.paint_item(ctx, f, n, item, &ig, rule, ib); + self.paint_item(ctx, f, n, item, ig, rule, ib); continue; } // The item's silhouette: its fill area, or the outline of its stroke. let outline = match item { AppearanceItem::Fill(_) => ig.clone(), AppearanceItem::Stroke(st) => { - let w = effects::stroke::aligned_width(st, effects::stroke::is_closed(&ig)); - effects::stroke::line_outline(&effects::stroke::stroke_pieces(&ig, st).line, st, w, f.px * 0.25) + let w = effects::stroke::aligned_width(st, effects::stroke::is_closed(ig)); + effects::stroke::line_outline(&effects::stroke::stroke_pieces(ig, st).line, st, w, f.px * 0.25) } }; let outset = n.appearance.outset(); let orule = if item.is_fill() { rule } else { FillRule::NonZero }; // Each item's shadows get their own cache slots. let content = Content { node: n, cache: Some((j + 1) << 8), reach: ib.inflate(outset, outset), outline: Some((&outline, orule)) }; - self.raster_fx(ctx, f, &content, &rfx, &mut |r, c, fr| r.paint_item(c, fr, n, item, &ig, rule, ib)); + self.raster_fx(ctx, f, &content, &rfx, &mut |r, c, fr| r.paint_item(c, fr, n, item, ig, rule, ib)); ctx.set_transform(Affine::IDENTITY); } } @@ -790,6 +849,85 @@ mod tests { const WHITE: u32 = 765; + /// A page of objects with object-level and item-level geometry effects (Offset Path, Roughen, + /// Warp, Outline Stroke, Transform) under drop shadows and in groups. + fn effects_page() -> Document { + let mut d = Document::new(200.0, 200.0); + let l = d.layers[0].id; + let fx = |id: &str, p: serde_json::Value| Effect { id: id.into(), params: p, visible: true }; + for i in 0..24 { + let (x, y) = (10.0 + (i % 6) as f64 * 30.0, 10.0 + (i / 6) as f64 * 45.0); + let id = d.alloc_id(); + let r = Rect::new(x, y, x + 20.0, y + 30.0); + let shape = if i % 2 == 0 { shapes::ellipse(r) } else { shapes::star(r.center(), 12.0, 5.0, 5, 0.0) }; + let mut n = Node::path(id, shape, Appearance::basic(Paint::solid(Color::rgb(0.2, 0.4, 0.9)), Paint::solid(Color::BLACK), 1.5)); + n.appearance.effects = match i % 4 { + 0 => vec![fx("path.offsetPath", json!({"offset": 3}))], + 1 => vec![fx("distort.roughen", json!({"size": 4, "relative": true})), fx("stylize.dropShadow", json!({}))], + 2 => vec![fx("warp.arc", json!({"bend": 30})), fx("path.outlineStroke", json!({}))], + _ => vec![fx("path.offsetPath", json!({"offset": -2})), fx("distort.transform", json!({"moveH": 4, "rotate": 10}))], + }; + if let Some(f) = n.appearance.fill_mut() { + f.effects.push(fx("path.offsetPath", json!({"offset": 2}))); + } + d.insert(Some(l), usize::MAX, n).unwrap(); + } + d + } + + /// Cached live-effect geometry draws exactly what computing it afresh draws: a second (warm) + /// frame, a frame after panning, and a fresh renderer all give the same pixels, and an edited + /// object (a new node) draws its new geometry, not the cached one. + #[test] + fn cached_effect_geometry_draws_the_same_pixels() { + let d = effects_page(); + let opts = RenderOptions { background: Some([255, 255, 255, 255]), ..Default::default() }; + let view = Affine::scale(2.0); + let mut r = Renderer::new(); + let cold = r.render(&d, 400, 400, view, &opts); + let warm = r.render(&d, 400, 400, view, &opts); + assert!(cold.pixels == warm.pixels, "a warm frame differs from the cold one"); + let _ = r.render(&d, 400, 400, Affine::translate((15.0, 0.0)) * view, &opts); + assert!(r.render(&d, 400, 400, view, &opts).pixels == cold.pixels, "after a pan"); + assert!(Renderer::new().render(&d, 400, 400, view, &opts).pixels == cold.pixels, "a fresh renderer"); + // Edit one object's effect: the warm renderer must draw the new result. + let mut e = d.clone(); + let id = e.layers[0].children().unwrap()[0].id; + e.node_mut(id).unwrap().appearance.effects = vec![Effect { id: "path.offsetPath".into(), params: json!({"offset": 9}), visible: true }]; + let edited = r.render(&e, 400, 400, view, &opts); + assert!(edited.pixels != cold.pixels, "the edit shows"); + assert!(edited.pixels == Renderer::new().render(&e, 400, 400, view, &opts).pixels, "and matches a fresh render"); + } + + /// The multithreaded rasterizer draws the same pixels with any number of workers, so the + /// automatic thread count can follow the machine. + #[test] + fn pixels_do_not_depend_on_the_thread_count() { + let mut d = effects_page(); + // Overlapping translucent and blended objects on top of the effects. + let l = d.layers[0].id; + for i in 0..400 { + let id = d.alloc_id(); + let (x, y) = ((i * 37 % 190) as f64, (i * 53 % 190) as f64); + let mut n = Node::path( + id, + shapes::ellipse(Rect::new(x, y, x + 14.0, y + 9.0)), + Appearance::basic(Paint::solid(Color::rgb(0.9, 0.3, 0.1)), Paint::solid(Color::BLACK), 0.7), + ); + n.opacity = 0.6; + d.insert(Some(l), usize::MAX, n).unwrap(); + } + let opts = RenderOptions { background: Some([255, 255, 255, 255]), ..Default::default() }; + let mut frames = vec![]; + for threads in [Some(0), Some(1), Some(4), Some(8)] { + crate::set_default_threads(threads); + let mut r = Renderer::new(); + frames.push(r.render(&d, 1200, 900, Affine::scale(6.0), &opts).pixels); + } + crate::set_default_threads(None); + assert!(frames.windows(2).all(|w| w[0] == w[1]), "the thread count changed pixels"); + } + #[test] fn thumbnails_of_glowing_objects_render_without_panicking() { // Regression: thumbnails used vello's default (multithreaded) context, which panics on filters. @@ -809,7 +947,7 @@ mod tests { let l = d.layers[0].id; let id = d.layers[0].children().unwrap()[0].id; let mut n = (*d.node(id).unwrap()).clone(); - n.appearance = Appearance::basic(Paint::None, Paint::solid(Color::BLACK), 4.0); + n.appearance = Appearance::basic(Paint::None, Paint::solid(Color::BLACK), 4.0).into(); if let Some(vectorcraft_doc::AppearanceItem::Stroke(s)) = n.appearance.items.iter_mut().find(|i| matches!(i, vectorcraft_doc::AppearanceItem::Stroke(_))) { diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index e2f93205..36bc4b06 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -358,6 +358,10 @@ pub struct Renderer { strokes: PtrMap<(usize, u64, i32), StrokeEntry>, /// The node being drawn on the plain-path fast path (owner of cached stroke outlines). cur: Option>, + /// The node `draw_shape_fx` may cache the live-effect geometry of (the one `draw_arc` is drawing). + fx_owner: Option>, + /// Live-effect geometry per node ([`fx::FxGeom`]), keyed by `Arc` identity like `geom`. + fx_geom: PtrMap, stamp: u64, /// Opacity folded into paint alpha for the leaf being drawn (avoids a compositing layer). alpha: f32, @@ -480,6 +484,8 @@ impl Renderer { geom: PtrMap::default(), strokes: PtrMap::default(), cur: None, + fx_owner: None, + fx_geom: PtrMap::default(), stamp: 0, alpha: 1.0, ctx: None, @@ -572,6 +578,9 @@ impl Renderer { if self.shadows.len() > 256 { self.shadows.retain(|_, e| g - e.stamp <= 3); } + if self.fx_geom.len() > 256 { + self.fx_geom.retain(|_, e| g - e.stamp <= 3); + } // Filtered rasters are large: only those of the last frame stay. self.pixel_fx.retain(|_, e| e.stamp == g); if !inks { @@ -911,7 +920,10 @@ impl Renderer { if vectorcraft_doc::live::is_live(a) || effects::has_pathfinder(a) { return self.draw_live(ctx, f, a); } + // Its live-effect geometry depends on the node alone, so it can be kept across frames. + let outer = self.fx_owner.replace(a.clone()); self.draw_node(ctx, f, a, true); + self.fx_owner = outer; } fn draw_node(&mut self, ctx: &mut RenderContext, f: &Frame, n: &Node, force: bool) { @@ -1755,7 +1767,9 @@ pub fn set_default_threads(n: Option) { THREADS_OVERRIDE.store(n.map_or(-1, i32::from), std::sync::atomic::Ordering::Relaxed); } -/// Rasterizer worker threads: 0 on wasm; otherwise up to 4 (vello's sweet spot), leaving a core free. +/// Rasterizer worker threads: 0 on wasm; otherwise one per core up to 8, leaving a core free. On a +/// 10-core machine 8 workers render a 50,000-path page 1.4× faster than 4 (9 are slower again); +/// the pixels don't depend on the count (`pixels_do_not_depend_on_the_thread_count`). pub fn default_threads() -> u16 { #[cfg(target_arch = "wasm32")] { @@ -1770,7 +1784,7 @@ pub fn default_threads() -> u16 { if std::env::var_os("VECTORCRAFT_RENDER_THREADS").is_some() { return std::env::var("VECTORCRAFT_RENDER_THREADS").ok().and_then(|v| v.parse().ok()).unwrap_or(0); } - std::thread::available_parallelism().map(|n| (n.get().saturating_sub(1)).min(4) as u16).unwrap_or(0) + std::thread::available_parallelism().map(|n| (n.get().saturating_sub(1)).min(8) as u16).unwrap_or(0) } } diff --git a/crates/svg/src/export.rs b/crates/svg/src/export.rs index 2605109e..50e7291a 100644 --- a/crates/svg/src/export.rs +++ b/crates/svg/src/export.rs @@ -656,9 +656,13 @@ impl Writer<'_> { fn path_d(&self, p: &PathData, m: Affine) -> String { let mut s = String::new(); - let pt = |p: Point| { + // Each point as "x y", appended in place (no string per number or per point). + let dec = self.opts.decimals; + let pt = |s: &mut String, p: Point| { let p = m * p; - format!("{} {}", self.num(p.x), self.num(p.y)) + crate::push_num(s, p.x, dec); + s.push(' '); + crate::push_num(s, p.y, dec); }; for sp in &p.subpaths { let n = sp.anchors.len(); @@ -669,16 +673,22 @@ impl Writer<'_> { s.push(' '); } s.push('M'); - s.push_str(&pt(sp.anchors[0].p)); + pt(&mut s, sp.anchors[0].p); for i in 0..sp.segment_count() { let (a, b) = (&sp.anchors[i], &sp.anchors[(i + 1) % n]); if sp.segment_is_line(i) { if sp.closed && i == n - 1 { break; } - s.push_str(&format!(" L{}", pt(b.p))); + s.push_str(" L"); + pt(&mut s, b.p); } else { - s.push_str(&format!(" C{} {} {}", pt(a.h_out), pt(b.h_in), pt(b.p))); + s.push_str(" C"); + pt(&mut s, a.h_out); + s.push(' '); + pt(&mut s, b.h_in); + s.push(' '); + pt(&mut s, b.p); } } if sp.closed { diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index 4deefef7..67983cfb 100644 --- a/crates/svg/src/import.rs +++ b/crates/svg/src/import.rs @@ -860,7 +860,7 @@ impl Importer { text::fold_scale(&mut obj, grow); let mut n = self.named(&name, NodeKind::Text(Box::new(obj))); if let Some(ap) = under { - n.appearance = ap; + n.appearance = ap.into(); } Some(vec![Arc::new(n)]) } @@ -1190,7 +1190,7 @@ impl Importer { .map(|sp| { let id = self.doc.alloc_id(); let mut c = Node::new(id, NodeKind::Path { path: PathData::single(sp), rule: r, live: None, clipping: false, guide: false }); - c.appearance = appearance.clone(); + c.appearance = appearance.clone().into(); Arc::new(c) }) .collect(); @@ -1198,7 +1198,7 @@ impl Importer { } else { self.named(p.id(), NodeKind::Path { path, rule: r, live: None, clipping: false, guide: false }) }; - n.appearance = appearance; + n.appearance = appearance.into(); Some(n) } diff --git a/crates/svg/src/lib.rs b/crates/svg/src/lib.rs index 3789d8f4..7cbc8531 100644 --- a/crates/svg/src/lib.rs +++ b/crates/svg/src/lib.rs @@ -567,10 +567,26 @@ pub fn xml_escape(s: &str) -> String { /// Format a number with at most `decimals` places, trimming trailing zeros. pub fn fmt_num(v: f64, decimals: u8) -> String { + let mut s = String::new(); + push_num(&mut s, v, decimals); + s +} + +/// [`fmt_num`] appended to `out`, without allocating. +pub fn push_num(out: &mut String, v: f64, decimals: u8) { + use std::fmt::Write as _; let v = if v.is_finite() { v } else { 0.0 }; - let s = format!("{:.*}", decimals as usize, v); - let s = if s.contains('.') { s.trim_end_matches('0').trim_end_matches('.').to_string() } else { s }; - if s == "-0" || s.is_empty() { "0".into() } else { s } + let start = out.len(); + let _ = write!(out, "{:.*}", decimals as usize, v); // writing to a String can't fail + let s = out.get(start..).unwrap_or_default(); + if s.contains('.') { + let keep = s.trim_end_matches('0').trim_end_matches('.').len(); + out.truncate(start + keep); + } + if matches!(out.get(start..), Some("-0" | "")) { + out.truncate(start); + out.push('0'); + } } #[cfg(test)] diff --git a/crates/svg/tests/css_rules.rs b/crates/svg/tests/css_rules.rs index d3065416..4004c04c 100644 --- a/crates/svg/tests/css_rules.rs +++ b/crates/svg/tests/css_rules.rs @@ -188,7 +188,7 @@ fn shared_properties_read_as_svg_export_writes_them() { fn gradients_give_css_gradients() { let mut d = doc(); let mut n = rect(&mut d, (0.0, 0.0, 200.0, 100.0), 0.0); - n.appearance = Appearance::basic(Paint::Gradient(Box::new(GradientPaint::new(Gradient::default()))), Paint::None, 0.0); + n.appearance = Appearance::basic(Paint::Gradient(Box::new(GradientPaint::new(Gradient::default()))), Paint::None, 0.0).into(); let id = add(&mut d, n); let r = one(&d, id, &CssOptions::default()); assert_eq!(prop(&r, "background-image"), Some("linear-gradient(90deg, #ffffff 0%, #000000 100%)")); @@ -196,13 +196,13 @@ fn gradients_give_css_gradients() { let mut g = GradientPaint::new(Gradient::default()); g.geom = Some(GradientGeom { start: Point::new(100.0, 0.0), end: Point::new(100.0, 50.0), aspect: 1.0, focal: None }); let mut n = rect(&mut d, (0.0, 0.0, 200.0, 100.0), 0.0); - n.appearance = Appearance::basic(Paint::Gradient(Box::new(g)), Paint::None, 0.0); + n.appearance = Appearance::basic(Paint::Gradient(Box::new(g)), Paint::None, 0.0).into(); let id = add(&mut d, n); assert_eq!(prop(&one(&d, id, &CssOptions::default()), "background-image"), Some("linear-gradient(180deg, #ffffff 0%, #000000 50%)")); // Radial: a circle at the box's centre. let radial = GradientPaint::new(Gradient { kind: GradientKind::Radial, ..Gradient::default() }); let mut n = rect(&mut d, (0.0, 0.0, 100.0, 100.0), 0.0); - n.appearance = Appearance::basic(Paint::Gradient(Box::new(radial)), Paint::None, 0.0); + n.appearance = Appearance::basic(Paint::Gradient(Box::new(radial)), Paint::None, 0.0).into(); let id = add(&mut d, n); let bg = one(&d, id, &CssOptions::default()).props.into_iter().find(|(k, _)| *k == "background-image").unwrap().1; assert!(bg.starts_with("radial-gradient(circle ") && bg.contains(" at 50px 50px, #ffffff 0%, #000000 100%)"), "{bg}"); diff --git a/crates/svg/tests/roundtrip.rs b/crates/svg/tests/roundtrip.rs index c7433a7a..53c732ee 100644 --- a/crates/svg/tests/roundtrip.rs +++ b/crates/svg/tests/roundtrip.rs @@ -144,7 +144,7 @@ fn compound_evenodd_roundtrip() { let i = Node::path(d.alloc_id(), shapes::rectangle(Rect::new(25.0, 25.0, 75.0, 75.0)), Appearance::default()); let cid = d.alloc_id(); let mut c = Node::new(cid, NodeKind::Compound { children: vec![Arc::new(o), Arc::new(i)], rule: FillRule::EvenOdd }); - c.appearance = Appearance::basic(solid("#123456"), Paint::None, 1.0); + c.appearance = Appearance::basic(solid("#123456"), Paint::None, 1.0).into(); let d = doc_with(vec![c]); let s = export(&d, &ExportOptions::default()); assert_eq!(s.matches("