diff --git a/crates/engine/src/cmd/atn_cmds/events.rs b/crates/engine/src/cmd/atn_cmds/events.rs index cffb144f4b..e77b088878 100644 --- a/crates/engine/src/cmd/atn_cmds/events.rs +++ b/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: "", diff --git a/crates/engine/src/cmd/atn_cmds/steps_image.rs b/crates/engine/src/cmd/atn_cmds/steps_image.rs index 50ad30f139..b84b21c227 100644 --- a/crates/engine/src/cmd/atn_cmds/steps_image.rs +++ b/crates/engine/src/cmd/atn_cmds/steps_image.rs @@ -304,15 +304,37 @@ fn camera_raw_range(key: &str) -> std::ops::RangeInclusive { } } +/// 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 { + 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 { diff --git a/crates/engine/src/cmd/atn_cmds/tests.rs b/crates/engine/src/cmd/atn_cmds/tests.rs index 979e6864fb..91435c2ca5 100644 --- a/crates/engine/src/cmd/atn_cmds/tests.rs +++ b/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 = 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) -> AtnFile { AtnFile { name: UnicodeString::new_nul("Saves"),