Merge pull request #6691 from dotCooCoo/fix/color-group-takes-swatches-once

Swatches: New Color Group, Delete, Move and Merge take named swatches in one pass
This commit is contained in:
BFlat
2026-11-12 09:13:38 +07:00
committed by GitHub
2 changed files with 103 additions and 23 deletions
+74 -23
View File
@@ -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<Value> {
/// The `names` list and `name` parameters, without repeats; at least one is required.
pub(super) fn names_param(p: &Value, cmd: &str) -> Result<Vec<String>> {
let mut seen: HashSet<String> = HashSet::new();
let mut names: Vec<String> = 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<Vec<String>> {
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<Swatch> {
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<usize> = 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<Swatch>| {
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<Value> {
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<String> = vec![];
let mut global: HashSet<String> = 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<Value> {
|_: &mut Color, link: &mut Option<String>, _: &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)
@@ -420,15 +476,9 @@ fn swatch_new_group(s: &mut Session, p: &Value) -> Result<Value> {
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();
@@ -484,8 +534,9 @@ fn swatch_move(s: &mut Session, p: &Value) -> Result<Value> {
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")));
}
@@ -504,7 +555,7 @@ fn swatch_move(s: &mut Session, p: &Value) -> Result<Value> {
}
}
let at = anchor(list(d, dest), to, &names, |w| &w.name);
let items: Vec<Swatch> = 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(())
})?;
@@ -956,8 +1007,9 @@ fn swatch_merge(s: &mut Session, p: &Value) -> Result<Value> {
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")));
}
@@ -971,9 +1023,10 @@ fn swatch_merge(s: &mut Session, p: &Value) -> Result<Value> {
}
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<String>, 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);
@@ -982,9 +1035,7 @@ fn swatch_merge(s: &mut Session, p: &Value) -> Result<Value> {
};
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)
+29
View File
@@ -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();