Integration: Trap round trip, Match Color source None, Split Channels only on single-layer images (#10630)

- A Trap step recorded without Wdth loaded as {} and saved back as Wdth 1 px,
  so it reloaded as {"width":1}. It now loads as width 1 (what the command
  plays), so load -> save -> load gives the same step.
- The Match Color dialog records Source "None" as "source": -1, which the
  export refused because "source" wasn't an allowed key, so a Match Color
  recorded in PhotoCraft could never be saved. A null or negative source
  (and a sourceLayer with it) is now written as the source-less event; a
  real source (>= 0) is still refused.
- Split Channels closes the original, but split and its enabled check never
  checked the layer count, so a played SplC split and closed a layered
  document with no undo. Like the reference application it now refuses
  (Err, document untouched and open) a document with more than one layer
  (a hidden one counts) or a lone group; a Background-only image or a lone
  normal layer still splits.

Co-authored-by: hectorviov <1501713+hectorviov@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/atn_cmds/steps_channels.rs

@@ -67,15 +67,16 @@ fn pixels(v: &D) -> Option<f64> {
     }
 }
 
-/// Trap (`Trap`): `Wdth` in pixels. A width that isn't a number of pixels, or is below
-/// `MIN_TRAP`, keeps the Photoshop step.
+/// Trap (`Trap`): `Wdth` in pixels. A step without `Wdth` plays the command's 1 px, written
+/// out so it saves back as the width it plays. A width that isn't a number of pixels, or is
+/// below `MIN_TRAP`, keeps the Photoshop step.
 pub(super) fn trap_from(d: &Descriptor) -> Mapped {
     if !only_keys(d, &["Wdth"]) {
         return Mapped::Unsupported { harmless: false };
     }
     match setting(d, "Wdth", |v| pixels(v).map(Some), None) {
         Some(Some(w)) if w >= MIN_TRAP => step("image.trap", json!({"width": w.min(MAX_TRAP)})),
-        Some(None) => step("image.trap", json!({})),
+        Some(None) => step("image.trap", json!({"width": MIN_TRAP})),
         _ => Mapped::Unsupported { harmless: false },
     }
 }
@@ -128,11 +129,15 @@ fn match_color_params(d: &Descriptor) -> Option<serde_json::Map<String, Value>>
 
 /// Every key in recorded order. A source document or layer, Neutralize or a selection option
 /// has no key in the event, so such a step is refused; the integers refuse a fraction or a
-/// value outside the command's range, which it plays clamped.
+/// value outside the command's range, which it plays clamped. A negative `source` is the
+/// dialog's Source "None", which the command plays without a source, so it is written as one
+/// (as is a `sourceLayer` with it, which the command then ignores).
 pub(super) fn match_color_to(_: &str, p: &Value) -> Option<Descriptor> {
     let off = |k: &str| p.get(k).is_none_or(|v| v.is_null() || *v == Value::Bool(false));
-    if !pixel_target(p)
-        || !sets_only(p, &["target", "luminance", "intensity", "fade", "neutralize", "useSelectionInSource", "useSelectionInTarget"])
+    let no_source = p.get("source").is_none_or(|v| v.is_null() || v.as_i64().is_some_and(|i| i < 0));
+    if !no_source
+        || !pixel_target(p)
+        || !sets_only(p, &["target", "source", "sourceLayer", "luminance", "intensity", "fade", "neutralize", "useSelectionInSource", "useSelectionInTarget"])
         || !["neutralize", "useSelectionInSource", "useSelectionInTarget"].iter().all(|k| off(k))
     {
         return None;

crates/engine/src/cmd/atn_cmds/tests.rs

@@ -3388,3 +3388,36 @@ fn match_color_with_a_source_is_kept() {
         }
     }
 }
+
+/// A Trap step recorded without `Wdth` plays the command's 1 px and saves back as `Wdth` 1 px,
+/// so it must load as that width too: load, save and load give the same step.
+#[test]
+fn trap_step_without_a_width_saves_back_the_same() {
+    let file = AtnFile {
+        name: UnicodeString::new_nul("S"),
+        actions: vec![AtnAction { name: UnicodeString::new_nul("T"), items: vec![item("Trap", "Trap", Some(desc("null")))], ..Default::default() }],
+        ..Default::default()
+    };
+    let mut s = Session::new();
+    s.execute("actions.load", json!({"data": b64(&atn::write(&file).unwrap())})).unwrap();
+    let loaded = s.actions.list.clone();
+    assert_eq!(loaded[0].steps, vec![("image.trap".to_string(), json!({"width": 1}))]);
+    let saved = s.execute("actions.save", json!({"set": "S"})).unwrap();
+    let mut t = Session::new();
+    t.execute("actions.load", json!({"data": saved["data"].as_str().unwrap()})).unwrap();
+    assert_eq!(t.actions.list, loaded);
+}
+
+/// The Match Color dialog records Source "None" as `"source": -1`, which the command plays
+/// without a source: such a step saves, while a real source document is still refused.
+#[test]
+fn match_color_recorded_with_source_none_saves() {
+    let none = json!({"source": -1, "luminance": 100.0, "intensity": 100.0, "fade": 0.0, "neutralize": false, "useSelectionInSource": false, "useSelectionInTarget": false});
+    let d = export_item("image.adjustments.matchColor", &none, "S").unwrap().descriptor.unwrap();
+    let expected = desc("null").with("Lght", D::Integer(100)).with("ClrR", D::Integer(100)).with("Fade", D::Integer(0)).with("noReference", D::Boolean(true));
+    assert_eq!(d.items, expected.items);
+    assert!(export_item("image.adjustments.matchColor", &json!({"source": -1, "sourceLayer": 4}), "S").is_some());
+    for source in [json!({"source": 0}), json!({"source": 0, "sourceLayer": 4}), json!({"source": "x"}), json!({"source": -1, "neutralize": true})] {
+        assert!(export_item("image.adjustments.matchColor", &source, "S").is_none(), "{source} exported");
+    }
+}

crates/engine/src/cmd/channel_cmds.rs

@@ -46,7 +46,13 @@ fn has_spot(s: &Session) -> std::result::Result<(), String> {
 }
 fn can_split(s: &Session) -> std::result::Result<(), String> {
     let d = s.active().ok_or("no document open")?;
-    if color_count(&d.doc) + d.doc.channels.len() < 2 { Err("only one channel to split".into()) } else { Ok(()) }
+    if !is_flat(&d.doc) {
+        Err(spot::NOT_FLAT.into())
+    } else if color_count(&d.doc) + d.doc.channels.len() < 2 {
+        Err("only one channel to split".into())
+    } else {
+        Ok(())
+    }
 }
 fn can_merge(s: &Session) -> std::result::Result<(), String> {
     has_doc(s)?;
@@ -122,7 +128,7 @@ pub use planes::{
 pub(crate) use refs::source_spec;
 pub use refs::{Blending, ChanRef, parse_ref};
 use selection::{load_selection, quick_mask, save_selection};
-use spot::{merge, merge_spot, mergeable, split};
+use spot::{is_flat, merge, merge_spot, mergeable, split};
 use targeting::{SLOT_KEY, target_cmd, target_slot};
 pub(crate) use targeting::{
     Target, channel_surface_for_filter, clear_channel, clear_color_channel, color_restriction, edits_channel, filter_mask_layer, inject_target, invert_region,

crates/engine/src/cmd/channel_cmds/spot.rs

@@ -55,9 +55,21 @@ pub(super) fn merge_spot(s: &mut Session, p: &Value) -> Result<Value> {
     Ok(Value::Null)
 }
 
+/// Whether `doc` has a single layer (a Background or a normal one; a group counts as a layer,
+/// so a lone group is refused too). As in the reference application only such an image can be
+/// split: the split closes the original, so a layered one would lose its layers.
+pub(super) fn is_flat(doc: &Document) -> bool {
+    matches!(doc.layers.as_slice(), [l] if l.children().is_none())
+}
+
+pub(super) const NOT_FLAT: &str = "only a single-layer image can be split: flatten the image first";
+
 /// Split Channels: one grayscale document per colour and alpha channel; closes the original.
 pub(super) fn split(s: &mut Session, p: &Value) -> Result<Value> {
     let idx = s.active_index().ok_or(EngineError::NoDocument)?;
+    if !s.documents().get(idx).is_some_and(|d| is_flat(&d.doc)) {
+        return Err(bad("channel.split", NOT_FLAT));
+    }
     let doc = s.documents()[idx].doc.clone();
     let area = doc.bounds();
     let (px, n) = native_pixels(&doc, None)?;

crates/engine/src/cmd/channel_cmds/tests.rs

@@ -1185,3 +1185,37 @@ fn layers_panel_mask_target_isolates_the_curve_channel() {
     let row = isolated_curve_channel_for(s.active().unwrap(), true).expect("quick mask wins");
     assert_eq!((row.name.as_str(), row.gray), ("Quick Mask", Some(GraySurface::QuickMask)));
 }
+
+/// Split Channels closes the original, so as in the reference application it splits only an
+/// image with a single layer: two layers (even with one hidden) or a lone group are refused and
+/// the document stays open with its layers; a flattened image or a lone normal layer still
+/// splits (a played `SplC` step goes through the same check).
+#[test]
+fn split_refuses_an_image_with_more_than_one_layer() {
+    let mut s = session();
+    s.execute("edit.fill", json!({"color": "#ff8000"})).unwrap();
+    s.execute("layer.new.layer", json!({})).unwrap();
+    assert!(!s.is_enabled("channel.split"));
+    let err = s.execute("channel.split", json!({})).unwrap_err();
+    assert!(err.to_string().contains("flatten"), "{err}");
+    assert_eq!(s.documents().len(), 1, "the layered document stays open");
+    assert_eq!(doc(&s).layers.len(), 2, "with its layers");
+    std::sync::Arc::make_mut(&mut s.active_mut().unwrap().doc).layers[1].visible = false;
+    assert!(s.execute("channel.split", json!({})).is_err(), "a hidden second layer still counts");
+    assert_eq!((s.documents().len(), doc(&s).layers.len()), (1, 2));
+    s.execute("layer.flattenImage", json!({})).unwrap();
+    assert!(s.is_enabled("channel.split"));
+    assert_eq!(s.execute("channel.split", json!({})).unwrap()["documents"].as_array().unwrap().len(), 3);
+    // A lone normal layer (Layer 0) splits too.
+    let mut s = session();
+    s.execute("layer.new.layerFromBackground", json!({})).unwrap();
+    assert!(!doc(&s).is_background(doc(&s).layers[0].id));
+    assert!(s.is_enabled("channel.split"));
+    assert_eq!(s.execute("channel.split", json!({})).unwrap()["documents"].as_array().unwrap().len(), 3);
+    // A lone group is a layer the split would lose.
+    let mut s = session();
+    std::sync::Arc::make_mut(&mut s.active_mut().unwrap().doc).layers = vec![Layer::group("Group 1", vec![])];
+    assert!(!s.is_enabled("channel.split"));
+    assert!(s.execute("channel.split", json!({})).is_err());
+    assert_eq!(s.documents().len(), 1);
+}