diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2e76c76ba1..b9e7451013 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,6 +22,10 @@ on: fuzz_seconds: description: "Seconds per fuzz target" default: "600" + dry_mixer: + description: "Compare the live Mixer Brush against the origin/main merge base on 24 MP (Linux)" + type: boolean + default: false # The token is only used to check out the code: no job writes to the repository. permissions: @@ -481,3 +485,60 @@ jobs: path: | target/perf/*.json target/perf/summary.md + + dry-mixer: + if: inputs.dry_mixer == true + runs-on: ubuntu-latest + timeout-minutes: 90 + env: + RAYON_NUM_THREADS: "4" + CARGO_PROFILE_RELEASE_LTO: "false" + CARGO_PROFILE_RELEASE_CODEGEN_UNITS: "16" + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + persist-credentials: false + - name: Pin baseline to the merge base with origin/main + id: baseline + run: | + git remote add upstream https://github.com/storytold/photocraft.git + git fetch --no-tags upstream main + base=$(git merge-base HEAD upstream/main) + echo "sha=$base" >> "$GITHUB_OUTPUT" + git worktree add --detach "$RUNNER_TEMP/photocraft-baseline" "$base" + mkdir -p "$RUNNER_TEMP/photocraft-baseline/crates/engine/examples" + cp crates/engine/examples/bench_dry_mixer.rs "$RUNNER_TEMP/photocraft-baseline/crates/engine/examples/bench_dry_mixer.rs" + - uses: dtolnay/rust-toolchain@b725eda525362e7cf34af9a7243cac1e3f01a7ec # stable + - name: Install Linux build dependencies + uses: storytold/craft-common-actions/actions/install-apt-packages@main + with: + packages: >- + libxkbcommon-dev libwayland-dev libx11-dev libxrandr-dev libxi-dev libgl1-mesa-dev + libgtk-3-dev + - name: Benchmark baseline and candidate on this runner + run: | + set -o pipefail + echo "baseline=${{ steps.baseline.outputs.sha }}" | tee dry-mixer-perf.txt + echo "candidate=${GITHUB_SHA}" | tee -a dry-mixer-perf.txt + CARGO_TARGET_DIR="$RUNNER_TEMP/target-baseline" cargo run --release --manifest-path "$RUNNER_TEMP/photocraft-baseline/Cargo.toml" -p photocraft-engine --example bench_dry_mixer | tee baseline-mixer.txt + CARGO_TARGET_DIR="$RUNNER_TEMP/target-candidate" cargo run --release -p photocraft-engine --example bench_dry_mixer | tee candidate-mixer.txt + cat baseline-mixer.txt candidate-mixer.txt >> dry-mixer-perf.txt + python3 - <<'PY' + import re + def fingerprints(path): + rows = open(path).readlines() + result = {} + for row in rows: + match = re.search(r'wet=\s*(\d+)% sample_all=\s*(\w+) size=\s*(\d+).*blake3=(\w+)', row) + if match: + result[match.group(1, 2, 3)] = match.group(4) + return result + before, after = fingerprints('baseline-mixer.txt'), fingerprints('candidate-mixer.txt') + assert len(before) == 12 and before == after, 'baseline/candidate mixer output or state differs' + PY + - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + if: always() + with: + name: dry-mixer-performance + path: dry-mixer-perf.txt diff --git a/crates/engine/examples/bench_dry_mixer.rs b/crates/engine/examples/bench_dry_mixer.rs new file mode 100644 index 0000000000..539053022f --- /dev/null +++ b/crates/engine/examples/bench_dry_mixer.rs @@ -0,0 +1,86 @@ +//! Release benchmark for the live Mixer Brush on a 24 MP document. +//! Setup and checksum are outside the timed command. Run with +//! `cargo run --release -p photocraft-engine --example bench_dry_mixer`. + +use std::time::Instant; + +use photocraft_engine::{EngineError, Session}; +use photocraft_geom::Rect; +use serde_json::json; + +const W: i32 = 6000; +const H: i32 = 4000; +const SIZES: [u32; 3] = [32, 128, 512]; +const REPEATS: usize = 4; + +fn fingerprint(session: &Session) -> Result> { + let state = session.active().ok_or("no active document")?; + let id = state.active_layer.ok_or("no active layer")?; + let surface = state.doc.layer(id).and_then(|layer| layer.surface()).ok_or("no raster surface")?; + let pixels = surface.read_region(Rect::new(0, 0, W, H)); + let mut hash = blake3::Hasher::new(); + for channel in pixels { + hash.update(&channel.to_le_bytes()); + } + hash.update(&serde_json::to_vec(&session.tools.mixer)?); + Ok(hash.finalize().to_hex().to_string()) +} + +fn run() -> Result<(), Box> { + for wet in [0, 100] { + for sample_all in [false, true] { + for size in SIZES { + let mut session = Session::new(); + session.execute("file.new", json!({"width": W, "height": H, "depth": 8, "mode": "rgb"}))?; + session.execute("layer.new.layer", json!({}))?; + session.edit("benchmark source", |doc, active| { + let id = active.ok_or_else(|| EngineError::Other("no source layer".into()))?; + let surface = doc.layer_mut(id).and_then(|layer| layer.surface_mut()).ok_or_else(|| EngineError::Other("no source pixels".into()))?; + for x in (0..W).step_by(64) { + let color = if x % 128 == 0 { [0.9, 0.25, 0.08, 1.0] } else { [0.08, 0.35, 0.9, 1.0] }; + surface.fill_rect(Rect::new(x, 0, (x + 64).min(W), H), &color); + } + Ok(()) + })?; + session.execute("layer.new.layer", json!({}))?; + + let params = json!({ + "points": [[1200, 2000], [4800, 2000]], + "size": size, + "spacing": 0.25, + "wet": wet, + "load": 50, + "mix": 50, + "flow": 100, + "sampleAllLayers": sample_all, + "cleanAfterStroke": true + }); + let mut timings = Vec::with_capacity(REPEATS); + let mut output = String::new(); + for repeat in 0..REPEATS { + let start = Instant::now(); + session.execute("paint.mixerBrush", params.clone())?; + let elapsed_ms = start.elapsed().as_secs_f64() * 1000.0; + if repeat > 0 { + timings.push(elapsed_ms); + } + output = fingerprint(&session)?; + session.execute("edit.undo", json!({}))?; + } + timings.sort_by(f64::total_cmp); + println!( + "wet={wet:3}% sample_all={sample_all:5} size={size:3} median_ms={:.2} min_ms={:.2} max_ms={:.2} blake3={output}", + timings[1], timings[0], timings[2] + ); + } + } + } + Ok(()) +} + +fn main() { + if let Err(error) = run() { + eprintln!("{error}"); + std::process::exit(1); + } +} diff --git a/crates/engine/src/cmd/brush_cmds/mixer_live.rs b/crates/engine/src/cmd/brush_cmds/mixer_live.rs index acdd897fd4..6b9747e6dc 100644 --- a/crates/engine/src/cmd/brush_cmds/mixer_live.rs +++ b/crates/engine/src/cmd/brush_cmds/mixer_live.rs @@ -86,7 +86,7 @@ impl Core { } // Render the visible document including the active layer exactly once, before // deposition. Masks, transforms, blend modes and adjustments use the compositor. - let sample = self.settings.sample_all_layers.then(|| { + let sample = (self.settings.sample_all_layers && self.settings.samples_canvas(dab)).then(|| { let buf = photocraft_compose::render(&self.doc, r); let fmt = self.pre.format(); let mut sample = Region::new(r, fmt.channels()); diff --git a/crates/engine/src/cmd/brush_cmds/mixer_live_tests.rs b/crates/engine/src/cmd/brush_cmds/mixer_live_tests.rs index d0c04c30e3..af8e754ce2 100644 --- a/crates/engine/src/cmd/brush_cmds/mixer_live_tests.rs +++ b/crates/engine/src/cmd/brush_cmds/mixer_live_tests.rs @@ -70,6 +70,29 @@ fn mixer_live_empty_layer_exact_commit_undo_source_preservation_and_chunking() { } } +/// A dry live stroke commits exactly its preview for either sampling setting and supports undo. +#[test] +fn dry_mixer_live_matches_commit_with_and_without_all_layer_sampling() { + for fmt in [PixelFormat::RGBA8, PixelFormat::RGBA16, PixelFormat::RGBA32F] { + for sample_all in [false, true] { + let mut s = fixture(fmt); + let before = target(&s); + let p = + json!({"points":[[40,48],[80,48],[120,48]],"size":24,"wet":0,"sampleAllLayers":sample_all,"cleanAfterStroke":false,"loadAfterStroke":false}); + let live = LiveMixer::begin(&s, &p).unwrap(); + let id = s.active().unwrap().active_layer.unwrap(); + let bounds = s.active().unwrap().doc.bounds(); + let preview = live.doc.layer(id).unwrap().surface().unwrap().read_region(bounds); + s.execute("paint.mixerBrush", p).unwrap(); + assert_eq!(preview, target(&s).read_region(bounds)); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(before.read_region(bounds), target(&s).read_region(bounds)); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(preview, target(&s).read_region(bounds)); + } + } +} + /// Current-layer mixing matches the frozen kernel; dropping a preview and cleaning the load paint nothing. #[test] fn mixer_live_current_layer_matches_reference_and_cancels() { diff --git a/crates/paint/src/mixer.rs b/crates/paint/src/mixer.rs index 32b4c265ea..d11e4fa10c 100644 --- a/crates/paint/src/mixer.rs +++ b/crates/paint/src/mixer.rs @@ -39,6 +39,18 @@ impl Default for MixerSettings { } } +impl MixerSettings { + /// Effective per-dab wetness, including Transfer's Wetness control. + pub fn effective_wet(&self, dab: &crate::Dab) -> f32 { + self.wet.clamp(0.0, 1.0) * dab.wet.clamp(0.0, 1.0) + } + + /// Whether this dab can update the pickup well from the canvas. + pub fn samples_canvas(&self, dab: &crate::Dab) -> bool { + self.effective_wet(dab) > 0.0 + } +} + /// Paint carried between strokes (session tool state). #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] #[serde(default, rename_all = "camelCase")] @@ -158,7 +170,7 @@ pub fn apply_mixer_dab( state: &mut MixerState, lock_transparency: bool, ) { - let sample = sample.map(|surface| { + let sample = sample.filter(|_| m.samples_canvas(&fp.dab)).map(|surface| { let source = crate::retouch::Region::read(surface, work.rect); if surface.format().mode == fmt.mode && surface.format().alpha == fmt.alpha { source @@ -198,37 +210,38 @@ pub fn apply_mixer_dab_region( let res = state.reservoir.map(native); let mut amount = if res.is_some() { state.amount.clamp(0.0, 1.0) } else { 0.0 }; let mut pickup: Option> = state.pickup.map(native); - let (wet, mix, flow, load) = (m.wet.clamp(0.0, 1.0), m.mix.clamp(0.0, 1.0), m.flow.clamp(0.0, 1.0), m.load.clamp(0.0, 1.0)); - let mut acc = vec![0.0f32; n]; + let (mix, flow, load) = (m.mix.clamp(0.0, 1.0), m.flow.clamp(0.0, 1.0), m.load.clamp(0.0, 1.0)); let mut paint = vec![0.0f32; n]; // Transfer › Wetness / Mix Jitter scale the settings per dab. - let (wet, mix) = (wet * fp.dab.wet.clamp(0.0, 1.0), mix * fp.dab.mix.clamp(0.0, 1.0)); + let (wet, mix) = (m.effective_wet(&fp.dab), mix * fp.dab.mix.clamp(0.0, 1.0)); // Sample the canvas under the dab. - acc.iter_mut().for_each(|v| *v = 0.0); - let (mut wsum, mut asum) = (0.0f32, 0.0f32); let r = fp.rect.intersect(&work.rect); - for y in r.y0..r.y1 { - for x in r.x0..r.x1 { - let w = fp.at(x, y); - if w <= 0.0 { - continue; + if wet > 0.0 { + let mut acc = vec![0.0f32; n]; + let (mut wsum, mut asum) = (0.0f32, 0.0f32); + for y in r.y0..r.y1 { + for x in r.x0..r.x1 { + let w = fp.at(x, y); + if w <= 0.0 { + continue; + } + let px = sample.unwrap_or(work).px(x, y); + let al = a.map_or(1.0, |ai| px[ai]); + for c in 0..nc { + acc[c] += px[c] * w * al; + } + wsum += w; + asum += w * al; } - let px = sample.unwrap_or(work).px(x, y); - let al = a.map_or(1.0, |ai| px[ai]); - for c in 0..nc { - acc[c] += px[c] * w * al; - } - wsum += w; - asum += w * al; } - } - if asum > 1e-6 && wet > 0.0 { - let ca = asum / wsum.max(1e-6); - let canvas: Vec = (0..n).map(|c| if c < nc { acc[c] / asum } else { 1.0 }).collect(); - pickup = Some(match pickup.take() { - None => canvas, - Some(p) => p.iter().zip(&canvas).map(|(p, c)| p + (c - p) * PICKUP_RATE * wet * ca).collect(), - }); + if asum > 1e-6 { + let ca = asum / wsum.max(1e-6); + let canvas: Vec = (0..n).map(|c| if c < nc { acc[c] / asum } else { 1.0 }).collect(); + pickup = Some(match pickup.take() { + None => canvas, + Some(p) => p.iter().zip(&canvas).map(|(p, c)| p + (c - p) * PICKUP_RATE * wet * ca).collect(), + }); + } } let wr = if res.is_some() { amount * (1.0 - mix * wet) } else { 0.0 }; let wp = if pickup.is_some() { mix * wet } else { 0.0 }; diff --git a/crates/paint/src/mixer/reference_tests.rs b/crates/paint/src/mixer/reference_tests.rs index 73242fa2f9..0f6f0a2a82 100644 --- a/crates/paint/src/mixer/reference_tests.rs +++ b/crates/paint/src/mixer/reference_tests.rs @@ -152,3 +152,56 @@ fn contiguous_mixer_matches_frozen_kernel_across_models_depths_and_spatial_loads } } } + +/// Dry dabs must preserve both the painted pixels and the carried pickup across transitions. +#[test] +fn dry_and_wet_transitions_match_frozen_kernel() { + use crate::retouch::Region; + use crate::{BrushContext, BrushSettings, Dab}; + use photocraft_color::{ColorMode, PixelFormat, SampleType}; + use photocraft_geom::Point; + + let rect = Rect::new(0, 0, 48, 48); + for mode in [ColorMode::Rgb, ColorMode::Grayscale, ColorMode::Cmyk, ColorMode::Lab] { + for depth in [SampleType::U8, SampleType::U16, SampleType::F32] { + for alpha in [false, true] { + let fmt = PixelFormat::new(mode, depth, alpha); + let mut surface = Surface::new(fmt); + surface.fill_rect(rect, &from_rgba(&fmt, [0.25, 0.5, 0.75, 0.6])); + let mut source = Surface::new(PixelFormat::RGBA32F); + source.fill_rect(rect, &[0.7, 0.1, 0.3, 0.8]); + let ctx = BrushContext::new(&BrushSettings { size: 16.0, ..Default::default() }); + for spatial in [false, true] { + for loaded in [false, true] { + for sample in [None, Some(&source)] { + for lock in [false, true] { + let mut actual = Region::read(&surface, rect); + let mut expected = actual.clone(); + let mut state = MixerState::default(); + if loaded { + state.load([0.8, 0.2, 0.1, 1.0]); + } + if spatial { + state.spatial = Some(vec![[0.2, 0.8, 0.4, 0.7]; 4096]); + } + let mut expected_state = state.clone(); + for (i, (base_wet, dab_wet, mix)) in + [(0.7, 1.0, 0.0), (0.0, 1.0, 0.7), (0.7, 0.0, 0.7), (0.7, 1.0, 0.7)].into_iter().enumerate() + { + let mut dab = Dab::round(Point::new(20.0 + i as f64, 20.0), 8.0, 1.0); + dab.wet = dab_wet; + let fp = ctx.footprint(&dab, i); + let settings = MixerSettings { wet: base_wet, mix, ..Default::default() }; + reference(&mut expected, &fp, fmt, sample, &settings, &mut expected_state, lock); + apply_mixer_dab(&mut actual, &fp, fmt, sample, &settings, &mut state, lock); + assert_eq!(actual, expected, "{mode:?} {depth:?} alpha={alpha} spatial={spatial} loaded={loaded} lock={lock} dab={i}"); + assert_eq!(state, expected_state, "mixer state after dab {i}"); + } + } + } + } + } + } + } + } +}