Keep the layered original when saving a flat copy (#2550, #2579) (#2611)

Save As to a flat format always made the written file the document's
file: a layered or PSD-backed document saved as JPEG was renamed to the
JPEG and marked saved, so Ctrl+S then opened Save As with JPEG chosen,
and closing no longer asked about the unsaved layers. app.save on the
desktop did the same for any format, unlike the headless server (#1547).

Save As now writes a copy when the flat file can't hold the document
(anything beyond one plain raster layer, or the document already lives in a layered
PSD/PSB/.pcraft/OpenRaster/TIFF file), as Photoshop does. A flat
document not in a layered file still becomes the flat file. app.save
only retargets for layered formats, through a helper shared with the
headless server (file_cmds::layered_extension).

Co-authored-by: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Fabio Lussi
2026-10-10 19:12:41 +02:00
committed by GitHub
parent d4b9f06891
commit f8efd089ad
6 changed files with 118 additions and 15 deletions
+1 -2
View File
@@ -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);
+8 -1
View File
@@ -152,7 +152,14 @@ pub fn extension(path: &str) -> Option<String> {
/// 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
+3
View File
@@ -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!({}));
+2 -1
View File
@@ -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);
+38 -11
View File
@@ -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<dyn Fn(&Document, &str, &ExportSettings) -> Result<(Vec<u8>, Vec<String>), String>>;
pub type WriteFn = Box<dyn FnMut(&str, &[u8]) -> 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}");
+66
View File
@@ -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);
}