Fix Artboard action round trips and singleton links

This commit is contained in:
yoriswind committed 2026-11-01 17:48:22 -08:00
1 parent c06d7d1e34
commit 7b0d68c8e3
7 files changed
+115 -11

No files matched your search

+1
View File
@@ -332,6 +332,7 @@ fn artboard_from_group(s: &mut Session, p: &Value) -> Result<Value> {
*active = Some(id);
Ok(rect)
})?;
crate::layer_multi_cmds::set_selection(s, vec![id], Some(id), Some(id))?;
Ok(json!({"layer": id.0, "rect": [rect.x0, rect.y0, rect.width(), rect.height()]}))
}
@@ -20,8 +20,12 @@ fn settings_from(board: &Descriptor, preset_key: &str) -> Option<Value> {
};
let Some(D::List(guides)) = board.get("guideIDs") else { return None };
let guides: Vec<i32> = guides.iter().map(|v| if let D::Integer(n) = v { Some(*n) } else { None }).collect::<Option<_>>()?;
let rgb: Vec<f64> = ["Rd ", "Grn ", "Bl "].iter().map(|key| get_num(color, key).map(|n| n / 255.0)).collect::<Option<_>>()?;
let recorded: Vec<f64> = ["Rd ", "Grn ", "Bl "].iter().map(|key| get_num(color, key)).collect::<Option<_>>()?;
let rgb: Vec<f64> = recorded.iter().map(|n| n / 255.0).collect();
let mut p = json!({"background": background, "color": rgb, "guideIDs": guides});
if recorded.iter().zip(&rgb).any(|(recorded, native)| *recorded != native * 255.0) {
p["recordedColor"] = json!(recorded);
}
p[preset_key] = preset.into();
if board.get("artboardBackgroundType").is_none() {
p["backgroundTypeOmitted"] = true.into();
@@ -48,7 +52,17 @@ fn settings_to(p: &Value, preset_key: &str) -> Option<Descriptor> {
if !rgb.iter().all(|n| n.is_finite() && (0.0..=1.0).contains(n)) {
return None;
}
let color = ["Rd ", "Grn ", "Bl "].into_iter().zip(rgb).fold(desc("RGBC"), |d, (key, n)| d.with(key, D::Double(n * 255.0)));
let recorded = match p.get("recordedColor") {
None => rgb.iter().map(|n| n * 255.0).collect(),
Some(value) => {
let values = value.as_array()?.iter().map(Value::as_f64).collect::<Option<Vec<_>>>()?;
if values.len() != 3 || values.iter().zip(&rgb).any(|(wire, native)| !(0.0..=255.0).contains(wire) || wire / 255.0 != *native) {
return None;
}
values
}
};
let color = ["Rd ", "Grn ", "Bl "].into_iter().zip(recorded).fold(desc("RGBC"), |d, (key, n)| d.with(key, D::Double(n)));
let board = desc("artboard").with("guideIDs", D::List(guides)).with("artboardPresetName", text(preset)).with("Clr ", D::Descriptor(color));
match p.get("backgroundTypeOmitted") {
None => Some(board.with("artboardBackgroundType", D::Integer(background))),
@@ -72,17 +86,25 @@ pub(super) fn artboard_from(d: &Descriptor) -> Mapped {
if artboard_to(FROM_GROUP, &p).is_some_and(|expected| expected.items == d.items) { step(FROM_GROUP, p) } else { unsupported }
}
/// Save complete recorded conversion settings; unknown native options must not disappear.
/// Fill behavior-neutral metadata for native settings; unknown options must not disappear.
pub(super) fn artboard_to(_: &str, p: &Value) -> Option<Descriptor> {
let fields = p.as_object()?;
let d = desc("null").with("null", target_ref("Lyr "));
if fields.is_empty() {
return Some(d);
}
if fields.keys().any(|k| !["background", "color", "preset", "guideIDs", "changeBackground", "backgroundTypeOmitted"].contains(&k.as_str())) {
if fields.keys().any(|k| !["background", "color", "recordedColor", "preset", "guideIDs", "changeBackground", "backgroundTypeOmitted"].contains(&k.as_str()))
{
return None;
}
let d = d.with("artboard", D::Descriptor(settings_to(p, "preset")?));
let mut settings = p.clone();
let fields = settings.as_object_mut()?;
let background = if fields.contains_key("color") { "custom" } else { "white" };
// Native custom-without-color means white. Other types ignore this last-used color.
for (key, value) in [("background", json!(background)), ("color", json!([1.0, 1.0, 1.0])), ("preset", json!("")), ("guideIDs", json!([]))] {
fields.entry(key).or_insert(value);
}
let d = d.with("artboard", D::Descriptor(settings_to(&settings, "preset")?));
match p.get("changeBackground") {
None => Some(d),
// Older native action files used true for the recorded integer 1.
@@ -108,7 +130,8 @@ pub(super) fn artboard_edit_from(d: &Descriptor) -> Mapped {
pub(super) fn artboard_edit_to(_: &str, p: &Value) -> Option<Descriptor> {
let fields = p.as_object()?;
if fields.keys().any(|k| {
!["background", "color", "recordedPreset", "guideIDs", "recordedRect", "changeSizes", "changeBackground", "backgroundTypeOmitted"].contains(&k.as_str())
!["background", "color", "recordedColor", "recordedPreset", "guideIDs", "recordedRect", "changeSizes", "changeBackground", "backgroundTypeOmitted"]
.contains(&k.as_str())
}) || p.get("changeSizes")?.as_i64()? != 8
|| p.get("changeBackground")?.as_i64()? != 1
{
+7
View File
@@ -737,6 +737,13 @@ fn every_mapped_command_exports_and_imports_back() {
Mapped::Unsupported { .. } => panic!("{id} does not import back"),
}
}
// Partial native backgrounds need no recorded preset, guide IDs or last-used color.
for background in ["white", "black", "transparent", "custom"] {
let item = export_item("layer.new.artboardFromGroup", &json!({"background":background}), "S").unwrap();
let Mapped::Steps(steps) = (event_for(&item.event).unwrap().from)(item.descriptor.as_ref().unwrap()) else { panic!("Artboard did not import") };
assert_eq!(steps, [("layer.new.artboardFromGroup".into(), json!({"background":background,"color":[1.0,1.0,1.0],"preset":"","guideIDs":[]}))]);
assert_eq!(export_item(&steps[0].0, &steps[0].1, "S").unwrap().descriptor, item.descriptor);
}
// Every command the table claims to export has a case above.
let covered: std::collections::BTreeSet<&str> = cases.iter().map(|(id, _)| *id).collect();
let mut missing: Vec<&str> = EVENTS.iter().flat_map(|e| e.ids.iter().copied()).filter(|id| !covered.contains(id)).collect();
@@ -17,13 +17,14 @@ pub(super) fn link_layers(s: &mut Session, p: &Value) -> Result<Value> {
let same_group = first.is_some() && groups.iter().all(|group| *group == first);
// A recorded Link/Unlink is an operation, not the UI's state-dependent toggle.
// An explicit repeat must neither allocate a group nor add an undo entry.
if requested == Some(true) && same_group || requested == Some(false) && groups.iter().all(Option::is_none) {
let linked_group = same_group && doc.walk().iter().filter(|(_, _, layer)| layer.link_group == first).take(2).count() == 2;
if requested == Some(true) && linked_group || requested == Some(false) && groups.iter().all(Option::is_none) {
return Ok(json!({"linked": requested == Some(true)}));
}
let label = if requested == Some(false) { "Unlink Layers" } else { "Link Layers" };
let linked = s.edit(label, |doc, _| {
// Already one link group (or a single linked layer): unlink, like Photoshop's toggle.
let unlink = requested.map_or(same_group, |linked| !linked);
let unlink = requested.map_or(same_group, |linked| !linked) || sel.len() < 2;
// A loaded file can store any id up to u64::MAX, so the new id must not be a bare
// max + 1 (#961). Unlinking needs no new id at all.
let next = if unlink { None } else { Some(doc.unused_link_group().ok_or_else(|| bad("layer.linkLayers", "no free link group id is left"))?) };
+70 -1
View File
@@ -130,6 +130,26 @@ fn unlink_cleans_singletons_preserves_other_groups_and_undoes() {
assert!(!one.is_enabled("layer.linkLayers"), "the UI toggle still requires two unlinked layers");
}
/// Explicit Link cleans an orphan without touching a real group's unselected member.
#[test]
fn explicit_link_cleans_a_singleton_but_preserves_a_linked_pair() {
for initial in [vec![Some(77)], vec![Some(77), None], vec![Some(77), Some(77)]] {
let mut s = fixture(&initial, &[0]);
let before = s.active().unwrap().doc.clone();
let history = s.active().unwrap().history.past_len();
let linked = initial == [Some(77), Some(77)];
assert_eq!(s.execute("layer.linkLayers", json!({"linked": true})).unwrap()["linked"], linked);
if linked {
assert_eq!(s.active().unwrap().doc, before);
assert_eq!(s.active().unwrap().history.past_len(), history);
} else {
assert!(groups(&s).iter().all(Option::is_none));
assert!(s.undo());
assert_eq!(s.active().unwrap().doc, before);
}
}
}
#[test]
fn invalid_link_state_fails_without_editing() {
let mut s = fixture(&[None, None], &[0, 1]);
@@ -511,6 +531,55 @@ fn artboard_target_only_form_keeps_existing_native_group_behavior() {
assert_eq!(s.active().unwrap().doc, before);
}
/// A conversion selects its result, so a following Delete cannot remove another selected layer.
#[test]
fn artboard_from_group_replaces_the_previous_layer_selection() {
let mut s = artboard_fixture();
let group = s.active().unwrap().active_layer.unwrap();
let doc = Arc::make_mut(&mut s.active_mut().unwrap().doc);
let other = Layer::raster("Other", doc.pixel_format());
let id = other.id;
doc.layers.push(other);
s.restore_layer_selection(Some(group), &[id, group], Some(id)).unwrap();
load(&mut s, &file("artboardFromLayerGroupEvent", Descriptor::new("null").with("null", target())));
play(&mut s);
assert_eq!(s.active().unwrap().selected_layers(), vec![group]);
s.execute("layer.delete", json!({})).unwrap();
assert!(s.active().unwrap().doc.layer(id).is_some());
assert!(s.active().unwrap().doc.layer(group).is_none());
}
/// Preserve fractional wire values exactly while playback uses normalized native RGB.
#[test]
fn artboard_fractional_colors_survive_import_save_and_playback() {
let rgb = [7.97, 31.88, 127.6];
for event in ["artboardFromLayerGroupEvent", "editArtboardEvent"] {
let mut s = artboard_fixture();
let mut d = if event == "editArtboardEvent" {
s.execute("layer.new.artboardFromGroup", json!({})).unwrap();
artboard_edit_descriptor(4, 8)
} else {
artboard_descriptor(4, vec![], D::Integer(1))
};
let D::Descriptor(board) = &mut d.items.iter_mut().find(|(key, _)| key.is("artboard")).unwrap().1 else { unreachable!() };
board.items.iter_mut().find(|(key, _)| key.is("Clr ")).unwrap().1 =
D::Descriptor(["Rd ", "Grn ", "Bl "].into_iter().zip(rgb).fold(Descriptor::new("RGBC"), |color, (key, n)| color.with(key, D::Double(n))));
assert_eq!(load(&mut s, &file(event, d.clone()))["unsupported"], json!([]));
assert_eq!(saved_item(&mut s).descriptor, Some(d));
let (id, mut changed) = s.actions.list[0].steps[0].clone();
changed["color"][0] = 0.5.into();
assert!(atn_cmds::export("Links", &[&Action::new("Changed", vec![(id, changed)])], &[], false).is_err());
let before = s.active().unwrap().doc.clone();
play(&mut s);
let pixel = photocraft_compose::render(&s.active().unwrap().doc, photocraft_doc::Rect::new(3, 3, 4, 4)).px[0];
for (actual, expected) in pixel.into_iter().zip(rgb.map(|n| n as f32 / 255.0)) {
assert!((actual - expected).abs() < 0.005, "{event}: {pixel:?}");
}
assert!(s.undo());
assert_eq!(s.active().unwrap().doc, before);
}
}
/// Unproved settings remain byte-preserving kept steps and cannot partially edit the document.
#[test]
fn artboard_unmeasured_types_and_extra_settings_stay_whole() {
@@ -532,7 +601,7 @@ fn artboard_unmeasured_types_and_extra_settings_stay_whole() {
assert_eq!(s.active().unwrap().doc, before);
assert_eq!(s.active().unwrap().history.past_len(), history);
}
for p in [json!({"layer":1}), json!({"preset":"iPhone 14"}), json!({"guideIDs":[]}), json!({"changeBackground":false})] {
for p in [json!({"layer":1}), json!({"preset":7}), json!({"guideIDs":[7.5]}), json!({"changeBackground":false})] {
let action = Action::new("A", vec![("layer.new.artboardFromGroup".into(), p)]);
assert!(atn_cmds::export("Links", &[&action], &[], false).is_err());
}
+3
View File
@@ -440,6 +440,9 @@ commands record).
makes a white artboard (type 1, `Clr ` 255/255/255), and the step's result then has no `changeBackground` key.
`artboardBackgroundType` absent with `changeBackground` 1 gives type 1 (white).
- The artboard takes the group's bounds; `Clr ` is stored on the artboard as given even when the type is 1..3.
- PhotoCraft preserves fractional recorded RGB values exactly. Native actions with partial settings export an empty preset and guide list when absent.
An absent native color exports as white: it matches the native custom-color fallback and is visually ignored for white, black and transparent backgrounds.
This is canonical export metadata, not a claim about the reference application's remembered color. Imported steps retain their recorded color instead.
## Measured through dialogs and recordings
+2 -2
View File
@@ -148,8 +148,8 @@ Embedded SVG and PDF-compatible AI/PDF/EPS payloads can use an explicit VectorCr
| Date | Change | Summary |
|---|---|---|
| 2026-11-01 | minor | `.atn`: 22 newer measured forms mapped, including explicit Link/Unlink, Smart Object file operations, Make Type to Shape, radius-only Shape Blur and the Crop Photos parent; 138 supported event names. Artboard from Group and generated Crop helper steps remain kept. |
| 2026-11-01 | minor | `.atn`: event reference [atn.md](atn.md): the 117 supported events, and measured keys, defaults, ranges and enum values for 101 events not mapped yet |
| 2026-11-01 | minor | `.atn`: 23 newer measured forms mapped, including explicit Link/Unlink, Smart Object file operations, Make Type to Shape, radius-only Shape Blur, Artboard from Group and the Crop Photos parent; 139 supported event names. Artboard forms with unmeasured settings and generated Crop helper steps remain kept. |
| 2026-11-01 | minor | `.atn`: event reference [atn.md](atn.md): the 139 supported events, and measured keys, defaults, ranges and enum values for 101 events not mapped yet |
| 2026-10-26 | minor | Embedded AI/PDF/EPS save-back retains native format and a separate source-bound display preview; original sources remain undoable. Native Adobe interoperability and standalone EPS/DCS remain unverified; no corpus floor or percentage changed. |
| 2026-10-22 | minor | Embedded PSD vectors gain explicit bounded SVG editor/save-back; standalone EPS/DCS and native-format fidelity remain open. |
| 2026-10-22 | minor | TIFF: flat pages up to 300,000 px a side open (was 2^30 px, about 32,768²): `codecs::TiffBands` decodes a group of rows at a time into the layer's tiles. The 117,504 × 22,464 px banner from #8456 opens in 3.3 s at 11.45 GB peak for a 10.56 GB layer; 30,000² 6.8 s / 6.43 GB → 1.1 s / 3.86 GB; 36 MP 220 → 52 ms. Layered TIFFs still decode whole (#8169) |