From 86ddfb72f766be906109bb935c7b17ab74814ea5 Mon Sep 17 00:00:00 2001 From: Khawar Zamman Wani Date: Mon, 2 Nov 2026 13:59:59 +0530 Subject: [PATCH 1/4] Reuse large buffers so returning freed memory to the OS doesn't slow filters (#10479) (#10514) * perf: reuse large buffers so a pinned glibc trim threshold costs no speed (#10479) With MALLOC_TRIM_THRESHOLD_ set (#8743), glibc maps every allocation over 128 KiB fresh, so tiles, compositor bands and filter scratch were page-faulted in on every call: viewport compose 3.6x slower, Gaussian Blur up to 1.8x, .pcraft save 2.4x. - photocraft_raster::scratch: a process-wide, capped pool of large Vec buffers (u8/u16/f32/[f32; 4]). A miss drops the oldest buffers worth its size down to a 256 MiB floor, so peak memory stays near glibc's defaults. The engine releases the pool after any command that closes a document, Headless::close does too, and an idle thread releases it after 2 s unused. - Tiles, compose Buffer/masks/bands, algo Image and the blur paths, engine adjustment/filter scratch, GPU tile upload and io import buffers go through the pool. - .pcraft save reuses one zstd FrameCompressor per worker (byte-identical). - interactive_bench reports minor page faults per run. 36 MP, trim threshold set vs glibc defaults: viewport 5.6 vs 6.1 ms (was 22.7), Blur r4 445 vs 437 ms (was 852), Curves CPU proxy 35.0 vs 35.7 ms (was 50), flatten 146 vs 147 ms (was 237), save .pcraft 251 vs 230 ms (was 545). RSS after closing a 5400x7200 document: 21 MB with trim set. * fix: corpus tests build again; return remaining pooled buffers (#10479 review) - Corpus-gated io tests and the Photoshop-oracle test moved `.px` out of a `Buffer`, which now implements `Drop` (E0509 with `--features corpus`): use `into_px()`. - io: masks, the merged image, extra channels, smart-filter masks, smart-object pixels and patterns give their `interleave` buffers back to the scratch pool after writing them. - compose: recycle the mask values of a pass-through group, `effect_shape` and both `split_parts` callers. - Styled Free Transform preview copies each band out instead of moving it, so the band returns to the pool. - interactive_bench: the fault count covers each whole run, untimed setup included; say so. * fix: recycle the masks of adjustment-atop and shape layers (#10479 review) * fix: recycle effect hiding masks (#10479 review) * fix: recycle knockout, CMYK and GPU shape masks; release the pool when Split/Merge Channels close a document (#10479 review) * fix: release the pool when a closed document's cancelled workers exit; idle-release switch for tests; don't pool PSD planes (#10479 review) * fix: release the pool for workers cancelled before their document closes; take the pool lock for budget and idle-release changes (#10479 review) * fix: idle release rechecks activity under the lock; try_take_exact releases the pool and retries before failing (#10479 review) * test: cover a successful try_take_exact miss; name the failed-reservation test for what it checks (#10479 review) * fix: pool the final GPU effect shape buffer and return it after use (#10479 review) * docs: describe scratch pool lifecycle regression invariants * test: verify polling releases cancelled document scratch buffers * perf: recycle remaining transform selection and PSD scratch buffers * fix: free PSD gather bands before allocating channel planes * test: assert exact scratch take reuses pooled storage --- crates/algo/src/blur.rs | 47 ++- crates/algo/src/blur/tests.rs | 5 +- crates/algo/src/image.rs | 29 +- crates/algo/src/lib.rs | 6 +- crates/algo/src/sharpen/tests.rs | 7 +- crates/algo/src/transform.rs | 1 + crates/automation/src/headless.rs | 2 + crates/compose/src/advanced.rs | 15 +- crates/compose/src/advanced_tests.rs | 8 +- crates/compose/src/cmyk.rs | 4 + crates/compose/src/composite.rs | 36 +- crates/compose/src/content.rs | 63 +-- crates/compose/src/extents.rs | 12 +- crates/compose/src/lib.rs | 27 +- crates/compose/src/render.rs | 5 +- crates/compose/src/soft_shape_tests.rs | 2 +- crates/compose/src/tests.rs | 20 +- crates/compose/tests/buffer_reuse.rs | 27 ++ crates/engine/examples/indexed_color_perf.rs | 2 +- crates/engine/src/cmd/adjust_cmds.rs | 6 +- crates/engine/src/cmd/adjust_cmds/tests.rs | 2 +- crates/engine/src/cmd/face_cmds.rs | 2 +- crates/engine/src/cmd/layer_menu_cmds.rs | 2 +- .../engine/src/cmd/layer_menu_cmds/tests.rs | 8 +- crates/engine/src/cmd/mask_view_cmds/tests.rs | 2 +- crates/engine/src/cmd/mode_cmds.rs | 2 +- crates/engine/src/cmd/mode_cmds/tests.rs | 4 +- crates/engine/src/cmd/select_extra_cmds.rs | 2 +- crates/engine/src/cmd/selection_cmds.rs | 2 +- crates/engine/src/cmd/smart_cmds/rendering.rs | 2 +- crates/engine/src/cmd/smart_cmds/tests.rs | 2 +- crates/engine/src/cmd/smartselect_cmds.rs | 2 +- crates/engine/src/cmd/transform_cmds/tests.rs | 2 +- crates/engine/src/cmd/video_cmds/tests.rs | 2 +- crates/engine/src/color_channels/tone.rs | 2 +- crates/engine/src/doc_path/tests.rs | 6 +- crates/engine/src/filters.rs | 2 + crates/engine/src/jobs.rs | 59 ++- crates/engine/src/pixels.rs | 13 +- crates/engine/src/select_mask_edits.rs | 4 +- crates/engine/src/tests.rs | 4 +- .../tests/it/atn_crystallize_cell_size.rs | 2 +- crates/engine/tests/it/atn_ripple_amount.rs | 2 +- crates/engine/tests/it/atn_selective_color.rs | 2 +- crates/engine/tests/it/atn_stroke_width.rs | 2 +- crates/engine/tests/it/photoshop_oracles.rs | 2 +- crates/engine/tests/scratch_release.rs | 114 ++++++ crates/format/src/store.rs | 133 +++++- crates/gpu/src/convert.rs | 14 +- crates/gpu/src/fx.rs | 12 +- crates/gpu/src/fx_passes.rs | 2 + crates/gpu/tests/it/parity.rs | 2 +- crates/io/examples/oracle_diff.rs | 4 +- crates/io/src/flat.rs | 9 +- crates/io/src/lib.rs | 4 +- crates/io/src/ora.rs | 4 +- crates/io/src/pattern_map.rs | 4 +- crates/io/src/pixels.rs | 7 +- crates/io/src/psd_export.rs | 2 + crates/io/src/psd_import.rs | 8 + crates/io/src/smart_map.rs | 2 + crates/io/tests/it/banded.rs | 2 +- crates/io/tests/it/blend_modes_corpus.rs | 2 +- crates/io/tests/it/cmyk_composite_export.rs | 3 +- crates/io/tests/it/color.rs | 8 +- crates/io/tests/it/composite.rs | 6 +- crates/io/tests/it/corpus.rs | 4 +- crates/io/tests/it/effects_multi.rs | 2 +- crates/io/tests/it/flat.rs | 2 +- crates/io/tests/it/lab16.rs | 4 +- crates/io/tests/it/large_files.rs | 2 +- crates/io/tests/it/multichannel.rs | 4 +- crates/io/tests/it/psd32_export.rs | 4 +- crates/io/tests/it/psd_damaged_merged.rs | 2 +- crates/io/tests/it/psd_type_bounds.rs | 2 +- crates/io/tests/it/tiff_corpus.rs | 4 +- crates/io/tests/it/tiff_layers.rs | 4 +- crates/raster/src/lib.rs | 38 +- crates/raster/src/scratch.rs | 384 ++++++++++++++++++ crates/raster/tests/scratch.rs | 361 ++++++++++++++++ crates/testkit/src/perf.rs | 21 + crates/ui-egui/examples/interactive_bench.rs | 19 +- crates/ui-egui/src/dialogs/color_range_ui.rs | 2 +- crates/ui-egui/src/dialogs/file_ui/web.rs | 2 +- .../src/dialogs/filter_dialog/tests.rs | 2 +- crates/ui-egui/src/dialogs/vp_ui/tests.rs | 8 +- crates/ui-egui/src/tools/transform_fx.rs | 3 +- crates/ui-egui/tests/it/adjust_preview_gpu.rs | 6 +- 88 files changed, 1444 insertions(+), 238 deletions(-) create mode 100644 crates/compose/tests/buffer_reuse.rs create mode 100644 crates/engine/tests/scratch_release.rs create mode 100644 crates/raster/src/scratch.rs create mode 100644 crates/raster/tests/scratch.rs diff --git a/crates/algo/src/blur.rs b/crates/algo/src/blur.rs index 819bf6fa78..7ca9d34f99 100644 --- a/crates/algo/src/blur.rs +++ b/crates/algo/src/blur.rs @@ -1,7 +1,7 @@ //! Blurs: Gaussian, box, motion, radial, surface. use photocraft_geom::Rect; -use photocraft_raster::Interrupt; +use photocraft_raster::{Interrupt, scratch}; use crate::image::{Edge, Image, premultiply, unpremultiply}; use crate::photo_util::{par_map, par_rows}; @@ -78,7 +78,7 @@ pub(crate) fn conv_sep(src: &Image, out: Rect, kx: &[f32], ky: &[f32], alpha: bo // Premultiplied copy of the needed source window. let win = Rect::new(out.x0 - rx, out.y0 - ry, out.x1 + rx, out.y1 + ry); let ww = win.width() as usize; - let mut p = vec![0.0f32; ww * win.height() as usize * n]; + let mut p = scratch::Buf::new(ww * win.height() as usize * n, 0.0f32); for y in win.y0..win.y1 { for x in win.x0..win.x1 { let o = ((y - win.y0) as usize * ww + (x - win.x0) as usize) * n; @@ -89,7 +89,7 @@ pub(crate) fn conv_sep(src: &Image, out: Rect, kx: &[f32], ky: &[f32], alpha: bo } premultiply(&mut p, n, alpha); // Horizontal pass: (ow × th). - let mut tmp = vec![0.0f32; ow * th * n]; + let mut tmp = scratch::Buf::new(ow * th * n, 0.0f32); for ty in 0..th { let row = &p[ty * ww * n..(ty + 1) * ww * n]; let dst = &mut tmp[ty * ow * n..(ty + 1) * ow * n]; @@ -113,7 +113,7 @@ pub(crate) fn conv_sep(src: &Image, out: Rect, kx: &[f32], ky: &[f32], alpha: bo } } // Vertical pass. - let mut res = vec![0.0f32; ow * oh * n]; + let mut res = scratch::take(ow * oh * n, 0.0f32); for oy in 0..oh { let d = &mut res[oy * ow * n..(oy + 1) * ow * n]; for (i, kv) in ky.iter().enumerate() { @@ -182,11 +182,27 @@ pub(crate) fn box_widths(params: &FilterParams) -> Option> { } /// Scratch buffers of one worker for [`box_blur`]. -#[derive(Default)] struct Scratch { line: Vec, next: Vec, acc: Vec, + /// The source row being read (horizontal passes only). + row: Vec, +} + +impl Scratch { + /// Buffers with room for lines of `line` floats (and the shorter ones after the first pass). + fn new(line: usize, row: usize) -> Self { + Scratch { line: scratch::take_cap(line), next: scratch::take_cap(line), acc: Vec::new(), row: scratch::take_cap(row) } + } +} + +impl Drop for Scratch { + fn drop(&mut self) { + scratch::give(std::mem::take(&mut self.line)); + scratch::give(std::mem::take(&mut self.next)); + scratch::give(std::mem::take(&mut self.row)); + } } /// One running-sum box pass of radius `r` along a line of elements of `w` floats each (a pixel's @@ -252,7 +268,7 @@ fn box_pass_px(src: &[f32], dst: &mut [f32], r: usize) { /// `dst` unfinished) when `ctl` is cancelled before a pass: a column strip's passes take tens of /// milliseconds on a tall image, too long to finish once cancelled (#6205). fn line_passes(dst: &mut [f32], w: usize, radii: &[usize], s: &mut Scratch, ctl: &Interrupt) { - let Scratch { line, next, acc } = s; + let Scratch { line, next, acc, .. } = s; let Some(len) = dst.len().checked_div(w) else { return }; let mut tail: usize = radii.iter().sum(); for (k, &r) in radii.iter().enumerate() { @@ -306,21 +322,21 @@ pub(crate) fn box_blur( // Horizontal passes over `used`'s rows only (the repeated rows above and below it would just // repeat their results), for the output columns. Each row is extended to the passes' reach by // repeating its end pixels. - let mut h = vec![0.0f32; used.height() as usize * ow * n]; + let mut h = scratch::Buf::new(used.height() as usize * ow * n, 0.0f32); let (left, right) = ((used.x0 - out.x0.saturating_sub(reach)) as usize, (out.x1.saturating_add(reach) - used.x1) as usize); par_rows(&mut h, ow * ROWS_PER_TASK, n, |task, chunk| { - let (mut row, mut s) = (Vec::new(), Scratch::default()); + let mut s = Scratch::new((used.width() as usize + left + right) * n, used.width() as usize * n); for (k, dst) in chunk.chunks_exact_mut(ow * n).enumerate() { if ctl.cancelled() { return; } let y = used.y0 + (task * ROWS_PER_TASK + k) as i32; - read(Rect::new(used.x0, y, used.x1, y + 1), &mut row); - premultiply(&mut row, n, alpha); - let (Some(first), Some(last)) = (row.get(..n), row.get(row.len().saturating_sub(n)..)) else { return }; + read(Rect::new(used.x0, y, used.x1, y + 1), &mut s.row); + premultiply(&mut s.row, n, alpha); + let (Some(first), Some(last)) = (s.row.get(..n), s.row.get(s.row.len().saturating_sub(n)..)) else { return }; s.line.clear(); (0..left).for_each(|_| s.line.extend_from_slice(first)); - s.line.extend_from_slice(&row); + s.line.extend_from_slice(&s.row); (0..right).for_each(|_| s.line.extend_from_slice(last)); line_passes(dst, n, &radii, &mut s, ctl); } @@ -336,11 +352,11 @@ pub(crate) fn box_blur( } let (sx, w) = ((t.x0 - out.x0) as usize * n, t.width() as usize * n); let row = |y: usize| &h[y * stride + sx..y * stride + sx + w]; - let mut s = Scratch::default(); + let mut s = Scratch::new((rows + top + bottom) * w, 0); (0..top).for_each(|_| s.line.extend_from_slice(row(0))); (0..rows).for_each(|y| s.line.extend_from_slice(row(y))); (0..bottom).for_each(|_| s.line.extend_from_slice(row(rows - 1))); - let mut res = vec![0.0f32; oh * w]; + let mut res = scratch::take(oh * w, 0.0f32); line_passes(&mut res, w, &radii, &mut s, ctl); if ctl.cancelled() { return (t, Vec::new()); @@ -367,7 +383,7 @@ fn box_passes(src: &Image, out: Rect, alpha: bool, boxes: &[usize]) -> Vec buf.extend_from_slice(src.row(r)); }; let ow = out.width() as usize * n; - let mut res = vec![0.0f32; ow * out.height() as usize]; + let mut res = scratch::take(ow * out.height() as usize, 0.0f32); if res.is_empty() { return res; } @@ -376,6 +392,7 @@ fn box_passes(src: &Image, out: Rect, alpha: bool, boxes: &[usize]) -> Vec for (dst, s) in res.chunks_exact_mut(ow).zip(data.chunks_exact(w)) { dst[x0..x0 + w].copy_from_slice(s); } + scratch::give(data); } res } diff --git a/crates/algo/src/blur/tests.rs b/crates/algo/src/blur/tests.rs index 028ac65dcc..74cf7ab20c 100644 --- a/crates/algo/src/blur/tests.rs +++ b/crates/algo/src/blur/tests.rs @@ -204,7 +204,7 @@ fn direct_boxes(img: &Image, out: Rect, boxes: &[usize], alpha: bool) -> Vec = (0..(len + 2 * reach) * w).map(|i| (i % 7) as f32 + 1.0).collect(); let run = |ctl: &Interrupt| { - let mut s = Scratch { line: line.clone(), ..Scratch::default() }; + let mut s = Scratch::new(0, 0); + s.line.extend_from_slice(&line); let mut dst = vec![0.0f32; len * w]; line_passes(&mut dst, w, &radii, &mut s, ctl); dst diff --git a/crates/algo/src/image.rs b/crates/algo/src/image.rs index 6a17e08b01..c521f04348 100644 --- a/crates/algo/src/image.rs +++ b/crates/algo/src/image.rs @@ -1,7 +1,7 @@ //! Interleaved `f32` image regions and sampling helpers. use photocraft_geom::Rect; -use photocraft_raster::Surface; +use photocraft_raster::{Surface, scratch}; /// How samples outside an image are read. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -14,8 +14,9 @@ pub enum Edge { Wrap, } -/// A rectangle of interleaved, normalized samples. -#[derive(Clone, Debug, PartialEq)] +/// A rectangle of interleaved, normalized samples. The samples come from, and go back to, the +/// scratch pool. +#[derive(Debug, PartialEq)] pub struct Image { pub rect: Rect, /// Channels per pixel (alpha last when the surface has alpha). @@ -23,6 +24,20 @@ pub struct Image { pub data: Vec, } +impl Clone for Image { + fn clone(&self) -> Self { + let mut data = scratch::take(self.data.len(), 0.0f32); + data.copy_from_slice(&self.data); + Image { rect: self.rect, ch: self.ch, data } + } +} + +impl Drop for Image { + fn drop(&mut self) { + scratch::give(std::mem::take(&mut self.data)); + } +} + impl Image { /// Reads `rect` from a surface. pub fn read(s: &Surface, rect: Rect) -> Self { @@ -53,9 +68,15 @@ impl Image { img } + /// The samples, taken out of the image (it can't be destructured: it returns them to the + /// pool on drop). + pub fn into_data(mut self) -> Vec { + std::mem::take(&mut self.data) + } + /// New zeroed image. pub fn new(rect: Rect, ch: usize) -> Self { - Image { rect, ch, data: vec![0.0; rect.width() as usize * rect.height() as usize * ch] } + Image { rect, ch, data: scratch::take(rect.width() as usize * rect.height() as usize * ch, 0.0) } } #[inline] diff --git a/crates/algo/src/lib.rs b/crates/algo/src/lib.rs index 03ffe38144..bda29cb2ea 100644 --- a/crates/algo/src/lib.rs +++ b/crates/algo/src/lib.rs @@ -1269,7 +1269,10 @@ fn apply_tiled_with_impl( } for (t, data) in results { match data { - TileData::Samples(data) => out.write_region(t, &data), + TileData::Samples(data) => { + out.write_region(t, &data); + photocraft_raster::scratch::give(data); + } TileData::Encoded(data) => out.write_interleaved(t, &data), } } @@ -1324,6 +1327,7 @@ fn apply_box_blur( mix_selection(data, *t, sel, &Image::read(surface, *t), fmt.alpha); } s.write_region(*t, data); + photocraft_raster::scratch::give(std::mem::take(data)); } }; #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/algo/src/sharpen/tests.rs b/crates/algo/src/sharpen/tests.rs index 4699bb3a10..aef62259a3 100644 --- a/crates/algo/src/sharpen/tests.rs +++ b/crates/algo/src/sharpen/tests.rs @@ -267,7 +267,12 @@ fn malformed_inputs_and_allocations_stop_gracefully() { assert!(crate::apply_in_with(&s, &p, rect, rect, None, rect, &Interrupt::NONE).is_none()); assert_eq!(crate::apply_in(&s, &p, rect, rect, None, rect), s); } - for bad in [Image { ch: 0, ..src.clone() }, Image { data: vec![0.0], ..src.clone() }, Image { rect: Rect::new(5, 0, 0, 2), ..src.clone() }] { + let with = |edit: &dyn Fn(&mut Image)| { + let mut bad = src.clone(); + edit(&mut bad); + bad + }; + for bad in [with(&|i| i.ch = 0), with(&|i| i.data = vec![0.0]), with(&|i| i.rect = Rect::new(5, 0, 0, 2))] { assert!(exact::run(&bad, rect, &ctx, 100.0, 1.0, 0.0, &Interrupt::NONE).is_none()); } for rect in [Rect::new(i32::MIN, 0, i32::MAX, 2), Rect::new(0, 0, 300_000, 300_000), Rect::new(2, 0, 0, 2)] { diff --git a/crates/algo/src/transform.rs b/crates/algo/src/transform.rs index 1ed0ec7c04..b1f7bacaae 100644 --- a/crates/algo/src/transform.rs +++ b/crates/algo/src/transform.rs @@ -521,6 +521,7 @@ fn warp( outp[o + a] = al as f32; } } + photocraft_raster::scratch::give(px); Ok(any.then_some((*t, outp))) }; // Each tile is written as soon as it is warped, so only the tiles in flight hold their `f32` diff --git a/crates/automation/src/headless.rs b/crates/automation/src/headless.rs index 6722c5c00b..8b08daa6aa 100644 --- a/crates/automation/src/headless.rs +++ b/crates/automation/src/headless.rs @@ -358,6 +358,8 @@ impl Headless { if let Some(d) = self.session.close(i) { self.writers.remove(&d.doc.id.0); } + // The closed document's buffers went to the scratch pool: hand them back to the OS. + photocraft_raster::scratch::release(); Ok(self.session_list()) } diff --git a/crates/compose/src/advanced.rs b/crates/compose/src/advanced.rs index 8ea5a86c6d..7a4068012d 100644 --- a/crates/compose/src/advanced.rs +++ b/crates/compose/src/advanced.rs @@ -32,6 +32,7 @@ use std::borrow::Cow; use photocraft_doc::{Knockout, Layer, LayerContent}; use photocraft_geom::Rect; +use crate::extents::recycle_mask; use crate::{Buffer, Ctx, composite_atop, composite_layer, effects, mask_vals, render_content}; /// Where a layer stack sits, for knockouts: at the document's top level, and/or inside an @@ -157,6 +158,7 @@ pub(crate) fn knockout(layer: &Layer, backdrop: &mut Buffer, target: &Buffer, cl if target.rect != backdrop.rect || shape.len() != backdrop.px.len() { // Never reached (targets are built over the backdrop's rect); compositing without the // knockout beats a wrong pixel or a panic. + recycle_mask(Some(shape)); draw(backdrop); return; } @@ -172,7 +174,10 @@ pub(crate) fn knockout(layer: &Layer, backdrop: &mut Buffer, target: &Buffer, cl } let knocked = (layer.opacity < 1.0).then(|| backdrop.clone()); draw(backdrop); - let Some(knocked) = knocked else { return }; + let Some(knocked) = knocked else { + recycle_mask(Some(shape)); + return; + }; let rest = 1.0 - layer.opacity.clamp(0.0, 1.0); for (((p, b), k), s) in backdrop.px.iter_mut().zip(&before.px).zip(&knocked.px).zip(&shape) { if *s <= 0.0 { @@ -186,6 +191,7 @@ pub(crate) fn knockout(layer: &Layer, backdrop: &mut Buffer, target: &Buffer, cl let a = pm[3].clamp(0.0, 1.0); *p = if a > 0.0 { [(pm[0] / a).clamp(0.0, 1.0), (pm[1] / a).clamp(0.0, 1.0), (pm[2] / a).clamp(0.0, 1.0), a] } else { [0.0; 4] }; } + recycle_mask(Some(shape)); } /// Composite a clipping group's clipped layers atop `base` (the base's isolated content, Blend @@ -220,7 +226,12 @@ pub(crate) fn composite_clipped_individually(layer: &Layer, clipped: &[Layer], b let op = layer.opacity.clamp(0.0, 1.0); let clip: Vec = match render_content(layer, rect, cx) { Some(b) => b.px.iter().map(|p| p[3] * op).collect(), - None => mask_vals(layer, rect, cx).unwrap_or_else(|| vec![1.0; n]).iter().map(|m| m * op).collect(), + None => { + let mv = mask_vals(layer, rect, cx); + let clip = mv.as_ref().map_or_else(|| vec![op; n], |m| m.iter().map(|m| m * op).collect()); + recycle_mask(mv); + clip + } }; if clip.len() != n { return; diff --git a/crates/compose/src/advanced_tests.rs b/crates/compose/src/advanced_tests.rs index 608c6188e2..15d3cf2346 100644 --- a/crates/compose/src/advanced_tests.rs +++ b/crates/compose/src/advanced_tests.rs @@ -395,7 +395,7 @@ fn shapeless_stroke_tile_skips_match_full_region_across_blending_settings() { .unwrap(); let mut band_pixels = Vec::with_capacity(whole.px.len()); for band in bands { - band_pixels.extend(band.px); + band_pixels.extend(band.into_px()); } assert_eq!(whole.px, band_pixels, "banded: {depth:?}, {position:?}, {blend:?}, fill={fill}, blend_if={blend_if}"); } @@ -639,7 +639,7 @@ fn transparency_shapes_off_ignores_masks_and_reaches_groups() { l.mask = Some(LayerMask { surface: photocraft_raster::Surface::with_default(PixelFormat::GRAY8, &[77.0 / 255.0]), ..LayerMask::reveal_all() }); l.advanced.transparency_shapes = tsl; d.layers.push(l); - render(&d, d.bounds()).px + render(&d, d.bounds()).into_px() }; let (off, on) = (render_with(false), render_with(true)); assert!(off.iter().zip(&on).all(|(a, b)| close(*a, *b)), "{mode:?} ({depth:?}): a mask isn't the layer's transparency"); @@ -691,13 +691,13 @@ fn transparency_shapes_off_keeps_the_base_alpha_under_a_clipped_knockout() { base.advanced.transparency_shapes = false; base.vector_mask = Some(photocraft_doc::vector::VectorMask { feather: 1.5, ..photocraft_doc::vector::VectorMask::new(square.clone()) }); d.layers.push(base); - let alone = render(&d, d.bounds()).px; + let alone = render(&d, d.bounds()).into_px(); let mut red = solid(&d, "red", full(&d), [1.0, 0.0, 0.0, 0.5]); red.clipped = true; let mut knock = knocker(&d, k); knock.clipped = true; d.layers.extend([red, knock]); - let got = render(&d, d.bounds()).px; + let got = render(&d, d.bounds()).into_px(); let what = format!("{k:?} ({depth:?})"); for (i, (g, a)) in got.iter().zip(&alone).enumerate() { if i % 12 < 6 { diff --git a/crates/compose/src/cmyk.rs b/crates/compose/src/cmyk.rs index 96a6df0ceb..2f8eaa8861 100644 --- a/crates/compose/src/cmyk.rs +++ b/crates/compose/src/cmyk.rs @@ -27,6 +27,7 @@ use photocraft_doc::adjust::ToneSpace; use photocraft_doc::{Adjustment, Document, Knockout, Layer, LayerContent}; use photocraft_geom::Rect; +use super::extents::recycle_mask; use super::{Ctx, RENDER_TILE, advanced, empty_in, mask_vals, pattern}; #[path = "cmyk_split.rs"] @@ -355,6 +356,7 @@ fn apply_mask(l: &Layer, buf: &mut CmykBuffer, cx: &Ctx) { for (p, k) in buf.px.iter_mut().zip(&m) { p[4] *= clean(*k).max(0.0); } + recycle_mask(Some(m)); } } @@ -425,6 +427,7 @@ fn mix_adjusted(l: &Layer, base: &mut CmykBuffer, adjusted: &CmykBuffer, cx: &Ct *v = if k >= 1.0 { b } else { *v + (b - *v) * k }; } } + recycle_mask(mv); } /// Rounds to the document's levels after an adjustment layer, as the compositor does (on the @@ -534,6 +537,7 @@ fn composite_layer(l: &Layer, clipped: &[Layer], backdrop: &mut CmykBuffer, cx: [0.0; 5] }; } + recycle_mask(mv); } return; } diff --git a/crates/compose/src/composite.rs b/crates/compose/src/composite.rs index 8436ff7428..4ef497672f 100644 --- a/crates/compose/src/composite.rs +++ b/crates/compose/src/composite.rs @@ -1,6 +1,6 @@ -use super::content::{channel_weights, empty_in, recycle_buffer, render_content, restore_channels, scratch_buffer, surface_to_buffer}; +use super::content::{channel_weights, empty_in, render_content, restore_channels, surface_to_buffer}; use super::ctx::Ctx; -use super::extents::{effects_frame, mask_k, mask_vals, shapeless_stroke_bounds}; +use super::extents::{effects_frame, mask_k, mask_vals, recycle_mask, shapeless_stroke_bounds}; use super::fx_cache::effect_maps; use super::{BlendMode, Buffer, Fill, Layer, LayerContent, Rect, Surface, advanced, blend, effects, psblend, shape_split}; @@ -339,6 +339,7 @@ pub(crate) fn composite_pixel_layer(layer: &Layer, surf: &Surface, backdrop: &mu } blend_span_fill(dst, &row, rect.x0, y, layer.blend, layer.opacity, layer.fill_opacity, gamma); } + recycle_mask(mv); } pub(crate) fn composite_layer_plain(layer: &Layer, clipped: &[Layer], backdrop: &mut Buffer, cx: &Ctx, scope: advanced::Scope) { @@ -389,6 +390,7 @@ pub(crate) fn composite_layer_plain(layer: &Layer, clipped: &[Layer], backdrop: [0.0; 4] }; } + recycle_mask(mv); } // A Frame's stroke over the frame's content, at the group's opacity. if let Some(stroke) = shape_split::frame_stroke(layer, cx.canvas, cx.depth) { @@ -425,9 +427,10 @@ pub(crate) fn composite_layer_plain(layer: &Layer, clipped: &[Layer], backdrop: if let LayerContent::Adjustment(adj) = &layer.content { let mv = mask_vals(layer, rect, cx); if mv.as_ref().is_some_and(|values| values.iter().all(|&value| opacity * value <= 0.0)) { + recycle_mask(mv); return; } - let mut adjusted = scratch_buffer(rect); + let mut adjusted = Buffer::transparent(rect); adjusted.px.copy_from_slice(&backdrop.px); cx.apply_adjustment(adj, &mut adjusted); // Clipped layers onto an adjustment are uncommon; they composite atop the adjusted result. @@ -455,7 +458,7 @@ pub(crate) fn composite_layer_plain(layer: &Layer, clipped: &[Layer], backdrop: None => blend_back(&mut backdrop.px, &adjusted.px, std::iter::repeat(k), layer.blend, fill), } } - recycle_buffer(adjusted); + recycle_mask(mv); return; } @@ -469,6 +472,7 @@ pub(crate) fn composite_layer_plain(layer: &Layer, clipped: &[Layer], backdrop: if let (Some(m), Some(b)) = (&hide, &before) { advanced::apply_hiding_mask(backdrop, b, m); } + recycle_mask(hide); return; } if let Some((mut content, stroke, mask)) = shape_parts(layer, clipped, rect, cx) { @@ -478,6 +482,7 @@ pub(crate) fn composite_layer_plain(layer: &Layer, clipped: &[Layer], backdrop: p[3] *= mask_k(&mask, i); } blend_layer_into(backdrop, &content, layer, cx, 1.0); + recycle_mask(mask); return; } if !matches!(layer.content, LayerContent::Group(_) | LayerContent::Fill(_) | LayerContent::Adjustment(_)) @@ -512,7 +517,6 @@ pub(crate) fn composite_layer_plain(layer: &Layer, clipped: &[Layer], backdrop: let Some(mut content) = render_content(layer, rect, cx) else { return }; advanced::composite_clipped(clipped, &mut content, cx); blend_layer_into(backdrop, &content, layer, cx, text_gamma(layer)); - recycle_buffer(content); } /// A layer with effects (and its clipping group) onto `backdrop`. @@ -521,7 +525,7 @@ fn composite_layer_effects(layer: &Layer, clipped: &[Layer], backdrop: &mut Buff // Neighbourhoods are already captured by the full-region effect maps; // content and effect application only need the output rectangle. // A stroked shape's vector stroke stays above its clipped layers and interior effects. - let (mut content, vstroke) = match split_parts(layer, rect, cx) { + let (mut content, parts) = match split_parts(layer, rect, cx) { Some((fill, stroke, mask)) => (fill, Some((stroke, mask))), None => { let Some(content) = render_content(layer, rect, cx) else { return }; @@ -529,9 +533,12 @@ fn composite_layer_effects(layer: &Layer, clipped: &[Layer], backdrop: &mut Buff } }; advanced::composite_clipped(clipped, &mut content, cx); - let vstroke = vstroke.as_ref().map(|(s, m)| effects::VectorStroke { stroke: s, mask: m.as_deref() }); + let vstroke = parts.as_ref().map(|(s, m)| effects::VectorStroke { stroke: s, mask: m.as_deref() }); let maps = effect_maps(layer, rect, cx); effects::composite_with_effects_prepared(layer, &content, backdrop, &maps, effects_frame(layer, cx.canvas), cx.patterns, vstroke); + if let Some((_, mask)) = parts { + recycle_mask(mask); + } } /// The coverage-mixing gamma of a layer: the text blending gamma @@ -605,6 +612,7 @@ fn composite_atop_any(layer: &Layer, base: &mut Buffer, cx: &Ctx) { p[c] += (bl[c] - p[c]) * k; } } + recycle_mask(mv); return; } if effects::has_effects(layer) { @@ -615,21 +623,28 @@ fn composite_atop_any(layer: &Layer, base: &mut Buffer, cx: &Ctx) { let hide = advanced::hiding_mask(layer, rect, cx); let src = advanced::effects_source(layer); let layer = src.as_ref(); - let (content, vstroke) = match split_parts(layer, rect, cx) { + let (content, parts) = match split_parts(layer, rect, cx) { Some((fill, stroke, mask)) => (fill, Some((stroke, mask))), None => { - let Some(content) = render_content(layer, rect, cx) else { return }; + let Some(content) = render_content(layer, rect, cx) else { + recycle_mask(hide); + return; + }; (content, None) } }; - let vstroke = vstroke.as_ref().map(|(s, m)| effects::VectorStroke { stroke: s, mask: m.as_deref() }); + let vstroke = parts.as_ref().map(|(s, m)| effects::VectorStroke { stroke: s, mask: m.as_deref() }); let mut opaque = Buffer { rect, px: base.px.iter().map(|p| [p[0], p[1], p[2], 1.0]).collect() }; let maps = effect_maps(layer, rect, cx); effects::composite_with_effects_prepared(layer, &content, &mut opaque, &maps, effects_frame(layer, cx.canvas), cx.patterns, vstroke); + if let Some((_, mask)) = parts { + recycle_mask(mask); + } if let Some(m) = &hide { let plain = Buffer { rect, px: base.px.iter().map(|p| [p[0], p[1], p[2], 1.0]).collect() }; advanced::apply_hiding_mask(&mut opaque, &plain, m); } + recycle_mask(hide); for (p, o) in base.px.iter_mut().zip(&opaque.px) { if p[3] > 0.0 { *p = [o[0], o[1], o[2], p[3]]; @@ -689,6 +704,7 @@ fn blend_layer_into(backdrop: &mut Buffer, src: &Buffer, layer: &Layer, cx: &Ctx let alpha = (s[3] / m).clamp(0.0, 1.0); *b = psblend::composite_fill(mode, *b, [s[0], s[1], s[2], m], opacity, fill * alpha, gamma); } + recycle_mask(mv); } /// [`blend_into_g`] for a layer's own `opacity` and `fill`: Fill is part of the blend for diff --git a/crates/compose/src/content.rs b/crates/compose/src/content.rs index ea868d715e..1f91c6a942 100644 --- a/crates/compose/src/content.rs +++ b/crates/compose/src/content.rs @@ -1,63 +1,29 @@ use super::composite::{adjustment_quantum, blend_into, composite_stack}; use super::ctx::Ctx; -use super::extents::{effect_shape, effect_source_bounds, mask_vals, shapeless_stroke_bounds, transparent_outside}; +use super::extents::{effect_shape, effect_source_bounds, mask_vals, recycle_mask, shapeless_stroke_bounds, transparent_outside}; use super::fx_cache::effect_maps; use super::{BlendMode, Buffer, Document, Fill, Layer, LayerContent, Pattern, Rect, Surface, bounds, effects, gradient_fill, pattern, shape_split}; /// Tile size for parallel rendering (tile results are independent). pub const RENDER_TILE: i32 = 256; -// One tile of scratch per worker; nested rendering takes its own buffer. -thread_local! { - static SCRATCH: std::cell::RefCell> = const { std::cell::RefCell::new(Vec::new()) }; -} - -pub(crate) fn scratch_buffer(rect: Rect) -> Buffer { - if rect.width() as usize * rect.height() as usize > (RENDER_TILE * RENDER_TILE) as usize { - return Buffer::transparent(rect); - } - let mut px = SCRATCH.with(|slot| std::mem::take(&mut *slot.borrow_mut())); - px.resize(rect.width() as usize * rect.height() as usize, [0.0; 4]); - Buffer { rect, px } -} - -pub(crate) fn recycle_buffer(buf: Buffer) { - if buf.px.capacity() <= (RENDER_TILE * RENDER_TILE) as usize { - SCRATCH.with(|slot| { - let mut slot = slot.borrow_mut(); - if slot.capacity() < buf.px.capacity() { - *slot = buf.px; - } - }); - } -} - #[cfg(test)] #[test] -fn scratch_reuse_preserves_pixels_and_bounds_retention() { - SCRATCH.with(|slot| slot.borrow_mut().clear()); - let rect = Rect::new(-2, -3, 18, 11); +fn scratch_reuse_preserves_pixels() { + // 256 × 256 pixels of [f32; 4] (1 MiB): over `MIN_POOLED_BYTES`, so the pool keeps it. + let rect = Rect::new(-2, -3, 254, 253); for fmt in [photocraft_color::PixelFormat::RGBA8, photocraft_color::PixelFormat::RGBA16, photocraft_color::PixelFormat::RGBA32F] { let s = Surface::with_default(fmt, &[0.2, 0.4, 0.6, 0.8]); - let buf = surface_to_scratch(&s, rect); + let mut buf = surface_to_scratch(&s, rect); assert_eq!(buf, surface_to_buffer(&s, rect)); - let ptr = buf.px.as_ptr(); - recycle_buffer(buf); - let reused = surface_to_scratch(&Surface::new(fmt), rect); - assert_eq!(reused.px.as_ptr(), ptr); + buf.px.fill([1.0; 4]); + // A dropped buffer goes back to the pool; the next one must come out cleared. + drop(buf); + let reused = Buffer::transparent(rect); assert!(reused.px.iter().all(|p| p == &[0.0; 4])); - let nested = scratch_buffer(rect); + let nested = Buffer::transparent(rect); assert_ne!(nested.px.as_ptr(), reused.px.as_ptr()); - recycle_buffer(nested); - recycle_buffer(reused); } - let cached = SCRATCH.with(|slot| slot.borrow().as_ptr()); - let huge = scratch_buffer(Rect::new(0, 0, RENDER_TILE + 1, RENDER_TILE)); - recycle_buffer(huge); - SCRATCH.with(|slot| { - assert_eq!(slot.borrow().as_ptr(), cached); - assert!(slot.borrow().capacity() <= (RENDER_TILE * RENDER_TILE) as usize); - }); } /// Render a layer's own content (no blending into the backdrop yet) into an isolated buffer. @@ -93,6 +59,7 @@ pub(crate) fn render_content(layer: &Layer, rect: Rect, cx: &Ctx) -> Option Buffer { - let mut px = vec![[0.0f32; 4]; rect.width() as usize * rect.height() as usize]; - s.read_rgba_into(rect, &mut px); - Buffer { rect, px } + let mut buf = Buffer::transparent(rect); + s.read_rgba_into(rect, &mut buf.px); + buf } fn surface_to_scratch(s: &Surface, rect: Rect) -> Buffer { - let mut buf = scratch_buffer(rect); + let mut buf = Buffer::transparent(rect); s.read_rgba_into(rect, &mut buf.px); buf } diff --git a/crates/compose/src/extents.rs b/crates/compose/src/extents.rs index d895f14bc9..7e4f8d234e 100644 --- a/crates/compose/src/extents.rs +++ b/crates/compose/src/extents.rs @@ -134,6 +134,7 @@ pub(crate) fn effect_shape(layer: &Layer, rect: Rect, cx: &Ctx) -> Vec { out[i] = v * mask_k(&mv, i); } } + recycle_mask(mv); out } @@ -283,13 +284,13 @@ pub(crate) fn mask_vals(layer: &Layer, rect: Rect, cx: &Ctx) -> Option> if masks::has_feather(layer) && let Some(s) = masks::combined_mask(layer, cx.canvas) { - let mut v = Vec::new(); + let mut v = photocraft_raster::scratch::take_cap(rect.width() as usize * rect.height() as usize); s.read_region_into(rect, &mut v); return Some(v); } let vector = layer.vector_mask.as_ref().map(|vm| cx.vector_masks.values(vm, rect)); let Some(m) = layer.mask.as_ref() else { return vector }; - let mut v = Vec::new(); + let mut v = photocraft_raster::scratch::take_cap(rect.width() as usize * rect.height() as usize); m.values_into(rect, &mut v); if let Some(vm) = vector { for (a, b) in v.iter_mut().zip(vm) { @@ -299,6 +300,13 @@ pub(crate) fn mask_vals(layer: &Layer, rect: Rect, cx: &Ctx) -> Option> Some(v) } +/// Hand a [`mask_vals`] result back to the scratch pool once its tile is done. +pub(crate) fn recycle_mask(m: Option>) { + if let Some(v) = m { + photocraft_raster::scratch::give(v); + } +} + #[inline] pub(crate) fn mask_k(m: &Option>, i: usize) -> f32 { m.as_ref().map_or(1.0, |v| v[i]) diff --git a/crates/compose/src/lib.rs b/crates/compose/src/lib.rs index e593290571..25dc55984a 100644 --- a/crates/compose/src/lib.rs +++ b/crates/compose/src/lib.rs @@ -84,18 +84,39 @@ pub use render::{ }; /// Straight-alpha RGBA float buffer covering a rectangle. -#[derive(Clone, Debug, PartialEq)] +/// +/// The pixel storage comes from, and goes back to, the scratch pool. +#[derive(Debug, PartialEq)] pub struct Buffer { pub rect: Rect, pub px: Vec<[f32; 4]>, } +impl Clone for Buffer { + fn clone(&self) -> Self { + let mut px = photocraft_raster::scratch::take(self.px.len(), [0.0; 4]); + px.copy_from_slice(&self.px); + Self { rect: self.rect, px } + } +} + +impl Drop for Buffer { + fn drop(&mut self) { + photocraft_raster::scratch::give(std::mem::take(&mut self.px)); + } +} + impl Buffer { pub fn transparent(rect: Rect) -> Self { - Self { rect, px: vec![[0.0; 4]; rect.width() as usize * rect.height() as usize] } + Self::filled(rect, [0.0; 4]) } pub fn filled(rect: Rect, c: [f32; 4]) -> Self { - Self { rect, px: vec![c; rect.width() as usize * rect.height() as usize] } + Self { rect, px: photocraft_raster::scratch::take(rect.width() as usize * rect.height() as usize, c) } + } + /// The pixels, taken out of the buffer (it can't be destructured: it returns them to the + /// pool on drop). + pub fn into_px(mut self) -> Vec<[f32; 4]> { + std::mem::take(&mut self.px) } #[inline] pub fn get(&self, x: i32, y: i32) -> [f32; 4] { diff --git a/crates/compose/src/render.rs b/crates/compose/src/render.rs index b739d3f42a..250a3ebce1 100644 --- a/crates/compose/src/render.rs +++ b/crates/compose/src/render.rs @@ -229,14 +229,12 @@ pub fn composite_to_surface(doc: &Document, rect: Rect, fmt: photocraft_color::P let mut s = Surface::new(fmt); let n = fmt.channels(); let empty = s.default_pixel(); - let mut data = Vec::new(); let _ = render_bands(doc, rect, 0, |band| -> Result<(), ()> { let band = match background { Some(bg) => band.over_background(bg), None => band, }; - data.clear(); - data.resize(band.px.len() * n, 0.0); + let mut data = photocraft_raster::scratch::take(band.px.len() * n, 0.0f32); photocraft_color::convert::with_cmyk_space(space.as_ref(), || { for (p, out) in band.px.iter().zip(data.chunks_exact_mut(n)) { if fmt.alpha && (p[3] <= 0.0 || p[3].is_nan()) && empty.len() == n { @@ -247,6 +245,7 @@ pub fn composite_to_surface(doc: &Document, rect: Rect, fmt: photocraft_color::P } }); s.write_region(band.rect, &data); + photocraft_raster::scratch::give(data); Ok(()) }); s.prune(); diff --git a/crates/compose/src/soft_shape_tests.rs b/crates/compose/src/soft_shape_tests.rs index 37575beef5..9332f31af1 100644 --- a/crates/compose/src/soft_shape_tests.rs +++ b/crates/compose/src/soft_shape_tests.rs @@ -115,7 +115,7 @@ fn soft_shape_matches_an_equal_vector_mask() { fn soft_shape_keeps_its_stroke_crisp() { let stroke = ShapeStroke { width: 4.0, paint: Fill::Solid(Color::rgb(1.0, 0.0, 0.0)), ..ShapeStroke::default() }; let d = shape_doc(SampleType::U8, 0.5, 10.0, Some(stroke)); - let px = render(&d, Rect::new(0, 100, 200, 101)).px; + let px = render(&d, Rect::new(0, 100, 200, 101)).into_px(); // Centred 4 px stroke over x = 48..52: opaque red. for x in [48, 49, 50, 51] { let p = px[x]; diff --git a/crates/compose/src/tests.rs b/crates/compose/src/tests.rs index 917192f44a..7ea8655099 100644 --- a/crates/compose/src/tests.rs +++ b/crates/compose/src/tests.rs @@ -3001,7 +3001,7 @@ fn levels_steps_match_photoshop() { let px = xs.iter().map(|&x| f32::from(x) / 255.0).map(|v| [v, v, v, 1.0]).collect(); let mut b = Buffer { rect: Rect::new(0, 0, xs.len() as i32, 1), px }; adjust::apply_depth(adj, &mut b, adjust::Transfer::Srgb, Some(SampleType::U8)); - b.px + b.into_px() }; let rgb = levels(ch(0.0, 213.0, 1.22, 48.0, 255.0), [ch(0.0, 255.0, 0.47, 0.0, 194.0), id.clone(), ch(61.0, 255.0, 0.47, 0.0, 213.0)]); let xs = [2, 8, 9, 15, 16, 43, 45, 48, 55, 61, 72, 79, 84, 89, 90, 101, 106, 120, 150, 180, 200]; @@ -3364,7 +3364,7 @@ fn render_bands_cover_the_rect_in_order() { render_bands(&d, r, 100, |b| -> Result<(), ()> { assert_eq!((b.rect.x0, b.rect.x1), (r.x0, r.x1)); rows.push((b.rect.y0, b.rect.y1)); - px.extend(b.px); + px.extend(b.into_px()); Ok(()) }) .unwrap(); @@ -4469,13 +4469,13 @@ fn dissolve_adjustment_resolves_opacity_to_binary_coverage() { layer.blend = BlendMode::Dissolve; layer.opacity = 0.5; d.layers.push(layer); - let pixels = render(&d, d.bounds()).px; + let pixels = render(&d, d.bounds()).into_px(); let black = pixels.iter().filter(|p| p[0] < 0.01).count(); let white = pixels.iter().filter(|p| p[0] > 0.99).count(); assert!(black > 0 && white > 0, "Dissolve must select both original and adjusted pixels"); assert_eq!(black + white, pixels.len(), "no intermediate gray pixels"); d.layers.last_mut().unwrap().blend = BlendMode::Normal; - let continuous = render(&d, d.bounds()).px; + let continuous = render(&d, d.bounds()).into_px(); assert!(continuous.iter().all(|p| (p[0] - 0.5).abs() < E), "Normal remains continuously interpolated"); } @@ -4486,14 +4486,14 @@ fn clipped_dissolve_fill_matches_opaque_unclipped_coverage() { fill.blend = BlendMode::Dissolve; fill.opacity = 0.5; d.layers.push(fill); - let control = render(&d, d.bounds()).px; + let control = render(&d, d.bounds()).into_px(); d.layers.last_mut().unwrap().clipped = true; - let clipped = render(&d, d.bounds()).px; + let clipped = render(&d, d.bounds()).into_px(); assert_eq!(clipped, control, "clipping over a fully opaque base must preserve dissolve pixels"); let black = clipped.iter().filter(|p| p[0] < 0.01).count(); assert!(black > 0 && black < clipped.len()); d.layers.last_mut().unwrap().blend = BlendMode::Normal; - let continuous = render(&d, d.bounds()).px; + let continuous = render(&d, d.bounds()).into_px(); assert!(continuous.iter().all(|p| (p[0] - 0.5).abs() < E)); } @@ -4504,17 +4504,17 @@ fn dissolve_color_overlay_preserves_binary_layer_coverage() { layer.blend = BlendMode::Dissolve; layer.opacity = 0.5; d.layers.push(layer); - let control = render(&d, d.bounds()).px; + let control = render(&d, d.bounds()).into_px(); d.layers .last_mut() .unwrap() .effects .items .push(photocraft_doc::Effect::ColorOverlay { common: photocraft_doc::FxCommon::new(BlendMode::Normal, 1.0), color: Color::BLACK }); - let styled = render(&d, d.bounds()).px; + let styled = render(&d, d.bounds()).into_px(); assert_eq!(styled, control, "an identity overlay should not turn dissolve pixels gray"); d.layers.last_mut().unwrap().blend = BlendMode::Normal; - let continuous = render(&d, d.bounds()).px; + let continuous = render(&d, d.bounds()).into_px(); assert!(continuous.iter().all(|p| (p[0] - 0.5).abs() < E)); } diff --git a/crates/compose/tests/buffer_reuse.rs b/crates/compose/tests/buffer_reuse.rs new file mode 100644 index 0000000000..f07bf8b69f --- /dev/null +++ b/crates/compose/tests/buffer_reuse.rs @@ -0,0 +1,27 @@ +use photocraft_compose::Buffer; +use photocraft_geom::Rect; +use photocraft_raster::scratch; + +/// Restores idle release even when an assertion fails. +struct IdleReleaseOff; +impl Drop for IdleReleaseOff { + fn drop(&mut self) { + scratch::release(); + scratch::set_idle_release(true); + } +} + +/// Runs in its own binary so other compositor tests cannot consume the pooled allocation. +#[test] +fn dropped_buffer_storage_is_reused() { + scratch::set_idle_release(false); + let _idle = IdleReleaseOff; + scratch::release(); + let rect = Rect::from_xywh(0, 0, 256, 256); + let buf = Buffer::transparent(rect); + let ptr = buf.px.as_ptr(); + drop(buf); + let reused = Buffer::transparent(rect); + assert_eq!(reused.px.as_ptr(), ptr); + assert!(reused.px.iter().all(|p| *p == [0.0; 4])); +} diff --git a/crates/engine/examples/indexed_color_perf.rs b/crates/engine/examples/indexed_color_perf.rs index 6dba948ff2..112f6eb0d9 100644 --- a/crates/engine/examples/indexed_color_perf.rs +++ b/crates/engine/examples/indexed_color_perf.rs @@ -25,7 +25,7 @@ fn main() -> Result<(), Box> { let path = std::env::args().nth(1).ok_or("provide a local image path")?; let bytes = photocraft_format::read::read_file(std::path::Path::new(&path))?; let doc = photocraft_io::import(&path, &bytes)?.document; - let px = photocraft_compose::flatten(&doc).px; + let px = photocraft_compose::flatten(&doc).into_px(); println!("{}", json!({"width":doc.size.width,"height":doc.size.height,"preview_factor":1})); for count in [8usize, 16, 32, 64, 128, 256] { for run in 0..4 { diff --git a/crates/engine/src/cmd/adjust_cmds.rs b/crates/engine/src/cmd/adjust_cmds.rs index 39ab43c7a6..f5301979dc 100644 --- a/crates/engine/src/cmd/adjust_cmds.rs +++ b/crates/engine/src/cmd/adjust_cmds.rs @@ -459,9 +459,9 @@ fn stats_pixels(doc: &Document, layer: Option, use_selection: bool) -> Resu let px: Vec<[f32; 4]> = match layer { Some(id) => { let l = doc.layer(photocraft_doc::LayerId(id)).ok_or(EngineError::NoLayer(photocraft_doc::LayerId(id)))?; - photocraft_compose::render_layer(l, area).px + photocraft_compose::render_layer(l, area).into_px() } - None => photocraft_compose::flatten(doc).px, + None => photocraft_compose::flatten(doc).into_px(), }; let mask = doc.selection.as_ref().filter(|_| use_selection).map(|sel| photocraft_algo::selection::mask_from_surface(Some(sel), area)); Ok((px, mask)) @@ -521,7 +521,7 @@ fn hdr_toning(s: &mut Session, p: &Value) -> Result { s.edit("HDR Toning", |doc, active| { let buf = photocraft_compose::flatten(doc).over_background([1.0, 1.0, 1.0]); let (w, hgt) = (buf.rect.width() as usize, buf.rect.height() as usize); - let mut px = buf.px; + let mut px = buf.into_px(); tone::hdr_toning(&mut px, w, hgt, &h); let fmt = doc.pixel_format(); let data: Vec = px.iter().flat_map(|q| photocraft_raster::from_rgba(&fmt, *q)).collect(); diff --git a/crates/engine/src/cmd/adjust_cmds/tests.rs b/crates/engine/src/cmd/adjust_cmds/tests.rs index d60fa26d10..51843ae022 100644 --- a/crates/engine/src/cmd/adjust_cmds/tests.rs +++ b/crates/engine/src/cmd/adjust_cmds/tests.rs @@ -480,7 +480,7 @@ fn smart_session() -> Session { } fn flatten(s: &Session) -> Vec<[f32; 4]> { - photocraft_compose::flatten(doc(s)).px + photocraft_compose::flatten(doc(s)).into_px() } fn max_diff(a: &[[f32; 4]], b: &[[f32; 4]]) -> f32 { diff --git a/crates/engine/src/cmd/face_cmds.rs b/crates/engine/src/cmd/face_cmds.rs index d48df95b84..0ae0af3d35 100644 --- a/crates/engine/src/cmd/face_cmds.rs +++ b/crates/engine/src/cmd/face_cmds.rs @@ -36,7 +36,7 @@ pub fn faces_in(doc: &Document, active: Option, all_layers: bool, min_s surf.read_rgba_into(area, &mut px); px } - None => photocraft_compose::render(doc, area).px, + None => photocraft_compose::render(doc, area).into_px(), }; let found = detect(&px, w, h, min_score)?; let (x0, y0) = (area.x0 as f32, area.y0 as f32); diff --git a/crates/engine/src/cmd/layer_menu_cmds.rs b/crates/engine/src/cmd/layer_menu_cmds.rs index 34a1f914ee..75701d9ddd 100644 --- a/crates/engine/src/cmd/layer_menu_cmds.rs +++ b/crates/engine/src/cmd/layer_menu_cmds.rs @@ -230,7 +230,7 @@ fn mask_all_objects(s: &mut Session, p: &Value) -> Result { struct Composite<'a>(&'a Document); impl photocraft_algo::segment::Sampler for Composite<'_> { fn rgba(&self, r: Rect) -> Vec<[f32; 4]> { - photocraft_compose::render(self.0, r).px + photocraft_compose::render(self.0, r).into_px() } } photocraft_algo::segment::subject::select_subject(&Composite(&doc), doc.bounds()) diff --git a/crates/engine/src/cmd/layer_menu_cmds/tests.rs b/crates/engine/src/cmd/layer_menu_cmds/tests.rs index 702d65891b..62522ccc39 100644 --- a/crates/engine/src/cmd/layer_menu_cmds/tests.rs +++ b/crates/engine/src/cmd/layer_menu_cmds/tests.rs @@ -757,11 +757,11 @@ fn applying_a_feathered_mask_preserves_its_soft_composite() { Ok(()) }) .unwrap(); - let before = photocraft_compose::flatten(doc(&s)).px; + let before = photocraft_compose::flatten(doc(&s)).into_px(); assert!(before[15 * 40 + 19][3] > 0.0 && before[15 * 40 + 19][3] < 1.0, "mask must start with a soft edge"); s.execute("layer.layerMask.apply", json!({})).unwrap(); assert!(active(&s).mask.is_none(), "Apply removes the pixel mask"); - let after = photocraft_compose::flatten(doc(&s)).px; + let after = photocraft_compose::flatten(doc(&s)).into_px(); for (a, b) in before.iter().zip(&after) { assert!((a[3] - b[3]).abs() <= 1.0 / 255.0 + 1e-5, "depth {depth}: alpha {a:?} became {b:?}"); } @@ -791,13 +791,13 @@ fn from_transparency_preserves_mask_coverage_outside_the_canvas() { for depth in DEPTHS { let mut s = session(depth, "rgb"); paint(&mut s, |_, _| [1.0, 0.0, 0.0, 0.5]); - let before = photocraft_compose::flatten(doc(&s)).px; + let before = photocraft_compose::flatten(doc(&s)).into_px(); s.execute("layer.translate", json!({"dx": 20})).unwrap(); s.execute("layer.layerMask.fromTransparency", json!({})).unwrap(); let mask = active(&s).mask.as_ref().unwrap(); assert!((mask.value(50, 10) - 0.5).abs() < 1.0 / 255.0 + 1e-6, "off-canvas mask at depth {depth}"); s.execute("layer.translate", json!({"dx": -20})).unwrap(); - let after = photocraft_compose::flatten(doc(&s)).px; + let after = photocraft_compose::flatten(doc(&s)).into_px(); for (a, b) in before.iter().zip(&after) { assert!((a[3] - b[3]).abs() <= 1.0 / 255.0 + 1e-6, "alpha changed at depth {depth}: {a:?} -> {b:?}"); } diff --git a/crates/engine/src/cmd/mask_view_cmds/tests.rs b/crates/engine/src/cmd/mask_view_cmds/tests.rs index ce9bffa744..82179a5456 100644 --- a/crates/engine/src/cmd/mask_view_cmds/tests.rs +++ b/crates/engine/src/cmd/mask_view_cmds/tests.rs @@ -44,7 +44,7 @@ fn modes_toggle_and_show_in_inspect_without_history() { fn a_mask_view_never_alters_pixels() { let (mut s, masked, _) = session(); let before = s.active().unwrap().doc.clone(); - let px = |s: &Session| photocraft_compose::render(&s.active().unwrap().doc, Rect::new(0, 0, 40, 20)).px; + let px = |s: &Session| photocraft_compose::render(&s.active().unwrap().doc, Rect::new(0, 0, 40, 20)).into_px(); let composite = px(&s); for mode in ["gray", "overlay", "toggleOverlay", "toggleGray", "off"] { s.execute(ID, json!({"layer": masked, "mode": mode})).unwrap(); diff --git a/crates/engine/src/cmd/mode_cmds.rs b/crates/engine/src/cmd/mode_cmds.rs index ac30023d92..3043f080e6 100644 --- a/crates/engine/src/cmd/mode_cmds.rs +++ b/crates/engine/src/cmd/mode_cmds.rs @@ -186,7 +186,7 @@ fn with_released_movement_locks(l: &mut Layer, f: impl FnOnce(&mut Layer) -> /// The flattened image as straight RGBA (over white when `opaque`). fn composite(doc: &Document, opaque: bool) -> Vec<[f32; 4]> { let buf = photocraft_compose::flatten(doc); - if opaque { buf.over_background([1.0, 1.0, 1.0]).px } else { buf.px } + if opaque { buf.over_background([1.0, 1.0, 1.0]).into_px() } else { buf.into_px() } } /// Replace the document's layers by one 8-bit layer holding `px` in `mode`'s storage format. diff --git a/crates/engine/src/cmd/mode_cmds/tests.rs b/crates/engine/src/cmd/mode_cmds/tests.rs index 0d3f9b3cea..4d3c97b02b 100644 --- a/crates/engine/src/cmd/mode_cmds/tests.rs +++ b/crates/engine/src/cmd/mode_cmds/tests.rs @@ -568,7 +568,7 @@ fn bitmap_budgets_a_conversion_at_the_document_resolution_too() { /// The Bitmap conversion of `d` the pre-#8477 way, from one full-canvas composite over white. fn bitmap_oracle(d: &Document, method: Option, custom: Option<(&[f32], usize, usize)>) -> Vec { - let px = photocraft_compose::flatten(d).over_background([1.0; 3]).px; + let px = photocraft_compose::flatten(d).over_background([1.0; 3]).into_px(); let mut gray: Vec = px.iter().map(|q| photocraft_color::convert::rgb_to_gray([q[0], q[1], q[2]])).collect(); let w = d.size.width as usize; match (method, custom) { @@ -730,7 +730,7 @@ fn duotone_to_colour_modes_keeps_the_ink_colours() { s.execute("image.mode.duotone", cfg.clone()).unwrap(); let before = s.active().unwrap().doc.clone(); let gray: Vec = (0..16).map(|x| before.layers[0].surface().unwrap().pixel(x, 1)[0]).collect(); - let shown = photocraft_compose::render(&display_document(&before).unwrap(), Rect::from_xywh(0, 1, 16, 1)).px; + let shown = photocraft_compose::render(&display_document(&before).unwrap(), Rect::from_xywh(0, 1, 16, 1)).into_px(); s.execute(&format!("image.mode.{mode}"), json!({})).unwrap(); let d = doc(&s); assert!(d.duotone.is_none(), "{ctx}"); diff --git a/crates/engine/src/cmd/select_extra_cmds.rs b/crates/engine/src/cmd/select_extra_cmds.rs index 8ca012266d..9fd7b309cc 100644 --- a/crates/engine/src/cmd/select_extra_cmds.rs +++ b/crates/engine/src/cmd/select_extra_cmds.rs @@ -40,7 +40,7 @@ struct CompositeSampler<'a>(&'a Document); impl Sampler for CompositeSampler<'_> { fn rgba(&self, r: Rect) -> Vec<[f32; 4]> { - photocraft_compose::render(self.0, r).px + photocraft_compose::render(self.0, r).into_px() } } diff --git a/crates/engine/src/cmd/selection_cmds.rs b/crates/engine/src/cmd/selection_cmds.rs index e8d206285e..024f6b088b 100644 --- a/crates/engine/src/cmd/selection_cmds.rs +++ b/crates/engine/src/cmd/selection_cmds.rs @@ -70,7 +70,7 @@ fn sample_pixels(s: &Session, all_layers: bool, window: Option) -> Result< surf.read_rgba_into(area, &mut px); Ok((area, px)) } - _ => Ok((area, photocraft_compose::render(&d.doc, area).px)), + _ => Ok((area, photocraft_compose::render(&d.doc, area).into_px())), } } diff --git a/crates/engine/src/cmd/smart_cmds/rendering.rs b/crates/engine/src/cmd/smart_cmds/rendering.rs index 907c7d6947..ab862d5545 100644 --- a/crates/engine/src/cmd/smart_cmds/rendering.rs +++ b/crates/engine/src/cmd/smart_cmds/rendering.rs @@ -39,7 +39,7 @@ pub fn stack_image(file_name: &str, bytes: &[u8], fmt: PixelFormat, mode: photoc let mut one = l.clone(); one.blend = BlendMode::Normal; one.clipped = false; - photocraft_compose::render_layer(&one, bounds).px + photocraft_compose::render_layer(&one, bounds).into_px() }) .collect(); if frames.is_empty() { diff --git a/crates/engine/src/cmd/smart_cmds/tests.rs b/crates/engine/src/cmd/smart_cmds/tests.rs index d6306f0d18..6e8ce37f9c 100644 --- a/crates/engine/src/cmd/smart_cmds/tests.rs +++ b/crates/engine/src/cmd/smart_cmds/tests.rs @@ -110,7 +110,7 @@ fn paint(s: &mut Session) { } fn flat(s: &Session) -> Vec<[f32; 4]> { - photocraft_compose::flatten(&s.active().unwrap().doc).px + photocraft_compose::flatten(&s.active().unwrap().doc).into_px() } fn max_diff(a: &[[f32; 4]], b: &[[f32; 4]]) -> f32 { diff --git a/crates/engine/src/cmd/smartselect_cmds.rs b/crates/engine/src/cmd/smartselect_cmds.rs index c04df742a8..adbf9b5f46 100644 --- a/crates/engine/src/cmd/smartselect_cmds.rs +++ b/crates/engine/src/cmd/smartselect_cmds.rs @@ -49,7 +49,7 @@ struct CompositeSampler<'a>(&'a Document); impl Sampler for CompositeSampler<'_> { fn rgba(&self, r: Rect) -> Vec<[f32; 4]> { - photocraft_compose::render(self.0, r).px + photocraft_compose::render(self.0, r).into_px() } } diff --git a/crates/engine/src/cmd/transform_cmds/tests.rs b/crates/engine/src/cmd/transform_cmds/tests.rs index e9ed16f245..7bf247466b 100644 --- a/crates/engine/src/cmd/transform_cmds/tests.rs +++ b/crates/engine/src/cmd/transform_cmds/tests.rs @@ -366,7 +366,7 @@ fn off_from_fresh(s: &Session) -> f32 { let st = s.active().unwrap(); let mut fresh: Document = (*st.doc).clone(); fresh.layer_mut(st.active_layer.unwrap()).unwrap().fill_cache = None; - let (a, b) = (photocraft_compose::render(&st.doc, st.doc.bounds()).px, photocraft_compose::render(&fresh, fresh.bounds()).px); + let (a, b) = (photocraft_compose::render(&st.doc, st.doc.bounds()).into_px(), photocraft_compose::render(&fresh, fresh.bounds()).into_px()); a.iter().zip(&b).flat_map(|(p, q)| p.iter().zip(q).map(|(x, y)| (x - y).abs())).fold(0.0, f32::max) } diff --git a/crates/engine/src/cmd/video_cmds/tests.rs b/crates/engine/src/cmd/video_cmds/tests.rs index 66566bdb7d..d08135f60e 100644 --- a/crates/engine/src/cmd/video_cmds/tests.rs +++ b/crates/engine/src/cmd/video_cmds/tests.rs @@ -229,7 +229,7 @@ fn animated_gif_keeps_transparency() { // Each frame has a transparent index and is cleared before the next (disposal 2). let packed: Vec = gif.windows(4).filter(|w| w[..3] == [0x21, 0xF9, 0x04]).map(|w| w[3]).collect(); assert_eq!(packed, [0x09, 0x09]); - let px = photocraft_compose::flatten(&photocraft_io::import("t.gif", &gif).unwrap().document).px; + let px = photocraft_compose::flatten(&photocraft_io::import("t.gif", &gif).unwrap().document).into_px(); assert_eq!(px[0][3], 0.0, "the corner stays transparent"); assert_eq!(px[3 * 8 + 3], [1.0, 0.0, 0.0, 1.0], "the square stays red"); } diff --git a/crates/engine/src/color_channels/tone.rs b/crates/engine/src/color_channels/tone.rs index 97d6e8f7b2..c64b1debba 100644 --- a/crates/engine/src/color_channels/tone.rs +++ b/crates/engine/src/color_channels/tone.rs @@ -61,7 +61,7 @@ pub(super) fn adjust(surface: &mut Surface, colors: ColorChannels, adjustment: & .collect(), }; adjust::apply_in(&scalar, &mut buf, transfer, depth, mode); - Arc::new(IntegerTable { adjusted: buf.px.into_iter().map(|p| p[0]).collect(), encoded: Vec::new() }) + Arc::new(IntegerTable { adjusted: buf.px.iter().map(|p| p[0]).collect(), encoded: Vec::new() }) }) }); let mut component = Component { index, scalar, table, lab_invert }; diff --git a/crates/engine/src/doc_path/tests.rs b/crates/engine/src/doc_path/tests.rs index 1c9bc88874..049e126a47 100644 --- a/crates/engine/src/doc_path/tests.rs +++ b/crates/engine/src/doc_path/tests.rs @@ -157,7 +157,7 @@ fn replayed(s: &mut Session, id: LayerId, params: serde_json::Value) -> Vec<[f32 }) .unwrap(); let doc = &s.active().unwrap().doc; - photocraft_compose::render(doc, doc.bounds()).px + photocraft_compose::render(doc, doc.bounds()).into_px() } #[test] @@ -202,12 +202,12 @@ fn smart_displace_keeps_a_relative_map_path() { png(&d.join("map.png"), |_| [255, 255, 255]); let (mut s, id) = document(&d, &d.join("ramp.png")); let path = d.join("map.png").to_string_lossy().into_owned(); - let unfiltered = photocraft_compose::render(&s.active().unwrap().doc, s.active().unwrap().doc.bounds()).px; + let unfiltered = photocraft_compose::render(&s.active().unwrap().doc, s.active().unwrap().doc.bounds()).into_px(); s.execute("filter.distort.displace", json!({"layer": id.0, "mapPath": path, "horizontal": 50, "vertical": 0})).unwrap(); let state = |s: &Session| { let doc = &s.active().unwrap().doc; let Some(LayerContent::Smart(sm)) = doc.layer(id).map(|l| &l.content) else { panic!("a smart object") }; - (sm.smart_filters[0].params.clone(), photocraft_compose::render(doc, doc.bounds()).px) + (sm.smart_filters[0].params.clone(), photocraft_compose::render(doc, doc.bounds()).into_px()) }; let (params, displaced) = state(&s); assert_eq!(params["mapPath"], json!(path)); diff --git a/crates/engine/src/filters.rs b/crates/engine/src/filters.rs index aebee03caf..2bcccb7a5c 100644 --- a/crates/engine/src/filters.rs +++ b/crates/engine/src/filters.rs @@ -397,6 +397,8 @@ pub(crate) fn keep_alpha(old: &photocraft_raster::Surface, new: &mut photocraft_ } } s.write_region(*r, &v); + photocraft_raster::scratch::give(o); + photocraft_raster::scratch::give(v); true }) .reduce(|| true, |a, b| a && b); diff --git a/crates/engine/src/jobs.rs b/crates/engine/src/jobs.rs index ac9e38a723..5ebe45599c 100644 --- a/crates/engine/src/jobs.rs +++ b/crates/engine/src/jobs.rs @@ -277,6 +277,16 @@ impl Running { } } +/// A cancelled job's worker that may still be unwinding. +#[cfg(not(target_arch = "wasm32"))] +struct Draining { + handle: std::thread::JoinHandle<()>, + doc: Option, + /// Its document closed: the worker still holds that document's snapshot, so the scratch + /// pool is released again once it exits (the release at close came too early, #10479). + release: bool, +} + /// The session's job table (see the module docs). #[derive(Default)] pub struct Jobs { @@ -292,7 +302,7 @@ pub struct Jobs { recent: Vec, /// Workers of cancelled jobs that may still be unwinding (native). #[cfg(not(target_arch = "wasm32"))] - draining: Vec>, + draining: Vec, /// The context inline job work runs under (see [`Session::set_inline_job_ctx`]). inline: Option, } @@ -673,7 +683,17 @@ impl Session { } } #[cfg(not(target_arch = "wasm32"))] - self.jobs.draining.retain(|h| !h.is_finished()); + { + let mut release = false; + self.jobs.draining.retain(|d| { + let done = d.handle.is_finished(); + release |= done && d.release; + !done + }); + if release { + photocraft_raster::scratch::release(); + } + } std::mem::take(&mut self.jobs.events) } @@ -706,8 +726,8 @@ impl Session { job.ctx.cancel(); self.end_job(&job, JobOutcome::Cancelled); #[cfg(not(target_arch = "wasm32"))] - if let Some(h) = job.handle { - self.jobs.draining.push(h); + if let Some(handle) = job.handle { + self.jobs.draining.push(Draining { handle, doc: job.doc, release: false }); } true } @@ -754,8 +774,15 @@ impl Session { /// cancel latency with it). Native only; returns at once on wasm. pub fn join_cancelled_jobs(&mut self) { #[cfg(not(target_arch = "wasm32"))] - for h in self.jobs.draining.drain(..) { - let _ = h.join(); + { + let mut release = false; + for d in self.jobs.draining.drain(..) { + let _ = d.handle.join(); + release |= d.release; + } + if release { + photocraft_raster::scratch::release(); + } } } @@ -774,6 +801,11 @@ impl Session { for id in ids { self.cancel_job(id); } + // Workers cancelled now, or earlier and still unwinding, hold the closing document. + #[cfg(not(target_arch = "wasm32"))] + for d in self.jobs.draining.iter_mut().filter(|d| d.doc == Some(doc)) { + d.release = true; + } } /// Run the command `id`: inline, or (with `background`) as a job when it is one. @@ -857,6 +889,7 @@ impl Session { // being recorded, nor move the recording mark past the entries it is about to replace. let recording = if spec.journal && crate::actions_cmds::replayable(id) { self.actions.recording.take() } else { None }; let before = self.active().map(|st| st.doc.clone()); + let docs_before: Vec<_> = self.docs.iter().map(|st| st.doc.id).collect(); // Last-resort guard (AGENTS.md, Never crash): a command that panics anyway fails with an // error instead of taking the app down. `edit` only commits a document after its closure // returns, so the documents are unchanged. @@ -876,7 +909,7 @@ impl Session { self.hint_before = None; } let pending = self.jobs.pending.take(); - match (r, pending) { + let out = match (r, pending) { (Err(e), Some(job)) => { job.ctx.cancel(); Err(e) @@ -893,8 +926,8 @@ impl Session { Ok(Started::Job(jid)) } (Ok(v), None) => { - if let Some(before) = before { - self.add_relayout_damage(&before); + if let Some(before) = &before { + self.add_relayout_damage(before); } if let Some((doc_id, layer_id)) = new_fx_target && let Some(st) = self.active_mut() @@ -910,7 +943,15 @@ impl Session { self.after_command(id, params, spec.journal, Some(nested_from), pre_selected.as_deref()); Ok(Started::Done(v)) } + }; + // A closed document's freed buffers went to the scratch pool (`before` kept the active + // one alive until here): hand them back to the OS (#10479). Compared by id, since Split + // and Merge Channels close documents while opening others. + if docs_before.iter().any(|id| !self.docs.iter().any(|st| st.doc.id == *id)) { + drop(before); + photocraft_raster::scratch::release(); } + out } /// Grows the damage a command reported for the pixels it changed by what effects laid out diff --git a/crates/engine/src/pixels.rs b/crates/engine/src/pixels.rs index 906f0b23c6..f95a3feb0d 100644 --- a/crates/engine/src/pixels.rs +++ b/crates/engine/src/pixels.rs @@ -115,6 +115,7 @@ pub fn adjust_surface(s: &mut Surface, adj: &Adjustment, selection: Option<&Surf let tr = tc.rect().intersect(&r); let out = adjusted(&one, tr, adj, selection, mode); one.write_region(tr, &out); + photocraft_raster::scratch::give(out); one.take_tiles(tc.rect()) }) .collect(); @@ -184,6 +185,7 @@ pub(crate) fn adjust_mask_with_transfer( for t in &tiles { let out = adjusted_with_transfer(s, *t, adj, Some(sel), mode, transfer); s.write_region(*t, &out); + photocraft_raster::scratch::give(out); } if !tiles.is_empty() { s.prune(); @@ -197,7 +199,9 @@ pub(crate) fn adjust_mask_with_transfer( // So only the stored tiles are adjusted, not the (possibly huge) rectangle around them. let stored: Vec = s.tiles().map(|(c, _)| c.rect()).collect(); for r in stored { - out.write_region(r, &adjusted_with_transfer(s, r, adj, None, mode, transfer)); + let data = adjusted_with_transfer(s, r, adj, None, mode, transfer); + out.write_region(r, &data); + photocraft_raster::scratch::give(data); } out.prune(); *s = out; @@ -233,7 +237,10 @@ fn adjusted_with_transfer( } let n = fmt.channels(); let raw = s.read_region(r); - let mut buf = Buffer { rect: r, px: raw.chunks_exact(n).map(|p| to_rgba(&fmt, p)).collect() }; + let mut px = photocraft_raster::scratch::take_cap(raw.len() / n.max(1)); + px.extend(raw.chunks_exact(n).map(|p| to_rgba(&fmt, p))); + photocraft_raster::scratch::give(raw); + let mut buf = Buffer { rect: r, px }; let orig = buf.clone(); // 32-bit documents get the float behaviour of an adjustment layer there (Levels doesn't clip); // integer depths keep the unrounded, clipped curves, except that 16-bit Posterize and @@ -311,7 +318,7 @@ pub(crate) fn mix_through_selection( selection: Option<&Surface>, ) -> Vec { let w = (r.width() as usize).max(1); - let mut out = Vec::with_capacity(orig.len() * fmt.channels()); + let mut out = photocraft_raster::scratch::take_cap(orig.len() * fmt.channels()); // The selection's coverage (channel 0) one row at a time, each of its tiles looked up once // per row rather than once per pixel. let mut coverage = vec![1.0f32; w]; diff --git a/crates/engine/src/select_mask_edits.rs b/crates/engine/src/select_mask_edits.rs index 9fd29a2757..4285491eb7 100644 --- a/crates/engine/src/select_mask_edits.rs +++ b/crates/engine/src/select_mask_edits.rs @@ -239,7 +239,9 @@ fn invert(base: Option<&Surface>, canvas: Rect) -> Option { if r.is_empty() { continue; } - let inverted: Vec = b.read_region(r).chunks_exact(ch).map(|px| 1.0 - px.first().copied().unwrap_or(0.0).clamp(0.0, 1.0)).collect(); + let read = b.read_region(r); + let inverted: Vec = read.chunks_exact(ch).map(|px| 1.0 - px.first().copied().unwrap_or(0.0).clamp(0.0, 1.0)).collect(); + photocraft_raster::scratch::give(read); s.write_region(r, &inverted); } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index bae9cf4c92..30c2e32f59 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -814,7 +814,7 @@ fn fill_layer_pixel_rewrites_refresh_the_effect_maps() { .unwrap(); let render = |s: &Session| { let d = &s.active().unwrap().doc; - photocraft_compose::render(d, d.bounds()).px + photocraft_compose::render(d, d.bounds()).into_px() }; // Give the layer the cached-pixels state a PSD import produces (the cache is what Image // › Transform & friends rewrite in place, without touching the Fill spec). @@ -901,7 +901,7 @@ fn group_effect_maps_follow_child_visibility_and_opacity() { .unwrap(); let render = |s: &Session| { let d = &s.active().unwrap().doc; - photocraft_compose::render(d, d.bounds()).px + photocraft_compose::render(d, d.bounds()).into_px() }; let mut prev = render(&s); // populate the effect-map cache type Edit = (&'static str, fn(&mut photocraft_doc::Layer)); diff --git a/crates/engine/tests/it/atn_crystallize_cell_size.rs b/crates/engine/tests/it/atn_crystallize_cell_size.rs index 7b72a6ff70..8a015f1497 100644 --- a/crates/engine/tests/it/atn_crystallize_cell_size.rs +++ b/crates/engine/tests/it/atn_crystallize_cell_size.rs @@ -24,7 +24,7 @@ fn fixture(depth: u32) -> Session { /// Compare the actual flattened document rather than just the imported parameters. fn pixels(s: &Session) -> Vec<[f32; 4]> { - photocraft_compose::flatten(&s.active().unwrap().doc).px + photocraft_compose::flatten(&s.active().unwrap().doc).into_px() } /// Fractions remain valid native edits; strict integer ATN conversion must refuse them. diff --git a/crates/engine/tests/it/atn_ripple_amount.rs b/crates/engine/tests/it/atn_ripple_amount.rs index b6bde88f7a..0ff38f2036 100644 --- a/crates/engine/tests/it/atn_ripple_amount.rs +++ b/crates/engine/tests/it/atn_ripple_amount.rs @@ -24,7 +24,7 @@ fn fixture(depth: u32) -> Session { /// Flatten the active document to compare direct, native and ATN playback pixels. fn pixels(s: &Session) -> Vec<[f32; 4]> { - photocraft_compose::flatten(&s.active().unwrap().doc).px + photocraft_compose::flatten(&s.active().unwrap().doc).into_px() } /// Fractions are valid native edits, but the existing integer event cannot carry them. diff --git a/crates/engine/tests/it/atn_selective_color.rs b/crates/engine/tests/it/atn_selective_color.rs index 454846f59e..92a23aff5e 100644 --- a/crates/engine/tests/it/atn_selective_color.rs +++ b/crates/engine/tests/it/atn_selective_color.rs @@ -37,7 +37,7 @@ fn doc_bounds() -> photocraft_doc::Rect { } fn pixels(s: &Session) -> Vec<[f32; 4]> { - photocraft_compose::flatten(&s.active().unwrap().doc).px + photocraft_compose::flatten(&s.active().unwrap().doc).into_px() } fn check_cases(id: &str, cases: &[Value], require_effect: bool) { diff --git a/crates/engine/tests/it/atn_stroke_width.rs b/crates/engine/tests/it/atn_stroke_width.rs index 742359cf38..3034fb2f0a 100644 --- a/crates/engine/tests/it/atn_stroke_width.rs +++ b/crates/engine/tests/it/atn_stroke_width.rs @@ -23,7 +23,7 @@ fn fixture(depth: u32) -> Session { /// Compare the flattened document, including alpha, rather than only imported JSON. fn pixels(s: &Session) -> Vec<[f32; 4]> { - photocraft_compose::flatten(&s.active().unwrap().doc).px + photocraft_compose::flatten(&s.active().unwrap().doc).into_px() } /// Direct execution, native playback and strict binary ATN playback must agree exactly. diff --git a/crates/engine/tests/it/photoshop_oracles.rs b/crates/engine/tests/it/photoshop_oracles.rs index 88e63b67fc..435bab8cc1 100644 --- a/crates/engine/tests/it/photoshop_oracles.rs +++ b/crates/engine/tests/it/photoshop_oracles.rs @@ -124,7 +124,7 @@ fn photoshop_oracles_rerendered() { } }; let Ok(merged) = photocraft_io::merged_composite(&file) else { continue }; - let ours = photocraft_compose::flatten(&doc).px; + let ours = photocraft_compose::flatten(&doc).into_px(); let (m, bad) = max_diff(&ours, &merged); let group = name.split(['/', '\\']).next().unwrap_or(".").to_string(); let g = groups.entry(group).or_default(); diff --git a/crates/engine/tests/scratch_release.rs b/crates/engine/tests/scratch_release.rs new file mode 100644 index 0000000000..6976f0cb7f --- /dev/null +++ b/crates/engine/tests/scratch_release.rs @@ -0,0 +1,114 @@ +//! One integration-test process keeps the process-wide scratch pool isolated from parallel unit +//! tests (#10479). +use photocraft_engine::Session; +use photocraft_engine::jobs::Started; +use photocraft_raster::scratch; +use serde_json::json; +use std::sync::{Mutex, PoisonError}; + +static SERIAL: Mutex<()> = Mutex::new(()); + +/// Holds the test lock with the pool's idle release off, so a slow runner can't empty the pool +/// between a command and the check after it; the release comes back on when dropped. +struct Quiet { + _serial: std::sync::MutexGuard<'static, ()>, +} + +impl Quiet { + /// Locks the test process and disables idle pool release for this guard's lifetime. + fn new() -> Self { + let _serial = SERIAL.lock().unwrap_or_else(PoisonError::into_inner); + scratch::set_idle_release(false); + Quiet { _serial } + } +} + +impl Drop for Quiet { + fn drop(&mut self) { + scratch::set_idle_release(true); + } +} + +#[test] +/// Verifies closing one or all documents releases their scratch buffers. +fn closing_a_document_leaves_none_of_its_buffers_in_the_pool() { + let _q = Quiet::new(); + for close in ["file.close", "file.closeAll"] { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 1024, "height": 1024})).unwrap(); + s.execute("edit.fill", json!({"contents": "color", "color": "#808080"})).unwrap(); + s.execute("filter.blur.gaussianBlur", json!({"radius": 3})).unwrap(); + // Dispatch holds the active document across the command; its tiles must not land in + // the pool after the release. + s.execute(close, json!({})).unwrap(); + assert_eq!(scratch::retained_bytes(), 0, "{close}"); + } +} + +#[test] +/// Verifies a command that keeps its document does not release unrelated pooled buffers. +fn commands_that_keep_their_documents_keep_the_pool() { + let _q = Quiet::new(); + let mut s = Session::new(); + s.execute("file.new", json!({"width": 1024, "height": 1024})).unwrap(); + scratch::give(vec![0u8; scratch::MIN_POOLED_BYTES]); + s.execute("edit.fill", json!({"contents": "color", "color": "#808080"})).unwrap(); + assert!(scratch::retained_bytes() > 0); +} + +#[test] +/// Verifies a cancelled worker releases its document buffers after it exits. +fn a_cancelled_job_of_a_closed_document_returns_its_buffers_once_it_exits() { + let _q = Quiet::new(); + let mut s = Session::new(); + s.execute("file.new", json!({"width": 2048, "height": 2048})).unwrap(); + s.execute("edit.fill", json!({"contents": "color", "color": "#808080"})).unwrap(); + let Started::Job(_) = s.start("filter.blur.gaussianBlur", json!({"radius": 60})).unwrap() else { + panic!("Gaussian Blur runs as a background job"); + }; + // Closing cancels the job, but its worker still holds the document until it exits. + s.close(0); + scratch::give(vec![0u8; scratch::MIN_POOLED_BYTES]); + s.join_cancelled_jobs(); + assert_eq!(scratch::retained_bytes(), 0); +} + +#[test] +/// Verifies cancelling a job before document close still releases its buffers. +fn a_job_cancelled_before_its_document_closes_still_returns_its_buffers() { + let _q = Quiet::new(); + let mut s = Session::new(); + s.execute("file.new", json!({"width": 2048, "height": 2048})).unwrap(); + s.execute("edit.fill", json!({"contents": "color", "color": "#808080"})).unwrap(); + let Started::Job(id) = s.start("filter.blur.gaussianBlur", json!({"radius": 60})).unwrap() else { + panic!("Gaussian Blur runs as a background job"); + }; + // Cancelled first: the worker is already off the running list when the document closes. + assert!(s.cancel_job(id)); + s.close(0); + scratch::give(vec![0u8; scratch::MIN_POOLED_BYTES]); + s.join_cancelled_jobs(); + assert_eq!(scratch::retained_bytes(), 0); +} + +/// Verifies polling alone releases buffers after a closed document's cancelled worker exits. +#[test] +fn a_cancelled_job_of_a_closed_document_is_released_by_polling() { + let _q = Quiet::new(); + let mut s = Session::new(); + s.execute("file.new", json!({"width": 2048, "height": 2048})).unwrap(); + s.execute("edit.fill", json!({"contents": "color", "color": "#808080"})).unwrap(); + let Started::Job(_) = s.start("filter.blur.gaussianBlur", json!({"radius": 60})).unwrap() else { + panic!("Gaussian Blur runs as a background job"); + }; + s.close(0); + // Seed after close so the close path cannot satisfy the assertion. + scratch::give(vec![0u8; scratch::MIN_POOLED_BYTES]); + assert!(scratch::retained_bytes() > 0); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + while scratch::retained_bytes() > 0 && std::time::Instant::now() < deadline { + s.poll_jobs(); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert_eq!(scratch::retained_bytes(), 0); +} diff --git a/crates/format/src/store.rs b/crates/format/src/store.rs index af6678088b..67bb8df27b 100644 --- a/crates/format/src/store.rs +++ b/crates/format/src/store.rs @@ -30,8 +30,87 @@ fn blob_path(h: &str) -> String { format!("blobs/{h}.zst") } -fn compress(data: &[u8]) -> Vec { - ruzstd::encoding::compress_to_vec(data, ruzstd::encoding::CompressionLevel::Fastest) +/// The bytes an [`Object`] stores, owned by (or shared with) the compressor's source so one +/// [`Compressor`] can be reused for objects of any lifetime. +enum Source { + Tile(Arc), + Blob(Arc>), + /// A little-endian copy of a tile (big-endian hosts only). + Owned(Vec), +} + +struct SourceReader { + source: Source, + pos: usize, +} + +impl SourceReader { + fn all(&self) -> &[u8] { + match &self.source { + Source::Tile(t) => t.bytes(), + Source::Blob(b) => b.as_slice(), + Source::Owned(v) => v.as_slice(), + } + } + + fn len(&self) -> usize { + self.all().len() + } +} + +impl Read for SourceReader { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + let all = self.all(); + let rest = all.get(self.pos..).unwrap_or(&[]); + let n = rest.len().min(buf.len()); + if let (Some(dst), Some(src)) = (buf.get_mut(..n), rest.get(..n)) { + dst.copy_from_slice(src); + } + self.pos += n; + Ok(n) + } +} + +/// A reusable zstd compressor. `ruzstd` allocates a block buffer, a ~1 MiB suffix table and a +/// block output buffer per frame; keeping the [`FrameCompressor`](ruzstd::encoding::FrameCompressor) +/// between objects reuses the first two (its matcher resets itself on every `compress`). +/// Output is byte-identical to `compress_to_vec` at the same level. +struct Compressor { + inner: ruzstd::encoding::FrameCompressor, ruzstd::encoding::MatchGeneratorDriver>, +} + +impl Compressor { + fn new() -> Self { + Self { inner: ruzstd::encoding::FrameCompressor::new(ruzstd::encoding::CompressionLevel::Fastest) } + } + + fn run(&mut self, source: Source) -> Vec { + let reader = SourceReader { source, pos: 0 }; + // Room for the worst case (raw blocks: 3 bytes per 128 KiB, plus frame header and + // checksum), so the output never regrows; shrunk afterwards to what it used. Untouched + // pages cost nothing. + let len = reader.len(); + let mut drain = Vec::new(); + if drain.try_reserve_exact(len.saturating_add(len / 16384).saturating_add(64)).is_err() { + drain = Vec::new(); + } + self.inner.set_source(reader); + self.inner.set_drain(drain); + self.inner.compress(); + self.inner.take_source(); + let mut out = self.inner.take_drain().unwrap_or_default(); + out.shrink_to_fit(); + out + } + + fn object(&mut self, obj: &Object) -> Vec { + let source = match obj { + Object::Tile(tile, _) if cfg!(target_endian = "little") => Source::Tile(Arc::clone(tile)), + Object::Tile(tile, sample) => Source::Owned(tile_le(tile, *sample).into_owned()), + Object::Blob(blob) => Source::Blob(Arc::clone(blob)), + }; + self.run(source) + } } /// Decodes a zstd frame of at most `max` bytes (`LimitExceeded` beyond). With `exact`, the @@ -528,8 +607,8 @@ enum Object { } impl Object { - fn compressed(&self) -> Vec { - compress(&self.content()) + fn compressed(&self, compressor: &mut Compressor) -> Vec { + compressor.object(self) } /// The bytes this object stores (tiles little-endian). @@ -860,7 +939,8 @@ type Compressed<'a> = (&'a String, Arc>); fn par_compress<'a>(objects: &[(&'a String, &'a Object)]) -> Result>> { let threads = if cfg!(target_arch = "wasm32") { 1 } else { std::thread::available_parallelism().map_or(1, |n| n.get()).clamp(1, 32) }; if threads < 2 || objects.len() < 2 { - return Ok(objects.iter().map(|(p, o)| (*p, Arc::new(o.compressed()))).collect()); + let mut compressor = Compressor::new(); + return Ok(objects.iter().map(|(p, o)| (*p, Arc::new(o.compressed(&mut compressor)))).collect()); } let next = std::sync::atomic::AtomicUsize::new(0); std::thread::scope(|sc| { @@ -868,10 +948,11 @@ fn par_compress<'a>(objects: &[(&'a String, &'a Object)]) -> Result = None; for (path, obj) in &p.objects { let (key, data) = match self.compressed.remove_entry(path) { Some((key, data)) => { @@ -989,7 +1071,7 @@ impl PcraftWriter { Object::Tile(..) => stats.tiles_written += 1, Object::Blob(_) => stats.blobs_written += 1, } - (None, fresh.remove(path).unwrap_or_else(|| Arc::new(obj.compressed()))) + (None, fresh.remove(path).unwrap_or_else(|| Arc::new(obj.compressed(compressor.get_or_insert_with(Compressor::new))))) } }; z.add(path, &data)?; @@ -1036,6 +1118,7 @@ impl PcraftWriter { None => 0, }; let recheck = self.rolling_recheck(&p.objects); + let mut compressor: Option = None; for (path, obj) in &p.objects { let object_path = dir.join(path); // bundle_root and list_objects validated the fixed parent directories once. Check @@ -1070,7 +1153,7 @@ impl PcraftWriter { Object::Tile(..) => stats.tiles_written += 1, Object::Blob(_) => stats.blobs_written += 1, } - write_bundle_file(dir, &object_path, &obj.compressed(), fs)?; + write_bundle_file(dir, &object_path, &obj.compressed(compressor.get_or_insert_with(Compressor::new)), fs)?; let written = path_signature(&object_path)?; if let Some(cache) = &mut self.verified_directory { match written { @@ -1421,6 +1504,38 @@ pub(crate) fn write_atomic(path: &Path, data: &[u8]) -> Result<()> { #[cfg(all(test, windows))] mod durability_tests; +#[cfg(test)] +#[test] +fn reused_compressor_matches_compress_to_vec_and_round_trips() { + let mut state = 0x9e37_79b9_u32; + let mut noise = |n: usize| -> Vec { + (0..n) + .map(|_| { + state = state.wrapping_mul(1_664_525).wrapping_add(1_013_904_223); + (state >> 24) as u8 + }) + .collect() + }; + let inputs: Vec> = vec![ + Vec::new(), + vec![7], + b"hello hello hello hello hello".to_vec(), + noise(1000), + (0..300_000u32).map(|i| (i / 7 % 251) as u8).collect(), + noise(300_000), + vec![0; 262_144], + // A repeat after a big one proves the matcher state is reset between objects. + b"hello hello hello hello hello".to_vec(), + ]; + let mut compressor = Compressor::new(); + for data in inputs { + let expected = ruzstd::encoding::compress_to_vec(data.as_slice(), ruzstd::encoding::CompressionLevel::Fastest); + let got = compressor.object(&Object::Blob(Arc::new(data.clone()))); + assert_eq!(got, expected, "len {}", data.len()); + assert_eq!(decompress(&got, data.len(), "test").unwrap(), data); + } +} + #[cfg(test)] #[test] fn an_incompressible_blob_at_its_limit_still_loads() { @@ -1442,7 +1557,7 @@ fn an_incompressible_blob_at_its_limit_still_loads() { x as u8 }) .collect(); - let z = compress(&data); + let z = Compressor::new().object(&Object::Blob(Arc::new(data.clone()))); assert!(z.len() > data.len(), "the fixture must not compress"); let hash = hash_bytes(&data); let src = One(blob_path(&hash), z); diff --git a/crates/gpu/src/convert.rs b/crates/gpu/src/convert.rs index 9dbfc81814..19ad194765 100644 --- a/crates/gpu/src/convert.rs +++ b/crates/gpu/src/convert.rs @@ -1,6 +1,7 @@ use super::batch::write_tile; use super::textures::{Resident, TexKind}; use super::{Arc, F, HashMap, I, PixelFormat, Rect, SampleType, Surface, TILE_SIZE, Tile, TileCoord, U, fx}; +use photocraft_raster::scratch; /// Distinct tiles converted per parallel batch: bounds the converted bytes held at once (64 tiles /// of a 16-bit page are 32 MiB). @@ -141,6 +142,8 @@ pub(crate) fn convert_tiles( _ => put(t, cs, &bytes.concat()), } } + // The texels are on their way to the GPU: the buffers serve the next batch. + parts.into_iter().for_each(scratch::give); } groups.len() } @@ -157,7 +160,9 @@ pub(crate) fn colour_managed(fmt: &PixelFormat, kind: TexKind) -> bool { /// Tile pixels in the texture's format. `tile = None` gives the default pixel everywhere. pub(crate) fn convert_tile(surface: &Surface, tile: Option<&Arc>, kind: TexKind, c: TileCoord) -> Vec { if let (Some(t), TexKind::Rgba8Direct | TexKind::R8Direct) = (tile, kind) { - return t.bytes().to_vec(); + let mut out = scratch::take_cap(t.bytes().len()); + out.extend_from_slice(t.bytes()); + return out; } let fmt = surface.format(); if let (Some(t), TexKind::Rgba16F) = (tile, kind) @@ -179,7 +184,7 @@ fn convert_rows(surface: &Surface, tile: Option<&Arc>, kind: TexKind, rect Some(_) => surface.read_region(rect), None => surface.default_pixel().repeat(n), }; - let mut out = Vec::with_capacity(n * kind.bytes_per_pixel()); + let mut out = scratch::take_cap(n * kind.bytes_per_pixel()); // The previous pixel and its colour. let mut last: Option<(usize, [f32; 4])> = None; for (i, px) in raw.chunks_exact(ch).enumerate() { @@ -209,6 +214,7 @@ fn convert_rows(surface: &Surface, tile: Option<&Arc>, kind: TexKind, rect TexKind::R32F => out.extend(px[0].to_le_bytes()), } } + scratch::give(raw); out } @@ -229,7 +235,7 @@ pub(crate) fn rgb_gray16_to_f16(fmt: &PixelFormat, bytes: &[u8]) -> Option [u8; 2] { let v = px.get(i * 2..i * 2 + 2).map_or(0, |b| u16::from_ne_bytes([b[0], b[1]])); @@ -261,7 +267,7 @@ pub(crate) fn rgb_gray32_to_f16(fmt: &PixelFormat, bytes: &[u8]) -> Option [u8; 2] { let v = px.get(i * 4..i * 4 + 4).map_or(0.0, |b| f32::from_ne_bytes([b[0], b[1], b[2], b[3]])); diff --git a/crates/gpu/src/fx.rs b/crates/gpu/src/fx.rs index ceff607686..972ccadbea 100644 --- a/crates/gpu/src/fx.rs +++ b/crates/gpu/src/fx.rs @@ -427,12 +427,20 @@ fn fields_in_windows( }) } -/// The layer's shape (`compose::layer_shape`) over `r`, in parallel bands. +/// The layer's shape (`compose::layer_shape`) over `r`, in parallel bands. Callers hand the +/// result back with `photocraft_raster::scratch::give` once they have read it. pub(crate) fn shape(doc: &photocraft_doc::Document, layer: &Layer, r: Rect) -> Vec { if r.is_empty() { return Vec::new(); } - par_map(bands(r, BAND), |band| photocraft_compose::layer_shape(doc, layer, band)).concat() + let parts = par_map(bands(r, BAND), |band| photocraft_compose::layer_shape(doc, layer, band)); + let mut shape = photocraft_raster::scratch::take_cap(parts.iter().map(Vec::len).sum()); + // A band can be a pooled mask (Transparency Shapes Layer off): hand each back once copied. + for part in parts { + shape.extend_from_slice(&part); + photocraft_raster::scratch::give(part); + } + shape } /// Write `v` (row-major over `r`) into the region-sized `shape`. diff --git a/crates/gpu/src/fx_passes.rs b/crates/gpu/src/fx_passes.rs index f3f5ce6da3..9472141066 100644 --- a/crates/gpu/src/fx_passes.rs +++ b/crates/gpu/src/fx_passes.rs @@ -60,6 +60,7 @@ impl Compositor { e.region = region; rebuild = false; } + photocraft_raster::scratch::give(v); } if rebuild { let shape = Tex::map(device, "pc_fx_shape", region, MAP32); @@ -108,6 +109,7 @@ impl Compositor { fx::paste(&mut e.shape_cpu, region, damage, &v); e.shape.write_r32(queue, region, damage, &v); staged += v.len() as u64 * 4; + photocraft_raster::scratch::give(v); } // Programs, and the distance fields they read (max reach per field). diff --git a/crates/gpu/tests/it/parity.rs b/crates/gpu/tests/it/parity.rs index d22a4515d1..e8ab0b91c4 100644 --- a/crates/gpu/tests/it/parity.rs +++ b/crates/gpu/tests/it/parity.rs @@ -957,7 +957,7 @@ fn hard_stop_gradients_match_cpu_or_fall_back() { Ok(out) => out, Err(e) => { assert!(e.0.contains("coincident gradient stops"), "{e}"); - photocraft_compose::render(&d, d.bounds()).px + photocraft_compose::render(&d, d.bounds()).into_px() } }; let worst = worst_diff(&cpu.px, &out); diff --git a/crates/io/examples/oracle_diff.rs b/crates/io/examples/oracle_diff.rs index cc4b5aa0de..13977e1a61 100644 --- a/crates/io/examples/oracle_diff.rs +++ b/crates/io/examples/oracle_diff.rs @@ -34,7 +34,7 @@ fn run_file(path: &str) { println!("mode {:?} depth {:?} layers {} warnings {:?} light {:?}", doc.mode, doc.depth, doc.layer_count(), imp.warnings, doc.global_light); print_layer_details(doc); dump_filter_effects(&file); - let ours = photocraft_compose::flatten(doc).px; + let ours = photocraft_compose::flatten(doc).into_px(); let merged = photocraft_io::merged_composite(&file).unwrap(); compare_modes(doc, &ours, &merged); } @@ -487,7 +487,7 @@ fn corpus_summary(root: &std::path::Path) { return (name, None); } let Ok(merged) = photocraft_io::merged_composite(&file) else { return (name, None) }; - let ours = photocraft_compose::flatten(&imp.document).px; + let ours = photocraft_compose::flatten(&imp.document).into_px(); let mut m = 0.0f32; let mut bad = 0usize; for (a, b) in ours.iter().zip(merged) { diff --git a/crates/io/src/flat.rs b/crates/io/src/flat.rs index 8e169bd556..9863bdf9b0 100644 --- a/crates/io/src/flat.rs +++ b/crates/io/src/flat.rs @@ -452,9 +452,16 @@ pub(crate) fn image_surface(img: &Image, target_layout: ChannelLayout, csample: let band = (BAND_BYTES / row.max(1)).max(1); for (i, rows) in img.data().chunks(row.max(1) * band).enumerate() { let n = (rows.len() / row.max(1)) as u32; - let part = Image::from_raw(w, n, img.layout(), img.sample_type(), rows.to_vec())?.convert(target_layout, csample); + // Band buffers cycle through the scratch pool: at this size each fresh one is an + // mmap that gets page-faulted in. + let mut src = photocraft_raster::scratch::take_cap::(rows.len()); + src.extend_from_slice(rows); + let src = Image::from_raw(w, n, img.layout(), img.sample_type(), src)?; + let part = src.convert(target_layout, csample); let y0 = (i * band) as i32; s.write_interleaved(Rect::new(0, y0, w as i32, y0 + n as i32), part.data()); + photocraft_raster::scratch::give(src.into_data()); + photocraft_raster::scratch::give(part.into_data()); } } Ok(s) diff --git a/crates/io/src/lib.rs b/crates/io/src/lib.rs index b3135df1ad..d9bb69af2c 100644 --- a/crates/io/src/lib.rs +++ b/crates/io/src/lib.rs @@ -765,14 +765,14 @@ pub fn merged_composite(file: &PsdFile) -> Result, IoError> { let (doc, _) = psd_to_document(&PsdFile { layer_info, ..file.clone() }); // Multichannel documents keep their channels apart (no layer): composite them. if doc.layers.is_empty() && doc.mode == photocraft_color::ColorMode::Multichannel { - return Ok(photocraft_compose::flatten(&doc).px); + return Ok(photocraft_compose::flatten(&doc).into_px()); } let l = doc.layers.first().ok_or_else(|| IoError::Unsupported("no merged image".into()))?; let s = l.surface().ok_or_else(|| IoError::Unsupported("no merged image".into()))?; // CMYK reads through the file's own profile, as the compositor reads its layers, so a tagged // file's merged image and its composite are compared in the same colours. let cmyk = photocraft_compose::cmyk_space(&doc); - Ok(photocraft_color::convert::with_cmyk_space(cmyk.as_ref(), || photocraft_compose::surface_to_buffer(s, doc.bounds()).px)) + Ok(photocraft_color::convert::with_cmyk_space(cmyk.as_ref(), || photocraft_compose::surface_to_buffer(s, doc.bounds()).into_px())) } #[cfg(test)] diff --git a/crates/io/src/ora.rs b/crates/io/src/ora.rs index fb7968fbcc..36ebbc7b0a 100644 --- a/crates/io/src/ora.rs +++ b/crates/io/src/ora.rs @@ -629,7 +629,7 @@ impl Writer<'_> { LayerContent::Raster(s) => { let r = s.content_bounds(); admit_layer(r)?; - (r, photocraft_compose::surface_to_buffer(s, r).px) + (r, photocraft_compose::surface_to_buffer(s, r).into_px()) } _ => { // Rendered as it shows on its own, before the layer's blending options. @@ -643,7 +643,7 @@ impl Writer<'_> { solo.effects = Default::default(); let r = photocraft_compose::composite_bounds(&solo, canvas).unwrap_or(canvas).intersect(&canvas); admit_layer(r)?; - (r, photocraft_compose::render_layer(&solo, r).px) + (r, photocraft_compose::render_layer(&solo, r).into_px()) } }; if let Some(mask) = layer.mask.as_ref().filter(|m| m.enabled) { diff --git a/crates/io/src/pattern_map.rs b/crates/io/src/pattern_map.rs index 97417b7cc7..0b949a4bdf 100644 --- a/crates/io/src/pattern_map.rs +++ b/crates/io/src/pattern_map.rs @@ -120,7 +120,9 @@ fn pattern_from_psd(p: &PsdPattern, oom: &mut Option) -> Option crate::pixels::lab16_chroma(&mut bytes, refs.len(), true); } let mut surface = Surface::new(fmt); - if let Err(error) = surface.try_write_interleaved(Rect::new(0, 0, w as i32, h as i32), &bytes) { + let result = surface.try_write_interleaved(Rect::new(0, 0, w as i32, h as i32), &bytes); + photocraft_raster::scratch::give(bytes); + if let Err(error) = result { *oom = Some(error.to_string()); return None; } diff --git a/crates/io/src/pixels.rs b/crates/io/src/pixels.rs index 1ac715984f..63c6742a1c 100644 --- a/crates/io/src/pixels.rs +++ b/crates/io/src/pixels.rs @@ -47,7 +47,8 @@ fn invert_sample(bytes: &mut [u8], s: SampleType) { /// are filled with `fill[c]` (a native-endian encoded sample). Channels whose /// `invert[c]` is set are inverted (PSD stores CMYK as `max - ink`). pub fn interleave(planes: &[Option<&[u8]>], fill: &[Vec], n: usize, s: SampleType, invert: &[bool]) -> Vec { - let mut out = vec![0u8; n * planes.len() * s.bytes()]; + // Large results come from (and callers may return them to) the scratch pool. + let mut out = photocraft_raster::scratch::take(n * planes.len() * s.bytes(), 0u8); interleave_into(planes, fill, n, s, invert, &mut out); out } @@ -69,9 +70,7 @@ pub fn try_interleave(planes: &[Option<&[u8]>], fill: &[Vec], n: usize, s: S return Err(PsdError::Invalid("interleave fill has the wrong sample size".into())); } } - let mut out = Vec::new(); - out.try_reserve_exact(len).map_err(|_| PsdError::OutOfMemory("not enough memory to interleave PSD pixels"))?; - out.resize(len, 0); + let mut out = photocraft_raster::scratch::try_take_exact(len, 0u8).ok_or(PsdError::OutOfMemory("not enough memory to interleave PSD pixels"))?; interleave_into(planes, fill, n, s, invert, &mut out); Ok(out) } diff --git a/crates/io/src/psd_export.rs b/crates/io/src/psd_export.rs index 315744b354..6086aaf644 100644 --- a/crates/io/src/psd_export.rs +++ b/crates/io/src/psd_export.rs @@ -223,6 +223,8 @@ impl Ex { let mut bytes = Vec::with_capacity(row_len * h_of(r) * s.format().bytes_per_pixel()); for part in parts { bytes.extend_from_slice(&part); + // Free gathered bands before allocating channel planes below; pooling them here + // retains another layer-sized allocation throughout deinterleave. } if self.fmt.mode == ColorMode::Lab && self.fmt.sample == SampleType::U16 { crate::pixels::lab16_chroma(&mut bytes, self.cc + 1, false); diff --git a/crates/io/src/psd_import.rs b/crates/io/src/psd_import.rs index 6989941d31..a70577fb6f 100644 --- a/crates/io/src/psd_import.rs +++ b/crates/io/src/psd_import.rs @@ -328,6 +328,12 @@ impl Ctx<'_> { self.allocation_failed(format!("layer \"{name}\": {error}")); return Surface::new(self.fmt); } + // The interleaved band goes back to the scratch pool for the next band (and the next + // import): it is a fresh page-faulted mmap otherwise. The decoded planes don't: the + // decoder allocates its own, so pooled ones would only sit there. + drop(refs); + photocraft_raster::scratch::give(bytes); + drop(planes); } surf.prune(); // A white background, a solid fill's cache: one tile per value, shared across layers. @@ -1408,6 +1414,7 @@ fn import_layered(import: DecodedImport<'_>, doc: &mut Document, cx: &mut Ctx<'_ if let Err(error) = surface.try_write_interleaved(canvas, &bytes) { return cx.allocation_failed(format!("merged image: {error}")); } + photocraft_raster::scratch::give(bytes); if alpha_idx.is_some() { let white = photocraft_raster::from_rgba(&fmt, [1.0, 1.0, 1.0, 1.0]); let mut vals = Vec::new(); @@ -1547,6 +1554,7 @@ fn import_extra_channels(import: DecodedImport<'_>, doc: &mut Document, cx: &mut if let Err(error) = s.try_write_interleaved(canvas, &raw) { return cx.allocation_failed(format!("extra channel {idx}: {error}")); } + photocraft_raster::scratch::give(raw); s.prune(); let name = names.get(k).cloned().unwrap_or_else(|| format!("Alpha {}", k + 1)); doc.channels.push(AlphaChannel::new(name, s)); diff --git a/crates/io/src/smart_map.rs b/crates/io/src/smart_map.rs index 72adbe1291..cacce2b7cb 100644 --- a/crates/io/src/smart_map.rs +++ b/crates/io/src/smart_map.rs @@ -173,6 +173,7 @@ pub fn mask_from_item(item: &FilterEffectsItem, sample: SampleType, stack: &Filt let n = w.checked_mul(h).ok_or("the smart filter mask is too large")?; let bytes = crate::pixels::interleave(&[Some(&plane)], &[crate::pixels::zero_sample(sample)], n, sample, &[false]); surface.write_interleaved(GeomRect::new(rect.left, rect.top, rect.right, rect.bottom), &bytes); + photocraft_raster::scratch::give(bytes); surface.prune(); if surface.content_bounds().is_empty() { return Ok(None); @@ -215,6 +216,7 @@ pub fn unfiltered_from_item(item: &FilterEffectsItem, fmt: PixelFormat) -> Optio } let mut surface = Surface::new(fmt); surface.write_interleaved(GeomRect::new(item.rect.left, item.rect.top, item.rect.right, item.rect.bottom), &bytes); + photocraft_raster::scratch::give(bytes); surface.prune(); Some(surface) } diff --git a/crates/io/tests/it/banded.rs b/crates/io/tests/it/banded.rs index eb8e97d6b5..c526b79fdb 100644 --- a/crates/io/tests/it/banded.rs +++ b/crates/io/tests/it/banded.rs @@ -82,7 +82,7 @@ fn psd_merged_image_matches_composite_and_drops_near_opaque_alpha() { let file = document_to_psd(&d); assert_eq!(file.merged_has_alpha(), alpha < 0.99, "alpha {alpha}"); let merged = merged_composite(&file).unwrap(); - let want = photocraft_compose::flatten(&d).px; + let want = photocraft_compose::flatten(&d).into_px(); let m = merged.iter().zip(&want).flat_map(|(a, b)| (0..4).map(move |i| (a[i] - b[i]).abs())).fold(0.0f32, f32::max); // Un-matting divides the 8-bit rounding error by alpha. assert!(m <= 1.0 / (255.0 * alpha) + 1e-4, "alpha {alpha}: max diff {m}"); diff --git a/crates/io/tests/it/blend_modes_corpus.rs b/crates/io/tests/it/blend_modes_corpus.rs index 296e2e2a50..72980bed5b 100644 --- a/crates/io/tests/it/blend_modes_corpus.rs +++ b/crates/io/tests/it/blend_modes_corpus.rs @@ -18,7 +18,7 @@ fn cell_error(mode: BlendMode) -> (f32, f32) { let doc = import("rgb-blend-modes.psd", &bytes).expect("opens").document; let file = PsdFile::from_bytes(&bytes).expect("parses"); let merged = merged_composite(&file).expect("merged image"); - let ours = photocraft_compose::flatten(&doc).px; + let ours = photocraft_compose::flatten(&doc).into_px(); let width = doc.size.width as i32; let duck = doc.walk().into_iter().map(|(_, _, l)| l).find(|l| l.name == "rubber_duck_PNG1" && l.blend == mode).expect("the mode's duck"); let surface = duck.surface().expect("duck pixels"); diff --git a/crates/io/tests/it/cmyk_composite_export.rs b/crates/io/tests/it/cmyk_composite_export.rs index 0fea33695d..77f35a62d2 100644 --- a/crates/io/tests/it/cmyk_composite_export.rs +++ b/crates/io/tests/it/cmyk_composite_export.rs @@ -313,7 +313,8 @@ fn rgb_only_export_uses_direct_composite_not_reseparated_cmyk() { let doc = document(SampleType::U16, 0.5); let rgb = photocraft_compose::flatten(&doc); let expected = - photocraft_codecs::Image::from_normalized(8, 8, ChannelLayout::Rgba, CodecSample::U16, &rgb.px.into_iter().flatten().collect::>()).unwrap(); + photocraft_codecs::Image::from_normalized(8, 8, ChannelLayout::Rgba, CodecSample::U16, &rgb.into_px().into_iter().flatten().collect::>()) + .unwrap(); let result = export(&doc, "png", &ExportOptions::default()).unwrap(); diff --git a/crates/io/tests/it/color.rs b/crates/io/tests/it/color.rs index 15453a2518..5da5df1c7e 100644 --- a/crates/io/tests/it/color.rs +++ b/crates/io/tests/it/color.rs @@ -116,11 +116,11 @@ fn cmyk_fills_survive_psd_roundtrip_at_every_depth() { let mut doc = Document::new("CMYK fill", photocraft_geom::Size::new(9, 6), ColorMode::Cmyk, depth); doc.icc_profile = profile.clone(); doc.layers.push(Layer::new("Fill", LayerContent::Fill(fill))); - let expected = photocraft_compose::flatten(&doc).px; + let expected = photocraft_compose::flatten(&doc).into_px(); assert!(max_diff(&expected, &photocraft_compose::render_tiled(&doc, doc.bounds(), 3).px) <= 1e-6); let bytes = export(&doc, "x.psd", &ExportOptions::default()).unwrap().bytes; let back = import("x.psd", &bytes).unwrap().document; - let actual = photocraft_compose::flatten(&back).px; + let actual = photocraft_compose::flatten(&back).into_px(); let diff = max_diff(&expected, &actual); assert!(diff <= 1e-6, "{depth:?} {style:?}: round-trip error {diff}"); assert_eq!(back.icc_profile, doc.icc_profile); @@ -144,7 +144,7 @@ fn uncached_cmyk_gradient_corpus_roundtrip() { let bytes = std::fs::read(path).expect("run cargo xtask corpus --all"); let doc = import("x.psd", &bytes).unwrap().document; assert!(doc.layers.iter().any(|l| matches!(l.content, photocraft_doc::LayerContent::Fill(_)) && l.fill_cache.is_none())); - let expected = photocraft_compose::flatten(&doc).px; + let expected = photocraft_compose::flatten(&doc).into_px(); let saved = export(&doc, "x.psd", &ExportOptions::default()).unwrap().bytes; let back = import("x.psd", &saved).unwrap().document; let diff = max_diff(&expected, &photocraft_compose::flatten(&back).px); @@ -305,7 +305,7 @@ fn gradient_maps_with_cmyk_lab_gray_stops_import_and_keep_their_space() { _ => assert_eq!((first, last), ([1.0; 3], [0.0; 3])), } // The layer now renders: the composite differs from the background alone. - let with_map = photocraft_compose::flatten(&back).px; + let with_map = photocraft_compose::flatten(&back).into_px(); let mut hidden = back.clone(); hidden.layers[1].visible = false; assert!(max_diff(&with_map, &photocraft_compose::flatten(&hidden).px) > 0.01, "{mode:?} {depth:?}"); diff --git a/crates/io/tests/it/composite.rs b/crates/io/tests/it/composite.rs index cef80e6a08..74eb0c95f1 100644 --- a/crates/io/tests/it/composite.rs +++ b/crates/io/tests/it/composite.rs @@ -15,7 +15,7 @@ fn check_export_oracle(d: &photocraft_doc::Document, tol: f32) { let f = PsdFile::from_bytes(&r.bytes).unwrap(); let merged = merged_composite(&f).unwrap(); let d2 = import("x.psd", &r.bytes).unwrap().document; - let mut ours = photocraft_compose::flatten(&d2).px; + let mut ours = photocraft_compose::flatten(&d2).into_px(); if d.mode == ColorMode::Cmyk { // The merged CMYK image is the RGB composite separated through the CMYK profile, which // gamut-maps colours that blend modes push outside CMYK: project ours the same way. @@ -107,7 +107,7 @@ fn builder_oracle(depth: u16, mode: ColorMode) { let f = PsdFile::from_bytes(&bytes).unwrap(); let merged = merged_composite(&f).unwrap(); let d2 = import("b.psd", &bytes).unwrap().document; - let ours = photocraft_compose::flatten(&d2).px; + let ours = photocraft_compose::flatten(&d2).into_px(); let m = max_diff(&ours, &merged); // Density/feather of masks are not written by the builder; the generated // doc uses them only on a disabled mask, so results must match. @@ -157,7 +157,7 @@ fn hidden_and_clipped_affect_composite() { for l in &mut d2.layers { l.visible = true; } - let other = photocraft_compose::flatten(&d2).px; + let other = photocraft_compose::flatten(&d2).into_px(); assert!(max_diff(&other, &merged) > 0.0, "unhiding a layer must change the composite"); } diff --git a/crates/io/tests/it/corpus.rs b/crates/io/tests/it/corpus.rs index 182d60095a..3c8ca3288a 100644 --- a/crates/io/tests/it/corpus.rs +++ b/crates/io/tests/it/corpus.rs @@ -316,9 +316,9 @@ fn check_file(name: &str, bytes: &[u8]) -> (Outcome, Option) { return (Outcome::Error, None); } }; - let ours = photocraft_compose::flatten(doc).px; + let ours = photocraft_compose::flatten(doc).into_px(); // Our own export must not change how the document renders. - let again = photocraft_compose::flatten(&reimported).px; + let again = photocraft_compose::flatten(&reimported).into_px(); let rt = if again.len() == ours.len() { common::max_diff(&ours, &again) } else { f32::INFINITY }; let layers = doc.layer_count(); // Oracle choice: the merged image (Photoshop's composite; for files without layers it is the diff --git a/crates/io/tests/it/effects_multi.rs b/crates/io/tests/it/effects_multi.rs index 637e926085..7cabb04b4c 100644 --- a/crates/io/tests/it/effects_multi.rs +++ b/crates/io/tests/it/effects_multi.rs @@ -17,7 +17,7 @@ fn roundtrip(doc: &Document) -> Document { } fn max_diff(a: &Document, b: &Document) -> f32 { - let (x, y) = (photocraft_compose::flatten(a).px, photocraft_compose::flatten(b).px); + let (x, y) = (photocraft_compose::flatten(a).into_px(), photocraft_compose::flatten(b).into_px()); assert_eq!(x.len(), y.len()); x.iter().zip(&y).flat_map(|(p, q)| (0..4).map(move |c| (p[c] * p[3] - q[c] * q[3]).abs())).fold(0.0, f32::max) } diff --git a/crates/io/tests/it/flat.rs b/crates/io/tests/it/flat.rs index f6e888eb06..c40c76a12b 100644 --- a/crates/io/tests/it/flat.rs +++ b/crates/io/tests/it/flat.rs @@ -350,7 +350,7 @@ fn layered_to_png_warns_flatten() { assert!(r.warnings.iter().any(|w| w.contains("flattened")), "{:?}", r.warnings); let back = import("out.png", &r.bytes).unwrap().document; // Composite equals our flatten. - let flat = photocraft_compose::flatten(&d).px; + let flat = photocraft_compose::flatten(&d).into_px(); let s = back.layers[0].surface().unwrap(); let px: Vec<[f32; 4]> = s.read_region(back.bounds()).as_chunks::<4>().0.iter().map(|p| [p[0], p[1], p[2], p[3]]).collect(); assert!(max_diff(&px, &flat) <= 1.0 / 255.0 + 1e-4); diff --git a/crates/io/tests/it/lab16.rs b/crates/io/tests/it/lab16.rs index 0aee251861..ff0707c11b 100644 --- a/crates/io/tests/it/lab16.rs +++ b/crates/io/tests/it/lab16.rs @@ -60,8 +60,8 @@ fn lab_round_trips_at_every_depth() { let p = back.layers[0].surface().unwrap().pixel(2, 0); let tol = if depth == SampleType::U8 { 0.5 / 255.0 + 1e-6 } else { 2e-5 }; assert!((p[1] - A).abs() <= tol && (p[2] - B).abs() <= tol, "{depth:?}: {p:?}"); - let a = photocraft_compose::flatten(&doc).px; - let b = photocraft_compose::flatten(&back).px; + let a = photocraft_compose::flatten(&doc).into_px(); + let b = photocraft_compose::flatten(&back).into_px(); for (x, y) in a.iter().zip(&b) { for c in 0..4 { assert!((x[c] - y[c]).abs() <= 1.0 / 255.0, "{depth:?}: {x:?} vs {y:?}"); diff --git a/crates/io/tests/it/large_files.rs b/crates/io/tests/it/large_files.rs index b0a2669480..be4d5eeab7 100644 --- a/crates/io/tests/it/large_files.rs +++ b/crates/io/tests/it/large_files.rs @@ -42,7 +42,7 @@ fn check_grid(g: &Grid, doc: &Document, what: &str) { ]; assert_eq!(census(doc), want, "{what}: [raster, groups, masked, with effects, hidden]"); for (x, y) in g.sample_points() { - let px = photocraft_compose::render(doc, Rect::from_xywh(x as i32, y as i32, 1, 1)).px; + let px = photocraft_compose::render(doc, Rect::from_xywh(x as i32, y as i32, 1, 1)).into_px(); let got = px.first().copied().unwrap_or_default(); let want = g.expected(x, y); for c in 0..3 { diff --git a/crates/io/tests/it/multichannel.rs b/crates/io/tests/it/multichannel.rs index 4e7ffeeb2e..f4f7154d41 100644 --- a/crates/io/tests/it/multichannel.rs +++ b/crates/io/tests/it/multichannel.rs @@ -56,7 +56,7 @@ fn alpha_type_channels_import_as_alpha_and_round_trip() { as_ink.channels[1].surface = s; as_ink.channels[1].spot = Some((photocraft_color::Color::BLACK, 0.0)); // Within f32 rounding: the inverted copy is re-quantised to the channel depth. - let (shown, inked) = (photocraft_compose::flatten(&d).px, photocraft_compose::flatten(&as_ink).px); + let (shown, inked) = (photocraft_compose::flatten(&d).into_px(), photocraft_compose::flatten(&as_ink).into_px()); assert_eq!(shown.len(), inked.len()); for (a, b) in shown.iter().zip(&inked) { assert!(a.iter().zip(b).all(|(x, y)| (x - y).abs() < 1e-5), "{a:?} vs {b:?} ({depth}-bit)"); @@ -115,7 +115,7 @@ fn alpha_type_channels_are_stored_as_is_not_as_ink() { assert_eq!(values(&d.channels[0]), [1.0, 0.0, 1.0, 0.0], "spot channel ({depth}-bit)"); // The composite is what Photoshop shows: the alpha prints as black ink where unselected. - let px = photocraft_compose::flatten(&d).px; + let px = photocraft_compose::flatten(&d).into_px(); assert_eq!(px.iter().map(|p| [p[0], p[1], p[2]]).collect::>(), [[0.0, 1.0, 1.0], [1.0, 1.0, 1.0], [0.0, 0.0, 0.0], [0.0, 0.0, 0.0]]); // Saved back byte for byte. diff --git a/crates/io/tests/it/psd32_export.rs b/crates/io/tests/it/psd32_export.rs index f9a297a7f6..428fb7903e 100644 --- a/crates/io/tests/it/psd32_export.rs +++ b/crates/io/tests/it/psd32_export.rs @@ -118,7 +118,7 @@ fn layered_32bit_export_reimports_render_equal() { for mode in [ColorMode::Rgb, ColorMode::Grayscale] { let doc = common::gen_doc(mode, SampleType::F32, Features::ALL); let back = import("t.psd", &export_psd(&doc)).unwrap().document; - let (a, b) = (photocraft_compose::flatten(&doc).px, photocraft_compose::flatten(&back).px); + let (a, b) = (photocraft_compose::flatten(&doc).into_px(), photocraft_compose::flatten(&back).into_px()); assert_eq!(a.len(), b.len()); let d = common::max_diff(&a, &b); assert!(d <= 1e-4, "{mode:?}: render differs by {d}"); @@ -154,7 +154,7 @@ fn photoshop_32bit_corpus_round_trip() { assert_32bit_layout(&name, &out); let back = import(&name, &out).unwrap().document; assert_eq!(doc.layer_count(), back.layer_count(), "{name}: layer count"); - let (a, b) = (photocraft_compose::flatten(&doc).px, photocraft_compose::flatten(&back).px); + let (a, b) = (photocraft_compose::flatten(&doc).into_px(), photocraft_compose::flatten(&back).into_px()); assert_eq!(a.len(), b.len(), "{name}"); let d = common::max_diff(&a, &b); assert!(d <= 1e-4, "{name}: render differs by {d}"); diff --git a/crates/io/tests/it/psd_damaged_merged.rs b/crates/io/tests/it/psd_damaged_merged.rs index 8e20687bb5..51121d59b7 100644 --- a/crates/io/tests/it/psd_damaged_merged.rs +++ b/crates/io/tests/it/psd_damaged_merged.rs @@ -32,7 +32,7 @@ fn truncated_merged_image_keeps_layer_pixels_at_every_depth_and_compression() { for compression in Compression::ALL { let mut file = layered_in_mode(mode, depth, extension); let intact = import(extension, &file.to_bytes().unwrap()).unwrap(); - let expected = photocraft_compose::flatten(&intact.document).px; + let expected = photocraft_compose::flatten(&intact.document).into_px(); let pixels = file.decode_merged().unwrap(); file.image_data = photocraft_psd::ImageData::encode(compression, &pixels, &file.header).unwrap(); file.image_data.data.truncate(file.image_data.data.len() / 2); diff --git a/crates/io/tests/it/psd_type_bounds.rs b/crates/io/tests/it/psd_type_bounds.rs index b87d4f4a3a..43ce38c674 100644 --- a/crates/io/tests/it/psd_type_bounds.rs +++ b/crates/io/tests/it/psd_type_bounds.rs @@ -283,7 +283,7 @@ fn imported_type_layers_keep_their_full_bounds() { let o = iou(&ours.surface, cache); assert!(o > 0.97, "{}: re-render overlap {o}", c.name); // And the document composite is the file's merged composite. - let px = photocraft_compose::flatten(&doc).px; + let px = photocraft_compose::flatten(&doc).into_px(); let worst = common::max_diff(&px, &merged); assert!(worst <= 2.0 / 255.0, "{}: composite differs from the merged image by {worst}", c.name); } diff --git a/crates/io/tests/it/tiff_corpus.rs b/crates/io/tests/it/tiff_corpus.rs index 697eab6d32..1b3c36a05d 100644 --- a/crates/io/tests/it/tiff_corpus.rs +++ b/crates/io/tests/it/tiff_corpus.rs @@ -124,8 +124,8 @@ fn corpus_round_trips_through_layered_tiff() { } d }; - let x = photocraft_compose::flatten(&expected).px; - let y = photocraft_compose::flatten(&back).px; + let x = photocraft_compose::flatten(&expected).into_px(); + let y = photocraft_compose::flatten(&back).into_px(); let diff = max_diff(&x, &y); if diff > 1.0 / 255.0 + 1e-5 { return Err(format!("render differs by {diff}")); diff --git a/crates/io/tests/it/tiff_layers.rs b/crates/io/tests/it/tiff_layers.rs index f2ccddc92e..3c7815a37a 100644 --- a/crates/io/tests/it/tiff_layers.rs +++ b/crates/io/tests/it/tiff_layers.rs @@ -87,8 +87,8 @@ fn tiff_and_psd_open_as_the_same_document() { ) .unwrap() .document; - let a = photocraft_compose::flatten(&d).px; - let b = photocraft_compose::flatten(&flat).px; + let a = photocraft_compose::flatten(&d).into_px(); + let b = photocraft_compose::flatten(&flat).into_px(); assert!(max_diff(&a, &b) <= 2.0 / 255.0, "{mode:?} {depth:?}: composite differs by {}", max_diff(&a, &b)); } } diff --git a/crates/raster/src/lib.rs b/crates/raster/src/lib.rs index 5d0f6fe257..d949445bcc 100644 --- a/crates/raster/src/lib.rs +++ b/crates/raster/src/lib.rs @@ -9,6 +9,7 @@ mod interrupt; pub use interrupt::{Cancelled, Interrupt}; +pub mod scratch; use std::collections::BTreeMap; use std::fmt; @@ -62,18 +63,30 @@ impl TileInterner { } /// Pixel storage for one tile: `TILE_SIZE² × bytes_per_pixel`, row-major, interleaved channels. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq)] pub struct Tile { data: Box<[u8]>, } +impl Clone for Tile { + fn clone(&self) -> Self { + let mut data = scratch::take_exact(self.data.len(), 0u8); + data.copy_from_slice(&self.data); + Tile { data: data.into_boxed_slice() } + } +} + +impl Drop for Tile { + fn drop(&mut self) { + scratch::give(std::mem::take(&mut self.data).into_vec()); + } +} + impl Tile { fn try_filled(format: &PixelFormat, pixel: &[u8]) -> Result { let n = (TILE_SIZE * TILE_SIZE) as usize; let bytes = n.checked_mul(format.bytes_per_pixel()).ok_or(AllocationError::SizeOverflow)?; - let mut data = Vec::new(); - data.try_reserve_exact(bytes).map_err(|_| AllocationError::NotEnoughMemory { bytes })?; - data.resize(bytes, 0); + let mut data = scratch::try_take_exact(bytes, 0u8).ok_or(AllocationError::NotEnoughMemory { bytes })?; if pixel.iter().any(|&b| b != 0) { for px in data.chunks_exact_mut(pixel.len()) { px.copy_from_slice(pixel); @@ -85,15 +98,14 @@ impl Tile { /// Clone this tile's pixel buffer with a fallible reservation. pub fn try_clone(&self) -> Result { let bytes = self.data.len(); - let mut data = Vec::new(); - data.try_reserve_exact(bytes).map_err(|_| AllocationError::NotEnoughMemory { bytes })?; - data.extend_from_slice(&self.data); + let mut data = scratch::try_take_exact(bytes, 0u8).ok_or(AllocationError::NotEnoughMemory { bytes })?; + data.copy_from_slice(&self.data); Ok(Tile { data: data.into_boxed_slice() }) } fn filled(format: &PixelFormat, pixel: &[u8]) -> Self { let row_len = TILE_SIZE as usize * format.bytes_per_pixel(); - let mut data = vec![0u8; row_len * TILE_SIZE as usize]; + let mut data = scratch::take_exact(row_len * TILE_SIZE as usize, 0u8); if pixel.iter().any(|&b| b != 0) { // One prebuilt row copied per tile row: 256 bulk copies instead of one // copy_from_slice per pixel. @@ -358,9 +370,7 @@ impl Surface { pub fn try_read_region(&self, r: Rect) -> Result, AllocationError> { let len = (r.width() as usize).checked_mul(r.height() as usize).and_then(|n| n.checked_mul(self.channels())).ok_or(AllocationError::SizeOverflow)?; let bytes = len.checked_mul(std::mem::size_of::()).ok_or(AllocationError::SizeOverflow)?; - let mut out = Vec::new(); - out.try_reserve_exact(len).map_err(|_| AllocationError::NotEnoughMemory { bytes })?; - out.resize(len, 0.0); + let mut out = scratch::try_take_exact(len, 0.0f32).ok_or(AllocationError::NotEnoughMemory { bytes })?; self.read_region_into(r, &mut out); Ok(out) } @@ -761,7 +771,7 @@ impl Surface { pub fn read_region(&self, r: Rect) -> Vec { let n = self.channels(); let w = r.width() as usize; - let mut out = vec![0.0f32; w * r.height() as usize * n]; + let mut out = scratch::take(w * r.height() as usize * n, 0.0f32); self.read_samples_into(r, &mut out); out } @@ -935,7 +945,7 @@ impl Surface { pub fn to_interleaved(&self, r: Rect) -> Vec { let bpp = self.format.bytes_per_pixel(); let w = r.width() as usize; - let mut out = vec![0u8; w * r.height() as usize * bpp]; + let mut out = scratch::take(w * r.height() as usize * bpp, 0u8); for tc in r.tiles() { let tr = tc.rect().intersect(&r); let span = tr.width() as usize * bpp; @@ -1022,7 +1032,7 @@ impl Surface { return hit.clone().map(|t| (dc, t)); } - let mut tile = Tile { data: vec![0u8; (TILE_SIZE * TILE_SIZE) as usize * bpp].into_boxed_slice() }; + let mut tile = Tile { data: scratch::take_exact((TILE_SIZE * TILE_SIZE) as usize * bpp, 0u8).into_boxed_slice() }; let (tx0, ty0) = (i64::from(dc.tx) * i64::from(TILE_SIZE), i64::from(dc.ty) * i64::from(TILE_SIZE)); let row_len = TILE_SIZE as usize * bpp; // Per stored row: copy the source span that falls inside the content in as few diff --git a/crates/raster/src/scratch.rs b/crates/raster/src/scratch.rs new file mode 100644 index 0000000000..cb0f898306 --- /dev/null +++ b/crates/raster/src/scratch.rs @@ -0,0 +1,384 @@ +//! Process-wide, capped pool of large scratch buffers. +//! +//! With glibc's mmap threshold pinned (`MALLOC_TRIM_THRESHOLD_`, which is how PhotoCraft returns +//! memory to the OS), every allocation over 128 KiB is a fresh `mmap` that is page-faulted in and +//! `munmap`ed on free. Tiles, compositor bands and filter scratch are all that size, so a single +//! viewport render took 350× the page faults. This pool keeps freed buffers (at least +//! [`MIN_POOLED_BYTES`] of capacity) on per-type free lists and hands them back out, so the +//! steady state allocates nothing. +//! +//! Retention is capped by [`set_budget`] (default [`DEFAULT_BUDGET`]); [`release`] drops +//! everything, which is what returns the memory to the OS. The engine releases when a document +//! closes; off wasm a background thread also releases once the pool has gone unused for +//! [`IDLE_RELEASE_SECS`], so every host (app, CLI, tests) gives memory back the same way. + +use std::collections::{BTreeMap, VecDeque}; +use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}; +use std::sync::{Mutex, PoisonError}; + +/// Buffers with less capacity than this are cheap from the heap and are never retained. +pub const MIN_POOLED_BYTES: usize = 64 * 1024; +/// Default cap on the bytes the pool retains: about the working set of a 36 MP filter (tiles, +/// halo images and result bands); a browser's wasm heap never shrinks, so there it is small. +pub const DEFAULT_BUDGET: usize = if cfg!(target_arch = "wasm32") { 256 << 20 } else { 2 << 30 }; + +/// Pooled bytes a miss never evicts. A filter's output tiles are all misses (the old tiles stay +/// in the undo history); evicting every byte they need also dropped the scratch planes the next +/// tile reuses. On a 36 MP Gaussian Blur with the trim threshold set, 256 MiB keeps the timings +/// at glibc's defaults (128 MiB: r 40 ~17% slower) for ~10% more peak memory (0: ~20% slower). +pub const DEFAULT_MISS_FLOOR: usize = if cfg!(target_arch = "wasm32") { 64 << 20 } else { 256 << 20 }; + +static BUDGET: AtomicUsize = AtomicUsize::new(DEFAULT_BUDGET); +static MISS_FLOOR: AtomicUsize = AtomicUsize::new(DEFAULT_MISS_FLOOR); +static RETAINED: AtomicUsize = AtomicUsize::new(0); +static USES: AtomicU64 = AtomicU64::new(0); +static IDLE_RELEASE: AtomicBool = AtomicBool::new(true); + +/// Seconds without a [`take`] or [`give`] after which the idle thread releases the pool. +pub const IDLE_RELEASE_SECS: u64 = 2; + +/// Starts the idle-release thread once (off wasm, where there are threads). If the thread +/// can't be spawned the pool still works; it is then released only by [`release`]. +fn start_idle_release() { + #[cfg(not(target_arch = "wasm32"))] + { + static STARTED: std::sync::Once = std::sync::Once::new(); + STARTED.call_once(|| { + let _ = std::thread::Builder::new().name("scratch-idle".into()).spawn(|| { + let mut seen = uses(); + loop { + std::thread::sleep(std::time::Duration::from_secs(IDLE_RELEASE_SECS)); + let now = uses(); + if now == seen { + // Decided under the lock, so `set_idle_release(false)` can't return + // while a release it should have stopped is about to start. + let old = { + let mut pool = lock(); + // A `give` or `take` since `now` was read means the pool is in use. + (uses() == now && retained_bytes() > 0 && IDLE_RELEASE.load(Ordering::Relaxed)).then(|| empty(&mut pool)) + }; + drop(old); + } + seen = now; + } + }); + }); + } +} + +/// One free list: buffers keyed by capacity (in elements), each with the sequence number it +/// was returned at (oldest first within a capacity). +#[doc(hidden)] +pub type FreeList = BTreeMap)>>; + +/// All the free lists behind one lock, so the oldest buffer of any type can be evicted. +/// `order` maps every pooled buffer's sequence number to its type and capacity, so the oldest +/// is found in O(log n); best-fit lookups are a range query on the capacity key. +#[doc(hidden)] +pub struct Pool { + seq: u64, + order: BTreeMap, + u8s: FreeList, + u16s: FreeList, + f32s: FreeList, + px4: FreeList<[f32; 4]>, +} + +static POOL: Mutex = + Mutex::new(Pool { seq: 0, order: BTreeMap::new(), u8s: BTreeMap::new(), u16s: BTreeMap::new(), f32s: BTreeMap::new(), px4: BTreeMap::new() }); + +/// Which free list a buffer lives on. +#[doc(hidden)] +#[derive(Clone, Copy)] +pub enum Tag { + U8, + U16, + F32, + Px4, +} + +/// A buffer evicted from the pool, dropped by the caller once the lock is released. +#[allow(dead_code)] +enum Evicted { + U8(Vec), + U16(Vec), + F32(Vec), + Px4(Vec<[f32; 4]>), +} + +/// Element types that have a free list. Implemented for the buffers the hot paths use. +pub trait Pooled: Copy + Send + 'static { + #[doc(hidden)] + const TAG: Tag; + #[doc(hidden)] + fn list(pool: &mut Pool) -> &mut FreeList; +} + +macro_rules! pooled { + ($($t:ty => $field:ident, $tag:ident),*) => {$( + impl Pooled for $t { + const TAG: Tag = Tag::$tag; + fn list(pool: &mut Pool) -> &mut FreeList { + &mut pool.$field + } + } + )*}; +} +pooled!(u8 => u8s, U8, u16 => u16s, U16, f32 => f32s, F32, [f32; 4] => px4, Px4); + +fn bytes_of(v: &Vec) -> usize { + v.capacity().saturating_mul(std::mem::size_of::()) +} + +fn lock() -> std::sync::MutexGuard<'static, Pool> { + POOL.lock().unwrap_or_else(PoisonError::into_inner) +} + +/// Remove the oldest buffer of capacity `cap` from `list`. +fn drop_oldest(list: &mut FreeList, cap: usize) -> Option> { + let queue = list.get_mut(&cap)?; + let v = queue.pop_front()?.1; + if queue.is_empty() { + list.remove(&cap); + } + Some(v) +} + +impl Pool { + /// Remove the buffer returned longest ago, of any type. `None` when the pool is empty. + fn evict_oldest(&mut self) -> Option { + let (_, (tag, cap)) = self.order.pop_first()?; + let evicted = match tag { + Tag::U8 => drop_oldest(&mut self.u8s, cap).map(Evicted::U8), + Tag::U16 => drop_oldest(&mut self.u16s, cap).map(Evicted::U16), + Tag::F32 => drop_oldest(&mut self.f32s, cap).map(Evicted::F32), + Tag::Px4 => drop_oldest(&mut self.px4, cap).map(Evicted::Px4), + }; + if let Some(e) = &evicted { + let freed = match e { + Evicted::U8(v) => bytes_of(v), + Evicted::U16(v) => bytes_of(v), + Evicted::F32(v) => bytes_of(v), + Evicted::Px4(v) => bytes_of(v), + }; + RETAINED.fetch_sub(freed, Ordering::Relaxed); + } + evicted + } +} + +/// A `Vec` of `len` copies of `fill`, reusing a pooled buffer when one fits: the one whose +/// capacity equals `len`, else the smallest with capacity in `len..=2*len`. +pub fn take(len: usize, fill: T) -> Vec { + take_with(len, fill, 2) +} + +/// An empty `Vec` with room for at least `cap` elements, reusing a pooled buffer when one fits. +pub fn take_cap(cap: usize) -> Vec { + USES.fetch_add(1, Ordering::Relaxed); + match pop::(cap, 2) { + Some(mut v) => { + v.clear(); + v + } + None => Vec::with_capacity(cap), + } +} + +/// Like [`take`] but only reuses a buffer whose capacity is exactly `len`, so +/// `into_boxed_slice` never reallocates. +pub fn take_exact(len: usize, fill: T) -> Vec { + take_with(len, fill, 1) +} + +/// Pop the best-fitting free buffer: capacity `len`, else the smallest up to `len * slack`. +/// +/// On a miss the caller allocates `len` fresh, so the oldest free buffers of at least that many +/// bytes are dropped first, down to [`set_miss_floor`]: memory the pool can't reuse for this request +/// goes back instead of piling up beside the new buffer, so pool plus live buffers exceed what +/// the program would hold without the pool by at most the floor (#10479). +fn pop(len: usize, slack: usize) -> Option> { + let need = len.saturating_mul(std::mem::size_of::()); + if need < MIN_POOLED_BYTES { + return None; + } + let mut evicted = Vec::new(); + let hit = { + let mut pool = lock(); + let max = len.saturating_mul(slack); + let list = T::list(&mut pool); + let cap = list.range(len..=max).next().map(|(&cap, _)| cap); + // The newest buffer of that capacity is the one most likely still in cache. + let found = cap.and_then(|cap| { + let queue = list.get_mut(&cap)?; + let entry = queue.pop_back(); + if queue.is_empty() { + list.remove(&cap); + } + entry + }); + match found { + Some((seq, v)) => { + pool.order.remove(&seq); + RETAINED.fetch_sub(bytes_of(&v), Ordering::Relaxed); + Some(v) + } + None => { + let target = RETAINED.load(Ordering::Relaxed).saturating_sub(need).max(MISS_FLOOR.load(Ordering::Relaxed)); + while RETAINED.load(Ordering::Relaxed) > target { + match pool.evict_oldest() { + Some(e) => evicted.push(e), + None => break, + } + } + None + } + } + }; + // Freed outside the lock. + drop(evicted); + hit +} + +fn take_with(len: usize, fill: T, slack: usize) -> Vec { + USES.fetch_add(1, Ordering::Relaxed); + if len == 0 { + return Vec::new(); + } + match pop::(len, slack) { + Some(mut v) => { + v.clear(); + v.resize(len, fill); + v + } + None => vec![fill; len], + } +} + +/// Fallible [`take_exact`]: on a pool miss the heap allocation is `try_reserve_exact`, so +/// `None` means the allocation failed rather than aborting. +pub fn try_take_exact(len: usize, fill: T) -> Option> { + USES.fetch_add(1, Ordering::Relaxed); + let mut v = match pop::(len, 1) { + Some(v) => v, + None => { + let mut v = Vec::new(); + if v.try_reserve_exact(len).is_err() { + // The pool may hold up to the miss floor of buffers this request couldn't use: + // give them back, then try once more before failing the caller. + release(); + v.try_reserve_exact(len).ok()?; + } + v + } + }; + v.clear(); + v.resize(len, fill); + Some(v) +} + +/// Return a buffer to the pool. Small buffers, and buffers bigger than the whole budget, are +/// dropped; otherwise the buffers returned longest ago (of any type) make room for it. +pub fn give(v: Vec) { + USES.fetch_add(1, Ordering::Relaxed); + let bytes = bytes_of(&v); + if bytes < MIN_POOLED_BYTES { + return; + } + let mut evicted = Vec::new(); + { + let mut pool = lock(); + // Read under the lock, which `set_budget` also takes, so no insert uses a stale budget. + let budget = BUDGET.load(Ordering::Relaxed); + if bytes > budget { + return; + } + start_idle_release(); + while RETAINED.load(Ordering::Relaxed).saturating_add(bytes) > budget { + match pool.evict_oldest() { + Some(e) => evicted.push(e), + None => break, + } + } + RETAINED.fetch_add(bytes, Ordering::Relaxed); + pool.seq += 1; + let seq = pool.seq; + let cap = v.capacity(); + pool.order.insert(seq, (T::TAG, cap)); + T::list(&mut pool).entry(cap).or_default().push_back((seq, v)); + } + // Freed outside the lock. + drop(evicted); +} + +/// Set the cap on retained bytes (existing entries beyond it go on the next [`give`]). +pub fn set_budget(bytes: usize) { + let _pool = lock(); + BUDGET.store(bytes, Ordering::Relaxed); +} + +/// Set the pooled bytes a miss never evicts (default [`DEFAULT_MISS_FLOOR`]). +pub fn set_miss_floor(bytes: usize) { + MISS_FLOOR.store(bytes, Ordering::Relaxed); +} + +/// Let the idle thread release the pool (the default) or not. Tests that check +/// [`retained_bytes`] turn it off, so a slow runner can't have the pool emptied under them. +pub fn set_idle_release(on: bool) { + // Under the lock: once this returns, the idle thread can't be midway into a release. + let _pool = lock(); + IDLE_RELEASE.store(on, Ordering::Relaxed); +} + +/// Bytes currently held by the pool. +pub fn retained_bytes() -> usize { + RETAINED.load(Ordering::Relaxed) +} + +/// Number of [`take`]/[`give`] calls so far; a changing value means the pool is in use. +pub fn uses() -> u64 { + USES.load(Ordering::Relaxed) +} + +/// Drop every pooled buffer, handing the memory back to the OS. +pub fn release() { + let old = empty(&mut lock()); + // Freed outside the lock. + drop(old); +} + +/// Everything `pool` holds, swapped out of it (to be freed once the lock is dropped). +fn empty(pool: &mut Pool) -> Pool { + RETAINED.store(0, Ordering::Relaxed); + let seq = pool.seq; + std::mem::replace(pool, Pool { seq, order: BTreeMap::new(), u8s: BTreeMap::new(), u16s: BTreeMap::new(), f32s: BTreeMap::new(), px4: BTreeMap::new() }) +} + +/// A pooled buffer that derefs to a slice and returns itself to the pool on drop. +pub struct Buf(Vec); + +impl Buf { + /// `len` copies of `fill` from the pool. + pub fn new(len: usize, fill: T) -> Self { + Buf(take(len, fill)) + } +} + +impl std::ops::Deref for Buf { + type Target = [T]; + fn deref(&self) -> &[T] { + &self.0 + } +} + +impl std::ops::DerefMut for Buf { + fn deref_mut(&mut self) -> &mut [T] { + &mut self.0 + } +} + +impl Drop for Buf { + fn drop(&mut self) { + give(std::mem::take(&mut self.0)); + } +} diff --git a/crates/raster/tests/scratch.rs b/crates/raster/tests/scratch.rs new file mode 100644 index 0000000000..a0f532f174 --- /dev/null +++ b/crates/raster/tests/scratch.rs @@ -0,0 +1,361 @@ +//! The scratch pool is process-wide, so these tests live in their own binary (nothing else +//! allocates tiles here) and take a lock to run one at a time. + +use photocraft_raster::scratch::*; +use std::sync::{Mutex, PoisonError}; + +static SERIAL: Mutex<()> = Mutex::new(()); + +/// Lets a miss evict every pooled byte for the test's lifetime, so sizes can be counted. +struct NoFloor; +impl NoFloor { + /// Sets the miss floor to zero for this guard's lifetime. + fn new() -> Self { + set_miss_floor(0); + NoFloor + } +} +impl Drop for NoFloor { + fn drop(&mut self) { + set_miss_floor(DEFAULT_MISS_FLOOR); + } +} +/// Locks the process-wide pool tests against concurrent access. +fn lock_serial() -> std::sync::MutexGuard<'static, ()> { + SERIAL.lock().unwrap_or_else(PoisonError::into_inner) +} + +/// Runs a test alone with idle release off, so the idle thread can't empty the pool between a +/// `give` and the checks after it; idle release comes back on when dropped. +struct Serial { + _lock: std::sync::MutexGuard<'static, ()>, +} +impl Drop for Serial { + fn drop(&mut self) { + set_idle_release(true); + } +} +/// Serializes a test and disables idle release until the guard is dropped. +fn serial() -> Serial { + let _lock = lock_serial(); + set_idle_release(false); + Serial { _lock } +} + +#[test] +/// Verifies pooled buffers are reused by pointer, retain capacity, and are refilled to the requested length. +fn reuse_returns_same_capacity_and_refills() { + let _g = serial(); + release(); + let mut v = take(100_000, 0.0f32); + v.iter_mut().for_each(|x| *x = 7.0); + let ptr = v.as_ptr(); + give(v); + assert_eq!(retained_bytes(), 400_000); + let w = take(100_000, 1.5f32); + assert_eq!(w.as_ptr(), ptr); + assert_eq!(w.len(), 100_000); + assert!(w.iter().all(|&x| x == 1.5)); + assert_eq!(retained_bytes(), 0); + // A smaller request within 2x reuses it too, and is sized exactly. + give(w); + let s = take(60_000, 0.0f32); + assert_eq!(s.as_ptr(), ptr); + assert_eq!(s.len(), 60_000); + release(); +} + +#[test] +/// Verifies oversized or non-exact buffers are discarded instead of reused. +fn too_big_a_buffer_is_not_reused() { + let _g = serial(); + release(); + let _floor = NoFloor::new(); + give(vec![0u8; 1_000_000]); + assert_eq!(take_exact(1_000_000, 0u8).len(), 1_000_000); + assert_eq!(retained_bytes(), 0); + give(vec![0u8; 1_000_000]); + // More than 2x (or not exact): allocated fresh, and the miss drops the unusable buffer. + assert_eq!(take(100_000, 0u8).capacity(), 100_000); + assert_eq!(retained_bytes(), 0); + give(vec![0u8; 1_000_000]); + assert_eq!(take_exact(999_999, 0u8).capacity(), 999_999); + assert_eq!(retained_bytes(), 0); + release(); +} + +#[test] +/// Verifies exact try-takes reuse a matching buffer and succeed on a miss. +fn try_take_exact_reuses_and_allocates() { + let _g = serial(); + release(); + let buf = vec![0u8; 200_000]; + let ptr = buf.as_ptr(); + give(buf); + let v = try_take_exact(200_000, 9u8); + assert!(v.as_ref().is_some_and(|v| v.len() == 200_000 && v.iter().all(|&b| b == 9))); + assert_eq!(v.as_ref().map(|v| v.as_ptr()), Some(ptr)); + assert_eq!(retained_bytes(), 0); + assert!(try_take_exact(300_000, 0u8).is_some()); + release(); +} + +#[test] +/// Verifies buffers below the pooling threshold are not retained. +fn small_buffers_are_not_retained() { + let _g = serial(); + release(); + give(vec![0u8; MIN_POOLED_BYTES - 1]); + give(vec![0.0f32; 100]); + assert_eq!(retained_bytes(), 0); + assert_eq!(take(10, 3u8), vec![3u8; 10]); +} + +#[test] +/// Verifies zero-length take and give operations remain empty. +fn zero_length() { + let _g = serial(); + assert!(take(0, 0u8).is_empty()); + give(Vec::::new()); + assert!(take_exact(0, [0.0f32; 4]).is_empty()); +} + +#[test] +/// Verifies the byte budget evicts oldest buffers and rejects buffers larger than the budget. +fn budget_evicts_oldest() { + let _g = serial(); + release(); + set_budget(250_000); + let (a, b) = (vec![0u8; 100_000], vec![0u8; 100_000]); + let pb = b.as_ptr(); + give(a); + give(b); + give(vec![0u8; 100_000]); + assert_eq!(retained_bytes(), 200_000); + // The oldest was evicted; the survivors include b. + let first = take_exact(100_000, 0u8); + let second = take_exact(100_000, 0u8); + assert!(first.as_ptr() == pb || second.as_ptr() == pb); + // Larger than the whole budget: dropped. + give(vec![0u8; 300_000]); + assert_eq!(retained_bytes(), 0); + set_budget(DEFAULT_BUDGET); + release(); +} + +#[test] +/// Verifies release empties pooled buffers of several element types. +fn release_empties() { + let _g = serial(); + give(vec![0u8; 200_000]); + give(vec![0.0f32; 200_000]); + give(vec![[0.0f32; 4]; 100_000]); + assert!(retained_bytes() > 0); + release(); + assert_eq!(retained_bytes(), 0); +} + +#[test] +/// Verifies the usage counter advances for both take and give. +fn uses_counts_calls() { + let _g = serial(); + let before = uses(); + give(take(10, 0u8)); + assert_eq!(uses(), before + 2); +} + +#[test] +/// Verifies dropping a `Buf` returns its storage to the pool. +fn buf_returns_on_drop() { + let _g = serial(); + release(); + { + let mut b = Buf::new(150_000, 0.0f32); + b[3] = 1.0; + assert_eq!(b.len(), 150_000); + } + assert_eq!(retained_bytes(), 600_000); + release(); +} + +#[test] +/// Verifies concurrent take and give operations preserve initialized values and finish cleanly. +fn concurrent_smoke() { + let _g = serial(); + release(); + let handles: Vec<_> = (0..8u8) + .map(|t| { + std::thread::spawn(move || { + for i in 0..200 { + let v = take(70_000 + i % 5, t); + assert!(v.iter().all(|&x| x == t)); + give(v); + } + }) + }) + .collect(); + for h in handles { + assert!(h.join().is_ok()); + } + release(); + assert_eq!(retained_bytes(), 0); +} + +#[test] +/// Verifies budget eviction uses insertion age across different element types. +fn eviction_is_oldest_first_across_types() { + let _g = serial(); + release(); + set_budget(500_000); + give(vec![0u8; 300_000]); + give(vec![0.0f32; 50_000]); + assert_eq!(retained_bytes(), 500_000); + // Needs 200 000 more: the older u8 buffer goes, the f32 one stays. + give(vec![[0.0f32; 4]; 12_500]); + assert_eq!(retained_bytes(), 400_000); + assert_eq!(take_exact(50_000, 1.0f32).len(), 50_000); + assert_eq!(retained_bytes(), 200_000); + set_budget(DEFAULT_BUDGET); + release(); +} + +#[test] +/// Verifies reuse chooses the smallest eligible capacity and exact takes require an exact match. +fn best_fit_picks_the_smallest_fitting_capacity() { + let _g = serial(); + release(); + let _floor = NoFloor::new(); + for n in [180_000usize, 120_000, 150_000, 300_000] { + give(vec![0u8; n]); + } + // Within 2x of 100 000: 120 000 is the tightest fit, then 150 000, then 180 000. + assert_eq!(take(100_000, 0u8).capacity(), 120_000); + assert_eq!(take(100_000, 0u8).capacity(), 150_000); + assert_eq!(take(100_000, 0u8).capacity(), 180_000); + // 300 000 is more than 2x: allocated fresh, and the miss drops the pooled one. + assert_eq!(take(100_000, 0u8).capacity(), 100_000); + assert_eq!(retained_bytes(), 0); + // An exact capacity beats a smaller-slack scan. + give(vec![0u8; 100_000]); + assert_eq!(take_exact(100_000, 0u8).capacity(), 100_000); + assert_eq!(take_exact(250_000, 0u8).capacity(), 250_000); + release(); +} + +#[test] +/// Verifies repeated budget overflow evicts oldest entries across mixed types and capacities. +fn eviction_walks_oldest_first_through_mixed_types_and_capacities() { + let _g = serial(); + release(); + let _floor = NoFloor::new(); + set_budget(700_000); + // Oldest to newest, 100 000 bytes each, interleaved types and two u8 capacities. + give(vec![0u8; 100_000]); + let f = vec![0.0f32; 25_000]; + give(f); + give(vec![0u8; 100_000]); + give(vec![0u16; 50_000]); + let newest = vec![0u8; 150_000]; + let np = newest.as_ptr(); + give(newest); + assert_eq!(retained_bytes(), 550_000); + // 200 000 more overflows by 50 000: the oldest (u8) goes. + give(vec![[0.0f32; 4]; 12_500]); + assert_eq!(retained_bytes(), 650_000); + // 100 000 more: the next oldest (the f32 buffer) goes. + give(vec![0u8; 100_000]); + assert_eq!(retained_bytes(), 650_000); + // 200 000 more: the u8 then the u16 buffer go. + give(vec![0u8; 200_000]); + assert_eq!(retained_bytes(), 650_000); + // The newest u8 buffer of the old batch is still pooled; the f32 one was evicted. + let again = take_exact(150_000, 0u8); + assert_eq!(again.as_ptr(), np); + assert_eq!(retained_bytes(), 500_000); + // The evicted f32 buffer is a miss: the oldest buffers, at least its 100 000 bytes, leave + // (the 200 000-byte [f32; 4] one). + assert_eq!(take_exact(25_000, 0.0f32).len(), 25_000); + assert_eq!(retained_bytes(), 300_000); + set_budget(DEFAULT_BUDGET); + release(); +} + +#[test] +/// Verifies the idle-release worker clears an unused pooled buffer. +fn idle_pool_is_released_by_the_background_thread() { + // Idle release stays on here: it is what this test checks. + let _g = lock_serial(); + release(); + give(vec![0u8; MIN_POOLED_BYTES]); + assert_eq!(retained_bytes(), MIN_POOLED_BYTES); + // Released after one full idle interval without use (at most two from the give). + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(3 * IDLE_RELEASE_SECS + 1); + while retained_bytes() > 0 && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(100)); + } + assert_eq!(retained_bytes(), 0); +} + +#[test] +/// Verifies a miss discards pooled bytes in proportion to its allocation and a hit preserves the pool. +fn a_miss_drops_as_many_pooled_bytes_as_it_allocates() { + let _g = serial(); + release(); + let _floor = NoFloor::new(); + // Three 64 KiB f32 buffers a byte request can't use. + for _ in 0..3 { + give(vec![0.0f32; MIN_POOLED_BYTES / 4]); + } + assert_eq!(retained_bytes(), 3 * MIN_POOLED_BYTES); + // A 128 KiB miss drops the two oldest, so pool plus live buffers stay at 192 KiB. + let v = take(2 * MIN_POOLED_BYTES, 0u8); + assert_eq!(retained_bytes(), MIN_POOLED_BYTES); + give(v); + // A hit drops nothing. + let w = take(MIN_POOLED_BYTES / 4, 0.0f32); + assert_eq!(retained_bytes(), 2 * MIN_POOLED_BYTES); + drop(w); + release(); +} + +#[test] +/// Verifies miss eviction stops when the configured retained-byte floor is reached. +fn a_miss_keeps_the_floor() { + let _g = serial(); + release(); + set_miss_floor(2 * MIN_POOLED_BYTES); + for _ in 0..3 { + give(vec![0.0f32; MIN_POOLED_BYTES / 4]); + } + // A 192 KiB miss would drop all three; only one goes, down to the floor. + assert_eq!(take(3 * MIN_POOLED_BYTES, 0u8).len(), 3 * MIN_POOLED_BYTES); + assert_eq!(retained_bytes(), 2 * MIN_POOLED_BYTES); + set_miss_floor(DEFAULT_MISS_FLOOR); + release(); +} + +#[test] +/// Verifies a failed oversized reservation leaves the pool empty. +fn a_failed_reservation_releases_the_pool() { + let _g = serial(); + release(); + // Unmatched buffers under the miss floor stay pooled on a miss. + give(vec![0u8; 4 * MIN_POOLED_BYTES]); + assert!(retained_bytes() > 0); + // No allocator can reserve this: the call fails, but not before the pool was emptied. (The + // retry after the release isn't observable here: it needs an allocator that fails once.) + assert!(try_take_exact(isize::MAX as usize, 0u8).is_none()); + assert_eq!(retained_bytes(), 0); +} + +#[test] +/// Verifies a successful miss preserves unmatched buffers within the miss floor. +fn a_successful_miss_keeps_unmatched_pooled_buffers() { + let _g = serial(); + release(); + give(vec![0u8; 4 * MIN_POOLED_BYTES]); + let v = try_take_exact(2 * MIN_POOLED_BYTES, 0u8); + assert!(v.as_ref().is_some_and(|v| v.len() == 2 * MIN_POOLED_BYTES && v.iter().all(|&b| b == 0))); + assert_eq!(retained_bytes(), 4 * MIN_POOLED_BYTES, "within the miss floor, nothing is dropped"); + release(); +} diff --git a/crates/testkit/src/perf.rs b/crates/testkit/src/perf.rs index 6bdf68ab0a..38e6a14cb9 100644 --- a/crates/testkit/src/perf.rs +++ b/crates/testkit/src/perf.rs @@ -27,6 +27,19 @@ pub fn process_peak_rss_bytes() -> Option { parse_vm_hwm(&status) } +/// Minor page faults this process has taken so far (`/proc/self/stat` field 10), `None` off +/// Linux. A scenario that maps fresh memory on every run shows up here (#10479). +pub fn minor_faults() -> Option { + let stat = std::fs::read_to_string("/proc/self/stat").ok()?; + parse_minflt(&stat) +} + +fn parse_minflt(stat: &str) -> Option { + // `comm` (field 2) may contain spaces; the fields after its closing `)` start at field 3. + let rest = stat.get(stat.rfind(')')?.checked_add(1)?..)?; + rest.split_whitespace().nth(7)?.parse().ok() +} + fn parse_vm_hwm(status: &str) -> Option { let line = status.lines().find(|l| l.starts_with("VmHWM:"))?; let kb: u64 = line.split_whitespace().nth(1)?.parse().ok()?; @@ -149,6 +162,14 @@ pub fn write_report(path: &str, report: &Value) -> Result<(), String> { mod tests { use super::*; + #[test] + fn minflt_is_field_ten_even_with_spaces_in_comm() { + assert_eq!(parse_minflt("42 (cat) R 1 2 3 0 -1 4194304 122 0 0 0"), Some(122)); + assert_eq!(parse_minflt("42 (a b) c) S 1 2 3 0 -1 0 7 0"), Some(7)); + assert_eq!(parse_minflt("42 (cat) R 1"), None); + assert_eq!(parse_minflt("garbage"), None); + } + #[test] fn summary_nearest_rank() { let v: Vec = (1..=20).map(f64::from).collect(); diff --git a/crates/ui-egui/examples/interactive_bench.rs b/crates/ui-egui/examples/interactive_bench.rs index efeff0927d..0b4cc5c952 100644 --- a/crates/ui-egui/examples/interactive_bench.rs +++ b/crates/ui-egui/examples/interactive_bench.rs @@ -83,9 +83,9 @@ struct Bench { /// state later rows need). only: Option, rows: Vec<(String, f64, f64, f64, f64)>, - /// Per row (same order as `rows`): the timed samples and the peak RSS while they ran, for - /// `--json` (`cargo xtask perf`). - extra: Vec<(Vec, Option)>, + /// Per row (same order as `rows`): the timed samples, the peak RSS while they ran and the + /// minor page faults per warm run (all of it, untimed setup included), for `--json` (`cargo xtask perf`). + extra: Vec<(Vec, Option, Option)>, rss: photocraft_testkit::perf::RssSampler, } @@ -127,11 +127,17 @@ impl Bench { } self.rss.reset(); let cold = f(self); + let faults0 = photocraft_testkit::perf::minor_faults(); let mut v: Vec = (0..self.reps).map(|_| f(self)).collect(); - self.extra.push((v.clone(), self.rss.peak())); + // Minor page faults per warm run: memory mapped fresh from the kernel each time (#10479). + // Counted over the whole of `f`, so it includes untimed setup and cleanup (an undo, an + // extra refresh), not only the work the milliseconds measure. + let faults = faults0.zip(photocraft_testkit::perf::minor_faults()).map(|(a, b)| b.saturating_sub(a) / self.reps as u64); + self.extra.push((v.clone(), self.rss.peak(), faults)); v.sort_by(f64::total_cmp); let med = v[v.len() / 2]; - println!("{name:<44} {med:>9.1} ms (min {:>8.1}, max {:>8.1}, cold {:>8.1})", v[0], v[v.len() - 1], cold); + let fl = faults.map_or(String::new(), |f| format!(", {f} faults/run incl. setup")); + println!("{name:<44} {med:>9.1} ms (min {:>8.1}, max {:>8.1}, cold {:>8.1}{fl})", v[0], v[v.len() - 1], cold); self.rows.push((name.to_string(), med, v[0], v[v.len() - 1], cold)); } } @@ -473,11 +479,12 @@ fn main() { .rows .iter() .zip(&b.extra) - .map(|((n, med, min, max, cold), (samples, rss))| { + .map(|((n, med, min, max, cold), (samples, rss, faults))| { let mut r = json!({"name": n, "median_ms": med, "min_ms": min, "max_ms": max, "cold_ms": cold}); // For `cargo xtask perf`: raw samples (it computes p50 / p95) and peak RSS. r["samples_ms"] = json!(samples); r["peak_rss_bytes"] = json!(rss); + r["minor_faults_per_run"] = json!(faults); r }) .collect(); diff --git a/crates/ui-egui/src/dialogs/color_range_ui.rs b/crates/ui-egui/src/dialogs/color_range_ui.rs index 75fe3c67b9..3b02806d3a 100644 --- a/crates/ui-egui/src/dialogs/color_range_ui.rs +++ b/crates/ui-egui/src/dialogs/color_range_ui.rs @@ -236,7 +236,7 @@ fn proxy_mask(app: &PhotocraftApp, proxy: &Document, k: u32, params: &Value) -> surf.read_rgba_into(area, &mut px); px } - _ => photocraft_compose::render(proxy, area).px, + _ => photocraft_compose::render(proxy, area).into_px(), }; let invert = params.get("invert").and_then(Value::as_bool).unwrap_or(false); let mut mask: Vec = query diff --git a/crates/ui-egui/src/dialogs/file_ui/web.rs b/crates/ui-egui/src/dialogs/file_ui/web.rs index 0b5ef3cf58..865215d4b5 100644 --- a/crates/ui-egui/src/dialogs/file_ui/web.rs +++ b/crates/ui-egui/src/dialogs/file_ui/web.rs @@ -119,7 +119,7 @@ fn web_pane(app: &PhotocraftApp, ui: &mut egui::Ui, size: egui::Vec2, p: Option< }; let buf = photocraft_compose::flatten(&wd); let v = ( - Arc::new(buf.px), + Arc::new(buf.into_px()), wd.size.width, wd.size.height, (fw * fh) / (f64::from(wd.size.width) * f64::from(wd.size.height)).max(1.0), diff --git a/crates/ui-egui/src/dialogs/filter_dialog/tests.rs b/crates/ui-egui/src/dialogs/filter_dialog/tests.rs index 92daea2528..8c0ba385e6 100644 --- a/crates/ui-egui/src/dialogs/filter_dialog/tests.rs +++ b/crates/ui-egui/src/dialogs/filter_dialog/tests.rs @@ -628,7 +628,7 @@ fn smart_displace_preview_uses_the_map_document() { applied.add_document((**d).clone(), None); } applied.execute(SMART_FILTER_SET_PARAMS, p.clone()).unwrap(); - let want = photocraft_compose::render(&applied.active().unwrap().doc, doc.bounds()).px; + let want = photocraft_compose::render(&applied.active().unwrap().doc, doc.bounds()).into_px(); let preview = preview_document_for_target( &doc, layer, diff --git a/crates/ui-egui/src/dialogs/vp_ui/tests.rs b/crates/ui-egui/src/dialogs/vp_ui/tests.rs index 19f7786e33..f45063d35c 100644 --- a/crates/ui-egui/src/dialogs/vp_ui/tests.rs +++ b/crates/ui-egui/src/dialogs/vp_ui/tests.rs @@ -180,7 +180,7 @@ fn overlapping_stamp_preview_matches_commit_across_batches() { s.strokes[0].points.extend_from_slice(&points[2..]); assert!(s.paint_preview(|| 2)); let (shown, _) = s.shown().unwrap(); - let expected = photocraft_compose::flatten(&shown).px; + let expected = photocraft_compose::flatten(&shown).into_px(); assert_ne!(expected, photocraft_compose::flatten(&app.session.active().unwrap().doc).px); commit(&mut app).unwrap(); assert_eq!(photocraft_compose::flatten(&app.session.active().unwrap().doc).px, expected, "newLayer={new_layer}"); @@ -217,7 +217,7 @@ fn stamp_preview_paints_over_the_paste_like_ok() { let s = app.distort.vanishing.as_ref().unwrap(); assert!(s.preview_has_paste(), "the preview shows the paste, so the overlay isn't drawn over the strokes"); let (shown, _) = s.shown().unwrap(); - let expected = photocraft_compose::flatten(&shown).px; + let expected = photocraft_compose::flatten(&shown).into_px(); commit(&mut app).unwrap(); assert_eq!(photocraft_compose::flatten(&app.session.active().unwrap().doc).px, expected, "newLayer={new_layer}"); } @@ -349,7 +349,7 @@ fn ok_paints_the_layer_the_mode_was_opened_on() { app.session.execute("layer.new.layer", json!({"name": "other"})).unwrap(); let other = app.session.active().unwrap().active_layer.unwrap(); sync_preview(&mut app); - let expected = photocraft_compose::flatten(&crate::distort_ui::display_doc(&app, 0).unwrap().0).px; + let expected = photocraft_compose::flatten(&crate::distort_ui::display_doc(&app, 0).unwrap().0).into_px(); commit(&mut app).unwrap(); let d = &app.session.active().unwrap().doc; assert!(d.layer(other).unwrap().surface().unwrap().content_bounds().is_empty(), "newLayer={new_layer}"); @@ -389,7 +389,7 @@ fn an_edit_under_the_mode_shows_on_the_canvas() { let px = shown.layer(layer).unwrap().surface().unwrap(); assert!(px.rgba(210, 30)[1] > 0.9, "the edit shows: {:?}", px.rgba(210, 30)); assert!(px.rgba(120, 80) != st.doc.layer(layer).unwrap().surface().unwrap().rgba(120, 80), "and so does the stroke"); - let expected = photocraft_compose::flatten(&shown).px; + let expected = photocraft_compose::flatten(&shown).into_px(); commit(&mut app).unwrap(); assert_eq!(photocraft_compose::flatten(&app.session.active().unwrap().doc).px, expected); } diff --git a/crates/ui-egui/src/tools/transform_fx.rs b/crates/ui-egui/src/tools/transform_fx.rs index 60fc45d993..0ad888536d 100644 --- a/crates/ui-egui/src/tools/transform_fx.rs +++ b/crates/ui-egui/src/tools/transform_fx.rs @@ -256,7 +256,8 @@ pub fn compose(doc: &Document, work: &Layer, plan: &Plan, source: &Source) -> Op let band = Rect::new(shown.x0, y0, shown.x1, y0.saturating_add((px.len() / width.max(1)) as i32)); let mut over = Buffer::transparent(band); effects::composite_with_effects(work, &crop(&content, band), &mut over, &maps, plan.bounds, &doc.patterns); - for (o, p) in px.iter_mut().zip(over.px) { + // Copied, not moved out, so `over` hands its band back to the scratch pool on drop. + for (o, p) in px.iter_mut().zip(over.px.iter().copied()) { *o = p; } }); diff --git a/crates/ui-egui/tests/it/adjust_preview_gpu.rs b/crates/ui-egui/tests/it/adjust_preview_gpu.rs index 2fdae54cbe..4e8aa8552c 100644 --- a/crates/ui-egui/tests/it/adjust_preview_gpu.rs +++ b/crates/ui-egui/tests/it/adjust_preview_gpu.rs @@ -152,14 +152,14 @@ fn preview_layer_composites_on_the_gpu_like_the_command() { // may flip a few pixels sitting right at a step (1 % allowed). let skip = if matches!(kind, "threshold" | "posterize") && depth != SampleType::U8 { (b.width() * b.height()) as usize / 100 } else { 0 }; let on_gpu = render_to_vec(&mut comp, &device, &queue, &preview, b).unwrap_or_else(|e| panic!("{what}: {e:?}")); - let on_cpu = photocraft_compose::render(&preview, b).px; + let on_cpu = photocraft_compose::render(&preview, b).into_px(); let d = max_diff(&on_gpu, &on_cpu, skip); assert!(d <= tol_gpu, "{what}: GPU vs CPU {:.2}/255", d * 255.0); let mut s = photocraft_engine::Session::new(); s.add_document(doc.clone(), None); s.select_layer(target).unwrap(); s.execute(&format!("image.adjustments.{kind}"), params.clone()).unwrap(); - let cmd = photocraft_compose::render(&s.active().unwrap().doc, b).px; + let cmd = photocraft_compose::render(&s.active().unwrap().doc, b).into_px(); let d = max_diff(&on_gpu, &cmd, skip); worst = worst.max(d); assert!(d <= tol_cmd, "{what}: GPU preview vs command {:.2}/255", d * 255.0); @@ -184,7 +184,7 @@ fn proxy_preview_composites_on_the_gpu() { for kind in photocraft_ui_egui::adjust_editors::KINDS { let p = proxy_with_settings(&proxy, kind, &sample(kind)).unwrap(); let on_gpu = render_to_vec(&mut comp, &device, &queue, &p, p.bounds()).unwrap(); - let on_cpu = photocraft_compose::render(&p, p.bounds()).px; + let on_cpu = photocraft_compose::render(&p, p.bounds()).into_px(); let d = max_diff(&on_gpu, &on_cpu, 0); assert!(d <= 2.0 / 255.0, "{kind} selection {selection}: {:.2}/255", d * 255.0); } From 5d9c57e60db966382abda2638d0ccbb7489d8ee7 Mon Sep 17 00:00:00 2001 From: Khawar Zamman Wani Date: Mon, 2 Nov 2026 14:00:46 +0530 Subject: [PATCH 2/4] Linux: return memory of closed documents to the system (#8743) (#10515) * perf: reuse large buffers so a pinned glibc trim threshold costs no speed (#10479) With MALLOC_TRIM_THRESHOLD_ set (#8743), glibc maps every allocation over 128 KiB fresh, so tiles, compositor bands and filter scratch were page-faulted in on every call: viewport compose 3.6x slower, Gaussian Blur up to 1.8x, .pcraft save 2.4x. - photocraft_raster::scratch: a process-wide, capped pool of large Vec buffers (u8/u16/f32/[f32; 4]). A miss drops the oldest buffers worth its size down to a 256 MiB floor, so peak memory stays near glibc's defaults. The engine releases the pool after any command that closes a document, Headless::close does too, and an idle thread releases it after 2 s unused. - Tiles, compose Buffer/masks/bands, algo Image and the blur paths, engine adjustment/filter scratch, GPU tile upload and io import buffers go through the pool. - .pcraft save reuses one zstd FrameCompressor per worker (byte-identical). - interactive_bench reports minor page faults per run. 36 MP, trim threshold set vs glibc defaults: viewport 5.6 vs 6.1 ms (was 22.7), Blur r4 445 vs 437 ms (was 852), Curves CPU proxy 35.0 vs 35.7 ms (was 50), flatten 146 vs 147 ms (was 237), save .pcraft 251 vs 230 ms (was 545). RSS after closing a 5400x7200 document: 21 MB with trim set. * fix(linux): restart once with glibc's trim threshold set so closed documents return memory (#8743) With glibc's default malloc, the memory of a closed document stays with the process: freeing a large buffer raises glibc's dynamic mmap threshold, so later tile buffers land in heap pages glibc keeps. A session that opened and closed a 5400x7200 document held ~450 MiB afterwards. photocraft_engine::malloc_tuning::apply(), called first in both binaries' main, re-executes the binary once with MALLOC_TRIM_THRESHOLD_=131072 on Linux with glibc: same PID, arguments, argv[0] and environment, in safe Rust (CommandExt::exec). A malloc variable or glibc.malloc.* tunable the user or a package already set is left alone, so setting MALLOC_TRIM_THRESHOLD_ opts out. No-op on macOS, Windows, musl and wasm. Depends on the scratch-buffer pool (#10479), without which the pinned threshold slows filters, previews, flatten and .pcraft save by 26-209%. * fix: corpus tests build again; return remaining pooled buffers (#10479 review) - Corpus-gated io tests and the Photoshop-oracle test moved `.px` out of a `Buffer`, which now implements `Drop` (E0509 with `--features corpus`): use `into_px()`. - io: masks, the merged image, extra channels, smart-filter masks, smart-object pixels and patterns give their `interleave` buffers back to the scratch pool after writing them. - compose: recycle the mask values of a pass-through group, `effect_shape` and both `split_parts` callers. - Styled Free Transform preview copies each band out instead of moving it, so the band returns to the pool. - interactive_bench: the fault count covers each whole run, untimed setup included; say so. * fix: corpus tests build again; return remaining pooled buffers (#10479 review) - Corpus-gated io tests and the Photoshop-oracle test moved `.px` out of a `Buffer`, which now implements `Drop` (E0509 with `--features corpus`): use `into_px()`. - io: masks, the merged image, extra channels, smart-filter masks, smart-object pixels and patterns give their `interleave` buffers back to the scratch pool after writing them. - compose: recycle the mask values of a pass-through group, `effect_shape` and both `split_parts` callers. - Styled Free Transform preview copies each band out instead of moving it, so the band returns to the pool. - interactive_bench: the fault count covers each whole run, untimed setup included; say so. * fix: recycle the masks of adjustment-atop and shape layers (#10479 review) * fix: recycle the masks of adjustment-atop and shape layers (#10479 review) * fix: recycle effect hiding masks (#10479 review) * fix: recycle effect hiding masks (#10479 review) * fix: recycle knockout, CMYK and GPU shape masks; release the pool when Split/Merge Channels close a document (#10479 review) * fix: recycle knockout, CMYK and GPU shape masks; release the pool when Split/Merge Channels close a document (#10479 review) * fix: release the pool when a closed document's cancelled workers exit; idle-release switch for tests; don't pool PSD planes (#10479 review) * fix: release the pool when a closed document's cancelled workers exit; idle-release switch for tests; don't pool PSD planes (#10479 review) * fix: release the pool for workers cancelled before their document closes; take the pool lock for budget and idle-release changes (#10479 review) * fix: release the pool for workers cancelled before their document closes; take the pool lock for budget and idle-release changes (#10479 review) * fix: idle release rechecks activity under the lock; try_take_exact releases the pool and retries before failing (#10479 review) * fix: idle release rechecks activity under the lock; try_take_exact releases the pool and retries before failing (#10479 review) * test: cover a successful try_take_exact miss; name the failed-reservation test for what it checks (#10479 review) * test: cover a successful try_take_exact miss; name the failed-reservation test for what it checks (#10479 review) * fix: pool the final GPU effect shape buffer and return it after use (#10479 review) * fix: pool the final GPU effect shape buffer and return it after use (#10479 review) * docs: describe scratch pool lifecycle regression invariants * test: verify polling releases cancelled document scratch buffers * perf: recycle remaining transform selection and PSD scratch buffers * fix: free PSD gather bands before allocating channel planes * test: assert exact scratch take reuses pooled storage --- apps/photocraft-cli/src/main.rs | 2 + apps/photocraft-cli/tests/it/main.rs | 2 + apps/photocraft-cli/tests/it/malloc_tuning.rs | 41 ++++++++++ apps/photocraft/src/main.rs | 5 +- crates/engine/src/lib.rs | 1 + crates/engine/src/malloc_tuning.rs | 81 +++++++++++++++++++ 6 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 apps/photocraft-cli/tests/it/malloc_tuning.rs create mode 100644 crates/engine/src/malloc_tuning.rs diff --git a/apps/photocraft-cli/src/main.rs b/apps/photocraft-cli/src/main.rs index ea5598f99d..6e21c2ac44 100644 --- a/apps/photocraft-cli/src/main.rs +++ b/apps/photocraft-cli/src/main.rs @@ -1,6 +1,8 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] fn main() { + // May re-execute this process with glibc tuned to return freed document memory (#8743). + photocraft_engine::malloc_tuning::apply(); // `args_os`, not `args`: a non-Unicode argument (a Latin-1 file name on Linux) must reach clap, // not panic before anything is printed (issue #1108). let code = photocraft_cli::run(std::env::args_os().skip(1), &mut std::io::stdout(), &mut std::io::stderr()); diff --git a/apps/photocraft-cli/tests/it/main.rs b/apps/photocraft-cli/tests/it/main.rs index 65092cdee4..2729f74251 100644 --- a/apps/photocraft-cli/tests/it/main.rs +++ b/apps/photocraft-cli/tests/it/main.rs @@ -1,6 +1,8 @@ //! The crate's integration tests, linked into one test binary. mod cli; +#[cfg(all(target_os = "linux", target_env = "gnu"))] +mod malloc_tuning; mod man_and_completions; mod orientation; diff --git a/apps/photocraft-cli/tests/it/malloc_tuning.rs b/apps/photocraft-cli/tests/it/malloc_tuning.rs new file mode 100644 index 0000000000..a7513a714a --- /dev/null +++ b/apps/photocraft-cli/tests/it/malloc_tuning.rs @@ -0,0 +1,41 @@ +//! The binary re-executes itself with glibc's trim threshold set (#8743), keeping its arguments +//! and a value the user set. + +use std::io::{BufRead, BufReader, Write}; +use std::process::{Command, Stdio}; + +use photocraft_engine::malloc_tuning::{TRIM_THRESHOLD, TRIM_THRESHOLD_VAR}; + +/// The environment of a running `serve` once it answered a request, as `NAME=value` entries. +fn serve_environ(set: Option<&str>) -> Vec { + let mut cmd = Command::new(crate::binary()); + for var in [TRIM_THRESHOLD_VAR, "MALLOC_MMAP_THRESHOLD_", "MALLOC_TOP_PAD_", "MALLOC_MMAP_MAX_", "MALLOC_ARENA_MAX", "GLIBC_TUNABLES"] { + cmd.env_remove(var); + } + if let Some(v) = set { + cmd.env(TRIM_THRESHOLD_VAR, v); + } + let mut child = cmd.arg("serve").stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::null()).spawn().unwrap(); + let mut stdin = child.stdin.take().unwrap(); + writeln!(stdin, r#"{{"id":1,"method":"session.list"}}"#).unwrap(); + let mut reply = String::new(); + BufReader::new(child.stdout.take().unwrap()).read_line(&mut reply).unwrap(); + assert!(reply.contains(r#""id":1"#), "serve answered after the re-exec: {reply}"); + // The same process: `exec` keeps the PID, so this is the re-executed program's environment. + let environ = std::fs::read(format!("/proc/{}/environ", child.id())).unwrap(); + drop(stdin); + child.wait().unwrap(); + environ.split(|&b| b == 0).map(|e| String::from_utf8_lossy(e).into_owned()).collect() +} + +#[test] +fn sets_the_trim_threshold() { + let env = serve_environ(None); + assert!(env.contains(&format!("{TRIM_THRESHOLD_VAR}={TRIM_THRESHOLD}")), "{env:?}"); +} + +#[test] +fn keeps_a_value_the_user_set() { + let env = serve_environ(Some("999999")); + assert!(env.contains(&format!("{TRIM_THRESHOLD_VAR}=999999")), "{env:?}"); +} diff --git a/apps/photocraft/src/main.rs b/apps/photocraft/src/main.rs index e19a9d45e8..3738ffc8d1 100644 --- a/apps/photocraft/src/main.rs +++ b/apps/photocraft/src/main.rs @@ -340,7 +340,10 @@ fn write_start_marker(sentinel: &gpu_startup::SharedSentinel, plan: &gpu_startup } fn main() -> eframe::Result { - // First, so the panic hook and every start-up warning are recorded (`logging`). + // Before anything starts a thread: may re-execute this process with glibc tuned to return + // the memory of closed documents (#8743). + photocraft_engine::malloc_tuning::apply(); + // Then, so the panic hook and every start-up warning are recorded (`logging`). let logger = logging::install(); crash_guard::install_hook(); #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 5e6cc66ab0..b49675176e 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -29,6 +29,7 @@ pub mod kys; pub mod layer_place; pub mod layer_style; pub mod link_watch; +pub mod malloc_tuning; pub mod new_layer; mod param_check; mod pixels; diff --git a/crates/engine/src/malloc_tuning.rs b/crates/engine/src/malloc_tuning.rs new file mode 100644 index 0000000000..f9c5947c47 --- /dev/null +++ b/crates/engine/src/malloc_tuning.rs @@ -0,0 +1,81 @@ +//! glibc malloc tuning for the native binaries (#8743). +//! +//! With glibc's defaults, the memory of a closed document stays with the process: freeing a large +//! buffer raises glibc's dynamic mmap threshold (up to 32 MiB), so later tile buffers come from the +//! heap, whose freed pages glibc keeps for reuse instead of giving them back. A session that opened +//! and closed a 5400 × 7200 document held ~540 MiB afterwards; with `MALLOC_TRIM_THRESHOLD_` set it +//! went back to ~20 MiB, and the peak was lower too. Setting the threshold explicitly turns the +//! dynamic adjustment off (mallopt(3)), so large buffers are mapped and unmapped on their own and +//! the heap top is trimmed as it is freed. Mapping fresh pages on every call is slow, so the large +//! short-lived buffers come from `photocraft_raster::scratch` instead, which reuses them and +//! releases them when a document closes or the pool goes idle (#10479). +//! +//! glibc reads its tunables once, when the process starts, and changing the environment from Rust +//! needs `unsafe` (and comes too late), so [`apply`] re-executes the binary with the variable set: +//! the same process (PID), arguments, `argv[0]` and environment, on Linux with glibc only. A value +//! the user or a package already set, for this or another malloc tunable, is left alone, so setting +//! `MALLOC_TRIM_THRESHOLD_` to anything opts out. Programs PhotoCraft starts inherit the variable. + +use std::ffi::OsString; + +/// The glibc environment variable [`apply`] sets. +pub const TRIM_THRESHOLD_VAR: &str = "MALLOC_TRIM_THRESHOLD_"; + +/// Its value: glibc's own default trim threshold (128 KiB), now fixed instead of dynamic. +pub const TRIM_THRESHOLD: &str = "131072"; + +/// glibc malloc variables that mean someone tuned malloc themselves. +const MALLOC_VARS: [&str; 5] = [TRIM_THRESHOLD_VAR, "MALLOC_MMAP_THRESHOLD_", "MALLOC_TOP_PAD_", "MALLOC_MMAP_MAX_", "MALLOC_ARENA_MAX"]; + +/// Whether a start-up should re-execute with [`TRIM_THRESHOLD_VAR`] set, given the environment: +/// only when no malloc variable or `glibc.malloc.*` tunable is set (which also ends the re-exec). +pub fn needs_restart(var: impl Fn(&str) -> Option) -> bool { + if MALLOC_VARS.iter().any(|v| var(v).is_some()) { + return false; + } + !var("GLIBC_TUNABLES").is_some_and(|t| t.to_string_lossy().contains("glibc.malloc.")) +} + +/// Re-execute this binary with [`TRIM_THRESHOLD_VAR`] set when [`needs_restart`] says so. Call it +/// first in `main`, before any thread starts. It returns only when nothing needed to change or the +/// re-exec failed, and then the process carries on with glibc's defaults. +#[cfg(all(target_os = "linux", target_env = "gnu"))] +pub fn apply() { + use std::os::unix::process::CommandExt; + if !needs_restart(|k| std::env::var_os(k)) { + return; + } + let Ok(exe) = std::env::current_exe() else { return }; + let mut args = std::env::args_os(); + let mut cmd = std::process::Command::new(exe); + if let Some(argv0) = args.next() { + cmd.arg0(argv0); + } + // `exec` replaces the process and only returns on failure. + let err = cmd.args(args).env(TRIM_THRESHOLD_VAR, TRIM_THRESHOLD).exec(); + eprintln!("PhotoCraft: could not restart with {TRIM_THRESHOLD_VAR} set ({err}); memory of closed documents may not be returned to the system"); +} + +/// Nothing to tune off glibc. +#[cfg(not(all(target_os = "linux", target_env = "gnu")))] +pub fn apply() {} + +#[cfg(test)] +mod tests { + use super::*; + + fn env<'a>(vars: &'a [(&'a str, &'a str)]) -> impl Fn(&str) -> Option + 'a { + move |k| vars.iter().find(|(n, _)| *n == k).map(|(_, v)| OsString::from(v)) + } + + #[test] + fn restarts_only_without_malloc_tuning() { + assert!(needs_restart(env(&[]))); + assert!(needs_restart(env(&[("HOME", "/home/a"), ("GLIBC_TUNABLES", "glibc.cpu.hwcaps=-AVX2")]))); + assert!(!needs_restart(env(&[(TRIM_THRESHOLD_VAR, TRIM_THRESHOLD)])), "the re-executed process stops"); + assert!(!needs_restart(env(&[(TRIM_THRESHOLD_VAR, "")])), "any value opts out"); + assert!(!needs_restart(env(&[("MALLOC_MMAP_THRESHOLD_", "1048576")]))); + assert!(!needs_restart(env(&[("MALLOC_ARENA_MAX", "2")]))); + assert!(!needs_restart(env(&[("GLIBC_TUNABLES", "glibc.malloc.trim_threshold=0")]))); + } +} From 289dd0a874763d6528bab2184a87a1b53771c02a Mon Sep 17 00:00:00 2001 From: ".CooCoo" Date: Mon, 2 Nov 2026 08:43:46 -0800 Subject: [PATCH 3/4] Format: take the .pcraft envelope from craft-pcraft-format (#10724) * Format: take the .pcraft envelope from craft-pcraft-format The manifest envelope, TileRef, Hash, FORMAT_VERSION, EXTENSION, the entry names, tile_path and blob_path, is_valid_hash, the manifest nesting and item checks and the migration runner now come from craft-libs' craft-pcraft-format. photocraft-format re-exports them, so callers do not change. Manifest is craft_pcraft_format::Manifest. Saved bundles keep the same bytes. A save still checks only the nesting and the item count of its manifest, not its size, and the manifest nesting limit is still computed from photocraft_doc::MAX_GROUP_DEPTH. Tests: a save, load and save again gives the same bytes in every color mode and depth, with and without previews; MAX_GROUP_DEPTH equals craft_pcraft_format::MAX_GROUP_DEPTH. * Format: pin craft-pcraft-format from storytold/craft-libs * Format: read manifests with craft-pcraft-format's read_manifest_with Pins craft-pcraft-format to dca870a0, the head of craft-libs #407. Loading reads the manifest with read_manifest_with, which runs the limit check, the version check, the migration steps and the envelope read in one path; the layers are still decoded one at a time after the rest of the document. A save builds the manifest with Manifest::new and tile references with TileRef::new, and the bundle's directory names and the .zst suffix come from the crate's constants. Saved bytes and error messages are unchanged. * Format: pin craft-pcraft-format to craft-libs #407's head 04dd1bd 04dd1bd changes only the crate's docs and a test, so photocraft-format builds the same code. * Format: pin craft-pcraft-format to craft-libs main at b8f7d93 craft-libs #407 merged as b8f7d93. Its read_manifest also refuses a thumbnail other than thumb.png and a composite other than composite/preview.png, the only names photocraft-format writes. --- Cargo.lock | 11 +++ Cargo.toml | 1 + crates/format/Cargo.toml | 1 + crates/format/src/autosave.rs | 2 +- crates/format/src/convert.rs | 6 +- crates/format/src/lib.rs | 23 ++++- crates/format/src/manifest.rs | 28 +----- crates/format/src/migrate.rs | 44 ++++----- crates/format/src/store.rs | 134 +++++++++------------------- crates/format/tests/it/roundtrip.rs | 23 +++++ 10 files changed, 119 insertions(+), 154 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b22b872d1d..eb50bad6fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1388,6 +1388,16 @@ dependencies = [ "thiserror 2.0.21", ] +[[package]] +name = "craft-pcraft-format" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=b8f7d93b00dfb8f172938f3d2d796ae62dcfe123#b8f7d93b00dfb8f172938f3d2d796ae62dcfe123" +dependencies = [ + "serde", + "serde_json", + "thiserror 2.0.21", +] + [[package]] name = "craft-print" version = "0.1.0" @@ -5199,6 +5209,7 @@ name = "photocraft-format" version = "0.19.0" dependencies = [ "blake3", + "craft-pcraft-format", "crc32fast", "exacl", "flate2", diff --git a/Cargo.toml b/Cargo.toml index ef1f6d5fdb..4674a55e6f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ photocraft-ipp = { path = "crates/ipp" } craft-print = { git = "https://github.com/storytold/craft-libs", rev = "c5a1cbfb5f9049f9270955caffb5bee5997aebf7" } craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "c5a1cbfb5f9049f9270955caffb5bee5997aebf7" } craft-jpeg = { git = "https://github.com/storytold/craft-libs", rev = "c5a1cbfb5f9049f9270955caffb5bee5997aebf7" } +craft-pcraft-format = { git = "https://github.com/storytold/craft-libs", rev = "b8f7d93b00dfb8f172938f3d2d796ae62dcfe123" } # craft-update: app-specific pretend versions apply to startup and manual checks. craft-update = { git = "https://github.com/storytold/craft-libs", version = "=0.1.0", rev = "c5a1cbfb5f9049f9270955caffb5bee5997aebf7", features = ["egui"] } thiserror = "2" diff --git a/crates/format/Cargo.toml b/crates/format/Cargo.toml index 4267746633..a04875e443 100644 --- a/crates/format/Cargo.toml +++ b/crates/format/Cargo.toml @@ -15,6 +15,7 @@ photocraft-color = { workspace = true } photocraft-raster = { workspace = true } photocraft-doc = { workspace = true } photocraft-codecs = { workspace = true } +craft-pcraft-format = { workspace = true } serde = { workspace = true } serde_json = { workspace = true, features = ["unbounded_depth", "float_roundtrip"] } thiserror = { workspace = true } diff --git a/crates/format/src/autosave.rs b/crates/format/src/autosave.rs index 7b379451dd..2ab8676c66 100644 --- a/crates/format/src/autosave.rs +++ b/crates/format/src/autosave.rs @@ -451,7 +451,7 @@ fn sweep_orphans(recovery_dir: &Path, min_age: std::time::Duration) { } // Anything at the sidecar's path (even a broken one) keeps the bundle. let has_sidecar = std::fs::symlink_metadata(recovery_dir.join(format!("{key}.json"))).is_ok(); - let parts = [path.clone(), path.join("tiles"), path.join("blobs")]; + let parts = [path.clone(), path.join(craft_pcraft_format::TILES_DIR), path.join(craft_pcraft_format::BLOBS_DIR)]; // A committed bundle is recoverable work: keep exactly what `list_recovery` lists. if !has_sidecar && sidecarless_entry(recovery_dir, &path).is_none() && parts.iter().all(|p| untouched_for(p, min_age, true)) { let _ = std::fs::remove_dir_all(&path); diff --git a/crates/format/src/convert.rs b/crates/format/src/convert.rs index b5cfda9ad5..fbe121c153 100644 --- a/crates/format/src/convert.rs +++ b/crates/format/src/convert.rs @@ -81,9 +81,7 @@ fn key4(s: &str) -> Result<[u8; 4]> { unhex(s)?.try_into().map_err(|_| FormatError::corrupt(format!("bad 4-byte key `{s}`"))) } -pub(crate) fn is_valid_hash(h: &str) -> bool { - h.len() == 64 && h.bytes().all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) -} +pub(crate) use craft_pcraft_format::is_valid_hash; /// Convert native-endian sample bytes to little-endian in place (and back: /// the swap is an involution). @@ -110,7 +108,7 @@ fn surface_m(s: &Surface, sink: &mut dyn Sink) -> SurfaceM { let mut dp = vec![0u8; f.bytes_per_pixel()]; encode_pixel(&f, &s.default_pixel(), &mut dp); swap_to_le(&mut dp, f.sample); - SurfaceM { format: f, default: hex(&dp), tiles: s.tiles().map(|(c, t)| TileRef { tx: c.tx, ty: c.ty, hash: sink.tile(f, t) }).collect() } + SurfaceM { format: f, default: hex(&dp), tiles: s.tiles().map(|(c, t)| TileRef::new(c.tx, c.ty, sink.tile(f, t))).collect() } } fn opt_blob(b: &Option>>, sink: &mut dyn Sink) -> Option { diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 992e925ae9..cb1c5eb41d 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -50,7 +50,7 @@ pub use read::read_file; pub use store::{ByteSource, DEFAULT_REVERIFY_BUDGET, PcraftWriter, SaveStats, read_entry}; /// File extension of the native format. -pub const EXTENSION: &str = "pcraft"; +pub use craft_pcraft_format::EXTENSION; #[derive(Debug, thiserror::Error)] pub enum FormatError { @@ -76,6 +76,20 @@ impl FormatError { } } +impl From for FormatError { + fn from(e: craft_pcraft_format::Error) -> Self { + use craft_pcraft_format::Error; + match e { + Error::Corrupt(m) => FormatError::Corrupt(m), + Error::Unsupported(m) => FormatError::Unsupported(m), + Error::TooNew { found, supported, .. } => FormatError::TooNew { found, supported }, + Error::LimitExceeded(m) => FormatError::LimitExceeded(m), + Error::Json(e) => FormatError::Json(e), + other => FormatError::Corrupt(other.to_string()), + } + } +} + pub type Result = std::result::Result; /// Previews to embed. The format crate cannot render (it sits beside the @@ -119,10 +133,11 @@ pub const MAX_CANVAS_PIXELS: u64 = MAX_CANVAS_SIDE as u64 * MAX_CANVAS_SIDE as u impl Default for LoadOptions { fn default() -> Self { + let limits = craft_pcraft_format::Limits::default(); LoadOptions { - max_manifest_bytes: 256 << 20, - max_blob_bytes: 1 << 30, - max_total_bytes: 16 << 30, + max_manifest_bytes: limits.max_manifest_bytes, + max_blob_bytes: limits.max_blob_bytes, + max_total_bytes: limits.max_total_bytes, preserve_ids: true, max_canvas_side: MAX_CANVAS_SIDE, max_canvas_pixels: MAX_CANVAS_PIXELS, diff --git a/crates/format/src/manifest.rs b/crates/format/src/manifest.rs index 1164bc0a68..dd3a012ad2 100644 --- a/crates/format/src/manifest.rs +++ b/crates/format/src/manifest.rs @@ -14,24 +14,11 @@ use photocraft_doc::{ use photocraft_geom::{Affine, Size}; use serde::{Deserialize, Serialize}; -/// Current manifest version written by this build. -pub const FORMAT_VERSION: u32 = 1; +pub use craft_pcraft_format::{FORMAT_VERSION, Hash, TileRef}; -/// Hex-encoded BLAKE3 hash (64 chars). -pub type Hash = String; - -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct Manifest { - pub format_version: u32, - /// Free-form writer identification, e.g. `photocraft-format 0.1.0`. - pub generator: String, - pub document: DocM, - /// Optional previews present in the bundle. - #[serde(default)] - pub thumbnail: Option, - #[serde(default)] - pub composite: Option, -} +/// The manifest: craft-pcraft-format's envelope (`format_version`, `generator`, the optional +/// `thumbnail` and `composite` previews) around the document [`DocM`]. +pub type Manifest = craft_pcraft_format::Manifest; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct DocM { @@ -154,13 +141,6 @@ pub struct NamedPathM { pub psd_raw: Option, } -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct TileRef { - pub tx: i32, - pub ty: i32, - pub hash: Hash, -} - #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct SurfaceM { pub format: PixelFormat, diff --git a/crates/format/src/migrate.rs b/crates/format/src/migrate.rs index d24581275e..429b5bc281 100644 --- a/crates/format/src/migrate.rs +++ b/crates/format/src/migrate.rs @@ -1,43 +1,29 @@ //! Manifest migrations. Each step upgrades the raw JSON from version `n` to //! `n + 1`, so old bundles keep loading as the schema evolves. -use serde_json::Value; +use crate::FormatError; -use crate::manifest::FORMAT_VERSION; -use crate::{FormatError, Result}; - -pub(crate) type Step = fn(&mut Value) -> Result<()>; +pub(crate) type Step = craft_pcraft_format::Step; /// `STEPS[i]` upgrades version `i + 1` to `i + 2`. Empty while version 1 is -/// the only version. +/// the only version. `craft_pcraft_format::read_manifest_with` runs them on load. pub(crate) const STEPS: &[Step] = &[]; -/// Upgrade `v` in place to `target` using `steps`. -pub(crate) fn migrate_with(v: &mut Value, steps: &[Step], target: u32) -> Result { - let found = v.get("format_version").and_then(Value::as_u64).ok_or_else(|| FormatError::corrupt("manifest has no format_version"))?; - let found = u32::try_from(found).map_err(|_| FormatError::corrupt("format_version out of range"))?; - if found == 0 { - return Err(FormatError::corrupt("format_version 0 is invalid")); - } - if found > target { - return Err(FormatError::TooNew { found, supported: target }); - } - for ver in found..target { - let step = steps.get(ver as usize - 1).ok_or_else(|| FormatError::Unsupported(format!("no migration from format {ver}")))?; - step(v)?; - v["format_version"] = Value::from(ver + 1); - } - Ok(found) -} - -pub(crate) fn migrate(v: &mut Value) -> Result { - migrate_with(v, STEPS, FORMAT_VERSION) -} - #[cfg(test)] mod tests { use super::*; - use serde_json::json; + use crate::Result; + use crate::manifest::FORMAT_VERSION; + use serde_json::{Value, json}; + + /// Upgrade `v` in place to `target` using `steps`, with craft-pcraft-format's runner. + fn migrate_with(v: &mut Value, steps: &[Step], target: u32) -> Result { + craft_pcraft_format::migrate(v, steps, target) + } + + fn migrate(v: &mut Value) -> Result { + migrate_with(v, STEPS, FORMAT_VERSION) + } fn rename_title(v: &mut Value) -> Result<()> { let t = v["document"]["title"].take(); diff --git a/crates/format/src/store.rs b/crates/format/src/store.rs index 67bb8df27b..5fa9b55425 100644 --- a/crates/format/src/store.rs +++ b/crates/format/src/store.rs @@ -15,21 +15,12 @@ use crate::manifest::{ContentM, FORMAT_VERSION, Hash, LayerM, Manifest}; use crate::zip::{ZipReader, ZipWriter}; use crate::{FormatError, LoadOptions, Result, SaveOptions}; -pub(crate) const MANIFEST: &str = "manifest.json"; -pub(crate) const THUMB: &str = "thumb.png"; -pub(crate) const COMPOSITE: &str = "composite/preview.png"; -/// The file a directory save holds an exclusive lock on for its whole transaction (#4957). -pub(crate) const SAVE_LOCK: &str = "save.lock"; +use craft_pcraft_format::{BLOBS_DIR, COMPOSITE_DIR, TILES_DIR, blob_path, tile_path}; +// `SAVE_LOCK` is the file a directory save holds an exclusive lock on for its whole transaction (#4957). +pub(crate) use craft_pcraft_format::{COMPOSITE, MANIFEST, SAVE_LOCK, THUMB}; /// How long a directory save waits for another save of the same bundle to finish. const SAVE_LOCK_WAIT: std::time::Duration = std::time::Duration::from_secs(10); -fn tile_path(h: &str) -> String { - format!("tiles/{h}.zst") -} -fn blob_path(h: &str) -> String { - format!("blobs/{h}.zst") -} - /// The bytes an [`Object`] stores, owned by (or shared with) the compressor's source so one /// [`Compressor`] can be reused for objects of any lifetime. enum Source { @@ -228,64 +219,27 @@ pub fn read_entry(file: &mut impl std::io::Read, len: u64, max: usize, path: &st // Loading // --------------------------------------------------------------------------- -/// Deepest JSON nesting a manifest may have: each group level adds three (the layer object, its -/// content object and the `children` array), with headroom for the document and leaf layer fields. -/// The saver checks the same bound, so every bundle it writes loads again. -const MAX_MANIFEST_DEPTH: usize = 3 * (convert::MAX_GROUP_DEPTH + 1) + 64; - -/// Most JSON items (arrays and objects opened, plus elements and members after a comma) a -/// manifest may hold. Parsing builds a `serde_json::Value` tree of tens of bytes per item, so -/// `[0,0,…]` costs about 16× its size in memory; a typical manifest spends 20 bytes or more per -/// item, so this still admits manifests of about 80 MiB. -const MAX_MANIFEST_ITEMS: usize = 4 << 20; - -/// Rejects JSON nested deeper than [`MAX_MANIFEST_DEPTH`] or holding more than -/// [`MAX_MANIFEST_ITEMS`] items with a linear scan (no recursion), so parsing with serde_json's -/// recursion limit disabled can't exhaust the stack, nor tiny values the memory, on hostile input. -fn check_manifest_depth(json: &[u8]) -> Result<()> { - let (mut depth, mut items, mut in_str, mut escaped) = (0usize, 0usize, false, false); - for &b in json { - if in_str { - match b { - _ if escaped => escaped = false, - b'\\' => escaped = true, - b'"' => in_str = false, - _ => {} - } - continue; - } - match b { - b'"' => in_str = true, - b'[' | b'{' => { - depth += 1; - if depth > MAX_MANIFEST_DEPTH { - return Err(FormatError::LimitExceeded(format!("manifest JSON nested deeper than {MAX_MANIFEST_DEPTH} levels"))); - } - } - b']' | b'}' => depth = depth.saturating_sub(1), - _ => {} - } - if matches!(b, b'[' | b'{' | b',') { - items += 1; - if items > MAX_MANIFEST_ITEMS { - return Err(FormatError::LimitExceeded(format!("manifest JSON holds more than {MAX_MANIFEST_ITEMS} values"))); - } - } - } - Ok(()) +/// The limits a manifest is checked against: craft-pcraft-format's item limit, the JSON nesting +/// of [`convert::MAX_GROUP_DEPTH`] nested groups ([`craft_pcraft_format::manifest_depth`]) and at +/// most `max_manifest_bytes` bytes. The saver checks the same nesting and items, so every bundle +/// it writes loads again. +fn manifest_limits(max_manifest_bytes: usize) -> craft_pcraft_format::Limits { + let mut limits = craft_pcraft_format::Limits::default(); + limits.max_manifest_bytes = max_manifest_bytes; + limits.max_depth = craft_pcraft_format::manifest_depth(convert::MAX_GROUP_DEPTH); + limits } pub(crate) fn read_manifest(src: &dyn ByteSource, opts: &LoadOptions) -> Result { let raw = src.get(MANIFEST, opts.max_manifest_bytes)?; - // serde_json stops at 128 levels (about 40 nested groups); the bounded check replaces that limit. - check_manifest_depth(&raw)?; - let mut de = serde_json::Deserializer::from_slice(&raw); - de.disable_recursion_limit(); - let mut v = ::deserialize(&mut de)?; - de.end()?; - crate::migrate::migrate(&mut v)?; - let layers = take_array(&mut v, "/document/layers"); - let mut m: Manifest = serde_json::from_value(v)?; + // serde_json stops at 128 levels (about 40 nested groups); `read_manifest_with` checks the + // nesting and the items in one pass, then parses without that limit. + let limits = manifest_limits(opts.max_manifest_bytes); + let mut layers = Vec::new(); + let mut m: Manifest = craft_pcraft_format::read_manifest_with(&raw, &limits, crate::migrate::STEPS, FORMAT_VERSION, |mut document| { + layers = take_array(&mut document, "/layers"); + Ok::<_, FormatError>(serde_json::from_value(document)?) + })?; m.document.layers = layers_from_json(layers)?; // A number past f32's range decodes as infinity, which no save could write back (#6862). if let Err(crate::finite::Problem::NonFinite { path }) = crate::finite::check(&m) { @@ -342,11 +296,10 @@ impl LoadFetch<'_> { impl Fetch for LoadFetch<'_> { fn tile(&mut self, hash: &str, len: usize) -> Result> { - if !is_valid_hash(hash) { + let Some(path) = tile_path(hash) else { return Err(FormatError::corrupt(format!("invalid tile hash `{hash}`"))); - } + }; self.account(len)?; - let path = tile_path(hash); let z = self.src.get(&path, len + len / 8 + 4096)?; let data = decompress(&z, len, &path)?; if hash_bytes(&data) != hash { @@ -367,10 +320,9 @@ impl Fetch for LoadFetch<'_> { if let Some(b) = self.blobs.get(hash) { return if b.len() <= limit { Ok(b.clone()) } else { Err(FormatError::LimitExceeded("optional preview blob exceeds its limit".into())) }; } - if !is_valid_hash(hash) { + let Some(path) = blob_path(hash) else { return Err(FormatError::corrupt(format!("invalid blob hash `{hash}`"))); - } - let path = blob_path(hash); + }; // The stored zstd frame of incompressible data is a little larger than the data, so a // blob right at `limit` must still be read; `decompress_bounded` enforces `limit`. let z = self.src.get(&path, limit.saturating_add(limit / 8).saturating_add(4096))?; @@ -686,7 +638,7 @@ fn bundle_root(dir: &Path) -> Result { std::fs::create_dir_all(&dir)?; let root = std::fs::canonicalize(&dir)?; plain_directory_or_missing(&root)?; - for sub in ["tiles", "blobs", "composite"] { + for sub in [TILES_DIR, BLOBS_DIR, COMPOSITE_DIR] { let path = root.join(sub); plain_directory_or_missing(&path)?; if !path.exists() { @@ -706,7 +658,7 @@ fn bundle_root(dir: &Path) -> Result { fn validate_bundle_file(root: &Path, path: &Path) -> Result<()> { let parent = path.parent().ok_or_else(|| unsafe_bundle_entry(path))?; - if parent != root && parent != root.join("tiles") && parent != root.join("blobs") && parent != root.join("composite") { + if parent != root && parent != root.join(TILES_DIR) && parent != root.join(BLOBS_DIR) && parent != root.join(COMPOSITE_DIR) { return Err(unsafe_bundle_entry(path)); } plain_directory_or_missing(parent)?; @@ -863,9 +815,9 @@ impl BundleGc { let flags = gc_directory_flags(); let root = open(root, flags, Mode::empty()).map_err(std::io::Error::from)?; - let tiles = openat(&root, "tiles", flags, Mode::empty()).map_err(std::io::Error::from)?; - let blobs = openat(&root, "blobs", flags, Mode::empty()).map_err(std::io::Error::from)?; - let composite = openat(&root, "composite", flags, Mode::empty()).map_err(std::io::Error::from)?; + let tiles = openat(&root, TILES_DIR, flags, Mode::empty()).map_err(std::io::Error::from)?; + let blobs = openat(&root, BLOBS_DIR, flags, Mode::empty()).map_err(std::io::Error::from)?; + let composite = openat(&root, COMPOSITE_DIR, flags, Mode::empty()).map_err(std::io::Error::from)?; Ok(Self { root, tiles, blobs, composite }) } @@ -875,8 +827,8 @@ impl BundleGc { THUMB => (&self.root, THUMB), COMPOSITE => (&self.composite, "preview.png"), _ => match relative.split_once('/') { - Some(("tiles", name)) if name.strip_suffix(".zst").is_some_and(is_valid_hash) => (&self.tiles, name), - Some(("blobs", name)) if name.strip_suffix(".zst").is_some_and(is_valid_hash) => (&self.blobs, name), + Some((TILES_DIR, name)) if name.strip_suffix(craft_pcraft_format::OBJECT_SUFFIX).is_some_and(is_valid_hash) => (&self.tiles, name), + Some((BLOBS_DIR, name)) if name.strip_suffix(craft_pcraft_format::OBJECT_SUFFIX).is_some_and(is_valid_hash) => (&self.blobs, name), _ => return Err(FormatError::Unsupported(format!("invalid bundle cleanup target: {relative}"))), }, }; @@ -1000,13 +952,9 @@ 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 manifest = Manifest { - format_version: FORMAT_VERSION, - generator: format!("photocraft-format {}", env!("CARGO_PKG_VERSION")), - document, - thumbnail: opts.thumbnail.as_ref().map(|_| THUMB.to_owned()), - composite: opts.composite.as_ref().map(|_| COMPOSITE.to_owned()), - }; + let mut manifest = Manifest::new(format!("photocraft-format {}", env!("CARGO_PKG_VERSION")), document); + manifest.thumbnail = opts.thumbnail.as_ref().map(|_| THUMB.to_owned()); + manifest.composite = opts.composite.as_ref().map(|_| COMPOSITE.to_owned()); // serde_json writes NaN and the infinities as `null`, which the manifest's float fields // refuse on load (issue #1101): refuse the save instead, naming the value. crate::finite::check(&manifest).map_err(|e| match e { @@ -1014,14 +962,16 @@ impl PcraftWriter { crate::finite::Problem::Other(m) => FormatError::Unsupported(format!("manifest: {m}")), })?; let manifest = serde_json::to_vec_pretty(&manifest)?; - check_manifest_depth(&manifest)?; + // The nesting and the items only: a save checks no manifest size. + craft_pcraft_format::check_manifest(&manifest, &manifest_limits(usize::MAX))?; let stats = SaveStats { tiles_total: c.tiles.len(), blobs_total: c.blobs.len(), manifest_bytes: manifest.len(), ..Default::default() }; + let invalid = |h: &str| FormatError::corrupt(format!("invalid object hash `{h}`")); let mut objects = BTreeMap::new(); for (h, (t, s)) in c.tiles { - objects.insert(tile_path(&h), Object::Tile(t, s)); + objects.insert(tile_path(&h).ok_or_else(|| invalid(&h))?, Object::Tile(t, s)); } for (h, b) in c.blobs { - objects.insert(blob_path(&h), Object::Blob(b)); + objects.insert(blob_path(&h).ok_or_else(|| invalid(&h))?, Object::Blob(b)); } Ok(Prepared { manifest, objects, stats }) } @@ -1178,12 +1128,12 @@ impl PcraftWriter { // flush. The retry can reuse those valid bytes (including cached ones), so flush // both object directories again before publishing a manifest that references them. // This also persists the fixed subdirectories of a newly created bundle. - for sub in ["tiles", "blobs"] { + for sub in [TILES_DIR, BLOBS_DIR] { fs.sync_dir(&dir.join(sub)).map_err(|e| crate::atomic::publication_error(e, &dir.join(sub)))?; } } #[cfg(not(windows))] - for sub in ["tiles", "blobs"] { + for sub in [TILES_DIR, BLOBS_DIR] { let folder = dir.join(sub); if folder.is_dir() { confirm_flushed(&folder, &folder)?; @@ -1560,7 +1510,7 @@ fn an_incompressible_blob_at_its_limit_still_loads() { let z = Compressor::new().object(&Object::Blob(Arc::new(data.clone()))); assert!(z.len() > data.len(), "the fixture must not compress"); let hash = hash_bytes(&data); - let src = One(blob_path(&hash), z); + let src = One(blob_path(&hash).unwrap(), z); let mut fetch = LoadFetch { src: &src, opts: crate::LoadOptions::default(), total: 0, blobs: HashMap::new() }; assert_eq!(fetch.blob_bounded(&hash, data.len()).unwrap().as_slice(), data.as_slice()); assert!(LoadFetch { src: &src, opts: crate::LoadOptions::default(), total: 0, blobs: HashMap::new() }.blob_bounded(&hash, data.len() - 1).is_err()); diff --git a/crates/format/tests/it/roundtrip.rs b/crates/format/tests/it/roundtrip.rs index 25af8b22a6..be016f0aaf 100644 --- a/crates/format/tests/it/roundtrip.rs +++ b/crates/format/tests/it/roundtrip.rs @@ -216,6 +216,13 @@ fn nesting_beyond_the_load_limit_is_refused_at_save() { assert!(matches!(e, FormatError::LimitExceeded(_)), "{e}"); } +/// craft-pcraft-format's default manifest nesting limit is computed from its own copy of the +/// group depth, so the two must stay equal. +#[test] +fn group_depth_matches_craft_pcraft_format() { + assert_eq!(MAX_GROUP_DEPTH, craft_pcraft_format::MAX_GROUP_DEPTH); +} + #[test] fn empty_document_roundtrips() { let doc = Document::new("empty", photocraft_doc::Size::new(1, 1), ColorMode::Rgb, SampleType::U8); @@ -524,6 +531,22 @@ fn save_is_deterministic() { assert_eq!(a, b); } +/// A bundle that is read and saved again gives the same bytes, previews included. +#[test] +fn save_load_save_gives_the_same_bytes() { + let mut preview = Rgba8Image::new(4, 3); + preview.pixels.iter_mut().enumerate().for_each(|(i, p)| *p = i as u8); + for opts in [SaveOptions::default(), SaveOptions { thumbnail: Some(preview.clone()), composite: Some(preview.clone()) }] { + for mode in MODES { + for depth in SampleType::ALL { + let first = save_to_bytes(&rich_doc(mode, depth), &opts).unwrap(); + let again = save_to_bytes(&load_from_bytes(&first).unwrap(), &opts).unwrap(); + assert!(first == again, "{mode:?} {depth:?}: the second save differs"); + } + } + } +} + #[test] fn not_pcraft() { assert!(!is_pcraft(b"")); From 9034223c4b6d80fe15a12b9e0d173786ac5f7a4c Mon Sep 17 00:00:00 2001 From: Yuki-init Date: Mon, 2 Nov 2026 18:31:22 +0000 Subject: [PATCH 4/4] TIFF: open YCbCr images as RGB (#10316) (#10319) * TIFF: open YCbCr images as RGB (#10316) Scanners often write JPEG-compressed YCbCr TIFFs with 2x2 chroma subsampling. The tiff crate decodes and upsamples them but returns raw YCbCr samples, which the decoder rejected with "colour type YCbCr(8)". The new tiff/ycbcr.rs converts them to RGB after decoding: JPEG data uses the JFIF full-range BT.601 conversion (as libtiff does), other compressions honour ReferenceBlackWhite and YCbCrCoefficients with the TIFF defaults as fallback. 8 and 16-bit. YCbCr with extra samples, or in other sample formats, fails with a clear unsupported error. Co-Authored-By: Claude Opus 5.5 * Contributors: add Yuki-init's display name Co-Authored-By: Claude Opus 5.5 * TIFF YCbCr: test that YCbCrCoefficients set the luma weights Co-Authored-By: Claude Opus 5.5 * TIFF: YCbCr tags with the wrong value count use the defaults tag_f64s now returns None unless the tag holds exactly n values, so a YCbCrCoefficients or ReferenceBlackWhite tag with extra values falls back to the TIFF defaults instead of being truncated. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Lrgj5pnShYtsSnrsZq5iRn * TIFF YCbCr: tests for tiled JPEG with ICC, big-endian 16-bit and planar Covers the layouts the review asked for: a tiled 4:2:0 JPEG file with an embedded profile (checked against an independent JPEG decoder), a big-endian 16-bit file, and planar data in both byte orders. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Lrgj5pnShYtsSnrsZq5iRn * TIFF YCbCr: cite the formula sources and pin the green-channel choice Documents where the conversion comes from: TIFF 6.0 Section 21 (YCbCrCoefficients, pp. 90-91) and libtiff's tif_color.c (TIFFYCbCrToRGBInit, TIFFYCbCrtoRGB). Red and blue follow the spec; green follows libtiff and adds Y unscaled, which matches the spec only when the luma weights sum to 1. The module doc records this as a deliberate compatibility choice, and a new test pins it: with weights 0.25/0.25/0.25, mid gray stays (128, 128, 128). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Lrgj5pnShYtsSnrsZq5iRn * Codecs README: YCbCr TIFFs with three 8- or 16-bit samples decode to RGB Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Lrgj5pnShYtsSnrsZq5iRn * Format tiff_ycbcr tests with rustfmt Co-Authored-By: Claude Opus 5.5 * test(tiff): compare grouped YCbCr decoding and update support notes * style(tiff): align grouped decode call with test formatting * style(tiff): apply repository rustfmt to grouped YCbCr regression --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> Co-authored-by: FredApps --- contributors/people.toml | 4 + crates/codecs/README.md | 6 +- crates/codecs/src/codecs/tiff.rs | 1 + crates/codecs/src/codecs/tiff/decode.rs | 38 +++- crates/codecs/src/codecs/tiff/ycbcr.rs | 222 ++++++++++++++++++++ crates/codecs/src/codecs/tiff_ifd.rs | 25 ++- crates/codecs/tests/it/main.rs | 1 + crates/codecs/tests/it/tiff_ycbcr.rs | 259 ++++++++++++++++++++++++ docs/file-format-parity.md | 5 +- 9 files changed, 547 insertions(+), 14 deletions(-) create mode 100644 crates/codecs/src/codecs/tiff/ycbcr.rs create mode 100644 crates/codecs/tests/it/tiff_ycbcr.rs diff --git a/contributors/people.toml b/contributors/people.toml index e1c91b3049..f8dece1fdc 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -291,6 +291,10 @@ sync_github_name = true [people.dylanmtaylor] sync_github_name = true +[people.Yuki-init] +display_name = "DC@yuki.himejima" +sync_github_name = false + [people.sn-o-w] real_name = "Cristian Silaghi" sync_github_name = false diff --git a/crates/codecs/README.md b/crates/codecs/README.md index 43bf4ea361..ffd7b655b1 100644 --- a/crates/codecs/README.md +++ b/crates/codecs/README.md @@ -254,8 +254,10 @@ embedded table (and a PNG's transparent entry) so `photocraft-io` can open them ms in release). Image data cut off by the end of the file decodes with `DecodeWarning::Truncated`; a file far too small for its declared size (more than any compression could expand) is refused before allocating. - * YCbCr, CIE Lab, signed integers and compressions that the `tiff` crate lacks (CCITT, - JPEG-in-TIFF) return `Unsupported`. + * YCbCr with three 8- or 16-bit samples decodes to RGB. YCbCr with other sample counts or + bit depths returns `Unsupported`. JPEG-compressed YCbCr uses the full-range JFIF conversion. + * 8/16-bit CIELab and ICCLab decode to the Lab channel layout. + * Signed integer samples and unsupported compression methods return `Unsupported`. * **TIFF encode** writes classic TIFF, or BigTIFF when `EncodeOptions::tiff_bigtiff` is set or the file could pass 4 GiB. `Metadata::extra_samples` (up to 16) are written after the colour and alpha samples, with their `kind` in the ExtraSamples tag (a reserved `kind` above 2 is diff --git a/crates/codecs/src/codecs/tiff.rs b/crates/codecs/src/codecs/tiff.rs index a04460ea1a..8b2d5d4e8f 100644 --- a/crates/codecs/src/codecs/tiff.rs +++ b/crates/codecs/src/codecs/tiff.rs @@ -168,6 +168,7 @@ mod bands; mod decode; mod encode; mod tags; +mod ycbcr; pub use decode::TiffBands; pub(crate) use decode::{decode, decode_page, orientation}; diff --git a/crates/codecs/src/codecs/tiff/decode.rs b/crates/codecs/src/codecs/tiff/decode.rs index f698f04039..9747b7c2f0 100644 --- a/crates/codecs/src/codecs/tiff/decode.rs +++ b/crates/codecs/src/codecs/tiff/decode.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use super::bands::{Tally, convert_ab, decode_bands, extra_sample_indices, refill_zeroed, split_extra_samples, zeroed}; use super::tags::{F, TAG_IMAGE_SOURCE_DATA, TAG_PHOTOSHOP, TAG_XMP, TEXT_TAGS, photoshop_tags}; +use super::ycbcr::YCbCr; use super::{ ChannelLayout, ChunkType, CodecError, DecodeWarning, Decoder, ExifDirs, File, Ifd, Image, Limits, Metadata, Read, SampleType, Seek, SeekFrom, Tag, camera_raw, err, ifd, io, map_err, tiff_exif, @@ -185,6 +186,9 @@ impl<'a> TiffBands<'a> { // Missing chunks leave their rows as they are: they must start empty in every group. buf = refill_zeroed(buf, len)?; tally = tally + decode_bands(p, self.b, &mut buf, band, first, &mut [], 0)?; + if let Some(c) = p.ycbcr { + c.convert(&mut buf, p.lay.out_channels(), p.lay.out); + } if let Some(signed) = p.lab { convert_ab(&mut buf, p.lay.out_channels(), p.lay.out, signed, false); } @@ -520,6 +524,20 @@ fn header_patches( Ok(patches) } +/// Lab and YCbCr are converted after decoding, from 8 or 16-bit samples only: Lab needs at least +/// its three colour samples, YCbCr exactly three. +fn check_converted_model(photometric: u64, spp: u64, bits: u64, src: Src) -> Result<(), CodecError> { + let (name, samples_ok) = match photometric { + 8 | 9 => ("Lab", spp >= 3), + 6 => ("YCbCr", spp == 3), + _ => return Ok(()), + }; + if samples_ok && matches!(src, Src::U8 | Src::U16) { + return Ok(()); + } + Err(CodecError::unsupported(F, format!("{name} image with {spp} samples of {bits} bits"))) +} + /// A directory checked and laid out for decoding, before any pixel is read. pub(super) struct Prepared { w: u32, @@ -531,6 +549,8 @@ pub(super) struct Prepared { tl: tiff::decoder::Limits, /// Lab samples: `Some(true)` for CIELab (a* and b* stored signed), `Some(false)` for ICCLab. lab: Option, + /// YCbCr samples: the conversion to RGB (#10316). + ycbcr: Option, /// The colour samples of a pixel in the file's colour model. color: usize, /// The ExtraSamples tag (0 unspecified, 1 associated alpha, 2 unassociated alpha). @@ -567,6 +587,9 @@ fn decode_ifd(f: &File<'_>, dir: &Ifd, limits: &Limits) -> Result 0 { return Err(err("the image data is missing (cut off or past the end of the file)")); } + if let Some(c) = p.ycbcr { + c.convert(&mut data, p.lay.out_channels(), p.lay.out); + } if let Some(signed) = p.lab { convert_ab(&mut data, p.lay.out_channels(), p.lay.out, signed, false); } @@ -642,15 +665,17 @@ fn prepare(f: &File<'_>, dir: &Ifd, limits: &Limits, whole: bool) -> Result 3, + 2 | 6 | 8 | 9 => 3, 5 => 4, _ => 1, }; @@ -686,6 +711,7 @@ fn prepare(f: &File<'_>, dir: &Ifd, limits: &Limits, whole: bool) -> Result (ChannelLayout::Rgb, 3), + tiff::ColorType::YCbCr(_) if ycbcr => (ChannelLayout::Rgb, 3), tiff::ColorType::RGBA(_) => (ChannelLayout::Rgba, 4), tiff::ColorType::CMYK(_) => (ChannelLayout::Cmyk, 4), tiff::ColorType::CMYKA(_) => (ChannelLayout::CmykA, 5), @@ -729,8 +755,9 @@ fn prepare(f: &File<'_>, dir: &Ifd, limits: &Limits, whole: bool) -> Result 4, 32773 => 130, 4 => cw.clamp(4096, 65_535), @@ -814,6 +841,7 @@ fn prepare(f: &File<'_>, dir: &Ifd, limits: &Limits, whole: bool) -> Result +//! - libtiff, `libtiff/tif_color.c`: `TIFFYCbCrToRGBInit` (tables) and `TIFFYCbCrtoRGB` (per pixel). +//! +//! +//! # Green: a compatibility choice +//! +//! Red and blue follow the spec. Green follows libtiff, which rewrites the spec's equation as +//! `G = Y - (LumaRed·(2 - 2·LumaRed) / LumaGreen)·Cr - (LumaBlue·(2 - 2·LumaBlue) / LumaGreen)·Cb`, +//! adding Y unscaled. The two agree only when the weights sum to 1. Every standard set does +//! (the spec's own examples, BT.601 and BT.709), but the spec only calls the weights "the +//! proportions of red, green, and blue respectively in luminance" and does not require the sum. +//! For other weights the spec's equation scales Y in green by `(1 - LumaRed - LumaBlue) / LumaGreen`, +//! which tints neutral grays. +//! +//! PhotoCraft deliberately follows libtiff here (decided in PR #10319, for issue #10316), so such +//! files open as they do in libtiff-based tools (ImageMagick, GIMP) and grays stay neutral. This is +//! a contributor choice, not a requirement of the spec; the test +//! `weights_that_do_not_sum_to_one_keep_grays_neutral` in `tests/it/tiff_ycbcr.rs` pins it. + +use super::{File, Ifd, SampleType}; + +const TAG_YCBCR_COEFFICIENTS: u16 = 529; +const TAG_REFERENCE_BLACK_WHITE: u16 = 532; + +/// BT.601 luma weights: the TIFF default for YCbCrCoefficients. +const BT601: [f64; 3] = [0.299, 0.587, 0.114]; + +/// How one file's YCbCr codes map to RGB. +#[derive(Clone, Copy, Debug, PartialEq)] +pub(super) struct YCbCr { + /// Luma weights of red, green and blue. + luma: [f64; 3], + /// Black and white codes of Y, Cb and Cr. + ref_bw: [f64; 6], +} + +impl YCbCr { + /// The conversion for directory `dir`, whose samples have `max` as their largest code + /// (255 or 65535). Missing or unusable tags fall back to the TIFF defaults. + pub(super) fn of(f: &File<'_>, dir: &Ifd, compression: u64, max: f64) -> Self { + let defaults = Self::defaults(max); + if compression == 7 { + return defaults; + } + let luma = f + .tag_f64s(dir, TAG_YCBCR_COEFFICIENTS, 3) + .and_then(|v| <[f64; 3]>::try_from(v).ok()) + .filter(|l| l.iter().all(|c| c.is_finite() && *c > 0.0 && *c < 1.0)) + .unwrap_or(defaults.luma); + let ref_bw = f + .tag_f64s(dir, TAG_REFERENCE_BLACK_WHITE, 6) + .and_then(|v| <[f64; 6]>::try_from(v).ok()) + .filter(|r| r.iter().all(|v| v.is_finite()) && r.as_chunks::<2>().0.iter().all(|[black, white]| (white - black).abs() > f64::EPSILON)) + .unwrap_or(defaults.ref_bw); + YCbCr { luma, ref_bw } + } + + /// TIFF's defaults: BT.601 weights, full-range luma, chroma centred on the middle code. + fn defaults(max: f64) -> Self { + let mid = (max + 1.0) / 2.0; + YCbCr { luma: BT601, ref_bw: [0.0, max, mid, max, mid, max] } + } + + /// Converts interleaved Y, Cb, Cr pixels of `channels` samples (the first three are the + /// colour) to R, G, B in place. Other sample types are left as they are. + pub(super) fn convert(self, data: &mut [u8], channels: usize, sample: SampleType) { + match sample { + SampleType::U8 => { + let t = Tables::new(&self, 255); + for_each_pixel(data, channels, |px| { + if let [y, cb, cr, ..] = px { + let [r, g, b] = t.rgb(usize::from(*y), usize::from(*cb), usize::from(*cr)); + (*y, *cb, *cr) = (r as u8, g as u8, b as u8); + } + }); + } + SampleType::U16 => { + let t = Tables::new(&self, 65535); + for_each_pixel(data, channels * 2, |px| { + if let [y0, y1, b0, b1, r0, r1, ..] = px { + let get = |a: u8, b: u8| usize::from(u16::from_ne_bytes([a, b])); + let [r, g, b] = t.rgb(get(*y0, *y1), get(*b0, *b1), get(*r0, *r1)); + [*y0, *y1] = (r as u16).to_ne_bytes(); + [*b0, *b1] = (g as u16).to_ne_bytes(); + [*r0, *r1] = (b as u16).to_ne_bytes(); + } + }); + } + _ => {} + } + } +} + +/// Runs `f` on every `stride`-byte pixel, in parallel on native targets. +fn for_each_pixel(data: &mut [u8], stride: usize, f: impl Fn(&mut [u8]) + Sync + Send) { + if stride == 0 { + return; + } + #[cfg(not(target_arch = "wasm32"))] + { + use rayon::prelude::*; + // Bands of whole pixels, so each task has a useful amount of work. + let band = stride.saturating_mul(1 << 14); + data.par_chunks_mut(band).for_each(|b| b.chunks_exact_mut(stride).for_each(&f)); + } + #[cfg(target_arch = "wasm32")] + data.chunks_exact_mut(stride).for_each(f); +} + +/// Per-code contributions, so a pixel costs a few additions. +struct Tables { + max: f64, + /// Scaled luma of each Y code. + y: Vec, + /// Red and green contributions of each Cr code. + cr_r: Vec, + cr_g: Vec, + /// Green and blue contributions of each Cb code. + cb_g: Vec, + cb_b: Vec, +} + +impl Tables { + fn new(c: &YCbCr, max: u32) -> Self { + let m = f64::from(max); + let [lr, lg, lb] = c.luma; + let [y_black, y_white, cb_black, cb_white, cr_black, cr_white] = c.ref_bw; + // TIFF 6.0: chroma codes span ±(max + 1) / 2 - 1 around the black code; with the + // default ReferenceBlackWhite that is the code minus the middle code. + let half = (m + 1.0) / 2.0 - 1.0; + let codes = || (0..=max).map(f64::from); + let y = codes().map(|v| (v - y_black) * m / (y_white - y_black)).collect(); + let cb: Vec = codes().map(|v| (v - cb_black) * half / (cb_white - cb_black)).collect(); + let cr: Vec = codes().map(|v| (v - cr_black) * half / (cr_white - cr_black)).collect(); + // R = Y + (2 - 2·Lr)·Cr and B = Y + (2 - 2·Lb)·Cb (TIFF 6.0 p. 91); green as libtiff: + // G = Y - (Lr·(2 - 2·Lr) / Lg)·Cr - (Lb·(2 - 2·Lb) / Lg)·Cb (see the module notes). + let (kr, kb) = (2.0 - 2.0 * lr, 2.0 - 2.0 * lb); + Tables { + max: m, + y, + cr_r: cr.iter().map(|v| kr * v).collect(), + cr_g: cr.iter().map(|v| -lr * kr * v / lg).collect(), + cb_g: cb.iter().map(|v| -lb * kb * v / lg).collect(), + cb_b: cb.iter().map(|v| kb * v).collect(), + } + } + + /// R, G and B of one pixel, rounded and clamped to the code range. + fn rgb(&self, y: usize, cb: usize, cr: usize) -> [u32; 3] { + let at = |t: &[f64], i: usize| t.get(i).copied().unwrap_or(0.0); + let yv = at(&self.y, y); + // Green adds Y unscaled, as libtiff does (see "Green: a compatibility choice" above). + let r = yv + at(&self.cr_r, cr); + let g = yv + at(&self.cr_g, cr) + at(&self.cb_g, cb); + let b = yv + at(&self.cb_b, cb); + [r, g, b].map(|v| v.round().clamp(0.0, self.max) as u32) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn convert(c: &YCbCr, px: [u8; 3]) -> [u8; 3] { + let mut d = px.to_vec(); + c.convert(&mut d, 3, SampleType::U8); + [d[0], d[1], d[2]] + } + + #[test] + fn jfif_conversion_matches_known_colours() { + let c = YCbCr::defaults(255.0); + assert_eq!(convert(&c, [0, 128, 128]), [0, 0, 0]); + assert_eq!(convert(&c, [255, 128, 128]), [255, 255, 255]); + assert_eq!(convert(&c, [128, 128, 128]), [128, 128, 128]); + // Pure red, green and blue as JFIF encodes them. + assert_eq!(convert(&c, [76, 85, 255]), [254, 0, 0]); + assert_eq!(convert(&c, [150, 44, 21]), [0, 255, 1]); + assert_eq!(convert(&c, [29, 255, 107]), [0, 0, 254]); + } + + #[test] + fn sixteen_bit_and_extra_samples() { + let c = YCbCr::defaults(65535.0); + let mut d: Vec = [65535u16, 32768, 32768, 1234].iter().flat_map(|v| v.to_ne_bytes()).collect(); + c.convert(&mut d, 4, SampleType::U16); + let v: Vec = d.as_chunks::<2>().0.iter().map(|b| u16::from_ne_bytes(*b)).collect(); + assert_eq!(v, [65535, 65535, 65535, 1234]); + } + + #[test] + fn studio_range_reference_black_white() { + // Y 16–235, chroma 128 ± 112 (ITU-R BT.601 studio range). + let c = YCbCr { luma: BT601, ref_bw: [16.0, 235.0, 128.0, 240.0, 128.0, 240.0] }; + assert_eq!(convert(&c, [16, 128, 128]), [0, 0, 0]); + assert_eq!(convert(&c, [235, 128, 128]), [255, 255, 255]); + } + + #[test] + fn odd_buffers_do_not_panic() { + let c = YCbCr::defaults(255.0); + let mut d = vec![1u8, 2]; + c.convert(&mut d, 3, SampleType::U8); + c.convert(&mut d, 0, SampleType::U8); + c.convert(&mut d, 0, SampleType::U16); + assert_eq!(d, [1, 2]); + } +} diff --git a/crates/codecs/src/codecs/tiff_ifd.rs b/crates/codecs/src/codecs/tiff_ifd.rs index 63b39258cf..cdaa890ec4 100644 --- a/crates/codecs/src/codecs/tiff_ifd.rs +++ b/crates/codecs/src/codecs/tiff_ifd.rs @@ -236,18 +236,33 @@ impl<'a> File<'a> { /// The first value of a numeric tag as a float: RATIONAL, FLOAT, DOUBLE or an integer. pub fn tag_f64(&self, ifd: &Ifd, tag: u16) -> Option { + self.float(ifd.get(tag)?, 0) + } + + /// The values of a numeric tag as floats, when it holds exactly `n` (`None` for any other + /// count or an unreadable value). + pub fn tag_f64s(&self, ifd: &Ifd, tag: u16, n: u64) -> Option> { let e = ifd.get(tag)?; - if e.count == 0 { + if e.count != n { return None; } + (0..n).map(|i| self.float(e, i)).collect() + } + + /// Value `i` of a numeric entry as a float: RATIONAL, FLOAT, DOUBLE or an integer. + fn float(&self, e: &Entry, i: u64) -> Option { + if i >= e.count { + return None; + } + let at = e.value_at.checked_add(i.checked_mul(type_size(e.ty)?)?)?; match e.ty { 5 => { - let (n, d) = (self.u32_at(e.value_at)?, self.u32_at(e.value_at.checked_add(4)?)?); + let (n, d) = (self.u32_at(at)?, self.u32_at(at.checked_add(4)?)?); (d != 0).then(|| f64::from(n) / f64::from(d)) } - 11 => Some(f64::from(f32::from_bits(self.u32_at(e.value_at)?))), - 12 => Some(f64::from_bits(self.u64_at(e.value_at)?)), - _ => self.uint(e, 0).map(|v| v as f64), + 11 => Some(f64::from(f32::from_bits(self.u32_at(at)?))), + 12 => Some(f64::from_bits(self.u64_at(at)?)), + _ => self.uint(e, i).map(|v| v as f64), } } diff --git a/crates/codecs/tests/it/main.rs b/crates/codecs/tests/it/main.rs index a2ed94616a..4293fb4a48 100644 --- a/crates/codecs/tests/it/main.rs +++ b/crates/codecs/tests/it/main.rs @@ -31,6 +31,7 @@ mod tiff_lab; mod tiff_pages; mod tiff_photoshop; mod tiff_regressions; +mod tiff_ycbcr; mod webp_frames; mod webp_lossy; mod xpm; diff --git a/crates/codecs/tests/it/tiff_ycbcr.rs b/crates/codecs/tests/it/tiff_ycbcr.rs new file mode 100644 index 0000000000..94ff9f87e2 --- /dev/null +++ b/crates/codecs/tests/it/tiff_ycbcr.rs @@ -0,0 +1,259 @@ +//! YCbCr TIFFs (PhotometricInterpretation 6), as scanners write them: JPEG-compressed with 2×2 +//! chroma subsampling, and uncompressed with ReferenceBlackWhite and YCbCrCoefficients (#10316). +//! Every file here is hand-built by `common::tiffgen`. + +use crate::common::sample_icc; +use crate::common::tiffgen::*; +use photocraft_codecs::*; + +const COMPRESSION: u16 = 259; +const SUBSAMPLING: u16 = 530; +const YCBCR_COEFFICIENTS: u16 = 529; +const REFERENCE_BLACK_WHITE: u16 = 532; +const ICC_PROFILE: u16 = 34675; + +/// A directory whose Compression is `c`. +fn compressed(mut d: Dir, c: u64) -> Dir { + d.entries.retain(|e| e.0 != COMPRESSION); + d.tag(COMPRESSION, SHORT, &[c]) +} + +/// A little-endian RATIONAL value for `tiffgen`. +fn rational(num: u32, den: u32) -> u64 { + u64::from(num) | (u64::from(den) << 32) +} + +/// The image's pixels as RGB bytes. +fn rgb(img: &Image) -> Vec { + img.to_rgba8().as_chunks::<4>().0.iter().flat_map(|[r, g, b, _]| [*r, *g, *b]).collect() +} + +/// A smooth colour test card, `w`×`h` RGB. +fn card(w: u32, h: u32) -> Vec { + let mut v = Vec::new(); + for y in 0..h { + for x in 0..w { + v.extend([(x * 255 / (w - 1)) as u8, (y * 255 / (h - 1)) as u8, (255 - x * 255 / (w - 1)) as u8]); + } + } + v +} + +/// A 4:2:0 baseline JPEG stream of `rgb`. +fn jpeg(rgb: &[u8], w: u32, h: u32) -> Vec { + let img = Image::from_raw(w, h, ChannelLayout::Rgb, SampleType::U8, rgb.to_vec()).unwrap(); + encode(&img, Format::Jpeg, &EncodeOptions { jpeg_chroma_subsampling: true, ..Default::default() }).unwrap() +} + +#[test] +fn jpeg_compressed_ycbcr_with_subsampled_chroma_reads_as_rgb() { + let (w, h) = (32, 24); + let stream = jpeg(&card(w, h), w, h); + // The stream really is subsampled: the luma component samples 2×2 in the frame header. + let sof = stream.windows(2).position(|m| m == [0xFF, 0xC0]).expect("baseline frame header"); + assert_eq!(stream[sof + 11], 0x22, "luma sampling factors"); + let dir = compressed(strips(w, h, 8, 3, 6, h, &stream), 7).tag(SUBSAMPLING, SHORT, &[2, 2]); + let file = build(true, false, &[dir], &[0]).bytes; + + let img = decode(&file).unwrap(); + assert_eq!((img.width(), img.height(), img.layout(), img.sample_type()), (w, h, ChannelLayout::Rgb, SampleType::U8)); + // The oracle: the same JPEG stream decoded to RGB by an independent decoder. + let want = image::load_from_memory_with_format(&stream, image::ImageFormat::Jpeg).unwrap().to_rgb8().into_raw(); + let worst = rgb(&img).iter().zip(&want).map(|(g, o)| g.abs_diff(*o)).max().unwrap(); + assert!(worst <= 2, "largest difference from the oracle: {worst}"); +} + +#[test] +fn uncompressed_ycbcr_uses_the_default_conversion() { + // Black, white, mid gray and JFIF's pure red, without subsampling. + let px = [0, 128, 128, 255, 128, 128, 128, 128, 128, 76, 85, 255]; + let dir = strips(4, 1, 8, 3, 6, 1, &px).tag(SUBSAMPLING, SHORT, &[1, 1]); + let img = decode(&build(true, false, &[dir], &[0]).bytes).unwrap(); + assert_eq!(img.layout(), ChannelLayout::Rgb); + assert_eq!(rgb(&img), [0, 0, 0, 255, 255, 255, 128, 128, 128, 254, 0, 0]); +} + +#[test] +fn reference_black_white_sets_the_code_range() { + // Studio range: Y 16–235, chroma 128 ± 112. + let rbw = [rational(16, 1), rational(235, 1), rational(128, 1), rational(240, 1), rational(128, 1), rational(240, 1)]; + let dir = strips(2, 1, 8, 3, 6, 1, &[16, 128, 128, 235, 128, 128]).tag(SUBSAMPLING, SHORT, &[1, 1]).tag(REFERENCE_BLACK_WHITE, RATIONAL, &rbw); + let img = decode(&build(true, false, &[dir], &[0]).bytes).unwrap(); + assert_eq!(rgb(&img), [0, 0, 0, 255, 255, 255]); +} + +#[test] +fn weights_that_do_not_sum_to_one_keep_grays_neutral() { + // 0.25 each (sum 0.75): TIFF 6.0's full inverse would scale green's Y by (1 − Lr − Lb) / Lg + // = 2 and give mid gray G 255 (clamped). Like libtiff, green keeps Y, so gray stays gray, and + // the weights still set the chroma terms: R = Y + 1.5·Cr, B = Y + 1.5·Cb. + let weights = [rational(1, 4), rational(1, 4), rational(1, 4)]; + let dir = strips(2, 1, 8, 3, 6, 1, &[128, 128, 128, 128, 64, 192]).tag(SUBSAMPLING, SHORT, &[1, 1]).tag(YCBCR_COEFFICIENTS, RATIONAL, &weights); + let img = decode(&build(true, false, &[dir], &[0]).bytes).unwrap(); + // Green for the second pixel: 128 − (Lb·1.5/Lg)·Cb − (Lr·1.5/Lg)·Cr = 128 + 96 − 96. + assert_eq!(rgb(&img), [128, 128, 128, 224, 128, 32]); +} + +#[test] +fn ycbcr_coefficients_set_the_luma_weights() { + // Weights 0.25, 0.5, 0.25 with Cb −64 and Cr +64: R = Y + 1.5·Cr, B = Y + 1.5·Cb, and G keeps Y + // (BT.601 would give R 218 and B 15). + let weights = [rational(1, 4), rational(1, 2), rational(1, 4)]; + let dir = strips(1, 1, 8, 3, 6, 1, &[128, 64, 192]).tag(SUBSAMPLING, SHORT, &[1, 1]).tag(YCBCR_COEFFICIENTS, RATIONAL, &weights); + let img = decode(&build(true, false, &[dir], &[0]).bytes).unwrap(); + assert_eq!(rgb(&img), [224, 128, 32]); +} + +#[test] +fn ycbcr_tags_with_the_wrong_count_use_the_defaults() { + // Four weights and seven black/white codes: both tags are malformed, so BT.601 and full range + // apply (the weights alone would give 224, 128, 32). + let weights = [rational(1, 4), rational(1, 2), rational(1, 4), rational(1, 4)]; + let rbw = [rational(16, 1), rational(235, 1), rational(128, 1), rational(240, 1), rational(128, 1), rational(240, 1), rational(0, 1)]; + let dir = strips(1, 1, 8, 3, 6, 1, &[128, 64, 192]).tag(SUBSAMPLING, SHORT, &[1, 1]).tag(YCBCR_COEFFICIENTS, RATIONAL, &weights).tag( + REFERENCE_BLACK_WHITE, + RATIONAL, + &rbw, + ); + let img = decode(&build(true, false, &[dir], &[0]).bytes).unwrap(); + assert_eq!(rgb(&img), [218, 104, 15]); +} + +#[test] +fn sixteen_bit_ycbcr_reads_as_rgb() { + let px: Vec = [65535u16, 32768, 32768, 0, 32768, 32768].iter().flat_map(|v| v.to_le_bytes()).collect(); + let dir = strips(2, 1, 16, 3, 6, 1, &px).tag(SUBSAMPLING, SHORT, &[1, 1]); + let img = decode(&build(true, false, &[dir], &[0]).bytes).unwrap(); + assert_eq!((img.layout(), img.sample_type()), (ChannelLayout::Rgb, SampleType::U16)); + let n = img.to_normalized(); + assert!(n[..3].iter().all(|&v| v == 1.0) && n[3..].iter().all(|&v| v == 0.0), "{n:?}"); +} + +#[test] +#[cfg(target_pointer_width = "64")] +fn sixteen_bit_ycbcr_bands_match_whole_decode_with_a_missing_last_strip() { + // On 32-bit targets max_alloc also bounds the whole image, so a small allocation cap + // cannot force these tiny strips into separate output groups there. + let (w, h, rows) = (4u32, 5u32, 2u32); + let pixels = [[0u16, 32768, 32768], [65535, 32768, 32768], [19595, 21710, 65535], [32768, 40960, 24576]]; + let samples: Vec = (0..w * h).flat_map(|i| pixels[i as usize % pixels.len()]).collect(); + let weights = [rational(1, 4), rational(1, 2), rational(1, 4)]; + let icc = sample_icc(512); + for little in [true, false] { + for missing in [false, true] { + let dir = strips(w, h, 16, 3, 6, rows, &u16s(&samples, little)).tag(SUBSAMPLING, SHORT, &[1, 1]).tag(YCBCR_COEFFICIENTS, RATIONAL, &weights).bytes( + ICC_PROFILE, + UNDEFINED, + &icc, + ); + let mut file = build(little, false, &[dir], &[0]); + if missing { + let past = file.bytes.len() as u64 + 4096; + patch_value(&mut file, little, false, 0, 273, 2, past); + } + let want = decode_with(&file.bytes, &DecodeOptions { keep_orientation: true, ..Default::default() }).unwrap(); + let limits = Limits { max_alloc: u64::from(w * rows * 3 * 2), ..Limits::banded() }; + let mut bands = TiffBands::open(&file.bytes, None, &limits).unwrap(); + assert_eq!((bands.dimensions(), bands.layout(), bands.sample_type()), ((w, h), ChannelLayout::Rgb, SampleType::U16)); + let mut got = Image::new(w, h, bands.layout(), bands.sample_type()).unwrap(); + let mut positions = Vec::new(); + bands + .decode(|x, y, group: &Image| -> Result<(), CodecError> { + positions.push((x, y, group.height())); + assert_eq!((x, group.width(), group.layout(), group.sample_type()), (0, w, ChannelLayout::Rgb, SampleType::U16)); + let at = y as usize * w as usize * 3 * 2; + got.data_mut()[at..at + group.data().len()].copy_from_slice(group.data()); + Ok(()) + }) + .unwrap(); + assert_eq!(positions, [(0, 0, 2), (0, 2, 2), (0, 4, 1)], "one output group per strip, including the short last strip"); + assert_eq!(got.data(), want.data(), "little endian: {little}, missing last strip: {missing}"); + assert_eq!(bands.icc, want.icc); + assert_eq!(bands.meta, want.meta); + assert_eq!(bands.warnings, want.warnings); + assert_eq!(bands.warnings.iter().any(|w| matches!(w, DecodeWarning::Truncated { .. })), missing); + } + } +} + +#[test] +fn unsupported_ycbcr_files_fail_cleanly() { + // Four samples (an extra sample next to YCbCr). + let four = strips(1, 1, 8, 4, 6, 1, &[128, 128, 128, 255]).tag(SUBSAMPLING, SHORT, &[1, 1]); + assert!(decode(&build(true, false, &[four], &[0]).bytes).is_err()); + // Subsampled chroma without JPEG compression (the default subsampling is 2×2). + let sub = strips(2, 2, 8, 3, 6, 2, &[128; 12]); + assert!(decode(&build(true, false, &[sub], &[0]).bytes).is_err()); + // A JPEG strip that is not JPEG data. + let junk = compressed(strips(16, 16, 8, 3, 6, 16, &[0x5A; 64]), 7).tag(SUBSAMPLING, SHORT, &[2, 2]); + assert!(decode(&build(true, false, &[junk], &[0]).bytes).is_err()); + // Coefficients and ReferenceBlackWhite that would divide by zero fall back to the defaults. + let bad = strips(1, 1, 8, 3, 6, 1, &[255, 128, 128]) + .tag(SUBSAMPLING, SHORT, &[1, 1]) + .tag(YCBCR_COEFFICIENTS, RATIONAL, &[rational(0, 1), rational(0, 0), rational(5, 1)]) + .tag(REFERENCE_BLACK_WHITE, RATIONAL, &[rational(7, 1); 6]); + let img = decode(&build(true, false, &[bad], &[0]).bytes).unwrap(); + assert_eq!(&img.to_rgba8()[..3], &[255, 255, 255]); +} + +#[test] +fn tiled_jpeg_ycbcr_keeps_its_icc_profile() { + // Two 16×16 tiles, each its own 4:2:0 JPEG stream, and an embedded profile, like a scanner's + // tiled output. + let (w, h, t) = (32u32, 16u32, 16u32); + let px = card(w, h); + let tile = |tx: u32| -> Vec { + (0..t) + .flat_map(|y| { + let at = ((y * w + tx * t) * 3) as usize; + px[at..at + (t * 3) as usize].to_vec() + }) + .collect() + }; + let streams = [jpeg(&tile(0), t, t), jpeg(&tile(1), t, t)]; + let icc = sample_icc(512); + let dir = Dir { entries: base(w, h, 8, 3, 6), chunks: streams.to_vec(), tiled: true }; + let dir = compressed(dir, 7).tag(322, SHORT, &[u64::from(t)]).tag(323, SHORT, &[u64::from(t)]).tag(SUBSAMPLING, SHORT, &[2, 2]).bytes( + ICC_PROFILE, + UNDEFINED, + &icc, + ); + let img = decode(&build(true, false, &[dir], &[0]).bytes).unwrap(); + assert_eq!((img.width(), img.height(), img.layout()), (w, h, ChannelLayout::Rgb)); + assert_eq!(img.icc.as_deref(), Some(icc.as_slice()), "the embedded profile is kept"); + // The oracle: each tile's stream decoded by an independent decoder, placed side by side. + let tiles: Vec> = streams.iter().map(|s| image::load_from_memory_with_format(s, image::ImageFormat::Jpeg).unwrap().to_rgb8().into_raw()).collect(); + let row = (t * 3) as usize; + let want: Vec = (0..t as usize).flat_map(|y| [&tiles[0][y * row..(y + 1) * row], &tiles[1][y * row..(y + 1) * row]].concat()).collect(); + let worst = rgb(&img).iter().zip(&want).map(|(g, o)| g.abs_diff(*o)).max().unwrap(); + assert!(worst <= 2, "largest difference from the oracle: {worst}"); +} + +#[test] +fn big_endian_sixteen_bit_ycbcr_reads_as_rgb() { + // White, black, and red as full-range 16-bit YCbCr (Y 0.299·65535, Cb 32768 − 0.1687·65535, Cr max). + let px = [65535u16, 32768, 32768, 0, 32768, 32768, 19595, 21710, 65535]; + let dir = strips(3, 1, 16, 3, 6, 1, &u16s(&px, false)).tag(SUBSAMPLING, SHORT, &[1, 1]); + let file = build(false, false, &[dir], &[0]).bytes; + assert_eq!(&file[..2], b"MM"); + let img = decode(&file).unwrap(); + assert_eq!((img.layout(), img.sample_type()), (ChannelLayout::Rgb, SampleType::U16)); + let n = img.to_normalized(); + let want = [1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0]; + assert!(n.iter().zip(want).all(|(g, w)| (g - w).abs() < 2.0 / 255.0), "{n:?}"); +} + +#[test] +fn planar_ycbcr_reads_as_rgb_in_both_byte_orders() { + // The pixels of the default-conversion test, one plane per component. + let px = [0u8, 128, 128, 255, 128, 128, 128, 128, 128, 76, 85, 255]; + for little in [true, false] { + let dir = Dir { entries: base(4, 1, 8, 3, 6), chunks: planar_strips(&px, 4, 1, 3, 1, 1), tiled: false }.tag(278, LONG, &[1]).tag(284, SHORT, &[2]).tag( + SUBSAMPLING, + SHORT, + &[1, 1], + ); + let img = decode(&build(little, false, &[dir], &[0]).bytes).unwrap(); + assert_eq!(rgb(&img), [0, 0, 0, 255, 255, 255, 128, 128, 128, 254, 0, 0], "little endian: {little}"); + } +} diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 5ef09a2e7e..68cd49bbe9 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-26 · **Change:** minor (TIFFs up to 300,000 px a side open, decoded a group of rows at a time into the layer, #8169; embedded AI/PDF/EPS save-back retains native format with a source-bound SVG display preview; PSD Filter Gallery: Halftone Pattern dot/circle/line settings survive PSD/PSB saves (#8940); Save for Web writes WebP, lossy or lossless, #6391; Photoshop's `MTrn` merged-transparency marker, from a layered TIFF saved with Save Transparency, marks the merged transparency, #8877; layered TIFF writes the TySh text version and descriptor in PSD big-endian order while the header, matrix, warp and bounds keep container order, #9098; layered TIFF writes Photoshop's 14-byte global layer mask, so Photoshop opens the layers instead of only the composite, #9095; legacy PSD point text placed from the layout ink box after font resolution, #6094; `.atn`: a recorded Save As imports with its format and file (`file.saveAs`, `file.saveACopy` for As a Copy) instead of as a plain Save that wrote over the played file, #7102; folder destinations stop playback until #5430; `.asl` layer style libraries read and written, #1051; JPEG Options on File › Save, Save As and Save a Copy, FILE-215-19 partial (no Matte, progressive Scans or size preview); 32-bit Hue/Saturation unclipped and Colorize as Photoshop does them, Photoshop-oracle floor 190/258, #5781; JPEG Options on Save As and Save a Copy, FILE-215-19; TIFF compression for scripted saves, `tiffCompression`, #5301; the Nikon D4 white level follows the sample depth (12 and 14 bit), #5978; Apply Layer Comp action mapping, #7101; Cineon write: 10-bit gray/RGB; animated WebP frames load into video layers, #1198; Photoshop mixed-order TIFF type geometry, #4758; DDS textures save: uncompressed or BC1–BC3, optional mip chain, DDS Options in Save As (#1327); PSD smart filters: 53 filters (39 more: Pinch, Polar Coordinates, Ripple, Spherize, Twirl, Wave, ZigZag, Invert, Blur, Surface Blur, Smart Sharpen, Diffuse, Wind, Offset, Mezzotint, Clouds, Difference Clouds, Fibers, Lens Flare and others) plus Hue/Saturation and Perspective Warp mapped both ways, #6296, #6298, #6300, #6302, #6304, #6314, #6316, #6267, #6268, FILE-215-2; large-file fixtures: a 20 MP layered PSD, a 150-layer layout, a 2 GB PSD and a 4.4 GB PSB generated at test time and round-tripped, FILE-216-2 done, #5107; earlier: smart objects keep Photoshop's Cylinder warp through re-renders and saves, #4112; PNG `sTER` stereo pairing kept on save, #6063; PDF write one flattened page via Save As / Save a Copy / CLI, #3401; CMYK Invert, Posterize and Threshold on the inks; smart filter masks on PSD open and save; Photoshop-oracle floor 188/258; lossy DNG, compression 34892, decodes with its levels and LinearizationTable applied to the stored 8-bit samples, FILE-215-11; CMYK Hue/Saturation and Color Balance keep the black ink; CMYK Posterize on the stored values, psd-tools oracle floor 242/309; CMYK adjustment layers composite on the document's inks; Photoshop-oracle floor 165/258; CMYK Brightness/Contrast on the inks, CMYK Threshold's luma; modern Brightness/Contrast's brightness curve; Photoshop-oracle floor 151/258; Lab documents composite on their stored L, a, b; Photoshop-oracle floor 152/258; Levels on whole levels at every step with Photoshop's shadow toe; Photoshop-oracle floor 146/258, psd-tools 238/309; 32-bit adjustments unclipped; Photoshop-oracle floor 149/258; mask feathers blur with their exact variance; Threshold splits on Photoshop's luminosity; a pass-through clipping base with an adjustment renders isolated; psd-tools oracle floor 240/309; 16-bit Posterize as Photoshop bins it; psd-tools oracle floor 241/309; Gradient Map luminosity; Photoshop-oracle floor 147/258; PSD: `Mt16`/`Mt32` merged transparency read; palette PNGs open in Indexed Color mode, #4804; 16-bit Lab Invert; Photoshop-oracle floor 145/258; `.atn` import maps 112 events; DDS textures open, #1327; DPX read and write, #1049; #2465 fixed: CS2 placed layers with only a `PlLd` block open their contents; Photoshop Actions `.atn` read and write; `.shc` contour files read and write; earlier the same day: indexed BMP palette mode; Save keeps a JPEG's quality and a WebP's encoding; Amiga IFF write: 24-plane ILBM, Save As `.iff`; PSD stores spot channels as Photoshop does, ink 0 = solid, and numbers them in 1053, #7415 split; TIFF writes and reads alpha and spot channels as extra samples with Photoshop's channel resources, PSD output unchanged, FILE-215-7, FILE-215-17's TIFF part, #7415; Photoshop's `MTrn` merged-transparency marker, from a layered TIFF saved with Save Transparency, marks the merged transparency) · **Target:** Adobe Photoshop 2026 +> **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-30 · **Change:** patch (TIFF: YCbCr images open as RGB, including JPEG-compressed scans with subsampled chroma, #10316; TIFFs up to 300,000 px a side open, decoded a group of rows at a time into the layer, #8169; embedded AI/PDF/EPS save-back retains native format with a source-bound SVG display preview; PSD Filter Gallery: Halftone Pattern dot/circle/line settings survive PSD/PSB saves (#8940); Save for Web writes WebP, lossy or lossless, #6391; Photoshop's `MTrn` merged-transparency marker, from a layered TIFF saved with Save Transparency, marks the merged transparency, #8877; layered TIFF writes the TySh text version and descriptor in PSD big-endian order while the header, matrix, warp and bounds keep container order, #9098; layered TIFF writes Photoshop's 14-byte global layer mask, so Photoshop opens the layers instead of only the composite, #9095; legacy PSD point text placed from the layout ink box after font resolution, #6094; `.atn`: a recorded Save As imports with its format and file (`file.saveAs`, `file.saveACopy` for As a Copy) instead of as a plain Save that wrote over the played file, #7102; folder destinations stop playback until #5430; `.asl` layer style libraries read and written, #1051; JPEG Options on File › Save, Save As and Save a Copy, FILE-215-19 partial (no Matte, progressive Scans or size preview); 32-bit Hue/Saturation unclipped and Colorize as Photoshop does them, Photoshop-oracle floor 190/258, #5781; JPEG Options on Save As and Save a Copy, FILE-215-19; TIFF compression for scripted saves, `tiffCompression`, #5301; the Nikon D4 white level follows the sample depth (12 and 14 bit), #5978; Apply Layer Comp action mapping, #7101; Cineon write: 10-bit gray/RGB; animated WebP frames load into video layers, #1198; Photoshop mixed-order TIFF type geometry, #4758; DDS textures save: uncompressed or BC1–BC3, optional mip chain, DDS Options in Save As (#1327); PSD smart filters: 53 filters (39 more: Pinch, Polar Coordinates, Ripple, Spherize, Twirl, Wave, ZigZag, Invert, Blur, Surface Blur, Smart Sharpen, Diffuse, Wind, Offset, Mezzotint, Clouds, Difference Clouds, Fibers, Lens Flare and others) plus Hue/Saturation and Perspective Warp mapped both ways, #6296, #6298, #6300, #6302, #6304, #6314, #6316, #6267, #6268, FILE-215-2; large-file fixtures: a 20 MP layered PSD, a 150-layer layout, a 2 GB PSD and a 4.4 GB PSB generated at test time and round-tripped, FILE-216-2 done, #5107; earlier: smart objects keep Photoshop's Cylinder warp through re-renders and saves, #4112; PNG `sTER` stereo pairing kept on save, #6063; PDF write one flattened page via Save As / Save a Copy / CLI, #3401; CMYK Invert, Posterize and Threshold on the inks; smart filter masks on PSD open and save; Photoshop-oracle floor 188/258; lossy DNG, compression 34892, decodes with its levels and LinearizationTable applied to the stored 8-bit samples, FILE-215-11; CMYK Hue/Saturation and Color Balance keep the black ink; CMYK Posterize on the stored values, psd-tools oracle floor 242/309; CMYK adjustment layers composite on the document's inks; Photoshop-oracle floor 165/258; CMYK Brightness/Contrast on the inks, CMYK Threshold's luma; modern Brightness/Contrast's brightness curve; Photoshop-oracle floor 151/258; Lab documents composite on their stored L, a, b; Photoshop-oracle floor 152/258; Levels on whole levels at every step with Photoshop's shadow toe; Photoshop-oracle floor 146/258, psd-tools 238/309; 32-bit adjustments unclipped; Photoshop-oracle floor 149/258; mask feathers blur with their exact variance; Threshold splits on Photoshop's luminosity; a pass-through clipping base with an adjustment renders isolated; psd-tools oracle floor 240/309; 16-bit Posterize as Photoshop bins it; psd-tools oracle floor 241/309; Gradient Map luminosity; Photoshop-oracle floor 147/258; PSD: `Mt16`/`Mt32` merged transparency read; palette PNGs open in Indexed Color mode, #4804; 16-bit Lab Invert; Photoshop-oracle floor 145/258; `.atn` import maps 112 events; DDS textures open, #1327; DPX read and write, #1049; #2465 fixed: CS2 placed layers with only a `PlLd` block open their contents; Photoshop Actions `.atn` read and write; `.shc` contour files read and write; earlier the same day: indexed BMP palette mode; Save keeps a JPEG's quality and a WebP's encoding; Amiga IFF write: 24-plane ILBM, Save As `.iff`; PSD stores spot channels as Photoshop does, ink 0 = solid, and numbers them in 1053, #7415 split; TIFF writes and reads alpha and spot channels as extra samples with Photoshop's channel resources, PSD output unchanged, FILE-215-7, FILE-215-17's TIFF part, #7415; Photoshop's `MTrn` merged-transparency marker, from a layered TIFF saved with Save Transparency, marks the merged transparency) · **Target:** Adobe Photoshop 2026 **PSD Filter Gallery (#8940, 2026-10-23):** Halftone Pattern maps its size, contrast and dot/circle/line pattern as editable settings on import and export, with the reference application's descriptor keys (`HlSz`, `Cntr`, `ScrT`: `ScrD`/`ScrC`/`ScrL`). PSD/PSB round trips pass at 8/16/32 bits. This does not claim pixel equivalence for the existing renderer. Unknown or malformed settings remain verbatim on import. @@ -26,7 +26,7 @@ XL are missing (gap G10). Remaining: 300–500 h including PSD fidelity. | PSD | R/W | yes | yes | 1/8/16/32-bit; Bitmap, Gray, Indexed, RGB, CMYK, Multichannel, Lab, Duotone; layers, groups, masks, vector masks, adjustment and fill layers, effects, text, shapes, smart objects (SoLd/lnk2 with their warp: the bend presets, a custom mesh and, since #4112, Cylinder; CS2 PlLd/lnkD read with their warp, kept as is or re-saved as SoLd once edited), smart filters (53 mapped, among them Invert, the Distort filters but Shear, Clouds, Difference Clouds, Fibers and Lens Flare, plus Hue/Saturation, Perspective Warp, Filter Gallery with Chrome and a Camera Raw subset, rest verbatim), slices, comps, Advanced Blending, each layer's XMP packet (`layer.xmpMetadata`); alpha and spot channels as Photoshop stores them (spot ink 0 = solid, channel identifiers in 1053), and PSDs with spot channels from PhotoCraft 0.1.0–0.16.0, which stored the ink the other way round, keep their ink. Oracle floors: Photoshop-authored 190/258, io 148/170, psd-tools 249/309; round trip ≥ 169/170, 308/309, 258/258. Known failures: #1281, #2469 (#2374, text layers, closed 2026-10-10; #2465, CS2 `PlLd`-only smart objects, fixed 2026-10-12) | | PSB | R/W | yes | yes | Auto-switches above 2 GB (REL-212-2); generated 20 MP, 150-layer, 2 GB PSD and 4.4 GB PSB fixtures opened, saved and re-opened by io tests/it/large_files.rs (FILE-216-2, 2026-10-13; the 4.4 GB PSB opens on Windows); > 4 GB from the app's File › Open on Windows: #375 | | Cloud PSD (.psdc) | R/W | no | no | Out of scope (Adobe cloud) | -| TIFF | R/W | yes | yes | 8/16/32f, Gray/RGB/CMYK, BigTIFF, every IFD, banded parallel decode; flat pages up to 300,000 px a side open a group of rows at a time into the layer, with no full-size copy, and are refused with an error when the machine lacks the memory (Linux: free RAM and swap, cgroup limits; #8169); layered TIFF both byte orders, including Photoshop mixed-order type blocks on read (#4758/#4771) and TySh text descriptors written in PSD order (#9098) and Photoshop's 14-byte `LMsk` block (#9095); LZW/ZIP for scripted saves by `tiffCompression`; undecodable type keeps saved pixels (FILE-215-7); alpha and spot channels as extra samples (at most 16) with Photoshop's names, ink colours and solidity, spot ink stored 0 = solid (#7415); no Save Transparency choice yet (channels are written as Photoshop does with it off) | +| TIFF | R/W | yes | yes | 8/16/32f, Gray/RGB/CMYK, YCbCr read as RGB (JPEG-compressed with subsampled chroma, or uncompressed without subsampling; #10316), BigTIFF, every IFD, banded parallel decode; flat pages up to 300,000 px a side open a group of rows at a time into the layer, with no full-size copy, and are refused with an error when the machine lacks the memory (Linux: free RAM and swap, cgroup limits; #8169); layered TIFF both byte orders, including Photoshop mixed-order type blocks on read (#4758/#4771) and TySh text descriptors written in PSD order (#9098) and Photoshop's 14-byte `LMsk` block (#9095); LZW/ZIP for scripted saves by `tiffCompression`; undecodable type keeps saved pixels (FILE-215-7); alpha and spot channels as extra samples (at most 16) with Photoshop's names, ink colours and solidity, spot ink stored 0 = solid (#7415); no Save Transparency choice yet (channels are written as Photoshop does with it off) | | JPEG | R/W | yes | yes | 8-bit Gray/RGB/CMYK, ICC/EXIF/XMP; Save keeps the opened file's quality (estimated from its quantisation tables, #2399); File › Save, Save As and Save a Copy ask JPEG Options: Quality 0–12, starting at the opened file's quality or else the Export preference, and Baseline ("Standard") / Baseline Optimized / Progressive; scripts pass `quality`/`jpegFormat`, the CLI `--jpeg-format` (FILE-215-19). Baseline Optimized fits the Huffman tables only at 4:4:4 (Gray, CMYK) or progressive: RGB is written 4:2:0, which zune-jpeg 0.5.15 decodes wrongly with fitted tables, as in Save for Web. No Matte, progressive Scans (3–5) or size preview | | PNG | R/W | yes | yes | 8/16-bit, ICC/text; APNG first frame only; alpha channels on open (#2225); palette PNGs open in Indexed Color mode with their transparent entry (#4804); Save keeps a stereo PNG's `sTER` pairing (#6063) | | GIF | R/W | yes | yes | File › Open keeps the first frame; New Video Layer from File and Video Frames to Layers take every frame, with the timeline rate from the delays (#572). Per-frame delays and keeping the frames in `.pcraft` are not done | @@ -148,6 +148,7 @@ Embedded SVG and PDF-compatible AI/PDF/EPS payloads can use an explicit VectorCr | Date | Change | Summary | |---|---|---| +| 2026-10-30 | patch | TIFF: YCbCr (PhotometricInterpretation 6) opens as RGB instead of failing with "colour type YCbCr(8)": JPEG-compressed with subsampled chroma (as scanners write it) through the JPEG conversion, uncompressed with ReferenceBlackWhite and YCbCrCoefficients; 8 and 16-bit. Uncompressed subsampled YCbCr and YCbCr with extra samples still fail with a clear error (#10316) | | 2026-10-26 | minor | Embedded AI/PDF/EPS save-back retains native format and a separate source-bound display preview; original sources remain undoable. Native Adobe interoperability and standalone EPS/DCS remain unverified; no corpus floor or percentage changed. | | 2026-10-22 | minor | Embedded PSD vectors gain explicit bounded SVG editor/save-back; standalone EPS/DCS and native-format fidelity remain open. | | 2026-10-22 | minor | TIFF: flat pages up to 300,000 px a side open (was 2^30 px, about 32,768²): `codecs::TiffBands` decodes a group of rows at a time into the layer's tiles. The 117,504 × 22,464 px banner from #8456 opens in 3.3 s at 11.45 GB peak for a 10.56 GB layer; 30,000² 6.8 s / 6.43 GB → 1.1 s / 3.86 GB; 36 MP 220 → 52 ms. Layered TIFFs still decode whole (#8169) |