Repository navigation
perf(gpui): reduce screenshot editor startup time - #2703
dovidmoishe wants to merge 3 commits into
Conversation
|
@dovidmoishe is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
| let image = match cap_recording::screenshot::capture_screenshot(target.clone()).await { | ||
| Ok(image) | ||
| if image.width() <= crate::screenshot_editor::MAX_DIMENSION | ||
| && image.height() <= crate::screenshot_editor::MAX_DIMENSION => |
There was a problem hiding this comment.
Large screenshots are not saved
capture_and_write now rejects images over 16,384 pixels in either direction before saving the PNG, even for save-only and clipboard-only captures. If a window spans enough high-resolution displays to exceed that size, the X11 capture path uses its full physical bounds and reaches this check. Previously the PNG was saved even if the editor could not open it; now the user gets neither a file nor a clipboard image.
Keep the size limit at the editor boundary so larger screenshots can still be saved and copied.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/screenshot.rs
Line: 204-207
Comment:
**Large screenshots are not saved**
`capture_and_write` now rejects images over 16,384 pixels in either direction before saving the PNG, even for save-only and clipboard-only captures. If a window spans enough high-resolution displays to exceed that size, the X11 capture path uses its full physical bounds and reaches this check. Previously the PNG was saved even if the editor could not open it; now the user gets neither a file nor a clipboard image.
Keep the size limit at the editor boundary so larger screenshots can still be saved and copied.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
Thanks for this! The default-path changes look like real wins (no PNG re-decode, GPU device created during capture, raw first frame), and the PNG is fully written before the editor or clipboard sees it. Before we can merge, a few changes:
Once those are in and Greptile is still at 5/5 we'll run the full CI and merge. |
|
Thanks @richiemcilroy! I'll address these now. For benchmarking, I'm currently on Windows without access to a Mac, so I can't reliably verify the macOS numbers or frame transition, but I'll try to figure out a way. |
|
@greptileai please review the latest changes in commit 25380fa. |
|
@richiemcilroy The requested changes and macOS CI results are now included. Could you retrigger Greptile for 25380fa? The Retrigger link returns 404, and tagging @greptileai hasn’t started a review. |
Fresh screenshots previously saved a PNG and decoded it again when opening the editor.
This PR reuses captured pixels, prepares the shared GPU device during capture, and shows an initial raw preview while GPU rendering initializes. The PNG write completes before the editor or clipboard receives the result.
The raw preview applies to fresh captures with width divisible by four and height divisible by two. Other dimensions wait for the GPU frame.
Changes following review
Renderer layers now belong to the editor’s render task. The existing shared GPU-device preparation remains.
Default-path macOS measurements
GPU p90 increased from 310.6 to 396.7 ms, so these results support earlier visual feedback rather than a substantial improvement in GPU completion.
Baseline: upstream
346561b837aaa2cd27eea01b90b858d6596848b5.Revised: the four production Rust files in
25380faacmatch the benchmarked source snapshot.Conditions:
Timing starts at
take_screenshotentry and ends at the first image paint plus the next GPUI frame callback. The first GPU-rendered image callback is measured separately. These are draw-callback measurements, not physical display scanout or consumer-Mac cold-start results.Completed benchmark run
These numbers replace the earlier 909 → 483 ms result, which required the removed experiments. Benchmark instrumentation and workflows remain on a separate fork branch.
Preview transition check
A separate untimed recording captured 31 preview frames and 12 post-GPU frames. Inspection of the sampled frames showed no obvious blank image or color flash.
The largest actual-frame gap was 83.3 ms, so the recording job’s strict 50 ms coverage check remains inconclusive. This cannot rule out shorter unsampled events or establish physical-Mac behavior. Cap’s capture, rendering, and normal-close checks passed within that run.
Recording run and original frame artifacts
Recording adds load; its timings are not benchmark results. Physical-Mac confirmation is still requested. Native Linux testing remains outstanding.
Validation
cargo check: passed.Fresh Greptile review and full PR CI are still required for this updated commit.


Confidence Score: 5/5
The PR appears safe to merge; the previous large-screenshot saving issue is fixed, and no new blocking issues were found.Summary
Speeds up screenshot editor startup by passing captured pixels directly to the editor.
No new actionable issues were found. Checks were based on source review; native UI checks and tests were not run.
Reviews (2) · Last reviewed commit: "fix(gpui): preserve oversized screenshot..." · Reviewed by Greptile