Repository navigation
perf(recording): speed up screenshot corner rounding and rgb packing - #2808
Conversation
|
@kyzer1023 is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
|
@greptileai review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| assert!( | ||
| std::is_x86_feature_detected!("ssse3"), | ||
| "ssse3 is required to execute the shuffled path" | ||
| ); |
There was a problem hiding this comment.
If an x86_64 machine lacks SSSE3, or a virtual machine hides it, rgb_ssse3_matches_original_including_scalar_tail fails this assertion. Production supports those machines through its scalar fallback, so cargo test fails on an otherwise working configuration. Return early when SSSE3 is unavailable instead of asserting.
| assert!( | |
| std::is_x86_feature_detected!("ssse3"), | |
| "ssse3 is required to execute the shuffled path" | |
| ); | |
| if !std::is_x86_feature_detected!("ssse3") { | |
| return; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/screenshot.rs
Line: 2040-2043
Comment:
**Tests reject supported CPUs**
If an x86_64 machine lacks SSSE3, or a virtual machine hides it, `rgb_ssse3_matches_original_including_scalar_tail` fails this assertion. Production supports those machines through its scalar fallback, so `cargo test` fails on an otherwise working configuration. Return early when SSSE3 is unavailable instead of asserting.
```suggestion
if !std::is_x86_feature_detected!("ssse3") {
return;
}
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
Hey @kyzer1023, thanks for this, the corner-only alpha pass and the SSSE3 packing are nice wins and the scalar-tail coverage is thorough. Greptile's one finding this round ( Once pushed, could you comment |
|
@greptileai review |
Co-authored-by: Kyzer <kyzer1023@users.noreply.github.com>
|
@greptileai review |
|
@richiemcilroy merged latest main into this one. CI is green apart from the known macOS playback flakes (backward_jumps / vfr_holds hw decode) and Engine verify, and Greptile is at 5/5 on the merge commit. Ready for another look whenever you have time, thanks! |
Summary
Screenshot window corners are masked only in the four corner squares, and BGRA/RGBA to RGB packing no longer picks the channel order once per pixel. On x86_64 the packer uses SSSE3
pshufbwhen the CPU supports it, with a scalar tail and a scalar fallback. aarch64 keeps a tight scalar loop. Output matches the previous pixel loops byte for byte.Why
apply_window_rounded_cornerswalked every pixel and ran the rounded-corner alpha test even though only the corner squares of radiusrcan be non-opaque.rgb_from_rgbamatched RGBA vs BGRA inside the per-pixel loop.Both helpers were compiled only for macOS and Windows, so their tests never ran on Linux CI.
What changed
rounded_corner_alpharuns only inside the corner squares. If the radius is large enough that those squares overlap, each pixel is still evaluated once, with the same left-then-right branch order as before. Radius<= 0still uses the existing opaqueConvertBufferconversion.rgb_from_rgbachooses the channel order once per image and, on x86_64, checks SSSE3 once per image. Groups of 4 pixels are shuffled withpshufb. The leftover 0 to 3 pixels, and every pixel when SSSE3 is absent, use the scalar loop. aarch64 compiles only that scalar loop.cfg. Linux non-test builds do not compile the packers, so they are not flagged as dead code.Benchmarks
Apple Silicon / aarch64 was not measured.
Hardware: Intel(R) Xeon(R) Processor, family 6 model 173, 4 cores, 2700 MHz (
/proc/cpuinfo), Linux 6.12.94+ x86_64. One core viataskset -c 1. Warmup 2, then the median of 9 runs. Corner radius 16 px. Every case checked byte-identical output before timing, and all of them matched. The timed call allocates the output buffer, same as the production functions.The harness is one Rust file that contains the previous per-pixel loops and the new loops, so both sides share the compiler flags. It is not linked into
cap-recording(that crate needs FFmpeg). The corner "before" loop is the full-frame alpha walk over raw RGB bytes.rustc 1.88.0 (6b00bc388 2025-06-23). Flags match the workspace release profile:
opt-level = "s", thin LTO,codegen-units = 1,debuginfo = line-tables-only,panic = unwind. Times are milliseconds.stable rustc 1.99.1 (58cf8f9c3 2026-10-13).
opt-level = 3, thin LTO,codegen-units = 1,panic = unwind.Tests
FFmpeg 7 came from the repo native-deps bundle (
target/native-deps,libavcodec.so.61), not Ubuntu libav.test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 832 filtered out; finished in 0.65sThe tests compare against the original per-pixel implementations. They cover odd widths, radius 0, radius larger than half the shorter side, tiny images, non-finite radii, BGRA and RGBA, the SSSE3 path including widths that leave a scalar tail, and the scalar fallback called directly.
Finished dev profile [unoptimized + debuginfo] target(s)with exit code 0.Finished with exit code 0.
Merged main
Merged
mainat2012f25cinto the branch at4e24e082with no conflicts. Nothing onmaintouchedscreenshot.rsbetween the old base and that commit. After the merge,cargo fmt --all -- --checkandcargo clippy -p cap-recording --no-deps --lib --tests -- -D warningsare clean. The same 7 screenshot pixel tests pass, with 837 filtered out because main added tests.The same bench was rerun on a different VM (Xeon family 6 model 207, 2400 MHz). Every case was still byte-identical. With rustc 1.88.0 at the release profile, corners got 3.94x / 3.84x / 2.65x faster, BGRA to RGB 2.31x / 2.30x / 1.54x, and RGBA to RGB 2.02x / 2.32x / 1.50x at 1080p / 4K / 5K. The tables above are from the original 2700 MHz run.
The PR appears safe to merge; no actionable issues remain.Summary
This PR speeds up screenshot corner rounding and RGB packing.
No new actionable issues found. This review checked the code without running tests.
Reviews (3) · Last reviewed commit: "Merge main" · Reviewed by Greptile