Integration: Filter Gallery effect steps keep their recorded event (#10632)

Three gaps in loading and saving Filter Gallery steps from .atn files:

- An effect recorded under its own event by string ID (event `cutout`
  with `GEfk` = `Ct  `, the form newer versions write) stayed a kept
  Photoshop step: only 4-char codes were routed to the effect table.
  The effects' string IDs are now matched too.
- A step recorded as the effect's own event (`AccE` "Accented Edges")
  saved back as a `GEfc` "Filter Gallery" event, so load -> save changed
  the event and its panel name. The step now remembers the event it was
  loaded from (hidden `_atnEvent` param) and writes it back when that
  event is the step's own effect; `GEfc` steps, steps made in PhotoCraft
  and a stale or forged event naming another effect save as `GEfc`.
  Playback strips the param (`playback_params`, like `_actionLayer`), so
  the command, the journal (Last Filter), smart filters and the step's
  dialog never carry it.
- A key that was present but had the wrong type or a non-finite value
  took the default (Texturizer `InvT` as Integer 1 played with invert
  off; `EdgW` NaN played as 2). Only missing keys take defaults now; an
  unreadable present key keeps the step.

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

crates/engine/src/cmd/actions_cmds.rs

@@ -131,9 +131,16 @@ pub(crate) fn recorded_params(s: &Session, id: &str, mut p: Value) -> Value {
     p
 }
 
-/// Resolve the recorded visibility target identically in engine and shell playback.
+/// Resolve the recorded visibility target identically in engine and shell playback, and drop
+/// the `.atn` event a Filter Gallery step remembers for Save Actions: the command, its journal
+/// entry (Last Filter), a smart filter and the step's dialog never see it.
 pub fn playback_params(s: &Session, id: &str, p: &Value) -> Result<Value> {
     let mut p = p.clone();
+    if id.starts_with("filter.gallery.")
+        && let Some(object) = p.as_object_mut()
+    {
+        object.remove(crate::atn_cmds::RECORDED_EVENT);
+    }
     if id == "layer.setProps"
         && let Some(target) = p.get("_actionLayer")
     {

crates/engine/src/cmd/atn_cmds.rs

@@ -57,6 +57,7 @@ use fields::Mapped;
 use files::{load, save};
 #[cfg(test)]
 use steps_doc::crop_from;
+pub(crate) use steps_gallery::RECORDED_EVENT;
 #[cfg(test)]
 use steps_image::{apply_image_from, transform_to};
 #[cfg(test)]

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

@@ -72,7 +72,10 @@ fn convert_action(a: &AtnAction, set: &str, unsupported: &mut Vec<Unsupported>)
             (None, _) => Mapped::Unsupported { harmless: false },
         };
         let (new_steps, enabled) = match mapped {
-            Mapped::Steps(s) => (s, item.enabled),
+            Mapped::Steps(mut s) => {
+                super::steps_gallery::remember_event(item, &mut s);
+                (s, item.enabled)
+            }
             Mapped::Unsupported { harmless } => {
                 unsupported.push(Unsupported { action: name.clone(), step: i, event: item.event.id(), name: item.label() });
                 (vec![photoshop_step(item)], item.enabled && !harmless)
@@ -145,6 +148,12 @@ pub(super) fn export_item(id: &str, params: &Value, set: &str) -> Option<AtnItem
         params["set"] = json!(set);
     }
     let (e, descriptor) = event_and_descriptor(id, &params)?;
+    // A Filter Gallery effect loaded from its own event saves back as that event (#10592).
+    if e.code == "GEfc"
+        && let Some((event, name)) = super::steps_gallery::recorded_event(id, &params)
+    {
+        return Some(AtnItem { event, dictionary: name.into_bytes(), descriptor: Some(descriptor), ..Default::default() });
+    }
     let event = if e.code.is_empty() { AtnEvent::name(e.name) } else { AtnEvent::Code(e.code.as_bytes().try_into().ok()?) };
     Some(AtnItem { event, dictionary: e.label.as_bytes().to_vec(), descriptor: Some(descriptor), ..Default::default() })
 }

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

@@ -620,7 +620,8 @@ pub(super) fn event_for(item: &AtnEvent) -> Option<&'static Event> {
     let find = |id: &str| EVENTS.iter().find(|e| (!e.code.is_empty() && e.code.trim_end() == id.trim_end()) || e.name == id);
     find(&id).or_else(|| match item {
         // An effect run from its own menu item records its id as the event (#10592).
-        AtnEvent::Code(_) if super::steps_gallery::is_effect(&id) => find("filterGallery"),
+        // By 4-char code or, as newer versions record it, by string ID (`cutout`).
+        _ if super::steps_gallery::is_effect(item) => find("filterGallery"),
         AtnEvent::Code(_) => LEGACY_CODES.iter().find(|(code, _)| *code == id).and_then(|(_, name)| find(name)),
         AtnEvent::Name(_) => None,
     })

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

@@ -3,11 +3,11 @@
 //! the keys, limits and defaults Photoshop measures (`docs/atn.md`, Group A); a stack of several
 //! effects stays a Photoshop step.
 
-use super::descriptors::{color_desc, color_hex, desc, enm, get_bool, get_enum, get_num, get_obj, trimmed};
+use super::descriptors::{color_desc, color_hex, desc, enm, get_bool, get_enum, get_num, get_obj, jstr, trimmed};
 use super::events::pixel_target;
 use super::fields::{Mapped, step};
 use super::tables::{lookup, lookup_back};
-use super::{D, Descriptor, Map, Value};
+use super::{AtnEvent, AtnItem, D, Descriptor, Map, Value, json};
 use photocraft_algo::{GalleryEffect, GalleryFilter};
 
 /// One key of an effect, in recorded order.
@@ -523,10 +523,108 @@ pub(super) const GALLERY_IDS: &[&str] = &[
     "filter.gallery.grain",
 ];
 
-/// Whether `code` is a Filter Gallery effect's id. The reference has each effect recorded as
-/// its own event too (Filter › Artistic › Cutout…), carrying the same `GEfk` and settings.
-pub(super) fn is_effect(code: &str) -> bool {
-    GALLERY.iter().any(|(c, _, _)| *c == code.trim_end())
+/// Each effect's own event by string ID, the form newer Photoshop versions record it in
+/// (`docs/atn.md`, Group A). Mosaic Tiles (`MscT`) has none.
+const EVENT_NAMES: &[(&str, &str)] = &[
+    ("AccE", "accentedEdges"),
+    ("AngS", "angledStrokes"),
+    ("BsRl", "basRelief"),
+    ("ChlC", "chalkCharcoal"),
+    ("Chrc", "charcoal"),
+    ("Chrm", "chrome"),
+    ("ClrP", "coloredPencil"),
+    ("CntC", "conteCrayon"),
+    ("Crql", "craquelure"),
+    ("Crsh", "crosshatch"),
+    ("Ct", "cutout"),
+    ("DrkS", "darkStrokes"),
+    ("DfsG", "diffuseGlow"),
+    ("DryB", "dryBrush"),
+    ("FlmG", "filmGrain"),
+    ("Frsc", "fresco"),
+    ("Gls", "glass"),
+    ("GlwE", "glowingEdges"),
+    ("GraP", "graphicPen"),
+    ("HlfS", "halftoneScreen"),
+    ("InkO", "inkOutlines"),
+    ("NGlw", "neonGlow"),
+    ("NtPr", "notePaper"),
+    ("OcnR", "oceanRipple"),
+    ("PntD", "paintDaubs"),
+    ("PltK", "paletteKnife"),
+    ("Ptch", "patchwork"),
+    ("Phtc", "photocopy"),
+    ("Plst", "plaster"),
+    ("PlsW", "plasticWrap"),
+    ("PstE", "posterEdges"),
+    ("Rtcl", "reticulation"),
+    ("RghP", "roughPastels"),
+    ("SmdS", "smudgeStick"),
+    ("Spt", "spatter"),
+    ("Spng", "sponge"),
+    ("SprS", "sprayedStrokes"),
+    ("StnG", "stainedGlass"),
+    ("Stmp", "stamp"),
+    ("Smie", "sumie"),
+    ("Txtz", "texturizer"),
+    ("TrnE", "tornEdges"),
+    ("Undr", "underpainting"),
+    ("WtrP", "waterPaper"),
+    ("Wtrc", "watercolor"),
+    ("Grn", "grain"),
+];
+
+/// Whether `event` is a Filter Gallery effect's own event: its 4-char code, or its string ID.
+/// The reference has each effect recorded as its own event too (Filter › Artistic › Cutout…),
+/// carrying the same `GEfk` and settings.
+pub(super) fn is_effect(event: &AtnEvent) -> bool {
+    effect_command(event).is_some()
+}
+
+/// The command of the effect whose own event `event` is (`AccE` or `accentedEdges` →
+/// `filter.gallery.accentedEdges`).
+fn effect_command(event: &AtnEvent) -> Option<&'static str> {
+    let id = event.id();
+    let code = match event {
+        AtnEvent::Code(_) => GALLERY.iter().map(|(c, _, _)| *c).find(|c| *c == id.trim_end()),
+        AtnEvent::Name(_) => EVENT_NAMES.iter().find(|(_, n)| *n == id).map(|(c, _)| *c),
+    }?;
+    GALLERY.iter().find(|(c, _, _)| *c == code).map(|(_, cmd, _)| *cmd)
+}
+
+/// The hidden param a step loaded from an effect's own event carries, so it saves back as that
+/// event (and panel name) rather than as a `GEfc` Filter Gallery step. Playback strips it
+/// (`actions_cmds::playback_params`), so the command, the journal and smart filters never see it.
+pub(crate) const RECORDED_EVENT: &str = "_atnEvent";
+
+/// Remember the event of a step loaded from an effect's own event (`AccE`, `accentedEdges`).
+/// A `GEfc` step carries nothing and saves as `GEfc`, as does a step made in PhotoCraft.
+pub(super) fn remember_event(item: &AtnItem, steps: &mut [(String, Value)]) {
+    if !is_effect(&item.event) {
+        return;
+    }
+    let [(id, Value::Object(p))] = steps else { return };
+    if effect_command(&item.event) != Some(id.as_str()) {
+        return;
+    }
+    let kind = match item.event {
+        AtnEvent::Code(_) => "code",
+        AtnEvent::Name(_) => "name",
+    };
+    p.insert(RECORDED_EVENT.into(), json!({"event": item.event.id(), "eventKind": kind, "name": item.dictionary()}));
+}
+
+/// The event and panel name a step of command `id` remembered by [`remember_event`] saves as;
+/// `None` (save as `GEfc`) when it has none or it is not the own event of `id`'s effect.
+pub(super) fn recorded_event(id: &str, params: &Value) -> Option<(AtnEvent, String)> {
+    let r = params.get(RECORDED_EVENT)?;
+    let event = jstr(r, "event")?;
+    let event = match jstr(r, "eventKind")? {
+        "code" => AtnEvent::Code(event.as_bytes().try_into().ok()?),
+        "name" => AtnEvent::name(event),
+        _ => return None,
+    };
+    (effect_command(&event) == Some(id)).then(|| (event, jstr(r, "name").unwrap_or("").to_string()))
 }
 
 /// A Filter Gallery step: the effect `GEfk` names, every key clamped or looked up, Photoshop's
@@ -544,14 +642,22 @@ fn effect_step(d: &Descriptor) -> Option<Mapped> {
     }
     let mut p = Map::new();
     for k in keys.iter() {
+        // Only a missing key takes Photoshop's default: a key that is there with the wrong type
+        // or a non-finite number keeps the step (`docs/atn.md`) rather than playing a default.
+        let present = d.get(k.key()).is_some();
         let v = match k {
-            G::Int(key, _, min, max, default) => Value::from(get_num(d, key).map_or(*default, |n| n.round().clamp(f64::from(*min), f64::from(*max)) as i32)),
-            G::Enum(key, _, table, default) => Value::from(lookup(table, &get_enum(d, key).unwrap_or_else(|| (*default).to_string()))?),
-            G::Bool(key, _, default) => Value::from(get_bool(d, key).unwrap_or(*default)),
-            G::Color(key, _, default) => match get_obj(d, key) {
-                Some(c) => Value::from(color_hex(c)?),
-                None => Value::from(*default),
-            },
+            G::Int(key, _, min, max, default) => {
+                Value::from(if present { get_num(d, key)?.round().clamp(f64::from(*min), f64::from(*max)) as i32 } else { *default })
+            }
+            G::Enum(key, _, table, default) => Value::from(lookup(table, &if present { get_enum(d, key)? } else { (*default).to_string() })?),
+            G::Bool(key, _, default) => Value::from(if present { get_bool(d, key)? } else { *default }),
+            G::Color(key, _, default) => {
+                if present {
+                    Value::from(color_hex(get_obj(d, key)?)?)
+                } else {
+                    Value::from(*default)
+                }
+            }
         };
         p.insert(k.param().into(), v);
     }
@@ -573,7 +679,7 @@ fn default_color(id: &str) -> Option<Descriptor> {
 /// without a Photoshop code refuses the step.
 pub(super) fn gallery_to(id: &str, p: &Value) -> Option<Descriptor> {
     let (code, _, keys) = GALLERY.iter().find(|(_, cmd, _)| *cmd == id)?;
-    let allowed: Vec<&str> = keys.iter().map(G::param).chain(["target"]).collect();
+    let allowed: Vec<&str> = keys.iter().map(G::param).chain(["target", RECORDED_EVENT]).collect();
     if !pixel_target(p) || !p.as_object().is_none_or(|m| m.iter().all(|(k, v)| v.is_null() || allowed.contains(&k.as_str()))) {
         return None;
     }

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

@@ -3390,3 +3390,85 @@ fn gallery_neon_glow_without_a_colour_loads_photoshops_blue() {
     let back = (events::event_for(&event).unwrap().from)(&d);
     assert_eq!(gallery_params(back), (id, p));
 }
+
+/// An effect recorded under its own event by string ID (`cutout`, as newer versions record it)
+/// maps like its 4-char code; only the effect's string IDs count as names (#10592).
+#[test]
+fn gallery_effects_recorded_by_string_id_map_like_their_codes() {
+    for (code, name, effect) in [("Ct  ", "cutout", "Ct"), ("HlfS", "halftoneScreen", "HlfS"), ("Grn ", "grain", "Grn")] {
+        let d = desc("null").with("GEfk", enm("GEft", effect));
+        let by_name = (events::event_for(&AtnEvent::name(name)).unwrap_or_else(|| panic!("{name} by name")).from)(&d);
+        let by_code = (events::event_for(&AtnEvent::Code(code.as_bytes().try_into().unwrap())).unwrap().from)(&d);
+        assert_eq!(gallery_params(by_name), gallery_params(by_code), "{name}");
+    }
+    assert!(events::event_for(&AtnEvent::name("Ct  ")).is_none());
+    assert!(events::event_for(&AtnEvent::Code(*b"MscT")).is_some());
+}
+
+/// A step recorded as the effect's own event (by code or string ID) loads, plays and saves back
+/// as that event with its panel name and descriptor unchanged; a `GEfc` step stays `GEfc`
+/// (#10592).
+#[test]
+fn gallery_effect_steps_save_back_as_the_event_they_were_recorded_as() {
+    let edges = desc("null").with("GEfk", enm("GEft", "AccE")).with("EdgW", D::Integer(2)).with("EdgB", D::Integer(38)).with("Smth", D::Integer(5));
+    let cutout = desc("null").with("GEfk", enm("GEft", "Ct")).with("NmbL", D::Integer(6)).with("EdgS", D::Integer(4)).with("EdgF", D::Integer(2));
+    let gallery = desc("null").with("GEfk", enm("GEft", "AccE")).with("EdgW", D::Integer(3)).with("EdgB", D::Integer(20)).with("Smth", D::Integer(7));
+    let source = vec![item("AccE", "Accented Edges", Some(edges)), item("cutout", "Cutout", Some(cutout)), item("GEfc", "Filter Gallery", Some(gallery))];
+    let data = set_of("S", vec![("A", source.clone())]);
+    let mut s = Session::new();
+    let loaded = s.execute("actions.load", json!({"data": data})).unwrap();
+    assert_eq!(loaded["unsupported"], json!([]), "{loaded}");
+    let ids: Vec<&str> = s.actions.list[0].steps.iter().map(|(id, _)| id.as_str()).collect();
+    assert_eq!(ids, ["filter.gallery.accentedEdges", "filter.gallery.cutout", "filter.gallery.accentedEdges"]);
+
+    s.execute("file.new", json!({"width": 16, "height": 16})).unwrap();
+    s.execute("filter.render.clouds", json!({})).unwrap();
+    s.execute("layer.smartObjects.convertToSmartObject", json!({})).unwrap();
+    let played = s.execute("actions.play", json!({"action": "A"})).unwrap();
+    assert_eq!(played["ran"], 3, "{played}");
+    // The remembered event is for Save Actions only: playback strips it, so the journal (Last
+    // Filter) and the smart filters (`.pcraft` saves) never carry it.
+    let gallery: Vec<&Value> = s.journal.iter().filter(|(id, _)| id.starts_with("filter.gallery.")).map(|(_, p)| p).collect();
+    assert_eq!(gallery.len(), 3, "{:?}", s.journal);
+    assert!(gallery.iter().all(|p| p.get("_atnEvent").is_none()), "{gallery:?}");
+    let d = s.active().unwrap();
+    let photocraft_doc::LayerContent::Smart(sm) = &d.doc.layer(d.active_layer.unwrap()).unwrap().content else { panic!("not smart") };
+    assert_eq!(sm.smart_filters.len(), 3);
+    assert!(sm.smart_filters.iter().all(|f| f.params.get("_atnEvent").is_none()), "{:?}", sm.smart_filters);
+
+    let saved = s.execute("actions.save", json!({"set": "S"})).unwrap();
+    assert_eq!(saved["skipped"], json!([]));
+    let file = atn::parse(&photocraft_paint::tile::b64_decode(saved["data"].as_str().unwrap()).unwrap()).unwrap();
+    let got: Vec<_> = file.actions[0].items.iter().map(|i| (i.event.clone(), i.dictionary.clone(), i.descriptor.clone())).collect();
+    let want: Vec<_> = source.iter().map(|i| (i.event.clone(), i.dictionary.clone(), i.descriptor.clone())).collect();
+    assert_eq!(got, want);
+
+    // A step made in PhotoCraft keeps saving as `GEfc`, and so does a forged event param.
+    assert_eq!(export_item("filter.gallery.cutout", &json!({}), "S").unwrap().event, AtnEvent::Code(*b"GEfc"));
+    let forged = json!({"_atnEvent": {"event": "GsnB", "eventKind": "code", "name": "Gaussian Blur"}});
+    assert_eq!(export_item("filter.gallery.cutout", &forged, "S").unwrap().event, AtnEvent::Code(*b"GEfc"));
+    // Another effect's event (stale or forged) does not relabel the step either.
+    for (event, kind) in [("AccE", "code"), ("accentedEdges", "name")] {
+        let other = json!({"_atnEvent": {"event": event, "eventKind": kind, "name": "Accented Edges"}});
+        let item = export_item("filter.gallery.cutout", &other, "S").unwrap();
+        assert_eq!((item.event, item.dictionary), (AtnEvent::Code(*b"GEfc"), b"Filter Gallery".to_vec()), "{event}");
+    }
+    let own = json!({"_atnEvent": {"event": "cutout", "eventKind": "name", "name": "Cutout"}});
+    assert_eq!(export_item("filter.gallery.cutout", &own, "S").unwrap().event, AtnEvent::name("cutout"));
+}
+
+/// Only a missing key takes Photoshop's default: a key there with the wrong type or a non-finite
+/// number keeps the Photoshop step instead of playing a default (#10592).
+#[test]
+fn gallery_effect_keys_with_the_wrong_type_keep_the_step() {
+    let kept = |m: Mapped| matches!(m, Mapped::Unsupported { harmless: false });
+    assert!(kept(gallery_step("GEfc", "Txtz", desc("null").with("InvT", D::Integer(1)))));
+    assert!(kept(gallery_step("GEfc", "AccE", desc("null").with("EdgW", D::Double(f64::NAN)))));
+    assert!(kept(gallery_step("GEfc", "AccE", desc("null").with("EdgW", D::Double(f64::INFINITY)))));
+    assert!(kept(gallery_step("GEfc", "AccE", desc("null").with("EdgW", text("9")))));
+    assert!(kept(gallery_step("GEfc", "BsRl", desc("null").with("LghD", D::Integer(3)))));
+    assert!(kept(gallery_step("GEfc", "NGlw", desc("null").with("Clr ", D::Integer(3)))));
+    // Missing keys still take the defaults.
+    assert_eq!(gallery_params(gallery_step("GEfc", "Txtz", desc("null"))).1["invert"], false);
+    assert_eq!(gallery_params(gallery_step("GEfc", "AccE", desc("null").with("EdgB", D::Integer(9)))).1["edgeWidth"], 2);
+}