feat(capture): resize + redraw-settle before screenshot#46
Merged
Conversation
Restore the resize-before-screenshot mechanism that was lost during screenshot subsystem rewrites. Default renderer path (viewport + scrollback) now does fit + conn.resize + event-driven redraw settle (500ms timeout fallback) before capturing, fixing layout corruption in heavy TUI apps like claude. Co-Authored-By: Claude <noreply@anthropic.com>
Four-task TDD plan: extract waitForRedrawSettle + captureWithResizeSettle into a pure dependency-injected screenshotSync.ts module (testable without React/xterm/fake-timers), wire it into Terminal.tsx captureToPng, and add a release-test scenario for the layout-after-resize regression. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
The default-options test overrode sleep to bump a waited counter but left now() reading a frozen closure var, freezing the timeout check at 0 and looping forever. Override now() to return waited so the clock advances. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
🔒 门禁检查结果
Rust 测试覆盖率
前端测试覆盖率
✅ 所有检查通过，可以合入
|
Release test scenario 14 revealed two flaws in the original global-quiet settle (≤500ms): (1) it waited for global output quiet, so continuous output (spinner/streaming) always hit the 500ms cap, and steady-state screenshots paid the full 500ms for output that never arrives; (2) the "always resize triggers a redraw" rationale was wrong — TIOCSWINSZ only sends SIGWINCH when the size actually changes. Revised settle: 50ms no-output probe (skip the wait in steady state) + 30ms quiet early-exit (short reflows don't pay the full cap) + 120ms hard cap (continuous output bounded). Max added latency ≤120ms, which also removes the dependency on the never-merged 10s daemon timeout (fb1894f). Also corrects the spec's two factual errors (SIGWINCH-on-change-only; fb1894f never merged to main). Co-Authored-By: Claude <noreply@anthropic.com>
Single-task TDD plan: add PROBE_MS=50, lower DEFAULT_RESIZE_TIMEOUT_MS 500→120, add a no-output probe branch to waitForRedrawSettle. Updates the settle tests to the new bounded timing (probe / early-exit / continuous). captureWithResizeSettle and Terminal.tsx unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
The settle waited for global output quiet (≤500ms), so continuous TUI output (spinner/streaming) always hit the 500ms cap and steady-state screenshots paid the full 500ms for output that never arrives. Both coupled with the daemon's 2s renderer-screenshot timeout. Revised: 50ms no-output probe (steady state → capture immediately), 30ms quiet early-exit (short reflows don't pay the full cap), 120ms hard cap (continuous output bounded). Max added latency ≤120ms, which also removes the dependency on the never-merged 10s daemon timeout (fb1894f). Co-Authored-By: Claude <noreply@anthropic.com>
Release test scenario 14 (screenshot after window resize) hung at the daemon's 2s timeout. Root cause: captureToPng's awaitFrame used pure 2×requestAnimationFrame, which never fires when the app/tab is not frontmost (rAF is throttled) — e.g. after a window resize shifts focus to the terminal running cli-box. awaitFrame then never resolved, captureToPng stalled, and the daemon timed out. Confirmed via: --with-frame (ScreenCaptureKit, bypasses the renderer) succeeded while the default path hung; and activating the CLI Box app (frontmost → rAF resumes) immediately un-stuck the default screenshot. Fix: race 2×rAF against a 50ms setTimeout fallback so awaitFrame always resolves. Pre-PR code used direct canvas.toDataURL (no rAF wait) so it did not hang; this restores that robustness while keeping the frame commit for the normal frontmost case. After the fix, scenario 14's after-resize screenshot returns in 0.12s (was 2.05s timeout) with a correctly-laid-out, non-garbled claude TUI at the new window size. Co-Authored-By: Claude <noreply@anthropic.com>
Spec/impl/report consistency: the bounded-settle revision's Revision Notes now also cover the third scenario-14 finding — awaitFrame's pure 2×requestAnimationFrame hanging when the app isn't frontmost (rAF throttled after a window resize), stalling captureToPng until the daemon's 2s timeout. The fix (2×rAF raced against a setTimeout(50ms) fallback) is documented in the mechanism and risks sections, alongside the --with-frame/activate evidence that confirmed the root cause. Co-Authored-By: Claude <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Heavy TUI apps (claude) screenshot with corrupted layout — text wrapping misaligned, TUI elements misplaced, or stale half-redrawn frames. The "resize before screenshot" mechanism that previously fixed this was lost during the screenshot-subsystem rewrites (2026-06-05 frame, 2026-06-06 size-fix, 2026-06-17 start-shell-screenshot): the default renderer path's viewport capture reads the xterm canvas directly with zero resize, so any PTY/xterm size drift or mid-redraw state is frozen into the screenshot. The scrollback path only did a local
fit()with no PTY resize, so the TUI never got a SIGWINCH to reflow.Design spec:
docs/superpowers/specs/2026-06-22-screenshot-pre-resize-design.md. Implementation plan:docs/superpowers/plans/2026-06-22-screenshot-pre-resize.md.Solution
Restore resize-before-screenshot, event-driven so it's fast in the common case (redraw-settles in tens of ms) with a hard timeout fallback.
electron-app/src/renderer/screenshotSync.ts(precedent:terminalBuffer.ts, similarly extracted fromTerminal.tsxfor testability). Dependency-injected, so fully unit-testable without React/xterm/vitest-fake-timers:captureWithResizeSettle—fit → conn.resize (PTY SIGWINCH) → waitForRedrawSettle → 2×rAF → capture. Guaranteesresize()runs strictly before any canvas/buffer read. Covers both viewport (live canvas, buffer fallback) and scrollback paths.waitForRedrawSettle— blocks until the TUI's post-SIGWINCH redraw output goes quiet (DEFAULT_QUIESCENCE_MS = 30), capped atDEFAULT_RESIZE_TIMEOUT_MS = 500(silent / spinner TUIs). Redraw completion is approximated by watching PTY output settle — a reliable proxy since a redrawn TUI emits output.Terminal.tsxcaptureToPngdelegates to it;onOutputnow timestampslastOutputAtRefto feed the settle clock. Early-mount fallback (no fit/conn yet) preserves prior behavior."Always resize" (not "only if size changed") is intentional: same-size SIGWINCH forces a full-screen TUI redraw and doubles as a canvas refresh, which also fixes stale-frame captures.
Key commits:
a7defd0(waitForRedrawSettle),cd13e73(captureWithResizeSettle),97b7461(test-hang fix),c764329(Terminal wiring),cf9782c(release scenario).Test Plan
screenshotSync.test.ts— 8 new tests (settle / timeout / stale-output + order-guard / viewport / scrollback / null-canvas-fallback / default-options). The order-guard test is the regression guard: it provesresize()precedes the canvas read — exactly the invariant that was lost.pnpm typecheck— pass (inline deps object structurally satisfiesCaptureDeps).pnpm vitest run— 58/58, no regressions to existingcaptureToPty/connectPty/terminalBuffertests.cargo test288/288,cargo clippy -D warnings,cargo fmt --check— clean (no Rust touched).tests/release_test.md; run aftersh release.sh(CLAUDE.md 6.2/6.3). Requires GUI window-resize + visual layout verification.Note:
e2e-compound-start-screenshotfails locally on a stale Jun-19dist/electron/.../CLI Box.appbuild stub (ENOENT at app launch, never reaches screenshot logic). Environmental — resolved bysh release.sh; not a regression from this renderer-only diff.🤖 Generated with Claude Code