mirror of
https://github.com/storytold/photocraft.git
synced 2026-11-11 00:11:46 +00:00
Save over a flat JPEG: JPEG Options, then in place; transparent ones offered as PNG (#2223)
Fixes #2223. Carries the in-place flat Save of #5865 (closed as superseded by this PR) and revives what #2515 (@kayrockscreenprinting, closed when its author stepped away) added beyond it. File › Save now writes back over a flat file when that file still holds the document: one plain opaque layer the format can store, and only when the file held a single image. A JPEG (and a TIFF, Targa or DDS) asks its format options first, as Photoshop does; Cancel writes nothing. A JPEG whose lone layer has transparency it can't keep is offered as a PNG. Multi-image files (animations, multi-page TIFFs, multi-size icons, multi-part EXRs) still go through Save As; the source image count is kept through crash recovery. Co-authored-by: kayrockscreenprinting <257643019+kayrockscreenprinting@users.noreply.github.com>
This commit is contained in:
1 parent
4258fc838f
commit
6a676ee1be
17 files changed
+624
-22
No files matched your search
@@ -972,6 +972,13 @@ pub struct FileEncoding {
|
||||
/// side by side; saving a stereo PNG keeps this eye-order mark (#6063).
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub png_stereo: Option<u8>,
|
||||
/// How many images (frames, pages, parts, icon sizes) the file held when it held more than
|
||||
/// the one the document was opened from; `u32::MAX` when the count is unknown. File › Save
|
||||
/// then goes through Save As rather than writing one image over all of them (#5733). Kept in
|
||||
/// `.pcraft` on purpose: crash recovery reopens the autosaved bundle at the original path,
|
||||
/// where the count must still hold. A document's own Save or Save As clears it.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub source_images: Option<u32>,
|
||||
}
|
||||
|
||||
impl FileEncoding {
|
||||
|
||||
@@ -55,6 +55,12 @@ fn open_exr_parts(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let bytes = read_file(&path)?;
|
||||
let mut r = photocraft_io::exr_parts::import(&stem(&path), &bytes, &sel).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
|
||||
crate::file_cmds::tag_exr_colorspace(s, &path, &mut r.document);
|
||||
// The file holds parts or layers the selection may have left out: Save doesn't write the
|
||||
// document back over them (#5733).
|
||||
let entries = photocraft_io::exr_parts::entries(&bytes).map_or(0, |e| e.len());
|
||||
if entries > 1 {
|
||||
r.document.file_encoding.source_images = Some(u32::try_from(entries).unwrap_or(u32::MAX));
|
||||
}
|
||||
let (i, color) = s.open_document(r.document, Some(path.clone()));
|
||||
crate::file_cmds::remember_batch_open(s);
|
||||
super::file_cmds::batch_open_notice(s, &path, &r.warnings, &color);
|
||||
|
||||
@@ -128,6 +128,10 @@ impl crate::DocState {
|
||||
if self.doc.name != name {
|
||||
Arc::make_mut(&mut self.doc).name = name;
|
||||
}
|
||||
// The file now holds just this document, whatever the one it was opened from held.
|
||||
if self.doc.file_encoding.source_images.is_some() {
|
||||
Arc::make_mut(&mut self.doc).file_encoding.source_images = None;
|
||||
}
|
||||
self.path = Some(path);
|
||||
self.saved_revision = self.revision;
|
||||
}
|
||||
|
||||
@@ -28,6 +28,24 @@ fn autosave_then_recover() {
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
|
||||
/// A recovered document opened from a multi-frame GIF comes back at its original path with its
|
||||
/// frame count, so File › Save still goes through Save As rather than writing one frame over
|
||||
/// the animation (#5733).
|
||||
#[test]
|
||||
fn recovery_keeps_the_source_image_count() {
|
||||
let dir = temp_dir("recovery-source-images");
|
||||
let mut doc = rich_doc(ColorMode::Rgb, SampleType::U8);
|
||||
doc.file_encoding.source_images = Some(4);
|
||||
let saver = Autosaver::new(&dir, "gif-1");
|
||||
saver.request(Arc::new(doc), 1, Some("/work/anim.gif".into()), SaveOptions::default());
|
||||
saver.flush().expect("a save ran").unwrap();
|
||||
let entries = list_recovery(&dir);
|
||||
assert_eq!(entries.len(), 1);
|
||||
assert_eq!(entries[0].info.original_path.as_deref(), Some("/work/anim.gif"));
|
||||
assert_eq!(recover(&entries[0]).unwrap().file_encoding.source_images, Some(4));
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
|
||||
/// A queued snapshot can fail later; callers must observe and retry that revision.
|
||||
#[test]
|
||||
fn background_save_outcomes_report_failure_then_recovery() {
|
||||
|
||||
@@ -1091,3 +1091,26 @@ fn arrow_document_is_backward_compatible() {
|
||||
let loaded = load_from_bytes(&bytes).unwrap();
|
||||
assert_eq!(loaded, doc);
|
||||
}
|
||||
|
||||
/// `FileEncoding::source_images` is kept in `.pcraft` with the other encoder settings (#5733):
|
||||
/// the crash-recovery autosave is a `.pcraft` bundle reopened at the document's original path, so
|
||||
/// dropping the count would let Save write one frame over a multi-image file. A manifest written
|
||||
/// before the key existed loads with no count.
|
||||
#[test]
|
||||
fn file_encoding_source_images_roundtrips_and_defaults_when_absent() {
|
||||
for count in [3, u32::MAX] {
|
||||
let mut doc = Document::new("anim", photocraft_doc::Size::new(2, 2), ColorMode::Rgb, SampleType::U8);
|
||||
doc.file_encoding = photocraft_doc::FileEncoding { jpeg_quality: Some(80), source_images: Some(count), ..Default::default() };
|
||||
let bytes = save_to_bytes(&doc, &SaveOptions::default()).unwrap();
|
||||
let loaded = load_from_bytes(&bytes).unwrap();
|
||||
assert_eq!(loaded.file_encoding.source_images, Some(count));
|
||||
assert_eq!(loaded, doc);
|
||||
|
||||
// The same manifest without the key, as an older build wrote it.
|
||||
let mut json = serde_json::to_value(read_manifest(&bytes).unwrap().document).unwrap();
|
||||
let encoding = json["file_encoding"].as_object_mut().unwrap();
|
||||
assert_eq!(encoding.remove("sourceImages"), Some(serde_json::json!(count)));
|
||||
let old: photocraft_format::manifest::DocM = serde_json::from_value(json).unwrap();
|
||||
assert_eq!(old.file_encoding, photocraft_doc::FileEncoding { jpeg_quality: Some(80), ..Default::default() });
|
||||
}
|
||||
}
|
||||
+40
-8
@@ -237,7 +237,8 @@ impl FlatBuilder {
|
||||
}
|
||||
|
||||
/// The document, after copying and pruning whatever is left: the one-shot result and the last
|
||||
/// step of a stepped one.
|
||||
/// step of a stepped one. Like the one-shot import, it records how many images the file holds
|
||||
/// when the decoder read only the first, so Save doesn't write over the rest (#5733).
|
||||
pub(crate) fn finish(mut self, bytes: &[u8], format: Option<Format>) -> Result<ImportResult, IoError> {
|
||||
let copied = self.copy_done();
|
||||
while !self.copy_done() {
|
||||
@@ -251,6 +252,7 @@ impl FlatBuilder {
|
||||
let Self { name, img, mode, depth, surface, warnings, .. } = self;
|
||||
let (w, h) = img.dimensions();
|
||||
let mut r = flat_document(&name, (w, h, img.layout(), mode, depth), surface, img.icc.clone(), &img.meta, warnings, Some(&img))?;
|
||||
r.document.file_encoding.source_images = source_images(&img.warnings);
|
||||
apply_flat_format(&mut r, bytes, format, Some(&img))?;
|
||||
Ok(r)
|
||||
}
|
||||
@@ -274,7 +276,9 @@ pub(crate) fn apply_flat_format(r: &mut ImportResult, bytes: &[u8], format: Opti
|
||||
r.document.mode = ColorMode::Indexed;
|
||||
r.document.color_table = Some(photocraft_doc::ColorTable { colors, transparent });
|
||||
}
|
||||
r.document.file_encoding = file_encoding_as(format, bytes);
|
||||
// The decoder's count of frames or pages left out (set above) stays alongside the encoding.
|
||||
let encoding = file_encoding_as(format, bytes);
|
||||
r.document.file_encoding = photocraft_doc::FileEncoding { source_images: r.document.file_encoding.source_images.or(encoding.source_images), ..encoding };
|
||||
if format == Some(Format::Tga) && img.is_some_and(|i| i.layout() == ChannelLayout::Rgba) {
|
||||
tga_alpha_to_channel(&mut r.document)?;
|
||||
}
|
||||
@@ -370,6 +374,11 @@ fn file_encoding_as(format: Option<Format>, bytes: &[u8]) -> photocraft_doc::Fil
|
||||
}
|
||||
None => {}
|
||||
},
|
||||
// An icon file's directory count (ICONDIR: reserved, type, count, little-endian); the
|
||||
// decoder opens one of its sizes.
|
||||
Some(Format::Ico) => {
|
||||
e.source_images = bytes.get(4..6).and_then(|c| <[u8; 2]>::try_from(c).ok()).map(|c| u32::from(u16::from_le_bytes(c))).filter(|&n| n > 1);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
e
|
||||
@@ -539,6 +548,7 @@ fn import_tiff_layered(
|
||||
}
|
||||
ctl.check().map_err(|_| IoError::Cancelled)?;
|
||||
let mut r = crate::tiff_layers::import_layered(name, &img, plan, ctl)?;
|
||||
r.document.file_encoding.source_images = r.document.file_encoding.source_images.or(source_images(&img.warnings));
|
||||
if orientation != 1 {
|
||||
r.warnings.push("the TIFF's orientation tag was ignored so the layers stay aligned with the image".to_string());
|
||||
}
|
||||
@@ -663,7 +673,9 @@ fn tiff_to_document(
|
||||
let mut warnings: Vec<String> = bands.warnings.iter().map(ToString::to_string).collect();
|
||||
warnings.extend(f.warning.clone());
|
||||
let meta = std::mem::take(&mut bands.meta);
|
||||
Ok(single_layer_document(name, (w, h), f, layout.has_alpha(), s, bands.icc.take(), meta, warnings))
|
||||
let mut r = single_layer_document(name, (w, h), f, layout.has_alpha(), s, bands.icc.take(), meta, warnings);
|
||||
r.document.file_encoding.source_images = source_images(&bands.warnings);
|
||||
Ok(r)
|
||||
}
|
||||
|
||||
/// The bytes of a layer's tiles holding a `w` × `h` image of `bpp` bytes a pixel: tiles are whole
|
||||
@@ -798,7 +810,22 @@ fn image_to_document_with_memory(name: &str, img: &Image, ctl: &Interrupt, avail
|
||||
}
|
||||
ctl.check().map_err(|_| IoError::Cancelled)?;
|
||||
ctl.progress(0.95);
|
||||
flat_document(name, (w, h, img.layout(), mode, depth), s, img.icc.clone(), &img.meta, warnings, Some(img))
|
||||
let mut r = flat_document(name, (w, h, img.layout(), mode, depth), s, img.icc.clone(), &img.meta, warnings, Some(img))?;
|
||||
r.document.file_encoding.source_images = source_images(&img.warnings);
|
||||
Ok(r)
|
||||
}
|
||||
|
||||
/// How many frames, pages, parts or entries the file holds when the decoder read only one of
|
||||
/// them (`u32::MAX` when it couldn't count them), so Save doesn't write the document back over
|
||||
/// all of them (#5733).
|
||||
fn source_images(warnings: &[codecs::DecodeWarning]) -> Option<u32> {
|
||||
warnings.iter().find_map(|w| match *w {
|
||||
codecs::DecodeWarning::MoreFrames { total } | codecs::DecodeWarning::MorePages { total } | codecs::DecodeWarning::MoreParts { total } => {
|
||||
Some(total.unwrap_or(u32::MAX))
|
||||
}
|
||||
codecs::DecodeWarning::MoreElements { total } => Some(total),
|
||||
_ => None,
|
||||
})
|
||||
}
|
||||
|
||||
/// What the decoder noticed (frames or pages left out, data ending early) and the conversion
|
||||
@@ -972,12 +999,16 @@ impl PngStream {
|
||||
self.surface.prune_cursor()
|
||||
}
|
||||
|
||||
/// The document of the layer, with the profile and metadata the file ended with.
|
||||
/// The document of the layer, with the profile and metadata the file ended with, and the
|
||||
/// count of images the file holds when the decoder read only the first (an animated PNG's
|
||||
/// frames), so Save doesn't write one image over all of them (#5733).
|
||||
pub(crate) fn into_document(self, name: &str, extras: codecs::PngExtras) -> Result<ImportResult, IoError> {
|
||||
let Self { surface, header, .. } = self;
|
||||
let (mode, _, depth, _) = flat_target(header.layout, header.sample);
|
||||
let warnings = extras.warnings.iter().map(ToString::to_string).collect();
|
||||
flat_document(name, (header.width, header.height, header.layout, mode, depth), surface, extras.icc, &extras.meta, warnings, None)
|
||||
let mut r = flat_document(name, (header.width, header.height, header.layout, mode, depth), surface, extras.icc, &extras.meta, warnings, None)?;
|
||||
r.document.file_encoding.source_images = source_images(&extras.warnings);
|
||||
Ok(r)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1622,8 +1653,9 @@ fn compact_alpha(data: &mut Vec<u8>, color: usize, alpha: usize) {
|
||||
data.truncate(n * color);
|
||||
}
|
||||
|
||||
/// Whether every pixel of `s` over `r` is fully opaque (read from the tiles, no copy).
|
||||
pub(crate) fn opaque_surface(s: &Surface, r: Rect) -> bool {
|
||||
/// Whether every pixel of `s` over `r` is fully opaque (read from the tiles, no copy). `s` must
|
||||
/// have an alpha channel.
|
||||
pub fn opaque_surface(s: &Surface, r: Rect) -> bool {
|
||||
let fmt = s.format();
|
||||
let (ch, bps) = (fmt.channels(), fmt.sample.bytes());
|
||||
let a = ch - 1;
|
||||
|
||||
@@ -101,7 +101,7 @@ pub use adjust_map::ADJUSTMENT_KEYS;
|
||||
pub use flat::{
|
||||
DEFAULT_LOSSY_WEBP_QUALITY, apply_file_encoding, document_to_image, document_to_image_with_renderer, export_flat, export_flat_with_renderer,
|
||||
export_png_streamed_with_renderer, export_preview_png_with_renderer, file_encoding, import_gif_frames, import_tiff_page, import_tiff_page_with_options,
|
||||
import_webp_frames, tga_alpha_channel_to_transparency, tga_default_bits,
|
||||
import_webp_frames, opaque_surface, tga_alpha_channel_to_transparency, tga_default_bits,
|
||||
};
|
||||
pub use png_steps::PngImport;
|
||||
// Re-exported for the `flat_open_bench` example, which times the tile-building phase of a flat open.
|
||||
|
||||
@@ -77,6 +77,7 @@ mod resolution_axes;
|
||||
mod roundtrip;
|
||||
mod smart_contents_identity;
|
||||
mod smart_corpus;
|
||||
mod source_images;
|
||||
mod streaming_exr;
|
||||
mod svg;
|
||||
mod text_corpus;
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
//! `FileEncoding::source_images`: a file holding more images than the one a document opens from
|
||||
//! (an animation, a multi-size icon) is recorded so File › Save doesn't write over it (#5733).
|
||||
|
||||
use crate::common::raster;
|
||||
use photocraft_codecs::web::{GifFrame, encode_gif_animated};
|
||||
use photocraft_io::import;
|
||||
|
||||
/// A 3×2 animated GIF of `frames` identical frames.
|
||||
fn gif(frames: usize) -> Vec<u8> {
|
||||
let frames: Vec<GifFrame> = (0..frames).map(|_| GifFrame { indices: vec![0; 6], palette: vec![[255, 0, 0]], transparent: None, delay_cs: 5 }).collect();
|
||||
encode_gif_animated(3, 2, &frames, true).unwrap()
|
||||
}
|
||||
|
||||
/// An icon file whose `entries` directory entries all point at one PNG image.
|
||||
fn ico(entries: u16) -> Vec<u8> {
|
||||
let mut d = photocraft_doc::Document::new("i", photocraft_geom::Size::new(4, 4), photocraft_color::ColorMode::Rgb, photocraft_color::SampleType::U8);
|
||||
let fmt = d.pixel_format();
|
||||
d.layers.push(raster("Layer 0", fmt, d.bounds(), 3, true));
|
||||
let one = photocraft_io::export(&d, "i.ico", &Default::default()).unwrap().bytes;
|
||||
// ICONDIR (6 bytes) + one 16-byte ICONDIRENTRY whose last 4 bytes are the image offset.
|
||||
let (entry, image) = (&one[6..22], &one[22..]);
|
||||
let offset = 6 + 16 * u32::from(entries);
|
||||
let mut out = vec![0, 0, 1, 0];
|
||||
out.extend(entries.to_le_bytes());
|
||||
for _ in 0..entries {
|
||||
out.extend(&entry[..12]);
|
||||
out.extend(offset.to_le_bytes());
|
||||
}
|
||||
out.extend(image);
|
||||
out
|
||||
}
|
||||
|
||||
/// An animated GIF records how many frames it holds; a single frame records nothing.
|
||||
#[test]
|
||||
fn an_animation_records_its_frame_count() {
|
||||
assert_eq!(import("a.gif", &gif(2)).unwrap().document.file_encoding.source_images, Some(2));
|
||||
assert_eq!(import("a.gif", &gif(1)).unwrap().document.file_encoding.source_images, None);
|
||||
}
|
||||
|
||||
/// A multi-size icon records its directory count and opens one size; a single-size icon
|
||||
/// records nothing.
|
||||
#[test]
|
||||
fn a_multi_size_icon_records_its_entry_count() {
|
||||
let two = import("a.ico", &ico(2)).unwrap().document;
|
||||
assert_eq!(two.layers.len(), 1);
|
||||
assert_eq!(two.file_encoding.source_images, Some(2));
|
||||
assert_eq!(import("a.ico", &ico(1)).unwrap().document.file_encoding.source_images, None);
|
||||
}
|
||||
@@ -229,12 +229,17 @@ impl PhotocraftApp {
|
||||
let ext = st.path.as_deref().and_then(|p| std::path::Path::new(p).extension()).map(|e| e.to_string_lossy().to_ascii_lowercase());
|
||||
// An OpenRaster file only when it holds the whole document (#3806): else PSD is offered.
|
||||
let holds_ora = st.path.as_deref().is_some_and(|p| photocraft_engine::file_cmds::holds_at(&st.doc, p));
|
||||
// A JPEG whose lone layer has transparency it can't keep is offered as a PNG, as
|
||||
// Photoshop's Save does once pixels are deleted (#2223).
|
||||
let transparent_jpeg = ext.as_deref().and_then(photocraft_codecs::from_extension) == Some(photocraft_codecs::Format::Jpeg)
|
||||
&& plain_raster(&st.doc).is_some_and(|l| !crate::opaque_layer(&st.doc, l));
|
||||
let writable = ext.is_some_and(|e| {
|
||||
matches!(e.as_str(), photocraft_format::EXTENSION | "psd" | "psb" | "tif" | "tiff")
|
||||
|| (e == "ora" && holds_ora)
|
||||
|| (flat && photocraft_codecs::from_extension(&e).is_some_and(|f| f.caps().write))
|
||||
});
|
||||
let suggested = match &st.path {
|
||||
Some(p) if transparent_jpeg => std::path::Path::new(p).with_extension("png").to_string_lossy().into_owned(),
|
||||
Some(p) if writable => p.clone(),
|
||||
p => {
|
||||
let source = p.as_deref().unwrap_or(&st.doc.name);
|
||||
@@ -317,6 +322,38 @@ impl PhotocraftApp {
|
||||
Ok(r)
|
||||
}
|
||||
|
||||
/// File › Save over the flat file the document came from, which still holds it
|
||||
/// ([`crate::saves_flat_in_place`]): a JPEG asks JPEG Options first, as Photoshop does
|
||||
/// (#2223), a TIFF its TIFF Options, and an RGB Targa or a DDS texture its Targa or DDS
|
||||
/// Options, unless `choices` answer them; the answer writes the file and is recorded as a
|
||||
/// plain `file.save` (#3531). Anything else is written straight back.
|
||||
pub(crate) fn save_flat_in_place(&mut self, path: String, choices: FormatChoices) -> Result<Value, String> {
|
||||
if self.save_options_open() {
|
||||
return Err(SAVE_OPTIONS_OPEN.into());
|
||||
}
|
||||
if choices.tga_bits.is_none() && targa_options_ui::wants_prompt(self, &path) {
|
||||
let copy = self.saves_copy(&path, None)?;
|
||||
targa_options_ui::park(self, path.clone(), copy, "file.save")?;
|
||||
return Ok(serde_json::json!({"path": path, "warnings": []}));
|
||||
}
|
||||
if choices.dds.is_none() && dds_options_ui::wants_prompt(&path) {
|
||||
let copy = self.saves_copy(&path, None)?;
|
||||
dds_options_ui::park(self, path.clone(), copy, "file.save")?;
|
||||
return Ok(serde_json::json!({"path": path, "warnings": []}));
|
||||
}
|
||||
if choices.jpeg.is_none() && jpeg_options_ui::wants_prompt(self, &path) {
|
||||
let copy = self.saves_copy(&path, None)?;
|
||||
jpeg_options_ui::park(self, path.clone(), copy, "file.save")?;
|
||||
return Ok(serde_json::json!({"path": path, "warnings": []}));
|
||||
}
|
||||
// A TIFF asks TIFF Options over its own file too, as Save As does.
|
||||
if choices.tiff.is_none() && tiff_options_ui::wants_prompt(self, &path) {
|
||||
tiff_options_ui::park(self, path.clone(), false, "file.save")?;
|
||||
return Ok(serde_json::json!({"path": path, "warnings": []}));
|
||||
}
|
||||
self.save_in_place(path, choices)
|
||||
}
|
||||
|
||||
/// Writes the Save As and journals it as a `file.saveAs` step with its path (and TIFF,
|
||||
/// Targa, DDS or JPEG answer), so an action being recorded keeps it (#2032).
|
||||
fn save_as_step(&mut self, path: String, tiff_layers: Option<bool>, choices: FormatChoices) -> Result<Value, String> {
|
||||
@@ -445,17 +482,19 @@ impl PhotocraftApp {
|
||||
if id == "file.save" {
|
||||
// The file the action is played on: one that holds the whole document, or a flat
|
||||
// file a recorded Save confirmed in the dialog while the document is still a plain
|
||||
// raster, so it is never flattened over (#3531, #3806). A flat file's recorded Targa,
|
||||
// DDS or JPEG answer writes the same way, without asking.
|
||||
// raster, so it is never flattened over (#3531, #3806), and only while that file held
|
||||
// just this one image (#5733). A flat file's recorded Targa, DDS or JPEG answer writes
|
||||
// the same way, without asking (#2223).
|
||||
let choices = FormatChoices::from_step(params, id)?;
|
||||
let st = self.session.active().ok_or("no document")?;
|
||||
let flat_own = |p: &str| {
|
||||
!photocraft_engine::file_cmds::saves_in_place(p)
|
||||
&& plain_raster(&st.doc).is_some()
|
||||
&& st.doc.file_encoding.source_images.is_none()
|
||||
&& photocraft_engine::file_cmds::extension(p).is_some_and(|e| photocraft_codecs::from_extension(&e).is_some_and(|f| f.caps().write))
|
||||
};
|
||||
let path = st.path.clone().filter(|p| photocraft_engine::file_cmds::holds_at(&st.doc, p) || flat_own(p)).ok_or(
|
||||
"Save writes back only to the document's own PSD, PSB, .pcraft or OpenRaster file (OpenRaster only when it holds everything), or to its own flat file while the document is a single plain layer",
|
||||
"Save writes back only to the document's own PSD, PSB, .pcraft or OpenRaster file (OpenRaster only when it holds everything), or to its own flat file while the document is a single plain layer and the file held only that image",
|
||||
)?;
|
||||
return self.save_in_place(path, choices);
|
||||
}
|
||||
|
||||
@@ -54,8 +54,15 @@ pub fn confirm(app: &mut PhotocraftApp, fields: &Map<String, Value>) -> Result<V
|
||||
let r = app.save_to(path.clone(), choices)?;
|
||||
app.journal_as_plain_save(&path, from);
|
||||
Ok(r)
|
||||
} else {
|
||||
} else if app.session.active().is_some_and(|st| crate::saves_flat_in_place(&st.doc, &path)) {
|
||||
// Its own flat file: a JPEG, TIFF, Targa or DDS texture asks its format's options next (#2223).
|
||||
app.save_flat_in_place(path, choices)
|
||||
} else if app.session.active().is_some_and(|st| photocraft_engine::file_cmds::holds_at(&st.doc, &path)) {
|
||||
app.save_in_place(path, crate::FormatChoices::default())
|
||||
} else {
|
||||
// The document changed while the prompt was open and its file can't keep it any more:
|
||||
// Save As, never a write over the original with less (#5733).
|
||||
app.save_dialog_flow(None, choices, true)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -16,7 +16,8 @@
|
||||
//! is written by [`confirm`] and journalled as the `record` command passed to `park`: with its
|
||||
//! path, `quality` (0–12, left out when the file's own quality was kept) and `jpegFormat`, or
|
||||
//! without the path for a plain `file.save`, which writes whichever file an action is played on
|
||||
//! ([`plain_save_step`]).
|
||||
//! ([`plain_save_step`]). File › Save over a flat JPEG that still holds the document asks too,
|
||||
//! as Photoshop does (#2223): `PhotocraftApp::save_flat_in_place`.
|
||||
|
||||
use photocraft_codecs::JpegFormat;
|
||||
use serde_json::Value;
|
||||
|
||||
@@ -108,6 +108,60 @@ fn save_dialog_preserves_flat_and_existing_layered_format_defaults() {
|
||||
}
|
||||
}
|
||||
|
||||
/// Checked in Photoshop: once some pixels of a JPEG are deleted (transparency a JPEG can't keep),
|
||||
/// Save asks where to save, defaulting to PNG; while the layer is still opaque the JPEG stays the
|
||||
/// default (#2223).
|
||||
#[test]
|
||||
fn a_jpeg_with_transparency_is_offered_as_png() {
|
||||
let ctx = egui::Context::default();
|
||||
let (mut app, open, written) = jpeg_app(Some("/pics/photo.jpg"), false);
|
||||
app.run("layer.new.layerFromBackground", json!({})).unwrap();
|
||||
menus::invoke(&mut app, &ctx, "file.saveAs", json!({})).unwrap();
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
assert!(matches!(open.borrow().as_slice(), [(FileDialogRequest::Save { suggested }, _)] if suggested == "/pics/photo.jpg"), "opaque: still a JPEG");
|
||||
answer(&open, None);
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
// Not the Background any more but still opaque: the JPEG keeps it, so Save writes back
|
||||
// after JPEG Options.
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
assert!(open.borrow().is_empty(), "no Save dialog");
|
||||
crate::jpeg_options_ui::confirm(&mut app, &ctx);
|
||||
assert_eq!(*written.borrow(), ["/pics/photo.jpg"]);
|
||||
written.borrow_mut().clear();
|
||||
// Delete some pixels: the lone layer is now partly transparent.
|
||||
app.run("select.rect", json!({"x": 0, "y": 0, "width": 2, "height": 2})).unwrap();
|
||||
app.run("edit.clear", json!({})).unwrap();
|
||||
app.run("select.deselect", json!({})).unwrap();
|
||||
for command in ["file.save", "file.saveAs"] {
|
||||
menus::invoke(&mut app, &ctx, command, json!({})).unwrap();
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
assert!(
|
||||
matches!(open.borrow().as_slice(), [(FileDialogRequest::Save { suggested }, _)] if suggested == "/pics/photo.png"),
|
||||
"{command}: {:?}",
|
||||
open.borrow().first().map(|(r, _)| r.clone())
|
||||
);
|
||||
answer(&open, None);
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
}
|
||||
assert!(written.borrow().is_empty());
|
||||
// Saved as the PNG it suggests, the document lives there and keeps its transparency.
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
answer(&open, Some(FileDialogAnswer::SaveTo("/pics/photo.png".into())));
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
assert_eq!(*written.borrow(), ["/pics/photo.png"]);
|
||||
assert_eq!(app.session.active().unwrap().path.as_deref(), Some("/pics/photo.png"));
|
||||
}
|
||||
|
||||
/// A Grayscale layer without an alpha channel is opaque, never read past its colour samples.
|
||||
#[test]
|
||||
fn a_layer_without_alpha_is_opaque() {
|
||||
let doc = photocraft_doc::Document::new("gray", photocraft_geom::Size::new(3, 3), photocraft_doc::ColorMode::Grayscale, photocraft_color::SampleType::U8);
|
||||
let layer = photocraft_doc::Layer::raster("gray", photocraft_color::PixelFormat::GRAY8);
|
||||
assert!(crate::opaque_layer(&doc, &layer));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn transparent_png_keeps_its_format_until_its_lone_layer_becomes_smart() {
|
||||
let (mut app, open, written) = app();
|
||||
@@ -168,12 +222,17 @@ fn layered_save_respects_explicit_jpeg_choices() {
|
||||
}
|
||||
}
|
||||
|
||||
/// File › Save on a flat JPEG asks for its name; confirming its own file asks JPEG Options and,
|
||||
/// once answered, records a plain Save that writes the file an action is played on (#3531).
|
||||
/// File › Save on a flat JPEG it can't write back in place (here: transparency) asks for its
|
||||
/// name; confirming its own file asks JPEG Options and, once answered, records a plain Save that
|
||||
/// writes the file an action is played on (#3531).
|
||||
#[test]
|
||||
fn plain_save_of_a_flat_jpeg_asks_jpeg_options_and_records_a_plain_save() {
|
||||
let ctx = egui::Context::default();
|
||||
let (mut app, open, written) = jpeg_app(Some("/pics/photo.jpg"), false);
|
||||
app.run("layer.new.layerFromBackground", json!({})).unwrap();
|
||||
app.run("select.rect", json!({"x": 0, "y": 0, "width": 2, "height": 2})).unwrap();
|
||||
app.run("edit.clear", json!({})).unwrap();
|
||||
app.run("select.deselect", json!({})).unwrap();
|
||||
app.run("actions.record", json!({})).unwrap();
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
@@ -199,6 +258,11 @@ fn plain_save_of_a_flat_jpeg_records_and_replays_a_changed_quality() {
|
||||
q.borrow_mut().push(s.jpeg_quality);
|
||||
Ok((b"out".to_vec(), Vec::new()))
|
||||
}));
|
||||
// Some pixels deleted: no longer saved in place (#2223), so Save goes through the dialog.
|
||||
app.run("layer.new.layerFromBackground", json!({})).unwrap();
|
||||
app.run("select.rect", json!({"x": 0, "y": 0, "width": 2, "height": 2})).unwrap();
|
||||
app.run("edit.clear", json!({})).unwrap();
|
||||
app.run("select.deselect", json!({})).unwrap();
|
||||
app.run("actions.record", json!({})).unwrap();
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
@@ -1418,6 +1482,171 @@ fn save_asks_before_replacing_a_file_another_app_changed() {
|
||||
assert!(disk_prompt(&app).is_none());
|
||||
}
|
||||
|
||||
/// A document that no longer fits its flat file by the time the changed-on-disk prompt is
|
||||
/// answered goes through Save As rather than being written over the original.
|
||||
#[test]
|
||||
fn replace_after_the_document_outgrew_its_flat_file_asks_where_to_save() {
|
||||
let ctx = egui::Context::default();
|
||||
let path = "/pics/My image.jpg";
|
||||
let (mut app, open, written) = jpeg_app(None, false);
|
||||
let disk = on_disk(&mut app);
|
||||
changed_by_another_app(&disk, path);
|
||||
app.opened_from(path);
|
||||
app.run("image.adjustments.invert", json!({})).unwrap();
|
||||
changed_by_another_app(&disk, path);
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
let id = disk_prompt(&app).expect("Save asks first").id;
|
||||
app.run("layer.new.layer", json!({})).unwrap();
|
||||
crate::dialogs::confirm(&mut app, id).unwrap();
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
assert!(matches!(open.borrow().as_slice(), [(FileDialogRequest::Save { .. }, _)]), "Save As");
|
||||
assert!(written.borrow().is_empty(), "the JPEG is not written over");
|
||||
}
|
||||
|
||||
/// File › Save in place over a flat JPEG another app changed asks first (#6596), then JPEG
|
||||
/// Options (#2223); Cancel in either writes nothing, and OK in both writes a plain Save.
|
||||
#[test]
|
||||
fn save_in_place_of_a_flat_file_asks_first_when_another_app_changed_it() {
|
||||
let ctx = egui::Context::default();
|
||||
let path = "/pics/My image.jpg";
|
||||
let (mut app, open, written) = jpeg_app(None, false);
|
||||
let disk = on_disk(&mut app);
|
||||
changed_by_another_app(&disk, path);
|
||||
app.opened_from(path);
|
||||
app.run("image.adjustments.invert", json!({})).unwrap();
|
||||
changed_by_another_app(&disk, path);
|
||||
// One plain Background layer: the JPEG keeps it, so Save writes back without a dialog (#5733)
|
||||
// but asks before replacing the changed file (#6596).
|
||||
let r = menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
assert_eq!(r["confirmationPending"], true, "{r}");
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
assert!(open.borrow().is_empty(), "no Save dialog");
|
||||
assert!(written.borrow().is_empty(), "the changed file is not replaced before the answer");
|
||||
let id = disk_prompt(&app).expect("Save asks first").id;
|
||||
crate::dialogs::confirm(&mut app, id).unwrap();
|
||||
// Then JPEG Options (#2223): Cancel there writes nothing either.
|
||||
assert!(written.borrow().is_empty());
|
||||
assert_eq!(app.jpeg_options.as_ref().map(|p| (p.record, p.copy)), Some(("file.save", false)));
|
||||
app.jpeg_options = None;
|
||||
assert!(written.borrow().is_empty());
|
||||
assert!(app.session.active().unwrap().is_dirty());
|
||||
// Saving again asks both again; OK in both writes it.
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
let id = disk_prompt(&app).expect("Save asks first").id;
|
||||
assert!(app.jpeg_options.is_none(), "the changed-on-disk prompt comes first");
|
||||
crate::dialogs::confirm(&mut app, id).unwrap();
|
||||
crate::jpeg_options_ui::confirm(&mut app, &ctx);
|
||||
assert_eq!(*written.borrow(), [path]);
|
||||
assert!(!app.session.active().unwrap().is_dirty());
|
||||
assert_eq!(app.session.journal.last(), Some(&("file.save".to_string(), json!({}))), "recorded as a plain Save");
|
||||
}
|
||||
|
||||
/// The JPEG quality of each save, as the exporter was given it.
|
||||
type Qualities = Rc<RefCell<Vec<Option<u8>>>>;
|
||||
|
||||
/// [`jpeg_app`] opened from `path` and edited, whose exporter records the JPEG quality of each
|
||||
/// save (#2223).
|
||||
fn flat_jpeg_app(path: &str) -> (PhotocraftApp, Open, Rc<RefCell<Vec<String>>>, Qualities) {
|
||||
let (mut app, open, written) = jpeg_app(Some(path), false);
|
||||
let qualities = Qualities::default();
|
||||
let q = qualities.clone();
|
||||
app.services.export = Some(Box::new(move |_: &photocraft_doc::Document, _: &str, s: &crate::ExportSettings| {
|
||||
q.borrow_mut().push(s.jpeg_quality);
|
||||
Ok((b"out".to_vec(), Vec::new()))
|
||||
}));
|
||||
app.run("image.adjustments.invert", json!({})).unwrap();
|
||||
(app, open, written, qualities)
|
||||
}
|
||||
|
||||
/// File › Save on a flat JPEG that still holds the document asks JPEG Options, then writes over
|
||||
/// it with no Save dialog, as Photoshop does; Cancel writes nothing (#2223).
|
||||
#[test]
|
||||
fn save_asks_jpeg_options_then_writes_a_flat_jpeg_over_itself() {
|
||||
let ctx = egui::Context::default();
|
||||
let (mut app, open, written, qualities) = flat_jpeg_app("/pics/photo.jpg");
|
||||
let own = app.session.active().unwrap().doc.file_encoding.jpeg_quality.expect("an opened JPEG knows its quality");
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
assert!(open.borrow().is_empty(), "no Save dialog");
|
||||
assert!(written.borrow().is_empty(), "nothing written before the options are answered");
|
||||
let start = crate::jpeg_options_ui::Start::for_active(&app);
|
||||
assert_eq!(app.jpeg_options.as_ref().map(|p| (p.path.as_str(), p.quality, p.copy, p.record)), Some(("/pics/photo.jpg", start.quality, false, "file.save")));
|
||||
// Another save waits for the answer.
|
||||
assert!(menus::invoke(&mut app, &ctx, "file.save", json!({})).is_err());
|
||||
// Cancel (what the dialog does): nothing is written and the document stays unsaved.
|
||||
app.jpeg_options = None;
|
||||
assert!(written.borrow().is_empty());
|
||||
assert!(app.session.active().unwrap().is_dirty());
|
||||
// OK as it starts: the file's own quality, exactly, recorded as a plain Save.
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
crate::jpeg_options_ui::confirm(&mut app, &ctx);
|
||||
assert_eq!(*written.borrow(), ["/pics/photo.jpg"]);
|
||||
assert_eq!(*qualities.borrow(), [Some(own)]);
|
||||
assert_eq!(app.session.journal.last(), Some(&("file.save".to_string(), json!({}))));
|
||||
assert!(!app.session.active().unwrap().is_dirty());
|
||||
// OK at another quality and format: written with them, and recorded with them.
|
||||
app.run("image.adjustments.invert", json!({})).unwrap();
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
if let Some(p) = app.jpeg_options.as_mut() {
|
||||
p.quality = 3;
|
||||
p.format = photocraft_codecs::JpegFormat::Progressive;
|
||||
}
|
||||
crate::jpeg_options_ui::confirm(&mut app, &ctx);
|
||||
assert_eq!(*written.borrow(), ["/pics/photo.jpg", "/pics/photo.jpg"]);
|
||||
assert_eq!(qualities.borrow().last().copied().flatten(), Some(photocraft_engine::file_cmds::jpeg_quality(3.0)));
|
||||
assert_eq!(app.session.journal.last(), Some(&("file.save".to_string(), json!({"quality": 3, "jpegFormat": "progressive"}))));
|
||||
assert_eq!(app.session.active().unwrap().path.as_deref(), Some("/pics/photo.jpg"));
|
||||
// A script's `quality` answers it: no prompt.
|
||||
app.run("image.adjustments.invert", json!({})).unwrap();
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({"quality": 12})).unwrap();
|
||||
assert!(app.jpeg_options.is_none());
|
||||
assert_eq!(written.borrow().len(), 3);
|
||||
assert_eq!(qualities.borrow().last().copied().flatten(), Some(photocraft_engine::file_cmds::jpeg_quality(12.0)));
|
||||
// Once a layer is added, Save asks where to save a PSD instead (#2234).
|
||||
app.run("layer.new.layer", json!({})).unwrap();
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
assert!(app.jpeg_options.is_none());
|
||||
assert!(matches!(open.borrow().as_slice(), [(FileDialogRequest::Save { suggested }, _)] if suggested == "/pics/photo.psd"));
|
||||
answer(&open, None);
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
}
|
||||
|
||||
/// Adds another flat JPEG, opened from `path`, and makes it active.
|
||||
fn add_flat_jpeg(app: &mut PhotocraftApp, path: &str) {
|
||||
let bytes = photocraft_io::export(&app.session.active().unwrap().doc, "other.jpg", &Default::default()).unwrap().bytes;
|
||||
let doc = photocraft_io::import("other.jpg", &bytes).unwrap().document;
|
||||
let index = app.session.add_document(doc, Some(path.to_string()));
|
||||
app.session.set_active(index);
|
||||
}
|
||||
|
||||
/// An action being recorded keeps File › Save over a flat JPEG as a `file.save` step with the
|
||||
/// JPEG Options answer, and playing it writes the active flat JPEG over itself with that answer
|
||||
/// and no prompt (#2223). A cancelled JPEG Options records nothing.
|
||||
#[test]
|
||||
fn flat_jpeg_saves_are_recorded_and_replay_over_the_open_file() {
|
||||
let ctx = egui::Context::default();
|
||||
let (mut app, open, written, qualities) = flat_jpeg_app("/pics/photo.jpg");
|
||||
app.run("actions.record", json!({})).unwrap();
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
app.jpeg_options = None;
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
if let Some(p) = app.jpeg_options.as_mut() {
|
||||
p.quality = 5;
|
||||
}
|
||||
crate::jpeg_options_ui::confirm(&mut app, &ctx);
|
||||
app.run("actions.stop", json!({})).unwrap();
|
||||
assert_eq!(app.session.actions.list[0].steps, [("file.save".to_string(), json!({"quality": 5}))]);
|
||||
add_flat_jpeg(&mut app, "/pics/other.jpg");
|
||||
written.borrow_mut().clear();
|
||||
let played = app.run("actions.play", json!({"action": 0})).unwrap();
|
||||
assert!(played.get("failed").is_none(), "{played}");
|
||||
assert!(app.jpeg_options.is_none(), "playback does not ask");
|
||||
assert_eq!(*written.borrow(), ["/pics/other.jpg"]);
|
||||
assert_eq!(qualities.borrow().last().copied().flatten(), Some(photocraft_engine::file_cmds::jpeg_quality(5.0)));
|
||||
assert!(open.borrow().is_empty(), "no Save dialog");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn save_of_a_flat_file_asks_after_the_dialog_when_another_app_changed_it() {
|
||||
let ctx = egui::Context::default();
|
||||
@@ -1427,6 +1656,12 @@ fn save_of_a_flat_file_asks_after_the_dialog_when_another_app_changed_it() {
|
||||
let disk = on_disk(&mut app);
|
||||
changed_by_another_app(&disk, path);
|
||||
app.opened_from(path);
|
||||
// Transparency JPEG can't keep: Save goes through the dialog (which suggests a PNG; the
|
||||
// user picks the JPEG anyway).
|
||||
app.run("layer.new.layerFromBackground", json!({})).unwrap();
|
||||
app.run("select.rect", json!({"x": 0, "y": 0, "width": 2, "height": 2})).unwrap();
|
||||
app.run("edit.clear", json!({})).unwrap();
|
||||
app.run("select.deselect", json!({})).unwrap();
|
||||
app.run("image.adjustments.invert", json!({})).unwrap();
|
||||
changed_by_another_app(&disk, path);
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
@@ -1472,10 +1707,10 @@ fn a_scripted_quality_survives_the_changed_on_disk_prompt() {
|
||||
app.opened_from(path);
|
||||
app.run("image.adjustments.invert", json!({})).unwrap();
|
||||
changed_by_another_app(&disk, path);
|
||||
// Saved in place here (#2223): no Save dialog, only the changed-on-disk prompt.
|
||||
menus::invoke(&mut app, &ctx, "file.save", json!({"quality": 5})).unwrap();
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
answer(&open, Some(FileDialogAnswer::SaveTo(path.into())));
|
||||
app.poll_file_dialog(&ctx, None);
|
||||
assert!(open.borrow().is_empty(), "no Save dialog");
|
||||
assert!(written.borrow().is_empty(), "the changed file is not replaced before the answer");
|
||||
let id = disk_prompt(&app).expect("Save asks first").id;
|
||||
crate::dialogs::confirm(&mut app, id).unwrap();
|
||||
@@ -1483,7 +1718,7 @@ fn a_scripted_quality_survives_the_changed_on_disk_prompt() {
|
||||
assert_eq!(*written.borrow(), [path]);
|
||||
assert_eq!(qualities.borrow().last().copied().flatten(), Some(photocraft_engine::file_cmds::jpeg_quality(5.0)));
|
||||
// Recorded as a plain Save keeping the answer, so a replay writes the file it's played on.
|
||||
assert_eq!(app.session.journal.last(), Some(&("file.save".to_string(), json!({"quality": 5, "jpegFormat": "baseline"}))));
|
||||
assert_eq!(app.session.journal.last(), Some(&("file.save".to_string(), json!({"quality": 5}))));
|
||||
}
|
||||
|
||||
/// File › Save a Copy suggests "<name> copy", and with Preferences › File Handling › Do not
|
||||
|
||||
@@ -219,6 +219,124 @@ fn pcraft_documents_save_in_place_but_flat_files_ask() {
|
||||
assert!(!app.ui.status_error);
|
||||
}
|
||||
|
||||
/// #5733: like Photoshop's Save, a flat file still holding the whole document (one plain layer
|
||||
/// it can store) is written back in place; once it would lose something (a second layer, or
|
||||
/// transparency a JPEG can't keep) Save goes through Save As.
|
||||
#[test]
|
||||
fn a_flat_file_that_keeps_the_document_saves_in_place() {
|
||||
let (mut app, written) = app_with(vec![None, None]);
|
||||
let ctx = egui::Context::default();
|
||||
app.open_file("/pics/flat.png", b"x").unwrap();
|
||||
app.run("layer.new.layer", json!({})).unwrap();
|
||||
let r = menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap();
|
||||
assert_eq!(r["path"], "/pics/flat.png", "{r}");
|
||||
assert_eq!(written.borrow().len(), 1);
|
||||
// A second layer: PNG can't keep it, so Save asks (cancelled).
|
||||
app.run("layer.new.layer", json!({})).unwrap();
|
||||
assert_eq!(menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap(), json!({"fileDialog": "save"}));
|
||||
answer(&mut app);
|
||||
assert_eq!(written.borrow().len(), 1);
|
||||
// A JPEG with one transparent-capable layer that isn't the Background: JPEG has no alpha.
|
||||
app.open_file("/pics/photo.jpg", b"x").unwrap();
|
||||
app.run("layer.new.layer", json!({})).unwrap();
|
||||
assert_eq!(menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap(), json!({"fileDialog": "save"}));
|
||||
answer(&mut app);
|
||||
assert_eq!(written.borrow().len(), 1);
|
||||
}
|
||||
|
||||
/// #5733: Save doesn't write one image back over a file that held more (here a two-size icon;
|
||||
/// animations and multi-page TIFFs are recorded the same way), nor through an encoder that
|
||||
/// quantises (GIF), nor a Grayscale document into a format that stores only colour.
|
||||
#[test]
|
||||
fn save_in_place_skips_files_it_would_write_back_with_less() {
|
||||
let (mut app, written) = app_with(vec![None, None, None]);
|
||||
app.services.import = Some(Box::new(|name: &str, bytes: &[u8], _opts: &photocraft_io::ImportOptions| {
|
||||
photocraft_io::import(name, bytes).map(|r| (r.document, r.warnings)).map_err(|e| e.to_string())
|
||||
}));
|
||||
let ctx = egui::Context::default();
|
||||
let save = |app: &mut PhotocraftApp| menus::invoke(app, &ctx, "file.save", json!({})).unwrap();
|
||||
// One icon size: written back.
|
||||
app.open_file("/pics/one.ico", &ico(1)).unwrap();
|
||||
assert_eq!(save(&mut app)["path"], "/pics/one.ico");
|
||||
assert_eq!(written.borrow().len(), 1);
|
||||
// Two sizes: the document holds one, so Save asks (cancelled).
|
||||
app.open_file("/pics/two.ico", &ico(2)).unwrap();
|
||||
assert_eq!(save(&mut app), json!({"fileDialog": "save"}));
|
||||
answer(&mut app);
|
||||
// A recorded Save played on it doesn't write over the other sizes either.
|
||||
assert!(app.replay_save("file.save", &json!({})).is_err());
|
||||
// A GIF: its palette and on/off transparency would lose colours.
|
||||
app.open_file("/pics/still.gif", &gif_bytes()).unwrap();
|
||||
assert_eq!(save(&mut app), json!({"fileDialog": "save"}));
|
||||
answer(&mut app);
|
||||
// Grayscale in a BMP, which stores only colour.
|
||||
app.open_file("/pics/gray.bmp", &png_bytes(4, 3)).unwrap();
|
||||
app.run("image.mode.grayscale", json!({})).unwrap();
|
||||
assert_eq!(save(&mut app), json!({"fileDialog": "save"}));
|
||||
answer(&mut app);
|
||||
assert_eq!(written.borrow().len(), 1);
|
||||
}
|
||||
|
||||
/// #5733: Save doesn't write in place where the format's writer would drop what the document has:
|
||||
/// a gray layer's transparency in DPX, an embedded colour profile in BMP, or a clipped layer.
|
||||
#[test]
|
||||
fn save_in_place_skips_alpha_profiles_and_layer_state_the_file_would_drop() {
|
||||
let (mut app, written) = app_with(vec![None, None, None]);
|
||||
let ctx = egui::Context::default();
|
||||
let edit = |app: &mut PhotocraftApp, f: &dyn Fn(&mut Document)| f(std::sync::Arc::make_mut(&mut app.session.active_mut().unwrap().doc));
|
||||
// An RGB layer with transparency: DPX keeps its alpha, so it is written back.
|
||||
app.open_file("/pics/rgb.dpx", b"x").unwrap();
|
||||
app.run("layer.new.layer", json!({})).unwrap();
|
||||
assert_eq!(menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap()["path"], "/pics/rgb.dpx");
|
||||
assert_eq!(written.borrow().len(), 1);
|
||||
// A gray one: DPX writes gray without alpha.
|
||||
app.open_file("/pics/gray.dpx", b"x").unwrap();
|
||||
app.run("layer.new.layer", json!({})).unwrap();
|
||||
edit(&mut app, &|d| d.mode = ColorMode::Grayscale);
|
||||
assert_eq!(menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap(), json!({"fileDialog": "save"}));
|
||||
answer(&mut app);
|
||||
// A profile BMP can't embed.
|
||||
app.open_file("/pics/tagged.bmp", b"x").unwrap();
|
||||
app.run("layer.new.layer", json!({})).unwrap();
|
||||
edit(&mut app, &|d| d.icc_profile = Some(std::sync::Arc::new(vec![0; 128])));
|
||||
assert_eq!(menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap(), json!({"fileDialog": "save"}));
|
||||
answer(&mut app);
|
||||
// A clipped layer: the flat file keeps only its pixels.
|
||||
app.open_file("/pics/clipped.png", b"x").unwrap();
|
||||
app.run("layer.new.layer", json!({})).unwrap();
|
||||
edit(&mut app, &|d| d.layers[0].clipped = true);
|
||||
assert_eq!(menus::invoke(&mut app, &ctx, "file.save", json!({})).unwrap(), json!({"fileDialog": "save"}));
|
||||
answer(&mut app);
|
||||
assert_eq!(written.borrow().len(), 1);
|
||||
}
|
||||
|
||||
/// An icon file whose `entries` directory entries all point at one 4×3 PNG image.
|
||||
fn ico(entries: u16) -> Vec<u8> {
|
||||
let one = export_as(&png_bytes(4, 3), "i.ico");
|
||||
// ICONDIR (6 bytes) + one 16-byte ICONDIRENTRY whose last 4 bytes are the image offset.
|
||||
let (entry, image) = (&one[6..22], &one[22..]);
|
||||
let offset = 6 + 16 * u32::from(entries);
|
||||
let mut out = vec![0, 0, 1, 0];
|
||||
out.extend(entries.to_le_bytes());
|
||||
for _ in 0..entries {
|
||||
out.extend(&entry[..12]);
|
||||
out.extend(offset.to_le_bytes());
|
||||
}
|
||||
out.extend(image);
|
||||
out
|
||||
}
|
||||
|
||||
/// A one-frame GIF.
|
||||
fn gif_bytes() -> Vec<u8> {
|
||||
export_as(&png_bytes(4, 3), "g.gif")
|
||||
}
|
||||
|
||||
/// `bytes` (a PNG) re-encoded for the file name `name`.
|
||||
fn export_as(bytes: &[u8], name: &str) -> Vec<u8> {
|
||||
let doc = photocraft_io::import("p.png", bytes).unwrap().document;
|
||||
photocraft_io::export(&doc, name, &Default::default()).unwrap().bytes
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn templates_open_as_new_untitled_documents() {
|
||||
let (mut app, written) = app_with(Vec::new());
|
||||
|
||||
@@ -267,6 +267,7 @@ pub use services::{
|
||||
LastUpdateFn, LoadTextFn, OpenExternalSvgFn, OpenUrlFn, OsEventsFn, PenEndFn, PhotoshopShortcutsFn, PlaceInbox, QuitFn, RecoverFn, Recoverable,
|
||||
RightButtonDownFn, SameFileFn, SaveFileFn, SaveTextFn, Services, SystemThemeFn, TabletPollFn, WindowFrameFn, WriteFn,
|
||||
};
|
||||
pub(crate) use services::{opaque_layer, saves_flat_in_place};
|
||||
pub use text::bidi_text;
|
||||
pub use text::cjk_fonts;
|
||||
pub use text::direction;
|
||||
|
||||
@@ -239,6 +239,8 @@ fn invoke_shell_commands(app: &mut PhotocraftApp, ctx: &egui::Context, id: &str,
|
||||
None
|
||||
}
|
||||
|
||||
/// The commands the shell answers itself before the engine (File › Open and Save with the
|
||||
/// dialogs they ask, loading a path as a selection, …); `Ok(None)` for any other.
|
||||
fn invoke_file_commands(app: &mut PhotocraftApp, ctx: &egui::Context, id: &str, params: &Value) -> Result<Option<Value>, String> {
|
||||
let result = match id {
|
||||
// ⌘↩ / Ctrl+Enter (#306): load the path selected in the Paths panel (or the one being
|
||||
@@ -320,7 +322,19 @@ fn invoke_file_commands(app: &mut PhotocraftApp, ctx: &egui::Context, id: &str,
|
||||
crate::notices::post(app, title, lines, false, None);
|
||||
app.save_as_with(None, choices)
|
||||
}
|
||||
// A flat file asks for its name; confirming its own file records a
|
||||
// Its own flat file, while it still keeps the whole document (one plain
|
||||
// layer it can store): written back, as Photoshop's Save does (#5733),
|
||||
// asking first if another app changed it (#6596), then JPEG Options for a
|
||||
// JPEG (#2223), or TIFF, Targa or DDS Options for those. Cancel in either writes
|
||||
// nothing.
|
||||
Some((false, path, d)) if crate::saves_flat_in_place(&d.doc, &path) => {
|
||||
let doc = d.doc.id;
|
||||
if app.save_options_open() {
|
||||
return Err(crate::SAVE_OPTIONS_OPEN.into());
|
||||
}
|
||||
crate::disk_change_prompt::intercept(app, doc, &path, false, choices).unwrap_or_else(|| app.save_flat_in_place(path, choices))
|
||||
}
|
||||
// Any other flat file asks for its name; confirming its own file records a
|
||||
// plain Save that writes the file an action is played on (#3531).
|
||||
_ => app.save_dialog_flow(None, choices, true),
|
||||
}
|
||||
|
||||
@@ -209,8 +209,53 @@ impl ExportSettings {
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether File › Save writes straight back to `path`, the flat file (JPEG, PNG, TIFF…) the
|
||||
/// document came from, as Photoshop's Save does (#5733): only while that file keeps everything,
|
||||
/// i.e. the document is one plain raster layer ([`plain_raster`]) with no extra channels, in RGB
|
||||
/// or Grayscale at a depth (and, for Grayscale, a gray layout) the format stores, and either the
|
||||
/// format keeps transparency or the layer is opaque (the Background, or a layer made from it with
|
||||
/// no pixels deleted, as checked in Photoshop, #2223). The file itself must have held only
|
||||
/// this one image (not an animation, a multi-page TIFF or a multi-size icon), and its encoder
|
||||
/// must not quantise (GIF's palette and on/off transparency); a JPEG is rewritten at its own
|
||||
/// quality (#4032). An embedded colour profile needs a format that keeps one. Anything else still
|
||||
/// goes through Save As, so the original is never overwritten with less than the document has.
|
||||
pub(crate) fn saves_flat_in_place(doc: &Document, path: &str) -> bool {
|
||||
let Some(layer) = plain_raster(doc) else { return false };
|
||||
let Some(format) = photocraft_engine::file_cmds::extension(path).and_then(|e| photocraft_codecs::from_extension(&e)) else {
|
||||
return false;
|
||||
};
|
||||
let caps = format.caps();
|
||||
// Transparency is kept where the format's plan writes alpha for this document: DPX has no
|
||||
// gray + alpha, and float PNM (PFM) no alpha at all.
|
||||
let keeps_alpha = caps.alpha
|
||||
&& !(format == photocraft_codecs::Format::Dpx && doc.mode == photocraft_doc::ColorMode::Grayscale)
|
||||
&& !(format == photocraft_codecs::Format::Pnm && doc.depth == photocraft_color::SampleType::F32);
|
||||
!photocraft_engine::file_cmds::saves_in_place(path)
|
||||
&& doc.file_encoding.source_images.is_none()
|
||||
&& (!caps.lossy || format == photocraft_codecs::Format::Jpeg)
|
||||
&& (doc.mode != photocraft_doc::ColorMode::Grayscale || caps.layouts.iter().any(|l| l.is_gray()))
|
||||
&& caps.write
|
||||
&& caps.depths.contains(&match doc.depth {
|
||||
photocraft_color::SampleType::U8 => photocraft_codecs::SampleType::U8,
|
||||
photocraft_color::SampleType::U16 => photocraft_codecs::SampleType::U16,
|
||||
photocraft_color::SampleType::F32 => photocraft_codecs::SampleType::F32,
|
||||
})
|
||||
&& (doc.icc_profile.is_none() || caps.icc)
|
||||
&& doc.channels.is_empty()
|
||||
&& matches!(doc.mode, photocraft_doc::ColorMode::Rgb | photocraft_doc::ColorMode::Grayscale)
|
||||
// Last: it may read every tile.
|
||||
&& (keeps_alpha || doc.is_background(layer.id) || opaque_layer(doc, layer))
|
||||
}
|
||||
|
||||
/// Whether `layer` is fully opaque over the canvas, so a format without transparency (JPEG)
|
||||
/// keeps all of it. Read from its tiles, stopping at the first transparent pixel (#2223).
|
||||
pub(crate) fn opaque_layer(doc: &Document, layer: &photocraft_doc::Layer) -> bool {
|
||||
layer.surface().is_some_and(|s| !s.format().alpha || photocraft_io::opaque_surface(s, doc.bounds()))
|
||||
}
|
||||
|
||||
/// 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).
|
||||
/// opacity, in Normal mode, with no masks, effects, clipping, advanced blending, blending
|
||||
/// restrictions or video frames (#2234).
|
||||
pub(crate) fn plain_raster(doc: &Document) -> Option<&photocraft_doc::Layer> {
|
||||
match doc.layers.as_slice() {
|
||||
[layer]
|
||||
@@ -224,7 +269,10 @@ pub(crate) fn plain_raster(doc: &Document) -> Option<&photocraft_doc::Layer> {
|
||||
&& 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) =>
|
||||
&& !photocraft_compose::blend_if_active(layer, doc.mode)
|
||||
&& !layer.clipped
|
||||
&& layer.advanced.is_default()
|
||||
&& layer.video.is_none() =>
|
||||
{
|
||||
Some(layer)
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user