Swatches: importing an .ase file numbers repeated group names in linear time (#5087)

swatches.import named each imported group with unique_group, which
compares a candidate name with the library's groups one by one and
tries "Name", "Name 1", "Name 2" and so on until one is free. Groups
without a name all take the import's group name, so a file of n
unnamed groups cost about n^3/6 string comparisons. In a test build,
importing a file of 9,994 unnamed groups took about 4.5 minutes.

The import now builds the set of the library's group names once and
keeps the next number to try for each repeated name, as it already
does for swatch names. Group numbers still start at 1, and groups get
the same names as before.

unnamed_groups_are_numbered_in_linear_time imports such a file on a
thread of its own and fails when the import takes more than 20
seconds. repeated_group_names_take_the_first_free_number checks the
names when a group in the file has a name that the import already gave
to an earlier group.
This commit is contained in:
.CooCoo authored and GitHub committed 2026-10-13 12:47:17 -04:00
1 parent ef8dc9ea30
commit 0a5cc0d1a0
2 files changed
+55 -1

No files matched your search

+11 -1
View File
@@ -600,8 +600,18 @@ fn import(s: &mut Session, p: &Value) -> Result<Value> {
// Names stay unique across the library (agents address swatches by name).
let mut next: std::collections::HashMap<String, usize> = std::collections::HashMap::new();
let mut taken: std::collections::HashSet<String> = lib.iter().flat_map(|g| g.items.iter().map(|i| i.name.clone())).collect();
// Group names are numbered from 1 as in `unique_group` ("Grays 1"), and the numbering
// resumes per base name.
let mut group_next: std::collections::HashMap<String, usize> = std::collections::HashMap::new();
let mut group_taken: std::collections::HashSet<String> = lib.iter().map(|g| g.name.clone()).collect();
for mut g in groups {
g.name = unique_group(&lib, &g.name);
if group_taken.contains(&g.name) {
let from = group_next.get(&g.name).copied().unwrap_or(1);
let (i, n) = (from..).map(|i| (i, format!("{} {i}", g.name))).find(|(_, n)| !group_taken.contains(n)).unwrap_or((from, g.name.clone()));
group_next.insert(g.name.clone(), i + 1);
g.name = n;
}
group_taken.insert(g.name.clone());
for sw in &mut g.items {
if taken.contains(&sw.name) {
// Resume numbering per base name, so a file of identical names stays linear.
+44
View File
@@ -291,6 +291,50 @@ fn library_limits() {
assert_eq!(g[0].name.chars().count(), MAX_NAME);
}
/// Groups without a name take the import's group name. The import used to number each repeated
/// group name by searching the library once for every number it tried, so importing about 10,000
/// unnamed groups took minutes. The test fails when the import takes more than 20 seconds.
#[test]
fn unnamed_groups_are_numbered_in_linear_time() {
let (tx, rx) = std::sync::mpsc::channel();
std::thread::spawn(move || {
let mut s = Session::new();
s.execute("swatches.newGroup", json!({"name": "Ink 2"})).unwrap();
let n = MAX_GROUPS - s.presets.swatches.len();
let paper = AseSwatch { name: "Paper".into(), color: AseColor::Gray(1.0), kind: AseKind::Process };
let mut entries = vec![AseEntry::Group { name: String::new(), swatches: Vec::new() }; n];
entries[0] = AseEntry::Group { name: String::new(), swatches: vec![paper] };
let file = b64_encode(&photocraft_psd::ase::write(&AseFile { entries }).unwrap());
let r = s.execute("swatches.import", json!({"data": file, "group": "Ink"})).unwrap();
assert_eq!(s.presets.swatches.len(), MAX_GROUPS);
let names: Vec<String> = r["groups"].as_array().unwrap().iter().map(|v| v.as_str().unwrap().to_string()).collect();
let _ = tx.send(names);
});
let names = match rx.recv_timeout(std::time::Duration::from_secs(20)) {
Ok(names) => names,
Err(std::sync::mpsc::RecvTimeoutError::Timeout) => panic!("importing the groups did not finish"),
Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => panic!("the import thread panicked"),
};
// Numbering starts at 1 and skips the number the library already uses.
assert_eq!(names[..4], ["Ink", "Ink 1", "Ink 3", "Ink 4"]);
assert_eq!(names.last(), Some(&format!("Ink {}", names.len())));
assert_eq!(names.iter().collect::<std::collections::HashSet<_>>().len(), names.len());
}
#[test]
fn repeated_group_names_take_the_first_free_number() {
let mut s = Session::new();
for name in ["Brand", "Brand 2"] {
s.execute("swatches.newGroup", json!({"name": name})).unwrap();
}
let paper = AseSwatch { name: "Paper".into(), color: AseColor::Gray(1.0), kind: AseKind::Process };
let entries = ["Brand", "Brand", "Brand", "Brand 1"].into_iter().map(|name| AseEntry::Group { name: name.into(), swatches: vec![paper.clone()] }).collect();
let file = b64_encode(&photocraft_psd::ase::write(&AseFile { entries }).unwrap());
let r = s.execute("swatches.import", json!({"data": file})).unwrap();
// The file's "Brand 1" comes after the first "Brand" took that name.
assert_eq!(r["groups"], json!(["Brand 1", "Brand 3", "Brand 4", "Brand 1 1"]));
}
/// A value beyond f32 range used to pass the finite check, become inf as f32 and wrap to a NaN
/// hue, which the prefs file stores as `null`; the next launch then dropped every saved preset.
#[test]