perf(gpui): SIMD vectorize PNG adaptive filters and eliminate buffer copies

[nikolas-sapa]

Summary

• Vectorizes all 4 PNG adaptive filter encoders (Up, Sub, Avg, Paeth) in apps/desktop-gpui/kernels/src/png.rs using ARM NEON on aarch64 with unrolled 64-byte block processing.
• Zero-copy Up filter: Filter 2 (the initial filter evaluated by adaptive_filter) filters directly into the output row buffer while accumulating the absolute sum in vector registers, eliminating the intermediate candidate buffer write and subsequent memcpy.
• Vectorized Average filter (vhaddq_u8): evaluates floor halving addition and subtraction in vector instructions with zero scalar division or bitwise overhead.
• Vectorized Paeth filter: evaluates parallel 8-lane 16-bit absolute distance comparisons and bitselect (vbslq_u16), eliminating all per-pixel scalar branches.
• Fast SIMD sum_abs: replaces the double-nested .chunks(32).map().sum() iterator loop with NEON vector absolute and horizontal pairwise addition (vabsq_s8 + vaddlvq_u8).
• Unit tests: adds neon_filters_match_scalar_reference verifying all 4 filters and their cost calculations match the scalar reference byte-for-byte across RGB and RGBA formats and varied row lengths.
• Benchmark harness: adds default synthetic screen workloads (1080p, Retina, 4K) to benches/png_encode.rs when executed without CLI file arguments.

Benchmark Results (Apple Silicon M-series)

Workload	Pixels	Baseline (image crate)	Optimized (cap-gpui-kernels)	Speedup	Correctness
1080p	2,073,600	76.1 ms	19.4 ms	~3.9x	Bit-exact
Retina	5,939,136	197.8 ms	49.8 ms	~4.0x	Bit-exact
4K	8,294,400	285.4 ms	66.6 ms	~4.3x	Bit-exact

Test Plan

• Run cargo test --manifest-path apps/desktop-gpui/kernels/Cargo.toml
• Run cargo bench --manifest-path apps/desktop-gpui/kernels/Cargo.toml --bench png_encode
• Run cargo clippy --manifest-path apps/desktop-gpui/kernels/Cargo.toml

Confidence Score: 5/5

The PR appears safe to merge, with one non-blocking cleanup for an unused test helper.

Findings

1.  Non-ARM tests gain a warning ▶

Fix with agent prompt

### Issue 1
apps/desktop-gpui/kernels/src/png.rs:709
`scalar_filter_cost` is compiled for tests on every platform, but its only callers are inside the `aarch64`-only test. Tests on x86_64 therefore gain an unused-function warning. Add `#[cfg(target_arch = "aarch64")]` to the helper so it follows the test's platform limit.

```suggestion
    #[cfg(target_arch = "aarch64")]
    fn scalar_filter_cost(buf: &[u8]) -> u64 {
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR adds ARM NEON PNG filters and default synthetic benchmark images.

• PNG rows are filtered and scored with NEON on aarch64.
• The PNG benchmark runs screen-sized samples without image files.

_{Reviews (2) · Last reviewed commit: "fix(gpui): address Greptile findings on ..." · Reviewed by Greptile}

[vercel]

@nikolas-sapa is attempting to deploy a commit to the Cap Software Inc Team on Vercel.

A member of the Team first needs to authorize it.

[greptile-apps]

Loops violate required Rust style

The new loops index slices with a range variable. The repository instructions require slice iteration or .iter().enumerate() instead. Use zipped iterators for these equal-length slices. The same pattern appears at lines 353–356 and 835–837, and elsewhere in the added filters. This repository requirement must be satisfied before merging.

Context Used: AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/desktop-gpui/kernels/src/png.rs
Line: 282-285

Comment:
**Loops violate required Rust style**

The new loops index slices with a range variable. The repository instructions require slice iteration or `.iter().enumerate()` instead. Use zipped iterators for these equal-length slices. The same pattern appears at lines 353–356 and 835–837, and elsewhere in the added filters. This repository requirement must be satisfied before merging.

**Context Used:** AGENTS.md ([source](https://github.com/capsoftware/cap/blob/main/AGENTS.md))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Image batches retain every buffer

The CLI branch now decodes every image into targets before benchmarking the first one. Previously, it decoded and released one image at a time. Large screenshot batches now need memory for all decoded images at once, which can cause swapping, distort timings, or stop the benchmark with an out-of-memory failure. Keep CLI targets lazy and decode each image just before benchmarking it.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/desktop-gpui/kernels/benches/png_encode.rs
Line: 93

Comment:
**Image batches retain every buffer**

The CLI branch now decodes every image into `targets` before benchmarking the first one. Previously, it decoded and released one image at a time. Large screenshot batches now need memory for all decoded images at once, which can cause swapping, distort timings, or stop the benchmark with an out-of-memory failure. Keep CLI targets lazy and decode each image just before benchmarking it.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Cost tests share their implementation

The expected cost uses sum_abs, which this PR also replaces with NEON on aarch64. Both sides therefore use the same vector absolute-value and sum operations, so a shared cost bug could pass every assertion. Compute the expected cost with a scalar iterator in this test, including explicit 0x80 values, to check the new cost code independently.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/desktop-gpui/kernels/src/png.rs
Line: 842

Comment:
**Cost tests share their implementation**

The expected cost uses `sum_abs`, which this PR also replaces with NEON on `aarch64`. Both sides therefore use the same vector absolute-value and sum operations, so a shared cost bug could pass every assertion. Compute the expected cost with a scalar iterator in this test, including explicit `0x80` values, to check the new cost code independently.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[richiemcilroy]

Thanks @nikolas-sapa! Greptile is at 4/5 here. We only merge once Greptile reaches 5/5, so please address its findings in the summary above (or reply to them if you think one is wrong), then comment @greptileai review to re-run it. Once it is 5/5 we will run the full verification suite on it.

[nikolas-sapa]

Addressed all three Greptile findings in 2a38c5b39.

1. benches/png_encode.rs — CLI targets were all decoded up front. Valid. cli_paths.into_iter().map(...).collect() materialised every decoded image before the first encode. Moved decoding into the benchmark loop (BenchmarkTarget::from_path, run_target), so one image is resident at a time and released before the next.

Measured peak RSS over eight 1200x900 targets:

	peak RSS
before	59,572,224 B (56.8 MiB)
after	35,717,120 B (34.1 MiB)

~23.9 MB saved, which is the ~25.9 MB of decoded buffers that were being held simultaneously. Wall time was unchanged (1.30 s -> 1.22 s), so this is a memory fix, not a timing one.

2. src/png.rs — range-indexed loops. Valid. Converted the loops this PR adds in apply_up_neon, apply_sub_neon, apply_avg_neon and apply_paeth_neon to zipped slice iterators. Two notes:

• needless_range_loop does not actually fire on these loops locally, because each body also accumulates into sum. This is a documented-style fix, not a lint failure. apps/desktop-gpui is a standalone workspace with no [workspace.lints], and needless_range_loop = "deny" lives in the root workspace's lint set, which CI's cargo clippy --workspace --exclude cap-desktop never reaches for this crate. Worth knowing if you want the rule actually enforced here.
• The for i in 0..len { out[i] = current[i].wrapping_sub(previous[i]); } loops in the scalar apply_filter fallback are pre-existing upstream code, not part of this PR, so I left them alone.

3. src/png.rs — cost test shared its implementation with the code under test. Valid, and the most important of the three. sum_abs gained a NEON path in this PR built from the same vabsq_s8 + vaddlvq_u8 pair the filters use, so using it as the expected value means both sides can be wrong together. Added scalar_filter_cost, an i16-widened scalar fold, and used it for all four filter assertions. Also seeded 0x80 bytes into prev/curr so the saturating-abs boundary is actually reached.

I checked this test is not vacuous: mutating the NEON abs to the saturating vqabsq_s8 makes it fail (1402 vs 1404). Honest caveat — I could not construct a mutation that the old sum_abs-based expectation would have silently passed; under my mutation it failed too, for a different reason (the filters and sum_abs use different block boundaries, so the tails disagree). So what I have verified is that the new expectation is independent and non-vacuous, not that I reproduced the exact blind spot.

Verification: cargo test -p cap-gpui-kernels -> 31 passed, 0 failed, 1 ignored. cargo clippy -p cap-gpui-kernels --all-targets -> no errors, zero needless_range_loop.

[nikolas-sapa]

@greptileai review

[nikolas-sapa]

@richiemcilroy addressed — details in my summary above. Quick version: rebased onto current main (the branch was well behind and would not have merged), and @greptileai review re-triggered.

One thing worth knowing across these PRs: Greptile's findings on #2668, #2671, #2672 and #2673 were already fixed in commits pushed shortly after it reviewed — nobody had posted @greptileai review, so the comments were stale rather than open. #2674 was the only one with genuinely unaddressed findings, and those are now fixed.

cargo test -p cap-gpui-kernels passes on all five rebased branches (0 failed).

[greptile-apps]

Non-ARM tests gain a warning

scalar_filter_cost is compiled for tests on every platform, but its only callers are inside the aarch64-only test. Tests on x86_64 therefore gain an unused-function warning. Add #[cfg(target_arch = "aarch64")] to the helper so it follows the test's platform limit.

Suggested change

	fn scalar_filter_cost(buf: &[u8]) -> u64 {
	#[cfg(target_arch = "aarch64")]
	fn scalar_filter_cost(buf: &[u8]) -> u64 {

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/desktop-gpui/kernels/src/png.rs
Line: 709

Comment:
**Non-ARM tests gain a warning**

`scalar_filter_cost` is compiled for tests on every platform, but its only callers are inside the `aarch64`-only test. Tests on x86_64 therefore gain an unused-function warning. Add `#[cfg(target_arch = "aarch64")]` to the helper so it follows the test's platform limit.

```suggestion
    #[cfg(target_arch = "aarch64")]
    fn scalar_filter_cost(buf: &[u8]) -> u64 {
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[richiemcilroy]

Thanks! I checked this for output equivalence and it matches main: the vabsq_s8 0x80 wrap, the per-vector sum range, the floor average and Paeth tie rules all agree with the scalar adaptive_filter, and the tail handling is right. Before we merge:

1. Add #[cfg(target_arch = "aarch64")] to scalar_filter_cost (otherwise x86/Windows test builds warn about an unused function, the Greptile note that's still open).
2. Re-benchmark against main's own crate::png::write, not the image crate's encoder. Deflate dominates the encode time, so please post the end-to-end gain on main's path.
3. Add debug_assert!s that previous, current and out have equal lengths, since the unsafe NEON paths rely on it.
   Then we'll run the full CI on it.