Skip to content

perf(gpui): reduce screenshot editor startup time - #2703

Open
dovidmoishe wants to merge 3 commits into
CapSoftware:mainfrom
dovidmoishe:perf/screenshot-editor-startup
Open

dovidmoishe wants to merge 3 commits into
CapSoftware:mainfrom
dovidmoishe:perf/screenshot-editor-startup

Conversation

@dovidmoishe

@dovidmoishe dovidmoishe commented Oct 18, 2026 •

Copy link
Copy Markdown

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

  • Removed both opt-in experiments: hidden-window preparation and the persistent screenshot renderer-layer cache.
  • Retain the extra RGBA copy only when opening the editor.
  • Read the post-screenshot setting once at capture start and carry it through completion.
  • Preserve saving and copying oversized screenshots; enforce the dimension limit at the editor boundary.
  • Remove the PowerShell benchmark that measured only window opening.
  • Drop rendered results for older edits so they cannot replace the current preview while a newer render is pending.

Renderer layers now belong to the editor’s render task. The existing shared GPU-device preparation remains.

Default-path macOS measurements

  • First image median: 287.4 → 141.1 ms — 50.9% lower
  • First GPU frame median: 287.5 → 281.6 ms — essentially unchanged

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 25380faac match the benchmarked source snapshot.

Conditions:

  • macOS 15.7.9 GitHub Actions runner, Apple Paravirtual GPU.
  • 1024×768 display, scale 1.
  • Release builds using Rust 1.95.0.
  • Two excluded warmups and 12 measured captures per version, alternating versions.
  • Fresh Cap process/profile per capture; warmed filesystem and driver caches.
  • No measured samples removed.

Timing starts at take_screenshot entry 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

  • Rust formatting: passed.
  • Windows cargo check: passed.
  • Screenshot-related Rust tests: 65 passed, 0 failed, two existing benchmark tests ignored.
  • Production source hashes verified against the macOS benchmark snapshot.

Fresh Greptile review and full PR CI are still required for this updated commit.

RetriggerView in GreptileConfidence 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.

  • Fresh screenshots reach the editor without another PNG decode.
  • Screenshot rendering can reuse GPU layers warmed ahead of time.
  • The editor window can be prepared while a screenshot is being captured.

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

@vercel

vercel Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

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

Comment thread apps/desktop-gpui/src/screenshot.rs Outdated
Comment on lines +204 to +207
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 =>

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.

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

@dovidmoishe

Copy link
Copy Markdown
Author

@greptileai

@richiemcilroy

Copy link
Copy Markdown
Member

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:

  1. Remove the two opt-in experiments (CAP_GPUI_SCREENSHOT_PREPARE_WINDOW and CAP_GPUI_SCREENSHOT_WARM_LAYERS: the prepared hidden window/bundle, and the process-lifetime WARM_SCREENSHOT_LAYERS that keeps a wgpu device and renderer layers alive and rebuilds them on every editor close). They are off by default, but the prepared bundle has no PNG while the capture runs (a library refresh could list it), and the warm cache is a permanent leak. Keep the default-path wins.
  2. Only keep the RGBA copy in write_png when the editor will actually open (it is about 59 MB at 5K even for copy/save-only), and read GeneralSettings once per screenshot instead of three to five synchronous loads on the main thread.
  3. Please post default-path before/after numbers on macOS, measured to the first rendered frame rather than window-open, and confirm the raw preview does not flash against the first GPU frame. The 909 → 483 ms headline needs both experiments enabled, and scripts/benchmark-screenshot-startup.ps1 pins them to 0 and times only the window-open log, so it cannot reproduce it. Either change the script to measure the first rendered frame or drop it.
  4. Mention in the description that frames rendered for an older edit are now dropped (screenshot_editor.rs around the render-result handling); it reads like a fix, but it is a behaviour change.

Once those are in and Greptile is still at 5/5 we'll run the full CI and merge.

@dovidmoishe

dovidmoishe commented Oct 19, 2026 •

Copy link
Copy Markdown
Author

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.

@dovidmoishe

dovidmoishe commented Oct 20, 2026 •

Copy link
Copy Markdown
Author

@greptileai please review the latest changes in commit 25380fa.

@dovidmoishe

Copy link
Copy Markdown
Author

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

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.

2 participants