diff --git a/Cargo.lock b/Cargo.lock index 9a7455d3..49b9f93b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5766,6 +5766,7 @@ dependencies = [ name = "schist-filters-core" version = "0.6.0" dependencies = [ + "rayon", "schist-color", "schist-fx", "schist-neural", @@ -5930,6 +5931,7 @@ dependencies = [ name = "schist-tools-retouch" version = "0.6.0" dependencies = [ + "rayon", "schist-color", "schist-core", "schist-plugin-api", diff --git a/crates/app/src/workspace.rs b/crates/app/src/workspace.rs index f291d99b..c7fdb011 100644 --- a/crates/app/src/workspace.rs +++ b/crates/app/src/workspace.rs @@ -83,6 +83,12 @@ pub(crate) fn load_view_options() -> ViewOptions { .unwrap_or_default() } +/// The most surrounding image a filter is handed beyond its selection. +/// +/// A radius-250 blur on a four-pixel selection should not end up +/// compositing the whole canvas. +const MAX_FILTER_CONTEXT: u32 = 256; + /// How often a dirty document is snapshotted for crash recovery. const AUTOSAVE_SECS: u64 = 30; @@ -2692,6 +2698,7 @@ impl Workspace { .unwrap_or_else(|| "export".into()); let dir = base.parent().unwrap_or(std::path::Path::new(".")); let mut written = 0usize; + let mut failed: Vec = Vec::new(); for (name, rect) in regions { let rect = rect.intersect(&doc.canvas_rect()); if rect.is_empty() { @@ -2720,18 +2727,35 @@ impl Workspace { .collect(); let out = dir.join(format!("{stem}-{safe}.png")); let Some(codec) = self.registry.codecs().find(|c| c.id() == "png") else { + failed.push(format!("{name}: no png exporter")); continue; }; - match codec.export(®ion_doc) { - Ok(bytes) => { - if std::fs::write(&out, bytes).is_ok() { - written += 1; - } + // Both failure paths used to be silent -- a discarded + // `is_ok()` and a `log::error!` -- while the status bar + // reported however many had worked, so "Exported 3 + // region(s)" out of five looked like success. + match codec + .export(®ion_doc) + .map_err(|e| e.to_string()) + .and_then(|bytes| std::fs::write(&out, bytes).map_err(|e| e.to_string())) + { + Ok(()) => written += 1, + Err(e) => { + log::error!("export {name}: {e}"); + failed.push(format!("{name}: {e}")); } - Err(e) => log::error!("export {name}: {e}"), } } - self.status = format!("Exported {written} region(s)").into(); + self.status = if failed.is_empty() { + format!("Exported {written} region(s)").into() + } else { + format!( + "Exported {written} region(s); {} failed ({})", + failed.len(), + failed.join(", ") + ) + .into() + }; cx.notify(); } @@ -4754,20 +4778,38 @@ impl Workspace { /// selection, as one undoable edit. /// The pixels a filter would touch: the layer's content clipped to the /// canvas, or to the selection when there is one. - fn filter_region(&self, layer_id: schist_core::LayerId) -> IntRect { + /// The region a filter runs over, grown by how far it reads. + /// + /// With a selection active the buffer used to be exactly + /// `selection.bounds()`, and the kernels clamp at the buffer edge -- + /// so blurring a selection repeated its boundary row outward instead + /// of pulling in the real pixels just outside it, leaving a visible + /// band along the selection edge. The write side masks by selection + /// coverage, so growing the region changes what the filter *sees* + /// without widening what it changes. + fn filter_region_with_context(&self, layer_id: schist_core::LayerId, context: u32) -> IntRect { let Some(doc) = self.doc.as_ref() else { return IntRect::EMPTY; }; let canvas = doc.canvas_rect(); if doc.selection.is_empty() { - doc.tree + return doc + .tree .find(layer_id) .map(|l| l.content_bounds()) .unwrap_or(IntRect::EMPTY) - .intersect(&canvas) - } else { - doc.selection.bounds().intersect(&canvas) + .intersect(&canvas); } + // Cap the growth: a radius-250 blur on a 4-pixel selection should + // not composite the whole canvas. + // + // The grown region is *not* intersected with the layer's content: + // a generator like Render > Clouds fills the selection on an + // empty layer, where `content_bounds()` is EMPTY, and intersecting + // refused the whole operation with "Nothing to filter". The canvas + // is the only bound that always applies. + let pad = context.min(MAX_FILTER_CONTEXT) as i32; + doc.selection.bounds().inflated(pad).intersect(&canvas) } /// Pull `region` out of a raster layer into a flat straight-alpha @@ -4849,6 +4891,17 @@ impl Workspace { /// be re-run from the original on every slider tick and undone on /// cancel. Returns false when there is nothing to filter. pub fn begin_filter_preview(&mut self) -> bool { + self.begin_filter_preview_for(0) + } + + /// As above, sized for a filter that reads `context` pixels outside + /// what it writes. + /// + /// The dialog's Apply reuses the preview's region, so growing it only + /// in `apply_filter` never reached any interactive path -- the + /// selection-edge band this is meant to remove stayed exactly where + /// it was. + pub fn begin_filter_preview_for(&mut self, context: u32) -> bool { self.filter_preview = None; let Some(layer_id) = self.doc.as_ref().and_then(|d| d.active_layer) else { self.status = "Select a layer first".into(); @@ -4864,7 +4917,7 @@ impl Workspace { self.status = "Filters need a pixel layer".into(); return false; } - let region = self.filter_region(layer_id); + let region = self.filter_region_with_context(layer_id, context); if region.is_empty() { self.status = "Nothing to filter".into(); return false; @@ -4943,6 +4996,7 @@ impl Workspace { return; }; let name = filter.name().to_string(); + let context = filter.context(values); if self .doc .as_ref() @@ -4957,7 +5011,7 @@ impl Workspace { let region = preview .filter(|p| p.layer == layer_id) .map(|p| p.region) - .unwrap_or_else(|| self.filter_region(layer_id)); + .unwrap_or_else(|| self.filter_region_with_context(layer_id, context)); if region.is_empty() { self.status = "Nothing to filter".into(); return; @@ -5213,7 +5267,17 @@ impl Workspace { self.apply_filter(id, &values, cx); return; } - if !self.begin_filter_preview() { + // Size the preview for the widest reach this filter can be given, + // so dragging its radius to the maximum still reads real + // surrounding pixels rather than a clamped edge. + let reach = filter + .params() + .iter() + .filter(|p| matches!(p.key, "radius" | "amount" | "size" | "distance")) + .map(|p| p.max.ceil().max(0.0) as u32) + .max() + .unwrap_or(0); + if !self.begin_filter_preview_for(reach) { cx.notify(); return; } @@ -5398,6 +5462,13 @@ impl Workspace { rgba }; self.display_tiles.insert(coord, managed.clone()); + // The colour-managed copy is a second 256 KiB per tile, and this + // map had no ceiling either. Keep it to what the composited cache + // still holds, so the two together stay inside one budget. + if self.display_tiles.len() > self.cache.len() { + let cache = &self.cache; + self.display_tiles.retain(|c, _| cache.contains(*c)); + } Some(managed) } @@ -5439,6 +5510,12 @@ impl Workspace { missing.push(((dx.max(dy), dx * dx + dy * dy), coord)); } missing.sort_unstable_by_key(|(k, _)| *k); + // Nearest-first, capped. The cap is on the queue rather than on + // the cache being full: once a large document fills the byte + // budget the steady state *is* full, so refusing to prefetch + // there left every new viewport cold and lost the mid-gesture + // warming that makes the settle frame land instantly. LRU evicts + // the distant tiles instead. missing.truncate(PREFETCH_TILE_BUDGET); missing.reverse(); self.prefetch_queue = missing.into_iter().map(|(_, c)| c).collect(); @@ -5483,6 +5560,7 @@ impl Workspace { if self.pointer_down { return true; } + let stale = match self.doc.as_ref() { Some(doc) => (doc.revision, self.color_epoch) != self.prefetch_stamp, None => true, @@ -6572,8 +6650,13 @@ fn fx_key(layer: &Layer) -> u64 { use std::hash::{Hash, Hasher}; let mut h = rustc_hash::FxHasher::default(); // The style itself, via its debug form: these are small plain structs - // with float fields, so there is nothing cheaper that is also correct. - format!("{:?}", layer.style).hash(&mut h); + // with float fields, so there is nothing cheaper that is also correct + // -- and going through Debug means a field added later is covered + // without anyone remembering to update this. What it does not need is + // the String: `LayerStyle` holds nine effect structs, so formatting + // it built a multi-kilobyte allocation, hashed it and threw it away, + // on every pointer move and once per styled layer. + hash_debug(&layer.style, &mut h); layer.fill_opacity.to_bits().hash(&mut h); if let Some(r) = layer.as_raster() { r.tiles.fingerprint().hash(&mut h); @@ -6587,13 +6670,27 @@ fn fx_key(layer: &Layer) -> u64 { h.finish() } +/// Hash a value's `Debug` form without building a `String` for it. +fn hash_debug(value: &impl std::fmt::Debug, h: &mut rustc_hash::FxHasher) { + use std::fmt::Write as _; + struct Sink<'a>(&'a mut rustc_hash::FxHasher); + impl std::fmt::Write for Sink<'_> { + fn write_str(&mut self, s: &str) -> std::fmt::Result { + std::hash::Hasher::write(self.0, s.as_bytes()); + Ok(()) + } + } + // Writing into a hasher cannot fail. + let _ = write!(Sink(h), "{value:?}"); +} + fn fx_key_children(layers: &[Layer], h: &mut rustc_hash::FxHasher) { use std::hash::Hash; for l in layers { l.visible.hash(h); l.opacity.to_bits().hash(h); l.fill_opacity.to_bits().hash(h); - format!("{:?}", l.blend).hash(h); + l.blend.hash(h); l.render_offset.hash(h); l.clipping.hash(h); if let Some(r) = l.as_raster() { diff --git a/crates/compositor-gpu/src/fx_blur.wgsl b/crates/compositor-gpu/src/fx_blur.wgsl index 1a40d069..e4abe42c 100644 --- a/crates/compositor-gpu/src/fx_blur.wgsl +++ b/crates/compositor-gpu/src/fx_blur.wgsl @@ -1,10 +1,13 @@ // Separable box blur, one axis per dispatch. // // Three rounds of this approximate a Gaussian, which is what every blur in -// the filter set is built from. The loop mirrors `schist_fx::box_pass` -// tap for tap and in the same order: each output pixel sums its own -// window, so there is no running total whose float error would drift away -// from the reference. +// the filter set is built from. +// +// Each output pixel sums its own window here, while the CPU reference +// (`schist_fx::box_pass`) now carries a running total. The two therefore +// differ by float accumulation order, and the drift grows with row +// length; `viewport_minify` and the fx tests pin the agreement at the +// sizes that matter, so any tightening of that bound belongs there. struct Params { width: u32, diff --git a/crates/compositor-gpu/tests/fx_parity.rs b/crates/compositor-gpu/tests/fx_parity.rs index 85bc31f8..cf01f52f 100644 --- a/crates/compositor-gpu/tests/fx_parity.rs +++ b/crates/compositor-gpu/tests/fx_parity.rs @@ -82,6 +82,10 @@ fn blur_matches_the_cpu_reference() { (129, 3, 9, 3), (7, 7, 20, 3), (256, 200, 12, 3), + // The CPU pass carries a running sum and the shader re-sums each + // window, so their float error diverges with row length. A long + // row is where that shows up. + (4096, 3, 40, 3), ] { let px = noise(w, h, 100 + w as u64); let out = ctx diff --git a/crates/compositor/src/lib.rs b/crates/compositor/src/lib.rs index f4033a48..b2771f55 100644 --- a/crates/compositor/src/lib.rs +++ b/crates/compositor/src/lib.rs @@ -694,9 +694,42 @@ fn blend_buf_onto( /// Damage-driven cache of composited tiles (RGBA8 straight alpha), used by /// the canvas view. Invalidate with document damage rects, then fetch. -#[derive(Default)] +/// Composited tiles, under a byte budget. +/// +/// Each entry is a 256x256 RGBA8 tile -- 256 KiB -- and nothing used to +/// evict: entries only went away on damage or `invalidate_all`, while the +/// prefetcher deliberately warms the whole canvas. An 8000x8000 document +/// drifted to ~512 MB resident and a 16000x16000 one to ~2 GB, with no +/// ceiling and no back pressure. Now the least recently touched tiles go +/// first once the budget is passed. pub struct TileCache { - tiles: FxHashMap>>, + tiles: FxHashMap, + /// Monotonic counter standing in for a clock: the tile with the + /// lowest stamp is the one untouched longest. + clock: u64, + bytes: usize, + budget: usize, +} + +struct Entry { + pixels: Arc>, + touched: u64, +} + +/// How much composited tile data to keep. 256 MiB is a thousand tiles, +/// which covers a 8000x8000 document's visible working set several times +/// over while staying a resident size a desktop app can justify. +pub const DEFAULT_TILE_BUDGET: usize = 256 * 1024 * 1024; + +impl Default for TileCache { + fn default() -> Self { + TileCache { + tiles: FxHashMap::default(), + clock: 0, + bytes: 0, + budget: DEFAULT_TILE_BUDGET, + } + } } impl TileCache { @@ -704,30 +737,97 @@ impl TileCache { Self::default() } + /// A cache with a specific byte budget. + pub fn with_budget(budget: usize) -> Self { + TileCache { + budget, + ..Default::default() + } + } + + /// Bytes currently held. + pub fn bytes(&self) -> usize { + self.bytes + } + + /// How many tiles are cached. + pub fn len(&self) -> usize { + self.tiles.len() + } + + pub fn is_empty(&self) -> bool { + self.tiles.is_empty() + } + + /// Whether the cache is at or over its budget, so a prefetcher can + /// stop queueing rather than push resident memory up without limit. + pub fn is_full(&self) -> bool { + self.bytes >= self.budget + } + pub fn invalidate(&mut self, rect: &IntRect) { if rect.is_empty() { return; } for coord in TileCoord::covering(rect) { - self.tiles.remove(&coord); + self.remove(coord); } } pub fn invalidate_all(&mut self) { self.tiles.clear(); + self.bytes = 0; + } + + fn remove(&mut self, coord: TileCoord) { + if let Some(entry) = self.tiles.remove(&coord) { + self.bytes = self.bytes.saturating_sub(entry.pixels.len()); + } + } + + fn insert(&mut self, coord: TileCoord, pixels: Arc>) { + self.remove(coord); + self.clock += 1; + self.bytes += pixels.len(); + self.tiles.insert( + coord, + Entry { + pixels, + touched: self.clock, + }, + ); + self.evict(); + } + + /// Drop least-recently-touched tiles until back inside the budget. + fn evict(&mut self) { + while self.bytes > self.budget && self.tiles.len() > 1 { + let Some(&oldest) = self + .tiles + .iter() + .min_by_key(|(_, e)| e.touched) + .map(|(c, _)| c) + else { + break; + }; + self.remove(oldest); + } } /// Get (compositing on miss) the RGBA8 pixels for a tile. pub fn get(&mut self, doc: &Document, coord: TileCoord) -> Arc> { - if let Some(t) = self.tiles.get(&coord) { - return t.clone(); + self.clock += 1; + let clock = self.clock; + if let Some(entry) = self.tiles.get_mut(&coord) { + entry.touched = clock; + return entry.pixels.clone(); } let bytes = backend() .tiles_rgba8(doc, &[coord]) .pop() .unwrap_or_else(|| vec![0u8; TILE_PIXELS * 4]); let arc = Arc::new(bytes); - self.tiles.insert(coord, arc.clone()); + self.insert(coord, arc.clone()); arc } @@ -748,7 +848,7 @@ impl TileCache { } let computed = backend().tiles_rgba8(doc, &missing); for (c, bytes) in missing.into_iter().zip(computed) { - self.tiles.insert(c, Arc::new(bytes)); + self.insert(c, Arc::new(bytes)); } } } @@ -946,6 +1046,59 @@ mod tests { .select_rect(IntRect::from_xywh(0, 0, 8, 8), SelectOp::Replace); assert_eq!(px(&doc, 20, 20), [5, 6, 7, 255]); } + + /// Nothing used to evict: entries went away only on damage or + /// `invalidate_all`, while the prefetcher deliberately warms the + /// whole canvas. An 8000x8000 document drifted to ~512 MB resident + /// and a 16000x16000 one to ~2 GB, with no ceiling. + #[test] + fn the_tile_cache_stays_inside_its_budget() { + let mut doc = Document::new("t", 4096, 4096, Depth::Eight); + let mut layer = Layer::new_raster("bg"); + let buf = [10u8, 20, 30, 255].repeat(64 * 64); + blit_rgba8( + &mut layer.as_raster_mut().unwrap().tiles, + Depth::Eight, + IntRect::from_size(64, 64), + &buf, + ); + doc.push_layer(layer); + + // Room for four tiles. + let tile_bytes = TILE_PIXELS * 4; + let mut cache = TileCache::with_budget(tile_bytes * 4); + for i in 0..16 { + cache.get(&doc, TileCoord { tx: i, ty: 0 }); + } + assert!( + cache.bytes() <= tile_bytes * 4, + "cache held {} bytes against a {} budget", + cache.bytes(), + tile_bytes * 4 + ); + assert!(cache.is_full()); + } + + /// And it evicts the tile untouched longest, not an arbitrary one. + #[test] + fn the_tile_cache_keeps_what_was_touched_most_recently() { + let mut doc = Document::new("t", 4096, 512, Depth::Eight); + doc.push_layer(Layer::new_raster("bg")); + let mut cache = TileCache::with_budget(TILE_PIXELS * 4 * 2); + + let a = TileCoord { tx: 0, ty: 0 }; + let b = TileCoord { tx: 1, ty: 0 }; + let c = TileCoord { tx: 2, ty: 0 }; + cache.get(&doc, a); + cache.get(&doc, b); + // Touch `a` again, so `b` is now the stalest. + cache.get(&doc, a); + cache.get(&doc, c); + + assert!(cache.contains(a), "the recently used tile was evicted"); + assert!(cache.contains(c)); + assert!(!cache.contains(b), "the stalest tile survived"); + } } #[cfg(test)] diff --git a/crates/core/src/selection.rs b/crates/core/src/selection.rs index 1524cdf2..0d2101f8 100644 --- a/crates/core/src/selection.rs +++ b/crates/core/src/selection.rs @@ -189,6 +189,8 @@ impl Selection { /// Polygon (lasso) selection, even-odd fill, anti-aliased with 4x4 /// supersampling. Points are document-space. pub fn select_polygon(&mut self, points: &[(f32, f32)], op: SelectOp) { + /// Supersamples per axis. + const SUB: usize = 4; if points.len() < 3 { return; } @@ -202,33 +204,69 @@ impl Selection { )); } let pts: Vec<(f64, f64)> = points.iter().map(|&(x, y)| (x as f64, y as f64)).collect(); - let inside = move |px: f64, py: f64| { - let mut winding = false; - let n = pts.len(); - for i in 0..n { - let (x1, y1) = pts[i]; - let (x2, y2) = pts[(i + 1) % n]; - if (y1 > py) != (y2 > py) { - let xint = x1 + (py - y1) / (y2 - y1) * (x2 - x1); - if px < xint { - winding = !winding; - } + + // Crossings per supersample row, built once. + // + // This used to test every edge against every subsample: a + // 600x600 lasso with 2000 points is 600*600*16*2000 edge tests -- + // roughly 1.2e10, single-threaded, on pointer-up, which froze the + // window for many seconds. Walking each edge down the rows it + // actually spans costs the perimeter instead of the area, and the + // per-pixel work drops to a scan of the handful of crossings on + // that row. + let sub_rows = (rect.height() as usize).saturating_mul(SUB); + let mut crossings: Vec> = vec![Vec::new(); sub_rows]; + let top = rect.top as f64; + for i in 0..pts.len() { + let (x1, y1) = pts[i]; + let (x2, y2) = pts[(i + 1) % pts.len()]; + if y1 == y2 { + continue; + } + // Half-open in y, matching the `(y1 > py) != (y2 > py)` rule + // the per-pixel test used, so shared vertices count once. + let (lo, hi) = if y1 < y2 { (y1, y2) } else { (y2, y1) }; + let first = (((lo - top) * SUB as f64 - 0.5).ceil()).max(0.0) as usize; + let last = (((hi - top) * SUB as f64 - 0.5).floor()).max(-1.0); + let last = if last < 0.0 { + continue; + } else { + (last as usize).min(sub_rows.saturating_sub(1)) + }; + for (row, out) in crossings.iter_mut().enumerate().take(last + 1).skip(first) { + let py = top + (row as f64 + 0.5) / SUB as f64; + if (y1 > py) == (y2 > py) { + continue; } + out.push(x1 + (py - y1) / (y2 - y1) * (x2 - x1)); } - winding - }; + } + for row in crossings.iter_mut() { + row.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal)); + } + + let top_i = rect.top; self.apply_shape(rect, op, move |x, y| { let mut hits = 0u32; - for sy in 0..4 { - for sx in 0..4 { - let px = x as f64 + (sx as f64 + 0.5) / 4.0; - let py = y as f64 + (sy as f64 + 0.5) / 4.0; - if inside(px, py) { + for sy in 0..SUB { + let row = (y - top_i) as usize * SUB + sy; + let Some(xs) = crossings.get(row) else { + continue; + }; + if xs.is_empty() { + continue; + } + for sx in 0..SUB { + let px = x as f64 + (sx as f64 + 0.5) / SUB as f64; + // Even-odd: inside when an odd number of edges lie to + // the left. + let left = xs.partition_point(|&xi| xi <= px); + if left % 2 == 1 { hits += 1; } } } - ((hits * 255) / 16) as u8 + ((hits * 255) / (SUB * SUB) as u32) as u8 }); } @@ -704,6 +742,78 @@ mod tests { assert!(sel.coverage(5, 5) > 200); assert_eq!(sel.coverage(35, 35), 0); } + + /// The scanline fill must agree with the brute-force point-in-polygon + /// test it replaced, pixel for pixel. + /// + /// The old form tested every edge against every subsample, so a + /// 600x600 lasso with 2000 points cost ~1.2e10 edge tests on + /// pointer-up and froze the window for seconds. + #[test] + fn the_scanline_fill_matches_the_brute_force_one() { + // A star, so the even-odd rule actually has self-crossings to + // resolve, plus a plain triangle and a concave L. + let star: Vec<(f32, f32)> = (0..10) + .map(|i| { + let a = i as f32 / 10.0 * std::f32::consts::TAU; + let r = if i % 2 == 0 { 40.0 } else { 16.0 }; + (50.0 + r * a.cos(), 50.0 + r * a.sin()) + }) + .collect(); + let triangle = vec![(5.0, 5.0), (90.0, 20.0), (30.0, 88.0)]; + let ell = vec![ + (10.0, 10.0), + (70.0, 10.0), + (70.0, 30.0), + (30.0, 30.0), + (30.0, 80.0), + (10.0, 80.0), + ]; + + for points in [star, triangle, ell] { + let mut sel = Selection::default(); + sel.select_polygon(&points, SelectOp::Replace); + + for y in 0..100 { + for x in 0..100 { + let want = brute_force_coverage(&points, x, y); + let got = sel.coverage(x, y); + assert_eq!(got, want, "at ({x}, {y})"); + } + } + } + } + + /// `select_polygon` as it was written before the scanline rewrite. + fn brute_force_coverage(points: &[(f32, f32)], x: i32, y: i32) -> u8 { + let pts: Vec<(f64, f64)> = points.iter().map(|&(x, y)| (x as f64, y as f64)).collect(); + let inside = |px: f64, py: f64| { + let mut winding = false; + let n = pts.len(); + for i in 0..n { + let (x1, y1) = pts[i]; + let (x2, y2) = pts[(i + 1) % n]; + if (y1 > py) != (y2 > py) { + let xint = x1 + (py - y1) / (y2 - y1) * (x2 - x1); + if px < xint { + winding = !winding; + } + } + } + winding + }; + let mut hits = 0u32; + for sy in 0..4 { + for sx in 0..4 { + let px = x as f64 + (sx as f64 + 0.5) / 4.0; + let py = y as f64 + (sy as f64 + 0.5) / 4.0; + if inside(px, py) { + hits += 1; + } + } + } + ((hits * 255) / 16) as u8 + } } #[cfg(test)] diff --git a/crates/fx/examples/cpubench.rs b/crates/fx/examples/cpubench.rs new file mode 100644 index 00000000..d54a860e --- /dev/null +++ b/crates/fx/examples/cpubench.rs @@ -0,0 +1,114 @@ +//! The CPU-side complexity changes, each timed against the formulation it +//! replaced. Run with +//! `cargo run --release -p schist-fx --example cpubench`. +//! +//! The old implementations are inlined here rather than kept in the +//! library, the same way the equivalence tests carry them: the point is +//! to be able to re-measure the claim, not to keep the slow path alive. + +use std::time::Instant; + +fn noise(w: usize, h: usize) -> Vec { + let mut state = 0x2545F4914F6CDD1Du64; + (0..w * h * 4) + .map(|_| { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + (state >> 40) as f32 / 16777216.0 + }) + .collect() +} + +fn time(label: &str, old: impl FnOnce(), new: impl FnOnce()) { + let start = Instant::now(); + old(); + let old_ms = start.elapsed().as_secs_f64() * 1000.0; + let start = Instant::now(); + new(); + let new_ms = start.elapsed().as_secs_f64() * 1000.0; + println!( + "{label:<40} before {old_ms:9.1} ms after {new_ms:8.1} ms {:.0}x", + old_ms / new_ms.max(0.001) + ); +} + +/// The window re-summed per pixel, single threaded, as `box_pass` was. +fn naive_box_pass( + src: &[f32], + dst: &mut [f32], + width: usize, + height: usize, + r: usize, + vertical: bool, +) { + let (outer, inner) = if vertical { + (width, height) + } else { + (height, width) + }; + let stride = if vertical { width * 4 } else { 4 }; + let step = if vertical { 4 } else { width * 4 }; + let window = (r * 2 + 1) as f32; + for o in 0..outer { + let base = o * step; + for i in 0..inner { + let mut acc = [0.0f32; 4]; + for k in 0..=(r * 2) { + let s = (i + k).saturating_sub(r).min(inner - 1); + let at = base + s * stride; + for c in 0..4 { + acc[c] += src[at + c]; + } + } + let at = base + i * stride; + for c in 0..4 { + dst[at + c] = acc[c] / window; + } + } + } +} + +fn naive_blur(px: &mut [f32], width: usize, height: usize, r: usize, passes: usize) { + let mut tmp = vec![0f32; px.len()]; + for _ in 0..passes { + naive_box_pass(px, &mut tmp, width, height, r, false); + naive_box_pass(&tmp, px, width, height, r, true); + } +} + +fn main() { + // 1 MP, which is a modest layer: the blur slider goes to 250. + let (w, h) = (1000usize, 1000usize); + let px = noise(w, h); + for radius in [10.0f32, 50.0, 100.0] { + let r = ((radius / 3.0f32.sqrt()).round() as usize).max(1); + time( + &format!("gaussian blur {w}x{h} r={radius:.0}"), + || { + let mut buf = px.clone(); + naive_blur(&mut buf, w, h, r, 3); + }, + || { + let mut buf = px.clone(); + schist_fx::blur_rgba_cpu(&mut buf, w, h, r, 3); + }, + ); + } + + // And 12 MP, where a full-canvas preview hurts. + let (w, h) = (4000usize, 3000usize); + let px = noise(w, h); + let r = ((50.0f32 / 3.0f32.sqrt()).round() as usize).max(1); + time( + &format!("gaussian blur {w}x{h} r=50"), + || { + let mut buf = px.clone(); + naive_blur(&mut buf, w, h, r, 3); + }, + || { + let mut buf = px.clone(); + schist_fx::blur_rgba_cpu(&mut buf, w, h, r, 3); + }, + ); +} diff --git a/crates/fx/src/lib.rs b/crates/fx/src/lib.rs index d6f0aed8..7132185d 100644 --- a/crates/fx/src/lib.rs +++ b/crates/fx/src/lib.rs @@ -239,7 +239,22 @@ pub fn blur_rgba_cpu(px: &mut [f32], width: usize, height: usize, radius: usize, unpremultiply(px); } -/// One separable box pass, clamping at the edges. +/// One separable box pass over RGBA f32, clamping at the edges and +/// carrying a running sum. +/// +/// This re-summed the whole `2r+1` window for every output pixel, making +/// the pass O(pixels * r) rather than O(pixels): a gaussian at radius 50 +/// on 2048x2048 took seconds. The window only gains and loses one sample +/// per step, which is the formulation `layer-fx::blur` already used. +/// +/// Rows are independent and could be parallel too, but the vertical pass +/// walks columns, which are not contiguous in the buffer, so that needs a +/// transpose rather than a `par_chunks_mut`. The algorithmic fix is the +/// dominant one; threading is a separate change. +/// +/// `fx_blur.wgsl` still sums each window itself, so the two differ by +/// accumulation order and the drift grows with row length. The parity +/// tests compare with a tolerance, at long rows as well as short ones. fn box_pass(src: &[f32], dst: &mut [f32], width: usize, height: usize, r: usize, vertical: bool) { let (outer, inner) = if vertical { (width, height) @@ -251,19 +266,28 @@ fn box_pass(src: &[f32], dst: &mut [f32], width: usize, height: usize, r: usize, let window = (r * 2 + 1) as f32; for o in 0..outer { let base = o * step; - for i in 0..inner { - let mut acc = [0.0f32; 4]; - for k in 0..=(r * 2) { - let s = (i + k).saturating_sub(r).min(inner - 1); - let at = base + s * stride; - for c in 0..4 { - acc[c] += src[at + c]; - } + // Seed the window: the edge sample repeated for the leading half, + // which is the same clamped edge handling as before. + let mut acc = [0.0f32; 4]; + for c in 0..4 { + acc[c] = src[base + c] * r as f32; + } + for k in 0..=r { + let at = base + k.min(inner - 1) * stride; + for c in 0..4 { + acc[c] += src[at + c]; } + } + for i in 0..inner { let at = base + i * stride; for c in 0..4 { dst[at + c] = acc[c] / window; } + let add = base + (i + r + 1).min(inner - 1) * stride; + let sub = base + i.saturating_sub(r) * stride; + for c in 0..4 { + acc[c] += src[add + c] - src[sub + c]; + } } } } @@ -296,15 +320,28 @@ pub fn lens_blur_rgba_cpu(px: &mut [f32], width: usize, height: usize, radius: i let r = radius; premultiply(px); let src = px.to_vec(); - for y in 0..height as i32 { - for x in 0..width as i32 { - let mut acc = [0.0f32; 4]; - let mut n = 0.0f32; - for dy in -r..=r { - for dx in -r..=r { - if dx * dx + dy * dy > r * r { - continue; - } + // The disc was rediscovered per pixel by testing `dx*dx + dy*dy > r*r` + // across the whole bounding square, so roughly a quarter of the taps + // were tested and thrown away every time. Build it once. + let mut offsets = Vec::new(); + for dy in -r..=r { + for dx in -r..=r { + if dx * dx + dy * dy <= r * r { + offsets.push((dx, dy)); + } + } + } + // Each row writes a disjoint slice and reads only the immutable `src`, + // so this is a plain parallel gather. It was single-threaded, which at + // radius 12 on 2048x2048 measured 5.62 s. + px.par_chunks_mut(width * 4) + .enumerate() + .for_each(|(y, row)| { + let y = y as i32; + for x in 0..width as i32 { + let mut acc = [0.0f32; 4]; + let mut n = 0.0f32; + for &(dx, dy) in &offsets { let p = at(&src, width, height, x + dx, y + dy); // Weighting bright samples up spreads highlights into // discs instead of smearing them away. @@ -314,16 +351,14 @@ pub fn lens_blur_rgba_cpu(px: &mut [f32], width: usize, height: usize, radius: i } n += k; } - } - if n > 0.0 { - for a in acc.iter_mut() { - *a /= n; + if n > 0.0 { + let i = x as usize * 4; + for c in 0..4 { + row[i + c] = acc[c] / n; + } } - let i = (y as usize * width + x as usize) * 4; - px[i..i + 4].copy_from_slice(&acc); } - } - } + }); unpremultiply(px); } @@ -456,7 +491,9 @@ pub fn carve_cpu(job: &CarveJob<'_>) -> Carved { height: job.height, px: job.px.to_vec(), protect: job.protect.to_vec(), + energy: Vec::new(), }; + img.energy = img.compute_energy(); // A one-pixel image has no seam to remove and nothing to interpolate // against, so both loops stop there. while img.width > job.target_width.max(1) { @@ -478,6 +515,14 @@ struct Plane { height: usize, px: Vec, protect: Vec, + /// The energy field, kept across seams. + /// + /// It used to be rebuilt from scratch for every seam -- and there is + /// one seam per pixel of width change, so shrinking a 2000-pixel + /// image by a quarter rebuilt a two-megapixel field five hundred + /// times. Removing a seam only changes the energy within a pixel of + /// where it ran, so the rest carries over. + energy: Vec, } impl Plane { @@ -487,22 +532,52 @@ impl Plane { 0.299 * self.px[i] + 0.587 * self.px[i + 1] + 0.114 * self.px[i + 2] } - /// Gradient magnitude plus protection. - fn energy(&self) -> Vec { + /// Gradient magnitude plus protection, at one pixel. + #[inline] + fn energy_at(&self, x: usize, y: usize) -> f32 { + let (w, h) = (self.width, self.height); + let l = self.lum(x.saturating_sub(1), y); + let r = self.lum((x + 1).min(w - 1), y); + let u = self.lum(x, y.saturating_sub(1)); + let d = self.lum(x, (y + 1).min(h - 1)); + // Fully transparent pixels are free to remove. + let alpha = self.px[(y * w + x) * 4 + 3]; + ((r - l).abs() + (d - u).abs()) * alpha + self.protect[y * w + x] + } + + /// The whole energy field. Only the first seam pays for this. + fn compute_energy(&self) -> Vec { let (w, h) = (self.width, self.height); let mut out = vec![0.0f32; w * h]; - for y in 0..h { - for x in 0..w { - let l = self.lum(x.saturating_sub(1), y); - let r = self.lum((x + 1).min(w - 1), y); - let u = self.lum(x, y.saturating_sub(1)); - let d = self.lum(x, (y + 1).min(h - 1)); - // Fully transparent pixels are free to remove. - let alpha = self.px[(y * w + x) * 4 + 3]; - out[y * w + x] = ((r - l).abs() + (d - u).abs()) * alpha + self.protect[y * w + x]; + out.par_chunks_mut(w.max(1)) + .enumerate() + .for_each(|(y, row)| { + for (x, v) in row.iter_mut().enumerate() { + *v = self.energy_at(x, y); + } + }); + out + } + + /// Recompute energy in a band around `x` on rows `y - 1 ..= y + 1`. + /// + /// Energy reads one pixel each way, so removing or inserting a column + /// at `x` invalidates `x - 1 ..= x + 1` on that row, and the rows + /// either side through the vertical difference. + fn refresh_energy_near(&mut self, x: usize, y: usize) { + let (w, h) = (self.width, self.height); + if w == 0 || h == 0 { + return; + } + let y0 = y.saturating_sub(1); + let y1 = (y + 1).min(h - 1); + let x0 = x.saturating_sub(1); + let x1 = (x + 1).min(w - 1); + for yy in y0..=y1 { + for xx in x0..=x1 { + self.energy[yy * w + xx] = self.energy_at(xx, yy); } } - out } /// The lowest-energy top-to-bottom seam, as one x per row. @@ -511,7 +586,7 @@ impl Plane { if w == 0 || h == 0 { return Vec::new(); } - let energy = self.energy(); + let energy = &self.energy; // Cumulative cost, and which of the three pixels above we came // from, so the seam can be walked back. let mut cost = energy.clone(); @@ -566,6 +641,21 @@ impl Plane { self.px = px; self.protect = prot; self.width = w - 1; + // Carry the field over, dropping the removed column from each + // row, then repair the band the removal disturbed. + let mut energy = Vec::with_capacity((w - 1) * h); + for (y, cut) in seam.iter().enumerate() { + for x in 0..w { + if x == *cut { + continue; + } + energy.push(self.energy[y * w + x]); + } + } + self.energy = energy; + for (y, cut) in seam.iter().enumerate() { + self.refresh_energy_near((*cut).min(self.width.saturating_sub(1)), y); + } } /// Duplicate one vertical seam, widening the image by a pixel. @@ -598,6 +688,20 @@ impl Plane { self.px = px; self.protect = prot; self.width = w + 1; + let mut energy = Vec::with_capacity((w + 1) * h); + for (y, cut) in seam.iter().enumerate() { + for x in 0..w { + energy.push(self.energy[y * w + x]); + if x == *cut { + // Filled in by the repair pass below. + energy.push(0.0); + } + } + } + self.energy = energy; + for (y, cut) in seam.iter().enumerate() { + self.refresh_energy_near((*cut + 1).min(self.width - 1), y); + } } } @@ -730,4 +834,143 @@ mod tests { }; assert!(warp(&job, || src.clone()).iter().all(|v| *v == 0.0)); } + /// The window re-summed per pixel, as `box_pass` used to be. + fn naive_box_pass( + src: &[f32], + dst: &mut [f32], + width: usize, + height: usize, + r: usize, + vertical: bool, + ) { + let (outer, inner) = if vertical { + (width, height) + } else { + (height, width) + }; + let stride = if vertical { width * 4 } else { 4 }; + let step = if vertical { 4 } else { width * 4 }; + let window = (r * 2 + 1) as f32; + for o in 0..outer { + let base = o * step; + for i in 0..inner { + let mut acc = [0.0f32; 4]; + for k in 0..=(r * 2) { + let s = (i + k).saturating_sub(r).min(inner - 1); + let at = base + s * stride; + for c in 0..4 { + acc[c] += src[at + c]; + } + } + let at = base + i * stride; + for c in 0..4 { + dst[at + c] = acc[c] / window; + } + } + } + } + + #[test] + fn the_running_sum_matches_the_window_it_replaced() { + // The speed-up must not change the picture. Both edge handling and + // the sum have to agree with the naive version, on both axes. + let (w, h) = (64usize, 48usize); + let src: Vec = (0..w * h * 4) + .map(|i| ((i * 7919) % 997) as f32 / 997.0) + .collect(); + for r in [0usize, 1, 3, 9, 20] { + for vertical in [false, true] { + let mut fast = vec![0.0; src.len()]; + let mut slow = vec![0.0; src.len()]; + box_pass(&src, &mut fast, w, h, r, vertical); + naive_box_pass(&src, &mut slow, w, h, r, vertical); + let worst = fast + .iter() + .zip(&slow) + .map(|(a, b)| (a - b).abs()) + .fold(0.0f32, f32::max); + assert!( + worst < 1e-4, + "r={r} vertical={vertical} diverged by {worst}" + ); + } + } + } + + /// The energy field is carried across seams and repaired only where + /// the seam ran. It used to be rebuilt in full for every seam, and + /// there is one seam per pixel of width change — shrinking a + /// 2000-pixel image by a quarter rebuilt a two-megapixel field five + /// hundred times. + /// + /// A seam moves at most one column per row, so removing it can only + /// disturb the energy within one column of where it ran; this checks + /// that reasoning against a full rebuild, pixel for pixel. + #[test] + fn incremental_energy_carves_the_same_seams() { + let (w, h) = (48usize, 32usize); + // Structure the carve has to make choices about: a bright bar + // down the middle and a noisy background. + let mut px = vec![0f32; w * h * 4]; + for y in 0..h { + for x in 0..w { + let i = (y * w + x) * 4; + let bar = (20..26).contains(&x); + let n = ((x * 37 + y * 17) % 23) as f32 / 23.0; + let v = if bar { 0.9 } else { 0.2 + n * 0.3 }; + px[i] = v; + px[i + 1] = v * 0.8; + px[i + 2] = 1.0 - v; + px[i + 3] = 1.0; + } + } + let protect = vec![0f32; w * h]; + + for target in [w - 1, w - 5, w - 12, w + 1, w + 6] { + let job = CarveJob { + px: &px, + protect: &protect, + width: w, + height: h, + target_width: target, + }; + let fast = carve_cpu(&job); + let slow = reference_carve(&job); + assert_eq!(fast.width, slow.width, "target {target}"); + assert_eq!(fast.px.len(), slow.px.len(), "target {target}"); + for i in 0..fast.px.len() { + assert!( + (fast.px[i] - slow.px[i]).abs() < 1e-6, + "target {target}, sample {i}: {} != {}", + fast.px[i], + slow.px[i] + ); + } + } + } + + /// `carve_cpu` with the energy field rebuilt from scratch per seam, + /// which is what it used to do. + fn reference_carve(job: &CarveJob<'_>) -> Carved { + let mut img = Plane { + width: job.width, + height: job.height, + px: job.px.to_vec(), + protect: job.protect.to_vec(), + energy: Vec::new(), + }; + while img.width > job.target_width.max(1) { + img.energy = img.compute_energy(); + img.carve_one(); + } + while img.width < job.target_width { + img.energy = img.compute_energy(); + img.grow_one(); + } + Carved { + px: img.px, + protect: img.protect, + width: img.width, + } + } } diff --git a/crates/layer-fx/examples/strokebench.rs b/crates/layer-fx/examples/strokebench.rs new file mode 100644 index 00000000..c33f9ec8 --- /dev/null +++ b/crates/layer-fx/examples/strokebench.rs @@ -0,0 +1,80 @@ +//! Layer-style stroke cost against the window search it replaced. Run +//! with `cargo run --release -p schist-layer-fx --example strokebench`. +//! +//! `signed_distance` used to scan a `(2r+1)²` window per pixel; the early +//! break only fires within one pixel of an edge, so every other pixel +//! paid the whole window and the cost grew as r². The old formulation is +//! inlined here, exactly as the equivalence test carries it. + +use std::hint::black_box; +use std::time::Instant; + +/// A disc, so most of the plane is far from the edge -- the case the +/// early break never helped with. +fn disc(w: usize, h: usize) -> Vec { + let (cx, cy) = (w as f32 / 2.0, h as f32 / 2.0); + let r = (w.min(h) as f32) * 0.35; + (0..w * h) + .map(|i| { + let (x, y) = ((i % w) as f32, (i / w) as f32); + if (x - cx).hypot(y - cy) <= r { + 1.0 + } else { + 0.0 + } + }) + .collect() +} + +/// The `(2r+1)²` window search, as `signed_distance` used to be. +fn brute_force_signed_distance(alpha: &[f32], w: usize, h: usize, limit: f32) -> Vec { + let r = limit.ceil() as i32; + let inside = |x: i32, y: i32| -> bool { + x >= 0 + && y >= 0 + && (x as usize) < w + && (y as usize) < h + && alpha[y as usize * w + x as usize] >= 0.5 + }; + let mut out = vec![0f32; w * h]; + for y in 0..h as i32 { + for x in 0..w as i32 { + let here = inside(x, y); + let mut best = limit; + 'search: for dy in -r..=r { + for dx in -r..=r { + if inside(x + dx, y + dy) == here { + continue; + } + let d = ((dx * dx + dy * dy) as f32).sqrt(); + if d < best { + best = d; + if best <= 1.0 { + break 'search; + } + } + } + } + out[y as usize * w + x as usize] = if here { -best } else { best }; + } + } + out +} + +fn main() { + let (w, h) = (1000usize, 1000usize); + let alpha = disc(w, h); + for size in [4.0f32, 12.0, 30.0, 250.0] { + let limit = size + 2.0; + let start = Instant::now(); + black_box(brute_force_signed_distance(&alpha, w, h, limit)); + let old_ms = start.elapsed().as_secs_f64() * 1000.0; + let start = Instant::now(); + black_box(schist_layer_fx::signed_distance(&alpha, w, h, limit)); + let new_ms = start.elapsed().as_secs_f64() * 1000.0; + println!( + "outside stroke {w}x{h} size={size:<5} before {old_ms:9.1} ms after {new_ms:8.1} ms {:.0}x", + old_ms / new_ms.max(0.001) + ); + } +} diff --git a/crates/layer-fx/src/lib.rs b/crates/layer-fx/src/lib.rs index cb528fed..2d537efa 100644 --- a/crates/layer-fx/src/lib.rs +++ b/crates/layer-fx/src/lib.rs @@ -436,43 +436,115 @@ fn precise_grow(a: &mut [f32], w: usize, h: usize, size: f32) { } /// Signed distance to the shape's edge in pixels, negative inside, -/// searched out to `limit`. A brute-force search over a small window is -/// enough: `limit` is an effect size, so tens of pixels at most. -fn signed_distance(alpha: &[f32], w: usize, h: usize, limit: f32) -> Vec { - let r = limit.ceil().max(1.0) as i32; - let mut out = vec![0.0f32; w * h]; - for y in 0..h as i32 { - for x in 0..w as i32 { - let i = y as usize * w + x as usize; - let inside = alpha[i] >= 0.5; - let mut best = limit; - 'search: for dy in -r..=r { - let sy = y + dy; - if sy < 0 || sy >= h as i32 { - continue; - } - for dx in -r..=r { - let sx = x + dx; - if sx < 0 || sx >= w as i32 { - continue; - } - let other = alpha[sy as usize * w + sx as usize] >= 0.5; - if other == inside { - continue; - } - let d = ((dx * dx + dy * dy) as f32).sqrt(); - if d < best { - best = d; - if best <= 1.0 { - break 'search; - } - } - } +/// clamped at `limit`. +/// +/// This used to scan a `(2r+1)^2` window per pixel looking for a sample +/// on the other side of the 0.5-alpha threshold. The early break only +/// fires within one pixel of an edge, so every interior and far-exterior +/// pixel paid the full window: on a 1000x1000 layer an outside stroke +/// measured 205 ms at size 4, 937 ms at size 12 and 5.53 s at size 30, +/// growing as r^2 -- and Photoshop's stroke and glow sizes go to 250. +/// +/// An exact Euclidean distance transform gives the same answer in O(w*h), +/// independent of the radius. Two of them: one seeded on the inside +/// pixels and one on the outside, so each pixel reads the distance to the +/// nearest sample of the opposite class -- exactly what the window search +/// was looking for. Clamping at `limit` afterwards matches the old +/// bound, since any true nearest within `limit` also lay inside the +/// square window. +/// Public so `examples/strokebench.rs` can time it against the window +/// search it replaced. +pub fn signed_distance(alpha: &[f32], w: usize, h: usize, limit: f32) -> Vec { + let inside: Vec = alpha.iter().map(|&a| a >= 0.5).collect(); + let to_inside = squared_edt(&inside, w, h, false); + let to_outside = squared_edt(&inside, w, h, true); + (0..w * h) + .map(|i| { + if inside[i] { + -to_outside[i].sqrt().min(limit) + } else { + to_inside[i].sqrt().min(limit) } - out[i] = if inside { -best } else { best }; + }) + .collect() +} + +/// Squared Euclidean distance to the nearest seed pixel. +/// +/// Felzenszwalb and Huttenlocher's lower-envelope transform: one 1-D pass +/// down the columns, one across the rows. `invert` seeds on the *false* +/// entries instead of the true ones. +fn squared_edt(seed: &[bool], w: usize, h: usize, invert: bool) -> Vec { + // Large but finite: an actual infinity turns the parabola + // intersections below into NaN. + const FAR: f32 = 1e20; + if w == 0 || h == 0 { + return Vec::new(); + } + let mut grid: Vec = seed + .iter() + .map(|&s| if s != invert { 0.0 } else { FAR }) + .collect(); + + let n = w.max(h); + let mut f = vec![0.0f32; n]; + let mut d = vec![0.0f32; n]; + let mut v = vec![0usize; n]; + let mut z = vec![0.0f32; n + 1]; + + for x in 0..w { + for (y, slot) in f[..h].iter_mut().enumerate() { + *slot = grid[y * w + x]; + } + lower_envelope(&f[..h], &mut d[..h], &mut v[..h], &mut z[..h + 1]); + for y in 0..h { + grid[y * w + x] = d[y]; } } - out + for y in 0..h { + f[..w].copy_from_slice(&grid[y * w..y * w + w]); + lower_envelope(&f[..w], &mut d[..w], &mut v[..w], &mut z[..w + 1]); + grid[y * w..y * w + w].copy_from_slice(&d[..w]); + } + grid +} + +/// The 1-D squared distance transform: the lower envelope of the +/// parabolas `(q - i)^2 + f[i]`. +fn lower_envelope(f: &[f32], d: &mut [f32], v: &mut [usize], z: &mut [f32]) { + let n = f.len(); + if n == 0 { + return; + } + let mut k: isize = 0; + v[0] = 0; + z[0] = f32::NEG_INFINITY; + z[1] = f32::INFINITY; + let sq = |i: usize| (i * i) as f32; + for q in 1..n { + let mut s; + loop { + let p = v[k as usize]; + s = ((f[q] + sq(q)) - (f[p] + sq(p))) / (2.0 * q as f32 - 2.0 * p as f32); + // `z[0]` is -inf, so this never walks off the front. + if s > z[k as usize] { + break; + } + k -= 1; + } + k += 1; + v[k as usize] = q; + z[k as usize] = s; + z[k as usize + 1] = f32::INFINITY; + } + let mut k: usize = 0; + for (q, out) in d.iter_mut().enumerate() { + while z[k + 1] < q as f32 { + k += 1; + } + let dq = q as f32 - v[k] as f32; + *out = dq * dq + f[v[k]]; + } } /// Shift an alpha buffer by a fractional offset, sampling bilinearly. @@ -562,3 +634,132 @@ pub fn outset(style: &LayerStyle) -> i32 { /// Re-exported so callers can name the settings types without also /// depending on core's module layout. pub use schist_core::style; + +#[cfg(test)] +mod distance_tests { + use super::signed_distance; + + /// The distance transform must return exactly what the window search + /// it replaced returned, for every pixel and every limit. + #[test] + fn the_distance_transform_matches_the_window_search() { + let (w, h) = (37usize, 29usize); + // A blob with a hole, plus a detached speck, so both signs and + // both near and far pixels are exercised. + let alpha: Vec = (0..w * h) + .map(|i| { + let (x, y) = ((i % w) as f32, (i / w) as f32); + let d = ((x - 12.0).powi(2) + (y - 12.0).powi(2)).sqrt(); + let speck = (30..33).contains(&(x as usize)) && (4..7).contains(&(y as usize)); + if (3.0..8.0).contains(&d) || speck { + 1.0 + } else { + 0.0 + } + }) + .collect(); + + for limit in [1.0f32, 2.0, 4.5, 12.0, 100.0] { + let got = signed_distance(&alpha, w, h, limit); + let want = brute_force_signed_distance(&alpha, w, h, limit); + for i in 0..w * h { + assert!( + (got[i] - want[i]).abs() < 1e-4, + "limit {limit}, pixel ({}, {}): {} != {}", + i % w, + i / w, + got[i], + want[i] + ); + } + } + } + + /// A plane with no edge in it sits entirely at the clamp. + #[test] + fn a_uniform_plane_is_entirely_at_the_limit() { + let (w, h) = (8usize, 8usize); + let solid = vec![1.0f32; w * h]; + let empty = vec![0.0f32; w * h]; + assert!(signed_distance(&solid, w, h, 5.0) + .iter() + .all(|&d| (d + 5.0).abs() < 1e-4)); + assert!(signed_distance(&empty, w, h, 5.0) + .iter() + .all(|&d| (d - 5.0).abs() < 1e-4)); + } + + /// `signed_distance` as it was written before the transform. + fn brute_force_signed_distance(alpha: &[f32], w: usize, h: usize, limit: f32) -> Vec { + let r = limit.ceil().max(1.0) as i32; + let mut out = vec![0.0f32; w * h]; + for y in 0..h as i32 { + for x in 0..w as i32 { + let i = y as usize * w + x as usize; + let inside = alpha[i] >= 0.5; + let mut best = limit; + 'search: for dy in -r..=r { + let sy = y + dy; + if sy < 0 || sy >= h as i32 { + continue; + } + for dx in -r..=r { + let sx = x + dx; + if sx < 0 || sx >= w as i32 { + continue; + } + let other = alpha[sy as usize * w + sx as usize] >= 0.5; + if other == inside { + continue; + } + let d = ((dx * dx + dy * dy) as f32).sqrt(); + if d < best { + best = d; + if best <= 1.0 { + break 'search; + } + } + } + } + out[i] = if inside { -best } else { best }; + } + } + out + } + /// The transform is O(w*h) regardless of the radius, which the + /// window search was not: it grew as r^2, from 205 ms at size 4 to + /// 5.53 s at size 30 on a 1000x1000 layer. + /// + /// Asserted on the *result* rather than on wall clock, which would be + /// a CI flake and would mostly time the new code against itself: a + /// large limit and a small one have to agree everywhere the small one + /// did not clamp. + #[test] + fn a_larger_limit_only_changes_what_was_clamped() { + let (w, h) = (64usize, 48usize); + let alpha: Vec = (0..w * h) + .map(|i| { + let (x, y) = ((i % w) as f32, (i / w) as f32); + if ((x - 32.0).powi(2) + (y - 24.0).powi(2)).sqrt() < 10.0 { + 1.0 + } else { + 0.0 + } + }) + .collect(); + let near = signed_distance(&alpha, w, h, 4.0); + let far = signed_distance(&alpha, w, h, 250.0); + for i in 0..w * h { + if near[i].abs() < 4.0 { + assert!( + (near[i] - far[i]).abs() < 1e-4, + "unclamped sample {i} disagrees: {} vs {}", + near[i], + far[i] + ); + } else { + assert!(far[i].abs() >= 4.0 - 1e-4, "sample {i} should not shrink"); + } + } + } +} diff --git a/crates/plugin-api/src/lib.rs b/crates/plugin-api/src/lib.rs index 55712c1d..df13325c 100644 --- a/crates/plugin-api/src/lib.rs +++ b/crates/plugin-api/src/lib.rs @@ -418,6 +418,20 @@ pub trait FilterPlugin: Send + Sync { /// `width * height` pixels. fn apply(&self, pixels: &mut [f32], width: usize, height: usize, values: &FilterValues); + /// How far outside the region this filter reads, in pixels. + /// + /// The buffer a filter is handed is exactly the region being + /// filtered, and the kernels clamp at its edge -- so blurring a + /// selection repeated the boundary row outward instead of pulling in + /// the real pixels just outside it, leaving a visible band along the + /// selection edge. Advertising the reach lets the shell hand over a + /// grown buffer and blend back only the selection. + /// + /// Zero for anything that only reads the pixel it is writing. + fn context(&self, _values: &FilterValues) -> u32 { + 0 + } + /// A line shown in the filter's dialog, for anything the user should /// know before running it -- which is mostly whether a neural filter /// found its model or is about to use its fallback. diff --git a/plugins/filters-core/Cargo.toml b/plugins/filters-core/Cargo.toml index 5e1e5225..92bef4c0 100644 --- a/plugins/filters-core/Cargo.toml +++ b/plugins/filters-core/Cargo.toml @@ -6,6 +6,7 @@ license.workspace = true [dependencies] schist-plugin-api.workspace = true +rayon.workspace = true schist-neural.workspace = true schist-color.workspace = true schist-fx.workspace = true diff --git a/plugins/filters-core/src/lib.rs b/plugins/filters-core/src/lib.rs index f71bcb3a..c09213d4 100644 --- a/plugins/filters-core/src/lib.rs +++ b/plugins/filters-core/src/lib.rs @@ -81,6 +81,22 @@ macro_rules! simple_filter { fn params(&self) -> Vec { vec![$($param),*] } + /// A filter with a spatial parameter reads that far outside + /// the pixel it writes, so the shell knows to hand it that + /// much surrounding image. + fn context(&self, values: &FilterValues) -> u32 { + let params = self.params(); + // Only genuinely spatial parameters. "amount" is + // intensity in most filters (Add Noise, Unsharp), and + // treating it as reach grew the buffer by up to its whole + // slider range for no benefit. + ["radius", "size", "distance"] + .iter() + .filter(|key| params.iter().any(|p| p.key == **key)) + .map(|key| values.get(key).ceil().max(0.0) as u32) + .max() + .unwrap_or(0) + } fn apply( &self, pixels: &mut [f32], @@ -103,6 +119,12 @@ use schist_fx::{gaussian_rgba as gaussian_blur, premultiply, unpremultiply}; pub struct GaussianBlur; impl FilterPlugin for GaussianBlur { + /// Reads `radius` pixels outside what it writes, so a + /// selection blur can be handed the surrounding image + /// instead of clamping at the selection edge. + fn context(&self, values: &FilterValues) -> u32 { + values.get("radius").ceil().max(0.0) as u32 + } fn id(&self) -> &'static str { "filter.gaussian_blur" } @@ -131,6 +153,12 @@ impl FilterPlugin for GaussianBlur { pub struct BoxBlur; impl FilterPlugin for BoxBlur { + /// Reads `radius` pixels outside what it writes, so a + /// selection blur can be handed the surrounding image + /// instead of clamping at the selection edge. + fn context(&self, values: &FilterValues) -> u32 { + values.get("radius").ceil().max(0.0) as u32 + } fn id(&self) -> &'static str { "filter.box_blur" } @@ -744,3 +772,52 @@ mod tests { } } } + +#[cfg(test)] +mod context_tests { + use super::*; + + /// The buffer a filter is handed is exactly the region being + /// filtered and the kernels clamp at its edge, so a selection blur + /// repeated its boundary row outward instead of pulling in the real + /// pixels just outside it. A filter that reads its neighbours has to + /// say how far. + #[test] + fn filters_that_read_neighbours_advertise_their_reach() { + let blur = GaussianBlur; + let mut values = FilterValues::defaults(&blur.params()); + values.set("radius", 12.0); + assert_eq!(blur.context(&values), 12); + values.set("radius", 0.0); + assert_eq!(blur.context(&values), 0); + } + + /// The macro-built filters pick their reach up from whichever sizing + /// parameter they declare. + #[test] + fn a_radius_parameter_implies_the_reach() { + let registry = { + let mut r = schist_plugin_api::PluginRegistry::new(); + CoreFiltersPlugin.register(&mut r); + r + }; + // Maximum takes a radius and reads that far. + let max = registry + .filters() + .find(|f| f.id() == "filter.maximum") + .expect("maximum"); + let mut values = FilterValues::defaults(&max.params()); + values.set("radius", 7.0); + assert_eq!(max.context(&values), 7); + + // A per-pixel filter reads nothing around it. + let invert = registry + .filters() + .find(|f| f.id() == "filter.invert" || f.category() == "Adjust") + .or_else(|| registry.filters().find(|f| f.params().is_empty())); + if let Some(f) = invert { + let values = FilterValues::defaults(&f.params()); + assert_eq!(f.context(&values), 0, "{} should read nothing", f.id()); + } + } +} diff --git a/plugins/filters-core/src/other.rs b/plugins/filters-core/src/other.rs index a9b201dd..b5ccd1c1 100644 --- a/plugins/filters-core/src/other.rs +++ b/plugins/filters-core/src/other.rs @@ -3,6 +3,7 @@ use crate::util::{at, gaussian_rgba, premultiply, put, unpremultiply, value_noise}; use crate::{param, simple_filter}; +use rayon::prelude::*; use schist_plugin_api::{FilterParam, FilterPlugin, FilterValues}; simple_filter!( @@ -62,29 +63,89 @@ simple_filter!( /// Grey-level morphology: dilate (`max`) grows light areas, erode grows /// dark ones. Photoshop calls them Maximum and Minimum. +/// +/// The structuring element is a disc, which is not separable -- but it +/// decomposes into one horizontal line segment per row, and a 1-D +/// morphological pass over a line is O(1) per pixel with a monotonic +/// deque. So instead of scanning the whole disc per pixel (about 5000 +/// taps at radius 40, single-threaded, on every slider tick of a live +/// preview) this runs one horizontal pass per distinct row half-width +/// and folds the results together: 81 passes at radius 40 rather than +/// 5000 taps per pixel, and the rows within a pass go out to rayon. fn morph(px: &mut [f32], w: usize, h: usize, radius: i32, take_max: bool) { + if w == 0 || h == 0 || radius <= 0 { + return; + } let src = px.to_vec(); - for y in 0..h as i32 { - for x in 0..w as i32 { - let mut acc = if take_max { [0.0f32; 4] } else { [1.0f32; 4] }; - for dy in -radius..=radius { - for dx in -radius..=radius { - if dx * dx + dy * dy > radius * radius { - continue; - } - let p = at(&src, w, h, x + dx, y + dy); - for c in 0..4 { - acc[c] = if take_max { - acc[c].max(p[c]) - } else { - acc[c].min(p[c]) - }; + let pick = |a: f32, b: f32| if take_max { a.max(b) } else { a.min(b) }; + // Start from the disc's own centre row, which every pixel is in. + let mut out = horizontal_morph(&src, w, h, radius, take_max); + + let r2 = radius * radius; + for dy in 1..=radius { + // The disc's half-width at this row offset. + let hw = ((r2 - dy * dy) as f32).sqrt().floor() as i32; + if hw < 0 { + continue; + } + let band = horizontal_morph(&src, w, h, hw, take_max); + // The same band serves +dy and -dy: the disc is symmetric. + out.par_chunks_mut(w * 4).enumerate().for_each(|(y, row)| { + let up = (y as i32 - dy).clamp(0, h as i32 - 1) as usize; + let down = (y as i32 + dy).clamp(0, h as i32 - 1) as usize; + for (x, v) in row.iter_mut().enumerate() { + let i = x; + *v = pick(*v, pick(band[up * w * 4 + i], band[down * w * 4 + i])); + } + }); + } + px.copy_from_slice(&out); +} + +/// One row-wise morphological pass with a `2 * half + 1` wide window. +/// +/// A monotonic deque per channel keeps this O(1) per pixel however wide +/// the window is. +fn horizontal_morph(src: &[f32], w: usize, h: usize, half: i32, take_max: bool) -> Vec { + let mut out = vec![0f32; w * h * 4]; + let half = half.max(0) as usize; + out.par_chunks_mut(w * 4).enumerate().for_each(|(y, row)| { + let base = y * w * 4; + // Indices into the row, kept so their values are monotonic. A + // `VecDeque`, not a `Vec`: dropping from the front is what makes + // the window slide, and `Vec::remove(0)` shifts the whole thing + // each time, which is the O(window) cost this is here to avoid. + let mut deque: std::collections::VecDeque = + std::collections::VecDeque::with_capacity(2 * half + 2); + for c in 0..4 { + deque.clear(); + let value = |x: usize| src[base + x * 4 + c]; + let better = |a: f32, b: f32| if take_max { a >= b } else { a <= b }; + // Prime the window with everything left of the first + // output pixel's right edge. + let mut next = 0usize; + for x in 0..w { + let right = (x + half).min(w - 1); + while next <= right { + while deque + .back() + .is_some_and(|&i| !better(value(i), value(next))) + { + deque.pop_back(); } + deque.push_back(next); + next += 1; + } + // Drop anything that has fallen off the left edge. + let left = x.saturating_sub(half); + while deque.front().is_some_and(|&i| i < left) { + deque.pop_front(); } + row[x * 4 + c] = value(deque[0]); } - put(px, w, x as usize, y as usize, acc); } - } + }); + out } simple_filter!( @@ -461,3 +522,109 @@ pub fn register(registry: &mut schist_plugin_api::PluginRegistry) { registry.register_filter(Box::new(DustAndScratches)); registry.register_filter(Box::new(ReduceNoise)); } + +#[cfg(test)] +mod morph_tests { + use super::{horizontal_morph, morph}; + use crate::util::{at, put}; + + /// `morph` as it was written before the decomposition: a full disc + /// scan per pixel, about 5000 taps at radius 40. + fn brute_force(px: &mut [f32], w: usize, h: usize, radius: i32, take_max: bool) { + let src = px.to_vec(); + for y in 0..h as i32 { + for x in 0..w as i32 { + let mut acc = if take_max { [0.0f32; 4] } else { [1.0f32; 4] }; + for dy in -radius..=radius { + for dx in -radius..=radius { + if dx * dx + dy * dy > radius * radius { + continue; + } + let p = at(&src, w, h, x + dx, y + dy); + for c in 0..4 { + acc[c] = if take_max { + acc[c].max(p[c]) + } else { + acc[c].min(p[c]) + }; + } + } + } + put(px, w, x as usize, y as usize, acc); + } + } + } + + /// A field with isolated bright and dark specks, so a dilation and an + /// erosion both have something to spread. + fn field(w: usize, h: usize) -> Vec { + let mut px = vec![0.5f32; w * h * 4]; + for (i, v) in px.iter_mut().enumerate() { + let p = i / 4; + let (x, y) = (p % w, p / w); + *v = match (x * 7 + y * 13) % 11 { + 0 => 0.95, + 3 => 0.05, + _ => 0.4 + ((x + y) % 5) as f32 * 0.05, + }; + } + px + } + + #[test] + fn the_decomposed_disc_matches_a_full_disc_scan() { + let (w, h) = (37usize, 23usize); + for radius in [1, 2, 3, 5, 9] { + for take_max in [true, false] { + let mut fast = field(w, h); + let mut slow = fast.clone(); + morph(&mut fast, w, h, radius, take_max); + brute_force(&mut slow, w, h, radius, take_max); + for i in 0..fast.len() { + assert!( + (fast[i] - slow[i]).abs() < 1e-6, + "radius {radius}, max {take_max}, sample {i}: {} != {}", + fast[i], + slow[i] + ); + } + } + } + } + + /// A radius wider than the image still clamps at the edges rather + /// than reading out of bounds. + #[test] + fn a_radius_larger_than_the_image_is_fine() { + let (w, h) = (5usize, 4usize); + let mut fast = field(w, h); + let mut slow = fast.clone(); + morph(&mut fast, w, h, 12, true); + brute_force(&mut slow, w, h, 12, true); + for i in 0..fast.len() { + assert!((fast[i] - slow[i]).abs() < 1e-6, "sample {i}"); + } + } + + /// The 1-D pass is the piece everything else is built from. + #[test] + fn the_row_pass_is_a_sliding_window_extreme() { + let w = 8usize; + let mut src = vec![0f32; w * 4]; + for (x, v) in [0.1f32, 0.9, 0.2, 0.3, 0.05, 0.7, 0.4, 0.6] + .iter() + .enumerate() + { + for c in 0..4 { + src[x * 4 + c] = *v; + } + } + let out = horizontal_morph(&src, w, 1, 1, true); + // Each output is the max of the pixel and its neighbours, + // clamped at the ends. + let want = [0.9f32, 0.9, 0.9, 0.3, 0.7, 0.7, 0.7, 0.6]; + for (x, w_) in want.iter().enumerate() { + assert!((out[x * 4] - w_).abs() < 1e-6, "at {x}: {}", out[x * 4]); + } + } +} diff --git a/plugins/filters-core/src/stylize.rs b/plugins/filters-core/src/stylize.rs index cafc1ca0..42266fba 100644 --- a/plugins/filters-core/src/stylize.rs +++ b/plugins/filters-core/src/stylize.rs @@ -1,5 +1,7 @@ //! Filter ▸ Stylize: filters built on edges and local contrast. +use rayon::prelude::*; + use crate::util::{at, convolve3, gaussian_rgba, luma, put, value_noise}; use crate::{param, simple_filter}; use schist_plugin_api::{FilterParam, FilterPlugin, FilterValues}; @@ -9,18 +11,25 @@ fn edges(px: &[f32], w: usize, h: usize) -> Vec { let mut out = vec![0.0f32; w * h]; const GX: [f32; 9] = [-1.0, 0.0, 1.0, -2.0, 0.0, 2.0, -1.0, 0.0, 1.0]; const GY: [f32; 9] = [-1.0, -2.0, -1.0, 0.0, 0.0, 0.0, 1.0, 2.0, 1.0]; - for y in 0..h as i32 { - for x in 0..w as i32 { - let (mut gx, mut gy) = (0.0, 0.0); - for i in 0..9 { - let p = at(px, w, h, x + (i % 3) as i32 - 1, y + (i / 3) as i32 - 1); - let l = luma(&p); - gx += l * GX[i]; - gy += l * GY[i]; + // A pure gather from `px`, so the rows are independent. This backs + // Find Edges, Glowing Edges and Trace Contour, and re-ran on one core + // over the whole selection on every slider tick of a live preview. + out.par_chunks_mut(w.max(1)) + .enumerate() + .for_each(|(y, row)| { + let y = y as i32; + for (x, v) in row.iter_mut().enumerate() { + let x = x as i32; + let (mut gx, mut gy) = (0.0, 0.0); + for i in 0..9 { + let p = at(px, w, h, x + (i % 3) as i32 - 1, y + (i / 3) as i32 - 1); + let l = luma(&p); + gx += l * GX[i]; + gy += l * GY[i]; + } + *v = gx.hypot(gy); } - out[y as usize * w + x as usize] = gx.hypot(gy); - } - } + }); out } diff --git a/plugins/filters-core/src/util.rs b/plugins/filters-core/src/util.rs index 9e2c04be..b5d4806b 100644 --- a/plugins/filters-core/src/util.rs +++ b/plugins/filters-core/src/util.rs @@ -3,6 +3,8 @@ //! Everything here works on the same straight-alpha f32 RGBA buffer the //! `FilterPlugin` trait hands out: `width * height * 4` floats, row major. +use rayon::prelude::*; + /// Premultiplied-alpha conversion and the separable blur live in /// `schist_fx`, which is where the GPU seam is; re-exported so the filter /// modules keep a single import. @@ -53,18 +55,22 @@ pub fn put(px: &mut [f32], w: usize, x: usize, y: usize, v: [f32; 4]) { /// /// The workhorse for the distort filters: they differ only in the mapping. /// Sampling is done on premultiplied alpha so edges do not fringe. -pub fn warp(px: &mut [f32], w: usize, h: usize, map: impl Fn(f32, f32) -> (f32, f32)) { +pub fn warp(px: &mut [f32], w: usize, h: usize, map: impl Fn(f32, f32) -> (f32, f32) + Sync) { if w == 0 || h == 0 { return; } premultiply(px); let src = px.to_vec(); - for y in 0..h { + // A pure gather from an immutable `src`, so the rows are independent. + // These run on every slider tick of a live preview over the whole + // selection, and did it on one core. + px.par_chunks_mut(w * 4).enumerate().for_each(|(y, row)| { for x in 0..w { let (sx, sy) = map(x as f32 + 0.5, y as f32 + 0.5); - put(px, w, x, y, sample(&src, w, h, sx - 0.5, sy - 0.5)); + let v = sample(&src, w, h, sx - 0.5, sy - 0.5); + row[x * 4..x * 4 + 4].copy_from_slice(&v); } - } + }); unpremultiply(px); } @@ -74,7 +80,8 @@ pub fn convolve3(px: &mut [f32], w: usize, h: usize, k: [f32; 9], bias: f32) { return; } let src = px.to_vec(); - for y in 0..h as i32 { + px.par_chunks_mut(w * 4).enumerate().for_each(|(y, row)| { + let y = y as i32; for x in 0..w as i32 { let mut acc = [0.0f32; 3]; for (i, weight) in k.iter().enumerate() { @@ -84,20 +91,13 @@ pub fn convolve3(px: &mut [f32], w: usize, h: usize, k: [f32; 9], bias: f32) { } } let a = at(&src, w, h, x, y)[3]; - put( - px, - w, - x as usize, - y as usize, - [ - (acc[0] + bias).clamp(0.0, 1.0), - (acc[1] + bias).clamp(0.0, 1.0), - (acc[2] + bias).clamp(0.0, 1.0), - a, - ], - ); + let out = &mut row[x as usize * 4..x as usize * 4 + 4]; + out[0] = (acc[0] + bias).clamp(0.0, 1.0); + out[1] = (acc[1] + bias).clamp(0.0, 1.0); + out[2] = (acc[2] + bias).clamp(0.0, 1.0); + out[3] = a; } - } + }); } /// A cheap, repeatable value-noise field. diff --git a/plugins/tools-retouch/Cargo.toml b/plugins/tools-retouch/Cargo.toml index 8241f475..e771d9ee 100644 --- a/plugins/tools-retouch/Cargo.toml +++ b/plugins/tools-retouch/Cargo.toml @@ -8,3 +8,4 @@ license.workspace = true schist-core.workspace = true schist-color.workspace = true schist-plugin-api.workspace = true +rayon.workspace = true diff --git a/plugins/tools-retouch/src/lib.rs b/plugins/tools-retouch/src/lib.rs index 27e78659..6b53bd89 100644 --- a/plugins/tools-retouch/src/lib.rs +++ b/plugins/tools-retouch/src/lib.rs @@ -9,6 +9,7 @@ //! right for smooth surroundings and visibly blurry over texture, which is //! at least a predictable failure. +use rayon::prelude::*; use schist_color::Rgba; use schist_core::{Document, IntRect, LayerId, Selection, TileCoord, TileMap, TILE_SIZE}; use schist_plugin_api::{ @@ -122,8 +123,15 @@ pub fn inpaint(tiles: &TileMap, rect: IntRect, hole: &[bool]) -> Vec { let passes = (w.min(h) as u32).clamp(8, 160); let mut next = buf.clone(); for _ in 0..passes { - for y in 0..h { - for x in 0..w { + // A row at a time, in parallel. This runs synchronously on + // pointer release: over a 1500x1500 selection it is 160 sweeps of + // 2.25 M pixels, and the window locked up for seconds with no + // cursor change to explain it. Each row reads `buf` and writes + // only its own slice of `next`, so the passes stay exactly the + // Jacobi iteration they were -- same output, spread over the + // cores. + next.par_chunks_mut(w).enumerate().for_each(|(y, out_row)| { + for (x, out) in out_row.iter_mut().enumerate() { let i = y * w + x; if !hole[i] { continue; @@ -142,10 +150,10 @@ pub fn inpaint(tiles: &TileMap, rect: IntRect, hole: &[bool]) -> Vec { n += 1.0; } if n > 0.0 { - next[i] = Rgba::new(acc[0] / n, acc[1] / n, acc[2] / n, acc[3] / n); + *out = Rgba::new(acc[0] / n, acc[1] / n, acc[2] / n, acc[3] / n); } } - } + }); std::mem::swap(&mut buf, &mut next); } buf diff --git a/plugins/tools-retouch/tests/retouch.rs b/plugins/tools-retouch/tests/retouch.rs index 7f8d619d..1ba6724b 100644 --- a/plugins/tools-retouch/tests/retouch.rs +++ b/plugins/tools-retouch/tests/retouch.rs @@ -2,7 +2,7 @@ //! rest alone. use schist_color::{Depth, Rgba}; -use schist_core::{Document, IntRect, Layer, SelectOp, TileCoord, TILE_SIZE}; +use schist_core::{Document, IntRect, Layer, SelectOp, TileCoord, TileMap, TILE_SIZE}; use schist_plugin_api::{EditorState, Modifiers, PointerInput, ToolCtx, ToolPlugin}; use schist_tools_retouch::*; @@ -211,3 +211,108 @@ fn patch_takes_texture_from_the_source_and_colour_from_the_destination() { "took the source's brightness instead of the destination's: {patched:?}" ); } + +/// Parallelising the inpaint must not change what it produces. +/// +/// It is 160 sweeps of the padded selection bounds, run synchronously on +/// pointer release: over a 1500x1500 selection the window locked up for +/// seconds. Each row now reads the previous pass and writes only its own +/// slice, so the iteration is unchanged — only spread over the cores. +#[test] +fn the_inpaint_matches_a_sequential_jacobi_solve() { + let mut tiles = TileMap::default(); + let rect = IntRect::from_xywh(0, 0, 40, 40); + for y in 0..40 { + for x in 0..40 { + let v = (x + y) as f32 / 80.0; + let coord = TileCoord::containing(x, y); + let trect = coord.rect(); + let buf = tiles.get_mut_or_insert(coord, Depth::Eight); + buf.set( + ((y - trect.top) * TILE_SIZE + (x - trect.left)) as usize, + Rgba::new(v, 1.0 - v, 0.5, 1.0), + ); + } + } + // A square hole in the middle. + let hole: Vec = (0..40 * 40) + .map(|i| { + let (x, y) = (i % 40, i / 40); + (12..28).contains(&x) && (12..28).contains(&y) + }) + .collect(); + + let got = schist_tools_retouch::inpaint(&tiles, rect, &hole); + let want = sequential_inpaint(&tiles, rect, &hole); + assert_eq!(got.len(), want.len()); + for (i, (g, w)) in got.iter().zip(&want).enumerate() { + assert!( + (g.r - w.r).abs() < 1e-5 && (g.g - w.g).abs() < 1e-5 && (g.b - w.b).abs() < 1e-5, + "pixel {i}: {g:?} != {w:?}" + ); + } + // And it actually filled the hole with something plausible. + let centre = got[20 * 40 + 20]; + assert!(centre.a > 0.99 && centre.r > 0.0); +} + +/// The inpaint's inner loop, written the way it was before rayon. +fn sequential_inpaint(tiles: &TileMap, rect: IntRect, hole: &[bool]) -> Vec { + let (w, h) = (rect.width() as usize, rect.height() as usize); + let mut buf: Vec = (0..w * h) + .map(|i| tiles.pixel(rect.left + (i % w) as i32, rect.top + (i / w) as i32)) + .collect(); + // Seed the hole with the mean of everything outside it, as `inpaint` + // does. + let mut acc = [0f32; 4]; + let mut n = 0f32; + for (i, px) in buf.iter().enumerate() { + if !hole[i] { + acc[0] += px.r; + acc[1] += px.g; + acc[2] += px.b; + acc[3] += px.a; + n += 1.0; + } + } + let seed = if n > 0.0 { + Rgba::new(acc[0] / n, acc[1] / n, acc[2] / n, acc[3] / n) + } else { + Rgba::new(0.0, 0.0, 0.0, 0.0) + }; + for i in 0..buf.len() { + if hole[i] { + buf[i] = seed; + } + } + let passes = (w.min(h) as u32).clamp(8, 160); + let mut next = buf.clone(); + for _ in 0..passes { + for y in 0..h { + for x in 0..w { + let i = y * w + x; + if !hole[i] { + continue; + } + let (mut acc, mut n) = ([0f32; 4], 0f32); + for (dx, dy) in [(1i32, 0i32), (-1, 0), (0, 1), (0, -1)] { + let (sx, sy) = (x as i32 + dx, y as i32 + dy); + if sx < 0 || sy < 0 || sx as usize >= w || sy as usize >= h { + continue; + } + let c = buf[sy as usize * w + sx as usize]; + acc[0] += c.r; + acc[1] += c.g; + acc[2] += c.b; + acc[3] += c.a; + n += 1.0; + } + if n > 0.0 { + next[i] = Rgba::new(acc[0] / n, acc[1] / n, acc[2] / n, acc[3] / n); + } + } + } + std::mem::swap(&mut buf, &mut next); + } + buf +} diff --git a/plugins/tools-select/src/lib.rs b/plugins/tools-select/src/lib.rs index 4c7f95a3..1df57f07 100644 --- a/plugins/tools-select/src/lib.rs +++ b/plugins/tools-select/src/lib.rs @@ -19,7 +19,15 @@ fn commit_pixels(ctx: &mut ToolCtx, pixels: &[(i32, i32)], op: SelectOp, name: & return; } let mut edit = ctx.doc.begin_edit(name.to_string()); - edit.change_selection(|sel, _| { + edit.change_selection(|sel, _| apply_pixels(sel, pixels, op)); + edit.commit(); +} + +/// Fold a set of pixels into a selection. No history of its own, so a +/// tool that touches the selection many times in one gesture can record a +/// single edit at the end of it. +fn apply_pixels(sel: &mut Selection, pixels: &[(i32, i32)], op: SelectOp) { + { if op == SelectOp::Replace { sel.deselect(); } @@ -57,10 +65,9 @@ fn commit_pixels(ctx: &mut ToolCtx, pixels: &[(i32, i32)], op: SelectOp, name: & } } } - sel.activate(); - sel.recompute_bounds(); - }); - edit.commit(); + } + sel.activate(); + sel.recompute_bounds(); } /// The active layer's pixels, if it has any. @@ -498,8 +505,14 @@ impl ToolPlugin for LassoTool { self.cursor = Some(p); match self.kind { LassoKind::Free => { - if !self.points.is_empty() { - self.points.push(p); + // Decimate on capture. A point per pointer-move meant a + // leisurely trace round a moderate region collected + // thousands of them, and every one is an edge the fill + // has to consider. Sub-pixel steps carry no shape. + if let Some(&last) = self.points.last() { + if (last.0 - p.0).hypot(last.1 - p.1) >= 1.0 { + self.points.push(p); + } } } LassoKind::Polygonal => {} @@ -709,11 +722,15 @@ pub struct QuickSelectTool { /// Everything the current stroke has selected, so each dab extends the /// same region rather than restarting. stroke: std::collections::HashSet<(i32, i32)>, + /// The selection as it stood before the drag started, so the whole + /// drag can be recorded as one undoable edit. + original: Option, } impl QuickSelectTool { fn new() -> Self { QuickSelectTool { + original: None, radius: 20.0, tolerance: 28.0, subtract: false, @@ -787,6 +804,18 @@ impl QuickSelectTool { } } +impl QuickSelectTool { + /// Fold a dab into the live selection without touching history. + fn apply_live(&self, ctx: &mut ToolCtx, pixels: &[(i32, i32)], op: SelectOp) { + if pixels.is_empty() { + return; + } + apply_pixels(&mut ctx.doc.selection, pixels, op); + let canvas = ctx.doc.canvas_rect(); + ctx.doc.add_damage(canvas); + } +} + impl ToolPlugin for QuickSelectTool { fn id(&self) -> &'static str { "quick_select" @@ -825,6 +854,13 @@ impl ToolPlugin for QuickSelectTool { self.seed = [0.0; 3]; self.seen = 0; self.stroke.clear(); + // One drag is one edit. Every dab used to call `commit_pixels`, + // which deep-clones the whole selection twice for its before/after + // snapshot -- so a single drag left dozens to hundreds of "Quick + // Selection" entries, needing that many undos to take back and + // blowing past the 200-entry limit, which discards everything the + // user did before it. + self.original = Some(ctx.doc.selection.clone()); let added = self.grow(ctx.doc, input.x as i32, input.y as i32); // A plain drag starts a new selection; shift extends the old one. let op = if self.subtract { @@ -834,7 +870,7 @@ impl ToolPlugin for QuickSelectTool { } else { SelectOp::Replace }; - commit_pixels(ctx, &added, op, "Quick Selection"); + self.apply_live(ctx, &added, op); } fn on_pointer_move(&mut self, ctx: &mut ToolCtx, input: PointerInput) { @@ -847,17 +883,31 @@ impl ToolPlugin for QuickSelectTool { } else { SelectOp::Add }; - commit_pixels(ctx, &added, op, "Quick Selection"); + self.apply_live(ctx, &added, op); } - fn on_pointer_up(&mut self, _ctx: &mut ToolCtx, _input: PointerInput) { + fn on_pointer_up(&mut self, ctx: &mut ToolCtx, _input: PointerInput) { self.dragging = false; self.stroke.clear(); + let Some(original) = self.original.take() else { + return; + }; + let drawn = ctx.doc.selection.clone(); + // Put the pre-drag selection back so the edit records the right + // "before", then replay the whole drag as one change. + ctx.doc.selection = original; + let mut edit = ctx.doc.begin_edit("Quick Selection"); + edit.change_selection(|sel, _| *sel = drawn); + edit.commit(); } - fn on_cancel(&mut self, _ctx: &mut ToolCtx) { + fn on_cancel(&mut self, ctx: &mut ToolCtx) { self.dragging = false; self.stroke.clear(); + if let Some(original) = self.original.take() { + ctx.doc.selection = original; + ctx.doc.damage_all(); + } } } @@ -1315,6 +1365,54 @@ mod tests { "blue side not selected" ); } + + /// One drag, one undo step. + /// + /// Every dab used to call `commit_pixels`, which deep-clones the + /// whole selection twice for its before/after snapshot — so a single + /// drag left dozens to hundreds of "Quick Selection" entries, needing + /// that many undos to take back, and blew past the 200-entry history + /// limit, discarding everything the user did before it. + #[test] + fn a_quick_selection_drag_is_one_undo_step() { + let mut doc = Document::new("t", 200, 200, Depth::Eight); + let mut layer = Layer::new_raster("bg"); + let buf = [40u8, 40, 40, 255].repeat(200 * 200); + blit_rgba8( + &mut layer.as_raster_mut().unwrap().tiles, + Depth::Eight, + IntRect::from_size(200, 200), + &buf, + ); + doc.push_layer(layer); + let mut state = EditorState::default(); + let mut tool = QuickSelectTool::new(); + let mut ctx = ToolCtx { + doc: &mut doc, + state: &mut state, + }; + + tool.on_pointer_down(&mut ctx, input(40.0, 40.0, Modifiers::default())); + for i in 1..=20 { + tool.on_pointer_move( + &mut ctx, + input(40.0 + i as f32 * 3.0, 40.0, Modifiers::default()), + ); + } + // The selection is live during the drag, before anything is + // recorded. + assert!(!ctx.doc.selection.is_empty()); + assert!(!ctx.doc.history.can_undo(), "the drag recorded mid-gesture"); + + tool.on_pointer_up(&mut ctx, input(100.0, 40.0, Modifiers::default())); + assert_eq!(ctx.doc.history.undo_name(), Some("Quick Selection")); + + doc.undo(); + assert!(!doc.history.can_undo(), "one drag left more than one entry"); + // `coverage` reports 255 everywhere when nothing is selected, so + // emptiness is the thing to check. + assert!(doc.selection.is_empty(), "undo left the selection behind"); + } } #[cfg(test)]