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/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 5111fe1635..10d3114e85 100644 --- a/crates/engine/src/jobs.rs +++ b/crates/engine/src/jobs.rs @@ -294,6 +294,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 { @@ -309,7 +319,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, } @@ -691,7 +701,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) } @@ -730,8 +750,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 } @@ -778,8 +798,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(); + } } } @@ -798,6 +825,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. @@ -881,6 +913,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. @@ -900,7 +933,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) @@ -917,8 +950,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() @@ -934,7 +967,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/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")]))); + } +} 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); }