Port #8659: keep name, blend mode and clipping on adjustment-layer Make steps

An adjustment layer's Make step now loads its `Nm  `, `Md  ` and `Grup`
keys as the command's `name`, `mode` and `clipToPrevious` params and saves
them back. An unreadable value keeps the step as an unsupported kept step;
`layer.newAdjustmentLayer.*` validates the new params like New Layer and
clips through the clip-group rules. Fixes the remainder of #8384.

Co-Authored-By: bommerts <44821497+bommerts@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd.rs

@@ -1218,7 +1218,7 @@ fn build() -> Vec<CommandSpec> {
             label,
             menu: &["Layer", "New Adjustment Layer"],
             shortcut: None,
-            params: Box::leak(format!(r#"{params} + {{{NEW_LAYER_PATH},"opacity":0..1=1}}"#).into_boxed_str()),
+            params: Box::leak(format!(r#"{params} + {{{NEW_LAYER_PATH},"opacity":0..1=1,"name":str?=the adjustment's name + number,"mode":blend mode name="normal" (not Pass Through),"clipToPrevious":bool=false}}"#).into_boxed_str()),
             enabled: has_doc,
             run: |s, p| {
                 let kind = p.get("__kind").and_then(Value::as_str).unwrap_or("invert").to_string();

crates/engine/src/cmd/adjust_params.rs

@@ -1,3 +1,5 @@
+use photocraft_doc::BlendMode;
+
 use super::enabled::{check_all_unlocked, is_mask_target, paint_surface};
 use super::layer_params::{layer_param, mask_new_layer};
 use super::{Adjustment, ColorMode, EngineError, Layer, LayerContent, Map, Result, Session, Value, json, pixels};
@@ -12,13 +14,32 @@ pub(super) fn new_adjustment(s: &mut Session, adj: Adjustment, p: &Value, cmd: &
             .filter(|o| o.is_finite() && (0.0..=1.0).contains(o))
             .ok_or_else(|| EngineError::BadParams { cmd: cmd.into(), msg: "opacity must be a number in 0..1".into() })? as f32,
     };
+    let bad = |msg: String| EngineError::BadParams { cmd: cmd.into(), msg };
+    // Name, blend mode and clipping read like New Layer's (#8384).
+    let name = match p.get("name") {
+        None | Some(Value::Null) => None,
+        Some(v) => Some(v.as_str().ok_or_else(|| bad("`name` must be a string".into()))?.to_string()),
+    };
+    let mode = match p.get("mode") {
+        None | Some(Value::Null) => BlendMode::Normal,
+        Some(v) => {
+            let m = v.as_str().ok_or_else(|| bad("`mode` must be a blend mode name".into()))?;
+            crate::commands::blend_from_str(m).filter(|b| *b != BlendMode::PassThrough).ok_or_else(|| bad(format!("unknown layer blend mode `{m}`")))?
+        }
+    };
+    let clip = match p.get("clipToPrevious") {
+        None | Some(Value::Null) => false,
+        Some(v) => v.as_bool().ok_or_else(|| bad("`clipToPrevious` must be true or false".into()))?,
+    };
     let label = format!("New {} Layer", adj.label());
-    let name = adj.label().to_string();
+    let default_name = adj.label().to_string();
     let id = s.edit(&label, |doc, active| {
-        let mut l = Layer::new(doc.next_layer_name(&name), LayerContent::Adjustment(adj));
+        let mut l = Layer::new(name.clone().unwrap_or_else(|| doc.next_layer_name(&default_name)), LayerContent::Adjustment(adj));
         l.opacity = opacity;
+        l.blend = mode;
         mask_new_layer(doc, &mut l, p, cmd, true)?;
         let id = crate::layer_place::insert_new(doc, *active, l, crate::layer_place::Place::Auto);
+        crate::clip_group::settle(doc, &[id], &Default::default(), clip);
         *active = Some(id);
         Ok(id)
     })?;

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

@@ -241,6 +241,28 @@ pub(super) fn make_from(d: &Descriptor) -> Mapped {
                     // Recorded layer opacity uses 0..255 even in a 16-bit document.
                     params.insert("opacity".into(), ((opacity * 255.0 / 100.0).round() / 255.0).into());
                 }
+                // Name, blend mode and clipping travel on the same command; an unreadable value
+                // keeps the whole step rather than replaying a different layer (#8384).
+                if let Some(u) = using {
+                    let Some((_, Value::Object(params))) = mapped.first_mut() else { return Mapped::Unsupported { harmless: false } };
+                    if u.get("Nm  ").is_some() {
+                        let Some(name) = get_text(u, "Nm  ") else { return Mapped::Unsupported { harmless: false } };
+                        params.insert("name".into(), name.into());
+                    }
+                    if u.get("Md  ").is_some() {
+                        let Some(mode) = get_enum(u, "Md  ").and_then(|m| blend_label(&m)).filter(|m| *m != "Pass Through") else {
+                            return Mapped::Unsupported { harmless: false };
+                        };
+                        params.insert("mode".into(), mode.into());
+                    }
+                    match u.get("Grup") {
+                        None => {}
+                        Some(D::Boolean(clip)) => {
+                            params.insert("clipToPrevious".into(), (*clip).into());
+                        }
+                        Some(_) => return Mapped::Unsupported { harmless: false },
+                    }
+                }
                 if let Some(c) = using.and_then(|u| get_enum(u, "Clr ")) {
                     let Some(color) = lookup(LABEL_COLORS, &c) else { return Mapped::Unsupported { harmless: false } };
                     mapped.push(("layer.setLabelColor".into(), json!({"color": color})));
@@ -451,32 +473,67 @@ pub(super) fn make_to(id: &str, p: &Value) -> Option<Descriptor> {
             n = n.with("Fl  ", enm("Fl  ", fill)).with("Dpth", long(jnum(p, "depth").unwrap_or(8.0)));
             desc("null").with("Nw  ", D::Descriptor(n))
         }
-        _ => {
-            let suffix = id.strip_prefix("layer.newAdjustmentLayer.")?;
-            let a = adjustment_by_suffix(suffix)?;
-            let opacity = match p.get("opacity") {
-                None => None,
-                Some(value) => Some(value.as_f64().filter(|o| o.is_finite() && (0.0..=1.0).contains(o))?),
-            };
-            let mut params = p.clone();
-            if let Some(params) = params.as_object_mut() {
-                params.remove("opacity");
-            }
-            // Layer opacity must not turn default adjustment settings into a custom preset.
-            let t = if params.as_object().is_some_and(|o| o.is_empty()) {
-                desc(a.class).with("presetKind", enm("presetKindType", "presetKindDefault"))
-            } else {
-                (a.to)(&params)?
-            };
-            let mut using = desc("AdjL").with("Type", D::Descriptor(t));
-            if let Some(opacity) = opacity {
-                using = using.with("Opct", pct(recorded_percent(opacity)));
-            }
-            desc("null").with("null", class_ref("AdjL")).with("Usng", D::Descriptor(using))
-        }
+        _ => return adjustment_layer_to(id, p),
     })
 }
 
+/// `layer.newAdjustmentLayer.*` as a Make step: the adjustment's settings under `Type`, the new
+/// layer's opacity, name, blend mode and clipping beside it.
+fn adjustment_layer_to(id: &str, p: &Value) -> Option<Descriptor> {
+    let suffix = id.strip_prefix("layer.newAdjustmentLayer.")?;
+    let a = adjustment_by_suffix(suffix)?;
+    let opacity = match p.get("opacity") {
+        None => None,
+        Some(value) => Some(value.as_f64().filter(|o| o.is_finite() && (0.0..=1.0).contains(o))?),
+    };
+    // Layer properties go on the `Usng` layer, not into the adjustment's own settings; a
+    // value the reference application cannot record makes the step unexportable (#8384).
+    let name = match p.get("name") {
+        None | Some(Value::Null) => None,
+        Some(v) => Some(v.as_str()?),
+    };
+    let mode = match p.get("mode") {
+        None | Some(Value::Null) => None,
+        Some(v) => {
+            let label = v.as_str()?;
+            if crate::commands::blend_from_str(label)? == photocraft_doc::BlendMode::PassThrough {
+                return None;
+            }
+            Some(blend_ps(label)?)
+        }
+    };
+    let clip = match p.get("clipToPrevious") {
+        None | Some(Value::Null) => None,
+        Some(v) => Some(v.as_bool()?),
+    };
+    let mut params = p.clone();
+    if let Some(params) = params.as_object_mut() {
+        for key in ["opacity", "name", "mode", "clipToPrevious"] {
+            params.remove(key);
+        }
+    }
+    // Layer opacity must not turn default adjustment settings into a custom preset.
+    let t = if params.as_object().is_some_and(|o| o.is_empty()) {
+        desc(a.class).with("presetKind", enm("presetKindType", "presetKindDefault"))
+    } else {
+        (a.to)(&params)?
+    };
+    let mut using = desc("AdjL").with("Type", D::Descriptor(t));
+    if let Some(opacity) = opacity {
+        using = using.with("Opct", pct(recorded_percent(opacity)));
+    }
+    if let Some(name) = name {
+        using = using.with("Nm  ", text(name));
+    }
+    if let Some(mode) = mode {
+        using = using.with("Md  ", enm("BlnM", mode));
+    }
+    if let Some(clip) = clip {
+        using = using.with("Grup", D::Boolean(clip));
+    }
+    Some(desc("null").with("null", class_ref("AdjL")).with("Usng", D::Descriptor(using)))
+}
+
 /// Import selection geometry or a channel source, including its optional source layer.
 fn selection_from(d: &Descriptor) -> Mapped {
     let unsupported = Mapped::Unsupported { harmless: false };

crates/engine/tests/it/atn_adjustment_make_props.rs

@@ -0,0 +1,128 @@
+//! #8384: an adjustment layer's Make step keeps the new layer's name (`Nm  `), blend mode
+//! (`Md  `) and clipping (`Grup`) on load, playback and save.
+use photocraft_doc::{BlendMode, LayerContent};
+use photocraft_engine::{Session, actions_cmds::Action, atn_cmds};
+use photocraft_psd::atn::{self, AtnAction, AtnEvent, AtnFile, AtnItem};
+use photocraft_psd::descriptor::{Class, Descriptor, Id, ReferenceItem, UnicodeString, Value as D};
+use serde_json::{Value, json};
+
+const NAME: &str = "Independent named adjustment";
+
+fn text(s: &str) -> D {
+    D::Text(UnicodeString::new_nul(s))
+}
+
+fn blend(code: &str) -> D {
+    D::Enumerated { type_id: Id::new("BlnM"), value: Id::new(code) }
+}
+
+fn levels() -> Descriptor {
+    Descriptor::new("Lvls").with("presetKind", D::Enumerated { type_id: Id::new("presetKindType"), value: Id::new("presetKindDefault") })
+}
+
+/// `Make adjustment layer` with `extra` keys after `Type`, in the order Save Actions writes them.
+fn make(extra: &[(&str, D)]) -> Descriptor {
+    let mut using = Descriptor::new("AdjL").with("Type", D::Descriptor(levels()));
+    for (k, v) in extra {
+        using = using.with(k, v.clone());
+    }
+    let class_ref = ReferenceItem::Class(Class { name: UnicodeString::new_nul(""), class_id: Id::new("AdjL") });
+    Descriptor::new("null").with("null", D::Reference(vec![class_ref])).with("Usng", D::Descriptor(using))
+}
+
+fn bytes(descriptor: Descriptor) -> Vec<u8> {
+    let file = AtnFile {
+        name: UnicodeString::new_nul("Props"),
+        actions: vec![AtnAction {
+            name: UnicodeString::new_nul("Make"),
+            items: vec![AtnItem { event: AtnEvent::Code(*b"Mk  "), dictionary: b"Make".to_vec(), descriptor: Some(descriptor), ..Default::default() }],
+            ..Default::default()
+        }],
+        ..Default::default()
+    };
+    atn::write(&file).unwrap()
+}
+
+fn doc(depth: u32) -> Session {
+    let mut s = Session::new();
+    s.execute("file.new", json!({"width": 4, "height": 3, "depth": depth})).unwrap();
+    s.execute("layer.new.layer", json!({"name": "Pixels"})).unwrap();
+    s
+}
+
+#[test]
+fn make_keeps_name_mode_and_clipping_through_play_and_save_at_both_depths() {
+    let cases: [(&[(&str, D)], Value); 5] = [
+        (&[("Nm  ", text(NAME))], json!({"name": NAME})),
+        (&[("Md  ", blend("Mltp"))], json!({"mode": "Multiply"})),
+        (&[("Grup", D::Boolean(true))], json!({"clipToPrevious": true})),
+        (&[("Grup", D::Boolean(false))], json!({"clipToPrevious": false})),
+        (&[("Nm  ", text(NAME)), ("Md  ", blend("Scrn")), ("Grup", D::Boolean(true))], json!({"name": NAME, "mode": "Screen", "clipToPrevious": true})),
+    ];
+    for depth in [8, 16] {
+        for (extra, params) in &cases {
+            let descriptor = make(extra);
+            let mut s = doc(depth);
+            let base = s.active().unwrap().active_layer.unwrap();
+            let loaded = s.execute("actions.load", json!({"data": photocraft_paint::tile::b64_encode(&bytes(descriptor.clone()))})).unwrap();
+            assert_eq!(loaded["unsupported"], json!([]), "{params}");
+            let steps = &s.actions.list[0].steps;
+            assert_eq!(steps.len(), 1, "{params}");
+            assert_eq!(steps[0].0, "layer.newAdjustmentLayer.levels");
+            for (k, v) in params.as_object().unwrap() {
+                assert_eq!(steps[0].1.get(k), Some(v), "{params}: {}", steps[0].1);
+            }
+
+            let played = s.execute("actions.play", json!({"action": "Make"})).unwrap();
+            assert!(played.get("failed").is_none(), "{params}: {played}");
+            let st = s.active().unwrap();
+            let layer = st.doc.layer(st.active_layer.unwrap()).unwrap();
+            assert!(matches!(layer.content, LayerContent::Adjustment(_)), "{params}");
+            assert_eq!(layer.name, if params.get("name").is_some() { NAME } else { "Levels 1" }, "{params}");
+            let mode = match params.get("mode").and_then(Value::as_str) {
+                Some("Multiply") => BlendMode::Multiply,
+                Some("Screen") => BlendMode::Screen,
+                _ => BlendMode::Normal,
+            };
+            assert_eq!(layer.blend, mode, "{params}");
+            assert_eq!(layer.clipped, params.get("clipToPrevious") == Some(&json!(true)), "{params}");
+            let base = st.doc.layer(base).unwrap();
+            assert_eq!((base.name.as_str(), base.blend, base.clipped), ("Pixels", BlendMode::Normal, false), "{params}");
+
+            // Save Actions writes the descriptor the step was loaded from.
+            let saved = s.execute("actions.save", json!({"set": "Props"})).unwrap();
+            let data = photocraft_paint::tile::b64_decode(saved["data"].as_str().unwrap()).unwrap();
+            let file = atn::parse(&data).unwrap();
+            assert_eq!(file.actions[0].items[0].descriptor.as_ref(), Some(&descriptor), "{params}");
+        }
+    }
+}
+
+#[test]
+fn unreadable_layer_properties_keep_the_step() {
+    for extra in [("Nm  ", D::Integer(2)), ("Md  ", blend("unknownBlend")), ("Md  ", blend("PsTh")), ("Md  ", text("Multiply")), ("Grup", text("true"))] {
+        let imported = atn_cmds::import(&bytes(make(std::slice::from_ref(&extra)))).unwrap();
+        assert_eq!(imported.unsupported.len(), 1, "{:?}", extra.0);
+        assert_eq!(imported.actions[0].steps[0].0, atn_cmds::PHOTOSHOP_STEP, "{:?}", extra.0);
+    }
+}
+
+#[test]
+fn bad_layer_properties_fail_without_a_layer_or_an_export() {
+    let mut s = doc(8);
+    for params in [
+        json!({"name": false}),
+        json!({"name": 3}),
+        json!({"mode": false}),
+        json!({"mode": "unknown"}),
+        json!({"mode": "Pass Through"}),
+        json!({"clipToPrevious": 1}),
+        json!({"clipToPrevious": "true"}),
+    ] {
+        let layers = s.active().unwrap().doc.layers.len();
+        assert!(s.execute("layer.newAdjustmentLayer.levels", params.clone()).is_err(), "{params}");
+        assert_eq!(s.active().unwrap().doc.layers.len(), layers, "{params} must not add a layer");
+        let action = Action::new("Make", vec![("layer.newAdjustmentLayer.levels".into(), params.clone())]);
+        assert!(atn_cmds::export("Props", &[&action], &[], false).is_err(), "{params} must not save as a default layer");
+    }
+}

crates/engine/tests/it/main.rs

@@ -3,6 +3,7 @@
 mod actions_single_history;
 mod affinity;
 mod artboard_export_names;
+mod atn_adjustment_make_props;
 mod atn_classic_gradient;
 mod atn_content_aware_fill;
 mod atn_crystallize_cell_size;