mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-29 03:46:50 +01:00
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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * fix(engine): refuse malformed Hue/Saturation range bounds Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(engine): cover malformed and valid Hue/Saturation range bounds Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
FinnNkandClaude Opus 5.5 authored and GitHub committed
1 parent
b8ee65e030
commit
51e45fd394
2 files changed
+80
-1
No files matched your search
@@ -1661,7 +1661,37 @@ fn set_to(id: &str, p: &Value) -> Option<Descriptor> {
|
||||
"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,
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in new issue
Block a user