From 9a4f4555c6347812b3bf7888cd48b7a61c52e743 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 17:29:22 -0800 Subject: [PATCH] Graphic Styles: applying a style shares one appearance across the objects graphicStyle.apply and graphicStyle.redefine copied the style's whole appearance into a new allocation for every target. Targets that take the style's appearance as stored now share one copy-on-write handle; a target whose placed gradients are fitted to its own bounds still gets its own. Editing one object later gives it its own copy. Saved files and undo values are unchanged. The undo history's memory estimate counted a shared appearance once per object; it now counts each appearance allocation once per side. --- crates/doc/src/mem.rs | 44 ++++++++++++++--- crates/engine/src/cmd/style.rs | 26 ++++++---- crates/engine/src/tests/gradpanel.rs | 38 +++++++++++++++ crates/engine/src/tests/styles.rs | 73 +++++++++++++++++++++++++++- docs/target-app-parity.md | 1 + 5 files changed, 164 insertions(+), 18 deletions(-) diff --git a/crates/doc/src/mem.rs b/crates/doc/src/mem.rs index e2d27650b..12d233ba3 100644 --- a/crates/doc/src/mem.rs +++ b/crates/doc/src/mem.rs @@ -46,7 +46,7 @@ impl Document { return (0, 0); } let own = size_of::() + ALLOC + ARC; - let mut w = Walk { docs: [self, other], seen: [None, None], bytes: [own, own] }; + let mut w = Walk { docs: [self, other], seen: [None, None], bytes: [own, own], appearances: Default::default() }; w.list(&self.layers, &other.layers); for i in 0..self.symbols.len().max(other.symbols.len()) { match (self.symbols.get(i), other.symbols.get(i)) { @@ -134,6 +134,9 @@ struct Walk<'a> { seen: [Option>; 2], /// For each side, what it keeps that the other doesn't. bytes: [usize; 2], + /// For each side, the appearances counted so far (by address): objects that share one (a + /// graphic style applied to them all) count it once. + appearances: [HashSet; 2], } impl Walk<'_> { @@ -174,8 +177,9 @@ impl Walk<'_> { self.alone(1, b); return; } - self.bytes[0] += node_bytes(a, Some(b)); - self.bytes[1] += node_bytes(b, Some(a)); + let [ca, cb] = &mut self.appearances; + self.bytes[0] += node_bytes(a, Some(b), ca); + self.bytes[1] += node_bytes(b, Some(a), cb); match (a.children(), b.children()) { (Some(x), Some(y)) => self.list(x, y), (x, y) => { @@ -198,8 +202,8 @@ impl Walk<'_> { if self.other_holds(side, n) { return; } - if let Some(b) = self.bytes.get_mut(side) { - *b += node_bytes(n, None); + if let (Some(b), Some(counted)) = (self.bytes.get_mut(side), self.appearances.get_mut(side)) { + *b += node_bytes(n, None, counted); } for c in n.children().into_iter().flatten() { self.alone(side, c); @@ -242,14 +246,15 @@ impl Walk<'_> { } /// One node's own heap: the node, and what it owns besides its children and mask art (counted -/// as nodes of their own). Its appearance counts unless `other`, its other version, shares it. -fn node_bytes(n: &Node, other: Option<&Node>) -> usize { +/// as nodes of their own). Its appearance counts unless `other`, its other version, shares it, or +/// another node of the same side shares it and was counted already (its address is in `counted`). +fn node_bytes(n: &Node, other: Option<&Node>, counted: &mut HashSet) -> usize { fn boxed(b: &Option>) -> usize { b.as_ref().map_or(0, |_| ALLOC + size_of::()) } let mut b = ALLOC + ARC + size_of::(); b += n.name.as_ref().map_or(0, String::capacity); - if !other.is_some_and(|m| Shared::ptr_eq(&m.appearance, &n.appearance)) { + if !other.is_some_and(|m| Shared::ptr_eq(&m.appearance, &n.appearance)) && counted.insert(&*n.appearance as *const Appearance as usize) { b += ALLOC + ARC + appearance_heap_bytes(&n.appearance); } if let Some(e) = n.extras_box() { @@ -447,6 +452,29 @@ mod tests { assert!(added > kept + 3 * 300, "{added} for three new rectangles"); } + /// Objects given one shared appearance (a graphic style applied to them all) count it once. + #[test] + fn a_shared_appearance_counts_once() { + let n = 100; + let before = doc_with_rects(n); + let ids: Vec<_> = before.layers[0].children().unwrap().iter().map(|c| c.id).collect(); + let ap = Appearance::basic(Paint::None, Paint::solid(vectorcraft_color::Color::BLACK), 2.0); + let restyled = |each: bool| { + let mut d = before.clone(); + let one = Shared::new(ap.clone()); + for id in &ids { + d.node_mut(*id).unwrap().appearance = if each { Shared::new(ap.clone()) } else { one.clone() }; + } + d + }; + let (shared, copies) = (restyled(false), restyled(true)); + let (kept, added) = before.retained_bytes_each(&shared); + assert_eq!(shared.retained_bytes_each(&before), (added, kept)); + let (kept_copies, added_copies) = before.retained_bytes_each(&copies); + assert_eq!(kept, kept_copies, "the old version keeps the same either way"); + assert_eq!(added_copies - added, (n - 1) * (ALLOC + ARC + appearance_heap_bytes(&ap))); + } + #[test] fn a_resource_counts_only_when_changed() { let before = doc_with_rects(1); diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index d31031d5d..bca9ed6ea 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -13,11 +13,12 @@ //! to loses its characters' own fill (stroke) when the style has fills (strokes), so the style's //! paint shows instead of the character colour under it. +use std::borrow::Cow; use std::collections::HashMap; use serde_json::{Value, json}; use vectorcraft_color::Paint; -use vectorcraft_doc::{Appearance, DEFAULT_GRAPHIC_STYLE, Document, GraphicStyle, Node, NodeId, NodeKind}; +use vectorcraft_doc::{Appearance, DEFAULT_GRAPHIC_STYLE, Document, GraphicStyle, Node, NodeId, NodeKind, Shared}; use vectorcraft_geom::Rect; use super::{CommandSpec, Result, Session, always, bad, bool_or, cmd, has_doc, index, object_param, ok, str_param, unique_name}; @@ -260,9 +261,14 @@ 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().into(); +/// characters' colour too with `chars`, Override Character Color) and transparency. Objects that +/// take `g`'s appearance as it is share one copy of it, made in `shared` the first time; an object +/// whose placed gradients land on its own bounds gets its own. +fn style_node(n: &mut Node, g: &GraphicStyle, shared: &mut Option>, chars: bool) { + n.appearance = match g.appearance_on(n) { + Cow::Borrowed(ap) => shared.get_or_insert_with(|| Shared::new(ap.clone())).clone(), + Cow::Owned(ap) => ap.into(), + }; if chars { override_chars(n, &g.appearance); } @@ -270,9 +276,9 @@ fn style_node(n: &mut Node, g: &GraphicStyle, chars: bool) { } /// Give `id` style `g` ([`style_node`]) and link it. -fn apply_style(d: &mut Document, id: NodeId, g: &GraphicStyle, chars: bool) { +fn apply_style(d: &mut Document, id: NodeId, g: &GraphicStyle, shared: &mut Option>, chars: bool) { if let Some(n) = d.node_mut(id) { - style_node(n, g, chars); + style_node(n, g, shared, chars); n.extras_mut().graphic_style = Some(g.id); super::object::snap_if_aligned(n); } @@ -313,7 +319,7 @@ impl Session { /// Color as set): the Graphic Styles panel's previews. pub fn styled(&self, n: &Node, g: &GraphicStyle) -> Node { let mut n = n.clone(); - style_node(&mut n, g, self.prefs.override_char_color); + style_node(&mut n, g, &mut None, self.prefs.override_char_color); n } } @@ -354,8 +360,9 @@ fn style_apply(s: &mut Session, p: &Value) -> Result { } else { d.graphic_style_id(i); let g = d.graphic_styles[i].clone(); + let mut shared = None; for id in &ids { - apply_style(d, *id, &g, chars); + apply_style(d, *id, &g, &mut shared, chars); } super::appearance::capture_series_writes(&before, d, &ids, (true, true), (true, true), false, true); } @@ -474,8 +481,9 @@ fn style_redefine(s: &mut Session, p: &Value) -> Result { let (follow, edited): (Vec, Vec) = linked.into_iter().partition(|nid| d.node(*nid).is_some_and(|n| in_sync(n, &old))); set_link(d, &edited, None); let g = d.graphic_styles[i].clone(); + let mut shared = None; for nid in &follow { - apply_style(d, *nid, &g, chars); + apply_style(d, *nid, &g, &mut shared, chars); } super::appearance::capture_series_writes(&before, d, &follow, (true, true), (true, true), false, true); set_link(d, &[src], Some(id)); diff --git a/crates/engine/src/tests/gradpanel.rs b/crates/engine/src/tests/gradpanel.rs index 85fd4cf02..680abfa9e 100644 --- a/crates/engine/src/tests/gradpanel.rs +++ b/crates/engine/src/tests/gradpanel.rs @@ -132,6 +132,44 @@ fn graphic_styles_keep_gradients_relative_to_the_bounds() { assert!(close(fill_geom(&s, dst).unwrap().start, Point::new(0.2, 0.5)), "legacy styles are applied as stored"); } +/// Applying a style with a placed gradient to objects of different sizes gives each its own +/// appearance with the gradient on its own bounds, through undo and redo; a style from an older +/// file applies the same stored gradient to each, as one shared appearance. +#[test] +fn a_style_places_its_gradient_on_each_object() { + let (mut s, src, dst) = source_and_target(); + let other = rect(&mut s, 100.0, 300.0, 50.0, 200.0); + s.execute("select.set", &json!({"ids": [src.0]})).unwrap(); + s.execute("graphicStyle.new", &json!({"name": "Glow"})).unwrap(); + s.execute("graphicStyle.apply", &json!({"name": "Glow", "ids": [dst.0, other.0]})).unwrap(); + let placed = |s: &Session| { + let (g, h) = (fill_geom(s, dst).unwrap(), fill_geom(s, other).unwrap()); + close(g.start, Point::new(540.0, 60.0)) + && close(g.end, Point::new(660.0, 60.0)) + && close(h.start, Point::new(110.0, 400.0)) + && close(h.end, Point::new(140.0, 400.0)) + }; + let shared = |s: &Session| { + let d = &s.doc().unwrap().doc; + vectorcraft_doc::Shared::ptr_eq(&d.node(dst).unwrap().appearance, &d.node(other).unwrap().appearance) + }; + assert!(placed(&s) && !shared(&s)); + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("edit.redo", &json!({})).unwrap(); + assert!(placed(&s) && !shared(&s)); + + let glow = s.doc().unwrap().doc.graphic_styles.iter().find(|g| g.name == "Glow").cloned().unwrap(); + let legacy = vectorcraft_doc::GraphicStyle { name: "Old".into(), id: 0, unit_box: false, ..glow }; + s.edit("Add", |d, _| { + d.graphic_styles.push(legacy); + Ok(()) + }) + .unwrap(); + s.execute("graphicStyle.apply", &json!({"name": "Old", "ids": [dst.0, other.0]})).unwrap(); + assert!(shared(&s)); + assert!(close(fill_geom(&s, other).unwrap().start, Point::new(0.2, 0.5)), "legacy styles are applied as stored"); +} + #[test] fn eyedropper_options_choose_what_copy_from_takes() { let mut s = session(); diff --git a/crates/engine/src/tests/styles.rs b/crates/engine/src/tests/styles.rs index 3ffb2ed61..6f77b5897 100644 --- a/crates/engine/src/tests/styles.rs +++ b/crates/engine/src/tests/styles.rs @@ -3,7 +3,7 @@ use serde_json::{Value, json}; use vectorcraft_color::{BlendMode, Color, Paint}; -use vectorcraft_doc::{GraphicStyle, Knockout, Node}; +use vectorcraft_doc::{GraphicStyle, Knockout, Node, Shared}; use crate::*; @@ -202,6 +202,77 @@ fn redefine_leaves_unlinked_and_edited_objects_alone() { assert!(s.execute("graphicStyle.redefine", &json!({"id": c.0})).is_err()); } +/// Applying a style to several objects gives them one shared copy of its appearance; editing one +/// object gives that one its own, and the saved file is the same as with a copy for each. +#[test] +fn applying_a_style_shares_one_appearance() { + let mut s = session(); + let a = rect(&mut s, 10.0); + run(&mut s, "appearance.clear", json!({})); + run(&mut s, "paint.setFill", json!({"color": "#00ff00"})); + run(&mut s, "paint.setStroke", json!({"color": "#0000ff"})); + run(&mut s, "effect.apply", json!({"effect": "distort.roughen"})); + run(&mut s, "graphicStyle.new", json!({"name": "Rough"})); + let rough = style(&s, "Rough"); + let ids = [rect(&mut s, 100.0), rect(&mut s, 200.0), rect(&mut s, 300.0)]; + let [b, c, d] = ids; + run(&mut s, "graphicStyle.apply", json!({"name": "Rough", "ids": [b.0, c.0, d.0]})); + let shared = |s: &Session, x: NodeId, y: NodeId| Shared::ptr_eq(&node(s, x).appearance, &node(s, y).appearance); + assert!(shared(&s, b, c) && shared(&s, c, d), "one appearance for every object"); + for id in ids { + assert_eq!(node(&s, id).appearance, rough.appearance); + assert_eq!(node(&s, id).extras().graphic_style, Some(rough.id)); + } + // The saved file is the same as one where each object has its own copy. + let doc = s.doc().unwrap().doc.clone(); + let mut copies = (*doc).clone(); + for id in ids { + let n = copies.node_mut(id).unwrap(); + n.appearance = Shared::new((*n.appearance).clone()); + } + assert!(!Shared::ptr_eq(&copies.node(b).unwrap().appearance, &copies.node(c).unwrap().appearance)); + for pretty in [false, true] { + assert_eq!(vectorcraft_format::save(&doc, pretty), vectorcraft_format::save(&copies, pretty)); + } + + // Editing one object leaves the others and the style as they were. + set_fill(&mut s, b, "#ff0000"); + assert!(!shared(&s, b, c) && shared(&s, c, d)); + assert_eq!(fill(&s, b), Paint::solid(Color::from_hex("#ff0000").unwrap())); + assert_eq!(node(&s, c).appearance, rough.appearance); + assert_eq!(style(&s, "Rough"), rough); + assert_eq!(linked(&mut s, "Rough"), vec![a.0, c.0, d.0]); + + // Undo and redo restore the values, still shared. + run(&mut s, "edit.undo", json!({})); + assert!(shared(&s, b, c) && node(&s, b).appearance == rough.appearance); + run(&mut s, "edit.undo", json!({})); + for id in ids { + assert_ne!(node(&s, id).appearance, rough.appearance); + assert_eq!(node(&s, id).extras().graphic_style, None); + } + run(&mut s, "edit.redo", json!({})); + assert!(shared(&s, b, c) && shared(&s, c, d) && node(&s, d).appearance == rough.appearance); +} + +/// Redefine Graphic Style gives the objects still linked one shared copy of the new appearance. +#[test] +fn redefine_shares_one_appearance() { + let mut s = session(); + let (a, b, c) = linked_three(&mut s); + let d = rect(&mut s, 300.0); + run(&mut s, "graphicStyle.apply", json!({"name": "S"})); + set_fill(&mut s, d, "#00ff00"); + set_fill(&mut s, a, "#0000ff"); + run(&mut s, "graphicStyle.redefine", json!({"name": "S", "id": a.0})); + let (nb, nc) = (node(&s, b), node(&s, c)); + assert!(Shared::ptr_eq(&nb.appearance, &nc.appearance)); + assert_eq!(nb.appearance, style(&s, "S").appearance); + // The edited object keeps its own look. + assert_eq!(fill(&s, d), Paint::solid(Color::from_hex("#00ff00").unwrap())); + assert!(!Shared::ptr_eq(&node(&s, d).appearance, &nb.appearance)); +} + #[test] fn deleting_a_style_unlinks_and_renaming_keeps_links() { let mut s = session(); diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index b4c949f74..fe301b56a 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1333,6 +1333,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur Type inside a group, layer, symbol instance, blend or repeat keeps its opacity, blend mode, opacity mask, Opacity & Mask Define Knockout Shape, own effects, perspective and visibility when the effects on the container outline it. Hidden type with a geometry effect of its own is left out of exports. - **Graphic styles:** styles keep opacity and blend mode, capture groups and type, apply on top with Alt, and link to the objects using them; Redefine, Break Link, Graphic Style Options, Select All Unused, Sort by Name, and Select > Same Graphic Style / Appearance Attribute. The panel has list and thumbnail views with rendered previews, merge, reorder and drag and drop, plus a Control bar Style picker; eight generated style libraries (including Game HUD and Status Effects) save and load as `.vcstyles`. In the Graphic Styles panel, Shift-click selects a range of styles and Ctrl/Cmd-click adds or removes one. + Applying a style to several objects, and Redefine Graphic Style, give the objects one shared copy of the style's appearance instead of a copy each; an object that is edited later gets its own copy, and an object whose placed gradients are fitted to its own bounds gets its own. The undo history's memory estimate counts a shared appearance once. Saved files are unchanged (`crates/engine/src/tests/styles.rs`). - **Several text styles at once:** in the Character Styles and Paragraph Styles panels, Shift-click highlights a range of styles and Ctrl/Cmd-click adds or removes one, without applying them; the panel menu's Select All Unused (`charStyle.unused`, `paraStyle.unused`) highlights the styles no type uses (in the layers, opacity masks, symbols, graph designs, pattern tiles and the art of brushes), never the Normal style; Delete, the button or the menu item, deletes every highlighted style but Normal in one undo step (`charStyle.delete {names}`, `paraStyle.delete {names}`). The menu's Duplicate copies every highlighted style in one undo step and highlights the copies. A highlighted row dragged onto Create New Style or Delete carries every highlighted style: dropped on Create New Style it copies them all and highlights the copies, and dropped on Delete it deletes them all but Normal and clears the highlight; a row from outside the highlight carries only its own style. Public reference: [Character and paragraph styles](https://web.archive.org/web/20261015081534/https://helpx.adobe.com/illustrator/using/character-paragraph-styles.html). - **Load text styles from another document:** the Character Styles and Paragraph Styles panel menus have Load Character Styles… or Load Paragraph Styles… and Load All Styles…, which copy the styles of a document picked in an open dialog, with the swatches and patterns they paint with, in one undo step (`charStyle.load`, `paraStyle.load`, `textStyle.loadAll`); a style the document already has, under its name or a numbered one, is not added again, any other whose name is taken gets a number, and the file's Normal styles are not loaded. Public reference: [Character and paragraph styles](https://web.archive.org/web/20261015081534/https://helpx.adobe.com/illustrator/using/character-paragraph-styles.html). - **Neutral wording:** labels, MCP tool text, docs and packaging use neutral names, and `cargo xtask brands` (part of `cargo xtask ci`) fails on vendor names.