collective
Browse / github.com/storytold/photocraft / patch
perf(selection): combine GRAY8 masks directly in tiles
perf(selection): combine GRAY8 masks directly in tiles Add, Subtract and Intersect read the old selection into a dense `Vec<f32>` before combining it with a new mask. `combine_band` then read the result back into another float buffer and wrote it into the old surface. At 6000×6000, each dense float plane is 144 MB even though selections are stored as GRAY8. ## Change - Combine GRAY8 coverage directly in parallel tiles: Add takes the maximum byte, Subtract uses saturating subtraction, and Intersect takes the minimum. Float inputs are clamped and rounded once at the tile boundary, preserving the stored coverage. - Move band combining into `photocr
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- wanikhawar (imported from pull request #9822, open)
- Base
7ac7b8b106acccf6826d45ed5c9e481dfe341035- Size
- +337 −12 in 5 files
- Review
- pending
- Shared
- 2026-10-28
- Id
337f040f73b85fad44848cdd
How it went
No other operator has reported using it yet.
Diff
raw · .github/workflows/ci.yml crates/algo/src/selection.rs crates/algo/src/selection/combine_mask.rs crates/algo/src/selection/combine_mask/tests.rs crates/engine/src/cmd/selection_cmds.rs
@@ -11,6 +11,10 @@ on: types: [opened, synchronize, reopened, labeled] workflow_dispatch: inputs:+ selection_combining:+ description: "Compare direct GRAY8 selection combining on 36 MP (Linux, exact outputs)"+ type: boolean+ default: false selection_morphology: description: "Compare selection morphology on 36 MP (Linux, exact outputs)" type: boolean@@ -459,7 +463,7 @@ jobs: path: selection-morphology-perf.txt selection-morphology-quick-perf:- if: inputs.selection_morphology == true+ if: inputs.selection_morphology == true || inputs.selection_combining == true # Match perf-nightly.yml: Linux llvmpipe lacks the float render targets needed by # bevel_bench, so its required GPU rows cannot be measured there. runs-on: macos-15@@ -481,3 +485,27 @@ jobs: path: | target/perf/*.json target/perf/summary.md++ selection-combining:+ if: inputs.selection_combining == true+ runs-on: ubuntu-latest+ timeout-minutes: 30+ env:+ RAYON_NUM_THREADS: "4"+ steps:+ - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1+ with:+ persist-credentials: false+ - uses: dtolnay/rust-toolchain@b725eda525362e7cf34af9a7243cac1e3f01a7ec # stable+ - uses: Swatinem/rust-cache@d1527c8f67ef9af357290475d4af4b0fab8f12e0 # v2.9.3+ with:+ key: selection-combining-perf+ - name: Compare exact outputs and release timings on 36 MP+ run: |+ set -o pipefail+ cargo test --release -p photocraft-algo --lib gray8_combining_release_comparison -- --ignored --nocapture | tee selection-combining-perf.txt+ - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2+ if: always()+ with:+ name: selection-combining-performance+ path: selection-combining-perf.txt@@ -134,18 +134,30 @@ pub fn mask_to_surface(m: &[f32], area: Rect) -> Surface { surface_from_masks(area, m, None, |_, v| v) } +mod combine_mask;+pub use combine_mask::combine_band;+ /// Combines `new` with `old` by `mode`. Returns `None` when nothing is selected. pub fn combine(old: Option<&Surface>, new: &[f32], area: Rect, mode: SelectionMode) -> Option<Surface> { if area.is_empty() { return None; }+ // Replace already writes straight from the input mask to tiles; it needs no old mask.+ if mode == SelectionMode::Replace || (old.is_none() && mode == SelectionMode::Add) {+ let s = mask_to_surface(new, area);+ return (s.tile_count() > 0).then_some(s);+ }+ let old = old?;+ if old.format() == PixelFormat::GRAY8 {+ return combine_mask::gray8(Some(old), new, area, mode, false);+ } // Quantize to the 8-bit grid selections are stored on, so combining a mask with itself is // exact. Below half an 8-bit step is nothing. let q = |v: f32| f32::from(coverage8(v)) / 255.0; let s = match mode { SelectionMode::Replace => surface_from_masks(area, new, None, |_, v| v), _ => {- let prev = mask_from_surface(old, area);+ let prev = mask_from_surface(Some(old), area); match mode { SelectionMode::Add => surface_from_masks(area, new, Some(&prev), |p, v| p.max(q(v))), SelectionMode::Subtract => surface_from_masks(area, new, Some(&prev), |p, v| (p - q(v)).max(0.0)),@@ -381,6 +393,9 @@ pub fn combine_region(old: Option<&Surface>, new: Option<&Region>, mode: Selecti let empty = Region { bbox: Rect::EMPTY, mask: Vec::new() }; let new = new.unwrap_or(&empty); let b = new.bbox;+ if old.is_none_or(|s| s.format() == PixelFormat::GRAY8) {+ return combine_mask::gray8_bytes(old, &new.mask, b, mode);+ } let prev_u8 = |s: &Surface| -> Vec<u8> { let mut v = Vec::new(); s.read_region_into(b, &mut v);new file mode 100644@@ -0,0 +1,112 @@+//! Float mask inputs meet stored GRAY8 selections at the tile boundary. No dense copy of+//! the previous selection or of the combined result is needed.++use super::{SelectionMode, combine, coverage8};+use crate::photo_util::par_map;+use photocraft_color::PixelFormat;+use photocraft_geom::{Rect, TILE_SIZE, TileCoord};+use photocraft_raster::{Surface, Tile};+use std::sync::Arc;++/// Combines a mask covering only `band` (zero elsewhere). Add and Subtract retain the old+/// selection outside it, including pixels sharing a tile with the band's edges. Replace+/// and Intersect return only the selection inside the band. Missing mask samples read zero.+pub fn combine_band(old: Option<&Surface>, mask: &[f32], band: Rect, mode: SelectionMode) -> Option<Surface> {+ let keep = matches!(mode, SelectionMode::Add | SelectionMode::Subtract);+ if !keep || old.is_none() {+ return combine(old, mask, band, mode);+ }+ if band.is_empty() {+ return old.cloned();+ }+ if old.is_none_or(|s| s.format() == PixelFormat::GRAY8) {+ return gray8(old, mask, band, mode, true);+ }+ // Preserve the format and conversion behavior of unusual selection surfaces. The normal+ // GRAY8 path above never materializes these band-sized buffers.+ let inside = combine(old, mask, band, mode);+ let mut out = old.cloned()?;+ let values = inside.map_or_else(|| vec![0.0; band.width() as usize * band.height() as usize], |s| s.read_region(band));+ if out.channels() == 1 {+ out.write_region(band, &values);+ } else {+ // A selection uses its first channel as coverage. Broadcast it when writing back to+ // an unusual multichannel surface, rather than passing a short buffer to write_region.+ let pixels: Vec<f32> = values.iter().flat_map(|v| std::iter::repeat_n(*v, out.channels())).collect();+ out.write_region(band, &pixels);+ }+ out.prune();+ (out.tile_count() > 0).then_some(out)+}++pub(super) fn gray8(old: Option<&Surface>, mask: &[f32], area: Rect, mode: SelectionMode, keep_outside: bool) -> Option<Surface> {+ build(old, area, mode, keep_outside, |i| coverage8(mask.get(i).copied().unwrap_or(0.0)))+}++pub(super) fn gray8_bytes(old: Option<&Surface>, mask: &[u8], area: Rect, mode: SelectionMode) -> Option<Surface> {+ let keep = matches!(mode, SelectionMode::Add | SelectionMode::Subtract);+ if area.is_empty() {+ return if keep { old.cloned().filter(|s| !s.content_bounds().is_empty()) } else { None };+ }+ build(old, area, mode, keep, |i| mask.get(i).copied().unwrap_or(0))+}++fn build(old: Option<&Surface>, area: Rect, mode: SelectionMode, keep_outside: bool, sample: impl Fn(usize) -> u8 + Sync) -> Option<Surface> {+ let mut out = if keep_outside { old.cloned().unwrap_or_else(|| Surface::new(PixelFormat::GRAY8)) } else { Surface::new(PixelFormat::GRAY8) };+ let default = old.and_then(|s| s.default_bytes().first()).copied().unwrap_or(0);+ let out_default = out.default_bytes().first().copied().unwrap_or(0);+ let (blank, full) = (out.solid_tile(&[0.0]), out.solid_tile(&[1.0]));+ let untouched = out.solid_tile(&[f32::from(out_default) / 255.0]);+ let coords: Vec<TileCoord> = area.tiles().collect();+ let width = area.width() as usize;+ let ts = TILE_SIZE as usize;+ let built = par_map(coords.len(), |k| {+ let tc = *coords.get(k)?;+ let rect = tc.rect();+ let cut = rect.intersect(&area);+ let previous = old.and_then(|s| s.tile(tc));+ let mut tile = Tile::clone(if keep_outside { previous.unwrap_or(&untouched) } else { &blank });+ for y in cut.y0..cut.y1 {+ let offset = y.abs_diff(rect.y0) as usize * ts + cut.x0.abs_diff(rect.x0) as usize;+ let len = cut.width() as usize;+ let Some(dst) = tile.bytes_mut().get_mut(offset..offset + len) else { continue };+ let src = previous.and_then(|t| t.bytes().get(offset..offset + len));+ let start = (y.abs_diff(area.y0) as usize).checked_mul(width).and_then(|i| i.checked_add(cut.x0.abs_diff(area.x0) as usize));+ for (i, d) in dst.iter_mut().enumerate() {+ let new = start.and_then(|j| j.checked_add(i)).map(&sample).unwrap_or(0);+ let prev = src.and_then(|s| s.get(i)).copied().unwrap_or(default);+ *d = match mode {+ SelectionMode::Replace => new,+ SelectionMode::Add => prev.max(new),+ SelectionMode::Subtract => prev.saturating_sub(new),+ SelectionMode::Intersect => prev.min(new),+ };+ }+ }+ let (lo, hi) = tile.bytes().iter().fold((u8::MAX, 0u8), |(lo, hi), b| (lo.min(*b), hi.max(*b)));+ let tile = if lo == out_default && hi == out_default {+ None+ } else if previous.is_some_and(|p| p.bytes() == tile.bytes()) {+ previous.cloned()+ } else if lo == u8::MAX {+ Some(Arc::clone(&full))+ } else {+ Some(Arc::new(tile))+ };+ Some((tc, tile))+ });+ for (tc, tile) in built.into_iter().flatten() {+ match tile {+ Some(tile) => out.put_tiles([(tc, tile)]),+ None if keep_outside && out.tile(tc).is_some() => drop(out.take_tiles(tc.rect())),+ None => {}+ }+ }+ if keep_outside {+ out.prune();+ }+ (out.tile_count() > 0).then_some(out)+}++#[cfg(test)]+mod tests;new file mode 100644@@ -0,0 +1,179 @@+use super::*;+use crate::selection::{Region, combine_region, mask_from_surface, surface_from_masks};+use photocraft_color::{ColorMode, SampleType};++const MODES: [SelectionMode; 4] = [SelectionMode::Replace, SelectionMode::Add, SelectionMode::Subtract, SelectionMode::Intersect];++// The pre-refactor float formulas, including quantization before the binary operation.+fn reference(old: Option<&Surface>, mask: &[f32], area: Rect, mode: SelectionMode) -> Option<Surface> {+ if area.is_empty() {+ return None;+ }+ if mode == SelectionMode::Replace {+ let s = surface_from_masks(area, mask, None, |_, v| v);+ return (s.tile_count() > 0).then_some(s);+ }+ let prev = mask_from_surface(old, area);+ let s = surface_from_masks(area, mask, Some(&prev), |p, v| {+ let q = f32::from(coverage8(v)) / 255.0;+ match mode {+ SelectionMode::Replace => v,+ SelectionMode::Add => p.max(q),+ SelectionMode::Subtract => (p - q).max(0.0),+ SelectionMode::Intersect => p.min(q),+ }+ });+ (s.tile_count() > 0).then_some(s)+}++fn reference_band(old: Option<&Surface>, mask: &[f32], area: Rect, mode: SelectionMode) -> Option<Surface> {+ let inside = reference(old, mask, area, mode);+ let Some(old) = old.filter(|_| matches!(mode, SelectionMode::Add | SelectionMode::Subtract)) else { return inside };+ if area.is_empty() {+ return Some(old.clone());+ }+ let values = inside.map_or_else(|| vec![0.0; area.width() as usize * area.height() as usize], |s| s.read_region(area));+ let mut out = old.clone();+ out.write_region(area, &values);+ out.prune();+ (out.tile_count() > 0).then_some(out)+}++#[test]+fn every_byte_pair_matches_float_combining() {+ let area = Rect::new(0, 0, 256, 256);+ let prev: Vec<f32> = (0..65536).map(|i| (i / 256) as f32 / 255.0).collect();+ let mask: Vec<f32> = (0..65536).map(|i| (i % 256) as f32 / 255.0).collect();+ let old = crate::selection::mask_to_surface(&prev, area);+ for mode in MODES {+ assert_eq!(combine(Some(&old), &mask, area, mode), reference(Some(&old), &mask, area, mode), "{mode:?}");+ }+}++#[test]+fn cropped_bands_match_dense_writeback_with_defaults_and_short_masks() {+ let values = [f32::NAN, f32::NEG_INFINITY, -0.2, 0.0, 0.49 / 255.0, 0.5 / 255.0, 0.51 / 255.0, 0.5, 1.0, 1.2, f32::INFINITY];+ for area in [Rect::new(-259, -2, 259, 3), Rect::new(255, 255, 258, 259), Rect::new(0, 0, 512, 256), Rect::EMPTY] {+ let n = area.width() as usize * area.height() as usize;+ let mask: Vec<f32> = (0..n).map(|i| values[i % values.len()]).collect();+ for default in [0.0, 0.5, 1.0] {+ let mut old = Surface::with_default(PixelFormat::GRAY8, &[default]);+ old.fill_rect(Rect::new(-300, -20, 300, 270), &[0.75]);+ old.write_pixel(1024, 1024, &[1.0]);+ old.write_pixel(-257, 0, &[0.0]);+ let snapshot = old.clone();+ for prev in [None, Some(&old)] {+ for len in [0, n / 2, n] {+ for mode in MODES {+ let mask = &mask[..len];+ assert_eq!(combine(prev, mask, area, mode), reference(prev, mask, area, mode), "{area:?} {mode:?} {default} {len}");+ assert_eq!(combine_band(prev, mask, area, mode), reference_band(prev, mask, area, mode), "band {area:?} {mode:?} {default} {len}");+ }+ }+ }+ assert_eq!(old, snapshot, "combining must leave undo snapshots unchanged");+ }+ }+}++#[test]+fn unusual_selection_formats_keep_their_float_behavior() {+ let area = Rect::new(-2, -1, 3, 2);+ for sample in SampleType::ALL {+ for color in [ColorMode::Grayscale, ColorMode::Rgb] {+ let mut old = Surface::with_default(PixelFormat::new(color, sample, false), &vec![0.123; if color == ColorMode::Rgb { 3 } else { 1 }]);+ old.write_pixel(0, 0, &vec![0.754; old.channels()]);+ for mode in MODES {+ let mask = [0.5; 15];+ assert_eq!(combine(Some(&old), &mask, area, mode), reference(Some(&old), &mask, area, mode));+ if color == ColorMode::Grayscale {+ assert_eq!(combine_band(Some(&old), &mask, area, mode), reference_band(Some(&old), &mask, area, mode));+ } else {+ let out = combine_band(Some(&old), &mask, area, mode).unwrap();+ let inside = reference(Some(&old), &mask, area, mode).unwrap();+ let expected = inside.pixel(0, 0)[0];+ let encoded = Surface::with_default(out.format(), &vec![expected; out.channels()]);+ assert_eq!(out.pixel(0, 0), encoded.pixel(0, 0));+ if matches!(mode, SelectionMode::Add | SelectionMode::Subtract) {+ assert_eq!(out.pixel(20, 20), old.pixel(20, 20));+ }+ }+ }+ }+ }+}++#[test]+fn region_bytes_match_float_bands() {+ let bbox = Rect::new(-257, 255, 258, 258);+ let mask: Vec<u8> = (0..bbox.width() * bbox.height()).map(|i| (i % 256) as u8).collect();+ let floats: Vec<f32> = mask.iter().map(|b| f32::from(*b) / 255.0).collect();+ let region = Region { bbox, mask };+ let mut old = Surface::new(PixelFormat::GRAY8);+ old.fill_rect(Rect::new(-300, 250, 300, 270), &[0.75]);+ for prev in [None, Some(&old)] {+ for mode in MODES {+ assert_eq!(combine_region(prev, Some(®ion), mode), reference_band(prev, &floats, bbox, mode));+ assert_eq!(combine_region(prev, None, mode), reference_band(prev, &[], Rect::EMPTY, mode));+ }+ }+}++#[test]+fn tiles_stay_shared_or_are_pruned() {+ let area = Rect::new(0, 0, 768, 256);+ let mut old = Surface::new(PixelFormat::GRAY8);+ old.fill_rect(area, &[1.0]);+ let tc = TileCoord::new(0, 0);+ let unchanged = combine(Some(&old), &[], area, SelectionMode::Add).unwrap();+ assert!(Arc::ptr_eq(unchanged.tile(tc).unwrap(), old.tile(tc).unwrap()));+ let band = Rect::new(257, 1, 260, 3);+ let changed = combine_band(Some(&old), &[1.0; 6], band, SelectionMode::Subtract).unwrap();+ assert!(Arc::ptr_eq(changed.tile(tc).unwrap(), old.tile(tc).unwrap()));+ assert_eq!(changed.pixel(256, 1), vec![1.0]);+ assert_eq!(changed.pixel(257, 1), vec![0.0]);+ assert_eq!(changed.pixel(260, 1), vec![1.0]);+ let full = combine(None, &vec![1.0; 768 * 256], area, SelectionMode::Replace).unwrap();+ assert!(Arc::ptr_eq(full.tile(tc).unwrap(), full.tile(TileCoord::new(1, 0)).unwrap()));+ assert!(combine_band(Some(&old), &vec![1.0; 768 * 256], area, SelectionMode::Subtract).is_none());+ assert!(combine(None, &[], area, SelectionMode::Replace).is_none());+}++/// Run only in release on CI: compares the former implementation, including dense reads and+/// band writeback, against the byte path with identical 36 MP inputs and exact output checks.+#[test]+#[ignore = "36 MP release performance comparison; run on GitHub Actions"]+fn gray8_combining_release_comparison() {+ use std::{hint::black_box, time::Instant};+ let area = Rect::new(0, 0, 6000, 6000);+ let mut old = Surface::new(PixelFormat::GRAY8);+ old.fill_rect(Rect::new(100, 100, 5900, 5900), &[0.75]);+ println!("36 MP; median of three release runs including allocation; exact output checks; previous dense mask alone = 144000000 bytes");+ for (case, size) in [("sparse", 50), ("dense", 4000), ("full", 6000)] {+ let mut mask = vec![0.0; 6000 * 6000];+ let start = (6000 - size) / 2;+ for row in mask.as_chunks_mut::<6000>().0.iter_mut().skip(start).take(size) {+ row[start..start + size].fill(0.5);+ }+ for mode in MODES {+ for band in [false, true] {+ let baseline = if band { reference_band } else { reference };+ let direct = if band { combine_band } else { combine };+ assert_eq!(direct(Some(&old), &mask, area, mode), baseline(Some(&old), &mask, area, mode));+ let mut medians = [0.0; 2];+ for (median, op) in medians.iter_mut().zip([baseline, direct]) {+ let mut runs = [0.0; 3];+ for ms in &mut runs {+ let start = Instant::now();+ let out = black_box(op(Some(black_box(&old)), black_box(&mask), area, mode));+ *ms = start.elapsed().as_secs_f64() * 1000.0;+ drop(out);+ }+ runs.sort_by(f64::total_cmp);+ *median = runs[1];+ }+ println!("{case} {mode:?} band={band}: old={:.2}ms new={:.2}ms speedup={:.2}x exact=true", medians[0], medians[1], medians[0] / medians[1]);+ }+ }+ }+}@@ -656,16 +656,7 @@ fn polygon_rows(pts: &[(f32, f32)], area: Rect) -> Rect { /// whole-canvas combine gives: Replace and Intersect are empty outside the band, and Add and /// Subtract keep the old selection there. pub(crate) fn combine_band(old: Option<&Surface>, mask: &[f32], band: Rect, m: SelectionMode) -> Option<Surface> {- let inside = sel::combine(old, mask, band, m);- let Some(old) = old.filter(|_| matches!(m, SelectionMode::Add | SelectionMode::Subtract)) else { return inside };- if band.is_empty() {- return Some(old.clone());- }- let values = inside.map_or_else(|| vec![0.0; band.width() as usize * band.height() as usize], |w| w.read_region(band));- let mut s = old.clone();- s.write_region(band, &values);- s.prune();- (s.tile_count() > 0).then_some(s)+ sel::combine_band(old, mask, band, m) } /// Keep dense feather masks local to the drawn shape. Clip before inflating: pixels outside