diff --git a/crates/codecs/src/codecs/png.rs b/crates/codecs/src/codecs/png.rs index 7c1a633d17..34ae84acc0 100644 --- a/crates/codecs/src/codecs/png.rs +++ b/crates/codecs/src/codecs/png.rs @@ -505,8 +505,15 @@ fn filter_row(row: &[u8], prev: Option<&[u8]>, bpp: usize, scratch: &mut [u8], o } }; let cost = |v: &[u8]| v.iter().map(|&b| u64::from((b as i8).unsigned_abs())).sum::(); - let mut best = (cost(row), 0u8); + let mut best = cost(row); + let start = out.len(); + out.push(0); + out.extend_from_slice(row); for kind in 1u8..=4 { + if best == 0 { + break; + } + let mut c = 0u64; for i in 0..n { let x = row[i]; scratch[i] = match kind { @@ -515,27 +522,14 @@ fn filter_row(row: &[u8], prev: Option<&[u8]>, bpp: usize, scratch: &mut [u8], o 3 => x.wrapping_sub(((u16::from(left(i)) + u16::from(up(i))) / 2) as u8), _ => x.wrapping_sub(paeth(left(i), up(i), ul(i))), }; + c += u64::from((scratch[i] as i8).unsigned_abs()); } - let c = cost(&scratch[..n]); - if c < best.0 { - best = (c, kind); + if c < best { + best = c; + out[start] = kind; + out[start + 1..].copy_from_slice(&scratch[..n]); } } - out.push(best.1); - if best.1 == 0 { - out.extend_from_slice(row); - return; - } - for i in 0..n { - let x = row[i]; - scratch[i] = match best.1 { - 1 => x.wrapping_sub(left(i)), - 2 => x.wrapping_sub(up(i)), - 3 => x.wrapping_sub(((u16::from(left(i)) + u16::from(up(i))) / 2) as u8), - _ => x.wrapping_sub(paeth(left(i), up(i), ul(i))), - }; - } - out.extend_from_slice(&scratch[..n]); } const ADLER_MOD: u32 = 65_521; @@ -659,6 +653,106 @@ mod indexed_tests { mod parallel_tests { use super::*; + #[test] + #[ignore = "manual 24 MP release benchmark"] + fn benchmark_filter_selection() { + use std::{hint::black_box, time::Instant}; + for pattern in ["zero", "identical", "varied"] { + let row: Vec = (0..6000 * 4).map(|i| if pattern == "zero" { 0 } else { (i * 17 + i / 13) as u8 }).collect(); + let prev: Vec<_> = row.iter().map(|v| if pattern == "identical" { *v } else { v.wrapping_add(3) }).collect(); + let mut scratch = vec![0; row.len()]; + let mut out = Vec::with_capacity(row.len() + 1); + for trial in 0..6 { + type Filter = fn(&[u8], Option<&[u8]>, usize, &mut [u8], &mut Vec); + let mut paths: [(&str, Filter); 2] = [("main", previous_filter_row), ("split", filter_row)]; + if trial % 2 == 1 { + paths.reverse(); + } + for (name, path) in paths { + let t = Instant::now(); + for _ in 0..4000 { + out.clear(); + black_box(path)(black_box(&row), Some(black_box(&prev)), 4, &mut scratch, &mut out); + black_box(&out); + } + println!("filter,{pattern},{trial},{name},{:.6}", t.elapsed().as_secs_f64() * 1000.0); + } + } + } + } + + // The main-branch implementation before this refactor. + fn previous_filter_row(row: &[u8], prev: Option<&[u8]>, bpp: usize, scratch: &mut [u8], out: &mut Vec) { + let n = row.len(); + let up = |i: usize| prev.map_or(0, |p| p[i]); + let left = |i: usize| if i >= bpp { row[i - bpp] } else { 0 }; + let ul = |i: usize| if i >= bpp { prev.map_or(0, |p| p[i - bpp]) } else { 0 }; + let paeth = |a: u8, b: u8, c: u8| { + let p = i16::from(a) + i16::from(b) - i16::from(c); + let (pa, pb, pc) = ((p - i16::from(a)).abs(), (p - i16::from(b)).abs(), (p - i16::from(c)).abs()); + if pa <= pb && pa <= pc { + a + } else if pb <= pc { + b + } else { + c + } + }; + let cost = |v: &[u8]| v.iter().map(|&b| u64::from((b as i8).unsigned_abs())).sum::(); + let mut best = (cost(row), 0u8); + for kind in 1u8..=4 { + for i in 0..n { + let x = row[i]; + scratch[i] = match kind { + 1 => x.wrapping_sub(left(i)), + 2 => x.wrapping_sub(up(i)), + 3 => x.wrapping_sub(((u16::from(left(i)) + u16::from(up(i))) / 2) as u8), + _ => x.wrapping_sub(paeth(left(i), up(i), ul(i))), + }; + } + let c = cost(&scratch[..n]); + if c < best.0 { + best = (c, kind); + } + } + out.push(best.1); + if best.1 == 0 { + out.extend_from_slice(row); + return; + } + for i in 0..n { + let x = row[i]; + scratch[i] = match best.1 { + 1 => x.wrapping_sub(left(i)), + 2 => x.wrapping_sub(up(i)), + 3 => x.wrapping_sub(((u16::from(left(i)) + u16::from(up(i))) / 2) as u8), + _ => x.wrapping_sub(paeth(left(i), up(i), ul(i))), + }; + } + out.extend_from_slice(&scratch[..n]); + } + + #[test] + fn adaptive_filter_preserves_minimum_cost_and_ties() { + for bpp in [1, 2, 3, 4, 6, 8] { + for seed in 0..32usize { + let row: Vec = (0..137).map(|i| (i * seed + i / 5) as u8).collect(); + let shifted: Vec<_> = row.iter().map(|v| v.wrapping_add(seed as u8)).collect(); + for prev in [None, Some(row.as_slice()), Some(shifted.as_slice())] { + let mut expected = vec![99]; + let mut actual = expected.clone(); + let mut scratch = vec![0; row.len()]; + previous_filter_row(&row, prev, bpp, &mut scratch, &mut expected); + filter_row(&row, prev, bpp, &mut scratch, &mut actual); + assert_eq!(actual, expected, "bpp={bpp}, seed={seed}"); + } + } + } + let mut out = Vec::new(); + filter_row(&[0; 16], None, 4, &mut [0; 16], &mut out); + assert_eq!(out, vec![0; 17]); + } + #[test] fn adler_combine_matches_one_pass() { let a: Vec = (0..100_000u32).map(|i| (i * 7 + i / 13) as u8).collect(); diff --git a/docs/png-filter-selection-performance.md b/docs/png-filter-selection-performance.md new file mode 100644 index 0000000000..34938b6d84 --- /dev/null +++ b/docs/png-filter-selection-performance.md @@ -0,0 +1,38 @@ +# PNG filter selection + +Only adaptive PNG filter selection changes: stop evaluating filters when their partial cost cannot win, and stop after a zero-cost winner. The kernel processes 4000 rows of 6000 RGBA8 pixels (24 MP) for zero, identical and varied rows; deflate is outside the timer. + +Measured on 2026-10-19 against `d95fdf3181c01ad41e95456091a394b105a9afb2` on an Apple M4 Max +(16 CPUs, 128 GiB), macOS arm64, rustc 1.99.1. Each branch starts directly at that +base; none of the other performance splits is applied. Release uses optimization +level 3, LTO off, 16 codegen units, debug info off and incremental compilation off. +Builds ran at nice 10 with APFS-cloned warm caches; measurements ran at normal +priority with no concurrent builds. Timings are medians of five samples after one +warm-up, except the compositor's ten Criterion samples. These synthetic workloads +are reproducible checks of this change, not a representative image corpus. + +| Workload | Main ms | This change ms | Time change | +|---|---:|---:|---:| +| zero | 448.138 | 5.734 | -98.7% | +| identical | 447.020 | 59.819 | -86.6% | +| varied | 450.469 | 277.254 | -38.5% | + +These are serial filter-kernel timings, not end-to-end PNG encoding timings. + +The committed oracle is the previous main-branch filter function. The unit test compares exact output bytes, including the selected filter/tie order, across six bytes-per-pixel layouts, 32 patterns and absent/identical/shifted previous rows. Existing encoder round trips and deterministic worker-count tests remain in place. + +Reproduce with the same release profile environment: + +```sh +CARGO_PROFILE_RELEASE_LTO=false CARGO_PROFILE_RELEASE_CODEGEN_UNITS=16 CARGO_PROFILE_RELEASE_DEBUG=0 CARGO_INCREMENTAL=0 nice -n 10 cargo test --release -p photocraft-codecs --lib --no-run +# Run the printed executable at normal priority: + benchmark_filter_selection --ignored --nocapture --test-threads=1 +``` + +## Local validation + +- `cargo test -p photocraft-codecs --locked`: passed (782 passed, 4 ignored). +- `cargo clippy -p photocraft-codecs --locked --all-targets -- -D warnings`: passed. +- `cargo xtask layers`: passed. +- `cargo check -p photocraft-codecs --locked --target wasm32-unknown-unknown`: passed. +- `cargo xtask test-corpus -p codecs`: passed (796 passed, 4 ignored).