diff --git a/crates/automation/src/headless.rs b/crates/automation/src/headless.rs index e9cbca9241..afdceaead5 100644 --- a/crates/automation/src/headless.rs +++ b/crates/automation/src/headless.rs @@ -151,8 +151,7 @@ impl Headless { // alone (#1547). let ext = format.map(|f| f.trim_start_matches('.').to_ascii_lowercase()).or_else(|| target.extension().map(|e| e.to_string_lossy().to_ascii_lowercase())); - let layered = matches!(ext.as_deref(), Some("psd" | "psb" | "ora")) || ext.as_deref() == Some(photocraft_format::EXTENSION); - if layered { + if ext.as_deref().is_some_and(file_cmds::layered_extension) { // Saving by index must not retarget the next automation command. let previously_active = self.session.active_index(); self.session.set_active(i); diff --git a/crates/engine/src/file_cmds.rs b/crates/engine/src/file_cmds.rs index d75cd64504..2d30b35d69 100644 --- a/crates/engine/src/file_cmds.rs +++ b/crates/engine/src/file_cmds.rs @@ -152,7 +152,14 @@ pub fn extension(path: &str) -> Option { /// Whether a save without a new path may write back to `path`: only layered files (PSD, PSB, /// .pcraft). A flat file goes through Save As instead, so it is never flattened over the original. pub fn saves_in_place(path: &str) -> bool { - extension(path).is_some_and(|ext| matches!(ext.as_str(), "psd" | "psb" | "pcraft" | "ora")) + extension(path).is_some_and(|ext| layered_extension(&ext)) +} + +/// Whether the lower-case extension `ext` names a layered document format (PSD, PSB, OpenRaster, +/// .pcraft). A save to one becomes the document's file; any other format is a copy for automation +/// saves, desktop and headless alike (#1547, #2579). +pub fn layered_extension(ext: &str) -> bool { + matches!(ext, "psd" | "psb" | "pcraft" | "ora") } /// Whether `path` names a document template (.psdt). A template opens as a new untitled document diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index ef399809ee..acb42bb016 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -1772,6 +1772,9 @@ mod tests { let r = call(&mut app, &ctx, "app.save", json!({"path": "out.png"})); assert_eq!(r["result"], json!({"path": "out.png", "warnings": ["Layers were flattened"]}), "{r}"); assert_eq!(*written.borrow(), vec!["out.png".to_string()]); + // A flat export is a copy: the document keeps its name and file (#2579). + let st = app.session.active().unwrap(); + assert_eq!((st.doc.name.as_str(), st.path.as_deref()), ("warn.psd", Some("in/warn.psd"))); // Without `path`, only a layered file is written back, like File › Save (#416). call(&mut app, &ctx, "app.open", json!({"path": "in/flat.jpg"})); let r = call(&mut app, &ctx, "app.save", json!({})); diff --git a/crates/ui-egui/src/file_dialog/tests.rs b/crates/ui-egui/src/file_dialog/tests.rs index b0c356ada3..3be31dace7 100644 --- a/crates/ui-egui/src/file_dialog/tests.rs +++ b/crates/ui-egui/src/file_dialog/tests.rs @@ -144,7 +144,8 @@ fn layered_save_respects_explicit_jpeg_choices() { answer(&open, Some(FileDialogAnswer::SaveTo("/exports/chosen.JPEG".into()))); app.poll_file_dialog(&ctx, None); assert_eq!(*written.borrow(), ["/exports/chosen.jpeg"]); - assert_eq!(app.session.active().unwrap().path.as_deref(), Some("/exports/chosen.jpeg")); + // The JPEG can't hold the layers, so it is a copy and the document keeps its file (#2550). + assert_eq!(app.session.active().unwrap().path.as_deref(), Some("/pics/My image.JPEG")); // Explicit command paths carry the same choice and bypass the picker. for command in ["file.save", "file.saveAs"] { let (mut app, open, written) = jpeg_app(Some("/pics/My image.JPEG"), true); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index d0046a3135..17fb5cfdd0 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -210,6 +210,29 @@ impl Default for ExportSettings { } } +/// The document's lone layer when it is a plain raster a flat file keeps: visible, at full +/// opacity, in Normal mode, with no masks, effects or blending restrictions (#2234). +pub(crate) fn plain_raster(doc: &Document) -> Option<&photocraft_doc::Layer> { + match doc.layers.as_slice() { + [layer] + if matches!(layer.content, photocraft_doc::LayerContent::Raster(_)) + && layer.visible + && layer.opacity >= 1.0 + && layer.fill_opacity >= 1.0 + && layer.mask.is_none() + && layer.vector_mask.is_none() + && layer.effects.items.is_empty() + && layer.effects.psd_raw.is_none() + && matches!(layer.blend, photocraft_color::BlendMode::Normal | photocraft_color::BlendMode::PassThrough) + && photocraft_compose::channel_weights(layer, doc.mode).is_none() + && !photocraft_compose::blend_if_active(layer, doc.mode) => + { + Some(layer) + } + _ => None, + } +} + /// Encode a document: (file bytes, warnings about anything approximated or dropped). pub type ExportFn = Box Result<(Vec, Vec), String>>; pub type WriteFn = Box Result<(), String>>; @@ -1029,18 +1052,11 @@ impl PhotocraftApp { // Other files keep their format only when it can retain their layers. A plain raster // (including an unlocked transparent PNG) can keep its flat format; editable contents, // masks and layer appearance settings need a layered format even with just one layer. - let plain_raster = matches!(st.doc.layers.as_slice(), [layer] - if matches!(layer.content, photocraft_doc::LayerContent::Raster(_)) - && layer.visible && layer.opacity >= 1.0 && layer.fill_opacity >= 1.0 - && layer.mask.is_none() && layer.vector_mask.is_none() - && layer.effects.items.is_empty() && layer.effects.psd_raw.is_none() - && matches!(layer.blend, photocraft_color::BlendMode::Normal | photocraft_color::BlendMode::PassThrough) - && photocraft_compose::channel_weights(layer, st.doc.mode).is_none() - && !photocraft_compose::blend_if_active(layer, st.doc.mode)); + let flat = plain_raster(&st.doc).is_some(); let ext = st.path.as_deref().and_then(|p| std::path::Path::new(p).extension()).map(|e| e.to_string_lossy().to_ascii_lowercase()); let writable = ext.is_some_and(|e| { matches!(e.as_str(), photocraft_format::EXTENSION | "psd" | "psb" | "tif" | "tiff" | "ora") - || (plain_raster && photocraft_codecs::from_extension(&e).is_some_and(|f| f.caps().write)) + || (flat && photocraft_codecs::from_extension(&e).is_some_and(|f| f.caps().write)) }); let suggested = match &st.path { Some(p) if writable => p.clone(), @@ -1062,7 +1078,14 @@ impl PhotocraftApp { tiff_options_ui::park(self, path.clone())?; return Ok(serde_json::json!({"path": path, "warnings": []})); } - match self.write_document(path.clone(), &ExportSettings::default(), false)? { + // A flat file that can't hold the document (its layers, or the layered file it lives in) + // is written as a copy, as in Photoshop: the document keeps its file, so Save still + // writes the layered original and the edits stay unsaved (#2550). + let st = self.session.active().ok_or("no document")?; + let layered = + |p: &str| photocraft_engine::file_cmds::saves_in_place(p) || matches!(photocraft_engine::file_cmds::extension(p).as_deref(), Some("tif" | "tiff")); + let copy = !layered(&path) && (plain_raster(&st.doc).is_none() || st.path.as_deref().is_some_and(layered)); + match self.write_document(path.clone(), &ExportSettings::default(), copy)? { Some((path, warnings)) => Ok(serde_json::json!({"path": path, "warnings": warnings})), None => Ok(serde_json::json!({"path": path, "warnings": [], "pending": true, "job": self.jobs.last_started.map(|j| j.0)})), } @@ -1134,7 +1157,11 @@ impl PhotocraftApp { let (bytes, warnings) = export(&state.doc, &target, &ExportSettings::default())?; let write = self.services.automation_write.as_mut().ok_or("automation write authority is not configured")?; write(&target, &bytes)?; - if let Some(state) = self.session.active_mut() { + // Only a layered save becomes the document's file; a flat one is a copy, as in the + // headless server (#2579). + if photocraft_engine::file_cmds::extension(&target).is_some_and(|e| photocraft_engine::file_cmds::layered_extension(&e)) + && let Some(state) = self.session.active_mut() + { state.saved_to(target.clone()); } self.ui.status = format!("Saved {target}"); diff --git a/crates/ui-egui/src/save_identity_tests.rs b/crates/ui-egui/src/save_identity_tests.rs index f5513a1854..9cfc86d500 100644 --- a/crates/ui-egui/src/save_identity_tests.rs +++ b/crates/ui-egui/src/save_identity_tests.rs @@ -116,3 +116,69 @@ fn web_download_and_automation_saves_use_the_same_identity_completion() { app.save_automation(Some("保存/agent save.psd".into())).unwrap(); assert_eq!(app.session.active().unwrap().doc.name, "agent save.psd"); } + +/// Save As to a flat format writes a copy when the flat file can't hold the document: its layers, +/// or the layered file it already lives in. The document keeps its file, so Save still writes the +/// layered original and the edits stay unsaved (#2550). +#[test] +fn save_as_flat_keeps_the_layered_original() { + let ctx = egui::Context::default(); + // A layered document, and a one-layer one (the issue's case: the dialog offered a JPEG). + for layered in [true, false] { + let mut app = app(); + if !layered { + app.run("file.new", json!({"width": 4, "height": 4, "name": "Untitled-2"})).unwrap(); + } + let written = Rc::new(RefCell::new(Vec::new())); + let w = written.clone(); + app.services.write = Some(Box::new(move |path, _| { + w.borrow_mut().push(path.to_string()); + Ok(()) + })); + menus::invoke(&mut app, &ctx, "file.saveAs", json!({"path": "art.psd"})).unwrap(); + app.run("edit.fill", json!({"color": "#000000"})).unwrap(); + let before = identity(&app); + for flat in ["art.jpg", "art.png"] { + menus::invoke(&mut app, &ctx, "file.saveAs", json!({"path": flat})).unwrap(); + assert_eq!(identity(&app), before, "{flat}, layered: {layered}"); + } + menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap(); + assert_eq!(*written.borrow(), ["art.psd", "art.jpg", "art.png", "art.psd"]); + let st = app.session.active().unwrap(); + assert_eq!(st.saved_revision, st.revision); + } + // An untitled layered document saved as a JPEG is still untitled and unsaved. + let mut app = app(); + let before = identity(&app); + app.save_as(Some("flat.jpg".into())).unwrap(); + assert_eq!(identity(&app), before); +} + +/// A flat document that isn't in a layered file becomes the flat file it is saved as, as before. +#[test] +fn save_as_flat_retargets_a_flat_document() { + let mut app = app(); + app.run("file.new", json!({"width": 4, "height": 4, "name": "Untitled-2"})).unwrap(); + app.save_as(Some("shot.png".into())).unwrap(); + assert_eq!(app.session.active().unwrap().path.as_deref(), Some("shot.png")); + app.save_as(Some("shot.jpg".into())).unwrap(); + let st = app.session.active().unwrap(); + assert_eq!((st.doc.name.as_str(), st.path.as_deref()), ("shot.jpg", Some("shot.jpg"))); + assert_eq!(st.saved_revision, st.revision); +} + +/// `app.save` (control channel, MCP bridge) to a flat format is a copy, as in the headless server +/// (#1547); only a layered format becomes the document's file (#2579). +#[test] +fn automation_flat_save_is_a_copy() { + let mut app = app(); + let before = identity(&app); + for flat in ["flat.png", "flat.jpg", "flat.tif"] { + app.save_automation(Some(flat.into())).unwrap(); + assert_eq!(identity(&app), before, "{flat}"); + } + app.save_automation(Some("layers.psd".into())).unwrap(); + let st = app.session.active().unwrap(); + assert_eq!((st.doc.name.as_str(), st.path.as_deref()), ("layers.psd", Some("layers.psd"))); + assert_eq!(st.saved_revision, st.revision); +}