Skip to content

perf(editor): pipeline Windows/Linux preview readback - #3585

Open
AatmanAJ wants to merge 2 commits into
CapSoftware:mainfrom
AatmanAJ:perf/editor-preview-pipeline
Open

AatmanAJ wants to merge 2 commits into
CapSoftware:mainfrom
AatmanAJ:perf/editor-preview-pipeline

Conversation

@AatmanAJ

@AatmanAJ AatmanAJ commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Pipeline realtime editor preview readback (RGBA and BGRA) so frame N−1 is read back while frame N renders; seeks and paused redraws still use immediate readback.
  • On capable Windows/Linux adapters, preview frames are read back as tight BGRA on the GPU instead of padded RGBA plus CPU channel swap.
  • GPUI preview uses StreamingImage on Windows/Linux by default to reuse one texture per stream and skip per-frame atlas uploads (disable with CAP_GPUI_STREAMING_IMAGE=0).

Test plan

  • Open a studio recording on Windows or Linux (hardware GPU): playback preview stays smooth and edits while playing remain responsive.
  • Scrub while paused: preview shows the exact seeked frame.
  • Software adapter / WARP path: preview still works via RGBA fallback (no BGRA readback).
  • CAP_GPUI_STREAMING_IMAGE=0: preview falls back to RenderImage conversion path.

RetriggerView in GreptileConfidence Score: 2/5

Do not merge until consecutive BGRA frames use separate buffers, seeks return their requested frame, and readbacks wait for late callbacks.

Findings

  1. P1 Playback reuses a mapped buffer ▶
  2. P1 Seeking shows an older frame ▶
  3. P2 Pixels and layout disagree ▶
  4. P2 Playback misses its final frame ▶
Fix with agent prompt
### Issue 1
crates/rendering/src/frame_pipeline.rs:2700-2708
`RgbaToBgraReadback` keeps one buffer at a fixed size. The next frame submits a copy and calls `map_async` before the previous frame is read and unmapped. The second realtime frame therefore reuses a buffer whose map is still active, violating wgpu’s buffer-use rules and breaking the default hardware preview path. Alternate between at least two readback buffers.

### Issue 2
crates/editor/src/editor.rs:485-494
A seek after playback can return the previous playback frame instead of the requested frame. Realtime rendering now leaves a readback pending, but the immediate helpers return that older frame when they receive `Some(frame)`. A lone seek has `drained_count == 0`, so nothing clears it first. The paused preview accepts the reply and stops retrying, leaving the requested frame pending. Drain the old readback before an immediate request, then return the newly requested frame.

### Issue 3
crates/editor/src/editor.rs:476-483
Realtime rendering now returns the previous frame, but `frame_layout` still comes from the current request. During a live crop, size, or camera-layout edit, the callback pairs old pixels with the new layout. GPUI uses that layout for preview size and edit positions, so they can briefly disagree with the image. Keep each pending frame’s layout with its readback and deliver them together.

### Issue 4
crates/editor/src/editor.rs:466-475
Each realtime call now returns the previous frame and leaves the current one pending. When playback ends, the stop path neither reads that last frame nor requests a final redraw. The preview can therefore stop one submitted frame behind. Add a final drain that delivers the pending frame and its layout when playback stops.

---

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

Summary

Adds pipelined preview readback and GPU conversion to tightly packed BGRA.

  • Editor playback renders a new frame while the previous frame is read back.
  • GPUI previews stream frames instead of rebuilding an image for each frame.

Reviews (1) · Last reviewed commit: "perf(editor): pipeline Windows/Linux pre..." · Reviewed by Greptile

Overlap GPU BGRA readback with rendering during realtime playback, use GPU BGRA on capable adapters, and stream preview frames into a reused texture on Windows/Linux to avoid per-frame CPU swizzle and atlas uploads.
@vercel

vercel Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Someone 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 +2700 to +2708
let previous = readback.take_pending();
let pending = readback.submit(
device,
queue,
encoder,
session.current_texture(),
uniforms.frame_number,
uniforms.frame_rate,
)?;

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 Playback reuses a mapped buffer

RgbaToBgraReadback keeps one buffer at a fixed size. The next frame submits a copy and calls map_async before the previous frame is read and unmapped. The second realtime frame therefore reuses a buffer whose map is still active, violating wgpu’s buffer-use rules and breaking the default hardware preview path. Alternate between at least two readback buffers.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/rendering/src/frame_pipeline.rs
Line: 2700-2708

Comment:
**Playback reuses a mapped buffer**

`RgbaToBgraReadback` keeps one buffer at a fixed size. The next frame submits a copy and calls `map_async` before the previous frame is read and unmapped. The second realtime frame therefore reuses a buffer whose map is still active, violating wgpu’s buffer-use rules and breaking the default hardware preview path. Alternate between at least two readback buffers.

---

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

Comment on lines +485 to +494
} else {
frame_renderer
.render_immediate_with_timings(
input.segment_frames,
input.uniforms,
&input.cursor,
true,
&mut layers,
)
.await

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 Seeking shows an older frame

A seek after playback can return the previous playback frame instead of the requested frame. Realtime rendering now leaves a readback pending, but the immediate helpers return that older frame when they receive Some(frame). A lone seek has drained_count == 0, so nothing clears it first. The paused preview accepts the reply and stops retrying, leaving the requested frame pending. Drain the old readback before an immediate request, then return the newly requested frame.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/editor/src/editor.rs
Line: 485-494

Comment:
**Seeking shows an older frame**

A seek after playback can return the previous playback frame instead of the requested frame. Realtime rendering now leaves a readback pending, but the immediate helpers return that older frame when they receive `Some(frame)`. A lone seek has `drained_count == 0`, so nothing clears it first. The paused preview accepts the reply and stops retrying, leaving the requested frame pending. Drain the old readback before an immediate request, then return the newly requested frame.

---

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

Comment on lines +476 to +483
.map(|(frame, timings)| {
frame.map(|frame| {
(
EditorFrameOutput::Rgba(frame),
PlaybackRenderOutputFormat::Rgba,
timings,
)
})

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 Pixels and layout disagree

Realtime rendering now returns the previous frame, but frame_layout still comes from the current request. During a live crop, size, or camera-layout edit, the callback pairs old pixels with the new layout. GPUI uses that layout for preview size and edit positions, so they can briefly disagree with the image. Keep each pending frame’s layout with its readback and deliver them together.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/editor/src/editor.rs
Line: 476-483

Comment:
**Pixels and layout disagree**

Realtime rendering now returns the previous frame, but `frame_layout` still comes from the current request. During a live crop, size, or camera-layout edit, the callback pairs old pixels with the new layout. GPUI uses that layout for preview size and edit positions, so they can briefly disagree with the image. Keep each pending frame’s layout with its readback and deliver them together.

---

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

Comment on lines +466 to +475
if realtime {
frame_renderer
.render_with_timings(
input.segment_frames,
input.uniforms,
&input.cursor,
true,
&mut layers,
)
.await

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 Playback misses its final frame

Each realtime call now returns the previous frame and leaves the current one pending. When playback ends, the stop path neither reads that last frame nor requests a final redraw. The preview can therefore stop one submitted frame behind. Add a final drain that delivers the pending frame and its layout when playback stops.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/editor/src/editor.rs
Line: 466-475

Comment:
**Playback misses its final frame**

Each realtime call now returns the previous frame and leaves the current one pending. When playback ends, the stop path neither reads that last frame nor requests a final redraw. The preview can therefore stop one submitted frame behind. Add a final drain that delivers the pending frame and its layout when playback stops.

---

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

@greptile-apps

greptile-apps Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff

These findings could not be posted inline.

  • P1 Successful readbacks can fail crates/rendering/src/frame_pipeline.rs:1756 ▶

    Replacing mapped_result with try_recv brings back the shared-device callback race. Another thread can collect the map callback while this thread’s submission wait finishes, but send its result slightly later. The new code treats that temporarily empty receiver as BufferMapWaitingFailed, causing needless render retries or failed readbacks. Keep the bounded receiver wait for both RGBA and NV12.

Double-buffer BGRA GPU readbacks, wait for map callbacks reliably, and flush pipelined frames before seek/immediate renders.
@AatmanAJ
AatmanAJ force-pushed the perf/editor-preview-pipeline branch from 34abbf8 to a11789f Compare October 30, 2026 00:26

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