Repository navigation
perf(gpui): optimize screenshot alpha checks, flattening, and premultiplication - #2673
nikolas-sapa wants to merge 2 commits into
Conversation
|
@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. |
| mixed.extend_from_slice(&[r, g, b, a]); | ||
| } | ||
|
|
||
| // 1. is_opaque on opaque buffer |
There was a problem hiding this comment.
The numbered benchmark headings only repeat the calls below them. The repository guide explicitly bans comments that narrate what the code does. Remove these headings throughout the file to satisfy that requirement 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/benches/screenshot_alpha.rs
Line: 83
Comment:
**Comments repeat the code**
The numbered benchmark headings only repeat the calls below them. The repository guide explicitly bans comments that narrate what the code does. Remove these headings throughout the file to satisfy that requirement 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!
| let a = if i % 100 == 0 { (i % 256) as u8 } else { 255 }; | ||
| mixed.extend_from_slice(&[r, g, b, a]); |
There was a problem hiding this comment.
Late transparency goes unmeasured
The mixed buffer always starts with alpha zero, so the new is_opaque checks stop at the first block. The benchmarks miss the costly case where transparency near the end makes a conversion scan almost the whole buffer before running its original loop.
Add a mostly opaque workload with a translucent final pixel for flatten_onto_white, premultiply, and demultiply. Also time demultiply on translucent input; its only current workload returns early. This would show the added scan cost rather than only the best-case fallback.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/kernels/benches/screenshot_alpha.rs
Line: 79-80
Comment:
**Late transparency goes unmeasured**
The `mixed` buffer always starts with alpha zero, so the new `is_opaque` checks stop at the first block. The benchmarks miss the costly case where transparency near the end makes a conversion scan almost the whole buffer before running its original loop.
Add a mostly opaque workload with a translucent final pixel for `flatten_onto_white`, `premultiply`, and `demultiply`. Also time `demultiply` on translucent input; its only current workload returns early. This would show the added scan cost rather than only the best-case fallback.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
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 |
Summary
is_opaque: Checks 16 pixels per loop iteration by accumulating 4xu128blocks with bitwise AND before applying the alpha mask. Reduces branch evaluations and loop overhead on large screenshot buffers.has_translucent_pixel: Replaces the slow scalarskip(3).step_by(4)byte iterator with!is_opaque(rgba), accelerating the transparency check used on every screenshot export.flatten_onto_white: Checksif is_opaque(rgba)and returnsrgba.to_vec()directly, avoiding millions of multiplications and divisions on opaque screenshots (the vast majority of screenshots).premultiply: Checksif is_opaque(rgba)and returns early without touching buffers.demultiply: Checksif is_opaque(rgba)and returns early without touching buffers.benches/screenshot_alpha.rs.Benchmark Results (Apple Silicon M-series)
has_translucent_pixelhas_translucent_pixelhas_translucent_pixelflatten_onto_white(solid)flatten_onto_white(solid)flatten_onto_white(solid)premultiply(solid)premultiply(solid)premultiply(solid)demultiply(solid)demultiply(solid)demultiply(solid)Test Plan
cargo test --manifest-path apps/desktop-gpui/kernels/Cargo.tomlcargo bench --manifest-path apps/desktop-gpui/kernels/Cargo.toml --bench screenshot_alphaThe pixel changes appear safe, but the explicit comments policy must be satisfied before merging.
Findings
Fix with agent prompt
Summary
Speeds up screenshot alpha checks with 64-byte blocks and skips conversion work for fully opaque images.
Reviews (1) · Last reviewed commit: "perf(gpui): optimize screenshot alpha ch..." · Reviewed by Greptile