diff --git a/crates/automation/src/headless.rs b/crates/automation/src/headless.rs index 1e3e83e1bf..bed78c0f65 100644 --- a/crates/automation/src/headless.rs +++ b/crates/automation/src/headless.rs @@ -261,7 +261,10 @@ impl Headless { files::save(&doc, &target, format, opts, Some(writer))? } Filesystem::Workspace(workspace) => { - let (bytes, warnings) = files::save_bytes(&doc, target_text, format, opts)?; + // Only the target is granted: a linked smart object's stored path is document + // data, so PSD export writes that layer as pixels instead of embedding the file. + let opts = ExportOptions { read_linked_files: false, ..opts.clone() }; + let (bytes, warnings) = files::save_bytes(&doc, target_text, format, &opts)?; workspace.write(target_text, &bytes)?; warnings } diff --git a/crates/automation/src/rpc.rs b/crates/automation/src/rpc.rs index c0369c917d..a0923a6744 100644 --- a/crates/automation/src/rpc.rs +++ b/crates/automation/src/rpc.rs @@ -836,6 +836,142 @@ mod tests { std::fs::remove_dir_all(base).unwrap(); } + #[test] + fn doc_export_data_sets_judges_pixel_replacement_paths_like_apply() { + let base = std::env::temp_dir().join(format!("pc-doc-data-set-pixels-{}", std::process::id())); + let root = base.join("workspace"); + std::fs::create_dir_all(&root).unwrap(); + let red = photocraft_codecs::Image::from_u8(4, 4, photocraft_codecs::ChannelLayout::Rgb, [255, 0, 0].repeat(16)).unwrap(); + let photo = base.join("photo.png"); + std::fs::write(&photo, photocraft_codecs::encode(&red, photocraft_codecs::Format::Png, &Default::default()).unwrap()).unwrap(); + let mut h = Headless::with_workspace(crate::AuthorizedWorkspace::new(Some(&root), Some(&root)).unwrap()); + h.handle("doc.new", json!({"width": 8, "height": 8, "background": "white", "name": "card"})).unwrap(); + let layer = h.handle("engine.execute", json!({"command": "layer.new.layer", "params": {}})).unwrap()["layer"].clone(); + h.handle( + "engine.execute", + json!({"command": "image.variables.define", "params": {"defs": [{"name": "photo", "layer": layer, "type": "pixelReplacement"}]}}), + ) + .unwrap(); + // Storing the path is allowed; reading it is what the gate judges. + h.handle( + "engine.execute", + json!({"command": "image.variables.dataSets", "params": {"dataSets": [ + {"name": "swap", "values": [{"variable": "photo", "kind": "pixels", "value": photo.to_string_lossy()}]}, + ]}}), + ) + .unwrap(); + let applied = h.handle("engine.execute", json!({"command": "image.applyDataSet", "params": {"name": "swap"}})).unwrap_err().to_string(); + assert!(applied.contains("ambient filesystem paths"), "{applied}"); + let sets = root.join("sets"); + let exported = h.handle("doc.exportDataSets", json!({"dir": sets.to_string_lossy(), "format": "png"})).unwrap_err().to_string(); + assert!(exported.contains("ambient filesystem paths"), "{exported}"); + assert_eq!(std::fs::read_dir(&sets).map_or(0, Iterator::count), 0, "nothing of the refused batch is written"); + // Release directory capabilities before cleanup on Windows. + drop(h); + std::fs::remove_dir_all(base).unwrap(); + } + + #[test] + fn workspace_psd_saves_embed_no_file_a_linked_smart_object_names() { + use photocraft_engine::doc::{LayerContent, SmartSource}; + + const MARKER: &[u8] = b"PHOTOCRAFT-LINKED-0001"; + let has_marker = |bytes: &[u8]| bytes.windows(MARKER.len()).any(|w| w == MARKER); + let base = std::env::temp_dir().join(format!("pc-psd-linked-embed-{}", std::process::id())); + let root = base.join("workspace"); + std::fs::create_dir_all(&root).unwrap(); + let outside = base.join("outside.txt"); + std::fs::write(&outside, MARKER).unwrap(); + // A document whose smart object names a file outside the roots as its source. + let mut session = photocraft_engine::Session::new(); + session.execute("file.new", json!({"width": 8, "height": 8, "background": "white"})).unwrap(); + session.execute("layer.smartObjects.convertToSmartObject", json!({})).unwrap(); + let state = session.active_mut().unwrap(); + let LayerContent::Smart(smart) = &mut std::sync::Arc::make_mut(&mut state.doc).layers[0].content else { panic!("smart object") }; + smart.source = SmartSource::Linked { path: outside.to_string_lossy().into_owned() }; + std::fs::write(root.join("in.pcraft"), photocraft_engine::smart_cmds::encode_source(&state.doc).unwrap()).unwrap(); + // A trusted export embeds the linked file, as before. + assert!(has_marker(&photocraft_io::export(&state.doc, "psd", &Default::default()).unwrap().bytes)); + + let mut h = Headless::with_workspace(crate::AuthorizedWorkspace::new(Some(&root), Some(&root)).unwrap()); + h.handle("doc.open", json!({"path": "in.pcraft"})).unwrap(); + let saved = h.handle("doc.save", json!({"path": "out.psd"})).unwrap(); + let warnings = saved["warnings"].to_string(); + assert!(warnings.contains("written as pixels"), "{warnings}"); + assert!(!has_marker(&std::fs::read(root.join("out.psd")).unwrap()), "the linked file outside the roots is not embedded"); + // Release directory capabilities before cleanup on Windows. + drop(h); + std::fs::remove_dir_all(base).unwrap(); + } + + #[test] + fn allowed_filters_do_not_rerender_a_smart_object_from_its_linked_file() { + use photocraft_engine::doc::{LayerContent, SmartSource}; + + let base = std::env::temp_dir().join(format!("pc-linked-rerender-{}", std::process::id())); + let root = base.join("workspace"); + std::fs::create_dir_all(&root).unwrap(); + let red = photocraft_codecs::Image::from_u8(8, 8, photocraft_codecs::ChannelLayout::Rgb, [255, 0, 0].repeat(64)).unwrap(); + let outside = base.join("red.png"); + std::fs::write(&outside, photocraft_codecs::encode(&red, photocraft_codecs::Format::Png, &Default::default()).unwrap()).unwrap(); + // A document whose white smart object names a file outside the roots as its source. + let mut session = photocraft_engine::Session::new(); + session.execute("file.new", json!({"width": 8, "height": 8, "background": "white"})).unwrap(); + session.execute("layer.smartObjects.convertToSmartObject", json!({})).unwrap(); + let state = session.active_mut().unwrap(); + let LayerContent::Smart(smart) = &mut std::sync::Arc::make_mut(&mut state.doc).layers[0].content else { panic!("smart object") }; + smart.source = SmartSource::Linked { path: outside.to_string_lossy().into_owned() }; + std::fs::write(root.join("p.pcraft"), photocraft_engine::smart_cmds::encode_source(&state.doc).unwrap()).unwrap(); + + let mut h = Headless::with_workspace(crate::AuthorizedWorkspace::new(Some(&root), Some(&root)).unwrap()); + h.handle("doc.open", json!({"path": "p.pcraft"})).unwrap(); + h.handle("engine.execute", json!({"command": "filter.blur.gaussianBlur", "params": {"radius": 0.5}})).unwrap(); + let doc = &h.session.active().unwrap().doc; + let LayerContent::Smart(smart) = &doc.layers[0].content else { panic!("smart object") }; + assert_eq!(smart.smart_filters.len(), 1, "the filter is still recorded"); + assert_eq!(smart.cache.as_ref().unwrap().rgba(4, 4), [1.0, 1.0, 1.0, 1.0], "the linked file outside the roots is not read"); + // Release directory capabilities before cleanup on Windows. + drop(h); + std::fs::remove_dir_all(base).unwrap(); + } + + #[test] + fn workspace_layer_exports_keep_linked_smart_objects_as_pixels() { + use photocraft_engine::doc::{LayerContent, SmartSource}; + + const MARKER: &[u8] = b"PHOTOCRAFT-LINKED-0002"; + let has_marker = |bytes: &[u8]| bytes.windows(MARKER.len()).any(|w| w == MARKER); + let base = std::env::temp_dir().join(format!("pc-export-layers-linked-{}", std::process::id())); + let root = base.join("workspace"); + std::fs::create_dir_all(&root).unwrap(); + let outside = base.join("outside.txt"); + std::fs::write(&outside, MARKER).unwrap(); + // A document whose smart object names a file outside the roots as its source. + let mut session = photocraft_engine::Session::new(); + session.execute("file.new", json!({"width": 8, "height": 8, "background": "white"})).unwrap(); + session.execute("layer.smartObjects.convertToSmartObject", json!({})).unwrap(); + let state = session.active_mut().unwrap(); + let LayerContent::Smart(smart) = &mut std::sync::Arc::make_mut(&mut state.doc).layers[0].content else { panic!("smart object") }; + smart.source = SmartSource::Linked { path: outside.to_string_lossy().into_owned() }; + std::fs::write(root.join("in.pcraft"), photocraft_engine::smart_cmds::encode_source(&state.doc).unwrap()).unwrap(); + + let mut h = Headless::with_workspace(crate::AuthorizedWorkspace::new(Some(&root), Some(&root)).unwrap()); + h.handle("doc.open", json!({"path": "in.pcraft"})).unwrap(); + let out = root.join("layers"); + for format in ["psd", "psb"] { + let exported = h.handle("doc.exportLayers", json!({"dir": out.to_string_lossy(), "format": format})).unwrap(); + assert_eq!(exported["count"], 1, "{exported}"); + } + let files: Vec<_> = std::fs::read_dir(&out).unwrap().map(|e| e.unwrap().path()).collect(); + assert_eq!(files.len(), 2, "{files:?}"); + for file in files { + assert!(!has_marker(&std::fs::read(&file).unwrap()), "{} embeds no file outside the roots", file.display()); + } + // Release directory capabilities before cleanup on Windows. + drop(h); + std::fs::remove_dir_all(base).unwrap(); + } + #[test] fn trusted_local_render_keeps_full_size_behavior() { let mut h = Headless::trusted_local(); diff --git a/crates/engine/src/file_cmds.rs b/crates/engine/src/file_cmds.rs index 406bdbe499..ebc7c49640 100644 --- a/crates/engine/src/file_cmds.rs +++ b/crates/engine/src/file_cmds.rs @@ -510,6 +510,8 @@ pub(crate) fn encode(doc: &Document, path: &str, save: impl Into) -> R psd_maximize_compatibility: save.psd_maximize_compatibility, // Linked files a PSD embeds are resolved against the document being worked on. document_path: crate::doc_path::current(), + // An untrusted session's stored paths are document data: embed none of them. + read_linked_files: !crate::jobs::authorization_installed(), ..Default::default() }; // No quality chosen: write the file the way it was opened (a JPEG at its own quality, a WebP @@ -1679,6 +1681,9 @@ pub fn export_layers_with(s: &Session, p: &Value, mut write: impl FnMut(&str, &[ let cmd = "file.export.layersToFiles"; let format = p.get("format").and_then(Value::as_str).unwrap_or("png").trim_start_matches('.').to_ascii_lowercase(); let visible_only = p.get("visibleOnly").and_then(Value::as_bool).unwrap_or(true); + // Callers outside `Session::execute` (headless workspace exports) carry the session's gate + // into `encode`, so a PSD embeds no file a linked smart object names. + let _authorization = crate::jobs::render_authorization_scope(s.authorize); let save = SaveOpts::from_params(s, p, cmd)?; let d = s.active().ok_or(EngineError::NoDocument)?; let prefix = p.get("prefix").and_then(Value::as_str).map(str::to_string).unwrap_or_else(|| stem(&d.doc.name)); diff --git a/crates/engine/src/jobs.rs b/crates/engine/src/jobs.rs index 57846617dd..4a1c304241 100644 --- a/crates/engine/src/jobs.rs +++ b/crates/engine/src/jobs.rs @@ -66,6 +66,11 @@ pub(crate) fn authorize_render_command(id: &str, params: &Value) -> Result<()> { } } +/// Whether the work on this thread is done for an untrusted session (a gate is installed). +pub(crate) fn authorization_installed() -> bool { + RENDER_AUTHORIZATION.with(std::cell::Cell::get).is_some() +} + /// Identifies one background job for the session's lifetime. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, serde::Deserialize)] pub struct JobId(pub u64); diff --git a/crates/engine/src/smart_cmds.rs b/crates/engine/src/smart_cmds.rs index fdd69e4568..556ad5c55a 100644 --- a/crates/engine/src/smart_cmds.rs +++ b/crates/engine/src/smart_cmds.rs @@ -79,10 +79,16 @@ fn base_name(path: &str) -> String { /// A linked file's bytes; `None` when it is missing or its path is not followed /// ([`crate::doc_path::resolve`]). +/// +/// The path is document data, not a request parameter, so every read rechecks it with the gate +/// of the session the work is done for: a filter, transform or refresh re-renders from here, +/// not only the contents commands that judge it up front. A refused path is handled like a +/// missing file: the layer keeps its last rendering. fn read_file(path: &str) -> Option> { if path.is_empty() { return None; } + crate::jobs::authorize_render_command("layer.smartObjects.updateModifiedContent", &json!({"path": path})).ok()?; photocraft_format::read_file(&crate::doc_path::resolve(path).ok()?).ok() } diff --git a/crates/engine/src/variables_cmds.rs b/crates/engine/src/variables_cmds.rs index 0560a9d593..67aa1fe4f4 100644 --- a/crates/engine/src/variables_cmds.rs +++ b/crates/engine/src/variables_cmds.rs @@ -243,10 +243,9 @@ fn replace_pixels(doc: &mut Document, layer: LayerId, path: &str, method: PixelM Ok(()) } -fn apply_data_set(s: &mut Session, p: &Value) -> Result { - let (set, idx) = resolve_set(s, p, "image.applyDataSet")?; - let vars = s.active().ok_or(EngineError::NoDocument)?.doc.variables.clone(); - // Pixel Replacement reads image files, so an untrusted session's gate judges each path. +/// Pixel Replacement reads image files, so an untrusted session's gate judges each path a data +/// set would read: whether it is applied or exported, the path is document data. +fn authorize_pixel_paths(s: &Session, set: &DataSet) -> Result<()> { if let Some(auth) = s.authorize { for dv in &set.values { if let VarValue::Pixels(path) = &dv.value { @@ -254,6 +253,13 @@ fn apply_data_set(s: &mut Session, p: &Value) -> Result { } } } + Ok(()) +} + +fn apply_data_set(s: &mut Session, p: &Value) -> Result { + let (set, idx) = resolve_set(s, p, "image.applyDataSet")?; + let vars = s.active().ok_or(EngineError::NoDocument)?.doc.variables.clone(); + authorize_pixel_paths(s, &set)?; s.edit(&format!("Apply Data Set \"{}\"", set.name), |doc, _| apply_to_doc(doc, &vars, &set))?; set_vars(s, |v| v.active = Some(idx))?; Ok(json!({"applied": set.name, "index": idx})) @@ -409,6 +415,10 @@ pub fn export_data_sets_with(s: &Session, p: &Value, mut write: impl FnMut(&str, if sets.is_empty() { return Err(bad("file.export.dataSetsAsFiles", "no data sets to export")); } + // Judge every image path before anything is written, as Apply Data Set does. + for set in &sets { + authorize_pixel_paths(s, set)?; + } // Filename template: `{name}` (sanitised data-set name), `{index}` (1-based). Default `{name}`. let template = p.get("naming").and_then(Value::as_str).unwrap_or("{name}"); let doc_stem = original.name.rsplit_once('.').map_or(original.name.as_str(), |(a, _)| a).to_string(); @@ -432,7 +442,8 @@ pub fn export_data_sets_with(s: &Session, p: &Value, mut write: impl FnMut(&str, for (set, name) in sets.iter().zip(&names) { let mut doc = (*original).clone(); apply_to_doc(&mut doc, &vars, set)?; - let opts = photocraft_io::ExportOptions::default(); + // An untrusted session's PSD export embeds no file a linked smart object names. + let opts = photocraft_io::ExportOptions { read_linked_files: s.authorize.is_none(), ..Default::default() }; let bytes = photocraft_io::export(&doc, format, &opts).map(|r| r.bytes).map_err(|e| EngineError::Other(format!("export `{}`: {e}", set.name)))?; write(name, &bytes)?; written.push(name.clone()); diff --git a/crates/io/src/lib.rs b/crates/io/src/lib.rs index a0fc3e87e2..2289670dbb 100644 --- a/crates/io/src/lib.rs +++ b/crates/io/src/lib.rs @@ -232,6 +232,9 @@ pub struct ExportOptions { pub document_path: Option, /// Reuse one cache per open document for native saves; other formats ignore it. pub pcraft_cache: Option, + /// PSD/PSB and layered TIFF: read and embed the files linked smart objects name (see + /// [`PsdExportOptions::read_linked_files`]). On by default; automation turns it off. + pub read_linked_files: bool, } impl Default for ExportOptions { @@ -246,6 +249,7 @@ impl Default for ExportOptions { psd_maximize_compatibility: true, document_path: None, pcraft_cache: None, + read_linked_files: true, } } } @@ -526,6 +530,7 @@ fn export_stages(doc: &Document, name_or_ext: &str, opts: &ExportOptions, ctl: & force_psb: opts.force_psb || ext == "psb", maximize_compatibility: opts.psd_maximize_compatibility, document_path: opts.document_path.clone(), + read_linked_files: opts.read_linked_files, ..Default::default() }; let (mut file, mut warnings) = with_span(ctl, 0.0, PSD_MODEL_DONE, |ctl| psd_export::document_to_psd_interruptible(doc, &o, ctl)); diff --git a/crates/io/src/psd_export.rs b/crates/io/src/psd_export.rs index 90a5d86ce9..fd07adde50 100644 --- a/crates/io/src/psd_export.rs +++ b/crates/io/src/psd_export.rs @@ -50,6 +50,10 @@ pub struct PsdExportOptions { /// The path of the document's own file: linked smart objects are resolved against it /// ([`crate::doc_path::resolve`]) before they are read and embedded. pub document_path: Option, + /// Read and embed the files linked smart objects name. On by default; automation turns it + /// off, since a stored path is document data rather than a granted file: such a layer is + /// then written as pixels, with a warning. + pub read_linked_files: bool, } impl Default for PsdExportOptions { @@ -61,6 +65,7 @@ impl Default for PsdExportOptions { merged_compression: Compression::Rle, maximize_compatibility: true, document_path: None, + read_linked_files: true, } } } @@ -71,6 +76,8 @@ struct Ex { dpi: f32, /// The document's own file, which linked smart objects are resolved against. document: Option, + /// Whether linked files may be read and embedded ([`PsdExportOptions::read_linked_files`]). + read_linked_files: bool, /// Character/paragraph styles written into every type layer's engine data. text_styles: photocraft_doc::TextStyles, mask_fmt: PixelFormat, @@ -977,6 +984,9 @@ impl Ex { SmartSource::Linked { path } => { #[cfg(not(target_arch = "wasm32"))] { + if !self.read_linked_files { + return Err(format!("the linked file {path} is not read for this export")); + } let file = crate::doc_path::resolve(path, self.document.as_deref()).map_err(|e| format!("the linked file {path} is not opened: {e}"))?; let bytes = std::fs::read(file).map_err(|e| format!("the linked file {path} can't be read: {e}"))?; let name = path.rsplit(['/', '\\']).next().unwrap_or(path).to_string(); @@ -1049,7 +1059,7 @@ impl Ex { let doc = photocraft_format::load_from_bytes(bytes).map_err(|e| format!("its contents can't be read: {e}"))?; let (file, warnings) = document_to_psd_nested( &doc, - &PsdExportOptions { force_psb: true, document_path: self.document.clone(), ..Default::default() }, + &PsdExportOptions { force_psb: true, document_path: self.document.clone(), read_linked_files: self.read_linked_files, ..Default::default() }, self.smart.depth + 1, &Interrupt::NONE, ); @@ -1595,6 +1605,7 @@ fn document_to_psd_nested(doc: &Document, opts: &PsdExportOptions, depth: u32, c fmt, dpi: doc.resolution_dpi, document: opts.document_path.clone(), + read_linked_files: opts.read_linked_files, text_styles: doc.text_styles.clone(), text_index: Default::default(), mask_fmt: PixelFormat::new(ColorMode::Grayscale, sample, false), diff --git a/crates/io/src/tiff_layers.rs b/crates/io/src/tiff_layers.rs index 6caf335768..d6c0a845b3 100644 --- a/crates/io/src/tiff_layers.rs +++ b/crates/io/src/tiff_layers.rs @@ -216,6 +216,7 @@ pub(crate) fn export_layered(doc: &Document, opts: &ExportOptions, ctl: &photocr merged_compression: Compression::Raw, maximize_compatibility: true, document_path: opts.document_path.clone(), + read_linked_files: opts.read_linked_files, }; // TIFF has no palette, bitmap or duotone layout: the PSD model of an Indexed Color, Bitmap // or Duotone document is made in its pixel format (RGB or Grayscale), which the PSD exporter diff --git a/crates/ui-egui/src/actions.rs b/crates/ui-egui/src/actions.rs index 3e946b0d72..b6925867d4 100644 --- a/crates/ui-egui/src/actions.rs +++ b/crates/ui-egui/src/actions.rs @@ -98,7 +98,7 @@ pub(crate) fn play(app: &mut PhotocraftApp, params: &Value) -> Result) -> Result { let (action, from) = match actions_cmds::playback_plan(&app.session, params) { @@ -129,41 +129,46 @@ fn play_owned(app: &mut PhotocraftApp, params: &Value, suspended: Option) failed = Some(json!({"step": i, "id": id, "error": format!("action playback exceeded the {}-step limit", actions_cmds::MAX_PLAYBACK_STEPS)})); break; } + // The gate judges the step before its dialog opens: a denied step neither runs nor + // pauses. + if let Some(auth) = app.session.authorize + && let Err(e) = auth(id, p) + { + failed = Some(json!({"step": i, "id": id, "error": e.to_string()})); + break; + } // The dialog toggle: show the step's dialog with its recorded values and continue // from the next step when it is confirmed (`resume`). Agents play headlessly. if action.dialogs.contains(&i) && !app.automation_input && (crate::filter_dialog::has_dialog(id) || crate::adjust_dialog::has_dialog(id)) { let label = label_of(id); let dialog = crate::dialogs::open_command_dialog(app, id, &label); + let token = app.paused_playbacks.pause(action.name.clone(), i + 1, app.session.authorize); if let Some(d) = app.ui.dialog_mut(dialog) { if let Some(o) = p.as_object() { - d.fields.extend(o.iter().map(|(k, v)| (k.clone(), v.clone()))); + // `__` keys are the dialog's own markers (`params_of` drops them), not step values. + d.fields.extend(o.iter().filter(|(k, _)| !k.starts_with("__")).map(|(k, v)| (k.clone(), v.clone()))); } - d.fields.insert(RESUME.into(), json!({"action": action.name, "from": i + 1})); + d.fields.insert(RESUME.into(), json!(token)); } paused = Some(json!({"step": i, "id": id, "dialog": dialog})); break; } let target = app.session.active().map(|d| d.doc.id); - let result = (|| { - if let Some(auth) = app.session.authorize { - auth(id, p).map_err(|e| e.to_string())?; - } - if id == "actions.play" { - play(app, p) - } else if actions_cmds::shell_save_command(id) { - // Write synchronously, so the next step doesn't meet a document locked by a - // background save. - let background = std::mem::replace(&mut app.background_jobs, false); - let r = app.replay_save(id, p); - app.background_jobs = background; - r - } else if actions_cmds::shell_view_command(id) { - app.sync_views(); - app.run(id, p.clone()) - } else { - app.session.execute(id, p.clone()).map_err(|e| e.to_string()) - } - })(); + let result = if id == "actions.play" { + play(app, p) + } else if actions_cmds::shell_save_command(id) { + // Write synchronously, so the next step doesn't meet a document locked by a + // background save. + let background = std::mem::replace(&mut app.background_jobs, false); + let r = app.replay_save(id, p); + app.background_jobs = background; + r + } else if actions_cmds::shell_view_command(id) { + app.sync_views(); + app.run(id, p.clone()) + } else { + app.session.execute(id, p.clone()).map_err(|e| e.to_string()) + }; match result { Ok(value) => { if let Some(error) = actions_cmds::nested_failure(id, &value).or_else(|| actions_cmds::save_target_closed(&app.session, id, target)) { @@ -186,13 +191,14 @@ fn play_owned(app: &mut PhotocraftApp, params: &Value, suspended: Option) actions_cmds::finish_playback(&mut app.session, None, &action, from); match paused.as_ref().and_then(|p| p.get("dialog")).and_then(Value::as_u64).and_then(|d| app.ui.dialog_mut(d)) { // Paused for a dialog: the call goes on after it, so recording stays suspended. - Some(d) if failed.is_none() => match (owner, d.fields.get_mut(RESUME)) { - (Some(owner), Some(Value::Object(r))) => { - r.insert(RECORD.into(), owner); + Some(d) if failed.is_none() => { + let entry = d.fields.get(RESUME).and_then(Value::as_u64).and_then(|t| app.paused_playbacks.paused.get_mut(&t)); + match (owner, entry) { + (Some(owner), Some(p)) => p.record = Some(owner), + // No continuation to carry the recording: the call ends here. + (owner, _) => restore_recording(app, owner.as_ref()), } - // No continuation to carry the recording: the call ends here. - (owner, _) => restore_recording(app, owner.as_ref()), - }, + } _ => restore_recording(app, owner.as_ref()), } Ok(match (failed, paused) { @@ -478,16 +484,9 @@ pub fn insert_stop_confirm(app: &mut PhotocraftApp, fields: &serde_json::Map) { let Some(owner) = owner else { return }; @@ -511,9 +510,9 @@ fn restore_recording(app: &mut PhotocraftApp, owner: Option<&Value>) { } } -/// Whether a step dialog holds a suspended recording. +/// Whether a paused playback holds a suspended recording. fn suspended(app: &PhotocraftApp) -> bool { - app.ui.dialogs.iter().any(|d| d.fields.get(RESUME).and_then(|r| r.get(RECORD)).is_some()) + app.paused_playbacks.paused.values().any(|p| p.record.is_some()) } /// While a step dialog holds a suspended recording, refuse what would lose its call: a new @@ -529,13 +528,58 @@ pub(crate) fn suspended_guard(app: &PhotocraftApp, id: &str) -> Result<(), Strin /// A step dialog's command failed: the action ends there. pub fn abandoned(app: &mut PhotocraftApp, fields: &serde_json::Map) { - restore_recording(app, fields.get(RESUME).and_then(|r| r.get(RECORD))); + let record = fields.get(RESUME).and_then(|t| app.paused_playbacks.take(t)).and_then(|p| p.record); + restore_recording(app, record.as_ref()); +} + +/// The automation gate a session installs ([`photocraft_engine::Session::authorize`]). +type Gate = fn(&str, &Value) -> photocraft_engine::Result<()>; + +/// A playback paused at a step dialog: the action and step it continues from, and the gate it +/// was playing under (`None` for one a person started). +#[derive(Clone, Debug)] +pub(crate) struct PausedPlayback { + action: String, + from: usize, + gate: Option, + /// The recording the paused call suspended: `{index, into, action, from}` (`into` is the + /// name of the action being recorded). A step dialog only closes through + /// `dialogs::confirm` (OK, `↩`, automation) or `dialogs::cancel` (Cancel, Esc, the close + /// box, automation), which end the call through `resume`, `abandoned` or `stopped`, so the + /// recording comes back exactly once. + record: Option, +} + +/// The paused playbacks, by the token their dialog carries. The dialog's fields hold only the +/// token; what plays next, and under which gate, is kept here. +#[derive(Debug, Default)] +pub(crate) struct PausedPlaybacks { + last: u64, + paused: std::collections::HashMap, +} + +impl PausedPlaybacks { + fn pause(&mut self, action: String, from: usize, gate: Option) -> u64 { + self.last = self.last.wrapping_add(1); + self.paused.insert(self.last, PausedPlayback { action, from, gate, record: None }); + self.last + } + + fn take(&mut self, token: &Value) -> Option { + token.as_u64().and_then(|token| self.paused.remove(&token)) + } } /// A step dialog confirmed: continue the action after it (`dialogs::confirm` calls this). +/// +/// The remaining steps run under the gate the playback paused with, however the dialog was +/// confirmed: a control request, synthetic Enter or OK, or a click. pub fn resume(app: &mut PhotocraftApp, fields: &serde_json::Map) -> Result<(), String> { - let Some(r) = fields.get(RESUME) else { return Ok(()) }; - let owner = r.get(RECORD).cloned(); + let Some(token) = fields.get(RESUME) else { return Ok(()) }; + let Some(paused) = app.paused_playbacks.take(token) else { + return Err("this dialog doesn't belong to a paused action".into()); + }; + let owner = paused.record.clone(); // The dialog's command may run as a background job; the next step needs its result. if let Some(doc) = app.session.active().map(|d| d.doc.id) && let Some(job) = app.session.job_on(doc) @@ -547,8 +591,12 @@ pub fn resume(app: &mut PhotocraftApp, fields: &serde_json::Map) app.ui.status_error = true; return Err(e.to_string()); } - let params = json!({"action": r.get("action").cloned().unwrap_or(Value::Null), "from": r.get("from").cloned().unwrap_or(Value::Null)}); - match play_owned(app, ¶ms, owner) { + let params = json!({"action": paused.action, "from": paused.from}); + let saved = app.session.authorize; + app.session.authorize = paused.gate.or(saved); + let played = play_owned(app, ¶ms, owner); + app.session.authorize = saved; + match played { Ok(v) => { report_play(app, &v); Ok(()) @@ -1821,7 +1869,7 @@ mod tests { let id = v["paused"]["dialog"].as_u64().unwrap(); let d = app.ui.dialog_mut(id).unwrap(); assert_eq!(d.fields["radius"], 3, "the dialog opens on the recorded values"); - assert_eq!(d.fields[RESUME], json!({"action": "Blur", "from": 2})); + assert!(d.fields[RESUME].is_u64(), "the dialog carries only the pause's token: {}", d.fields[RESUME]); assert!(app.session.active().unwrap().doc.selection.is_none(), "nothing after the dialog ran yet"); report_play(&mut app, &v); assert!(app.ui.status.contains("dialog"), "{}", app.ui.status); @@ -1939,6 +1987,18 @@ mod tests { assert_eq!(app.session.actions.list[1].steps, child_call()); } + #[test] + fn an_abandoned_step_dialog_leaves_no_paused_playback() { + let mut app = recording_with_dialog_child(); + let id = pause_child(&mut app); + app.ui.dialog_mut(id).unwrap().fields.insert("radius".into(), json!("not a radius")); + assert!(crate::dialogs::confirm(&mut app, id).is_err(), "the dialog command fails"); + assert!(app.paused_playbacks.paused.is_empty(), "the failed dialog ends its pause"); + app.run("actions.stop", json!({})).unwrap(); + assert_eq!(app.session.actions.list[1].steps, child_call(), "the call is recorded once"); + app.run("actions.record", json!({"name": "Next"})).unwrap(); + } + #[test] fn a_failing_resumed_playback_ends_the_recorded_call_once() { let mut app = recording_with_dialog_child(); @@ -1982,6 +2042,178 @@ mod tests { assert_eq!(app.session.actions.list[1].steps, child_call()); } + #[test] + fn a_step_dialog_keeps_its_continuation_token() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.background_jobs = false; + app.run("file.new", json!({"width": 8, "height": 8})).unwrap(); + for (name, steps) in [ + ("First", vec![("filter.blur.gaussianBlur".into(), json!({"radius": 1}))]), + ("Second", vec![("filter.blur.gaussianBlur".into(), json!({"radius": 1})), ("select.all".into(), json!({}))]), + ] { + let mut action = Action::new(name, steps); + action.dialogs.insert(0); + app.session.actions.list.push(action); + } + let first = play(&mut app, &json!({"action": "First"})).unwrap()["paused"]["dialog"].as_u64().unwrap(); + let second = play(&mut app, &json!({"action": "Second"})).unwrap()["paused"]["dialog"].as_u64().unwrap(); + let first_token = app.ui.dialog_mut(first).unwrap().fields[RESUME].clone(); + let second_token = app.ui.dialog_mut(second).unwrap().fields[RESUME].clone(); + for token in [second_token.clone(), Value::Null, json!("invalid"), json!({"action": "Second", "from": 1})] { + let (request, _) = crate::control::ControlRequest::new("ui.dialog.set", json!({"dialog": first, "field": RESUME, "value": token})); + let response = crate::control::handle(&mut app, &egui::Context::default(), &request); + assert!(matches!(response, crate::control::Outcome::Done(ref v) if v["error"].as_str().is_some_and(|e| e.contains("read-only")))); + assert_eq!(app.ui.dialog_mut(first).unwrap().fields[RESUME], first_token); + assert_eq!(app.ui.dialog_mut(second).unwrap().fields[RESUME], second_token); + assert_eq!(app.paused_playbacks.paused.len(), 2); + } + crate::dialogs::confirm(&mut app, first).unwrap(); + assert!(app.session.active().unwrap().doc.selection.is_none(), "First must not run Second's remaining step"); + crate::dialogs::confirm(&mut app, second).unwrap(); + assert!(app.session.active().unwrap().doc.selection.is_some()); + assert!(app.paused_playbacks.paused.is_empty()); + } + + fn recording_with_displace_child() -> PhotocraftApp { + let mut app = recording_with_dialog_child(); + app.background_jobs = false; + app.session.actions.list[0].steps[0] = ("filter.distort.displace".into(), json!({})); + app + } + + fn assert_displace_call_finished(app: &mut PhotocraftApp, suffix_count: usize) { + assert!(!app.file_dialog_open()); + assert!(app.ui.dialogs.is_empty()); + assert!(app.paused_playbacks.paused.is_empty()); + assert!(app.session.actions.recording.is_some()); + assert_eq!(suffixes(app), suffix_count); + app.run("actions.stop", json!({})).unwrap(); + assert_eq!(app.session.actions.list[1].steps, child_call(), "record the child call exactly once"); + app.run("actions.record", json!({"name": "Next"})).unwrap(); + } + + #[test] + fn a_displace_step_restores_recording_when_the_map_picker_fails_or_is_cancelled() { + use crate::file_dialog::FileDialogAnswer; + let dir = tempfile::tempdir().unwrap(); + let missing = dir.path().join("missing.png").to_string_lossy().into_owned(); + for answer in [ + None, + Some(FileDialogAnswer::Unavailable("no picker available".into())), + Some(FileDialogAnswer::Paths(Vec::new())), + Some(FileDialogAnswer::Contents("map.png".into(), Vec::new())), + Some(FileDialogAnswer::Paths(vec![missing])), + ] { + let mut app = recording_with_displace_child(); + let (show, _) = crate::file_dialog::fake(vec![answer]); + app.services.file_dialog = Some(show); + let id = pause_child(&mut app); + crate::dialogs::confirm(&mut app, id).unwrap(); + assert!(app.file_dialog_open()); + assert!(app.session.actions.recording.is_none()); + app.poll_file_dialog(&egui::Context::default(), None); + assert_displace_call_finished(&mut app, 0); + } + } + + #[test] + fn a_displace_step_restores_recording_when_the_map_picker_cannot_start() { + let mut app = recording_with_displace_child(); + let id = pause_child(&mut app); + assert!(crate::dialogs::confirm(&mut app, id).is_err()); + assert_displace_call_finished(&mut app, 0); + } + + #[test] + fn a_displace_step_resumes_and_records_its_call_once_after_choosing_a_map() { + let mut app = recording_with_displace_child(); + let dir = tempfile::tempdir().unwrap(); + let map = dir.path().join("map.png"); + let bytes = photocraft_io::export(&app.session.active().unwrap().doc, "png", &Default::default()).unwrap().bytes; + std::fs::write(&map, bytes).unwrap(); + let (show, _) = crate::file_dialog::fake(vec![Some(crate::file_dialog::FileDialogAnswer::Paths(vec![map.to_string_lossy().into_owned()]))]); + app.services.file_dialog = Some(show); + let id = pause_child(&mut app); + crate::dialogs::confirm(&mut app, id).unwrap(); + app.poll_file_dialog(&egui::Context::default(), None); + assert_displace_call_finished(&mut app, 1); + } + + #[test] + fn a_step_dialog_takes_only_the_steps_values() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 16, "height": 16})).unwrap(); + let mut a = Action::new("Blur", vec![("filter.blur.gaussianBlur".into(), json!({"radius": 2, "__command": "select.all", "__label": "Other"}))]); + a.dialogs.insert(0); + app.session.actions.list.push(a); + let v = play(&mut app, &json!({"action": "Blur"})).unwrap(); + let d = app.ui.dialog_mut(v["paused"]["dialog"].as_u64().unwrap()).unwrap(); + assert_eq!(d.fields["radius"], 2); + assert_eq!(d.fields["__command"], "filter.blur.gaussianBlur"); + assert_ne!(d.fields["__label"], "Other"); + } + + fn deny_blur(id: &str, _: &Value) -> photocraft_engine::Result<()> { + if id == "filter.blur.gaussianBlur" { Err(photocraft_engine::EngineError::Other("blur denied".into())) } else { Ok(()) } + } + + #[test] + fn a_step_the_gate_denies_opens_no_dialog() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 16, "height": 16})).unwrap(); + let mut a = Action::new("Blur", vec![("filter.blur.gaussianBlur".into(), json!({"radius": 1}))]); + a.dialogs.insert(0); + app.session.actions.list.push(a); + app.session.authorize = Some(deny_blur); + let v = play(&mut app, &json!({"action": "Blur"})).unwrap(); + app.session.authorize = None; + assert!(v["failed"]["error"].as_str().is_some_and(|e| e.contains("blur denied")), "{v}"); + assert!(app.ui.dialogs.is_empty(), "no step dialog opened"); + assert!(app.paused_playbacks.paused.is_empty()); + } + + fn deny_select_all(id: &str, _: &Value) -> photocraft_engine::Result<()> { + if id == "select.all" { Err(photocraft_engine::EngineError::Other("select.all denied".into())) } else { Ok(()) } + } + + /// An action an automation client plays (`control::handle` installs the gate, then removes + /// it) pauses at a step dialog; confirming the dialog later, here with synthetic Enter or + /// OK, runs the remaining steps under the same gate. + #[test] + fn a_paused_playback_keeps_its_automation_gate_when_the_dialog_is_confirmed() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 16, "height": 16})).unwrap(); + let mut a = Action::new("Seeded", vec![("filter.blur.gaussianBlur".into(), json!({"radius": 1})), ("select.all".into(), json!({}))]); + a.dialogs.insert(0); + app.session.actions.list.push(a); + app.session.authorize = Some(deny_select_all); + let v = play(&mut app, &json!({"action": "Seeded"})).unwrap(); + app.session.authorize = None; + let id = v["paused"]["dialog"].as_u64().unwrap(); + app.automation_input = true; + let _ = crate::dialogs::confirm(&mut app, id); + app.automation_input = false; + assert!(app.session.active().unwrap().doc.selection.is_none(), "the denied step did not run"); + assert!(app.ui.status.contains("select.all denied"), "{}", app.ui.status); + } + + /// The dialog names only its pause: changing its fields can't resume another action. + #[test] + fn a_step_dialog_resumes_only_the_playback_that_paused_it() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 16, "height": 16})).unwrap(); + let mut a = Action::new("Blur", vec![("filter.blur.gaussianBlur".into(), json!({"radius": 1}))]); + a.dialogs.insert(0); + app.session.actions.list.push(a); + app.session.actions.list.push(Action::new("Seeded", vec![("select.all".into(), json!({}))])); + let v = play(&mut app, &json!({"action": "Blur"})).unwrap(); + let id = v["paused"]["dialog"].as_u64().unwrap(); + app.ui.dialog_mut(id).unwrap().fields.insert(RESUME.into(), json!({"action": "Seeded", "from": 0})); + let r = crate::dialogs::confirm(&mut app, id); + assert!(app.session.active().unwrap().doc.selection.is_none(), "the other action did not play"); + assert!(r.is_err_and(|e| e.contains("paused action"))); + } + #[test] fn button_mode_plays_the_clicked_action_and_hides_its_steps() { use egui_kittest::{Harness, kittest::Queryable}; diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index c30ed4b8f1..93dc17afd6 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -902,6 +902,9 @@ fn dispatch(app: &mut PhotocraftApp, ctx: &egui::Context, req: &ControlRequest) }; let mut fields = if kind == DialogKind::NewDocument { app.new_document_fields() } else { Default::default() }; if let Some(f) = p.get("fields").and_then(Value::as_object) { + if let Some(marker) = f.keys().find(|k| crate::dialogs::is_route_marker(k)) { + return err(format!("`{marker}` is set only by the dialog it belongs to")); + } fields.extend(f.clone()); } ok(json!({"dialog": app.ui.open_dialog(kind, fields)})) @@ -955,6 +958,13 @@ fn dispatch(app: &mut PhotocraftApp, ctx: &egui::Context, req: &ControlRequest) let (Some(id), Some(field)) = (u("dialog"), s("field")) else { return err("need `dialog` and `field`") }; let value = p.get("value").cloned().unwrap_or(Value::Null); match app.ui.dialog_mut(id) { + // A continuation belongs to the dialog that paused it, even when another + // action has a valid token. Replacing it would resume or cancel that action. + Some(_) if field == crate::actions::RESUME => err("the paused action token is read-only"), + // A marker picks the handler OK runs: a dialog keeps the ones it was opened with. + Some(d) if !d.fields.contains_key(field) && crate::dialogs::is_route_marker(field) => { + err(format!("`{field}` is set only by the dialog it belongs to")) + } Some(d) => { d.fields.insert(field.to_string(), value); ok(Value::Null) @@ -2661,6 +2671,77 @@ mod tests { } } + /// An app whose automation gates refuse file commands, with a document open, and the paths + /// its writer was asked to write. + fn gated_app_recording_writes() -> (PhotocraftApp, egui::Context, std::rc::Rc>>, u64) { + let written: std::rc::Rc>> = std::rc::Rc::default(); + let w = written.clone(); + let services = crate::Services { + automation_authorize: Some(deny_ambient_file), + automation_command: Some(Box::new(|id, params| deny_ambient_file(id, params).map_err(|e| e.to_string()))), + export: Some(Box::new(|_: &photocraft_doc::Document, _: &str, _: &crate::ExportSettings| Ok((b"out".to_vec(), Vec::new())))), + write: Some(Box::new(move |path: &str, _: &[u8]| { + w.borrow_mut().push(path.to_string()); + Ok(()) + })), + ..Default::default() + }; + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), services); + app.background_jobs = false; + app.session.execute("file.new", json!({"width": 4, "height": 4})).unwrap(); + let doc = app.session.active().unwrap().doc.id.0; + (app, egui::Context::default(), written, doc) + } + + fn outside_target() -> String { + std::env::temp_dir().join("outside").join("x.png").to_string_lossy().into_owned() + } + + /// `ui.dialog.set` and `ui.dialog.open` never add a marker that picks a dialog's OK handler: + /// a View › Zoom In dialog can't be made to confirm as the disk-change prompt's save. + #[test] + fn automation_cannot_add_a_marker_that_reroutes_a_dialog() { + let (mut app, ctx, written, doc) = gated_app_recording_writes(); + let target = outside_target(); + let r = call(&mut app, &ctx, "ui.dialog.open", json!({"kind": "command", "command": "view.zoomIn"})); + let dialog = r["result"]["dialog"].clone(); + for (field, value) in [("document", json!(doc)), ("path", json!(target)), ("flat", json!(false))] { + let r = call(&mut app, &ctx, "ui.dialog.set", json!({"dialog": dialog, "field": field, "value": value})); + assert!(r.get("error").is_none(), "{r}"); + } + let injected = call(&mut app, &ctx, "ui.dialog.set", json!({"dialog": dialog, "field": "__diskChanged", "value": true})); + let replace = call(&mut app, &ctx, "ui.dialog.set", json!({"dialog": dialog, "field": "__replaceSaved", "value": true})); + assert!(replace["error"].as_str().is_some_and(|e| e.contains("set only by the dialog")), "{replace}"); + call(&mut app, &ctx, "ui.dialog.confirm", json!({"dialog": dialog})); + assert!(written.borrow().is_empty(), "nothing is written: {:?}", written.borrow()); + assert!(injected["error"].as_str().is_some_and(|e| e.contains("set only by the dialog")), "{injected}"); + let r = call(&mut app, &ctx, "ui.dialog.open", json!({"kind": "about", "fields": {"__export": true}})); + assert!(r["error"].as_str().is_some_and(|e| e.contains("set only by the dialog")), "{r}"); + } + + /// A handler that writes files itself (the disk-change prompt's save) is judged as the file + /// command it stands for, through a control request or synthetic input alike. + #[test] + fn automation_confirming_a_file_writing_dialog_is_judged_as_its_file_command() { + let (mut app, ctx, written, doc) = gated_app_recording_writes(); + let target = outside_target(); + // A real disk-change prompt keeps its own fields settable, but its save is a file command. + let fields = json!({"__diskChanged": true, "__label": "Save", "message": "", "document": doc, "path": target, "flat": false}); + let prompt = app.ui.open_dialog(DialogKind::Command, fields.as_object().cloned().unwrap()); + let r = call(&mut app, &ctx, "ui.dialog.set", json!({"dialog": prompt, "field": "__diskChanged", "value": true})); + assert!(r.get("error").is_none(), "{r}"); + let r = call(&mut app, &ctx, "ui.dialog.confirm", json!({"dialog": prompt})); + assert!(written.borrow().is_empty(), "nothing is written: {:?}", written.borrow()); + assert!(r["error"].as_str().is_some_and(|e| e.contains("file.save")), "{r}"); + // Synthetic input (Enter or OK) reaches the same handler without the control pre-check. + let synthetic = app.ui.open_dialog(DialogKind::Command, fields.as_object().cloned().unwrap()); + app.automation_input = true; + let r = crate::dialogs::confirm(&mut app, synthetic); + app.automation_input = false; + assert!(written.borrow().is_empty(), "nothing is written: {:?}", written.borrow()); + assert!(r.is_err_and(|e| e.contains("file.save"))); + } + #[test] fn app_open_opens_a_directory_pcraft_and_falls_back_for_the_zip_form() { use photocraft_doc::Document; diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index acf77870ef..4a7f5634f8 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -688,11 +688,82 @@ pub fn title(d: &Dialog) -> String { } } +type Fields = serde_json::Map; + +/// The markers [`confirm`] and [`cancel`] choose a dialog's handler by: keys the code opening a +/// dialog puts in its fields. Keep in step with the arms of `confirm`. +const ROUTES: &[fn(&Fields) -> bool] = &[ + crate::guide_layout_ui::owns, + crate::brush_preset_dialog::owns, + crate::fill_ui::owns, + crate::pattern_fill_ui::owns, + crate::stroke_ui::owns, + crate::channel_options_ui::owns, + crate::channel_options_ui::owns_new_spot, + crate::shape_dialog::owns, + crate::delete_layer_prompt::owns, + crate::disk_change_prompt::owns, + crate::actions::owns_insert_stop, + crate::new_layer_ui::owns, + crate::mask_trash_prompt::owns, + crate::rasterize_prompt::owns, + crate::duplicate_layer_ui::owns, + crate::stack_dialog::owns, + crate::tool_reset::owns, + crate::crop_presets::owns, + crate::variables_ui::owns, + crate::file_ui::owns, + crate::new_guide_ui::owns, + crate::color_picker_ui::owns, + crate::color_range_ui::owns, + crate::prefs_ui::owns, + crate::open_prompt::owns, + crate::replace_prompt::owns, + crate::layer_style::color_picker::owns, + |f| f.contains_key("__export"), + |f| f.contains_key("__smartFilter"), + |f| f.contains_key(crate::actions::RESUME), +]; + +/// Whether `key` is a marker that picks a dialog's handler. Automation may change the values of +/// a dialog's fields, private ones included, but never add a marker: a dialog it opened would +/// otherwise confirm through another dialog's handler, one its command was never judged for. +pub fn is_route_marker(key: &str) -> bool { + let probe = Fields::from_iter([(key.to_string(), Value::Bool(true))]); + ROUTES.iter().any(|owns| owns(&probe)) +} + +/// The file command a dialog's handler stands for when it reads or writes files itself, outside +/// the engine (a disk-change save, Export As, Save for Web, Print), so automation is judged for +/// that command rather than for the dialog's `__command`. +fn file_command(fields: &Fields) -> Option<&'static str> { + if crate::disk_change_prompt::owns(fields) { + Some("file.save") + } else if fields.contains_key("__export") { + Some("file.export.exportAs") + } else if fields.contains_key("__web") { + Some("file.export.saveForWebLegacy") + } else if fields.contains_key("__print") { + Some("file.print") + } else { + None + } +} + /// Confirm a dialog: run its action and close it. Used by the OK button and by automation. pub fn confirm(app: &mut PhotocraftApp, id: u64) -> Result { if crate::layer_style::color_picker::has_child(app, id) { return Err("confirm or cancel the Layer Style color picker first".into()); } + // A control request installs the session gate; synthetic input sets `automation_input`. + // Either way the handler's own file access is judged before it runs, as its command would be. + if (app.automation_input || app.session.authorize.is_some()) + && let Some(d) = app.ui.dialogs.iter().find(|d| d.id == id) + && let Some(command) = file_command(&d.fields) + && let Some(authorize) = app.services.automation_command.as_ref() + { + authorize(command, &Value::Object(d.fields.clone()))?; + } if let Some(d) = app.ui.dialogs.iter().find(|d| d.id == id).filter(|d| crate::guide_layout_ui::owns(&d.fields)) { if app.ui.dialogs.iter().any(|child| crate::color_picker_ui::owns(&child.fields) && child.fields.get("__dialog").and_then(Value::as_u64) == Some(id)) { return Err("confirm or cancel the guide color picker first".into()); @@ -747,7 +818,20 @@ pub fn confirm(app: &mut PhotocraftApp, id: u64) -> Result { }; if crate::filter_dialog::needs_displace_map(&cmd, &crate::filter_dialog::params_of(&d.fields)) { app.filter_preview = None; - return crate::filter_dialog::ask_displace_map(app, cmd, d.fields.clone()); + let result = crate::filter_dialog::ask_displace_map(app, cmd, d.fields.clone()); + if result.is_err() { + crate::actions::abandoned(app, &d.fields); + } else { + // Cancel and an unavailable picker finish before the Displace callback + // runs. Observe the whole file-dialog result, including those exits. + app.after_file_dialog(move |app, result| { + if result.is_err() { + crate::actions::abandoned(app, &d.fields); + } + result + }); + } + return result; } let (cmd, params) = match crate::smart_ui::confirm_command(app, &d.fields, cmd, crate::filter_dialog::params_of(&d.fields)) { Ok(edit) => edit, diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 274380ba96..6ea5992d43 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -369,6 +369,9 @@ pub struct ExportSettings { pub document_path: Option, /// Native save cache owned by the open document; exports without one remain one-shot. pub pcraft_cache: Option, + /// Read and embed the files linked smart objects name ([`photocraft_io::ExportOptions::read_linked_files`]). + /// Off for automation saves, which are granted only their target. + pub read_linked_files: bool, } impl Default for ExportSettings { @@ -388,6 +391,7 @@ impl Default for ExportSettings { embed_metadata: true, document_path: None, pcraft_cache: None, + read_linked_files: true, } } } @@ -415,6 +419,7 @@ impl ExportSettings { opts.psd_maximize_compatibility = self.psd_maximize_compatibility; opts.document_path = self.document_path.clone(); opts.pcraft_cache = self.pcraft_cache.clone(); + opts.read_linked_files = self.read_linked_files; opts } } @@ -851,6 +856,9 @@ pub struct PhotocraftApp { /// makes shortcut- and dialog-triggered commands pass the same policy as /// direct control calls. pub(crate) automation_input: bool, + /// Action playbacks paused at a step dialog, by the token in that dialog's fields + /// ([`actions::RESUME`]): what to play next and the automation gate it paused under. + pub(crate) paused_playbacks: actions::PausedPlaybacks, /// Levels/Curves histogram cache: (document, adjustment layer, revision it is valid for). pub(crate) tone_hist: Option<(DocId, photocraft_doc::LayerId, u64, std::sync::Arc)>, /// Histogram panel cache: (document, revision, computed at ms, histograms). @@ -1082,6 +1090,7 @@ impl PhotocraftApp { live_adjust_preview: None, synthetic: Vec::new(), automation_input: false, + paused_playbacks: Default::default(), channel_thumbs: None, channel_views: HashMap::new(), type_layout: None, @@ -2119,11 +2128,14 @@ impl PhotocraftApp { )? }; let reply = |path: &str, warnings: &[String]| json!({"path": path, "warnings": warnings}); + // Only the target is granted: a linked smart object's stored path is document data, so + // PSD export writes that layer as pixels instead of embedding the file. + let settings = ExportSettings { read_linked_files: false, ..choices.apply(self.save_settings()) }; // #2017: the File › Save worker path, so the encode doesn't block the frame loop. if self.background_jobs && let Some(save) = self.services.automation_save.clone() { - return match jobs_ui::start_save(self, target.clone(), choices.apply(self.save_settings()), false, save, true)? { + return match jobs_ui::start_save(self, target.clone(), settings, false, save, true)? { Some((path, warnings)) => Ok(reply(&path, &warnings)), None => { let job = self.jobs.last_started; @@ -2133,7 +2145,6 @@ impl PhotocraftApp { } let state = self.session.active().ok_or("no document")?; let (doc, revision) = (state.doc.id, state.revision); - let settings = choices.apply(self.save_settings()); let export = self.services.export.as_ref().ok_or("no exporter configured")?; let (bytes, warnings) = export(&state.doc, &target, &settings)?; let write = self.services.automation_write.as_mut().ok_or("automation write authority is not configured")?;