diff --git a/crates/algo/src/transform.rs b/crates/algo/src/transform.rs index 5bf4fd798..bb31945d5 100644 --- a/crates/algo/src/transform.rs +++ b/crates/algo/src/transform.rs @@ -126,19 +126,19 @@ fn catmull_rom(t: f64) -> [f64; 4] { /// Warp `src` (its content inside `src_rect`) by `h` (source → destination document space). /// The result has the same format as `src`, with alpha added if `src` had none; pixels outside /// the warped quad are transparent. -pub fn warp_surface(src: &Surface, src_rect: Rect, h: &Homography, interp: Interp) -> Surface { +pub fn warp_surface(src: &Surface, src_rect: Rect, h: &Homography, interp: Interp) -> Result { let mut fmt = src.format(); let converted; let mut src = if fmt.alpha { src } else { fmt = PixelFormat::new(fmt.mode, fmt.sample, true); - converted = src.convert(fmt); + converted = src.try_convert(fmt)?; &converted }; let mut out = Surface::new(fmt); if src_rect.is_empty() { - return out; + return Ok(out); } let mut h = *h; let mut src_rect = src_rect; @@ -153,12 +153,12 @@ pub fn warp_surface(src: &Surface, src_rect: Rect, h: &Homography, interp: Inter h = h.mul(&Homography([1.0 / f, 0.0, 0.0, 0.0, 1.0 / f, 0.0, 0.0, 0.0, 1.0])); src_rect = crate::resample::scaled_rect(src_rect, f, f); } - let Some(inv) = h.inverse() else { return out }; + let Some(inv) = h.inverse() else { return Ok(out) }; // Destination bounds: the warped corners (plus a pixel for filter support). let corners = [(src_rect.x0, src_rect.y0), (src_rect.x1, src_rect.y0), (src_rect.x1, src_rect.y1), (src_rect.x0, src_rect.y1)] .map(|(x, y)| h.apply(x as f64, y as f64)); if corners.iter().any(|c| !c.0.is_finite() || !c.1.is_finite()) { - return out; + return Ok(out); } let lim = 1 << 20; let bx0 = corners.iter().map(|c| c.0).fold(f64::MAX, f64::min).floor().max(-(lim as f64)) as i32 - 1; @@ -168,7 +168,11 @@ pub fn warp_surface(src: &Surface, src_rect: Rect, h: &Homography, interp: Inter let dst = Rect::new(bx0, by0, bx1, by1); let n = fmt.channels(); let a = n - 1; - let tiles: Vec = dst.tiles().map(|tc| tc.rect().intersect(&dst)).filter(|r| !r.is_empty()).collect(); + let tile_count = dst.tiles().count(); + let tile_bytes = tile_count.checked_mul(std::mem::size_of::()).ok_or(photocraft_raster::AllocationError::SizeOverflow)?; + let mut tiles = Vec::new(); + tiles.try_reserve_exact(tile_count).map_err(|_| photocraft_raster::AllocationError::NotEnoughMemory { bytes: tile_bytes })?; + tiles.extend(dst.tiles().map(|tc| tc.rect().intersect(&dst)).filter(|r| !r.is_empty())); let src_ref: &Surface = src; // A sampled pixel's inverse w is 1 / (forward w at its source point). While the forward w keeps one sign over the source // (plus the margin the sampler reads), every sampled pixel lies at least `w_lo` from the inverse horizon, so clipping a tile @@ -177,7 +181,7 @@ pub fn warp_surface(src: &Surface, src_rect: Rect, h: &Homography, interp: Inter let fwd_w = [(sx0, sy0), (sx1, sy0), (sx1, sy1), (sx0, sy1)].map(|(x, y)| h.0[6] * x + h.0[7] * y + h.0[8]); let side = [1.0, -1.0].into_iter().find(|&s| fwd_w.iter().all(|&c| s * c > 0.0)); let w_lo = 1.0 / fwd_w.iter().fold(0.0f64, |m, c| m.max(c.abs())); - let work = |t: &Rect| -> Option<(Rect, Vec)> { + let work = |t: &Rect| -> Result)>, photocraft_raster::AllocationError> { // Source footprint of this tile (inverse-mapped corners of its sampled part), padded for the filter. let tc = [(t.x0, t.y0), (t.x1, t.y0), (t.x1, t.y1), (t.x0, t.y1)].map(|(x, y)| (x as f64, y as f64)); let wc = tc.map(|(x, y)| inv.0[6] * x + inv.0[7] * y + inv.0[8]); @@ -201,7 +205,7 @@ pub fn warp_surface(src: &Surface, src_rect: Rect, h: &Homography, interp: Inter None => tc.to_vec(), }; if poly.is_empty() { - return None; + return Ok(None); } let fp: Vec<(f64, f64)> = poly.iter().map(|&(x, y)| inv.apply(x, y)).collect(); let fx0 = fp.iter().map(|c| c.0).fold(f64::MAX, f64::min).floor() as i32 - 3; @@ -212,10 +216,10 @@ pub fn warp_surface(src: &Surface, src_rect: Rect, h: &Homography, interp: Inter let straddles = side.is_none() && wc.iter().any(|w| *w <= 0.0) && wc.iter().any(|w| *w >= 0.0); let foot = if straddles { src_rect } else { Rect::new(fx0, fy0, fx1, fy1).intersect(&src_rect) }; if foot.is_empty() || !src_ref.has_tiles_in(foot) { - return None; + return Ok(None); } // Premultiplied source window. - let mut px = src_ref.read_region(foot); + let mut px = src_ref.try_read_region(foot)?; for p in px.chunks_exact_mut(n) { let al = p[a]; for v in &mut p[..a] { @@ -231,7 +235,11 @@ pub fn warp_surface(src: &Surface, src_rect: Rect, h: &Homography, interp: Inter } }; let w = t.width() as usize; - let mut outp = vec![0.0f32; w * t.height() as usize * n]; + let len = w.checked_mul(t.height() as usize).and_then(|v| v.checked_mul(n)).ok_or(photocraft_raster::AllocationError::SizeOverflow)?; + let bytes = len.checked_mul(std::mem::size_of::()).ok_or(photocraft_raster::AllocationError::SizeOverflow)?; + let mut outp = Vec::new(); + outp.try_reserve_exact(len).map_err(|_| photocraft_raster::AllocationError::NotEnoughMemory { bytes })?; + outp.resize(len, 0.0f32); let mut any = false; let mut acc = [0.0f64; 8]; for y in t.y0..t.y1 { @@ -285,20 +293,34 @@ pub fn warp_surface(src: &Surface, src_rect: Rect, h: &Homography, interp: Inter outp[o + a] = al as f32; } } - any.then_some((*t, outp)) + Ok(any.then_some((*t, outp))) }; + let result_bytes = tiles.len().checked_mul(std::mem::size_of::<(Rect, Vec)>()).ok_or(photocraft_raster::AllocationError::SizeOverflow)?; + let mut done = Vec::new(); + done.try_reserve_exact(tiles.len()).map_err(|_| photocraft_raster::AllocationError::NotEnoughMemory { bytes: result_bytes })?; + let done = std::sync::Mutex::new(done); #[cfg(not(target_arch = "wasm32"))] - let done: Vec<(Rect, Vec)> = { + { use rayon::prelude::*; - tiles.par_iter().filter_map(work).collect() - }; + tiles.into_par_iter().try_for_each(|tile| -> Result<(), photocraft_raster::AllocationError> { + if let Some(result) = work(&tile)? { + done.lock().unwrap_or_else(std::sync::PoisonError::into_inner).push(result); + } + Ok(()) + })?; + } #[cfg(target_arch = "wasm32")] - let done: Vec<(Rect, Vec)> = tiles.iter().filter_map(work).collect(); + for tile in tiles { + if let Some(result) = work(&tile)? { + done.lock().unwrap_or_else(std::sync::PoisonError::into_inner).push(result); + } + } + let done = done.into_inner().unwrap_or_else(std::sync::PoisonError::into_inner); for (r, v) in done { - out.write_region(r, &v); + out.try_write_region(r, &v)?; } out.prune(); - out + Ok(out) } #[cfg(test)] @@ -333,7 +355,7 @@ mod tests { s.fill_rect(Rect::new(0, 0, 100, 100), &[1.0, 0.0, 0.0, 1.0]); let r = s.content_bounds(); let h = Homography::rect_to_quad([0.0, 0.0, 100.0, 100.0], [[0.0, 0.0], [300.0, 0.0], [155.0, 155.0], [0.0, 300.0]]).unwrap(); - let w = warp_surface(&s, r, &h, Interp::Nearest); + let w = warp_surface(&s, r, &h, Interp::Nearest).unwrap(); for (x, y) in [(20, 20), (252, 14), (252, 42), (14, 252), (42, 252)] { let a = w.read_region(Rect::new(x, y, x + 1, y + 1))[3]; assert!(a > 0.99, "({x},{y}) inside the quad is transparent: {a}"); @@ -350,7 +372,7 @@ mod tests { let r = s.content_bounds(); for interp in [Interp::Nearest, Interp::Bilinear, Interp::Bicubic] { let h = Homography([1.0, 0.0, 7.0, 0.0, 1.0, -3.0, 0.0, 0.0, 1.0]); - let o = warp_surface(&s, r, &h, interp); + let o = warp_surface(&s, r, &h, interp).unwrap(); assert_eq!(o.content_bounds(), Rect::new(11, 1, 27, 9), "{interp:?}"); assert_eq!(o.pixel(15, 5), vec![1.0, 128.0 / 255.0, 0.0, 1.0], "{interp:?}"); } @@ -366,12 +388,13 @@ mod tests { r, &Homography::rect_to_quad([0.0, 0.0, 40.0, 20.0], [[0.0, 0.0], [80.0, 0.0], [80.0, 40.0], [0.0, 40.0]]).unwrap(), Interp::Bicubic, - ); + ) + .unwrap(); let b = up.content_bounds(); assert!(b.width() >= 80 && b.width() <= 82 && b.height() >= 40 && b.height() <= 42, "{b:?}"); // 90° rotation about (20, 10): a 40×20 box becomes 20×40. let q = [[30.0, -10.0], [30.0, 30.0], [10.0, 30.0], [10.0, -10.0]]; - let rot = warp_surface(&s, r, &Homography::rect_to_quad([0.0, 0.0, 40.0, 20.0], q).unwrap(), Interp::Bilinear); + let rot = warp_surface(&s, r, &Homography::rect_to_quad([0.0, 0.0, 40.0, 20.0], q).unwrap(), Interp::Bilinear).unwrap(); let b = rot.content_bounds(); assert!(b.width().abs_diff(20) <= 2 && b.height().abs_diff(40) <= 2, "{b:?}"); let p = rot.pixel(20, 10); @@ -389,7 +412,7 @@ mod tests { } } let h = Homography::rect_to_quad([0.0, 0.0, 64.0, 64.0], [[0.0, 0.0], [8.0, 0.0], [8.0, 8.0], [0.0, 8.0]]).unwrap(); - let o = warp_surface(&s, s.content_bounds(), &h, Interp::Bicubic); + let o = warp_surface(&s, s.content_bounds(), &h, Interp::Bicubic).unwrap(); let p = o.pixel(4, 4); assert!((p[0] - 0.5).abs() < 0.1, "{p:?}"); } @@ -399,7 +422,7 @@ mod tests { let mut s = Surface::new(PixelFormat::new(photocraft_color::ColorMode::Rgb, photocraft_color::SampleType::U8, false)); s.fill_rect(Rect::new(0, 0, 10, 10), &[1.0, 0.0, 0.0]); let h = Homography([1.0, 0.0, 5.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0]); - let o = warp_surface(&s, Rect::new(0, 0, 10, 10), &h, Interp::Nearest); + let o = warp_surface(&s, Rect::new(0, 0, 10, 10), &h, Interp::Nearest).unwrap(); assert!(o.format().alpha); assert_eq!(o.pixel(2, 2)[3], 0.0); assert_eq!(o.pixel(7, 2), vec![1.0, 0.0, 0.0, 1.0]); diff --git a/crates/algo/src/vanishing.rs b/crates/algo/src/vanishing.rs index 46e762b7f..5661d66c9 100644 --- a/crates/algo/src/vanishing.rs +++ b/crates/algo/src/vanishing.rs @@ -228,22 +228,22 @@ fn quad_mask(h: &Homography, area: Rect) -> Vec { /// Pastes `src` (content in `src_rect`) onto plane `i` with its top-left at plane coordinates /// `at` (0..1) and its width `width` (fraction of the plane's width), aspect preserved in the /// plane's metric. Returns the warped image clipped to the plane (transparent elsewhere). -pub fn paste(scene: &Scene, i: usize, src: &Surface, src_rect: Rect, at: [f64; 2], width: f64) -> Option { - let (h, lu, lv) = scene.frame(i)?; +pub fn paste(scene: &Scene, i: usize, src: &Surface, src_rect: Rect, at: [f64; 2], width: f64) -> Result, photocraft_raster::AllocationError> { + let Some((h, lu, lv)) = scene.frame(i) else { return Ok(None) }; let (sw, sh) = (src_rect.width() as f64, src_rect.height() as f64); if sw <= 0.0 || sh <= 0.0 { - return None; + return Ok(None); } // Source px → plane (u, v): u spans `width`, v keeps the metric aspect. let su = width / sw; let sv = width * lu / lv / sw; let to_plane = Homography([su, 0.0, at[0] - src_rect.x0 as f64 * su, 0.0, sv, at[1] - src_rect.y0 as f64 * sv, 0.0, 0.0, 1.0]); let full = h.mul(&to_plane); - let mut out = warp_surface(src, src_rect, &full, Interp::Bicubic); + let mut out = warp_surface(src, src_rect, &full, Interp::Bicubic)?; // Clip to the plane. let b = out.content_bounds(); if b.is_empty() { - return Some(out); + return Ok(Some(out)); } let mask = quad_mask(&h, b); let fmt = out.format(); @@ -254,7 +254,7 @@ pub fn paste(scene: &Scene, i: usize, src: &Surface, src_rect: Rect, at: [f64; 2 } out.write_region(b, &data); out.prune(); - Some(out) + Ok(Some(out)) } /// Perspective clone stamp: one stroke of dabs at image points `points` on the destination @@ -384,7 +384,7 @@ mod tests { let sc = Scene::new(floor_quad(500.0, c), c, None, 800.0); let mut img = Surface::new(PixelFormat::RGBA8); img.fill_rect(Rect::new(0, 0, 100, 50), &[1.0, 0.0, 0.0, 1.0]); - let out = paste(&sc, 0, &img, Rect::new(0, 0, 100, 50), [0.25, 0.25], 0.5).unwrap(); + let out = paste(&sc, 0, &img, Rect::new(0, 0, 100, 50), [0.25, 0.25], 0.5).unwrap().unwrap(); // The pasted rectangle lands inside the plane, around plane (0.5, 0.5). let h = sc.planes[0].homography().unwrap(); let (x, y) = h.apply(0.5, 0.5); diff --git a/crates/algo/src/warp.rs b/crates/algo/src/warp.rs index e2ca4e35d..2a8c7a586 100644 --- a/crates/algo/src/warp.rs +++ b/crates/algo/src/warp.rs @@ -30,31 +30,41 @@ const MAX_CELLS: usize = 512; /// through `warp` (source space) and then the affine `t` (source → document) in one resampling /// pass, an exact shift for whole-pixel translations (so conversions and re-renders are /// lossless), bicubic otherwise. Shared by the engine's re-render and PSD export's filter cache. -pub fn place_source(src: &Surface, src_rect: Rect, t: &photocraft_geom::Affine, warp: Option<&photocraft_geom::warp::Warp>) -> Surface { +pub fn place_source( + src: &Surface, + src_rect: Rect, + t: &photocraft_geom::Affine, + warp: Option<&photocraft_geom::warp::Warp>, +) -> Result { let [a, b, c, d, e, f] = t.m; if let Some(w) = warp.filter(|w| !w.is_identity()) { let map = |x: f64, y: f64| { let (u, v) = w.map(x, y); (a * u + c * v + e, b * u + d * v + f) }; - return warp_mesh_surface(src, src_rect, &map, Interp::Bicubic); + return Ok(warp_mesh_surface(src, src_rect, &map, Interp::Bicubic)); } let near = |x: f64, y: f64| (x - y).abs() < 1e-9; if near(a, 1.0) && near(b, 0.0) && near(c, 0.0) && near(d, 1.0) && near(e, e.round()) && near(f, f.round()) { - return crate::resample::translate_surface(src, e.round() as i32, f.round() as i32); + return Ok(crate::resample::translate_surface(src, e.round() as i32, f.round() as i32)); } crate::transform::warp_surface(src, src_rect, &crate::transform::Homography([a, c, e, b, d, f, 0.0, 0.0, 1.0]), Interp::Bicubic) } /// [`place_source`] through a projective map `h` (source → document; Distort, Perspective): the /// warp first (in source space), then `h`. -pub fn place_source_projective(src: &Surface, src_rect: Rect, h: &crate::transform::Homography, warp: Option<&photocraft_geom::warp::Warp>) -> Surface { +pub fn place_source_projective( + src: &Surface, + src_rect: Rect, + h: &crate::transform::Homography, + warp: Option<&photocraft_geom::warp::Warp>, +) -> Result { if let Some(w) = warp.filter(|w| !w.is_identity()) { let map = |x: f64, y: f64| { let (u, v) = w.map(x, y); h.apply(u, v) }; - return warp_mesh_surface(src, src_rect, &map, Interp::Bicubic); + return Ok(warp_mesh_surface(src, src_rect, &map, Interp::Bicubic)); } crate::transform::warp_surface(src, src_rect, h, Interp::Bicubic) } diff --git a/crates/engine/src/align_cmds.rs b/crates/engine/src/align_cmds.rs index 76a363cdc..99e47157a 100644 --- a/crates/engine/src/align_cmds.rs +++ b/crates/engine/src/align_cmds.rs @@ -84,11 +84,14 @@ fn auto_align(s: &mut Session, p: &Value) -> Result { let l = doc.layer(ids[i]).ok_or(EngineError::NoLayer(ids[i]))?; let surf = l.surface().ok_or(EngineError::NoLayer(ids[i]))?; let b = surf.content_bounds(); - let px = if b.is_empty() { surf.clone() } else { warp_placed(surf, b, (0.0, 0.0), pl, (0.0, 0.0), interp) }; - let mask = l.mask.as_ref().map(|m| { - let mb = m.surface.content_bounds(); - if mb.is_empty() { m.surface.clone() } else { warp_placed(&m.surface, mb, (0.0, 0.0), pl, (0.0, 0.0), Interp::Bilinear) } - }); + let px = if b.is_empty() { surf.clone() } else { warp_placed(surf, b, (0.0, 0.0), pl, (0.0, 0.0), interp)? }; + let mask = match l.mask.as_ref() { + Some(m) => { + let mb = m.surface.content_bounds(); + Some(if mb.is_empty() { m.surface.clone() } else { warp_placed(&m.surface, mb, (0.0, 0.0), pl, (0.0, 0.0), Interp::Bilinear)? }) + } + None => None, + }; Ok((ids[i], px, mask)) }) .collect::>()?; diff --git a/crates/engine/src/allocation.rs b/crates/engine/src/allocation.rs new file mode 100644 index 000000000..322e3ebd2 --- /dev/null +++ b/crates/engine/src/allocation.rs @@ -0,0 +1,70 @@ +//! Fallible buffers used by memory-intensive document operations. + +use crate::{EngineError, Result}; + +#[cfg(test)] +thread_local! { + static FAIL_NEXT: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +/// Reserve and initialize a buffer without invoking the allocator's abort-on-OOM path. +pub(crate) fn filled(len: usize, value: T, purpose: &str) -> Result> { + let mut out = capacity(len, purpose)?; + out.resize(len, value); + Ok(out) +} + +/// Reserve a vector for a known number of operation results. +pub(crate) fn capacity(len: usize, purpose: &str) -> Result> { + let bytes = len.checked_mul(std::mem::size_of::()).ok_or_else(|| error(purpose, "requested size overflowed"))?; + #[cfg(test)] + if FAIL_NEXT.with(|flag| flag.replace(false)) { + return Err(error(purpose, "not enough memory")); + } + let mut out = Vec::new(); + out.try_reserve_exact(len).map_err(|_| error(purpose, &format!("not enough memory for {bytes} bytes")))?; + Ok(out) +} + +/// Check an operation boundary where downstream encoders/decoders allocate internally. +/// The test-only failpoint lets command tests prove their transaction boundary. +pub(crate) fn checkpoint(purpose: &str) -> Result<()> { + #[cfg(test)] + if FAIL_NEXT.with(|flag| flag.replace(false)) { + return Err(error(purpose, "not enough memory")); + } + let _ = purpose; + Ok(()) +} + +/// Copy a region into an explicitly reserved buffer. +pub(crate) fn read_region(surface: &photocraft_raster::Surface, area: photocraft_geom::Rect, purpose: &str) -> Result> { + let len = (area.width() as usize) + .checked_mul(area.height() as usize) + .and_then(|n| n.checked_mul(surface.channels())) + .ok_or_else(|| error(purpose, "requested size overflowed"))?; + let mut out = filled(len, 0.0, purpose)?; + surface.read_region_into(area, &mut out); + Ok(out) +} + +fn error(purpose: &str, detail: &str) -> EngineError { + EngineError::Other(format!("not enough memory for {purpose} ({detail}); the document was not changed")) +} + +#[cfg(test)] +pub(crate) fn fail_next_for_test() { + FAIL_NEXT.with(|flag| flag.set(true)); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn allocation_failure_is_reported_without_panicking() { + fail_next_for_test(); + let result = filled::(16, 0, "test image"); + assert!(result.as_ref().is_err_and(|e| e.to_string().contains("not enough memory for test image"))); + } +} diff --git a/crates/engine/src/distort_cmds.rs b/crates/engine/src/distort_cmds.rs index 77cfdd88a..5ecb38eec 100644 --- a/crates/engine/src/distort_cmds.rs +++ b/crates/engine/src/distort_cmds.rs @@ -309,7 +309,7 @@ fn liquify(s: &mut Session, p: &Value) -> Result { fn deform_selected(surf: &Surface, sel: Option<&Surface>, f: &dyn Fn(&Surface) -> Result) -> Result { match sel { Some(sel) => { - let (lifted, mut rest) = crate::transform_cmds::split_selected(surf, sel); + let (lifted, mut rest) = crate::transform_cmds::split_selected(surf, sel)?; let moved = f(&lifted)?; crate::transform_cmds::composite_over(&mut rest, &moved); rest.prune(); diff --git a/crates/engine/src/edit_cmds.rs b/crates/engine/src/edit_cmds.rs index 49031f711..1d2ee0363 100644 --- a/crates/engine/src/edit_cmds.rs +++ b/crates/engine/src/edit_cmds.rs @@ -65,7 +65,7 @@ fn active_id(s: &Session) -> Result { } /// The pixels of `src` inside the selection (or all of them), as a straight-alpha surface. -fn lift(src: &Surface, sel: Option<&Surface>, canvas: Rect) -> Clip { +fn lift(src: &Surface, sel: Option<&Surface>, canvas: Rect) -> Result { let fmt = src.format(); let with_alpha = PixelFormat::new(fmt.mode, fmt.sample, true); let area = match sel { @@ -74,34 +74,45 @@ fn lift(src: &Surface, sel: Option<&Surface>, canvas: Rect) -> Clip { }; let mut out = Surface::new(with_alpha); if area.is_empty() { - return Clip { surface: out, bounds: Rect::EMPTY }; + return Ok(Clip { surface: out, bounds: Rect::EMPTY }); } // Only the selected area is read: converting the whole layer (a 24 MP Background gaining // alpha) cost about a second per Layer via Copy (#668). let n = with_alpha.channels(); let mut px = if fmt.alpha { - src.read_region(area) + crate::allocation::read_region(src, area, "copying pixels")? } else { - let opaque = src.read_region(area); + let opaque = crate::allocation::read_region(src, area, "copying pixels")?; let k = fmt.channels(); - let mut v = Vec::with_capacity(opaque.len() / k.max(1) * n); + let len = opaque + .len() + .checked_div(k.max(1)) + .and_then(|pixels| pixels.checked_mul(n)) + .ok_or_else(|| EngineError::Other("not enough memory for copying pixels (requested size overflowed); the document was not changed".into()))?; + let mut v = crate::allocation::filled(len, 0.0, "copying pixels")?; + let mut at = 0; for p in opaque.chunks_exact(k.max(1)) { - v.extend_from_slice(p); - v.push(1.0); + let Some(dst) = v.get_mut(at..at + k.max(1)) else { break }; + dst.copy_from_slice(p); + at += k.max(1); + if let Some(alpha) = v.get_mut(at) { + *alpha = 1.0; + } + at += 1; } v }; if let Some(m) = sel { - let mask = m.read_region(area); + let mask = crate::allocation::read_region(m, area, "applying the selection to copied pixels")?; let mk = m.format().channels().max(1); for (p, a) in px.chunks_exact_mut(n).zip(mask.chunks_exact(mk)) { p[n - 1] *= a[0]; } } - out.write_region(area, &px); + out.try_write_region(area, &px).map_err(|e| EngineError::Other(format!("not enough memory while copying pixels ({e}); the document was not changed")))?; out.prune(); let bounds = out.content_bounds(); - Clip { surface: out, bounds } + Ok(Clip { surface: out, bounds }) } /// Merged composite of the visible document as a surface in the document's format. @@ -114,11 +125,11 @@ fn copy(s: &mut Session, merged: bool) -> Result { let d = s.active().ok_or(EngineError::NoDocument)?; let canvas = d.doc.bounds(); let clip = if merged { - lift(&merged_surface(&d.doc), d.doc.selection.as_ref(), canvas) + lift(&merged_surface(&d.doc), d.doc.selection.as_ref(), canvas)? } else { let id = d.active_layer.ok_or(EngineError::Other("no active layer".into()))?; let surf = d.doc.layer(id).and_then(Layer::surface).ok_or(EngineError::Other("the active layer has no pixels".into()))?; - lift(surf, d.doc.selection.as_ref(), canvas) + lift(surf, d.doc.selection.as_ref(), canvas)? }; if clip.bounds.is_empty() { return Err(EngineError::Other("Could not copy: the selected area is empty".into())); @@ -141,9 +152,9 @@ pub(crate) fn clear_area(doc: &mut Document, id: LayerId, area: Rect, sel: Optio let bg = crate::extra_cmds::is_background(doc.layer(id).ok_or(EngineError::NoLayer(id))?); let surf = crate::commands::paint_surface(doc, id, &Value::Null)?; if bg { - crate::pixels::fill_surface(surf, area, background, sel, true); + crate::pixels::try_fill_surface(surf, area, background, sel, true)?; } else { - crate::pixels::clear_surface(surf, area, sel); + crate::pixels::try_clear_surface(surf, area, sel)?; } surf.prune(); Ok(()) @@ -152,6 +163,7 @@ pub(crate) fn clear_area(doc: &mut Document, id: LayerId, area: Rect, sel: Optio /// Paste as a new layer (or into the targeted mask or channel: [`paste_to_target`]). `in_place` keeps the original position; otherwise the pixels are centred /// on `center` (the view centre from the UI) or the canvas, unless they already overlap the canvas. fn paste(s: &mut Session, p: &Value, in_place: bool) -> Result { + crate::allocation::checkpoint("pasting pixels")?; let clip = s.clipboard.clone().ok_or(EngineError::Other("the clipboard is empty".into()))?; let Some(d) = s.active() else { // Nothing open to paste into: the clipboard becomes a document of its own (#368). @@ -199,6 +211,7 @@ fn shifted(surface: &Surface, dx: i32, dy: i32) -> Surface { /// over what was there. Their transparency, and `limit` (Paste Into / Outside), let it show /// through. One history step. pub(crate) fn paste_to_target(s: &mut Session, p: &Value, in_place: bool, limit: Option<&Surface>, label: &str) -> Result { + crate::allocation::checkpoint("pasting pixels")?; let clip = s.clipboard.clone().ok_or(EngineError::Other("the clipboard is empty".into()))?; let d = s.active().ok_or(EngineError::NoDocument)?; let canvas = d.doc.bounds(); @@ -208,7 +221,7 @@ pub(crate) fn paste_to_target(s: &mut Session, p: &Value, in_place: bool, limit: let area = src.content_bounds().intersect(&canvas); s.edit(label, |doc, _| { let (surf, _) = crate::channel_cmds::target_surface(doc, layer, p)?; - crate::fill_cmds::composite_over(surf, &src, area, limit); + crate::fill_cmds::composite_over(surf, &src, area, limit)?; surf.prune(); doc.selection = None; Ok(()) @@ -254,7 +267,7 @@ fn layer_via(s: &mut Session, cut: bool) -> Result { return Ok(json!({"layer": nid.0})); } let surf = d.doc.layer(id).and_then(Layer::surface).ok_or(EngineError::Other("the active layer has no pixels".into()))?; - let clip = lift(surf, d.doc.selection.as_ref(), d.doc.bounds()); + let clip = lift(surf, d.doc.selection.as_ref(), d.doc.bounds())?; if clip.bounds.is_empty() { return Err(EngineError::Other("Could not complete the command: the selected area is empty".into())); } @@ -623,6 +636,104 @@ mod tests { s } + #[test] + fn injected_clear_buffer_failure_leaves_the_document_unchanged() { + let mut s = session(); + let st = s.active().unwrap(); + let id = st.active_layer.unwrap(); + let before = st.doc.layer(id).unwrap().surface().unwrap().clone(); + let revision = st.revision; + + crate::allocation::fail_next_for_test(); + let result = s.execute("edit.clear", json!({})); + + assert!(result.as_ref().is_err_and(|e| e.to_string().contains("not enough memory for erasing pixels"))); + let st = s.active().unwrap(); + assert_eq!(st.doc.layer(id).unwrap().surface(), Some(&before)); + assert_eq!(st.revision, revision); + } + + #[test] + fn injected_copy_buffer_failure_does_not_change_document_or_clipboard() { + let mut s = session(); + let st = s.active().unwrap(); + let id = st.active_layer.unwrap(); + let before = st.doc.layer(id).unwrap().surface().unwrap().clone(); + let revision = st.revision; + + crate::allocation::fail_next_for_test(); + let result = s.execute("edit.copy", json!({})); + + assert!(result.as_ref().is_err_and(|e| e.to_string().contains("not enough memory for copying pixels"))); + let st = s.active().unwrap(); + assert_eq!(st.doc.layer(id).unwrap().surface(), Some(&before)); + assert_eq!(st.revision, revision); + assert!(s.clipboard.is_none()); + } + + #[test] + fn injected_fill_result_reservation_failure_keeps_pixels_and_history() { + let mut s = session(); + let st = s.active().unwrap(); + let id = st.active_layer.unwrap(); + let before = st.doc.layer(id).unwrap().surface().unwrap().clone(); + let revision = st.revision; + let history_len = st.history.past_len(); + + crate::allocation::fail_next_for_test(); + let result = s.execute("edit.fill", json!({"color": "#00ff00"})); + + assert!(result.as_ref().is_err_and(|e| e.to_string().contains("not enough memory for blending pixels"))); + let st = s.active().unwrap(); + assert_eq!(st.doc.layer(id).unwrap().surface(), Some(&before)); + assert_eq!(st.revision, revision); + assert_eq!(st.history.past_len(), history_len); + } + + #[test] + fn injected_paste_failure_does_not_change_document_or_clipboard() { + let mut s = session(); + s.execute("select.rect", json!({"x": 10, "y": 10, "width": 8, "height": 8})).unwrap(); + s.execute("edit.copy", json!({})).unwrap(); + let before_clip = s.clipboard.clone(); + let st = s.active().unwrap(); + let before_doc = (*st.doc).clone(); + let revision = st.revision; + let history_len = st.history.past_len(); + + crate::allocation::fail_next_for_test(); + let result = s.execute("edit.paste", json!({})); + + assert!(result.as_ref().is_err_and(|e| e.to_string().contains("not enough memory for pasting pixels"))); + let st = s.active().unwrap(); + assert_eq!(*st.doc, before_doc); + assert_eq!(st.revision, revision); + assert_eq!(st.history.past_len(), history_len); + let after_clip = s.clipboard.as_ref().expect("clipboard remains available"); + let before_clip = before_clip.as_ref().expect("clipboard was populated"); + assert_eq!(after_clip.bounds, before_clip.bounds); + assert_eq!(after_clip.surface, before_clip.surface); + } + + #[test] + fn injected_transform_failure_keeps_pixels_and_history() { + let mut s = session(); + let st = s.active().unwrap(); + let id = st.active_layer.unwrap(); + let before = st.doc.layer(id).unwrap().surface().unwrap().clone(); + let revision = st.revision; + let history_len = st.history.past_len(); + + crate::allocation::fail_next_for_test(); + let result = s.execute("edit.transform", json!({"matrix": [1, 0, 0, 1, 20, 30]})); + + assert!(result.as_ref().is_err_and(|e| e.to_string().contains("not enough memory for transforming pixels"))); + let st = s.active().unwrap(); + assert_eq!(st.doc.layer(id).unwrap().surface(), Some(&before)); + assert_eq!(st.revision, revision); + assert_eq!(st.history.past_len(), history_len); + } + fn active_bounds(s: &Session) -> Rect { let st = s.active().unwrap(); st.doc.layer(st.active_layer.unwrap()).unwrap().surface().unwrap().content_bounds() diff --git a/crates/engine/src/edit_menu_cmds.rs b/crates/engine/src/edit_menu_cmds.rs index 46f924511..edae7d64c 100644 --- a/crates/engine/src/edit_menu_cmds.rs +++ b/crates/engine/src/edit_menu_cmds.rs @@ -572,7 +572,7 @@ fn content_aware_scale(s: &mut Session, p: &Value) -> Result { move |s, out| { s.edit(label, |doc, _| { let surf = writable_surface(doc, id)?; - crate::pixels::clear_surface(surf, region, None); + crate::pixels::try_clear_surface(surf, region, None)?; surf.write_region(dst, &out); surf.prune(); Ok(()) diff --git a/crates/engine/src/extra_cmds.rs b/crates/engine/src/extra_cmds.rs index 8a2e925c4..c3453c314 100644 --- a/crates/engine/src/extra_cmds.rs +++ b/crates/engine/src/extra_cmds.rs @@ -218,7 +218,7 @@ fn stroke(s: &mut Session, p: &Value) -> Result { let surf = crate::commands::paint_surface(doc, id, &Value::Null)?; let area = band.content_bounds().intersect(&canvas); if !area.is_empty() { - crate::fill_cmds::blend_color_mask(surf, area, color, &band, mode, opacity, preserve || lock); + crate::fill_cmds::blend_color_mask(surf, area, color, &band, mode, opacity, preserve || lock)?; surf.prune(); } Ok(()) diff --git a/crates/engine/src/file_cmds.rs b/crates/engine/src/file_cmds.rs index 2d30b35d6..103a1f6f3 100644 --- a/crates/engine/src/file_cmds.rs +++ b/crates/engine/src/file_cmds.rs @@ -205,6 +205,7 @@ pub(crate) fn sanitize(name: &str) -> String { } pub(crate) fn import(name: &str, bytes: &[u8]) -> Result { + crate::allocation::checkpoint("importing document")?; let r = photocraft_io::import(name, bytes).map_err(|e| EngineError::Other(format!("{name}: {e}")))?; // Auxiliary imports return only a document and cannot surface the preview's fidelity warning. // Open has its own warning-preserving path; never silently place or process a thumbnail. @@ -245,6 +246,7 @@ impl From> for SaveOpts { /// Encodes `doc` for `path`'s extension. pub(crate) fn encode(doc: &Document, path: &str, save: impl Into) -> Result<(Vec, Vec)> { + crate::allocation::checkpoint("saving document")?; let save = save.into(); let mut opts = photocraft_io::ExportOptions { tiff_layers: save.tiff_layers, ..Default::default() }; if let Some(q) = save.quality { @@ -359,6 +361,7 @@ fn save_a_copy(s: &mut Session, p: &Value) -> Result { /// Open a file's bytes, decoding them as the format `as_ext` (Open As) when given. pub fn open_bytes_as(s: &mut Session, name: &str, bytes: &[u8], as_ext: Option<&str>, path: Option) -> Result { + crate::allocation::checkpoint("importing document")?; let decode_name = match as_ext { Some(ext) => format!("{}.{}", stem(name), ext.trim_start_matches('.')), None => name.to_string(), diff --git a/crates/engine/src/file_cmds/tests.rs b/crates/engine/src/file_cmds/tests.rs index 65cfea290..3ea6a319d 100644 --- a/crates/engine/src/file_cmds/tests.rs +++ b/crates/engine/src/file_cmds/tests.rs @@ -91,6 +91,28 @@ fn revert_reloads_as_one_undoable_step() { assert!(s.execute("file.revert", json!({})).is_err()); } +#[test] +fn injected_import_failure_does_not_open_a_document() { + let mut s = session(8, 8, 8); + let before = s.documents().len(); + crate::allocation::fail_next_for_test(); + let result = open_bytes_as(&mut s, "image.png", b"not decoded", None, None); + assert!(result.as_ref().is_err_and(|e| e.to_string().contains("not enough memory for importing document"))); + assert_eq!(s.documents().len(), before); +} + +#[test] +fn injected_save_failure_does_not_modify_the_destination() { + let dir = tmp("allocation-save"); + let path = join(&dir, "existing.psd"); + std::fs::write(&path, b"previous file").unwrap(); + let s = session(8, 8, 8); + crate::allocation::fail_next_for_test(); + let result = save_doc(doc(&s), &path, None); + assert!(result.as_ref().is_err_and(|e| e.to_string().contains("not enough memory for saving document"))); + assert_eq!(std::fs::read(path).unwrap(), b"previous file"); +} + #[test] fn save_a_copy_keeps_path_and_dirty_state() { let dir = tmp("copy"); diff --git a/crates/engine/src/fill_cmds.rs b/crates/engine/src/fill_cmds.rs index 9453652e7..4d25474f3 100644 --- a/crates/engine/src/fill_cmds.rs +++ b/crates/engine/src/fill_cmds.rs @@ -127,7 +127,7 @@ pub fn fill(s: &mut Session, p: &Value) -> Result { let mut source_doc = (*source_doc).clone(); let (src, _) = crate::channel_cmds::target_surface(&mut source_doc, id, p) .map_err(|_| EngineError::Other("the target did not exist (as pixels, a mask or a channel) in that history state".into()))?; - Source::Pixels(area, read_rgba(src, area)) + Source::Pixels(area, read_rgba(src, area, "reading Fill's history source")?) } "contentAware" => { let sel = sel.as_ref().ok_or_else(|| EngineError::Other("Content-Aware fill needs a selection".into()))?; @@ -149,23 +149,26 @@ pub fn fill(s: &mut Session, p: &Value) -> Result { s.edit("Fill", |doc, _| { let sel = doc.selection.clone(); let (surf, lock) = crate::channel_cmds::target_surface(doc, id, p)?; - blend_into(surf, area, &source, sel.as_ref(), Blend { mode, opacity, keep_alpha: preserve || lock, restore }); + blend_into(surf, area, &source, sel.as_ref(), Blend { mode, opacity, keep_alpha: preserve || lock, restore })?; Ok(()) })?; Ok(out) } -fn read_rgba(src: &Surface, area: Rect) -> Vec<[f32; 4]> { - let mut px = vec![[0.0f32; 4]; area.width() as usize * area.height() as usize]; +fn read_rgba(src: &Surface, area: Rect, purpose: &str) -> Result> { + let len = (area.width() as usize) + .checked_mul(area.height() as usize) + .ok_or_else(|| EngineError::Other(format!("not enough memory for {purpose} (requested size overflowed); the document was not changed")))?; + let mut px = crate::allocation::filled(len, [0.0f32; 4], purpose)?; src.read_rgba_into(area, &mut px); - px + Ok(px) } /// `src` over `surf` across `area` (Normal, 100 %), each pixel weighted by `limit` when given: /// a paste into a layer mask or channel, which keeps the pasted pixels' luminosity (#1035). -pub(crate) fn composite_over(surf: &mut Surface, src: &Surface, area: Rect, limit: Option<&Surface>) { - let source = Source::Pixels(area, read_rgba(src, area)); - blend_into(surf, area, &source, limit, Blend { mode: BlendMode::Normal, opacity: 1.0, keep_alpha: false, restore: false }); +pub(crate) fn composite_over(surf: &mut Surface, src: &Surface, area: Rect, limit: Option<&Surface>) -> Result<()> { + let source = Source::Pixels(area, read_rgba(src, area, "pasting pixels")?); + blend_into(surf, area, &source, limit, Blend { mode: BlendMode::Normal, opacity: 1.0, keep_alpha: false, restore: false }) } /// Reuse Edit › Fill's compositing for Edit › Stroke. The stroke's rasterized band @@ -179,8 +182,8 @@ pub(crate) fn blend_color_mask( mode: BlendMode, opacity: f32, preserve_transparency: bool, -) { - blend_into(surf, area, &Source::Color(color), Some(coverage), Blend { mode, opacity, keep_alpha: preserve_transparency, restore: false }); +) -> Result<()> { + blend_into(surf, area, &Source::Color(color), Some(coverage), Blend { mode, opacity, keep_alpha: preserve_transparency, restore: false }) } /// The selection filled from its surroundings (`photocraft_algo::content_aware`), as straight @@ -262,7 +265,7 @@ fn lerp(d: [f32; 4], c: [f32; 4], k: f32) -> [f32; 4] { /// Blend `source` into `surf` over `area`: per pixel, the selection coverage times the opacity, /// in the blend mode. /// Works tile by tile in parallel on the encoded pixels (no full-surface f32 round trip). -fn blend_into(surf: &mut Surface, area: Rect, source: &Source, sel: Option<&Surface>, b: Blend) { +fn blend_into(surf: &mut Surface, area: Rect, source: &Source, sel: Option<&Surface>, b: Blend) -> Result<()> { use rayon::prelude::*; let fmt = surf.format(); // An opaque colour in Normal mode at 100 % that may replace alpha: fully covered tiles become @@ -275,24 +278,35 @@ fn blend_into(surf: &mut Surface, area: Rect, source: &Source, sel: Option<&Surf } _ => None, }; - let tiles = surf.take_tiles(area); - let done: Vec<_> = tiles - .into_par_iter() - .map(|(tc, tile)| { - let tr = tc.rect().intersect(&area); - let mask = sel.map(|m| Mask { bytes: m.tile(tc).map_or(m.default_bytes(), |t| t.bytes()), fmt: m.format(), origin: tc.rect() }); - if let Some(s) = &solid - && tr == tc.rect() - && mask.as_ref().is_none_or(|m| m.full(tr)) - { - return (tc, s.clone()); - } - let mut tile = std::sync::Arc::unwrap_or_clone(tile); - blend_tile(tile.bytes_mut(), &fmt, tc.rect(), tr, source, mask.as_ref(), b); - (tc, std::sync::Arc::new(tile)) - }) - .collect(); + let count = area.tiles().count(); + let done = crate::allocation::capacity(count, "blending pixels")?; + let tiles = surf + .try_take_tiles(area) + .map_err(|e| EngineError::Other(format!("not enough memory while preparing pixel tiles ({e}); the document was not changed")))?; + let done = std::sync::Mutex::new(done); + tiles.into_par_iter().try_for_each(|(tc, tile)| -> Result<()> { + let tr = tc.rect().intersect(&area); + let mask = sel.map(|m| Mask { bytes: m.tile(tc).map_or(m.default_bytes(), |t| t.bytes()), fmt: m.format(), origin: tc.rect() }); + if let Some(s) = &solid + && tr == tc.rect() + && mask.as_ref().is_none_or(|m| m.full(tr)) + { + done.lock().unwrap_or_else(std::sync::PoisonError::into_inner).push((tc, s.clone())); + return Ok(()); + } + let mut tile = match std::sync::Arc::try_unwrap(tile) { + Ok(tile) => tile, + Err(tile) => tile + .try_clone() + .map_err(|e| EngineError::Other(format!("not enough memory while blending a pixel tile ({e}); the document was not changed")))?, + }; + blend_tile(tile.bytes_mut(), &fmt, tc.rect(), tr, source, mask.as_ref(), b); + done.lock().unwrap_or_else(std::sync::PoisonError::into_inner).push((tc, std::sync::Arc::new(tile))); + Ok(()) + })?; + let done = done.into_inner().unwrap_or_else(std::sync::PoisonError::into_inner); surf.put_tiles(done); + Ok(()) } /// The selection's coverage over one tile (its channel 0). @@ -501,8 +515,8 @@ mod tests { let st = s.active().unwrap(); let mut surf = st.doc.layer(st.active_layer.unwrap()).unwrap().surface().unwrap().clone(); let t = std::time::Instant::now(); - crate::pixels::fill_surface(&mut surf, Rect::new(0, 0, 6000, 4000), [0.2, 0.4, 0.6, 1.0], None, false); - println!("{:<48} {:>8.1} ms", "old fill_surface, no selection", t.elapsed().as_secs_f64() * 1000.0); + crate::pixels::try_fill_surface(&mut surf, Rect::new(0, 0, 6000, 4000), [0.2, 0.4, 0.6, 1.0], None, false).unwrap(); + println!("{:<48} {:>8.1} ms", "fallible fill_surface, no selection", t.elapsed().as_secs_f64() * 1000.0); } #[test] diff --git a/crates/engine/src/float_cmds.rs b/crates/engine/src/float_cmds.rs index e4e5acc48..892f75474 100644 --- a/crates/engine/src/float_cmds.rs +++ b/crates/engine/src/float_cmds.rs @@ -82,7 +82,7 @@ impl CutParts { rest.write_region(b, &rp); } if fill { - crate::pixels::fill_surface(&mut rest, b, background, Some(sel), true); + crate::pixels::try_fill_surface(&mut rest, b, background, Some(sel), true)?; } rest.prune(); piece.write_region(b, &pp); @@ -109,7 +109,7 @@ impl CutParts { let mut out = self.rest.clone(); let piece = photocraft_algo::resample::translate_surface(&self.piece, dx, dy); if self.mask { - crate::fill_cmds::composite_over(&mut out, &piece, piece.content_bounds(), None); + crate::fill_cmds::composite_over(&mut out, &piece, piece.content_bounds(), None)?; } else { crate::transform_cmds::composite_over(&mut out, &piece); } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index b6d8016e4..df55281a4 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -11,6 +11,7 @@ pub mod actions_cmds; pub mod adjust_cmds; pub mod adjust_params; pub mod align_cmds; +mod allocation; pub mod analysis_cmds; pub mod artboard_cmds; pub mod automate_cmds; @@ -140,6 +141,12 @@ pub enum EngineError { pub type Result = std::result::Result; +impl From for EngineError { + fn from(error: photocraft_raster::AllocationError) -> Self { + Self::Other(format!("not enough memory for this operation ({error}); the document was not changed")) + } +} + /// The pixels of a raster layer (typically one a command just created), as an error instead /// of a panic if the layer has none. pub(crate) fn pixels_mut(l: &mut photocraft_doc::Layer) -> Result<&mut photocraft_raster::Surface> { diff --git a/crates/engine/src/mode_cmds.rs b/crates/engine/src/mode_cmds.rs index f5e9c37f5..2fcd30ed4 100644 --- a/crates/engine/src/mode_cmds.rs +++ b/crates/engine/src/mode_cmds.rs @@ -87,7 +87,7 @@ pub(crate) fn rotate_arbitrary(s: &mut Session, p: &Value) -> Result { if background && let Some(surf) = l.surface_mut() { // The Background stays a Background: rotated pixels over the background colour. let src = surf.content_bounds(); - let rotated = photocraft_algo::transform::warp_surface(surf, src, &h, interp); + let rotated = photocraft_algo::transform::warp_surface(surf, src, &h, interp)?; let mut base = Surface::new(fmt); let fill = photocraft_raster::from_rgba(&fmt, bg); base.write_region(canvas, &fill.repeat(canvas.width() as usize * canvas.height() as usize)); @@ -95,7 +95,7 @@ pub(crate) fn rotate_arbitrary(s: &mut Session, p: &Value) -> Result { base.prune(); *surf = base; if let Some(m) = l.mask.as_mut() { - m.surface = crate::transform_cmds::warp_gray(&m.surface, &h, interp); + m.surface = crate::transform_cmds::warp_gray(&m.surface, &h, interp)?; } } else { // Rotating the whole image moves locked layers too. @@ -110,10 +110,10 @@ pub(crate) fn rotate_arbitrary(s: &mut Session, p: &Value) -> Result { } crate::canvas_geom::transform_doc_marks(doc, &a); for ch in doc.channels.iter_mut().chain(doc.quick_mask.as_mut()) { - ch.surface = crate::transform_cmds::warp_gray(&ch.surface, &h, interp); + ch.surface = crate::transform_cmds::warp_gray(&ch.surface, &h, interp)?; } if let Some(sel) = &doc.selection { - doc.selection = Some(crate::transform_cmds::warp_gray(sel, &h, Interp::Bilinear)).filter(|s| !s.content_bounds().is_empty()); + doc.selection = Some(crate::transform_cmds::warp_gray(sel, &h, Interp::Bilinear)?).filter(|s| !s.content_bounds().is_empty()); } // Type, shapes and smart objects re-render from their new geometry. crate::canvas_geom::refresh(doc, crate::canvas_geom::Refresh::All); diff --git a/crates/engine/src/photo_cmds.rs b/crates/engine/src/photo_cmds.rs index 50255c944..bd8c94742 100644 --- a/crates/engine/src/photo_cmds.rs +++ b/crates/engine/src/photo_cmds.rs @@ -156,12 +156,12 @@ fn exif_focal(srcs: &[Source]) -> Option { /// Warps the content of `surf` inside `rect` by a placement shifted by `offset`; image /// coordinates are surface coordinates minus `origin`. -pub(crate) fn warp_placed(surf: &Surface, rect: Rect, origin: (f64, f64), pl: &Placement, offset: (f64, f64), interp: Interp) -> Surface { +pub(crate) fn warp_placed(surf: &Surface, rect: Rect, origin: (f64, f64), pl: &Placement, offset: (f64, f64), interp: Interp) -> Result { let pl = pl.translated(offset.0, offset.1); if let Some(h) = plane_matrix(&pl, origin) { - return warp_surface(surf, rect, &h, interp); + return Ok(warp_surface(surf, rect, &h, interp)?); } - warp_mesh_surface(surf, rect, &|x, y| pl.forward(x - origin.0, y - origin.1), interp) + Ok(warp_mesh_surface(surf, rect, &|x, y| pl.forward(x - origin.0, y - origin.1), interp)) } /// The projective matrix of a planar, distortion-free placement (surface px → panorama px). @@ -447,7 +447,7 @@ fn photomerge(s: &mut Session, p: &Value) -> Result { .map(|(k, (&i, pl))| { ctx.check()?; ctx.progress(0.35 + 0.25 * k as f32 / placed.len().max(1) as f32, ""); - Ok(warp_placed(&srcs[i].surf, Rect::new(0, 0, srcs[i].w as i32, srcs[i].h as i32), (0.0, 0.0), pl, offset, Interp::Bicubic)) + warp_placed(&srcs[i].surf, Rect::new(0, 0, srcs[i].w as i32, srcs[i].h as i32), (0.0, 0.0), pl, offset, Interp::Bicubic) }) .collect::>>()? }; @@ -758,7 +758,7 @@ fn crop_and_straighten(s: &mut Session, _p: &Value) -> Result { let corners = f.corners(); let hq = Homography::rect_to_quad([0.0, 0.0, pw as f64, ph as f64], corners).and_then(|h| h.inverse()); let Some(hm) = hq else { continue }; - let warped = warp_surface(&flat, area, &hm, Interp::Bicubic); + let warped = warp_surface(&flat, area, &hm, Interp::Bicubic)?; let keep = Rect::new(0, 0, pw as i32, ph as i32); let mut px = Surface::new(fmt); px.write_region(keep, &warped.read_region(keep)); diff --git a/crates/engine/src/pixels.rs b/crates/engine/src/pixels.rs index 1bf1face4..b917ffac3 100644 --- a/crates/engine/src/pixels.rs +++ b/crates/engine/src/pixels.rs @@ -86,10 +86,24 @@ fn adjusted(s: &Surface, r: Rect, adj: &Adjustment, selection: Option<&Surface>, } /// Fill (respecting selection coverage) with a straight RGBA colour. -pub fn fill_surface(s: &mut Surface, area: Rect, color: [f32; 4], selection: Option<&Surface>, lock_transparency: bool) { +pub(crate) fn try_fill_surface(s: &mut Surface, area: Rect, color: [f32; 4], selection: Option<&Surface>, lock_transparency: bool) -> crate::Result<()> { let fmt = s.format(); let n = fmt.channels(); - let mut region = s.read_region(area); + let mut region = crate::allocation::read_region(s, area, "filling pixels")?; + fill_region(&mut region, fmt, n, area, color, selection, lock_transparency); + s.try_write_region(area, ®ion) + .map_err(|e| crate::EngineError::Other(format!("not enough memory while writing filled pixels ({e}); the document was not changed"))) +} + +fn fill_region( + region: &mut [f32], + fmt: photocraft_color::PixelFormat, + n: usize, + area: Rect, + color: [f32; 4], + selection: Option<&Surface>, + lock_transparency: bool, +) { let w = area.width() as usize; for (i, px) in region.chunks_exact_mut(n).enumerate() { let x = area.x0 + (i % w) as i32; @@ -112,17 +126,24 @@ pub fn fill_surface(s: &mut Surface, area: Rect, color: [f32; 4], selection: Opt photocraft_raster::from_rgba_into(&fmt, o, &mut enc); px.copy_from_slice(&enc[..n]); } - s.write_region(area, ®ion); } /// Clear pixels (make transparent) within the selection. -pub fn clear_surface(s: &mut Surface, area: Rect, selection: Option<&Surface>) { +pub(crate) fn try_clear_surface(s: &mut Surface, area: Rect, selection: Option<&Surface>) -> crate::Result<()> { let fmt = s.format(); let n = fmt.channels(); if !fmt.alpha { - return; + return Ok(()); } - let mut region = s.read_region(area); + let mut region = crate::allocation::read_region(s, area, "erasing pixels")?; + clear_region(&mut region, n, area, selection); + s.try_write_region(area, ®ion) + .map_err(|e| crate::EngineError::Other(format!("not enough memory while erasing pixels ({e}); the document was not changed")))?; + s.prune(); + Ok(()) +} + +fn clear_region(region: &mut [f32], n: usize, area: Rect, selection: Option<&Surface>) { let w = area.width() as usize; for (i, px) in region.chunks_exact_mut(n).enumerate() { let k = selection.map_or(1.0, |sel| sel.sample_channel(area.x0 + (i % w) as i32, area.y0 + (i / w) as i32, 0)); @@ -132,8 +153,6 @@ pub fn clear_surface(s: &mut Surface, area: Rect, selection: Option<&Surface>) { px.fill(0.0); } } - s.write_region(area, ®ion); - s.prune(); } /// Remap a surface through a pixel-coordinate mapping (used for flips/rotations). diff --git a/crates/engine/src/select_extra_cmds.rs b/crates/engine/src/select_extra_cmds.rs index 9d42cb36c..e2c9e7059 100644 --- a/crates/engine/src/select_extra_cmds.rs +++ b/crates/engine/src/select_extra_cmds.rs @@ -195,7 +195,7 @@ fn transform_selection(s: &mut Session, p: &Value) -> Result { } let selected = s.edit("Transform Selection", |doc, _| { if let Some(sel) = &doc.selection { - doc.selection = Some(crate::transform_cmds::warp_gray(sel, &h, interp)).filter(|s| !s.content_bounds().is_empty()); + doc.selection = Some(crate::transform_cmds::warp_gray(sel, &h, interp)?).filter(|s| !s.content_bounds().is_empty()); } Ok(doc.selection.is_some()) })?; diff --git a/crates/engine/src/smart_cmds.rs b/crates/engine/src/smart_cmds.rs index 3a3dd7e52..5298f8046 100644 --- a/crates/engine/src/smart_cmds.rs +++ b/crates/engine/src/smart_cmds.rs @@ -359,12 +359,12 @@ pub fn render(doc: &Document, sm: &SmartObject) -> Result> { Some(p) => { let [h0, h1, h2, h3, h4, h5, h6, h7, h8] = *p; let h = Homography([h0 / k, h1 / k, h2, h3 / k, h4 / k, h5, h6 / k, h7 / k, h8]); - photocraft_algo::warp::place_source_projective(&img.surface, img.bounds, &h, None) + photocraft_algo::warp::place_source_projective(&img.surface, img.bounds, &h, None)? } None => { let [a, b, c, d, e, f] = sm.transform.m; let t = Affine { m: [a / k, b / k, c / k, d / k, e, f] }; - photocraft_algo::warp::place_source(&img.surface, img.bounds, &t, None) + photocraft_algo::warp::place_source(&img.surface, img.bounds, &t, None)? } }; return Ok(Some(apply_smart_filters(&placed, sm, doc.bounds()))); @@ -375,8 +375,8 @@ pub fn render(doc: &Document, sm: &SmartObject) -> Result> { }; // Through the warp (source space) and the transform in one pass; whole-pixel moves are exact. let placed = match &sm.perspective { - Some(p) => photocraft_algo::warp::place_source_projective(&img.surface, img.bounds, &Homography(*p), sm.warp.as_ref()), - None => photocraft_algo::warp::place_source(&img.surface, img.bounds, &sm.transform, sm.warp.as_ref()), + Some(p) => photocraft_algo::warp::place_source_projective(&img.surface, img.bounds, &Homography(*p), sm.warp.as_ref())?, + None => photocraft_algo::warp::place_source(&img.surface, img.bounds, &sm.transform, sm.warp.as_ref())?, }; Ok(Some(apply_smart_filters(&placed, sm, doc.bounds()))) } diff --git a/crates/engine/src/transform_cmds.rs b/crates/engine/src/transform_cmds.rs index b2ad24a56..1199c6032 100644 --- a/crates/engine/src/transform_cmds.rs +++ b/crates/engine/src/transform_cmds.rs @@ -35,43 +35,53 @@ pub fn transform_bounds(doc: &Document, layer: &Layer) -> Rect { /// Warp a surface whose pixels outside the content read as `default` (masks, selections): /// warp the content with alpha, then flatten back onto the default value. -pub(crate) fn warp_gray(s: &Surface, h: &Homography, interp: Interp) -> Surface { +pub(crate) fn warp_gray(s: &Surface, h: &Homography, interp: Interp) -> Result { let default = s.default_pixel().first().copied().unwrap_or(0.0); let fmt = s.format(); let src = s.content_bounds(); let mut out = Surface::with_default(fmt, &[default]); if src.is_empty() { - return out; + return Ok(out); } // Content-only copy (default 0 + alpha) so the "outside" is transparent during the warp. let with_alpha = PixelFormat::new(fmt.mode, fmt.sample, true); let mut tmp = Surface::new(with_alpha); - let v = s.read_region(src); - tmp.write_region(src, &v.iter().flat_map(|g| [*g, 1.0]).collect::>()); - let w = warp_surface(&tmp, src, h, interp); + let v = crate::allocation::read_region(s, src, "transforming a mask or channel")?; + let mut lifted = + crate::allocation::filled(v.len().checked_mul(2).ok_or_else(|| bad("the mask is too large to transform"))?, 0.0, "transforming a mask or channel")?; + let (lifted_pairs, _) = lifted.as_chunks_mut::<2>(); + for (g, px) in v.iter().zip(lifted_pairs) { + px.copy_from_slice(&[*g, 1.0]); + } + tmp.try_write_region(src, &lifted)?; + let w = warp_surface(&tmp, src, h, interp)?; // Clear the old content region, then composite the warped content over it. These are written as // two sparse regions rather than one dense `src ∪ warped` block: a transform that moves the // content far away (e.g. a huge translation) would otherwise allocate a buffer spanning both and // hang/OOM. `Surface` is tile-sparse, so distant regions cost only their own tiles. - let old: Vec = vec![default; src.width() as usize * src.height() as usize]; - out.write_region(src, &old); + let old_len = (src.width() as usize).checked_mul(src.height() as usize).ok_or_else(|| bad("the mask is too large to transform"))?; + let old = crate::allocation::filled(old_len, default, "transforming a mask or channel")?; + out.try_write_region(src, &old)?; let b = w.content_bounds(); if !b.is_empty() { - let px = w.read_region(b); - let flat: Vec = px.as_chunks::<2>().0.iter().map(|p| p[0] * p[1] + default * (1.0 - p[1])).collect(); - out.write_region(b, &flat); + let px = crate::allocation::read_region(&w, b, "flattening a transformed mask or channel")?; + let mut flat = crate::allocation::filled(px.len() / 2, 0.0, "flattening a transformed mask or channel")?; + for (p, f) in px.as_chunks::<2>().0.iter().zip(&mut flat) { + *f = p[0] * p[1] + default * (1.0 - p[1]); + } + out.try_write_region(b, &flat)?; } out.prune(); - out + Ok(out) } /// Split a single-channel surface whose outside reads as its default (a mask, a channel) by a /// selection: (the selected values as grey + alpha = selection, the surface with the selected /// values replaced by the default). `None` for surfaces that aren't a single grey channel. -pub fn split_gray_selected(s: &Surface, sel: &Surface) -> Option<(Surface, Surface)> { +pub fn split_gray_selected(s: &Surface, sel: &Surface) -> Result> { let fmt = s.format(); if fmt.alpha || fmt.channels() != 1 { - return None; + return Ok(None); } let default = s.default_pixel().first().copied().unwrap_or(0.0); let mut rest = s.clone(); @@ -80,43 +90,49 @@ pub fn split_gray_selected(s: &Surface, sel: &Surface) -> Option<(Surface, Surfa // the moved region stays aligned with the moved pixels. let area = sel.content_bounds(); if area.is_empty() { - return Some((lifted, rest)); + return Ok(Some((lifted, rest))); } - let v = s.read_region(area); + let v = crate::allocation::read_region(s, area, "transforming a selected mask or channel")?; let w = area.width() as usize; - let mut lp = Vec::with_capacity(v.len() * 2); - let mut rp = Vec::with_capacity(v.len()); + let mut lp = crate::allocation::capacity( + v.len().checked_mul(2).ok_or_else(|| bad("the selected mask is too large to transform"))?, + "transforming a selected mask or channel", + )?; + let mut rp = crate::allocation::capacity(v.len(), "transforming a selected mask or channel")?; for (i, g) in v.iter().enumerate() { let (x, y) = (area.x0 + (i % w) as i32, area.y0 + (i / w) as i32); let k = sel.sample_channel(x, y, 0); lp.extend([*g, k]); rp.push(g * (1.0 - k) + default * k); } - lifted.write_region(area, &lp); + lifted.try_write_region(area, &lp)?; lifted.prune(); - rest.write_region(area, &rp); + rest.try_write_region(area, &rp)?; rest.prune(); - Some((lifted, rest)) + Ok(Some((lifted, rest))) } /// [`warp_gray`] limited to a selection: only the selected values move (the vacated area reads as /// the default), as the selected pixels of the layer do. -pub(crate) fn warp_gray_selected(s: &Surface, sel: &Surface, h: &Homography, interp: Interp) -> Surface { - let Some((lifted, mut out)) = split_gray_selected(s, sel) else { return warp_gray(s, h, interp) }; +pub(crate) fn warp_gray_selected(s: &Surface, sel: &Surface, h: &Homography, interp: Interp) -> Result { + let Some((lifted, mut out)) = split_gray_selected(s, sel)? else { return warp_gray(s, h, interp) }; let src = lifted.content_bounds(); if src.is_empty() { - return out; + return Ok(out); } - let w = warp_surface(&lifted, src, h, interp); + let w = warp_surface(&lifted, src, h, interp)?; let b = w.content_bounds(); if !b.is_empty() { - let moved = w.read_region(b); - let under = out.read_region(b); - let flat: Vec = moved.as_chunks::<2>().0.iter().zip(&under).map(|(p, u)| p[0] * p[1] + u * (1.0 - p[1])).collect(); - out.write_region(b, &flat); + let moved = crate::allocation::read_region(&w, b, "compositing transformed mask pixels")?; + let under = crate::allocation::read_region(&out, b, "compositing transformed mask pixels")?; + let mut flat = crate::allocation::filled(under.len(), 0.0, "compositing transformed mask pixels")?; + for (i, (p, u)) in moved.as_chunks::<2>().0.iter().zip(&under).enumerate() { + flat[i] = p[0] * p[1] + u * (1.0 - p[1]); + } + out.try_write_region(b, &flat)?; } out.prune(); - out + Ok(out) } /// The surface Free Transform moves by itself when the params target one (`"target"`): an @@ -167,6 +183,7 @@ pub(crate) fn group_locks(doc: &Document, id: LayerId) -> Locks { /// `group` holds the locks `l` inherits from the groups around it ([`group_locks`]). pub(crate) fn transform_layer(doc_sel: Option<&Surface>, group: Locks, l: &mut Layer, h: &Homography, affine: Option, interp: Interp) -> Result<()> { + crate::allocation::checkpoint("transforming pixels")?; // Photoshop turns the Background into a normal layer before transforming it. if l.locks.position && l.name == "Background" { l.locks.position = false; @@ -209,10 +226,10 @@ pub(crate) fn transform_layer(doc_sel: Option<&Surface>, group: Locks, l: &mut L // Fallback appearance for sources that can't be re-rendered. if let Some(c) = &mut sm.cache { let src = c.content_bounds(); - *c = warp_surface(c, src, h, interp); + *c = warp_surface(c, src, h, interp)?; } if let Some(m) = &mut sm.filter_mask { - m.surface = warp_gray(&m.surface, h, interp); + m.surface = warp_gray(&m.surface, h, interp)?; } } _ => { @@ -221,13 +238,13 @@ pub(crate) fn transform_layer(doc_sel: Option<&Surface>, group: Locks, l: &mut L *surf = match doc_sel { // Only the selected pixels move: lift them, clear them, warp and paste back. Some(sel) => { - let (lifted, mut rest) = split_selected(surf, sel); - let moved = warp_surface(&lifted, src, h, interp); + let (lifted, mut rest) = split_selected(surf, sel)?; + let moved = warp_surface(&lifted, src, h, interp)?; composite_over(&mut rest, &moved); rest.prune(); rest } - None => warp_surface(surf, src, h, interp), + None => warp_surface(surf, src, h, interp)?, }; } } @@ -236,8 +253,8 @@ pub(crate) fn transform_layer(doc_sel: Option<&Surface>, group: Locks, l: &mut L && m.linked { m.surface = match mask_sel { - Some(sel) => warp_gray_selected(&m.surface, sel, h, interp), - None => warp_gray(&m.surface, h, interp), + Some(sel) => warp_gray_selected(&m.surface, sel, h, interp)?, + None => warp_gray(&m.surface, h, interp)?, }; } if let Some(vm) = l.vector_mask.as_mut() @@ -250,18 +267,19 @@ pub(crate) fn transform_layer(doc_sel: Option<&Surface>, group: Locks, l: &mut L } /// Split a layer surface by a selection: (selected pixels, everything else), both with alpha. -pub fn split_selected(surf: &Surface, sel: &Surface) -> (Surface, Surface) { +pub fn split_selected(surf: &Surface, sel: &Surface) -> Result<(Surface, Surface)> { let fmt = surf.format(); let with_alpha = PixelFormat::new(fmt.mode, fmt.sample, true); let mut lifted = Surface::new(with_alpha); - let mut rest = surf.convert(with_alpha); + let mut rest = surf.try_convert(with_alpha)?; let src = surf.content_bounds(); if src.is_empty() { - return (lifted, rest); + return Ok((lifted, rest)); } let n = with_alpha.channels(); - let px = rest.read_region(src); - let mut lp = px.clone(); + let px = crate::allocation::read_region(&rest, src, "transforming selected pixels")?; + let mut lp = crate::allocation::filled(px.len(), 0.0, "transforming selected pixels")?; + lp.copy_from_slice(&px); let mut rp = px; let w = src.width() as usize; // A pixel the selection fully lifts out is reset to the surface's (transparent) default; @@ -280,10 +298,10 @@ pub fn split_selected(surf: &Surface, sel: &Surface) -> (Surface, Surface) { r.iter_mut().zip(&rest_clear).for_each(|(d, c)| *d = *c); } } - lifted.write_region(src, &lp); + lifted.try_write_region(src, &lp)?; lifted.prune(); - rest.write_region(src, &rp); - (lifted, rest) + rest.try_write_region(src, &rp)?; + Ok((lifted, rest)) } pub(crate) fn refresh_text(doc: &Document, l: &mut Layer) { @@ -390,11 +408,11 @@ fn transform(s: &mut Session, p: &Value) -> Result { // A targeted unlinked mask, alpha channel or Quick Mask transforms by itself. let (surf, _) = crate::channel_cmds::target_surface(doc, id, p)?; *surf = match &sel { - Some(sel) => warp_gray_selected(surf, sel, &h, interp), - None => warp_gray(surf, &h, interp), + Some(sel) => warp_gray_selected(surf, sel, &h, interp)?, + None => warp_gray(surf, &h, interp)?, }; if let Some(sel) = &doc.selection { - doc.selection = Some(warp_gray(sel, &h, Interp::Bilinear)).filter(|s| !s.content_bounds().is_empty()); + doc.selection = Some(warp_gray(sel, &h, Interp::Bilinear)?).filter(|s| !s.content_bounds().is_empty()); } return Ok(()); } @@ -410,7 +428,7 @@ fn transform(s: &mut Session, p: &Value) -> Result { } // The selection outline moves with the pixels. if let Some(sel) = &doc.selection { - doc.selection = Some(warp_gray(sel, &h, Interp::Bilinear)).filter(|s| !s.content_bounds().is_empty()); + doc.selection = Some(warp_gray(sel, &h, Interp::Bilinear)?).filter(|s| !s.content_bounds().is_empty()); } Ok(()) })?; @@ -690,6 +708,6 @@ mod tests { let s = Surface::new(PixelFormat::RGBA8); let mut sel = Surface::new(PixelFormat::GRAY8); sel.fill_rect(Rect::new(0, 0, 4, 4), &[1.0]); - assert!(split_gray_selected(&s, &sel).is_none()); + assert!(matches!(split_gray_selected(&s, &sel), Ok(None))); } } diff --git a/crates/engine/src/vp_cmds.rs b/crates/engine/src/vp_cmds.rs index 0c8cdbbec..4d386d60d 100644 --- a/crates/engine/src/vp_cmds.rs +++ b/crates/engine/src/vp_cmds.rs @@ -145,7 +145,7 @@ fn vanishing_point(s: &mut Session, p: &Value) -> Result { } let surf = l.surface_mut().ok_or_else(|| EngineError::Other("Vanishing Point needs a pixel layer".into()))?; for (plane, src, rect, at, width) in &pastes { - if let Some(out) = paste(&sc, *plane, src, *rect, *at, *width) { + if let Some(out) = paste(&sc, *plane, src, *rect, *at, *width)? { over(surf, &out); } } diff --git a/crates/engine/src/warp_cmds.rs b/crates/engine/src/warp_cmds.rs index bec3e1ecb..daecc6b9c 100644 --- a/crates/engine/src/warp_cmds.rs +++ b/crates/engine/src/warp_cmds.rs @@ -153,7 +153,7 @@ fn warp_layer(doc_sel: Option<&Surface>, group: Locks, l: &mut Layer, w: &Warp, if let Some(surf) = l.surface_mut() { *surf = match doc_sel { Some(sel) => { - let (lifted, mut rest) = crate::transform_cmds::split_selected(surf, sel); + let (lifted, mut rest) = crate::transform_cmds::split_selected(surf, sel)?; let moved = warp_mesh_surface(&lifted, rect, &map, interp); crate::transform_cmds::composite_over(&mut rest, &moved); rest.prune(); @@ -164,7 +164,7 @@ fn warp_layer(doc_sel: Option<&Surface>, group: Locks, l: &mut Layer, w: &Warp, let fmt = surf.format(); let warped = warp_mesh_surface(surf, rect, &map, interp); let mut rest = surf.convert(photocraft_color::PixelFormat::new(fmt.mode, fmt.sample, true)); - crate::pixels::clear_surface(&mut rest, rect, None); + crate::pixels::try_clear_surface(&mut rest, rect, None)?; crate::transform_cmds::composite_over(&mut rest, &warped); rest.prune(); rest diff --git a/crates/io/src/psd_export.rs b/crates/io/src/psd_export.rs index 58f297b7b..bc7cfdb00 100644 --- a/crates/io/src/psd_export.rs +++ b/crates/io/src/psd_export.rs @@ -715,7 +715,13 @@ impl Ex { } None => photocraft_algo::warp::place_source(&img, img_bounds, &sm.transform, sm.warp.as_ref()), }; - crate::smart_map::feid_item(&placed, &unfiltered, sm.filter_mask.as_ref(), bounds, self.fmt) + match unfiltered { + Ok(unfiltered) => crate::smart_map::feid_item(&placed, &unfiltered, sm.filter_mask.as_ref(), bounds, self.fmt), + Err(e) => { + warnings.push(format!("smart filter mask was not re-rendered for PSD export ({e})")); + None + } + } } _ => None, }; diff --git a/crates/raster/src/lib.rs b/crates/raster/src/lib.rs index bf52e0668..64c747686 100644 --- a/crates/raster/src/lib.rs +++ b/crates/raster/src/lib.rs @@ -11,11 +11,30 @@ mod interrupt; pub use interrupt::{Cancelled, Interrupt}; use std::collections::BTreeMap; +use std::fmt; use std::sync::Arc; use photocraft_color::{ColorMode, PixelFormat, SampleType, read_sample, write_sample}; use photocraft_geom::{Rect, TILE_SIZE, TileCoord}; +/// A checked surface allocation could not be represented or reserved. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum AllocationError { + SizeOverflow, + NotEnoughMemory { bytes: usize }, +} + +impl fmt::Display for AllocationError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::SizeOverflow => f.write_str("requested surface allocation is too large"), + Self::NotEnoughMemory { bytes } => write!(f, "not enough memory to allocate {} bytes", bytes), + } + } +} + +impl std::error::Error for AllocationError {} + /// Pixel storage for one tile: `TILE_SIZE² × bytes_per_pixel`, row-major, interleaved channels. #[derive(Clone, Debug, PartialEq, Eq)] pub struct Tile { @@ -23,6 +42,29 @@ pub struct Tile { } 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); + if pixel.iter().any(|&b| b != 0) { + for px in data.chunks_exact_mut(pixel.len()) { + px.copy_from_slice(pixel); + } + } + Ok(Tile { data: data.into_boxed_slice() }) + } + + /// 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); + Ok(Tile { data: data.into_boxed_slice() }) + } + fn filled(format: &PixelFormat, pixel: &[u8]) -> Self { let n = (TILE_SIZE * TILE_SIZE) as usize; let mut data = vec![0u8; n * format.bytes_per_pixel()]; @@ -55,6 +97,47 @@ impl PartialEq for Surface { } impl Surface { + /// Reads a region into a newly allocated buffer, returning an error instead of aborting if + /// the requested pixel buffer cannot be represented or reserved. + 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); + self.read_region_into(r, &mut out); + Ok(out) + } + + /// Reads a region as encoded interleaved bytes with fallible allocation. + pub fn try_to_interleaved(&self, r: Rect) -> Result, AllocationError> { + let bpp = self.format.bytes_per_pixel(); + let len = (r.width() as usize).checked_mul(r.height() as usize).and_then(|n| n.checked_mul(bpp)).ok_or(AllocationError::SizeOverflow)?; + let mut out = Vec::new(); + out.try_reserve_exact(len).map_err(|_| AllocationError::NotEnoughMemory { bytes: len })?; + out.resize(len, 0); + let w = r.width() as usize; + for tc in r.tiles() { + let tr = tc.rect().intersect(&r); + let span = tr.width() as usize * bpp; + for y in tr.y0..tr.y1 { + let dst = (((y - r.y0) as usize) * w + (tr.x0 - r.x0) as usize) * bpp; + match self.tiles.get(&tc) { + Some(t) => { + let src = (((y - tc.ty * TILE_SIZE) as usize) * TILE_SIZE as usize + (tr.x0 - tc.tx * TILE_SIZE) as usize) * bpp; + out[dst..dst + span].copy_from_slice(&t.data[src..src + span]); + } + None => { + for px in out[dst..dst + span].chunks_exact_mut(bpp) { + px.copy_from_slice(&self.default_pixel); + } + } + } + } + } + Ok(out) + } + /// Transparent/zero surface. pub fn new(format: PixelFormat) -> Self { Self { format, default_pixel: vec![0u8; format.bytes_per_pixel()].into_boxed_slice(), tiles: BTreeMap::new() } @@ -94,6 +177,44 @@ impl Surface { Arc::make_mut(arc) } + /// Fallible counterpart to [`Surface::tile_mut`]. Pixel storage is reserved before changing + /// the surface, and a shared tile is copied only after its replacement has been allocated. + pub fn try_tile_mut(&mut self, c: TileCoord) -> Result<&mut Tile, AllocationError> { + if !self.tiles.contains_key(&c) { + let tile = Arc::new(Tile::try_filled(&self.format, &self.default_pixel)?); + self.tiles.insert(c, tile); + } else if self.tiles.get(&c).is_some_and(|t| Arc::strong_count(t) > 1) { + let original = self.tiles.get(&c).ok_or(AllocationError::SizeOverflow)?; + let tile = Arc::new(original.try_clone()?); + self.tiles.insert(c, tile); + } + self.tiles.get_mut(&c).and_then(Arc::get_mut).ok_or(AllocationError::SizeOverflow) + } + + /// Writes a region using fallible pixel-tile allocations. Callers that need transactional + /// document updates should perform this on their staged document copy and commit on success. + pub fn try_write_region(&mut self, r: Rect, data: &[f32]) -> Result<(), AllocationError> { + let n = self.channels(); + let sample = self.format.sample; + let expected = (r.width() as usize).checked_mul(r.height() as usize).and_then(|v| v.checked_mul(n)).ok_or(AllocationError::SizeOverflow)?; + if data.len() != expected { + return Err(AllocationError::SizeOverflow); + } + for tc in r.tiles() { + let tr = tc.rect().intersect(&r); + let t = self.try_tile_mut(tc)?; + let span = tr.width() as usize * n; + for y in tr.y0..tr.y1 { + let src = (((y - r.y0) as usize) * r.width() as usize + (tr.x0 - r.x0) as usize) * n; + let dst = (((y - tc.ty * TILE_SIZE) as usize) * TILE_SIZE as usize + (tr.x0 - tc.tx * TILE_SIZE) as usize) * n; + for (i, v) in data[src..src + span].iter().enumerate() { + write_sample(&mut t.data, sample, dst + i, *v); + } + } + } + Ok(()) + } + /// Coarse bounds: union of allocated tile rects. pub fn tile_bounds(&self) -> Rect { self.tiles.keys().fold(Rect::EMPTY, |r, c| r.union(&c.rect())) @@ -429,6 +550,24 @@ impl Surface { .collect() } + /// Fallible counterpart to [`Surface::take_tiles`]. Reserves the complete result and any + /// default tile before removing a tile, so a reservation failure leaves the surface intact. + pub fn try_take_tiles(&mut self, r: Rect) -> Result)>, AllocationError> { + let count = r.tiles().count(); + let bytes = count.checked_mul(std::mem::size_of::<(TileCoord, Arc)>()).ok_or(AllocationError::SizeOverflow)?; + let mut out = Vec::new(); + out.try_reserve_exact(count).map_err(|_| AllocationError::NotEnoughMemory { bytes })?; + let has_missing = r.tiles().any(|c| !self.tiles.contains_key(&c)); + let blank = if has_missing { Some(Arc::new(Tile::try_filled(&self.format, &self.default_pixel)?)) } else { None }; + for c in r.tiles() { + let tile = self.tiles.remove(&c).or_else(|| blank.clone()); + if let Some(tile) = tile { + out.push((c, tile)); + } + } + Ok(out) + } + /// Put tiles back (see [`Surface::take_tiles`]); each replaces the tile at its coordinate. pub fn put_tiles(&mut self, tiles: impl IntoIterator)>) { for (c, t) in tiles { @@ -590,6 +729,29 @@ impl Surface { out } + /// Convert a surface while reserving each replacement tile fallibly. + pub fn try_convert(&self, to: PixelFormat) -> Result { + let mut out = Surface::with_default(to, &convert_pixel(&self.format, &to, &self.default_pixel())); + let from_n = self.channels(); + let mut src = Vec::new(); + src.try_reserve_exact(from_n).map_err(|_| AllocationError::NotEnoughMemory { bytes: from_n * std::mem::size_of::() })?; + src.resize(from_n, 0.0f32); + for (c, t) in &self.tiles { + let dst = out.try_tile_mut(*c)?; + let px_count = (TILE_SIZE * TILE_SIZE) as usize; + for i in 0..px_count { + for (k, v) in src.iter_mut().enumerate() { + *v = read_sample(&t.data, self.format.sample, i * from_n + k); + } + let px = convert_pixel(&self.format, &to, &src); + for (k, v) in px.iter().enumerate() { + write_sample(&mut dst.data, to.sample, i * to.channels() + k, *v); + } + } + } + Ok(out) + } + /// Composite-friendly RGBA read (converts from the surface's model). pub fn rgba(&self, x: i32, y: i32) -> [f32; 4] { let p = self.pixel(x, y); @@ -747,6 +909,22 @@ mod tests { } } + #[test] + fn fallible_region_reads_report_size_overflow() { + let surface = Surface::new(PixelFormat::RGBA8); + let huge = Rect::new(i32::MIN, i32::MIN, i32::MAX, i32::MAX); + assert_eq!(surface.try_read_region(huge), Err(AllocationError::SizeOverflow)); + assert_eq!(surface.try_to_interleaved(huge), Err(AllocationError::SizeOverflow)); + } + + #[test] + fn fallible_region_write_rejects_bad_input_without_mutating_surface() { + let mut surface = Surface::new(PixelFormat::RGBA8); + let before = surface.clone(); + assert_eq!(surface.try_write_region(Rect::new(0, 0, 2, 2), &[1.0]), Err(AllocationError::SizeOverflow)); + assert_eq!(surface, before); + } + #[test] fn fill_rect_shares_covered_tiles_and_edits_stay_local() { // A 300000² background used to materialize ~360 GB of tiles (#705). diff --git a/crates/ui-egui/src/transform_tool.rs b/crates/ui-egui/src/transform_tool.rs index 4af072005..786369216 100644 --- a/crates/ui-egui/src/transform_tool.rs +++ b/crates/ui-egui/src/transform_tool.rs @@ -173,7 +173,7 @@ pub fn begin(app: &mut PhotocraftApp, ctx: &egui::Context) -> Result<(), String> let mut lifted = None; if let Some(sel) = doc.selection.clone().filter(|_| !layer.is_group() && layer.surface().is_some()) { if let Some(surf) = pd.layer_mut(id).and_then(|l| l.surface_mut()) { - let (l, rest) = photocraft_engine::transform_cmds::split_selected(surf, &sel); + let (l, rest) = photocraft_engine::transform_cmds::split_selected(surf, &sel).map_err(|e| e.to_string())?; *surf = rest; lifted = Some(l); } @@ -308,7 +308,7 @@ fn begin_lone( &whole } }; - let (lifted, rest) = tc::split_gray_selected(&surf, sel).ok_or("this channel can't be transformed")?; + let (lifted, rest) = tc::split_gray_selected(&surf, sel).map_err(|e| e.to_string())?.ok_or("this channel can't be transformed")?; crate::type_tool::commit(app); let layer = app.session.active().and_then(|st| st.active_layer).map_or(0, |l| l.0); let mut pd = (*doc).clone(); @@ -3070,7 +3070,7 @@ mod tests { let st = app.session.active().unwrap(); let sel = st.doc.selection.clone().unwrap(); let id = st.active_layer.unwrap(); - let (lifted, _) = photocraft_engine::transform_cmds::split_selected(st.doc.layer(id).unwrap().surface().unwrap(), &sel); + let (lifted, _) = photocraft_engine::transform_cmds::split_selected(st.doc.layer(id).unwrap().surface().unwrap(), &sel).unwrap(); let b = photocraft_geom::Rect::new(8, 8, 20, 24); let (img, _) = preview_image(&st.doc, id, Some(&lifted), b, 4096); assert_eq!(texel_alpha(&img, 2, 4), 0, "masked"); diff --git a/docs/scorecard.md b/docs/scorecard.md index 31c1d20c3..4861aa629 100644 --- a/docs/scorecard.md +++ b/docs/scorecard.md @@ -12,7 +12,7 @@ Generated by `cargo xtask scorecard` from `perf/budgets.toml`, `perf/baseline.js | [Workspace and UI](#workspace-and-ui) | 5 | 7 | 11 | settings that do nothing: 46 of 149 (target 0) | | [Type](#type) | 0 | 3 | 6 | | | [Automation](#automation) | 3 | 2 | 6 | 627 of 627 menu items live (100.0%) | -| [Reliability](#reliability) | 5 | 0 | 2 | never-crash lints on 28 of 28 crates; panic_hunt present | +| [Reliability](#reliability) | 5 | 2 | 0 | never-crash lints on 28 of 28 crates; panic_hunt present | | [Distribution](#distribution) | 4 | 4 | 3 | | | [Open bugs](#open-bugs) | 22 | 2 | 8 | | @@ -246,9 +246,9 @@ Never-crash attribute on 28 of 28 crate roots. | REL-2 | Every workspace crate carries the never-crash deny(clippy::unwrap_used, ...) attribute | done | | Counted automatically above (crate roots under crates/ and apps/); testkit gained it in #221 | | REL-3 | The app catches escaped panics around command dispatch and import/export | done | | a_panic_in_a_command_... runs the test-only `test.panic` spec through Session::execute and asserts Err plus an unchanged document (engine jobs_tests.rs); the worker guard is covered by a_panic_in_the_worker_...; crash_guard.rs guard_turns_a_panic_into_an_error covers import/export (apps/photocraft) | | REL-4 | Atomic save (temp, fsync, rename) for every document write | done | [#203](https://github.com/storytold/photocraft/issues/203) | photocraft_format::atomic_write (format/src/atomic.rs, #230): same-dir temp, sync_all, rename with Windows PermissionDenied retry, Unix dir fsync; every document write site routes through it and format/src/atomic/tests.rs failure-injects write, rename and read-only-dir errors | -| REL-212-1 | Fallible allocation for paste/erase/fill/transform/save/import, clear out-of-memory error | missing | [#212](https://github.com/storytold/photocraft/issues/212) | try_reserve only in io/flat.rs:221; codec max_alloc limits cover decode only | +| REL-212-1 | Fallible allocation for paste/erase/fill/transform/save/import, clear out-of-memory error | partial | [#212](https://github.com/storytold/photocraft/issues/212) | Raster region/tile buffers, paste/erase/fill/transform and warp output use fallible reservations; save/import boundaries preserve existing state, but encoder/decoder internals still need a broader fallible-allocation audit | | REL-212-2 | Export switches to PSB when the estimated file size exceeds 2 GB | done | [#212](https://github.com/storytold/photocraft/issues/212) | PSD export uses checked size estimates for raster, masks, channels, and retained metadata | -| REL-212-3 | Tests: an allocation-limit injection makes each command Err with the document unchanged | missing | [#212](https://github.com/storytold/photocraft/issues/212) | Only codec/raw max_alloc tests (codecs/tests/limits.rs:48) | +| REL-212-3 | Tests: an allocation-limit injection makes each command Err with the document unchanged | partial | [#212](https://github.com/storytold/photocraft/issues/212) | Engine failpoint tests cover paste, erase, fill, transform, import and save transaction boundaries; codec-level allocator failure coverage remains open | ## Distribution diff --git a/scorecard/reliability.toml b/scorecard/reliability.toml index a777e6237..081af4461 100644 --- a/scorecard/reliability.toml +++ b/scorecard/reliability.toml @@ -33,9 +33,9 @@ note = "photocraft_format::atomic_write (format/src/atomic.rs, #230): same-dir t [[item]] id = "REL-212-1" target = "Fallible allocation for paste/erase/fill/transform/save/import, clear out-of-memory error" -status = "missing" +status = "partial" issue = 212 -note = "try_reserve only in io/flat.rs:221; codec max_alloc limits cover decode only" +note = "Raster region/tile buffers, paste/erase/fill/transform and warp output use fallible reservations; save/import boundaries preserve existing state, but encoder/decoder internals still need a broader fallible-allocation audit" [[item]] id = "REL-212-2" @@ -47,6 +47,6 @@ note = "PSD export uses checked size estimates for raster, masks, channels, and [[item]] id = "REL-212-3" target = "Tests: an allocation-limit injection makes each command Err with the document unchanged" -status = "missing" +status = "partial" issue = 212 -note = "Only codec/raw max_alloc tests (codecs/tests/limits.rs:48)" +note = "Engine failpoint tests cover paste, erase, fill, transform, import and save transaction boundaries; codec-level allocator failure coverage remains open"