From 6ca5bf359de5e3b37ad1799e7fb6e49c61c8229c Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 23 Oct 2026 15:33:01 -0400 Subject: [PATCH] security(compose): bound dense working sets and nested Blend If evaluation (#8949) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * security(compose): bound dense working sets and repeated group evaluation Smart-filter mixing and stroke fitting work tile by tile instead of over the union of sparse surfaces; render_layer admits its rect like render; hidden layers no longer disable the proxy and uniform-area shortcuts; Blend If groups nested deeper than two are judged by their result, so nesting costs polynomial instead of exponential work; effect builds past an absolute byte cap are skipped. * CodeRabbit #8949: exact tile skips in filter_mask, allocated-tile scan in fit_stroke, contrasting backdrop in the nested-group test - filter_mask mix/unmix: bare tiles (no tiles on either side) skip for non-alpha formats too when the two surfaces' default pixels are equal; differing defaults still get processed, so the clone's default pixel is never left wrong under a non-unit mask - shape_split fit_stroke: scan the union of the three surfaces' allocated tiles (exact skip when all three defaults are transparent) instead of every tile of a 300k-canvas document; non-transparent defaults fall back to the full scan - tests: the nested-groups assertion now uses a black backdrop so the asserted white can only come from rendering through the group chain fixes CodeRabbit review comments (major/minor) on filter_mask.rs:27,29, shape_split.rs:126, tests.rs:2354 * compose: skip a BTreeSet in fit_stroke — a sorted Vec compiles under the wasm toolchain (E0599 on ::drain there) --- crates/compose/src/filter_mask.rs | 127 +++++++++++++++++++----------- crates/compose/src/lib.rs | 55 ++++++++++++- crates/compose/src/proxy.rs | 22 +++++- crates/compose/src/shape_split.rs | 33 +++++++- crates/compose/src/tests.rs | 34 ++++++++ crates/compose/src/uniform.rs | 3 +- 6 files changed, 222 insertions(+), 52 deletions(-) diff --git a/crates/compose/src/filter_mask.rs b/crates/compose/src/filter_mask.rs index b845720456..2d9031f5d4 100644 --- a/crates/compose/src/filter_mask.rs +++ b/crates/compose/src/filter_mask.rs @@ -21,27 +21,38 @@ pub fn mix(base: &Surface, filtered: &Surface, mask: &LayerMask) -> Surface { return out; } let n = fmt.channels(); - let (b, f) = (base.read_region(area), filtered.read_region(area)); - let mut k = Vec::new(); - mask.values_into(area, &mut k); - let mut o = vec![0.0f32; b.len()]; - for (i, ((bp, fp), op)) in b.chunks_exact(n).zip(f.chunks_exact(n)).zip(o.chunks_exact_mut(n)).enumerate() { - let m = k.get(i).copied().unwrap_or(1.0).clamp(0.0, 1.0); - if !fmt.alpha { - for c in 0..n { - op[c] = bp[c] + (fp[c] - bp[c]) * m; - } + // Tile by tile: the union of two sparse surfaces far apart is a rectangle of mostly nothing, + // and transparent pixels in tiles neither surface has stay as they are. + let whole = area; + let same_default = base.default_pixel() == filtered.default_pixel(); + for t in whole.tiles() { + let area = t.rect().intersect(&whole); + let bare = !base.has_tiles_in(area) && !filtered.has_tiles_in(area); + if area.is_empty() || (bare && (fmt.alpha || same_default)) { continue; } - let a = n - 1; - let oa = bp[a] + (fp[a] - bp[a]) * m; - op[a] = oa; - for c in 0..a { - let pm = bp[c] * bp[a] + (fp[c] * fp[a] - bp[c] * bp[a]) * m; - op[c] = if oa > 0.0 { (pm / oa).clamp(0.0, 1.0) } else { 0.0 }; + let (b, f) = (base.read_region(area), filtered.read_region(area)); + let mut k = Vec::new(); + mask.values_into(area, &mut k); + let mut o = vec![0.0f32; b.len()]; + for (i, ((bp, fp), op)) in b.chunks_exact(n).zip(f.chunks_exact(n)).zip(o.chunks_exact_mut(n)).enumerate() { + let m = k.get(i).copied().unwrap_or(1.0).clamp(0.0, 1.0); + if !fmt.alpha { + for c in 0..n { + op[c] = bp[c] + (fp[c] - bp[c]) * m; + } + continue; + } + let a = n - 1; + let oa = bp[a] + (fp[a] - bp[a]) * m; + op[a] = oa; + for c in 0..a { + let pm = bp[c] * bp[a] + (fp[c] * fp[a] - bp[c] * bp[a]) * m; + op[c] = if oa > 0.0 { (pm / oa).clamp(0.0, 1.0) } else { 0.0 }; + } } + out.write_region(area, &o); } - out.write_region(area, &o); out.prune(); out } @@ -61,38 +72,51 @@ pub fn unmix(base: &Surface, mixed: &Surface, mask: &LayerMask) -> Surface { return out; } let n = fmt.channels(); - let (b, x) = (base.read_region(area), mixed.read_region(area)); - let mut k = Vec::new(); - mask.values_into(area, &mut k); - let mut o = vec![0.0f32; b.len()]; - for (i, ((bp, xp), op)) in b.chunks_exact(n).zip(x.chunks_exact(n)).zip(o.chunks_exact_mut(n)).enumerate() { - let m = k.get(i).copied().unwrap_or(1.0).clamp(0.0, 1.0); - if m < HIDDEN { - // A transparent base pixel (Photoshop's cache keeps white there) stays empty, so the - // stored pixels don't grow past the content. - if !fmt.alpha || bp[n - 1] > 0.0 { - op.copy_from_slice(bp); + // Tile by tile: the union of two sparse surfaces far apart is a rectangle of mostly nothing, + // and transparent pixels in tiles neither surface has stay as they are. + let whole = area; + // Non-alpha still needs every tile whose defaults differ (an unmix through a non-unit mask + // would change that default, and the clone already holds the old one). + let same_default = base.default_pixel() == mixed.default_pixel(); + for t in whole.tiles() { + let area = t.rect().intersect(&whole); + let bare = !base.has_tiles_in(area) && !mixed.has_tiles_in(area); + if area.is_empty() || (bare && (fmt.alpha || same_default)) { + continue; + } + let (b, x) = (base.read_region(area), mixed.read_region(area)); + let mut k = Vec::new(); + mask.values_into(area, &mut k); + let mut o = vec![0.0f32; b.len()]; + for (i, ((bp, xp), op)) in b.chunks_exact(n).zip(x.chunks_exact(n)).zip(o.chunks_exact_mut(n)).enumerate() { + let m = k.get(i).copied().unwrap_or(1.0).clamp(0.0, 1.0); + if m < HIDDEN { + // A transparent base pixel (Photoshop's cache keeps white there) stays empty, so the + // stored pixels don't grow past the content. + if !fmt.alpha || bp[n - 1] > 0.0 { + op.copy_from_slice(bp); + } + continue; } - continue; - } - if !fmt.alpha { - for c in 0..n { - op[c] = (bp[c] + (xp[c] - bp[c]) / m).clamp(0.0, 1.0); + if !fmt.alpha { + for c in 0..n { + op[c] = (bp[c] + (xp[c] - bp[c]) / m).clamp(0.0, 1.0); + } + continue; + } + let a = n - 1; + let fa = (bp[a] + (xp[a] - bp[a]) / m).clamp(0.0, 1.0); + if fa <= 0.0 { + continue; + } + op[a] = fa; + for c in 0..a { + let pm = bp[c] * bp[a] + (xp[c] * xp[a] - bp[c] * bp[a]) / m; + op[c] = (pm / fa).clamp(0.0, 1.0); } - continue; - } - let a = n - 1; - let fa = (bp[a] + (xp[a] - bp[a]) / m).clamp(0.0, 1.0); - if fa <= 0.0 { - continue; - } - op[a] = fa; - for c in 0..a { - let pm = bp[c] * bp[a] + (xp[c] * xp[a] - bp[c] * bp[a]) / m; - op[c] = (pm / fa).clamp(0.0, 1.0); } + out.write_region(area, &o); } - out.write_region(area, &o); out.prune(); out } @@ -122,6 +146,19 @@ mod tests { m } + #[test] + fn mixing_sparse_surfaces_far_apart_does_not_build_their_union() { + // 10¹⁰ px between two single pixels would be 160 GB of floats read densely. + let fmt = PixelFormat::new(ColorMode::Rgb, SampleType::F32, true); + let (mut base, mut filtered) = (Surface::new(fmt), Surface::new(fmt)); + base.write_pixel(0, 0, &[0.2, 0.4, 0.6, 1.0]); + filtered.write_pixel(100_000, 100_000, &[1.0, 0.0, 0.0, 1.0]); + let mixed = mix(&base, &filtered, &LayerMask::reveal_all()); + assert_eq!(mixed.pixel(100_000, 100_000), vec![1.0, 0.0, 0.0, 1.0]); + let back = unmix(&base, &mixed, &LayerMask::reveal_all()); + assert_eq!(back.pixel(100_000, 100_000), vec![1.0, 0.0, 0.0, 1.0]); + } + #[test] fn unmix_inverts_mix_wherever_the_mask_shows_the_filter() { for fmt in [PixelFormat::new(ColorMode::Rgb, SampleType::F32, true), PixelFormat::new(ColorMode::Rgb, SampleType::F32, false)] { diff --git a/crates/compose/src/lib.rs b/crates/compose/src/lib.rs index e74cfd6d2f..6cb6707e0b 100644 --- a/crates/compose/src/lib.rs +++ b/crates/compose/src/lib.rs @@ -150,6 +150,22 @@ fn scratch_reuse_preserves_pixels_and_bounds_retention() { }); } +/// Every layer the compositor draws, groups before their children: hidden layers and everything +/// inside hidden groups are left out. +pub(crate) fn visible_layers(layers: &[Layer]) -> Vec<&Layer> { + fn rec<'a>(layers: &'a [Layer], out: &mut Vec<&'a Layer>) { + for l in layers.iter().filter(|l| l.visible) { + out.push(l); + if let Some(children) = l.children() { + rec(children, out); + } + } + } + let mut out = Vec::new(); + rec(layers, &mut out); + out +} + /// Composite the whole document over `rect`, in parallel 256² tiles on /// native targets (single-threaded on wasm). pub fn render(doc: &Document, rect: Rect) -> Buffer { @@ -383,7 +399,11 @@ pub fn flatten(doc: &Document) -> Buffer { } /// Render an arbitrary subset: a single layer (e.g. for thumbnails), isolated. +/// Rects past [`MAX_RENDER_PIXELS`] give an empty buffer. pub fn render_layer(layer: &Layer, rect: Rect) -> Buffer { + if u64::from(rect.width()) * u64::from(rect.height()) > MAX_RENDER_PIXELS { + return Buffer::transparent(Rect::EMPTY); + } let mut buf = Buffer::transparent(rect); let patterns = pattern::PreparedPatterns::new(&[], pattern::PREPARED_PATTERN_BYTES); composite_stack_at( @@ -1720,14 +1740,34 @@ fn blend_if_level(x: f32) -> f32 { if (v - r).abs() < 1e-4 { r } else { v } } +/// Blend If groups inside one another whose own colour is still rendered separately. Each extra +/// level multiplies the work by the tree's depth, so the cost is polynomial, not exponential. +const MAX_BLEND_IF_GROUP_DEPTH: u32 = 2; + +thread_local! { + /// Blend If groups being rendered for their own colour on this thread. + static BLEND_IF_GROUPS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// Apply `layer`'s Blend If to a finished composite: `out` (the backdrop with the layer drawn) /// is mixed back towards `before` (the backdrop without it) where the ranges hide the layer. /// Mixing premultiplied colour by `k` equals compositing the layer at `k` × its alpha, since /// every blend mode's source-over result is linear in the source alpha. fn apply_blend_if(layer: &Layer, before: &Buffer, out: &mut Buffer, cx: &Ctx) { // "This Layer" is the layer's own colour; adjustment layers (no content of their own) are - // judged by their result. - let own = if matches!(layer.content, LayerContent::Adjustment(_)) { None } else { render_content(layer, out.rect, cx) }; + // judged by their result. So are groups nested deeper than `MAX_BLEND_IF_GROUP_DEPTH` Blend + // Ifs: a group's own colour re-composites its children, which doubles the work at every + // level of Blend If groups inside one another. + let is_group = matches!(layer.content, LayerContent::Group(_)); + let nested = BLEND_IF_GROUPS.with(std::cell::Cell::get); + let own = if matches!(layer.content, LayerContent::Adjustment(_)) || (is_group && nested >= MAX_BLEND_IF_GROUP_DEPTH) { + None + } else { + BLEND_IF_GROUPS.with(|d| d.set(nested + u32::from(is_group))); + let own = render_content(layer, out.rect, cx); + BLEND_IF_GROUPS.with(|d| d.set(nested)); + own + }; for (i, (p, b)) in out.px.iter_mut().zip(&before.px).enumerate() { let this = match &own { Some(o) => Some(o.px[i]).filter(|q| q[3] > 0.0), @@ -2226,12 +2266,21 @@ fn fx_region_bytes(layers: &[Layer], canvas: Rect) -> u64 { bytes(layers, canvas, 0) } +/// Most bytes one layer's effect maps and source may take to build, whatever the cache budget. +const MAX_FX_BUILD_BYTES: u64 = if cfg!(target_pointer_width = "64") { 4 << 30 } else { 512 << 20 }; + /// The layer's effect maps over its region (layer bounds grown by the effect reach, within the /// canvas grown likewise; see [`effect_region`]), built once per layer state and shared by every /// tile. `rect` is what the caller composites the layer over (see [`effect_region`]). fn effect_maps(layer: &Layer, rect: Rect, cx: &Ctx) -> std::sync::Arc { use std::hash::{Hash, Hasher}; - let (region, clipped) = effect_region(layer, rect, cx); + let (mut region, clipped) = effect_region(layer, rect, cx); + // Non-local effects keep their whole source region, which a few distant pixels stretch over + // the canvas: past the cap the effects are left out instead of allocating it. + let region_px = u64::from(region.width()) * u64::from(region.height()); + if (effects::map_count(layer) as u64 + 1).saturating_mul(region_px).saturating_mul(4) > MAX_FX_BUILD_BYTES { + region = Rect::EMPTY; + } static NO_CACHE: std::sync::OnceLock = std::sync::OnceLock::new(); if *NO_CACHE.get_or_init(|| std::env::var_os("PHOTOCRAFT_FX_NOCACHE").is_some()) { let shape = if region.is_empty() { Vec::new() } else { effect_shape(layer, region, cx) }; diff --git a/crates/compose/src/proxy.rs b/crates/compose/src/proxy.rs index f48f10d83f..dba123e21c 100644 --- a/crates/compose/src/proxy.rs +++ b/crates/compose/src/proxy.rs @@ -112,7 +112,8 @@ pub fn proxy_document(doc: &Document, k: u32) -> Document { /// with it. Empty vector masks have constant coverage, including inversion and density. /// Nonempty paths can still anchor fill layouts even when their mask density is zero. pub fn proxy_faithful(doc: &Document) -> bool { - doc.walk().iter().all(|(_, _, l)| !crate::effects::has_effects(l) && l.vector_mask.as_ref().is_none_or(|v| !v.enabled || v.path.is_empty())) + // Hidden layers (and what they hold) are not composited, so they don't count. + crate::visible_layers(&doc.layers).iter().all(|l| !crate::effects::has_effects(l) && l.vector_mask.as_ref().is_none_or(|v| !v.enabled || v.path.is_empty())) } #[cfg(test)] @@ -263,6 +264,25 @@ mod tests { assert!(proxy_faithful(&doc)); } + #[test] + fn a_hidden_layer_with_effects_does_not_rule_out_the_proxy_or_the_uniform_shortcut() { + let mut doc = two_colors(Size::new(32, 16), SampleType::U8); + doc.layers[1].effects.items.push(Effect::default_drop_shadow()); + doc.layers[1].effects.enabled = true; + assert!(!proxy_faithful(&doc)); + assert!(crate::uniform::UniformAreas::new(&doc).is_none()); + doc.layers[1].visible = false; + assert!(proxy_faithful(&doc)); + assert!(crate::uniform::UniformAreas::new(&doc).is_some()); + // Inside a hidden group too. + doc.layers[1].visible = true; + let group = Layer::group("g", vec![doc.layers.remove(1)]); + doc.layers.push(group); + assert!(!proxy_faithful(&doc)); + doc.layers[1].visible = false; + assert!(proxy_faithful(&doc)); + } + #[test] fn empty_vector_masks_keep_constant_coverage_at_every_depth() { for depth in [SampleType::U8, SampleType::U16, SampleType::F32] { diff --git a/crates/compose/src/shape_split.rs b/crates/compose/src/shape_split.rs index 09d404669b..b2fd28116e 100644 --- a/crates/compose/src/shape_split.rs +++ b/crates/compose/src/shape_split.rs @@ -8,7 +8,7 @@ use std::sync::{Mutex, OnceLock}; use photocraft_color::{PixelFormat, SampleType}; use photocraft_doc::vector::ShapeLayer; -use photocraft_geom::Rect; +use photocraft_geom::{Rect, TileCoord}; use photocraft_raster::Surface; /// (fill, stroke), last use, and the shape's pixels the stroke was fitted to (kept alive so @@ -118,13 +118,42 @@ fn fit_stroke(fill: &Surface, stroke: &mut Surface, cache: &Surface, canvas: Rec if r.is_empty() { return; } + let float = stroke.format().sample == SampleType::F32; + // Tiles none of the three reaches read as the defaults; with transparent defaults the fit + // leaves the stroke's default pixel there, so those tiles are exact skips and we can scan + // just the union of the three surfaces' allocated tiles instead of every canvas tile (two + // sparse regions on a 300k×300k document would otherwise walk ~1.4M empty tiles). A + // non-transparent default is everywhere, so the scan falls back to the full rectangle. + let whole = r; + let bare_transparent = [fill, &*stroke, cache].iter().all(|s| s.default_pixel().iter().all(|v| *v == 0.0)); + if bare_transparent { + let mut coords: Vec = fill.tiles().map(|(c, _)| *c).chain(stroke.tiles().map(|(c, _)| *c)).chain(cache.tiles().map(|(c, _)| *c)).collect(); + coords.sort_unstable(); + coords.dedup(); + for c in coords { + let r = c.rect().intersect(&whole); + if !r.is_empty() { + fit_stroke_rect(fill, stroke, cache, r, float); + } + } + } else { + for t in whole.tiles() { + let r = t.rect().intersect(&whole); + if !r.is_empty() { + fit_stroke_rect(fill, stroke, cache, r, float); + } + } + } +} + +/// [`fit_stroke`] over one rectangle. +fn fit_stroke_rect(fill: &Surface, stroke: &mut Surface, cache: &Surface, r: Rect, float: bool) { let n = r.width() as usize * r.height() as usize; let (mut f, mut s, mut c) = (vec![[0.0f32; 4]; n], vec![[0.0f32; 4]; n], vec![[0.0f32; 4]; n]); fill.read_rgba_into(r, &mut f); stroke.read_rgba_into(r, &mut s); cache.read_rgba_into(r, &mut c); let mut out = Vec::with_capacity(n * 4); - let float = stroke.format().sample == SampleType::F32; for ((fp, sp), cp) in f.iter().zip(&s).zip(&c) { let mut p = *sp; if fp[3] < 1.0 - 1e-3 { diff --git a/crates/compose/src/tests.rs b/crates/compose/src/tests.rs index 8f111269a8..11da5a62f9 100644 --- a/crates/compose/src/tests.rs +++ b/crates/compose/src/tests.rs @@ -1737,6 +1737,20 @@ fn vector_mask_combines_with_pixel_mask() { assert!(close4(px(&d, 6, 1), [1.0; 4])); } +#[test] +fn effect_maps_over_distant_pixels_are_not_built_over_the_canvas() { + // Two pixels 190000 apart: the effect source region spans 3.6·10¹⁰ px. + let mut doc = Document::new("big", Size::new(200_000, 200_000), ColorMode::Rgb, SampleType::U8); + let mut l = solid_layer("fx", Rect::new(0, 0, 1, 1), [1.0, 0.0, 0.0, 1.0]); + l.surface_mut().unwrap().fill_rect(Rect::new(190_000, 190_000, 190_001, 190_001), &[1.0, 0.0, 0.0, 1.0]); + l.effects.items.push(photocraft_doc::Effect::default_drop_shadow()); + doc.layers.push(l); + let t = std::time::Instant::now(); + let px = render(&doc, Rect::new(0, 0, 4, 4)); + assert_eq!(px.px.len(), 16); + assert!(t.elapsed().as_secs() < 20, "{:?}", t.elapsed()); +} + #[test] fn effect_maps_are_cached_and_invalidated_by_pixel_changes() { let mut doc = doc_white(64, 64); @@ -2326,6 +2340,26 @@ fn blend_if_this_layer_hides_by_the_layers_own_value() { assert!(close4(px(&d, 1, 1), [0.0, 0.0, 0.0, 1.0])); } +#[test] +fn nested_blend_if_groups_do_not_double_the_work_at_every_level() { + // Each Blend If group re-renders its children for its own colour; 40 levels would be 2^40. + let mut inner = solid_layer("dot", Rect::new(0, 0, 4, 4), [1.0, 1.0, 1.0, 1.0]); + inner.blend_if.set(0, [range([10, 10], [255, 255]), FULL]); + let mut layers = vec![inner]; + for _ in 0..40 { + let mut g = Layer::group("g", layers); + g.blend_if.set(0, [range([10, 10], [255, 255]), FULL]); + layers = vec![g]; + } + // A black backdrop, not doc_white's white: the asserted white can then only come from the + // inner layer rendered through the whole group chain, not from the backdrop itself. + let mut d = Document::with_background("t", Size::new(4, 4), ColorMode::Rgb, SampleType::U8, Color::BLACK); + d.layers.extend(layers); + let t = std::time::Instant::now(); + assert!(close4(px(&d, 1, 1), [1.0, 1.0, 1.0, 1.0])); + assert!(t.elapsed().as_secs() < 10, "{:?}", t.elapsed()); +} + #[test] fn blend_if_underlying_layer_hides_by_the_backdrop_value() { // Backdrop black on the left, white on the right; a red layer over all of it. diff --git a/crates/compose/src/uniform.rs b/crates/compose/src/uniform.rs index e9423d0e69..02bc4a4111 100644 --- a/crates/compose/src/uniform.rs +++ b/crates/compose/src/uniform.rs @@ -32,7 +32,8 @@ impl<'a> UniformAreas<'a> { return None; } let mut surfaces = Vec::new(); - for (_, _, l) in doc.walk() { + // Hidden layers are not composited: one with an effect must not rule the shortcut out. + for l in crate::visible_layers(&doc.layers) { let effects = l.effects.enabled && !l.effects.items.is_empty(); if effects || l.vector_mask.is_some() || l.video.is_some() || l.blend == BlendMode::Dissolve { return None;