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/2] 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/2] 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")]))); + } +}