From 07bccb19a7c1181786b9529527a84f92c1cc46ce Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 14:36:41 -0800 Subject: [PATCH] Swatches: New Color Group, Delete, Move and Merge take named swatches in one pass New Color Group looked up and removed each named swatch with a scan of every swatch list, so making a group from artwork linked to many global swatches took time quadratic in their number. Delete Swatch, Move and Merge did the same, and checked each paint's link against the name list. take_swatches now finds every named swatch in one pass and removes them together, in the order and with the same choice of swatch as removing them one at a time. The commands look names up in one name map and check links against sets. With 16,000 named swatches in a release build, New Color Group went from 175.9 ms to 16.3 ms; 4,000 went from 11.6 ms to 3.6 ms. --- crates/engine/src/cmd/swatch.rs | 97 ++++++++++++++++++++------- crates/engine/src/tests/swatchcmds.rs | 29 ++++++++ 2 files changed, 103 insertions(+), 23 deletions(-) diff --git a/crates/engine/src/cmd/swatch.rs b/crates/engine/src/cmd/swatch.rs index 2421441b9..eed8ee1f8 100644 --- a/crates/engine/src/cmd/swatch.rs +++ b/crates/engine/src/cmd/swatch.rs @@ -1,6 +1,6 @@ //! The Swatches panel: new, delete and duplicate swatches, colour groups and sorting. -use std::collections::{HashMap, HashSet}; +use std::collections::{HashMap, HashSet, VecDeque}; use serde_json::{Value, json}; use vectorcraft_color::{Color, Gradient, GradientKind, GradientMethod, GradientStop, Paint, Swatch, SwatchGroup, tint_percent}; @@ -222,9 +222,10 @@ fn swatch_new(s: &mut Session, p: &Value) -> Result { /// The `names` list and `name` parameters, without repeats; at least one is required. pub(super) fn names_param(p: &Value, cmd: &str) -> Result> { + let mut seen: HashSet = HashSet::new(); let mut names: Vec = vec![]; for n in str_list(p, "names").into_iter().chain(str_param(p, "name").map(str::to_string)) { - if !names.contains(&n) { + if seen.insert(n.clone()) { names.push(n); } } @@ -234,17 +235,73 @@ pub(super) fn names_param(p: &Value, cmd: &str) -> Result> { Ok(names) } +/// Take swatches `names` out of the document in that order, as [`Document::remove_swatch`] for +/// each name in turn would, in one pass over the swatches: a name takes the first swatch of that +/// name still there (ungrouped first, then each colour group's) when `take` accepts it, and +/// nothing when `take` refuses it or no swatch has the name. +fn take_swatches(d: &mut Document, names: &[String], take: impl Fn(&Swatch) -> bool) -> Vec { + let wanted: HashSet<&str> = names.iter().map(String::as_str).collect(); + // Each wanted name's swatches in lookup order: (list, index, accepted), where list 0 holds the + // ungrouped swatches and list g + 1 colour group g's. + let mut places: HashMap<&str, VecDeque<(usize, usize, bool)>> = HashMap::new(); + let lists = std::iter::once(&d.swatches[..]).chain(d.swatch_groups.iter().map(|g| &g.swatches[..])); + for (l, list) in lists.enumerate() { + for (i, w) in list.iter().enumerate() { + if let Some(n) = wanted.get(w.name.as_str()) { + places.entry(n).or_default().push_back((l, i, take(w))); + } + } + } + let mut picked: Vec<(usize, usize)> = vec![]; + for n in names { + if let Some(q) = places.get_mut(n.as_str()) + && let Some(&(l, i, true)) = q.front() + { + q.pop_front(); + picked.push((l, i)); + } + } + if picked.is_empty() { + return vec![]; + } + let hit: HashSet<(usize, usize)> = picked.iter().copied().collect(); + let hit_lists: HashSet = picked.iter().map(|&(l, _)| l).collect(); + let mut taken: HashMap<(usize, usize), Swatch> = HashMap::with_capacity(picked.len()); + let mut split = |l: usize, list: &mut Vec| { + for (i, w) in std::mem::take(list).into_iter().enumerate() { + if hit.contains(&(l, i)) { + taken.insert((l, i), w); + } else { + list.push(w); + } + } + }; + // Both lists are copied on write, so each is only written when a swatch leaves it. + if hit_lists.contains(&0) { + split(0, &mut d.swatches); + } + if hit_lists.iter().any(|&l| l > 0) { + for (g, group) in d.swatch_groups.iter_mut().enumerate() { + if hit_lists.contains(&(g + 1)) { + split(g + 1, &mut group.swatches); + } + } + } + picked.iter().filter_map(|k| taken.remove(k)).collect() +} + fn swatch_delete(s: &mut Session, p: &Value) -> Result { const C: &str = "swatch.delete"; let names = names_param(p, C)?; check_switches(p, &["unlink"], C)?; let d = &s.doc()?.doc; + let by_name = swatches_by_name(d); // Global swatches going away (named ones and those in named groups): their links are dropped. - let mut global: Vec = vec![]; + let mut global: HashSet = HashSet::new(); for n in &names { let swatches: Vec<&Swatch> = match d.swatch_groups.iter().find(|g| g.name == *n) { Some(g) => g.swatches.iter().collect(), - None => vec![d.swatch(n).ok_or_else(|| bad(C, format!("no swatch or colour group `{n}`")))?], + None => vec![by_name.get(n.as_str()).copied().ok_or_else(|| bad(C, format!("no swatch or colour group `{n}`")))?], }; if swatches.iter().any(|w| w.is_reserved()) { return Err(bad(C, format!("`{n}` can't be deleted"))); @@ -258,10 +315,9 @@ fn swatch_delete(s: &mut Session, p: &Value) -> Result { |_: &mut Color, link: &mut Option, _: &mut f32| link.as_ref().is_some_and(|l| global.contains(l)) && link.take().is_some(); let colorized_ok = super::paint::check_colorized(d).is_ok(); let unlinked = s.edit("Delete Swatch", |d, _| { - d.swatch_groups.retain(|g| !names.contains(&g.name)); - for n in &names { - d.remove_swatch(n); - } + let named: HashSet<&str> = names.iter().map(String::as_str).collect(); + d.swatch_groups.retain(|g| !named.contains(g.name.as_str())); + take_swatches(d, &names, |_| true); let unlinked = map_swatch_links(d, &mut unlink); super::paint::check_colorized_kept(colorized_ok, d).map_err(|e| bad(C, e))?; Ok(unlinked) @@ -419,15 +475,9 @@ fn swatch_new_group(s: &mut Session, p: &Value) -> Result { new.extend(art.into_iter().map(|c| (c, to_global))); } let name = d.free_swatch_name(requested.as_deref().unwrap_or("Color Group")); - let mut group = SwatchGroup { name: name.clone(), swatches: vec![] }; // Colour groups hold solid colours only. - for n in &names { - if d.swatch(n).is_some_and(|sw| matches!(sw.paint, Paint::Solid { .. })) - && let Some(sw) = d.remove_swatch(n) - { - group.swatches.push(sw); - } - } + let swatches = take_swatches(d, &names, |sw| matches!(sw.paint, Paint::Solid { .. })); + let mut group = SwatchGroup { name: name.clone(), swatches }; // The global swatches made from the art's colours, which its unlinked paints link to. let mut links: Vec<(Color, String)> = vec![]; let mut free = d.free_swatch_names(); @@ -483,8 +533,9 @@ fn swatch_move(s: &mut Session, p: &Value) -> Result { insert_before(&mut d.swatch_groups, items, at.as_deref(), |g| &g.name); return Ok(()); } + let by_name = swatches_by_name(d); for n in &names { - let sw = d.swatch(n).ok_or_else(|| bad(C, format!("no swatch `{n}` (swatches and colour groups move separately)")))?; + let sw = by_name.get(n.as_str()).ok_or_else(|| bad(C, format!("no swatch `{n}` (swatches and colour groups move separately)")))?; if sw.is_reserved() { return Err(bad(C, format!("`{n}` can't be moved"))); } @@ -503,7 +554,7 @@ fn swatch_move(s: &mut Session, p: &Value) -> Result { } } let at = anchor(list(d, dest), to, &names, |w| &w.name); - let items: Vec = names.iter().filter_map(|n| d.remove_swatch(n)).collect(); + let items = take_swatches(d, &names, |_| true); insert_before(list(d, dest), items, at.as_deref(), |w| &w.name); Ok(()) })?; @@ -955,8 +1006,9 @@ fn swatch_merge(s: &mut Session, p: &Value) -> Result { const C: &str = "swatch.merge"; let names = names_param(p, C)?; let d = &s.doc()?.doc; + let by_name = swatches_by_name(d); for n in &names { - let sw = d.swatch(n).ok_or_else(|| bad(C, format!("no swatch `{n}`")))?; + let sw = by_name.get(n.as_str()).ok_or_else(|| bad(C, format!("no swatch `{n}`")))?; if sw.is_reserved() { return Err(bad(C, format!("`{n}` can't be merged"))); } @@ -970,9 +1022,10 @@ fn swatch_merge(s: &mut Session, p: &Value) -> Result { } let kept = d.swatch(keep).ok_or_else(|| bad(C, format!("no swatch `{keep}`")))?; let (color, link) = (d.linked_color(kept.paint.color().unwrap_or_default(), kept.spot), kept.global.then(|| keep.clone())); + let merged_names: HashSet<&str> = merged.iter().map(String::as_str).collect(); // Tints keep their tint (of the kept colour). let mut relink = |c: &mut Color, l: &mut Option, t: &mut f32| { - if !l.as_ref().is_some_and(|l| merged.contains(l)) { + if !l.as_ref().is_some_and(|l| merged_names.contains(l.as_str())) { return false; } *c = color.tinted(*t); @@ -981,9 +1034,7 @@ fn swatch_merge(s: &mut Session, p: &Value) -> Result { }; let colorized_ok = super::paint::check_colorized(d).is_ok(); let relinked = s.edit("Merge Swatches", |d, _| { - for n in merged { - d.remove_swatch(n); - } + take_swatches(d, merged, |_| true); let relinked = map_swatch_links(d, &mut relink); super::paint::check_colorized_kept(colorized_ok, d).map_err(|e| bad(C, e))?; Ok(relinked) diff --git a/crates/engine/src/tests/swatchcmds.rs b/crates/engine/src/tests/swatchcmds.rs index cbf595d6d..af059e5e3 100644 --- a/crates/engine/src/tests/swatchcmds.rs +++ b/crates/engine/src/tests/swatchcmds.rs @@ -355,6 +355,35 @@ fn new_color_group_from_artwork_lists_each_color_once_with_free_names() { assert_eq!(*doc(&s), before); } +/// New Color Group takes the named solid swatches in the order given. A name listed twice takes +/// the next swatch of that name, found ungrouped first and then in each colour group, and the +/// lists the swatches leave keep their order. Gradients and names no swatch has take nothing. +#[test] +fn new_color_group_takes_named_swatches_in_order_from_every_list() { + let mut s = session(); + let twin = vectorcraft_color::Swatch { name: "Red".into(), paint: Paint::solid(Color::rgb(0.5, 0.0, 0.0)), global: false, spot: false }; + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc) + .swatch_groups + .iter_mut() + .find(|g| g.name == "Brights") + .unwrap() + .swatches + .push(twin.clone()); + let red = doc(&s).swatch("Red").cloned().unwrap(); + let mut ungrouped = names(&s, None); + let mut brights = names(&s, Some("Brights")); + let r = run(&mut s, "swatch.newGroup", json!({"name": "Mine", "swatches": ["Red", "Orange", "Sunset", "Red", "Missing", "Bright Red"]})); + assert_eq!(r["swatches"], json!(["Red", "Orange", "Red", "Bright Red"])); + let group = &doc(&s).swatch_groups.iter().find(|g| g.name == "Mine").unwrap().swatches; + assert_eq!(group[0], red); + assert_eq!(group[2], twin); + ungrouped.retain(|n| n != "Red" && n != "Orange"); + brights.retain(|n| n != "Red" && n != "Bright Red"); + assert_eq!(names(&s, None), ungrouped); + assert_eq!(names(&s, Some("Brights")), brights); + assert!(doc(&s).swatch("Sunset").is_some()); +} + #[test] fn edit_color_group_numbers_extra_colors_after_the_names_it_keeps() { let mut s = session();