mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-14 19:26:51 -04:00
Actions: record layer selections by name and display the target (#2441)
* Record action layer selections by name and show their targets * Use document pixel format in grouped-layer regression * Document the named layer action UI with a synthetic capture
This commit is contained in:
6 files changed
+189
-3
No files matched your search
@@ -957,6 +957,109 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recorded_layer_selection_replays_by_name_on_another_document() {
|
||||
for depth in [8, 16, 32] {
|
||||
let mut s = Session::new();
|
||||
s.execute("file.new", json!({"width": 8, "height": 8, "depth": depth})).unwrap();
|
||||
let original = s.active().unwrap().active_layer.unwrap();
|
||||
s.execute("layer.setProps", json!({"name": "Photo"})).unwrap();
|
||||
s.execute("layer.new.layer", json!({"name": "LogoBase"})).unwrap();
|
||||
s.execute("actions.record", json!({"name": "Duplicate photo"})).unwrap();
|
||||
s.execute("layer.select", json!({"layer": original.0, "mode": "replace"})).unwrap();
|
||||
// The target name must be captured now, not when recording stops.
|
||||
s.execute("layer.setProps", json!({"name": "Retouched"})).unwrap();
|
||||
s.execute("layer.duplicate", json!({})).unwrap();
|
||||
s.execute("actions.stop", json!({})).unwrap();
|
||||
assert_eq!(s.actions.list[0].steps[0].1, json!({"name":"Photo", "mode":"replace"}));
|
||||
// Persisted actions must work in an entirely different session too.
|
||||
let saved = serde_json::to_string(&s.actions.list).unwrap();
|
||||
let mut replay = Session::new();
|
||||
replay.actions.list = serde_json::from_str(&saved).unwrap();
|
||||
replay.execute("file.new", json!({"width": 8, "height": 8, "depth": depth})).unwrap();
|
||||
replay.execute("layer.setProps", json!({"name": "Photo"})).unwrap();
|
||||
let target = replay.active().unwrap().active_layer.unwrap();
|
||||
assert_ne!(original, target);
|
||||
replay.execute("layer.new.layer", json!({"name": "LogoBase"})).unwrap();
|
||||
let result = replay.execute("actions.play", json!({"action": 0})).unwrap();
|
||||
assert!(result.get("failed").is_none(), "{result}");
|
||||
assert_eq!(result["ran"], 3);
|
||||
assert_eq!(replay.active().unwrap().doc.layer(target).unwrap().name, "Retouched");
|
||||
let active = replay.active().unwrap().active_layer.unwrap();
|
||||
assert_eq!(replay.active().unwrap().doc.layer(active).unwrap().name, "Retouched copy");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn named_selection_preserves_modes_and_duplicate_occurrences() {
|
||||
let mut s = Session::new();
|
||||
s.execute("file.new", json!({"width":4,"height":4})).unwrap();
|
||||
s.execute("layer.setProps", json!({"name":"Photo"})).unwrap();
|
||||
let first = s.active().unwrap().active_layer.unwrap();
|
||||
s.execute("layer.new.group", json!({"name":"Group"})).unwrap();
|
||||
// A duplicate nested in a group must be found even when that group is collapsed.
|
||||
let second = s
|
||||
.edit("child", |doc, active| {
|
||||
let child = photocraft_doc::Layer::raster("Photo", doc.pixel_format());
|
||||
let id = child.id;
|
||||
if let photocraft_doc::LayerContent::Group(group) = &mut doc.layer_mut(active.unwrap()).unwrap().content {
|
||||
group.children.push(child);
|
||||
group.expanded = false;
|
||||
}
|
||||
Ok(id)
|
||||
})
|
||||
.unwrap();
|
||||
s.execute("actions.record", json!({"name":"Select duplicates"})).unwrap();
|
||||
s.execute("layer.select", json!({"layer":first.0})).unwrap();
|
||||
s.execute("layer.select", json!({"layer":second.0,"mode":"add"})).unwrap();
|
||||
s.execute("actions.stop", json!({})).unwrap();
|
||||
assert_eq!(s.actions.list[0].steps[0].1, json!({"name":"Photo","occurrence":0}));
|
||||
assert_eq!(s.actions.list[0].steps[1].1, json!({"name":"Photo","occurrence":1,"mode":"add"}));
|
||||
s.execute("layer.select", json!({"layer":first.0})).unwrap();
|
||||
let r = s.execute("actions.play", json!({"action":0})).unwrap();
|
||||
assert!(r.get("failed").is_none(), "{r}");
|
||||
assert_eq!(s.active().unwrap().selected_layers(), vec![first, second]);
|
||||
s.execute("layer.select", json!({"name":"Photo","occurrence":1,"mode":"toggle"})).unwrap();
|
||||
assert_eq!(s.active().unwrap().selected_layers(), vec![first]);
|
||||
s.execute("layer.select", json!({"name":"Photo","occurrence":1,"mode":"range"})).unwrap();
|
||||
assert!(s.active().unwrap().selected_layers().contains(&second));
|
||||
// Main's imported .atn steps use a string-valued `layer` and choose the topmost match.
|
||||
s.execute("layer.select", json!({"layer":"Photo"})).unwrap();
|
||||
assert_eq!(s.active().unwrap().selected_layers(), vec![second]);
|
||||
s.execute("layer.select", json!({"name":"Photo","occurrence":0,"mode":"add"})).unwrap();
|
||||
s.execute("layer.select", json!({"name":"Photo","occurrence":1,"mode":"remove"})).unwrap();
|
||||
assert_eq!(s.active().unwrap().selected_layers(), vec![first]);
|
||||
// Removing an unselected layer does not add it; removing the last target keeps it.
|
||||
s.execute("layer.select", json!({"layer":"Photo","mode":"remove"})).unwrap();
|
||||
s.execute("layer.select", json!({"name":"Photo","occurrence":0,"mode":"remove"})).unwrap();
|
||||
assert_eq!(s.active().unwrap().selected_layers(), vec![first]);
|
||||
for p in [
|
||||
json!({"name":"absent"}),
|
||||
json!({"name":"Photo"}),
|
||||
json!({"name":1}),
|
||||
json!({"name":"Photo","occurrence":-1}),
|
||||
json!({"name":"Photo","occurrence":1.5}),
|
||||
json!({"name":"Photo","occurrence":99}),
|
||||
json!({"name":"Photo","layer":first.0}),
|
||||
json!({"layer":first.0,"occurrence":0}),
|
||||
] {
|
||||
let before = s.active().unwrap().selected_layers();
|
||||
assert!(s.execute("layer.select", p.clone()).is_err(), "{p}");
|
||||
assert_eq!(s.active().unwrap().selected_layers(), before);
|
||||
}
|
||||
s.actions.list.push(Action {
|
||||
name: "Missing".into(),
|
||||
steps: vec![("layer.select".into(), json!({"name":"absent"})), ("layer.duplicate".into(), json!({}))],
|
||||
disabled: Default::default(),
|
||||
..Default::default()
|
||||
});
|
||||
let before = s.active().unwrap().doc.walk().len();
|
||||
let result = s.execute("actions.play", json!({"action":"Missing"})).unwrap();
|
||||
assert_eq!(result["ran"], 0);
|
||||
assert!(result["failed"]["error"].as_str().unwrap().contains("absent"));
|
||||
assert_eq!(s.active().unwrap().doc.walk().len(), before);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn play_stops_at_the_failing_step_and_reports_it() {
|
||||
let mut s = Session::new();
|
||||
|
||||
@@ -690,7 +690,7 @@ fn build() -> Vec<CommandSpec> {
|
||||
"Select Layer",
|
||||
[],
|
||||
None,
|
||||
r##"{"layer":id,"mode":"replace|toggle|range|add|remove"="replace"} (toggle = ⌘-click, range = ⇧-click, remove = deselect it, keeping at least one)"##,
|
||||
r##"{"layer":id|str | "name":str,"occurrence":index?,"mode":"replace|toggle|range|add|remove"="replace"} (name occurrence: zero-based document walk order; required for duplicate names; toggle = ⌘-click, range = ⇧-click, remove = deselect it, keeping at least one)"##,
|
||||
has_doc,
|
||||
crate::layer_multi_cmds::select
|
||||
),
|
||||
|
||||
@@ -724,6 +724,8 @@ impl Session {
|
||||
self.journal.truncate(from);
|
||||
}
|
||||
if !crate::brush_cmds::coalesce_journal(self, id, ¶ms) {
|
||||
let params =
|
||||
if id == "layer.select" && self.actions.recording.is_some() { crate::layer_multi_cmds::recorded_selection(self, params) } else { params };
|
||||
self.journal.push((id.to_string(), params));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -100,6 +100,48 @@ pub(crate) fn reselect(s: &mut Session, ids: Vec<LayerId>, active: Option<LayerI
|
||||
}
|
||||
}
|
||||
|
||||
/// Capture a selection target while its name is still known. Later renames/deletions must not
|
||||
/// change an earlier recorded selection. Direct, non-recording calls retain their numeric IDs.
|
||||
pub(crate) fn recorded_selection(s: &Session, mut p: Value) -> Value {
|
||||
let Some(id) = p.get("layer").and_then(Value::as_u64).map(LayerId) else { return p };
|
||||
let Some(st) = s.active() else { return p };
|
||||
let Some(layer) = st.doc.layer(id) else { return p };
|
||||
let matches: Vec<_> = st.doc.walk().into_iter().filter(|(_, _, l)| l.name == layer.name).map(|(_, _, l)| l.id).collect();
|
||||
let Some(occurrence) = matches.iter().position(|candidate| *candidate == id) else { return p };
|
||||
if let Some(fields) = p.as_object_mut() {
|
||||
fields.remove("layer");
|
||||
fields.insert("name".into(), json!(layer.name));
|
||||
if matches.len() > 1 {
|
||||
fields.insert("occurrence".into(), json!(occurrence));
|
||||
}
|
||||
}
|
||||
p
|
||||
}
|
||||
|
||||
fn selection_target(s: &Session, p: &Value) -> Result<LayerId> {
|
||||
let Some(name) = p.get("name") else {
|
||||
if p.get("occurrence").is_some() {
|
||||
return Err(bad("layer.select", "`occurrence` requires `name`"));
|
||||
}
|
||||
return crate::commands::explicit_layer(s, p)?.ok_or_else(|| bad("layer.select", "needs `layer` or `name`"));
|
||||
};
|
||||
if p.get("layer").is_some() {
|
||||
return Err(bad("layer.select", "use either `layer` or `name`, not both"));
|
||||
}
|
||||
let name = name.as_str().ok_or_else(|| bad("layer.select", "`name` must be a string"))?;
|
||||
let occurrence = p
|
||||
.get("occurrence")
|
||||
.map(|v| v.as_u64().and_then(|n| usize::try_from(n).ok()).ok_or_else(|| bad("layer.select", "`occurrence` must be a non-negative integer")))
|
||||
.transpose()?;
|
||||
let st = s.active().ok_or(EngineError::NoDocument)?;
|
||||
let matches: Vec<_> = st.doc.walk().into_iter().filter(|(_, _, l)| l.name == name).map(|(_, _, l)| l.id).collect();
|
||||
if matches.len() > 1 && occurrence.is_none() {
|
||||
return Err(bad("layer.select", format!("multiple layers named `{name}`; specify `occurrence`")));
|
||||
}
|
||||
let index = occurrence.unwrap_or(0);
|
||||
matches.get(index).copied().ok_or_else(|| bad("layer.select", format!("no layer named `{name}` at occurrence {index}")))
|
||||
}
|
||||
|
||||
/// Layer rows follow the Layers panel's visual order, not Document::walk's storage order.
|
||||
/// Closed groups hide their children unless both endpoints were selected programmatically.
|
||||
fn range_order(layers: &[Layer], include_collapsed: bool, out: &mut Vec<LayerId>) {
|
||||
@@ -115,7 +157,7 @@ fn range_order(layers: &[Layer], include_collapsed: bool, out: &mut Vec<LayerId>
|
||||
|
||||
/// `layer.select`: `{"layer": id, "mode": "replace|toggle|range|add|remove"}`.
|
||||
pub fn select(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let id = crate::commands::explicit_layer(s, p)?.ok_or_else(|| bad("layer.select", "missing `layer`"))?;
|
||||
let id = selection_target(s, p)?;
|
||||
let mode = p.get("mode").and_then(Value::as_str).unwrap_or("replace");
|
||||
let st = s.active().ok_or(EngineError::NoDocument)?;
|
||||
if st.doc.layer(id).is_none() {
|
||||
|
||||
@@ -250,6 +250,14 @@ fn step_label(id: &str, params: &Value) -> String {
|
||||
if id == "actions.play" {
|
||||
let target = params.get("action").map(|v| v.as_str().map(str::to_owned).unwrap_or_else(|| v.to_string())).unwrap_or_default();
|
||||
format!("{}: {target}", label_of(id))
|
||||
} else if id == "layer.select" {
|
||||
let target = params
|
||||
.get("name")
|
||||
.and_then(Value::as_str)
|
||||
.or_else(|| params.get("layer").and_then(Value::as_str))
|
||||
.map(str::to_owned)
|
||||
.or_else(|| params.get("layer").and_then(Value::as_u64).map(|id| format!("ID {id}")));
|
||||
target.map_or_else(|| label_of(id), |target| format!("{} ({target})", label_of(id)))
|
||||
} else if id == "actions.conditional" {
|
||||
// "If hasLayers: FS8-Layers / FS8-No-Layers"
|
||||
let cond = params.get("if").and_then(Value::as_str).unwrap_or("?");
|
||||
@@ -911,6 +919,26 @@ pub fn panel(app: &mut PhotocraftApp, ui: &mut egui::Ui) {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn selected_layer_name_is_visible_during_recording_and_after_stop() {
|
||||
use egui_kittest::{Harness, kittest::Queryable};
|
||||
let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
|
||||
app.run("file.new", json!({"width":4,"height":4})).unwrap();
|
||||
app.run("layer.setProps", json!({"name":"Background"})).unwrap();
|
||||
let layer = app.session.active().unwrap().active_layer.unwrap();
|
||||
app.run("actions.record", json!({"name":"Select photo"})).unwrap();
|
||||
app.ui.actions.expanded = vec![true];
|
||||
app.run("layer.select", json!({"layer":layer.0})).unwrap();
|
||||
let mut h = Harness::builder().with_size(vec2(400.0, 260.0)).build_ui_state(|ui, app| panel(app, ui), app);
|
||||
h.run_steps(4);
|
||||
h.get_by_label("Select Layer (Background)");
|
||||
h.state_mut().run("actions.stop", json!({})).unwrap();
|
||||
h.run_steps(4);
|
||||
h.get_by_label("Select Layer (Background)");
|
||||
assert_eq!(step_label("layer.select", &json!({"layer":26})), "Select Layer (ID 26)");
|
||||
assert_eq!(step_label("layer.select", &json!({"layer":"Background"})), "Select Layer (Background)");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn actions_list_grows_with_panel_and_keeps_footer_visible() {
|
||||
use egui_kittest::{Harness, kittest::Queryable};
|
||||
|
||||
@@ -100,7 +100,7 @@ store expose and persist the same setting; scripts keep using canonical command
|
||||
|---|---|
|
||||
| `file.new` | `{"width":1920,"height":1080,"mode":"rgb","depth":8,"background":"white"}` |
|
||||
| `layer.new.layer` | `{"name":"Ink"}` |
|
||||
| `layer.select` | `{"layer":id,"mode":"replace\|toggle\|range\|add"}`: ⌘-click = toggle, ⇧-click = range; `document.inspect` reports `selectedLayers` and a per-layer `selected` flag |
|
||||
| `layer.select` | `{"layer":id\|str,"mode":"replace\|toggle\|range\|add\|remove"}` or `{"name":"Background","occurrence":0?}` (occurrence is zero-based document walk order; required for duplicate names): ⌘-click = toggle, ⇧-click = range; `document.inspect` reports `selectedLayers` and a per-layer `selected` flag |
|
||||
| `channel.target` / `channel.setVisible` | Channels panel target and eyes (`"composite"`, colour name, alpha index or name, `"quickMask"`). `document.inspect` (and `channel.list`) report `channels`: composite/colour/alpha rows with visibility, channel options, `quickMask`, `target`. Pixel commands without a `target` param follow the targeted channel |
|
||||
| `layer.setProps` | `{"layer":id?,"name":…,"visible":…,"opacity":0..1,"blend":"Multiply"}` |
|
||||
| `layer.fill.solidColor.set` | `{"layer":id?,"document":id?,"color":"#rrggbb"}`: edit a Solid Color Fill in one undo step; RGB without alpha preserves the fill's alpha. Float RGB arrays and a stored `Color` are also accepted. `document` rejects stale dialog targets. |
|
||||
@@ -547,6 +547,17 @@ visibility reset when the dialog opens. HDR scopes are not implemented. See
|
||||
|
||||
### Editing and composing actions
|
||||
|
||||
Newly recorded Select Layer steps store the layer name instead of a document-specific ID,
|
||||
and the Actions panel shows `Select Layer (Background)`. Duplicate names include a zero-based
|
||||
`occurrence` in document walk order (including layers inside groups); keep that order consistent
|
||||
between source and target documents. A missing name or occurrence stops playback; a name without
|
||||
an occurrence is rejected when ambiguous. Names are captured at selection time, so a later
|
||||
rename in the action does not change the earlier target. Numeric `layer` IDs remain supported
|
||||
for direct commands and older recordings, which the panel labels `Select Layer (ID 26)`.
|
||||
String-valued `layer` targets from imported Photoshop actions keep main's topmost-name matching
|
||||
and show the same named label; `remove` selection mode is supported for either target form.
|
||||
Re-record old Select Layer steps: their original names cannot be recovered from an ID alone.
|
||||
|
||||
Creating an action scrolls its row into view; newly recorded steps are revealed immediately.
|
||||
Drag a step above or below another step in the same action to reorder it. Drag an action header
|
||||
to reorder the action list. The insertion line marks the drop position; dragging near the rows'
|
||||
|
||||
Reference in new issue
Block a user