Integration: Camera Raw Filter steps save back and refuse unknown keys (#8714)

A loaded Camera Raw Filter step became an import-only filter.cameraRaw, so
Save Actions dropped it (skipUnsupported) or failed, where before it was
written back byte for byte. Keys outside the table (e.g. PerV = 20, a
profile name) were ignored, so the step replayed a different look without
being reported.

The step now keeps its recorded descriptor in a hidden param and saves back
as that descriptor while its params are still what it imports as; edited or
PhotoCraft-recorded steps are still refused. A step with any unknown key, or
with no settings, stays a kept Photoshop step, as the PSD mapping does.

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

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

@@ -7,8 +7,8 @@ use super::steps_doc::{
     place_from, place_to, profile_from, profile_to, purge_from, purge_to, reset_from, reset_to, save_from, save_to, stop_from, stop_to,
 };
 use super::steps_image::{
-    GALLERY_IDS, REFINE_EDGE, RIPPLE_SIZE, SMART_BRUSH, apply_image_from, apply_image_to, camera_raw_from, color_range_from, color_range_to, gallery_from,
-    gallery_to, has_refine_strokes, radial_blur_from, radial_blur_to, refine_edge_to, transform_from, transform_to,
+    GALLERY_IDS, REFINE_EDGE, RIPPLE_SIZE, SMART_BRUSH, apply_image_from, apply_image_to, camera_raw_from, camera_raw_to, color_range_from, color_range_to,
+    gallery_from, gallery_to, has_refine_strokes, radial_blur_from, radial_blur_to, refine_edge_to, transform_from, transform_to,
 };
 use super::steps_layer::{
     STROKE, apply_comp_from, apply_comp_to, clear_from, clear_to, delete_from, delete_to, duplicate_from, duplicate_to, fill_from, fill_to, flip_from, flip_to,
@@ -249,9 +249,9 @@ pub(super) const EVENTS: &[Event] = &[
     ev!("Rple", "ripple", "Ripple", "filter.distort.ripple", &[F("Amnt", "amount", Kind::ExactLong), F("RplS", "size", Kind::Enum("RplS", RIPPLE_SIZE))]),
     ev!("Crst", "crystallize", "Crystallize", "filter.pixelate.crystallize", &[F("ClSz", "cellSize", Kind::ExactLong)]),
     Event { code: "GEfc", name: "filterGallery", label: "Filter Gallery", ids: GALLERY_IDS, from: gallery_from, to: gallery_to },
-    // Import only: what Photoshop needs in a written step (process version, the full settings
-    // set) is unverified, so Save Actions still refuses `filter.cameraRaw`.
-    Event { code: "", name: "Adobe Camera Raw Filter", label: "Camera Raw Filter", ids: &[], from: camera_raw_from, to: |_, _| None },
+    // What Photoshop needs in a written step (process version, the full settings set) is
+    // unverified, so only an imported step saves back, as its recorded descriptor while unedited.
+    Event { code: "", name: "Adobe Camera Raw Filter", label: "Camera Raw Filter", ids: &[crate::lens_cmds::RAW], from: camera_raw_from, to: camera_raw_to },
     // Adjustments (Image › Adjustments).
     Event {
         code: "",

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

@@ -304,15 +304,37 @@ fn camera_raw_range(key: &str) -> std::ops::RangeInclusive<f64> {
     }
 }
 
+/// The hidden param an imported Camera Raw Filter step keeps its recorded descriptor in, so
+/// Save Actions writes the step back unchanged while its settings are the ones it imported with.
+const CAMERA_RAW_DESCRIPTOR: &str = "__atnDescriptor";
+
+/// The colour grading wheels' keys, the tone curve splits and the vignette style.
+const CAMERA_RAW_GRADES: &[&str] =
+    &["STSH", "STSS", "CgSL", "CgMH", "CgMS", "CgML", "STHH", "STHS", "CgHL", "CgGH", "CgGS", "CgGL", "PC_1", "PC_2", "PC_3", "PCVS"];
+
 /// A recorded Camera Raw Filter step (`Adobe Camera Raw Filter`) as `filter.cameraRaw` (#6745).
 /// Settings the command has no counterpart for refuse the step rather than replaying a different
-/// look: the Calibration panel, a white balance other than Custom, and the controls the PSD
-/// mapping keeps only at their neutral values. Version metadata and a full settings snapshot's
-/// lens and geometry keys are ignored, as other events ignore keys they don't map.
+/// look: the Calibration panel, a white balance other than Custom, the controls the PSD mapping
+/// keeps only at their neutral values, and any key outside this table (lens, geometry, profile,
+/// masks…), as the PSD mapping's `known_key` does.
 pub(super) fn camera_raw_from(d: &Descriptor) -> Mapped {
     const CALIBRATION: &[&str] = &["RHue", "RSat", "GHue", "GSat", "BHue", "BSat", "BlkB"];
     const NEUTRAL_ONLY: &[(&str, f64)] =
         &[("LNRC", 0.0), ("CNRS", 50.0), ("MDis", 0.0), ("VigA", 0.0), ("VigM", 50.0), ("crfs", 100.0), ("TMMs", 0.0), ("PGTM", 0.0), ("RGBt", 0.0)];
+    const OTHER: &[&str] = &["CrVe", "PrVe", "PrVN", "WBal", "Crv ", "CrvR", "CrvG", "CrvB"];
+    let known = |k: &str| {
+        (k.len() == 4 && ["HA_", "SA_", "LA_"].iter().any(|p| k.starts_with(p)) && ["R", "O", "Y", "G", "A", "B", "P", "M"].iter().any(|c| k.ends_with(c)))
+            || CAMERA_RAW.iter().any(|F(f, ..)| *f == k)
+            || CAMERA_RAW_GRADES.contains(&k)
+            || OTHER.contains(&k)
+            || CALIBRATION.contains(&k)
+            || NEUTRAL_ONLY.iter().any(|(n, _)| *n == k)
+    };
+    // An empty step (no descriptor) would play as a no-op, and a key the table doesn't know may
+    // be active processing (lens, geometry, profile, masks) that playback would leave out.
+    if d.items.is_empty() || d.items.iter().any(|(k, _)| !std::str::from_utf8(k.as_bytes()).is_ok_and(known)) {
+        return Mapped::Unsupported { harmless: false };
+    }
     let active = |k: &str, neutral: f64| d.get(k).is_some() && get_num(d, k) != Some(neutral);
     let custom_wb = d.get("WBal").is_none() || get_enum(d, "WBal").as_deref() == Some("Cst");
     if !custom_wb || CALIBRATION.iter().any(|k| active(k, 0.0)) || NEUTRAL_ONLY.iter().any(|(k, n)| active(k, *n)) {
@@ -327,8 +349,7 @@ pub(super) fn camera_raw_from(d: &Descriptor) -> Mapped {
         (Some(_), Some(v)) => k != "PCVS" && !(v.is_finite() && camera_raw_range(k).contains(&v)),
     };
     let grouped = ["HA_", "SA_", "LA_"].iter().flat_map(|p| ["R", "O", "Y", "G", "A", "B", "P", "M"].map(|c| format!("{p}{c}")));
-    let grades = ["STSH", "STSS", "CgSL", "CgMH", "CgMS", "CgML", "STHH", "STHS", "CgHL", "CgGH", "CgGS", "CgGL", "PC_1", "PC_2", "PC_3", "PCVS"];
-    if CAMERA_RAW.iter().any(|F(k, ..)| unreadable(k)) || grouped.into_iter().any(|k| unreadable(&k)) || grades.iter().any(|k| unreadable(k)) {
+    if CAMERA_RAW.iter().any(|F(k, ..)| unreadable(k)) || grouped.into_iter().any(|k| unreadable(&k)) || CAMERA_RAW_GRADES.iter().any(|k| unreadable(k)) {
         return Mapped::Unsupported { harmless: false };
     }
     let Some(mut m) = fields_from(d, CAMERA_RAW) else { return Mapped::Unsupported { harmless: false } };
@@ -390,9 +411,25 @@ pub(super) fn camera_raw_from(d: &Descriptor) -> Mapped {
             m.insert(param.into(), Value::Array(flat.as_chunks::<2>().0.iter().map(|[a, b]| json!([a, b])).collect()));
         }
     }
+    m.insert(CAMERA_RAW_DESCRIPTOR.into(), json!(photocraft_paint::tile::b64_encode(&d.to_bytes())));
     step(crate::lens_cmds::RAW, Value::Object(m))
 }
 
+/// An imported Camera Raw Filter step back as its recorded descriptor, unchanged. Only while the
+/// params are still what that descriptor imports as: an edited step, or one PhotoCraft recorded,
+/// has no verified Photoshop form and Save Actions refuses it.
+pub(super) fn camera_raw_to(_: &str, p: &Value) -> Option<Descriptor> {
+    let d = Descriptor::from_bytes(&photocraft_paint::tile::b64_decode(jstr(p, CAMERA_RAW_DESCRIPTOR)?)?).ok()?;
+    let Mapped::Steps(steps) = camera_raw_from(&d) else { return None };
+    let [(_, imported)] = steps.as_slice() else { return None };
+    let (mut a, mut b) = (imported.clone(), p.clone());
+    for v in [&mut a, &mut b] {
+        super::convert::normalise(v);
+        v.as_object_mut()?.remove(CAMERA_RAW_DESCRIPTOR);
+    }
+    (a == b).then_some(d)
+}
+
 /// Whether a Select and Mask step carries Refine Edge Brush strokes or the workspace's tool
 /// edits (Quick Selection, Brush, Lasso…), which no descriptor holds.
 pub(super) fn has_refine_strokes(p: &Value) -> bool {

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

@@ -764,6 +764,8 @@ fn every_mapped_command_exports_and_imports_back() {
         "filter.sharpen.sharpen",
         "filter.sharpen.sharpenEdges",
         "filter.sharpen.sharpenMore",
+        // Saves back only as the descriptor it was loaded from (loaded_camera_raw_filter_steps_save_back_unchanged).
+        "filter.cameraRaw",
     ];
     let unexpected: Vec<&str> = missing.iter().copied().filter(|id| !allowed.contains(id)).collect();
     assert!(unexpected.is_empty(), "add round-trip cases for {unexpected:?}");
@@ -3096,8 +3098,11 @@ fn camera_raw_filter_steps_import_as_filter_camera_raw() {
     let steps = &imp.actions[0].steps;
     let (id, p) = &steps[0];
     assert_eq!(id, "filter.cameraRaw");
+    // The recorded descriptor rides along, so Save Actions can write the step back.
+    let mut p = p.clone();
+    assert!(p.as_object_mut().unwrap().remove("__atnDescriptor").is_some(), "{p}");
     assert_eq!(
-        *p,
+        p,
         json!({
             "temperature": 12, "tint": -4, "exposure": 0.35, "contrast": 10, "highlights": -30, "shadows": 25,
             "clarity": 8, "vibrance": 15, "curveSplits": [20, 50, 80],
@@ -3119,6 +3124,77 @@ fn camera_raw_filter_steps_import_as_filter_camera_raw() {
     s.execute(id, p.clone()).unwrap();
 }
 
+/// Review of #8714: a Camera Raw Filter step that was loaded saves back as the recorded step,
+/// byte for byte, as it did while it was a kept Photoshop step; once edited (or when PhotoCraft
+/// recorded it) Save Actions refuses it as before. The loaded step plays.
+#[test]
+fn loaded_camera_raw_filter_steps_save_back_unchanged() {
+    let recorded = desc("null")
+        .with("CrVe", D::Text(UnicodeString::new_nul("16.1.1")))
+        .with("PrVe", D::Integer(184_549_376))
+        .with("WBal", enm("WBal", "Cst "))
+        .with("Ex12", D::Double(0.35))
+        .with("Cr12", D::Integer(10))
+        .with("Crv ", D::List([0, 0, 64, 50, 255, 255].map(D::Integer).to_vec()));
+    let file = one_action(vec![item("Adobe Camera Raw Filter", "Camera Raw Filter", Some(recorded.clone()))]);
+    let mut s = Session::new();
+    let loaded = s.execute("actions.load", json!({"data": b64(&atn::write(&file).unwrap())})).unwrap();
+    assert_eq!(loaded["unsupported"], json!([]), "{loaded}");
+    assert_eq!(s.actions.list[0].steps[0].0, "filter.cameraRaw");
+    let saved_step = |r: &Value| {
+        let saved = atn::parse(&photocraft_paint::tile::b64_decode(r["data"].as_str().unwrap()).unwrap()).unwrap();
+        saved.actions[0].items.first().map(|i| (i.event.clone(), i.descriptor.clone()))
+    };
+    // Strict save, and the Actions panel's save (which skips what can't be written).
+    for skip in [false, true] {
+        let r = s.execute("actions.save", json!({"action": "A", "skipUnsupported": skip})).unwrap();
+        assert_eq!(r["skipped"], json!([]), "{r}");
+        assert_eq!(saved_step(&r), Some((AtnEvent::name("Adobe Camera Raw Filter"), Some(recorded.clone()))));
+    }
+    // It plays.
+    s.execute("file.new", json!({"width": 16, "height": 12})).unwrap();
+    s.execute("layer.new.layer", json!({})).unwrap();
+    s.execute("edit.fill", json!({"color": "#808080"})).unwrap();
+    let played = s.execute("actions.play", json!({"action": "A"})).unwrap();
+    assert_eq!(played["ran"], 1, "{played}");
+    assert!(played.get("failed").is_none(), "{played}");
+    // Edited, it no longer matches what was recorded.
+    let original = s.actions.list[0].steps[0].1.clone();
+    s.actions.list[0].steps[0].1["exposure"] = json!(1.5);
+    let e = s.execute("actions.save", json!({"action": "A"})).unwrap_err().to_string();
+    assert!(e.contains("filter.cameraRaw"), "{e}");
+    // Recorded by PhotoCraft: no descriptor to write back.
+    s.actions.list[0].steps[0].1 = original;
+    s.actions.list[0].steps[0].1.as_object_mut().unwrap().remove("__atnDescriptor");
+    let r = s.execute("actions.save", json!({"action": "A", "skipUnsupported": true})).unwrap();
+    assert_eq!(r["skipped"].as_array().unwrap().len(), 1, "{r}");
+}
+
+/// Review of #8714: a Camera Raw Filter step with a key the table doesn't map (an Upright
+/// setting, a profile name, any other panel) or with no settings at all stays a Photoshop step
+/// and is reported, rather than replaying without that processing.
+#[test]
+fn camera_raw_filter_steps_with_unknown_keys_stay_photoshop_steps() {
+    let upright = desc("null").with("Ex12", D::Double(0.2)).with("PerV", D::Integer(20));
+    let profile = desc("null").with("Cr12", D::Integer(10)).with("LookName", D::Text(UnicodeString::new_nul("B&W 05")));
+    let file = one_action(vec![
+        item("Adobe Camera Raw Filter", "Camera Raw Filter", Some(upright.clone())),
+        item("Adobe Camera Raw Filter", "Camera Raw Filter", Some(profile)),
+        item("Adobe Camera Raw Filter", "Camera Raw Filter", None),
+        item("Adobe Camera Raw Filter", "Camera Raw Filter", Some(desc("null").with("Ex12", D::Double(0.2)))),
+    ]);
+    let mut s = Session::new();
+    let loaded = s.execute("actions.load", json!({"data": b64(&atn::write(&file).unwrap())})).unwrap();
+    let unsupported: Vec<u64> = loaded["unsupported"].as_array().unwrap().iter().map(|u| u["step"].as_u64().unwrap()).collect();
+    assert_eq!(unsupported, [0, 1, 2], "{loaded}");
+    let ids: Vec<&str> = s.actions.list[0].steps.iter().map(|(id, _)| id.as_str()).collect();
+    assert_eq!(ids, [PHOTOSHOP_STEP, PHOTOSHOP_STEP, PHOTOSHOP_STEP, "filter.cameraRaw"]);
+    // Kept, it still saves back as recorded.
+    let r = s.execute("actions.save", json!({"action": "A"})).unwrap();
+    let saved = atn::parse(&photocraft_paint::tile::b64_decode(r["data"].as_str().unwrap()).unwrap()).unwrap();
+    assert_eq!(saved.actions[0].items[0].descriptor, Some(upright));
+}
+
 fn one_action(items: Vec<AtnItem>) -> AtnFile {
     AtnFile {
         name: UnicodeString::new_nul("Saves"),