From 86245d0719c374b39cd982bedcff2185aa7e7e2c Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:29:59 +0100 Subject: [PATCH] Encode .pcraft previews while the bundle is prepared and compressed A ZIP save encoded the thumbnail and the 1024 px composite preview as PNG on the calling thread before any tile was compressed. The composite alone is a ~200 ms single-threaded deflate on a 36 MP document, about 40 % of a full save. The previews are now encoded on their own threads while the document is collected and its new objects are compressed on all cores; the bundle is assembled afterwards in the same order, so the bytes are unchanged. Directory saves encode them up front as before. wasm stays sequential. bench_export, 36 MP, 3 layers, 16 interleaved rounds of 7 reps (median of medians / min), Ryzen 16 threads, Linux: .pcraft 517.9 / 471.1 ms -> 319.3 / 287.8 ms (-38 %) .pcraft with cache 360.1 / 334.1 ms -> 277.5 / 268.3 ms (-23 %) Co-Authored-By: Claude Opus 5.5 --- crates/format/src/store.rs | 110 +++++++++++++++++++++++++++++++------ 1 file changed, 94 insertions(+), 16 deletions(-) diff --git a/crates/format/src/store.rs b/crates/format/src/store.rs index fb73819e32..e9c1bf1022 100644 --- a/crates/format/src/store.rs +++ b/crates/format/src/store.rs @@ -64,6 +64,34 @@ fn png(img: &Rgba8Image) -> Result> { photocraft_codecs::encode(&image, photocraft_codecs::Format::Png, &Default::default()).map_err(|e| FormatError::Unsupported(format!("preview: {e}"))) } +fn previews(opts: &SaveOptions) -> Result)>> { + let (composite, thumbnail) = beside( + opts.thumbnail.is_some() && opts.composite.is_some(), + || opts.composite.as_ref().map(png).transpose(), + || opts.thumbnail.as_ref().map(png).transpose(), + ); + let mut out = Vec::new(); + if let Some(t) = thumbnail? { + out.push((THUMB, t)); + } + if let Some(t) = composite? { + out.push((COMPOSITE, t)); + } + Ok(out) +} + +fn beside(parallel: bool, a: impl FnOnce() -> Result + Send, b: impl FnOnce() -> B) -> (Result, B) { + if !parallel || cfg!(target_arch = "wasm32") { + let a = a(); + return (a, b()); + } + std::thread::scope(|sc| { + let h = sc.spawn(a); + let b = b(); + (h.join().unwrap_or_else(|_| Err(FormatError::Io(std::io::Error::other("preview worker panicked")))), b) + }) +} + // --------------------------------------------------------------------------- // Sources // --------------------------------------------------------------------------- @@ -481,7 +509,6 @@ struct FileSignature { struct Prepared { manifest: Vec, objects: BTreeMap, - previews: Vec<(&'static str, Vec)>, stats: SaveStats, } @@ -882,13 +909,6 @@ impl PcraftWriter { convert::check_nesting(&doc.layers)?; let mut c = Collect { hash_cache: Some(&mut self.hash_cache), blob_hash_cache: Some(&mut self.blob_hash_cache), ..Default::default() }; let document = convert::doc_m(doc, &mut c); - let mut previews = Vec::new(); - if let Some(t) = &opts.thumbnail { - previews.push((THUMB, png(t)?)); - } - if let Some(t) = &opts.composite { - previews.push((COMPOSITE, png(t)?)); - } let manifest = Manifest { format_version: FORMAT_VERSION, generator: format!("photocraft-format {}", env!("CARGO_PKG_VERSION")), @@ -912,23 +932,33 @@ impl PcraftWriter { for (h, b) in c.blobs { objects.insert(blob_path(&h), Object::Blob(b)); } - Ok(Prepared { manifest, objects, previews, stats }) + Ok(Prepared { manifest, objects, stats }) } /// Save as a ZIP bundle in memory. pub fn save_zip(&mut self, doc: &Document, opts: &SaveOptions) -> Result<(Vec, SaveStats)> { - let p = self.prepare(doc, opts)?; + // Another thread encodes the previews while the document is collected and its new + // objects are compressed on all cores (zstd dominates a full save). + let (previews, prepared) = beside( + opts.thumbnail.is_some() || opts.composite.is_some(), + || previews(opts), + || -> Result<_> { + let p = self.prepare(doc, opts)?; + let todo: Vec<(&String, &Object)> = p.objects.iter().filter(|(path, _)| !self.compressed.contains_key(*path)).collect(); + let fresh: HashMap>> = par_compress(&todo)?.into_iter().map(|(path, data)| (path.clone(), data)).collect(); + Ok((p, fresh)) + }, + ); + let (p, mut fresh) = prepared?; + let previews = previews?; let mut stats = p.stats; let mut z = ZipWriter::new(); z.add(MANIFEST, &p.manifest)?; - for (name, data) in &p.previews { + for (name, data) in &previews { z.add(name, data)?; } let mut next = HashMap::with_capacity(p.objects.len()); let mut cached_bytes = 0usize; - // New objects are compressed on all cores first (zstd dominates a full save). - let todo: Vec<(&String, &Object)> = p.objects.iter().filter(|(path, _)| !self.compressed.contains_key(*path)).collect(); - let mut fresh: HashMap<&String, Arc>> = par_compress(&todo)?.into_iter().collect(); for (path, obj) in &p.objects { let (key, data) = match self.compressed.remove_entry(path) { Some((key, data)) => { @@ -976,6 +1006,7 @@ impl PcraftWriter { validate_bundle_file(dir, &dir.join(path))?; } let p = self.prepare(doc, opts)?; + let previews = previews(opts)?; let mut stats = p.stats; if self.verified_directory.as_ref().is_none_or(|cache| cache.directory != canonical_dir) { self.verified_directory = Some(DirectoryVerificationCache { directory: canonical_dir.clone(), ..Default::default() }); @@ -1035,7 +1066,7 @@ impl PcraftWriter { } } } - for (name, data) in &p.previews { + for (name, data) in &previews { write_bundle_file(dir, &dir.join(name), data, fs)?; } // Pin all GC directories before removing stale previews or objects. @@ -1062,7 +1093,7 @@ impl PcraftWriter { // Omitted previews belong to the previous manifest until the new one commits. // After commit, a locked leftover is only housekeeping, just like unused objects. for stale in [THUMB, COMPOSITE] { - if !p.previews.iter().any(|(n, _)| *n == stale) { + if !previews.iter().any(|(n, _)| *n == stale) { let _ = remove_bundle_file(dir, &dir.join(stale), &gc); } } @@ -1190,6 +1221,53 @@ fn compressed_cache_budget_limits_retained_capacity_without_changing_bytes() { assert!(writer.compressed.is_empty()); } +#[cfg(test)] +#[test] +fn previews_follow_the_manifest_with_the_bytes_of_a_serial_encode() { + use photocraft_doc::{Layer, LayerContent}; + use photocraft_geom::Size; + use photocraft_raster::Surface; + + let mut doc = Document::new("previews", Size::new(600, 3), photocraft_color::ColorMode::Rgb, SampleType::U8); + let mut surface = Surface::new(doc.pixel_format()); + for x in 0..600 { + surface.write_pixel(x, 1, &[x as f32 / 600.0, 0.5, 0.25, 1.0]); + } + doc.layers.push(Layer::new("pixels", LayerContent::Raster(surface))); + let image = |w: u32, h: u32, k: u8| { + let mut i = Rgba8Image::new(w, h); + i.pixels.iter_mut().enumerate().for_each(|(j, p)| *p = (j as u8).wrapping_mul(k)); + i + }; + let (thumb, composite) = (image(4, 3, 7), image(9, 5, 13)); + for (t, c) in [(true, true), (true, false), (false, true), (false, false)] { + let opts = SaveOptions { thumbnail: t.then(|| thumb.clone()), composite: c.then(|| composite.clone()) }; + let (bytes, _) = PcraftWriter::new().save_zip(&doc, &opts).unwrap(); + let zip = ZipReader::new(&bytes).unwrap(); + let names: Vec<&str> = zip.entries.iter().map(|e| e.name.as_str()).collect(); + let mut want = vec![MANIFEST]; + want.extend(t.then_some(THUMB)); + want.extend(c.then_some(COMPOSITE)); + assert_eq!(names[..want.len()], want[..]); + assert!(names[want.len()..].iter().all(|n| n.starts_with("tiles/") || n.starts_with("blobs/")), "{names:?}"); + for (name, img) in [(THUMB, opts.thumbnail.as_ref()), (COMPOSITE, opts.composite.as_ref())] { + assert_eq!(zip.read_by_name(name, usize::MAX).ok(), img.map(|i| png(i).unwrap()), "{name}"); + } + } + let bad = SaveOptions { thumbnail: Some(Rgba8Image { width: 4, height: 3, pixels: vec![0; 5] }), composite: Some(composite.clone()) }; + assert!(matches!(PcraftWriter::new().save_zip(&doc, &bad), Err(FormatError::Unsupported(m)) if m.starts_with("preview"))); + let bad = SaveOptions { thumbnail: Some(thumb.clone()), composite: Some(Rgba8Image { width: 9, height: 5, pixels: vec![0; 5] }) }; + assert!(matches!(PcraftWriter::new().save_zip(&doc, &bad), Err(FormatError::Unsupported(m)) if m.starts_with("preview"))); + + let dir = std::env::temp_dir().join(format!("pcraft-previews-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let opts = SaveOptions { thumbnail: Some(thumb.clone()), composite: Some(composite.clone()) }; + PcraftWriter::new().save_dir(&doc, &dir, &opts).unwrap(); + assert_eq!(std::fs::read(dir.join(THUMB)).unwrap(), png(&thumb).unwrap()); + assert_eq!(std::fs::read(dir.join(COMPOSITE)).unwrap(), png(&composite).unwrap()); + std::fs::remove_dir_all(&dir).unwrap(); +} + #[cfg(all(test, unix))] #[test] fn gc_pinned_directory_handles_resist_subdirectory_swap() {