Fix imported gradient presets sharing selection names (#10521)

* Fix duplicate gradient preset names on import and restore

* Preserve addressable gradient names during duplicate migration

* Preserve gradient identities during import and restore

* Assert restored selected gradient retains its stops

* Pass replace in the duplicate gradient names test after #10640

Main now imports a re-imported .grd beside its group unless replace is set
(#10640). The test re-imports over the same group, so ask for that.

* Use the same replace default for gradient name allocation and group insertion

After #10640 the group insertion defaults replace to false, but the name
allocation still defaulted it to true, so a default re-import created a
sibling group with the same preset names. Resolve replace once and add a
regression for the default and explicit replace paths.

---------

Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com>

crates/engine/src/cmd/preset_import_cmds.rs

@@ -59,21 +59,26 @@ fn file_bytes_within(p: &Value, cmd: &str, what: &str, max: u64) -> Result<(Vec<
 }
 
 /// The preset names in use, for naming new presets.
-struct UniqueNames {
-    /// Every name in use, ASCII-lowercased.
+pub(crate) struct UniqueNames {
+    /// Every name in use, normalized according to the comparison mode.
     taken: std::collections::HashSet<String>,
-    /// The next number to try for each repeated name (lowercased).
+    /// The next number to try for each normalized repeated name.
     next: std::collections::HashMap<String, usize>,
+    exact: bool,
 }
 
 impl UniqueNames {
-    fn new<'a>(names: impl IntoIterator<Item = &'a str>) -> Self {
-        UniqueNames { taken: names.into_iter().map(str::to_ascii_lowercase).collect(), next: std::collections::HashMap::new() }
+    pub(crate) fn new<'a>(names: impl IntoIterator<Item = &'a str>) -> Self {
+        UniqueNames { taken: names.into_iter().map(str::to_ascii_lowercase).collect(), next: std::collections::HashMap::new(), exact: false }
     }
 
-    /// A name not in use (case-insensitive): `name`, `name 2`, `name 3`… It is in use from then on.
-    fn unique(&mut self, name: &str) -> String {
-        let key = name.to_ascii_lowercase();
+    pub(crate) fn new_exact<'a>(names: impl IntoIterator<Item = &'a str>) -> Self {
+        UniqueNames { taken: names.into_iter().map(str::to_string).collect(), next: std::collections::HashMap::new(), exact: true }
+    }
+
+    /// A name not in use under the selected comparison mode: `name`, `name 2`, `name 3`… It is in use from then on.
+    pub(crate) fn unique(&mut self, name: &str) -> String {
+        let key = if self.exact { name.to_string() } else { name.to_ascii_lowercase() };
         if self.taken.insert(key.clone()) {
             return name.to_string();
         }
@@ -348,6 +353,14 @@ fn import_grd(s: &mut Session, p: &Value) -> Result<Value> {
         None => "Imported Gradients".to_string(),
     };
     let grads = photocraft_psd::grd::parse(&bytes).map_err(|e| bad(cmd, format!("not a readable gradient file (.grd): {e}")))?;
+    // Same default as the group insertion below and the documented contract (#10640).
+    let replace = p.get("replace").and_then(Value::as_bool).unwrap_or(false);
+    let mut taken =
+        UniqueNames::new_exact(s.presets.gradients.iter().filter(|g| !replace || g.name != group).flat_map(|g| g.items.iter().map(|i| i.name.as_str())));
+    let original_names: Vec<String> =
+        grads.iter().enumerate().map(|(i, g)| if g.name.trim().is_empty() { format!("Gradient {}", i + 1) } else { g.name.trim().to_string() }).collect();
+    let mut reserved = taken.taken.clone();
+    reserved.extend(original_names.iter().cloned());
     let mut warnings = Vec::new();
     let mut items = Vec::new();
     let (mut noise, mut midpoints) = (0, false);
@@ -365,6 +378,15 @@ fn import_grd(s: &mut Session, p: &Value) -> Result<Value> {
             opacity.clear();
         }
         let name = if g.name.trim().is_empty() { format!("Gradient {}", i + 1) } else { g.name.trim().to_string() };
+        let name = if taken.taken.insert(name.clone()) {
+            name
+        } else {
+            std::mem::swap(&mut taken.taken, &mut reserved);
+            let unique = taken.unique(&name);
+            std::mem::swap(&mut taken.taken, &mut reserved);
+            taken.taken.insert(unique.clone());
+            unique
+        };
         items.push(GradientPreset { name, stops, opacity });
     }
     if noise > 0 {
@@ -379,7 +401,7 @@ fn import_grd(s: &mut Session, p: &Value) -> Result<Value> {
     let names: Vec<String> = items.iter().map(|g| g.name.clone()).collect();
     let st = &mut s.presets;
     // As with brushes: a new group beside one of the same name unless `replace` (#10640).
-    let group = if p.get("replace").and_then(Value::as_bool).unwrap_or(false) {
+    let group = if replace {
         st.gradients.retain(|g| g.name != group);
         group
     } else {
@@ -502,6 +524,104 @@ mod tests {
         assert_eq!(s.tools.presets.len(), n);
     }
 
+    #[test]
+    fn duplicate_gradient_names_remain_independently_selectable() {
+        use photocraft_psd::grd::{GrdColor, GrdGradient, GrdStop, write};
+        let gradients: Vec<_> = [0.2, 0.8]
+            .into_iter()
+            .map(|red| GrdGradient {
+                name: "Duplicate".into(),
+                stops: vec![
+                    GrdStop { location: 0.0, midpoint: 0.5, color: GrdColor::Rgb([red, 0.0, 0.0]) },
+                    GrdStop { location: 1.0, midpoint: 0.5, color: GrdColor::Rgb([1.0, 1.0, 1.0]) },
+                ],
+                ..Default::default()
+            })
+            .collect();
+        let data = photocraft_paint::tile::b64_encode(&write(&gradients));
+        let mut s = Session::new();
+        // Re-importing over the group replaces it (`replace`, #10640), so it keeps the same names.
+        for _ in 0..2 {
+            let result = s.execute("gradient.presets.importGrd", json!({"data": data, "group": "Duplicates", "replace": true})).unwrap();
+            assert_eq!(result["imported"], json!(["Duplicate", "Duplicate 2"]));
+        }
+        s.execute("gradient.presets.select", json!({"preset": "Duplicate"})).unwrap();
+        let first = s.presets.gradient.clone();
+        s.execute("gradient.presets.select", json!({"preset": "Duplicate 2"})).unwrap();
+        assert_ne!(first.stops, s.presets.gradient.stops);
+        let mut saved = s.presets.to_json(&s);
+        let groups = saved["gradients"].as_array_mut().unwrap();
+        let group = groups.iter_mut().find(|g| g["name"] == "Duplicates").unwrap();
+        group["items"][1]["name"] = json!("Duplicate");
+        s.load_presets_json(saved);
+        let restored = s.presets.gradients.iter().find(|g| g.name == "Duplicates").unwrap();
+        assert_eq!(restored.items[1].name, "Duplicate 2");
+        assert_eq!(s.presets.gradient.name, "Duplicate 2");
+        let result = s.execute("gradient.presets.importGrd", json!({"data": data, "group": "Other", "replace": false})).unwrap();
+        assert_eq!(result["imported"], json!(["Duplicate 3", "Duplicate 4"]));
+    }
+
+    /// Re-importing without `replace` (the default) adds a sibling group, so its presets must get
+    /// names distinct from the existing group's, keeping name-only selection unambiguous.
+    #[test]
+    fn default_gradient_reimport_keeps_names_unique_across_sibling_groups() {
+        use photocraft_psd::grd::{GrdColor, GrdGradient, GrdStop, write};
+        let gradients = vec![GrdGradient {
+            name: "Sunset".into(),
+            stops: vec![
+                GrdStop { location: 0.0, midpoint: 0.5, color: GrdColor::Rgb([1.0, 0.0, 0.0]) },
+                GrdStop { location: 1.0, midpoint: 0.5, color: GrdColor::Rgb([1.0, 1.0, 1.0]) },
+            ],
+            ..Default::default()
+        }];
+        let data = photocraft_paint::tile::b64_encode(&write(&gradients));
+        let mut s = Session::new();
+        let first = s.execute("gradient.presets.importGrd", json!({"data": data, "group": "Sky"})).unwrap();
+        assert_eq!(first["imported"], json!(["Sunset"]));
+        let second = s.execute("gradient.presets.importGrd", json!({"data": data, "group": "Sky"})).unwrap();
+        assert_eq!(second["group"], json!("Sky (2)"));
+        assert_eq!(second["imported"], json!(["Sunset 2"]));
+        // Explicit `replace` drops the same-named group, so the original name is free again.
+        let third = s.execute("gradient.presets.importGrd", json!({"data": data, "group": "Sky", "replace": true})).unwrap();
+        assert_eq!(third["group"], json!("Sky"));
+        assert_eq!(third["imported"], json!(["Sunset"]));
+    }
+
+    #[test]
+    fn gradient_import_preserves_original_and_case_distinct_names() {
+        use photocraft_psd::grd::{GrdColor, GrdGradient, GrdStop, write};
+        let gradients: Vec<_> = ["Foo", "Foo", "Foo 2", "FOO"]
+            .into_iter()
+            .enumerate()
+            .map(|(i, name)| GrdGradient {
+                name: name.into(),
+                stops: vec![
+                    GrdStop { location: 0.0, midpoint: 0.5, color: GrdColor::Rgb([i as f32 / 4.0, 0.0, 0.0]) },
+                    GrdStop { location: 1.0, midpoint: 0.5, color: GrdColor::Rgb([1.0, 1.0, 1.0]) },
+                ],
+                ..Default::default()
+            })
+            .collect();
+        let mut s = Session::new();
+        let result = s
+            .execute(
+                "gradient.presets.importGrd",
+                json!({
+                    "data": photocraft_paint::tile::b64_encode(&write(&gradients)), "group": "Names"
+                }),
+            )
+            .unwrap();
+        assert_eq!(result["imported"], json!(["Foo", "Foo 3", "Foo 2", "FOO"]));
+        for (name, red) in [("Foo", 0.0), ("Foo 3", 0.25), ("Foo 2", 0.5), ("FOO", 0.75)] {
+            s.execute("gradient.presets.select", json!({"preset": name})).unwrap();
+            let current = s.presets.gradient.clone();
+            assert_eq!(current.name, name);
+            assert_eq!(current.stops[0].1, crate::presets::gradients::StopColor::Rgb([red, 0.0, 0.0]));
+        }
+        let mut tools = UniqueNames::new(["Foo"]);
+        assert_eq!(tools.unique("FOO"), "FOO 2");
+    }
+
     #[test]
     fn grd_import_adds_a_gradient_group() {
         use crate::presets::gradients::StopColor;

crates/engine/src/presets.rs

@@ -200,12 +200,39 @@ impl Session {
             contours: take(&mut m, "contours"),
         };
         let st = &mut self.presets;
-        if let Some(g) = p.gradients {
-            st.gradients = g;
-        }
         if let Some(g) = p.gradient {
             st.gradient = g;
         }
+        let saved_current = st.gradient.clone();
+        if let Some(g) = p.gradients {
+            st.gradients = g;
+            // Older imports could contain duplicate names, which alias panel selections.
+            // Reserve all original names so a duplicate cannot steal a later preset's name.
+            // Lookup is case-sensitive: Foo and FOO are already independently addressable.
+            let mut reserved: std::collections::HashSet<String> =
+                st.gradients.iter().flat_map(|group| group.items.iter().map(|item| item.name.clone())).collect();
+            let mut seen = std::collections::HashSet::new();
+            let mut next = std::collections::HashMap::<String, usize>::new();
+            for group in &mut st.gradients {
+                for item in &mut group.items {
+                    if seen.insert(item.name.clone()) {
+                        continue;
+                    }
+                    let suffix = next.entry(item.name.clone()).or_insert(2);
+                    loop {
+                        let candidate = format!("{} {}", item.name, suffix);
+                        *suffix = suffix.saturating_add(1);
+                        if reserved.insert(candidate.clone()) {
+                            if *item == saved_current {
+                                st.gradient.name = candidate.clone();
+                            }
+                            item.name = candidate;
+                            break;
+                        }
+                    }
+                }
+            }
+        }
         if let Some(g) = p.styles {
             st.styles = g;
         }

crates/engine/src/presets/tests.rs

@@ -879,6 +879,37 @@ fn a_damaged_preset_set_keeps_the_other_sets() {
     assert_eq!(next.presets.export_presets, s.presets.export_presets);
 }
 
+#[test]
+fn restoring_duplicate_gradient_names_preserves_addressable_presets() {
+    use crate::presets::Group;
+    use crate::presets::gradients::GradientPreset;
+
+    let mut s = Session::new();
+    let presets = vec![
+        Group::new("First", vec![GradientPreset::new("Foo", &["#ff0000", "#000000"]), GradientPreset::new("Foo", &["#00ff00", "#000000"])]),
+        Group::new("Second", vec![GradientPreset::new("Foo 2", &["#0000ff", "#000000"]), GradientPreset::new("FOO", &["#ffff00", "#000000"])]),
+    ];
+    s.presets.gradient = presets[0].items[1].clone();
+    s.presets.gradients = presets;
+    let saved = s.presets.to_json(&s);
+    let mut restored = Session::new();
+    restored.load_presets_json(saved.clone());
+    let names: Vec<String> = restored.presets.gradients.iter().flat_map(|g| g.items.iter().map(|p| p.name.clone())).collect();
+    assert_eq!(names, ["Foo", "Foo 3", "Foo 2", "FOO"]);
+    assert_eq!(restored.presets.gradient.name, "Foo 3");
+    assert_eq!(restored.presets.gradient.stops, GradientPreset::new("", &["#00ff00", "#000000"]).stops);
+    for (name, color) in [("Foo 2", "#0000ff"), ("FOO", "#ffff00")] {
+        restored.execute("gradient.presets.select", json!({"preset": name})).unwrap();
+        assert_eq!(restored.presets.gradient.stops, GradientPreset::new("", &[color, "#000000"]).stops);
+    }
+    let current = restored.presets.gradient.clone();
+    let once = restored.presets.to_json(&restored);
+    restored.load_presets_json(once);
+    assert_eq!(restored.presets.gradient, current);
+    let names: Vec<String> = restored.presets.gradients.iter().flat_map(|g| g.items.iter().map(|p| p.name.clone())).collect();
+    assert_eq!(names, ["Foo", "Foo 3", "Foo 2", "FOO"]);
+}
+
 // ------------------------------------------------------------------ export presets
 
 #[test]