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