Skip to content

perf(desktop-gpui): optimize 32-bit row conversions, NV12 planes, and screenshot blitting - #2668

Open
nikolas-sapa wants to merge 2 commits into
CapSoftware:mainfrom
nikolas-sapa:perf/gpui-pixel-kernels
Open

nikolas-sapa wants to merge 2 commits into
CapSoftware:mainfrom
nikolas-sapa:perf/gpui-pixel-kernels

Conversation

@nikolas-sapa

@nikolas-sapa nikolas-sapa commented Oct 17, 2026 •

Copy link
Copy Markdown

Description

Optimizes pixel kernel operations used in GPUI desktop app rendering, recording frame conversions, thumbnail downsampling, and screenshot blitting.

Changes

  1. convert_32bit_rows (RGBA, BGRA, ARGB, ABGR):
    • RGBA: Copies contiguous row slices directly instead of per-pixel 4-byte chunk pushes.
    • BGRA: Direct row-by-row channel swap allowing auto-vectorization across cache-friendly rows.
    • ARGB / ABGR: Unrolled 16-byte lane swaps reordering 4 pixels at a time, followed by 4-byte remainder processing.
  2. convert_nv12_planes:
    • Precomputes Y scale and subtraction factors once per frame based on Nv12Range.
    • Shares chroma calculations across adjacent pixel pairs ($C_b$ and $C_r$ terms computed once per 2 luma samples).
  3. downsample_nearest:
    • Precomputes source X-coordinate offsets into a stack buffer instead of computing (x * width / target_width) * 4 for every pixel on every row.
  4. blit_over_offset (screenshot blitting):
    • Short-circuits fully transparent overlay pixels (alpha == 0) without calling blend_pixel_over.
    • Fast-paths fully opaque overlay pixels (alpha == 255) using direct slice copies without float conversions and roundtrip clamping.
    • Preserves bit-exact IEEE-754 precision matching baseline output bit-for-bit.
  5. Benchmarks:
    • Adds frame_convert benchmark suite to lock in and track speedups.

Benchmarks

Ran on Apple Silicon M-series (macOS):

convert_32bit_rows

Workload Baseline Candidate Speedup
1080p BGRA 1311.1 µs 292.6 µs 4.48x
1080p RGBA 831.8 µs 219.0 µs 3.80x
1080p ARGB 1298.7 µs 242.2 µs 5.36x
1080p ABGR 1332.6 µs 231.0 µs 5.77x
1080p BGRA (padded) 1323.8 µs 234.5 µs 5.64x

downsample_nearest

Workload Baseline Candidate Speedup
1080p -> 720p 510.0 µs 412.7 µs 1.24x
1080p -> 360p 136.4 µs 107.6 µs 1.27x
4K -> 1080p 1177.0 µs 1077.5 µs 1.09x

convert_nv12_planes

Workload Baseline Candidate Speedup
720p NV12 2455.0 µs 2152.7 µs 1.14x
1080p NV12 5522.2 µs 4800.1 µs 1.15x
4K NV12 22231.5 µs 19521.7 µs 1.14x

screenshot_blit

Workload Baseline Candidate Speedup
Opaque 1080p 8.3 ms 0.7 ms ~11.8x
Opaque 4K 28.1 ms 3.5 ms ~8.0x
Edge alpha 4K 28.5 ms 11.2 ms ~2.5x
Sparse alpha 4K 35.6 ms 11.1 ms ~3.2x
Transparent 4K 15.3 ms 5.5 ms ~2.8x
Out-of-bounds clipped 4.5 ms 83 ns >50,000x

Verification

  • Ran cargo test -p cap-gpui-kernels: 30 passed, 0 failed.
  • Ran cargo bench --bench frame_convert: all assertions verify expected == actual.
  • Ran cargo bench --bench screenshot_blit: all assertions verify before == after bit-for-bit.

RetriggerView in GreptileConfidence Score: 4/5

The PR appears safe to merge with non-blocking fixes for NV12 edge cases and test coverage.

Findings

  1. P2 Odd widths now panic ▶
  2. P2 Green bytes change ▶
  3. P2 Mixed row paths lack tests ▶
Fix with agent prompt
### Issue 1
apps/desktop-gpui/kernels/src/frame.rs:354-356
`convert_nv12_planes` now panics for positive odd widths instead of returning `None`. With `width = 3`, `height = 1`, and sufficient plane data, the pair loop leaves `x = 2`. The remainder reads `uv_row[3]`, but that slice has length 3-even when the original UV plane has padding. Keep the previous rejection of incomplete chroma pairs and add an odd-width test.

### Issue 2
apps/desktop-gpui/kernels/src/frame.rs:335-340
Combining `g_chroma` changes the green byte for some valid NV12 pixels. In video range, `(Y, Cb, Cr) = (36, 80, 58)` previously produces green 99. The new expression produces approximately 98.999992 and truncates to 98. The benchmark's repeating inputs miss this case. Precompute the two chroma products separately, preserve the original subtraction order, and add this comparison case.

### Issue 3
apps/desktop-gpui/kernels/benches/frame_convert.rs:60-65
The new `convert_32bit_rows` benchmark only uses widths divisible by four. Existing tests use two pixels, so neither checks an `Argb` or `Abgr` row that runs both the new 16-byte loop and its remainder. Add padded, multi-row cases with widths 5, 6, and 7 against the baseline. Otherwise, errors where those paths meet can pass every current comparison.

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!

---

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

Summary

Optimizes row copies, channel swaps, NV12 conversion, downsampling, and screenshot blending. Adds a reference-based frame benchmark.

  • Frame pixels convert and shrink with less repeated work.
  • Screenshot blits skip blending for transparent and opaque pixels.

Reviews (1) · Last reviewed commit: "perf(desktop-gpui): optimize 32-bit row ..." · Reviewed by Greptile

… screenshot blitting

- Vectorize convert_32bit_rows across RGBA, BGRA, ARGB, and ABGR
- Precompute scales and share UV chroma math across pixel pairs in convert_nv12_planes
- Precalculate X coordinate offsets in downsample_nearest
- Short-circuit alpha == 0 and fast-path alpha == 255 in screenshot blit_over_offset
- Add frame_convert benchmark suite to lock in speedups

Benchmarks:
- 1080p BGRA conversion: 1311 µs -> 293 µs (4.48x speedup)
- 1080p RGBA conversion: 832 µs -> 219 µs (3.80x speedup)
- 1080p ARGB conversion: 1299 µs -> 242 µs (5.36x speedup)
- 1080p ABGR conversion: 1333 µs -> 231 µs (5.77x speedup)
- 1080p BGRA padded: 1324 µs -> 235 µs (5.64x speedup)
- 1080p -> 720p downsample: 510 µs -> 413 µs (1.24x speedup)
- NV12 plane conversions: ~1.15x speedup
- Screenshot blitting: up to 11x faster on opaque overlays, 2.5x-3.2x on edge/sparse alpha
@vercel

vercel Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

@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.

Comment on lines +354 to +356
if x < width {
let cb = uv_row[x];
let cr = uv_row[x + 1];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Odd widths now panic

convert_nv12_planes now panics for positive odd widths instead of returning None. With width = 3, height = 1, and sufficient plane data, the pair loop leaves x = 2. The remainder reads uv_row[3], but that slice has length 3—even when the original UV plane has padding. Keep the previous rejection of incomplete chroma pairs and add an odd-width test.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/kernels/src/frame.rs
Line: 354-356

Comment:
**Odd widths now panic**

`convert_nv12_planes` now panics for positive odd widths instead of returning `None`. With `width = 3`, `height = 1`, and sufficient plane data, the pair loop leaves `x = 2`. The remainder reads `uv_row[3]`, but that slice has length 3—even when the original UV plane has padding. Keep the previous rejection of incomplete chroma pairs and add an odd-width test.

---

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

Comment thread apps/desktop-gpui/kernels/src/frame.rs Outdated
Comment on lines +335 to +340
let g_chroma = -0.391762_f32 * cb_f - 0.812968_f32 * cr_f;
let b_chroma = 2.017232_f32 * cb_f;

let y1 = ((f32::from(y_row[x]) - y_sub).max(0.0)) * scale;
rgba_row[dst_idx] = clamp_channel(y1 + r_chroma);
rgba_row[dst_idx + 1] = clamp_channel(y1 + g_chroma);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Green bytes change

Combining g_chroma changes the green byte for some valid NV12 pixels. In video range, (Y, Cb, Cr) = (36, 80, 58) previously produces green 99. The new expression produces approximately 98.999992 and truncates to 98. The benchmark's repeating inputs miss this case. Precompute the two chroma products separately, preserve the original subtraction order, and add this comparison case.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/kernels/src/frame.rs
Line: 335-340

Comment:
**Green bytes change**

Combining `g_chroma` changes the green byte for some valid NV12 pixels. In video range, `(Y, Cb, Cr) = (36, 80, 58)` previously produces green 99. The new expression produces approximately 98.999992 and truncates to 98. The benchmark's repeating inputs miss this case. Precompute the two chroma products separately, preserve the original subtraction order, and add this comparison case.

---

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

Comment on lines +60 to +65
for (name, width, height, stride, order) in [
("1080p-bgra", 1920, 1080, 1920 * 4, ChannelOrder::Bgra),
("1080p-rgba", 1920, 1080, 1920 * 4, ChannelOrder::Rgba),
("1080p-argb", 1920, 1080, 1920 * 4, ChannelOrder::Argb),
("1080p-abgr", 1920, 1080, 1920 * 4, ChannelOrder::Abgr),
("1080p-bgra-padded", 1920, 1080, 7936, ChannelOrder::Bgra),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Mixed row paths lack tests

The new convert_32bit_rows benchmark only uses widths divisible by four. Existing tests use two pixels, so neither checks an Argb or Abgr row that runs both the new 16-byte loop and its remainder. Add padded, multi-row cases with widths 5, 6, and 7 against the baseline. Otherwise, errors where those paths meet can pass every current comparison.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/kernels/benches/frame_convert.rs
Line: 60-65

Comment:
**Mixed row paths lack tests**

The new `convert_32bit_rows` benchmark only uses widths divisible by four. Existing tests use two pixels, so neither checks an `Argb` or `Abgr` row that runs both the new 16-byte loop and its remainder. Add padded, multi-row cases with widths 5, 6, and 7 against the baseline. Otherwise, errors where those paths meet can pass every current comparison.

---

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!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant