From 51e45fd394498ba064239fdbefc7649e15415efb Mon Sep 17 00:00:00 2001 From: FinnNk Date: Fri, 23 Oct 2026 22:38:48 +0100 Subject: [PATCH] fix(engine): refuse partial Color Balance and Hue/Saturation ATN exports (#8701) * fix(engine): refuse partial Color Balance and Hue/Saturation ATN exports A `layer.setAdjustment` step changes only the settings it names, but the Color Balance and Hue/Saturation descriptors written to the ATN carry every setting and filled absent ones with defaults, so a partial edit replayed as one that also reset the rest. Strict export now refuses such a step (with skipUnsupported it is listed in Skipped); complete ones export and round-trip as before. Fixes #8531 Co-Authored-By: Claude Opus 5.5 * fix(engine): refuse Hue/Saturation ranges missing their own settings A colour range written without its own hue, saturation and lightness would replay with the missing ones reset to 0, so strict export refuses it like a partial master setting. Co-Authored-By: Claude Opus 5.5 * fix(engine): refuse wrong-shaped Color Balance and Hue/Saturation settings Strict export now checks the triplets, numbers and flags it needs are of the shape the descriptor writer reads, since it would write anything else as 0. Co-Authored-By: Claude Opus 5.5 * fix(engine): refuse malformed Hue/Saturation range bounds Co-Authored-By: Claude Opus 5.5 * test(engine): cover malformed and valid Hue/Saturation range bounds Co-Authored-By: Claude Opus 5.5 * fix(engine): refuse fractional Hue/Saturation range bounds The descriptor stores them as integers, so a fractional bound would be rounded on export. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- crates/engine/src/atn_cmds.rs | 32 ++++++++++++++++++- crates/engine/src/atn_cmds/tests.rs | 49 +++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/atn_cmds.rs b/crates/engine/src/atn_cmds.rs index ac1d2df8a0..a3f7b0e0da 100644 --- a/crates/engine/src/atn_cmds.rs +++ b/crates/engine/src/atn_cmds.rs @@ -1661,7 +1661,37 @@ fn set_to(id: &str, p: &Value) -> Option { "layer.setAdjustment" => { // The adjustment kind isn't in the params: Photoshop needs it, so only steps that // name one (`kind`) export. - let a = adjustment_by_suffix(jstr(p, "kind")?)?; + let kind = jstr(p, "kind")?; + // `layer.setAdjustment` changes only the settings it names. Color Balance and + // Hue/Saturation descriptors carry every setting, filling absent ones with defaults, + // so a partial edit would replay as one that also resets the rest (#8531). + // Present and of the shape the writer reads: anything else it would write as 0. + let number = |p: &Value, k: &str| p.get(k).and_then(Value::as_f64).is_some_and(f64::is_finite); + let numbers = |p: &Value, keys: &[&str]| keys.iter().all(|k| number(p, k)); + let flag = |k: &str| p.get(k).is_some_and(Value::is_boolean); + let triplet = + |k: &str| p.get(k).and_then(Value::as_array).is_some_and(|a| a.len() == 3 && a.iter().all(|v| v.as_f64().is_some_and(f64::is_finite))); + let complete = match kind { + "colorBalance" => ["shadows", "midtones", "highlights"].iter().all(|k| triplet(k)) && flag("preserveLuminosity"), + // Every colour range written carries its own hue, saturation and lightness too. + "hueSaturation" => { + flag("colorize") + && numbers(p, &["hue", "saturation", "lightness"]) + && HUE_RANGES.iter().filter_map(|r| p.get(*r).filter(|v| !v.is_null())).all(|r| { + // Bounds, when given, are the four whole numbers `hue_entry` writes (as integers); + // otherwise it drops or rounds them. + let bounds = r.get("range").filter(|v| !v.is_null()).is_none_or(|v| { + v.as_array().is_some_and(|a| a.len() == 4 && a.iter().all(|b| b.as_f64().is_some_and(|b| b.is_finite() && b.fract() == 0.0))) + }); + r.is_object() && numbers(r, &["hue", "saturation", "lightness"]) && bounds + }) + } + _ => true, + }; + if !complete { + return None; + } + let a = adjustment_by_suffix(kind)?; desc("null").with("null", target_ref("AdjL")).with("T ", D::Descriptor((a.to)(p)?)) } _ => return None, diff --git a/crates/engine/src/atn_cmds/tests.rs b/crates/engine/src/atn_cmds/tests.rs index 60236b57c3..065080b02e 100644 --- a/crates/engine/src/atn_cmds/tests.rs +++ b/crates/engine/src/atn_cmds/tests.rs @@ -2053,6 +2053,55 @@ fn options_the_descriptors_cannot_carry_refuse_strict_export() { } } +/// #8531: a partial `layer.setAdjustment` of Color Balance or Hue/Saturation can't be written +/// (their descriptors carry every setting, filling absent ones with defaults), so strict export +/// refuses it instead of replaying an edit that also resets the rest. Complete ones still export. +#[test] +fn partial_color_balance_and_hue_saturation_edits_refuse_strict_export() { + let one = |p: Value| Action { name: "Probe".into(), steps: vec![("layer.setAdjustment".into(), p)], ..Default::default() }; + for p in [ + json!({"kind": "colorBalance", "shadows": [32, -24, 16]}), + json!({"kind": "colorBalance", "preserveLuminosity": true}), + json!({"kind": "hueSaturation", "hue": 20}), + json!({"kind": "hueSaturation", "reds": {"hue": 10}}), + ] { + let err = export("Audit8", &[&one(p.clone())], &[], false).unwrap_err(); + assert!(err.contains("`layer.setAdjustment`"), "{p}: {err}"); + let (_, skipped) = export("Audit8", &[&one(p.clone())], &[], true).unwrap(); + assert_eq!(skipped, vec![Skipped { action: "Probe".into(), step: 0, id: "layer.setAdjustment".into() }], "{p}"); + } + let full = json!({"kind": "colorBalance", "shadows": [32, -24, 16], "midtones": [-17, 27, -31], "highlights": [11, -19, 23], "preserveLuminosity": false}); + let (bytes, _) = export("Audit8", &[&one(full)], &[], false).unwrap(); + let steps = import(&bytes).unwrap().actions[0].steps.clone(); + let (id, p) = steps.first().unwrap(); + assert_eq!(id, "layer.setAdjustment"); + assert_eq!(p.get("midtones"), Some(&json!([-17, 27, -31])), "{p}"); + assert_eq!(p.get("preserveLuminosity"), Some(&json!(false)), "{p}"); + let hue = json!({"kind": "hueSaturation", "colorize": false, "hue": 20, "saturation": -5, "lightness": 0}); + assert!(export("Audit8", &[&one(hue)], &[], false).is_ok()); + // A colour range written without all of its own settings would reset the rest too. + let range = json!({"kind": "hueSaturation", "colorize": false, "hue": 0, "saturation": 0, "lightness": 0, "reds": {"hue": 10}}); + assert!(export("Audit8", &[&one(range)], &[], false).is_err()); + let range = + json!({"kind": "hueSaturation", "colorize": false, "hue": 0, "saturation": 0, "lightness": 0, "reds": {"hue": 10, "saturation": 5, "lightness": -5}}); + assert!(export("Audit8", &[&one(range)], &[], false).is_ok()); + let bounded = json!({"kind": "hueSaturation", "colorize": false, "hue": 0, "saturation": 0, "lightness": 0, "reds": {"hue": 10, "saturation": 5, "lightness": -5, "range": [315, 345, 15, 45]}}); + assert!(export("Audit8", &[&one(bounded)], &[], false).is_ok()); + // Values of the wrong shape would be written as 0: refused too. + for bad in [ + json!({"kind": "colorBalance", "shadows": "invalid", "midtones": [0, 0, 0], "highlights": [0, 0, 0], "preserveLuminosity": true}), + json!({"kind": "colorBalance", "shadows": [1, 2], "midtones": [0, 0, 0], "highlights": [0, 0, 0], "preserveLuminosity": true}), + json!({"kind": "colorBalance", "shadows": [0, 0, 0], "midtones": [0, 0, 0], "highlights": [0, 0, 0], "preserveLuminosity": "yes"}), + json!({"kind": "hueSaturation", "colorize": false, "hue": "20", "saturation": 0, "lightness": 0}), + json!({"kind": "hueSaturation", "colorize": false, "hue": 0, "saturation": 0, "lightness": 0, "reds": 5}), + json!({"kind": "hueSaturation", "colorize": false, "hue": 0, "saturation": 0, "lightness": 0, "reds": {"hue": 1, "saturation": 0, "lightness": 0, "range": [0, 20, 40]}}), + json!({"kind": "hueSaturation", "colorize": false, "hue": 0, "saturation": 0, "lightness": 0, "reds": {"hue": 1, "saturation": 0, "lightness": 0, "range": "invalid"}}), + json!({"kind": "hueSaturation", "colorize": false, "hue": 0, "saturation": 0, "lightness": 0, "reds": {"hue": 1, "saturation": 0, "lightness": 0, "range": [315, 345.5, 15, 45]}}), + ] { + assert!(export("Audit8", &[&one(bad.clone())], &[], false).is_err(), "{bad}"); + } +} + /// Photoshop's Close has no document target: a Close of a specific document must not be /// exported as a plain Close, which would close the active document on playback. #[test]