From 7a7ab587234f5d0bfc646b39051c92bcdab45cef Mon Sep 17 00:00:00 2001 From: flamboh Date: Tue, 13 Oct 2026 17:24:29 -0700 Subject: [PATCH 1/4] perf(desktop-gpui): rebuild only changed screenshot privacy masks --- .../kernels/benchmarks/mask-cache/Cargo.lock | 65 ++++ .../kernels/benchmarks/mask-cache/Cargo.toml | 22 ++ .../kernels/benchmarks/mask-cache/README.md | 60 ++++ .../kernels/benchmarks/mask-cache/lib.rs | 4 + .../screenshot-mask-cache-benchmark.rs | 160 ++++++++++ apps/desktop-gpui/kernels/src/lib.rs | 1 + apps/desktop-gpui/kernels/src/mask_cache.rs | 285 ++++++++++++++++++ .../src/screenshot_annotations.rs | 253 +++++++++------- apps/desktop-gpui/src/screenshot_editor.rs | 6 + 9 files changed, 750 insertions(+), 106 deletions(-) create mode 100644 apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.lock create mode 100644 apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml create mode 100644 apps/desktop-gpui/kernels/benchmarks/mask-cache/README.md create mode 100644 apps/desktop-gpui/kernels/benchmarks/mask-cache/lib.rs create mode 100644 apps/desktop-gpui/kernels/examples/screenshot-mask-cache-benchmark.rs create mode 100644 apps/desktop-gpui/kernels/src/mask_cache.rs diff --git a/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.lock b/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.lock new file mode 100644 index 00000000000..65c42077127 --- /dev/null +++ b/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.lock @@ -0,0 +1,65 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "autocfg" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" + +[[package]] +name = "bytemuck" +version = "1.25.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95832e849adfb21180ccb6826a99da14e5d266ae5c2e668e1602cf234f153797" + +[[package]] +name = "byteorder-lite" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f1fe948ff07f4bd06c30984e69f5b4899c516a3ef74f34df92a2df2ab535495" + +[[package]] +name = "cap-screenshot-mask-cache-benchmark" +version = "0.1.0" +dependencies = [ + "image", +] + +[[package]] +name = "image" +version = "0.25.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85ab80394333c02fe689eaf900ab500fbd0c2213da414687ebf995a65d5a6104" +dependencies = [ + "bytemuck", + "byteorder-lite", + "moxcms", + "num-traits", +] + +[[package]] +name = "moxcms" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bb85c154ba489f01b25c0d36ae69a87e4a1c73a72631fc6c0eb6dde34a73e44b" +dependencies = [ + "num-traits", + "pxfm", +] + +[[package]] +name = "num-traits" +version = "0.2.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" +dependencies = [ + "autocfg", +] + +[[package]] +name = "pxfm" +version = "0.1.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d55d956fa96f5ec02be2e13af0e20391a5aa83d6a074e3ad368959d0fab299ea" diff --git a/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml b/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml new file mode 100644 index 00000000000..bda064d8078 --- /dev/null +++ b/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml @@ -0,0 +1,22 @@ +[workspace] + +[package] +name = "cap-screenshot-mask-cache-benchmark" +version = "0.1.0" +edition = "2024" +publish = false + +[lib] +name = "cap_gpui_kernels" +path = "lib.rs" + +[[example]] +name = "screenshot-mask-cache-benchmark" +path = "../../examples/screenshot-mask-cache-benchmark.rs" + +[dependencies] +image = { version = "=0.25.10", default-features = false } + +[profile.release] +codegen-units = 1 +lto = "thin" diff --git a/apps/desktop-gpui/kernels/benchmarks/mask-cache/README.md b/apps/desktop-gpui/kernels/benchmarks/mask-cache/README.md new file mode 100644 index 00000000000..30fca7dabe6 --- /dev/null +++ b/apps/desktop-gpui/kernels/benchmarks/mask-cache/README.md @@ -0,0 +1,60 @@ +# Screenshot mask cache benchmark + +Run from the repository root: + +```sh +cargo +stable run --locked --release \ + --manifest-path apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml \ + --example screenshot-mask-cache-benchmark +``` + +This standalone package compiles the production `mask_cache.rs` and `screenshot.rs` directly. It requires Rust and the `image` crate, with a committed lockfile matching the desktop workspace's image/filter dependencies. It does not build GPUI, audio, FFmpeg, or platform libraries, and can run on Linux x86_64 as well as macOS arm64. The same example is also available through `cargo run -p cap-gpui-kernels --release --example screenshot-mask-cache-benchmark` in the desktop workspace, when its full dependency prerequisites are installed. + +The fixture is a deterministic, nonuniform 3840 × 2160 RGBA image with varying alpha. Each workload has 1, 4, 8, or 16 masks of 640 × 360 pixels at level 16, using either blur or pixelate. One mask moves one pixel per event; the others stay fixed. Each workload primes the cache, runs 12 warmup pairs, then measures 120 drag events. The old path filters every mask on every update. The new path calls the production cache diff and filters only changed masks. Both paths call the unchanged production `mask_filter`. + +Execution order alternates between old-first and new-first to reduce order bias. Every overlay's request and all of its output bytes are compared after every warmup and measured event, outside the timed intervals. A mismatch or unexpected filter-call count fails the run. Filter-call counts in the table cover the 120 measured events only; cache priming and warmup are excluded. The speedup is the ratio of means. The median is the average of the two middle samples. + +Timings include cache diffing, region extraction/filtering, image allocation, `Arc` ownership, and release of retired CPU images. Request/geometry construction and correctness comparisons are outside timing. The benchmark excludes GPUI image conversion, task scheduling/cancellation, GPU atlas uploads, painting, and screenshot export. It measures completed CPU preparation work, not pointer-event latency or application FPS. One mask is a control case: it has no reusable image during a drag. A replaced frame invalidates every mask and is not expected to benefit. Overlapping masks, deletion, reordering, style changes, frame/layout invalidation, and fractional/clipped rectangles are covered by the production module's unit tests. + +Run the portable tests and lint checks: + +```sh +cargo +stable test --locked \ + --manifest-path apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml \ + --all-targets +cargo +stable clippy --locked \ + --manifest-path apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml \ + --all-targets -- -D warnings +``` + +Run the release command at least twice on an otherwise idle machine. Keep the full output, CPU model, operating system, Rust version, and power mode with results. Compare both means and medians, including the one-mask control. Cross-architecture results should use the same lockfile and workload. + +## Recorded results + +Two consecutive release runs on Apple M4 Pro, macOS 27.2, Rust 1.97.0, battery power with automatic power mode, October 13, 2026. All overlay bytes matched after every event in both runs. These are CPU overlay-preparation measurements with the exclusions described above. + +Run 1: + +| Mode | Masks | Old mean ms | New mean ms | Old median ms | New median ms | Mean speedup | Old calls | New calls | +| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | +| blur | 1 | 1.637 | 1.631 | 1.630 | 1.621 | 1.00x | 120 | 120 | +| blur | 4 | 6.701 | 1.687 | 6.697 | 1.688 | 3.97x | 480 | 120 | +| blur | 8 | 13.266 | 1.664 | 13.291 | 1.660 | 7.97x | 960 | 120 | +| blur | 16 | 26.504 | 1.688 | 26.494 | 1.685 | 15.70x | 1920 | 120 | +| pixelate | 1 | 0.805 | 0.812 | 0.804 | 0.804 | 0.99x | 120 | 120 | +| pixelate | 4 | 3.310 | 0.828 | 3.306 | 0.826 | 4.00x | 480 | 120 | +| pixelate | 8 | 6.938 | 0.900 | 6.956 | 0.902 | 7.71x | 960 | 120 | +| pixelate | 16 | 13.723 | 0.911 | 13.605 | 0.878 | 15.07x | 1920 | 120 | + +Run 2: + +| Mode | Masks | Old mean ms | New mean ms | Old median ms | New median ms | Mean speedup | Old calls | New calls | +| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | +| blur | 1 | 1.600 | 1.600 | 1.585 | 1.589 | 1.00x | 120 | 120 | +| blur | 4 | 6.569 | 1.657 | 6.548 | 1.651 | 3.96x | 480 | 120 | +| blur | 8 | 13.384 | 1.713 | 13.293 | 1.672 | 7.82x | 960 | 120 | +| blur | 16 | 26.587 | 1.708 | 26.138 | 1.661 | 15.57x | 1920 | 120 | +| pixelate | 1 | 0.790 | 0.789 | 0.791 | 0.791 | 1.00x | 120 | 120 | +| pixelate | 4 | 3.252 | 0.812 | 3.234 | 0.809 | 4.00x | 480 | 120 | +| pixelate | 8 | 6.749 | 0.862 | 6.664 | 0.857 | 7.83x | 960 | 120 | +| pixelate | 16 | 13.750 | 0.888 | 13.766 | 0.883 | 15.48x | 1920 | 120 | diff --git a/apps/desktop-gpui/kernels/benchmarks/mask-cache/lib.rs b/apps/desktop-gpui/kernels/benchmarks/mask-cache/lib.rs new file mode 100644 index 00000000000..334cafd746a --- /dev/null +++ b/apps/desktop-gpui/kernels/benchmarks/mask-cache/lib.rs @@ -0,0 +1,4 @@ +#[path = "../../src/mask_cache.rs"] +pub mod mask_cache; +#[path = "../../src/screenshot.rs"] +pub mod screenshot; diff --git a/apps/desktop-gpui/kernels/examples/screenshot-mask-cache-benchmark.rs b/apps/desktop-gpui/kernels/examples/screenshot-mask-cache-benchmark.rs new file mode 100644 index 00000000000..9c1dc29afef --- /dev/null +++ b/apps/desktop-gpui/kernels/examples/screenshot-mask-cache-benchmark.rs @@ -0,0 +1,160 @@ +use cap_gpui_kernels::mask_cache::{MaskEntry, MaskFrame, MaskRequest, mask_region, update_masks}; +use std::{hint::black_box, sync::Arc, time::Instant}; + +const SIZE: (u32, u32) = (3840, 2160); +const WARMUP: usize = 12; +const EVENTS: usize = 120; +type Overlay = MaskEntry>; + +fn requests(count: usize, pixelate: bool, event: usize) -> Vec { + let content = [0., 0., f64::from(SIZE.0), f64::from(SIZE.1)]; + (0..count) + .map(|index| { + let x = 80. + (index % 4) as f64 * 720. + if index == 0 { event as f64 } else { 0. }; + let y = 80. + (index / 4) as f64 * 440.; + MaskRequest { + id: index.to_string(), + frame: MaskFrame { + revision: 1, + size: SIZE, + content: content.map(f64::to_bits), + padding: 0_f64.to_bits(), + crop: None, + }, + region: mask_region([x, y, 640., 360.], content, SIZE).unwrap(), + pixelate, + level: 16_f64.to_bits(), + } + }) + .collect() +} + +fn rebuild_all(requests: &[MaskRequest], rgba: &[u8], calls: &mut usize) -> Vec { + requests + .iter() + .map(|request| { + *calls += 1; + MaskEntry { + request: request.clone(), + value: request.filter(rgba).map(Arc::new), + } + }) + .collect() +} + +fn cached( + previous: &[Overlay], + requests: &[MaskRequest], + rgba: &[u8], + calls: &mut usize, +) -> Vec { + update_masks(previous, requests, |request| { + *calls += 1; + request.filter(rgba).map(Arc::new) + }) +} + +fn stats(samples: &mut [f64]) -> (f64, f64) { + samples.sort_by(f64::total_cmp); + ( + samples.iter().sum::() / samples.len() as f64, + (samples[samples.len() / 2 - 1] + samples[samples.len() / 2]) / 2., + ) +} + +fn main() { + let rgba: Vec = (0..SIZE.0 * SIZE.1) + .flat_map(|i| { + let x = i % SIZE.0; + let y = i / SIZE.0; + [ + (x * 13 + y * 7) as u8, + (x * 3 + y * 17) as u8, + (x ^ (y * 11)) as u8, + (128 + (x * 5 + y * 3) % 128) as u8, + ] + }) + .collect(); + println!( + "Screenshot mask overlay preparation: {} / {}", + std::env::consts::OS, + std::env::consts::ARCH + ); + println!( + "3840x2160 RGBA; 640x360 masks; level 16; {WARMUP} warmup pairs; {EVENTS} measured drag events" + ); + println!( + "Alternating old/new execution order; warm cache; byte comparison after every event outside timing." + ); + println!( + "Includes cache diff, filtering, Arc allocation and retired CPU image release. Excludes GPUI conversion, scheduling, atlas and painting." + ); + println!( + "| Mode | Masks | Old mean ms | New mean ms | Old median ms | New median ms | Mean speedup | Old calls | New calls |" + ); + println!("| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: |"); + for pixelate in [false, true] { + for count in [1, 4, 8, 16] { + let initial = requests(count, pixelate, 0); + let mut old = rebuild_all(&initial, &rgba, &mut 0); + let mut new = cached(&[], &initial, &rgba, &mut 0); + let mut old_samples = Vec::with_capacity(EVENTS); + let mut new_samples = Vec::with_capacity(EVENTS); + let mut old_calls = 0; + let mut new_calls = 0; + for event in 1..=WARMUP + EVENTS { + let requests = requests(count, pixelate, event); + let mut run_old = || { + let start = Instant::now(); + old = rebuild_all(black_box(&requests), black_box(&rgba), &mut old_calls); + start.elapsed().as_secs_f64() * 1000. + }; + let mut run_new = || { + let start = Instant::now(); + new = cached( + black_box(&new), + black_box(&requests), + black_box(&rgba), + &mut new_calls, + ); + start.elapsed().as_secs_f64() * 1000. + }; + let (old_ms, new_ms) = if event % 2 == 0 { + let new_ms = run_new(); + (run_old(), new_ms) + } else { + let old_ms = run_old(); + (old_ms, run_new()) + }; + assert_eq!(old.len(), count); + assert_eq!(new.len(), count); + for (old, new) in old.iter().zip(&new) { + assert_eq!(old.request, new.request); + assert_eq!( + old.value.as_ref().unwrap().as_raw(), + new.value.as_ref().unwrap().as_raw(), + "event {event}, mask {}", + old.request.id + ); + } + if event <= WARMUP { + old_calls = 0; + new_calls = 0; + } else { + old_samples.push(old_ms); + new_samples.push(new_ms); + } + } + assert_eq!(old_calls, count * EVENTS); + assert_eq!(new_calls, EVENTS); + let (old_mean, old_median) = stats(&mut old_samples); + let (new_mean, new_median) = stats(&mut new_samples); + let mode = if pixelate { "pixelate" } else { "blur" }; + println!( + "| {mode} | {count} | {old_mean:.3} | {new_mean:.3} | {old_median:.3} | {new_median:.3} | {:.2}x | {old_calls} | {new_calls} |", + old_mean / new_mean + ); + } + } + println!("All overlay bytes matched at every warmup and measured event."); +} diff --git a/apps/desktop-gpui/kernels/src/lib.rs b/apps/desktop-gpui/kernels/src/lib.rs index f35b4f5ede1..009db0e7ceb 100644 --- a/apps/desktop-gpui/kernels/src/lib.rs +++ b/apps/desktop-gpui/kernels/src/lib.rs @@ -1,5 +1,6 @@ pub mod codec; pub mod frame; +pub mod mask_cache; pub mod screenshot; pub mod texture; pub mod thumbnail; diff --git a/apps/desktop-gpui/kernels/src/mask_cache.rs b/apps/desktop-gpui/kernels/src/mask_cache.rs new file mode 100644 index 00000000000..9421b9a1ddd --- /dev/null +++ b/apps/desktop-gpui/kernels/src/mask_cache.rs @@ -0,0 +1,285 @@ +use crate::screenshot::mask_filter; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct MaskFrame { + pub revision: u64, + pub size: (u32, u32), + pub content: [u64; 4], + pub padding: u64, + pub crop: Option<[u32; 4]>, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct MaskRequest { + pub id: String, + pub frame: MaskFrame, + pub region: (u32, u32, u32, u32), + pub pixelate: bool, + pub level: u64, +} + +impl MaskRequest { + pub fn filter(&self, rgba: &[u8]) -> Option { + mask_filter( + rgba, + self.frame.size.0, + self.frame.size.1, + self.region, + self.pixelate, + f64::from_bits(self.level), + ) + } +} + +#[derive(Clone)] +pub struct MaskEntry { + pub request: MaskRequest, + pub value: Option, +} + +#[derive(Debug, PartialEq, Eq)] +pub struct MaskPlan { + pub reuse: Vec>, + pub drop: Vec, +} + +pub fn plan_masks(previous: &[MaskEntry], requests: &[MaskRequest]) -> MaskPlan { + let reuse: Vec<_> = requests + .iter() + .map(|request| previous.iter().position(|entry| entry.request == *request)) + .collect(); + let drop = (0..previous.len()) + .filter(|index| !reuse.contains(&Some(*index))) + .collect(); + MaskPlan { reuse, drop } +} + +pub fn update_masks( + previous: &[MaskEntry], + requests: &[MaskRequest], + mut compute: impl FnMut(&MaskRequest) -> Option, +) -> Vec> { + let plan = plan_masks(previous, requests); + requests + .iter() + .zip(plan.reuse) + .map(|(request, reuse)| MaskEntry { + request: request.clone(), + value: match reuse { + Some(index) => previous[index].value.clone(), + None => compute(request), + }, + }) + .collect() +} + +pub fn mask_region( + mask: [f64; 4], + content: [f64; 4], + frame: (u32, u32), +) -> Option<(u32, u32, u32, u32)> { + let [x, y, width, height] = mask; + let [content_x, content_y, content_width, content_height] = content; + let left = content_x.max(x.min(x + width)); + let top = content_y.max(y.min(y + height)); + let right = (content_x + content_width).min(x.max(x + width)); + let bottom = (content_y + content_height).min(y.max(y + height)); + if right <= left || bottom <= top { + return None; + } + let x0 = (left.round().max(0.) as u32).min(frame.0); + let y0 = (top.round().max(0.) as u32).min(frame.1); + let x1 = (right.round().max(0.) as u32).min(frame.0); + let y1 = (bottom.round().max(0.) as u32).min(frame.1); + (x1 > x0 && y1 > y0).then(|| (x0, y0, x1 - x0, y1 - y0)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Arc; + + fn requests(count: usize) -> Vec { + (0..count) + .map(|index| MaskRequest { + id: index.to_string(), + frame: MaskFrame { + revision: 1, + size: (64, 48), + content: [0., 0., 64., 48.].map(f64::to_bits), + padding: 0_f64.to_bits(), + crop: None, + }, + region: (index as u32 * 4, 4, 32, 24), + pixelate: false, + level: 16_f64.to_bits(), + }) + .collect() + } + + fn cached(requests: &[MaskRequest]) -> Vec>> { + update_masks(&[], requests, |request| Some(Arc::new(request.id.clone()))) + } + + #[test] + fn moving_one_mask_reuses_the_other_images() { + let mut requests = requests(4); + let previous = cached(&requests); + requests[1].region.0 += 1; + assert_eq!( + plan_masks(&previous, &requests), + MaskPlan { + reuse: vec![Some(0), None, Some(2), Some(3)], + drop: vec![1], + } + ); + let mut calls = 0; + let next = update_masks(&previous, &requests, |request| { + calls += 1; + Some(Arc::new(request.id.clone())) + }); + assert_eq!(calls, 1); + for index in [0, 2, 3] { + assert!(Arc::ptr_eq( + previous[index].value.as_ref().unwrap(), + next[index].value.as_ref().unwrap() + )); + } + } + + #[test] + fn deleting_and_reordering_evicts_only_the_deleted_mask() { + let mut requests = requests(4); + let previous = cached(&requests); + requests.remove(1); + requests.reverse(); + assert_eq!( + plan_masks(&previous, &requests), + MaskPlan { + reuse: vec![Some(3), Some(2), Some(0)], + drop: vec![1], + } + ); + let next = update_masks(&previous, &requests, |_| panic!("unexpected filter")); + assert_eq!( + next.iter() + .map(|entry| entry.request.id.as_str()) + .collect::>(), + ["3", "2", "0"] + ); + assert_eq!(plan_masks(&next, &[]).drop, [0, 1, 2]); + } + + #[test] + fn new_and_changed_styles_are_computed() { + let mut requests = requests(4); + let previous = cached(&requests[..3]); + requests[0].pixelate = true; + requests[1].level = 20_f64.to_bits(); + assert_eq!( + plan_masks(&previous, &requests), + MaskPlan { + reuse: vec![None, None, Some(2), None], + drop: vec![0, 1], + } + ); + } + + #[test] + fn frame_and_layout_changes_invalidate_every_mask() { + let original = requests(4); + let previous = cached(&original); + for change in 0..5 { + let mut requests = original.clone(); + for request in &mut requests { + match change { + 0 => request.frame.revision += 1, + 1 => request.frame.size.0 += 1, + 2 => request.frame.content[0] = 0.25_f64.to_bits(), + 3 => request.frame.padding = 1_f64.to_bits(), + _ => request.frame.crop = Some([1, 0, 64, 48]), + } + } + assert_eq!( + plan_masks(&previous, &requests), + MaskPlan { + reuse: vec![None; 4], + drop: vec![0, 1, 2, 3], + } + ); + } + } + + #[test] + fn identical_requests_reuse_even_a_failed_filter() { + let requests = requests(4); + let previous = update_masks::>(&[], &requests, |_| None); + let next = update_masks(&previous, &requests, |_| panic!("unexpected filter")); + assert!(next.iter().all(|entry| entry.value.is_none())); + assert_eq!(plan_masks(&previous, &requests).drop, []); + } + + #[test] + fn regions_round_after_clipping_and_normalize_negative_sizes() { + let content = [10.25, 8.75, 40.5, 30.5]; + assert_eq!( + mask_region([-2.4, 1.2, 80., 60.], content, (48, 36)), + Some((10, 9, 38, 27)) + ); + assert_eq!( + mask_region([40.4, 30.4, -20.2, -10.2], content, (64, 48)), + Some((20, 20, 20, 10)) + ); + assert_eq!(mask_region([10.1, 10.1, 0.2, 0.2], content, (64, 48)), None); + assert_eq!(mask_region([60., 10., 10., 10.], content, (64, 48)), None); + assert_eq!(mask_region([0., 0., 20., 20.], content, (0, 0)), None); + } + + #[test] + fn fractional_moves_with_the_same_filtered_pixels_reuse() { + let mut requests = requests(1); + let content = [0., 0., 64., 48.]; + requests[0].region = mask_region([0.1, 4.1, 32., 24.], content, (64, 48)).unwrap(); + let previous = cached(&requests); + requests[0].region = mask_region([0.2, 4.2, 32., 24.], content, (64, 48)).unwrap(); + assert_eq!(plan_masks(&previous, &requests).reuse, [Some(0)]); + requests[0].region = mask_region([0.6, 4.2, 32., 24.], content, (64, 48)).unwrap(); + assert_eq!(plan_masks(&previous, &requests).reuse, [None]); + } + + #[test] + fn overlapping_masks_always_sample_the_unmodified_frame() { + let rgba: Vec = (0..64 * 48 * 4).map(|i| (i * 37 % 251) as u8).collect(); + let mut requests = requests(4); + requests[1].pixelate = true; + let previous = update_masks(&[], &requests, |request| { + request.filter(&rgba).map(Arc::new) + }); + requests[1].region.0 += 1; + requests.reverse(); + let mut calls = 0; + let next = update_masks(&previous, &requests, |request| { + calls += 1; + request.filter(&rgba).map(Arc::new) + }); + assert_eq!(calls, 1); + for entry in next { + let expected = mask_filter( + &rgba, + 64, + 48, + entry.request.region, + entry.request.pixelate, + 16., + ) + .unwrap(); + assert_eq!(entry.value.unwrap().as_ref(), &expected); + } + } + + #[test] + fn invalid_buffers_do_not_produce_images() { + let request = requests(1).remove(0); + assert!(request.filter(&[0; 16]).is_none()); + } +} diff --git a/apps/desktop-gpui/src/screenshot_annotations.rs b/apps/desktop-gpui/src/screenshot_annotations.rs index 30118d7e579..1f69bdf4a56 100644 --- a/apps/desktop-gpui/src/screenshot_annotations.rs +++ b/apps/desktop-gpui/src/screenshot_annotations.rs @@ -25,10 +25,10 @@ use std::cell::Cell; use std::collections::HashMap; -use std::hash::{Hash as _, Hasher as _}; use std::rc::Rc; use std::sync::Arc; +use cap_gpui_kernels::mask_cache::{MaskEntry, MaskFrame, MaskRequest, update_masks}; use cap_project::{Annotation, AnnotationType, AspectRatio, Crop, MaskType}; use gpui::{ AnyElement, App, AppContext as _, Bounds, Context, Corners, CursorStyle, Entity, FontWeight, @@ -1201,11 +1201,8 @@ pub struct AnnotationState { /// The layers list's rows container, for the reorder drag's hit testing. list: BoundsCell, reorder: Option, - masks: Vec, - /// The inputs the painted overlays were computed from, and the ones a - /// computation in flight is for. - mask_key: u64, - mask_pending: Option, + masks: Vec>, + mask_pending: Option>, mask_task: Option>, } @@ -2241,88 +2238,86 @@ impl ScreenshotEditorWindow { // -- Mask overlays --------------------------------------------------------- - /// What the painted overlays were computed from: the frame, the image rect, - /// and every mask's geometry and style. - fn mask_key(&self) -> u64 { - let mut hasher = std::hash::DefaultHasher::new(); - match &self.frame_rgba { - Some(rgba) => { - (Arc::as_ptr(rgba) as usize).hash(&mut hasher); - rgba.len().hash(&mut hasher); - } - None => 0usize.hash(&mut hasher), + fn mask_requests(&self) -> Vec { + if self.frame_rgba.is_none() { + return Vec::new(); } - self.frame_size.0.to_bits().hash(&mut hasher); - self.frame_size.1.to_bits().hash(&mut hasher); + let size = ( + self.frame_size.0.max(0.) as u32, + self.frame_size.1.max(0.) as u32, + ); let rect = self.annotation_image_rect(); - for value in [rect.x, rect.y, rect.width, rect.height] { - value.to_bits().hash(&mut hasher); - } - for annotation in &self.project.annotations { - if annotation.annotation_type != AnnotationType::Mask { - continue; - } - annotation.id.hash(&mut hasher); - for value in [ - annotation.x, - annotation.y, - annotation.width, - annotation.height, - annotation.mask_level.unwrap_or(MASK_FALLBACK_LEVEL), - ] { - value.to_bits().hash(&mut hasher); - } - matches!(annotation.mask_type, Some(MaskType::Pixelate)).hash(&mut hasher); - } - hasher.finish() + let frame = MaskFrame { + revision: self.mask_frame_revision, + size, + content: [rect.x, rect.y, rect.width, rect.height].map(f64::to_bits), + padding: self.project.background.padding.to_bits(), + crop: self + .project + .background + .crop + .as_ref() + .map(|crop| [crop.position.x, crop.position.y, crop.size.x, crop.size.y]), + }; + self.project + .annotations + .iter() + .filter_map(|mask| mask_request(mask, frame)) + .collect() } /// `renderMaskOverlays` (`Preview.tsx:579-687`), off the main thread. The /// stale overlay keeps painting until the new one lands, so a drag never /// blocks on a resample. pub(crate) fn refresh_mask_overlays(&mut self, window: &mut Window, cx: &mut Context) { - let key = self.mask_key(); - if self.annotation_state.mask_key == key || self.annotation_state.mask_pending == Some(key) - { + let requests = self.mask_requests(); + if self.annotation_state.mask_pending.as_ref() == Some(&requests) { return; } + self.annotation_state.mask_task = None; + self.annotation_state.mask_pending = None; - let masks: Vec = self - .project - .annotations + let retained: Vec<_> = requests .iter() - .filter(|annotation| annotation.annotation_type == AnnotationType::Mask) - .cloned() + .filter_map(|request| { + self.annotation_state + .masks + .iter() + .find(|entry| entry.request.id == request.id) + .cloned() + }) .collect(); - let frame = self.frame_rgba.clone(); - - let (Some(rgba), false) = (frame, masks.is_empty()) else { - self.drop_mask_overlays(window); - // Dropping the task matters as much as dropping the tiles: a - // resample still in flight would re-install the regions of a mask - // that has just been deleted, and stamp its key over this one. - self.annotation_state.mask_task = None; - self.annotation_state.mask_key = key; - self.annotation_state.mask_pending = None; + if !retained.iter().map(|entry| &entry.request.id).eq(self + .annotation_state + .masks + .iter() + .map(|entry| &entry.request.id)) + { + self.install_mask_overlays(retained, window); cx.notify(); + } + if self + .annotation_state + .masks + .iter() + .map(|entry| &entry.request) + .eq(requests.iter()) + { return; - }; + } - let size = ( - self.frame_size.0.max(0.) as u32, - self.frame_size.1.max(0.) as u32, - ); - let rect = self.annotation_image_rect(); - self.annotation_state.mask_pending = Some(key); + let previous = self.annotation_state.masks.clone(); + let Some(rgba) = self.frame_rgba.clone() else { + return; + }; + self.annotation_state.mask_pending = Some(requests.clone()); self.annotation_state.mask_task = Some(cx.spawn_in(window, async move |this, cx| { let overlays = cx .background_executor() - .spawn(async move { build_mask_overlays(&rgba, size, &masks, rect) }) + .spawn(async move { build_mask_overlays(&rgba, &previous, &requests) }) .await; this.update_in(cx, |this, window, cx| { - this.drop_mask_overlays(window); - this.annotation_state.masks = overlays; - this.annotation_state.mask_key = key; + this.install_mask_overlays(overlays, window); this.annotation_state.mask_pending = None; cx.notify(); // The masks may have moved on while this was in flight. @@ -2332,11 +2327,22 @@ impl ScreenshotEditorWindow { })); } - /// Hand the atlas back the tiles the old overlays held. - fn drop_mask_overlays(&mut self, window: &mut Window) { - for overlay in self.annotation_state.masks.drain(..) { - let _ = window.drop_image(overlay.image); + fn install_mask_overlays( + &mut self, + overlays: Vec>, + window: &mut Window, + ) { + for previous in &self.annotation_state.masks { + if let Some(previous) = &previous.value + && !overlays + .iter() + .filter_map(|entry| entry.value.as_ref()) + .any(|next| Arc::ptr_eq(&previous.image, &next.image)) + { + let _ = window.drop_image(previous.image.clone()); + } } + self.annotation_state.masks = overlays; } } @@ -2365,7 +2371,12 @@ impl ScreenshotEditorWindow { let paint = LayerPaint { annotations: self.project.annotations.clone(), temp, - masks: self.annotation_state.masks.clone(), + masks: self + .annotation_state + .masks + .iter() + .filter_map(|entry| entry.value.clone()) + .collect(), selected: self.selected_annotation.clone(), editing: self .annotation_state @@ -3943,34 +3954,49 @@ fn checker_quarters(size: f32) -> Vec { /// pixelate over a 4K screenshot is still not main-thread work. fn build_mask_overlays( rgba: &[u8], - size: (u32, u32), - masks: &[Annotation], - rect: Rect, -) -> Vec { - let mut overlays = Vec::new(); - for mask in masks { - let Some((x0, y0, processed)) = masked_region_image(rgba, size, mask, rect) else { - continue; - }; - overlays.push(MaskOverlay { + previous: &[MaskEntry], + requests: &[MaskRequest], +) -> Vec> { + update_masks(previous, requests, |request| { + let processed = request.filter(rgba)?; + Some(MaskOverlay { rect: Rect { - x: x0 as f64, - y: y0 as f64, + x: request.region.0 as f64, + y: request.region.1 as f64, width: processed.width() as f64, height: processed.height() as f64, }, image: crate::library::rgba_to_render_image(processed), - }); + }) + }) +} + +fn mask_request(mask: &Annotation, frame: MaskFrame) -> Option { + if mask.annotation_type != AnnotationType::Mask { + return None; } - overlays + Some(MaskRequest { + id: mask.id.clone(), + region: cap_gpui_kernels::mask_cache::mask_region( + [mask.x, mask.y, mask.width, mask.height], + frame.content.map(f64::from_bits), + frame.size, + )?, + frame, + pixelate: mask.mask_type == Some(MaskType::Pixelate), + level: mask + .mask_level + .unwrap_or(MASK_FALLBACK_LEVEL) + .max(1.) + .to_bits(), + }) } /// One mask's blurred or pixelated pixels, sampled from `rgba` (an unmodified /// copy of the frame) and returned with the region's top-left in frame -/// coordinates. This is the single implementation of the two filters -- the -/// preview's overlay tiles come through it, and so does the export's -/// `applyMaskAnnotations` pass, which calls it with `rect` set to the whole -/// canvas (`screenshotExport.ts:290-295` passes `{x:0,y:0,w,h}`). +/// coordinates. The export's `applyMaskAnnotations` pass calls it with +/// `rect` set to the whole canvas (`screenshotExport.ts:290-295` passes +/// `{x:0,y:0,w,h}`). pub(crate) fn masked_region_image( rgba: &[u8], size: (u32, u32), @@ -4003,18 +4029,11 @@ pub(crate) fn masked_region_image( /// One mask's region in whole frame pixels: its own rect, clipped to the /// screenshot's content rect and then to the frame. fn mask_region(mask: &Annotation, rect: Rect, frame: (u32, u32)) -> Option<(u32, u32, u32, u32)> { - let left = rect.x.max(mask.x.min(mask.x + mask.width)); - let top = rect.y.max(mask.y.min(mask.y + mask.height)); - let right = (rect.x + rect.width).min(mask.x.max(mask.x + mask.width)); - let bottom = (rect.y + rect.height).min(mask.y.max(mask.y + mask.height)); - if right <= left || bottom <= top { - return None; - } - let x0 = (left.round().max(0.) as u32).min(frame.0); - let y0 = (top.round().max(0.) as u32).min(frame.1); - let x1 = (right.round().max(0.) as u32).min(frame.0); - let y1 = (bottom.round().max(0.) as u32).min(frame.1); - (x1 > x0 && y1 > y0).then(|| (x0, y0, x1 - x0, y1 - y0)) + cap_gpui_kernels::mask_cache::mask_region( + [mask.x, mask.y, mask.width, mask.height], + [rect.x, rect.y, rect.width, rect.height], + frame, + ) } #[cfg(test)] @@ -4523,10 +4542,21 @@ mod tests { pixelate.mask_type = Some(MaskType::Pixelate); pixelate.mask_level = Some(7.); - let overlays = build_mask_overlays(&rgba, frame, &[blur, pixelate], rect); + let input = MaskFrame { + revision: 1, + size: frame, + content: [rect.x, rect.y, rect.width, rect.height].map(f64::to_bits), + padding: 0_f64.to_bits(), + crop: None, + }; + let requests: Vec<_> = [blur, pixelate] + .iter() + .filter_map(|mask| mask_request(mask, input)) + .collect(); + let overlays = build_mask_overlays(&rgba, &[], &requests); assert_eq!(overlays.len(), 2); assert_eq!( - overlays[0].rect, + overlays[0].value.as_ref().unwrap().rect, Rect { x: 8., y: 8., @@ -4535,7 +4565,7 @@ mod tests { } ); assert_eq!( - overlays[1].rect, + overlays[1].value.as_ref().unwrap().rect, Rect { x: 40., y: 30., @@ -4546,7 +4576,18 @@ mod tests { // A frame buffer that does not match the size it was handed is not // sliced at all. - assert!(build_mask_overlays(&rgba[..16], frame, &[], rect).is_empty()); + assert!( + build_mask_overlays(&rgba[..16], &[], &requests) + .iter() + .all(|entry| entry.value.is_none()) + ); + let reused = build_mask_overlays(&rgba, &overlays, &requests); + for (previous, next) in overlays.iter().zip(&reused) { + assert!(Arc::ptr_eq( + &previous.value.as_ref().unwrap().image, + &next.value.as_ref().unwrap().image + )); + } } /// A finished stroke stores fractions of its own box, so a later resize diff --git a/apps/desktop-gpui/src/screenshot_editor.rs b/apps/desktop-gpui/src/screenshot_editor.rs index 5d727399df5..f5f4175882e 100644 --- a/apps/desktop-gpui/src/screenshot_editor.rs +++ b/apps/desktop-gpui/src/screenshot_editor.rs @@ -917,6 +917,7 @@ pub struct ScreenshotEditorWindow { /// back to blur/pixelate mask regions (`:537-687`); the mask overlay /// resamples this instead of the atlas image. pub(crate) frame_rgba: Option>>, + pub(crate) mask_frame_revision: u64, pub(crate) frame_size: (f32, f32), /// `originalImageSize` -- the source PNG's dimensions, which the crop /// button gates on and the frame-space transform needs. @@ -1001,6 +1002,7 @@ impl ScreenshotEditorWindow { export_status: ExportStatus::Idle, frame: None, frame_rgba: None, + mask_frame_revision: 0, frame_size: (1., 1.), image_size: None, previous_transform: None, @@ -1130,6 +1132,10 @@ impl ScreenshotEditorWindow { self.rescale_annotations_for(next); self.frame_size = next; self.frame_rgba = Some(rgba); + self.mask_frame_revision = self + .mask_frame_revision + .checked_add(1) + .expect("frame revision"); self.ready = true; if let Some(previous) = self.frame.replace(image) { let _ = window.drop_image(previous); From 63593ed71a707e665996bf5d5f72c2d2c8215ac2 Mon Sep 17 00:00:00 2001 From: flamboh Date: Tue, 13 Oct 2026 17:40:55 -0700 Subject: [PATCH 2/4] fix(desktop-gpui): retain duplicate-id screenshot masks safely --- .../screenshot-mask-cache-benchmark.rs | 65 ++++++--- apps/desktop-gpui/kernels/src/mask_cache.rs | 134 +++++++++++++----- .../src/screenshot_annotations.rs | 82 ++++++++--- 3 files changed, 204 insertions(+), 77 deletions(-) diff --git a/apps/desktop-gpui/kernels/examples/screenshot-mask-cache-benchmark.rs b/apps/desktop-gpui/kernels/examples/screenshot-mask-cache-benchmark.rs index 9c1dc29afef..0dfc358085f 100644 --- a/apps/desktop-gpui/kernels/examples/screenshot-mask-cache-benchmark.rs +++ b/apps/desktop-gpui/kernels/examples/screenshot-mask-cache-benchmark.rs @@ -4,7 +4,26 @@ use std::{hint::black_box, sync::Arc, time::Instant}; const SIZE: (u32, u32) = (3840, 2160); const WARMUP: usize = 12; const EVENTS: usize = 120; -type Overlay = MaskEntry>; +#[derive(Clone)] +struct Overlay { + rect: [f64; 4], + image: Arc, +} + +type CachedOverlay = MaskEntry; + +fn overlay(request: &MaskRequest, rgba: &[u8]) -> Option { + let processed = request.filter(rgba)?; + Some(Overlay { + rect: [ + f64::from(request.region.0), + f64::from(request.region.1), + f64::from(processed.width()), + f64::from(processed.height()), + ], + image: Arc::new(processed), + }) +} fn requests(count: usize, pixelate: bool, event: usize) -> Vec { let content = [0., 0., f64::from(SIZE.0), f64::from(SIZE.1)]; @@ -30,27 +49,25 @@ fn requests(count: usize, pixelate: bool, event: usize) -> Vec { } fn rebuild_all(requests: &[MaskRequest], rgba: &[u8], calls: &mut usize) -> Vec { - requests - .iter() - .map(|request| { - *calls += 1; - MaskEntry { - request: request.clone(), - value: request.filter(rgba).map(Arc::new), - } - }) - .collect() + let mut overlays = Vec::new(); + for request in requests { + *calls += 1; + if let Some(overlay) = overlay(request, rgba) { + overlays.push(overlay); + } + } + overlays } fn cached( - previous: &[Overlay], + previous: &[CachedOverlay], requests: &[MaskRequest], rgba: &[u8], calls: &mut usize, -) -> Vec { +) -> Vec { update_masks(previous, requests, |request| { *calls += 1; - request.filter(rgba).map(Arc::new) + overlay(request, rgba) }) } @@ -84,7 +101,7 @@ fn main() { "3840x2160 RGBA; 640x360 masks; level 16; {WARMUP} warmup pairs; {EVENTS} measured drag events" ); println!( - "Alternating old/new execution order; warm cache; byte comparison after every event outside timing." + "Alternating old/new execution order; warm cache; geometry and byte comparison after every event outside timing." ); println!( "Includes cache diff, filtering, Arc allocation and retired CPU image release. Excludes GPUI conversion, scheduling, atlas and painting." @@ -129,12 +146,20 @@ fn main() { assert_eq!(old.len(), count); assert_eq!(new.len(), count); for (old, new) in old.iter().zip(&new) { - assert_eq!(old.request, new.request); + let expected_rect = [ + f64::from(new.request.region.0), + f64::from(new.request.region.1), + f64::from(new.request.region.2), + f64::from(new.request.region.3), + ]; + assert_eq!(old.rect, expected_rect); + let new_overlay = new.value.as_ref().unwrap(); + assert_eq!(old.rect, new_overlay.rect); assert_eq!( - old.value.as_ref().unwrap().as_raw(), - new.value.as_ref().unwrap().as_raw(), + old.image.as_raw(), + new_overlay.image.as_raw(), "event {event}, mask {}", - old.request.id + new.request.id ); } if event <= WARMUP { @@ -156,5 +181,5 @@ fn main() { ); } } - println!("All overlay bytes matched at every warmup and measured event."); + println!("All overlay geometry and bytes matched at every warmup and measured event."); } diff --git a/apps/desktop-gpui/kernels/src/mask_cache.rs b/apps/desktop-gpui/kernels/src/mask_cache.rs index 9421b9a1ddd..958d09a8eca 100644 --- a/apps/desktop-gpui/kernels/src/mask_cache.rs +++ b/apps/desktop-gpui/kernels/src/mask_cache.rs @@ -1,4 +1,5 @@ use crate::screenshot::mask_filter; +use std::{collections::HashSet, hash::Hash}; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct MaskFrame { @@ -37,21 +38,63 @@ pub struct MaskEntry { pub value: Option, } -#[derive(Debug, PartialEq, Eq)] -pub struct MaskPlan { - pub reuse: Vec>, - pub drop: Vec, +pub fn plan_masks(previous: &[MaskEntry], requests: &[MaskRequest]) -> Vec> { + let mut used = vec![false; previous.len()]; + requests + .iter() + .map(|request| { + let index = previous + .iter() + .enumerate() + .position(|(index, entry)| !used[index] && entry.request == *request)?; + used[index] = true; + Some(index) + }) + .collect() +} + +pub fn retain_masks( + previous: &[MaskEntry], + requests: &[MaskRequest], +) -> Vec> { + let mut matches = plan_masks(previous, requests); + let mut used = vec![false; previous.len()]; + for index in matches.iter().flatten() { + used[*index] = true; + } + for (request, matched) in requests.iter().zip(&mut matches) { + if matched.is_none() + && let Some(index) = previous + .iter() + .enumerate() + .position(|(index, entry)| !used[index] && entry.request.id == request.id) + { + used[index] = true; + *matched = Some(index); + } + } + matches + .into_iter() + .flatten() + .map(|index| previous[index].clone()) + .collect() } -pub fn plan_masks(previous: &[MaskEntry], requests: &[MaskRequest]) -> MaskPlan { - let reuse: Vec<_> = requests +pub fn retired_mask_values<'a, T, K: Eq + Hash>( + previous: &'a [MaskEntry], + next: &[MaskEntry], + key: impl Fn(&T) -> K, +) -> Vec<&'a T> { + let mut referenced: HashSet<_> = next .iter() - .map(|request| previous.iter().position(|entry| entry.request == *request)) - .collect(); - let drop = (0..previous.len()) - .filter(|index| !reuse.contains(&Some(*index))) + .filter_map(|entry| entry.value.as_ref()) + .map(&key) .collect(); - MaskPlan { reuse, drop } + previous + .iter() + .filter_map(|entry| entry.value.as_ref()) + .filter(|value| referenced.insert(key(value))) + .collect() } pub fn update_masks( @@ -62,7 +105,7 @@ pub fn update_masks( let plan = plan_masks(previous, requests); requests .iter() - .zip(plan.reuse) + .zip(plan) .map(|(request, reuse)| MaskEntry { request: request.clone(), value: match reuse { @@ -128,10 +171,7 @@ mod tests { requests[1].region.0 += 1; assert_eq!( plan_masks(&previous, &requests), - MaskPlan { - reuse: vec![Some(0), None, Some(2), Some(3)], - drop: vec![1], - } + vec![Some(0), None, Some(2), Some(3)] ); let mut calls = 0; let next = update_masks(&previous, &requests, |request| { @@ -155,10 +195,7 @@ mod tests { requests.reverse(); assert_eq!( plan_masks(&previous, &requests), - MaskPlan { - reuse: vec![Some(3), Some(2), Some(0)], - drop: vec![1], - } + vec![Some(3), Some(2), Some(0)] ); let next = update_masks(&previous, &requests, |_| panic!("unexpected filter")); assert_eq!( @@ -167,7 +204,7 @@ mod tests { .collect::>(), ["3", "2", "0"] ); - assert_eq!(plan_masks(&next, &[]).drop, [0, 1, 2]); + assert_eq!(retired_mask_values(&next, &[], Arc::as_ptr).len(), 3); } #[test] @@ -178,10 +215,7 @@ mod tests { requests[1].level = 20_f64.to_bits(); assert_eq!( plan_masks(&previous, &requests), - MaskPlan { - reuse: vec![None, None, Some(2), None], - drop: vec![0, 1], - } + vec![None, None, Some(2), None] ); } @@ -200,13 +234,7 @@ mod tests { _ => request.frame.crop = Some([1, 0, 64, 48]), } } - assert_eq!( - plan_masks(&previous, &requests), - MaskPlan { - reuse: vec![None; 4], - drop: vec![0, 1, 2, 3], - } - ); + assert_eq!(plan_masks(&previous, &requests), vec![None; 4]); } } @@ -216,7 +244,43 @@ mod tests { let previous = update_masks::>(&[], &requests, |_| None); let next = update_masks(&previous, &requests, |_| panic!("unexpected filter")); assert!(next.iter().all(|entry| entry.value.is_none())); - assert_eq!(plan_masks(&previous, &requests).drop, []); + assert!(retired_mask_values(&previous, &next, Arc::as_ptr).is_empty()); + } + + #[test] + fn duplicate_ids_reserve_exact_matches_before_retaining_changed_masks() { + let mut requests = requests(3); + requests[1].id = requests[0].id.clone(); + let previous = cached(&requests); + requests.swap(0, 1); + requests[0].region.0 += 1; + requests.pop(); + let retained = retain_masks(&previous, &requests); + assert_eq!(retained[0].request, previous[1].request); + assert_eq!(retained[1].request, previous[0].request); + let retired = retired_mask_values(&previous, &retained, Arc::as_ptr); + assert_eq!(retired.len(), 1); + assert!(Arc::ptr_eq(retired[0], previous[2].value.as_ref().unwrap())); + } + + #[test] + fn identical_duplicate_requests_use_each_previous_entry_once() { + let request = requests(1).remove(0); + let requests = vec![request; 3]; + let previous = cached(&requests[..2]); + assert_eq!(plan_masks(&previous, &requests), [Some(0), Some(1), None]); + assert_eq!(retain_masks(&previous, &requests).len(), 2); + } + + #[test] + fn retirement_preserves_shared_images_and_retires_them_only_once() { + let requests = requests(2); + let image = Arc::new("shared".to_string()); + let previous = update_masks(&[], &requests, |_| Some(image.clone())); + assert!(retired_mask_values(&previous, &previous[1..], Arc::as_ptr).is_empty()); + let retired = retired_mask_values(&previous, &[], Arc::as_ptr); + assert_eq!(retired.len(), 1); + assert!(Arc::ptr_eq(retired[0], &image)); } #[test] @@ -242,9 +306,9 @@ mod tests { requests[0].region = mask_region([0.1, 4.1, 32., 24.], content, (64, 48)).unwrap(); let previous = cached(&requests); requests[0].region = mask_region([0.2, 4.2, 32., 24.], content, (64, 48)).unwrap(); - assert_eq!(plan_masks(&previous, &requests).reuse, [Some(0)]); + assert_eq!(plan_masks(&previous, &requests), [Some(0)]); requests[0].region = mask_region([0.6, 4.2, 32., 24.], content, (64, 48)).unwrap(); - assert_eq!(plan_masks(&previous, &requests).reuse, [None]); + assert_eq!(plan_masks(&previous, &requests), [None]); } #[test] diff --git a/apps/desktop-gpui/src/screenshot_annotations.rs b/apps/desktop-gpui/src/screenshot_annotations.rs index 1f69bdf4a56..484e14e3c67 100644 --- a/apps/desktop-gpui/src/screenshot_annotations.rs +++ b/apps/desktop-gpui/src/screenshot_annotations.rs @@ -28,7 +28,9 @@ use std::collections::HashMap; use std::rc::Rc; use std::sync::Arc; -use cap_gpui_kernels::mask_cache::{MaskEntry, MaskFrame, MaskRequest, update_masks}; +use cap_gpui_kernels::mask_cache::{ + MaskEntry, MaskFrame, MaskRequest, retain_masks, retired_mask_values, update_masks, +}; use cap_project::{Annotation, AnnotationType, AspectRatio, Crop, MaskType}; use gpui::{ AnyElement, App, AppContext as _, Bounds, Context, Corners, CursorStyle, Entity, FontWeight, @@ -2277,21 +2279,12 @@ impl ScreenshotEditorWindow { self.annotation_state.mask_task = None; self.annotation_state.mask_pending = None; - let retained: Vec<_> = requests - .iter() - .filter_map(|request| { - self.annotation_state - .masks - .iter() - .find(|entry| entry.request.id == request.id) - .cloned() - }) - .collect(); - if !retained.iter().map(|entry| &entry.request.id).eq(self + let retained = retain_masks(&self.annotation_state.masks, &requests); + if !retained.iter().map(|entry| &entry.request).eq(self .annotation_state .masks .iter() - .map(|entry| &entry.request.id)) + .map(|entry| &entry.request)) { self.install_mask_overlays(retained, window); cx.notify(); @@ -2332,15 +2325,10 @@ impl ScreenshotEditorWindow { overlays: Vec>, window: &mut Window, ) { - for previous in &self.annotation_state.masks { - if let Some(previous) = &previous.value - && !overlays - .iter() - .filter_map(|entry| entry.value.as_ref()) - .any(|next| Arc::ptr_eq(&previous.image, &next.image)) - { - let _ = window.drop_image(previous.image.clone()); - } + for previous in retired_mask_values(&self.annotation_state.masks, &overlays, |overlay| { + Arc::as_ptr(&overlay.image) + }) { + let _ = window.drop_image(previous.image.clone()); } self.annotation_state.masks = overlays; } @@ -4590,6 +4578,56 @@ mod tests { } } + #[test] + fn duplicate_mask_ids_keep_both_regions_when_another_mask_is_deleted() { + let frame = MaskFrame { + revision: 1, + size: (64, 48), + content: [0., 0., 64., 48.].map(f64::to_bits), + padding: 0_f64.to_bits(), + crop: None, + }; + let a = shape(AnnotationType::Mask, 0., 0., 16., 16.); + let b = shape(AnnotationType::Mask, 32., 24., 16., 16.); + let mut c = shape(AnnotationType::Mask, 48., 0., 16., 16.); + c.id = "unique".into(); + assert_eq!(a.id, b.id); + let mut requests: Vec<_> = [a, b, c] + .iter() + .filter_map(|mask| mask_request(mask, frame)) + .collect(); + let rgba = vec![128; 64 * 48 * 4]; + let previous = build_mask_overlays(&rgba, &[], &requests); + requests.pop(); + for reorder in [false, true] { + if reorder { + requests.reverse(); + } + let retained = retain_masks(&previous, &requests); + assert_eq!(retained.len(), 2); + for (entry, request) in retained.iter().zip(&requests) { + assert_eq!(&entry.request, request); + let overlay = entry.value.as_ref().unwrap(); + assert_eq!(overlay.rect.x, f64::from(request.region.0)); + assert_eq!(overlay.rect.y, f64::from(request.region.1)); + } + let retired = + retired_mask_values(&previous, &retained, |overlay| Arc::as_ptr(&overlay.image)); + assert_eq!(retired.len(), 1); + assert!(Arc::ptr_eq( + &retired[0].image, + &previous[2].value.as_ref().unwrap().image + )); + let rebuilt = build_mask_overlays(&rgba, &retained, &requests); + assert!( + retired_mask_values(&retained, &rebuilt, |overlay| { + Arc::as_ptr(&overlay.image) + }) + .is_empty() + ); + } + } + /// A finished stroke stores fractions of its own box, so a later resize /// carries the samples with it (`:449-453`, `:915-921`). #[test] From a2b578c1223ffbd8b02f668a29f960d0a928f224 Mon Sep 17 00:00:00 2001 From: flamboh Date: Tue, 13 Oct 2026 18:49:21 -0700 Subject: [PATCH 3/4] fix(desktop-gpui): coalesce screenshot mask overlay builds --- apps/desktop-gpui/kernels/src/mask_cache.rs | 184 +++++++++++++++++- .../src/screenshot_annotations.rs | 97 ++++++++- 2 files changed, 268 insertions(+), 13 deletions(-) diff --git a/apps/desktop-gpui/kernels/src/mask_cache.rs b/apps/desktop-gpui/kernels/src/mask_cache.rs index 958d09a8eca..b9e3afdcf03 100644 --- a/apps/desktop-gpui/kernels/src/mask_cache.rs +++ b/apps/desktop-gpui/kernels/src/mask_cache.rs @@ -38,6 +38,42 @@ pub struct MaskEntry { pub value: Option, } +pub fn should_cancel_mask_build(previous: &[MaskRequest], requests: &[MaskRequest]) -> bool { + let Some(first) = previous.first() else { + return true; + }; + if previous + .iter() + .chain(requests) + .any(|request| request.frame != first.frame) + { + return true; + } + let mut used = vec![false; requests.len()]; + previous.iter().any(|request| { + let duplicated = previous + .iter() + .filter(|other| other.id == request.id) + .count() + > 1 + || requests + .iter() + .filter(|other| other.id == request.id) + .count() + > 1; + let matched = requests.iter().enumerate().position(|(index, next)| { + !used[index] && next.id == request.id && (!duplicated || next == request) + }); + match matched { + Some(index) => { + used[index] = true; + false + } + None => true, + } + }) +} + pub fn plan_masks(previous: &[MaskEntry], requests: &[MaskRequest]) -> Vec> { let mut used = vec![false; previous.len()]; requests @@ -53,10 +89,7 @@ pub fn plan_masks(previous: &[MaskEntry], requests: &[MaskRequest]) -> Vec .collect() } -pub fn retain_masks( - previous: &[MaskEntry], - requests: &[MaskRequest], -) -> Vec> { +fn match_masks(previous: &[MaskEntry], requests: &[MaskRequest]) -> Vec> { let mut matches = plan_masks(previous, requests); let mut used = vec![false; previous.len()]; for index in matches.iter().flatten() { @@ -74,12 +107,38 @@ pub fn retain_masks( } } matches +} + +pub fn retain_masks( + previous: &[MaskEntry], + requests: &[MaskRequest], +) -> Vec> { + match_masks(previous, requests) .into_iter() .flatten() .map(|index| previous[index].clone()) .collect() } +pub fn finish_mask_build( + current: &[MaskEntry], + completed: Vec>, +) -> Vec> { + let requests: Vec<_> = completed + .iter() + .map(|entry| entry.request.clone()) + .collect(); + let matches = match_masks(current, &requests); + completed + .into_iter() + .zip(matches) + .map(|(entry, matched)| match matched { + Some(index) if current[index].request == entry.request => current[index].clone(), + _ => entry, + }) + .collect() +} + pub fn retired_mask_values<'a, T, K: Eq + Hash>( previous: &'a [MaskEntry], next: &[MaskEntry], @@ -164,6 +223,123 @@ mod tests { update_masks(&[], requests, |request| Some(Arc::new(request.id.clone()))) } + #[test] + fn mask_build_keeps_moves_styles_additions_and_reorders() { + let original = requests(3); + for change in 0..5 { + let mut next = original.clone(); + match change { + 0 => next[0].region.0 += 1, + 1 => next[0].level = 20_f64.to_bits(), + 2 => next[0].pixelate = true, + 3 => next.push(requests(4).pop().unwrap()), + _ => next.reverse(), + } + assert!(!should_cancel_mask_build(&original, &next)); + } + assert!(!should_cancel_mask_build(&original, &original)); + } + + #[test] + fn mask_build_cancels_removals_even_when_another_mask_is_added() { + let original = requests(3); + assert!(should_cancel_mask_build(&original, &[])); + assert!(should_cancel_mask_build(&original, &original[1..])); + let mut next = original[1..].to_vec(); + next.push(requests(4).pop().unwrap()); + assert!(should_cancel_mask_build(&original, &next)); + } + + #[test] + fn mask_build_cancels_frame_and_layout_changes() { + let original = requests(2); + for change in 0..5 { + let mut next = original.clone(); + for request in &mut next { + match change { + 0 => request.frame.revision += 1, + 1 => request.frame.size.0 += 1, + 2 => request.frame.content[0] = 0.25_f64.to_bits(), + 3 => request.frame.padding = 8_f64.to_bits(), + _ => request.frame.crop = Some([0, 0, 32, 24]), + } + } + assert!(should_cancel_mask_build(&original, &next)); + } + } + + #[test] + fn mask_build_cancels_ambiguous_duplicate_changes_and_removals() { + let mut original = requests(2); + original[1].id = original[0].id.clone(); + let mut next = original.clone(); + next.reverse(); + assert!(!should_cancel_mask_build(&original, &next)); + next[0].region.0 += 1; + assert!(should_cancel_mask_build(&original, &next)); + assert!(should_cancel_mask_build(&original, &original[..1])); + next[0] = requests(3).pop().unwrap(); + assert!(should_cancel_mask_build(&original, &next)); + let identical = vec![original[0].clone(); 2]; + assert!(!should_cancel_mask_build(&identical, &identical)); + assert!(should_cancel_mask_build(&identical, &identical[..1])); + } + + #[test] + fn mask_build_checks_removal_against_the_latest_coalesced_request() { + let initial = requests(1); + let added = requests(2); + assert!(!should_cancel_mask_build(&initial, &added)); + assert!(should_cancel_mask_build(&added, &initial)); + assert!(!should_cancel_mask_build(&initial, &added)); + } + + #[test] + fn mask_build_cancels_inconsistent_frames_and_empty_builds() { + let original = requests(2); + let mut inconsistent = original.clone(); + inconsistent[1].frame.revision += 1; + assert!(should_cancel_mask_build(&original, &inconsistent)); + assert!(should_cancel_mask_build(&inconsistent, &original)); + assert!(should_cancel_mask_build(&[], &original)); + } + + #[test] + fn completed_mask_build_matches_current_duplicates_before_retiring_images() { + let mut requests = requests(2); + requests[1].id = requests[0].id.clone(); + let snapshot = cached(&requests); + let mut current = cached(&requests); + current.reverse(); + let mut built_requests = requests.clone(); + built_requests[0].region.0 += 1; + let completed = update_masks(&snapshot, &built_requests, |request| { + Some(Arc::new(request.id.clone())) + }); + let changed = completed[0].value.clone().unwrap(); + let installed = finish_mask_build(¤t, completed); + assert!(Arc::ptr_eq(installed[0].value.as_ref().unwrap(), &changed)); + assert!(Arc::ptr_eq( + installed[1].value.as_ref().unwrap(), + current[0].value.as_ref().unwrap() + )); + let retired = retired_mask_values(¤t, &installed, Arc::as_ptr); + assert_eq!(retired.len(), 1); + assert!(Arc::ptr_eq(retired[0], current[1].value.as_ref().unwrap())); + } + + #[test] + fn completed_mask_build_does_not_retire_shared_current_images() { + let requests = requests(2); + let image = Arc::new("shared".to_string()); + let current = update_masks(&[], &requests, |_| Some(image.clone())); + let mut built_requests = requests.clone(); + built_requests[0].region.0 += 1; + let installed = finish_mask_build(¤t, cached(&built_requests)); + assert!(retired_mask_values(¤t, &installed, Arc::as_ptr).is_empty()); + assert!(Arc::ptr_eq(installed[1].value.as_ref().unwrap(), &image)); + } + #[test] fn moving_one_mask_reuses_the_other_images() { let mut requests = requests(4); diff --git a/apps/desktop-gpui/src/screenshot_annotations.rs b/apps/desktop-gpui/src/screenshot_annotations.rs index 484e14e3c67..2bd7b539353 100644 --- a/apps/desktop-gpui/src/screenshot_annotations.rs +++ b/apps/desktop-gpui/src/screenshot_annotations.rs @@ -29,7 +29,8 @@ use std::rc::Rc; use std::sync::Arc; use cap_gpui_kernels::mask_cache::{ - MaskEntry, MaskFrame, MaskRequest, retain_masks, retired_mask_values, update_masks, + MaskEntry, MaskFrame, MaskRequest, finish_mask_build, retain_masks, retired_mask_values, + should_cancel_mask_build, update_masks, }; use cap_project::{Annotation, AnnotationType, AspectRatio, Crop, MaskType}; use gpui::{ @@ -2276,8 +2277,18 @@ impl ScreenshotEditorWindow { if self.annotation_state.mask_pending.as_ref() == Some(&requests) { return; } - self.annotation_state.mask_task = None; - self.annotation_state.mask_pending = None; + let keep_build = self + .annotation_state + .mask_pending + .as_ref() + .is_some_and(|pending| !should_cancel_mask_build(pending, &requests)); + if keep_build { + // Cancelling on each drag tick can starve installation even when filtering is fast. + self.annotation_state.mask_pending = Some(requests.clone()); + } else { + self.annotation_state.mask_task = None; + self.annotation_state.mask_pending = None; + } let retained = retain_masks(&self.annotation_state.masks, &requests); if !retained.iter().map(|entry| &entry.request).eq(self @@ -2289,12 +2300,13 @@ impl ScreenshotEditorWindow { self.install_mask_overlays(retained, window); cx.notify(); } - if self - .annotation_state - .masks - .iter() - .map(|entry| &entry.request) - .eq(requests.iter()) + if keep_build + || self + .annotation_state + .masks + .iter() + .map(|entry| &entry.request) + .eq(requests.iter()) { return; } @@ -2310,6 +2322,7 @@ impl ScreenshotEditorWindow { .spawn(async move { build_mask_overlays(&rgba, &previous, &requests) }) .await; this.update_in(cx, |this, window, cx| { + let overlays = finish_mask_build(&this.annotation_state.masks, overlays); this.install_mask_overlays(overlays, window); this.annotation_state.mask_pending = None; cx.notify(); @@ -4628,6 +4641,72 @@ mod tests { } } + #[test] + fn coalesced_mask_build_installs_progress_then_catches_up_without_retiring_reused_images() { + let frame = MaskFrame { + revision: 1, + size: (64, 48), + content: [0., 0., 64., 48.].map(f64::to_bits), + padding: 0_f64.to_bits(), + crop: None, + }; + let a = shape(AnnotationType::Mask, 0., 0., 16., 16.); + let b = shape(AnnotationType::Mask, 32., 24., 16., 16.); + let mut moving = shape(AnnotationType::Mask, 40., 0., 16., 16.); + moving.id = "moving".into(); + let original: Vec<_> = [a, b, moving] + .iter() + .filter_map(|mask| mask_request(mask, frame)) + .collect(); + let rgba = vec![128; 64 * 48 * 4]; + let previous = build_mask_overlays(&rgba, &[], &original); + let mut started = original.clone(); + started[2].region.0 += 1; + let mut latest = started.clone(); + for _ in 0..8 { + let mut next = latest.clone(); + next[2].region.0 -= 1; + assert!(!should_cancel_mask_build(&latest, &next)); + latest = next; + } + latest.reverse(); + assert!(!should_cancel_mask_build(&started, &latest)); + let current = retain_masks(&previous, &latest); + let completed = build_mask_overlays(&rgba, &previous, &started); + let installed = finish_mask_build(¤t, completed); + assert_eq!(installed[2].request, started[2]); + let retired = + retired_mask_values(¤t, &installed, |overlay| Arc::as_ptr(&overlay.image)); + assert_eq!(retired.len(), 1); + assert!(Arc::ptr_eq( + &retired[0].image, + &previous[2].value.as_ref().unwrap().image + )); + for index in 0..2 { + assert!(Arc::ptr_eq( + &installed[index].value.as_ref().unwrap().image, + &previous[index].value.as_ref().unwrap().image + )); + } + let retained = retain_masks(&installed, &latest); + assert!( + retired_mask_values(&installed, &retained, |overlay| Arc::as_ptr(&overlay.image)) + .is_empty() + ); + let completed = build_mask_overlays(&rgba, &retained, &latest); + let caught_up = finish_mask_build(&retained, completed); + assert!( + caught_up + .iter() + .map(|entry| &entry.request) + .eq(latest.iter()) + ); + assert_eq!( + retired_mask_values(&retained, &caught_up, |overlay| Arc::as_ptr(&overlay.image)).len(), + 1 + ); + } + /// A finished stroke stores fractions of its own box, so a later resize /// carries the samples with it (`:449-453`, `:915-921`). #[test] From 0c0ef19114ceae124447d8d5b97b0cf159f1bb97 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Tue, 20 Oct 2026 09:07:42 +0800 Subject: [PATCH 4/4] chore: drop the standalone mask-cache benchmark package --- .../kernels/benchmarks/mask-cache/Cargo.lock | 65 ------------------- .../kernels/benchmarks/mask-cache/Cargo.toml | 22 ------- .../kernels/benchmarks/mask-cache/README.md | 60 ----------------- .../kernels/benchmarks/mask-cache/lib.rs | 4 -- 4 files changed, 151 deletions(-) delete mode 100644 apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.lock delete mode 100644 apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml delete mode 100644 apps/desktop-gpui/kernels/benchmarks/mask-cache/README.md delete mode 100644 apps/desktop-gpui/kernels/benchmarks/mask-cache/lib.rs diff --git a/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.lock b/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.lock deleted file mode 100644 index 65c42077127..00000000000 --- a/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.lock +++ /dev/null @@ -1,65 +0,0 @@ -# This file is automatically @generated by Cargo. -# It is not intended for manual editing. -version = 4 - -[[package]] -name = "autocfg" -version = "1.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" - -[[package]] -name = "bytemuck" -version = "1.25.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "95832e849adfb21180ccb6826a99da14e5d266ae5c2e668e1602cf234f153797" - -[[package]] -name = "byteorder-lite" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f1fe948ff07f4bd06c30984e69f5b4899c516a3ef74f34df92a2df2ab535495" - -[[package]] -name = "cap-screenshot-mask-cache-benchmark" -version = "0.1.0" -dependencies = [ - "image", -] - -[[package]] -name = "image" -version = "0.25.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85ab80394333c02fe689eaf900ab500fbd0c2213da414687ebf995a65d5a6104" -dependencies = [ - "bytemuck", - "byteorder-lite", - "moxcms", - "num-traits", -] - -[[package]] -name = "moxcms" -version = "0.8.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb85c154ba489f01b25c0d36ae69a87e4a1c73a72631fc6c0eb6dde34a73e44b" -dependencies = [ - "num-traits", - "pxfm", -] - -[[package]] -name = "num-traits" -version = "0.2.19" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" -dependencies = [ - "autocfg", -] - -[[package]] -name = "pxfm" -version = "0.1.30" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d55d956fa96f5ec02be2e13af0e20391a5aa83d6a074e3ad368959d0fab299ea" diff --git a/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml b/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml deleted file mode 100644 index bda064d8078..00000000000 --- a/apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml +++ /dev/null @@ -1,22 +0,0 @@ -[workspace] - -[package] -name = "cap-screenshot-mask-cache-benchmark" -version = "0.1.0" -edition = "2024" -publish = false - -[lib] -name = "cap_gpui_kernels" -path = "lib.rs" - -[[example]] -name = "screenshot-mask-cache-benchmark" -path = "../../examples/screenshot-mask-cache-benchmark.rs" - -[dependencies] -image = { version = "=0.25.10", default-features = false } - -[profile.release] -codegen-units = 1 -lto = "thin" diff --git a/apps/desktop-gpui/kernels/benchmarks/mask-cache/README.md b/apps/desktop-gpui/kernels/benchmarks/mask-cache/README.md deleted file mode 100644 index 30fca7dabe6..00000000000 --- a/apps/desktop-gpui/kernels/benchmarks/mask-cache/README.md +++ /dev/null @@ -1,60 +0,0 @@ -# Screenshot mask cache benchmark - -Run from the repository root: - -```sh -cargo +stable run --locked --release \ - --manifest-path apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml \ - --example screenshot-mask-cache-benchmark -``` - -This standalone package compiles the production `mask_cache.rs` and `screenshot.rs` directly. It requires Rust and the `image` crate, with a committed lockfile matching the desktop workspace's image/filter dependencies. It does not build GPUI, audio, FFmpeg, or platform libraries, and can run on Linux x86_64 as well as macOS arm64. The same example is also available through `cargo run -p cap-gpui-kernels --release --example screenshot-mask-cache-benchmark` in the desktop workspace, when its full dependency prerequisites are installed. - -The fixture is a deterministic, nonuniform 3840 × 2160 RGBA image with varying alpha. Each workload has 1, 4, 8, or 16 masks of 640 × 360 pixels at level 16, using either blur or pixelate. One mask moves one pixel per event; the others stay fixed. Each workload primes the cache, runs 12 warmup pairs, then measures 120 drag events. The old path filters every mask on every update. The new path calls the production cache diff and filters only changed masks. Both paths call the unchanged production `mask_filter`. - -Execution order alternates between old-first and new-first to reduce order bias. Every overlay's request and all of its output bytes are compared after every warmup and measured event, outside the timed intervals. A mismatch or unexpected filter-call count fails the run. Filter-call counts in the table cover the 120 measured events only; cache priming and warmup are excluded. The speedup is the ratio of means. The median is the average of the two middle samples. - -Timings include cache diffing, region extraction/filtering, image allocation, `Arc` ownership, and release of retired CPU images. Request/geometry construction and correctness comparisons are outside timing. The benchmark excludes GPUI image conversion, task scheduling/cancellation, GPU atlas uploads, painting, and screenshot export. It measures completed CPU preparation work, not pointer-event latency or application FPS. One mask is a control case: it has no reusable image during a drag. A replaced frame invalidates every mask and is not expected to benefit. Overlapping masks, deletion, reordering, style changes, frame/layout invalidation, and fractional/clipped rectangles are covered by the production module's unit tests. - -Run the portable tests and lint checks: - -```sh -cargo +stable test --locked \ - --manifest-path apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml \ - --all-targets -cargo +stable clippy --locked \ - --manifest-path apps/desktop-gpui/kernels/benchmarks/mask-cache/Cargo.toml \ - --all-targets -- -D warnings -``` - -Run the release command at least twice on an otherwise idle machine. Keep the full output, CPU model, operating system, Rust version, and power mode with results. Compare both means and medians, including the one-mask control. Cross-architecture results should use the same lockfile and workload. - -## Recorded results - -Two consecutive release runs on Apple M4 Pro, macOS 27.2, Rust 1.97.0, battery power with automatic power mode, October 13, 2026. All overlay bytes matched after every event in both runs. These are CPU overlay-preparation measurements with the exclusions described above. - -Run 1: - -| Mode | Masks | Old mean ms | New mean ms | Old median ms | New median ms | Mean speedup | Old calls | New calls | -| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | -| blur | 1 | 1.637 | 1.631 | 1.630 | 1.621 | 1.00x | 120 | 120 | -| blur | 4 | 6.701 | 1.687 | 6.697 | 1.688 | 3.97x | 480 | 120 | -| blur | 8 | 13.266 | 1.664 | 13.291 | 1.660 | 7.97x | 960 | 120 | -| blur | 16 | 26.504 | 1.688 | 26.494 | 1.685 | 15.70x | 1920 | 120 | -| pixelate | 1 | 0.805 | 0.812 | 0.804 | 0.804 | 0.99x | 120 | 120 | -| pixelate | 4 | 3.310 | 0.828 | 3.306 | 0.826 | 4.00x | 480 | 120 | -| pixelate | 8 | 6.938 | 0.900 | 6.956 | 0.902 | 7.71x | 960 | 120 | -| pixelate | 16 | 13.723 | 0.911 | 13.605 | 0.878 | 15.07x | 1920 | 120 | - -Run 2: - -| Mode | Masks | Old mean ms | New mean ms | Old median ms | New median ms | Mean speedup | Old calls | New calls | -| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | -| blur | 1 | 1.600 | 1.600 | 1.585 | 1.589 | 1.00x | 120 | 120 | -| blur | 4 | 6.569 | 1.657 | 6.548 | 1.651 | 3.96x | 480 | 120 | -| blur | 8 | 13.384 | 1.713 | 13.293 | 1.672 | 7.82x | 960 | 120 | -| blur | 16 | 26.587 | 1.708 | 26.138 | 1.661 | 15.57x | 1920 | 120 | -| pixelate | 1 | 0.790 | 0.789 | 0.791 | 0.791 | 1.00x | 120 | 120 | -| pixelate | 4 | 3.252 | 0.812 | 3.234 | 0.809 | 4.00x | 480 | 120 | -| pixelate | 8 | 6.749 | 0.862 | 6.664 | 0.857 | 7.83x | 960 | 120 | -| pixelate | 16 | 13.750 | 0.888 | 13.766 | 0.883 | 15.48x | 1920 | 120 | diff --git a/apps/desktop-gpui/kernels/benchmarks/mask-cache/lib.rs b/apps/desktop-gpui/kernels/benchmarks/mask-cache/lib.rs deleted file mode 100644 index 334cafd746a..00000000000 --- a/apps/desktop-gpui/kernels/benchmarks/mask-cache/lib.rs +++ /dev/null @@ -1,4 +0,0 @@ -#[path = "../../src/mask_cache.rs"] -pub mod mask_cache; -#[path = "../../src/screenshot.rs"] -pub mod screenshot;