diff --git a/crates/engine/src/cmd/atn_cmds/steps_channels.rs b/crates/engine/src/cmd/atn_cmds/steps_channels.rs index 37c4c0bf567..a11e199efb5 100644 --- a/crates/engine/src/cmd/atn_cmds/steps_channels.rs +++ b/crates/engine/src/cmd/atn_cmds/steps_channels.rs @@ -67,15 +67,16 @@ fn pixels(v: &D) -> Option { } } -/// 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> /// 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 { 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; diff --git a/crates/engine/src/cmd/atn_cmds/tests.rs b/crates/engine/src/cmd/atn_cmds/tests.rs index 9f74da7b925..fd7295d5008 100644 --- a/crates/engine/src/cmd/atn_cmds/tests.rs +++ b/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"); + } +} diff --git a/crates/engine/src/cmd/channel_cmds.rs b/crates/engine/src/cmd/channel_cmds.rs index 49a45cba6a0..3ee57c8abbe 100644 --- a/crates/engine/src/cmd/channel_cmds.rs +++ b/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, diff --git a/crates/engine/src/cmd/channel_cmds/spot.rs b/crates/engine/src/cmd/channel_cmds/spot.rs index 773089faad6..f2aa4a6e34b 100644 --- a/crates/engine/src/cmd/channel_cmds/spot.rs +++ b/crates/engine/src/cmd/channel_cmds/spot.rs @@ -55,9 +55,21 @@ pub(super) fn merge_spot(s: &mut Session, p: &Value) -> Result { 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 { 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)?; diff --git a/crates/engine/src/cmd/channel_cmds/tests.rs b/crates/engine/src/cmd/channel_cmds/tests.rs index 079b2e4c86d..a640a7b45b7 100644 --- a/crates/engine/src/cmd/channel_cmds/tests.rs +++ b/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); +}