Conversation
…g minimize
Export hang: ModernVideoExporter.export() awaits collectRuntimeDiagnostics()
as its very first step, before any progress is reported. That function calls
window.electronAPI.getExportHardwareInfo(), whose main-process handler awaits
app.getGPUInfo("complete") with no timeout. That Electron API can hang
indefinitely (never resolve or reject) when the GPU process is crashed,
disabled, or blocklisted, which silently stalls the entire export pipeline at
the "preparing" phase forever with no error ever surfaced. Race it against a
3s timeout in the main process, and add a defense-in-depth timeout around both
diagnostics IPC calls on the renderer side.
Black preview after minimizing: the editor's live preview renders through a
PixiJS WebGL/WebGPU canvas (VideoPlayback.tsx). Windows can reclaim GPU
resources from a window minimized/occluded for a long time, silently killing
the WebGL context with no recovery — nothing listened for
`webglcontextlost`/`contextrestored` or `visibilitychange` anywhere in the
app, so the canvas stayed permanently black until a full app relaunch. Add a
`webglcontextlost` listener that forces a full teardown/recreate of the Pixi
renderer (via a `rendererGeneration` counter), plus a `visibilitychange`
safety net that does the same after the window was hidden for over a minute,
to also cover the WebGPU backend (which doesn't fire that event) and stalled
hidden-<video> decoder sessions.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughChangesThe PR adds Pixi renderer recovery for WebGL context loss and extended document hiding. It also bounds Electron export diagnostic requests with timeouts so stalled GPU or application information calls do not block export. Runtime resilience
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Document
participant VideoPlayback
participant PixiRenderer
Document->>VideoPlayback: emit webglcontextlost or resume after extended hiding
VideoPlayback->>PixiRenderer: destroy existing renderer
VideoPlayback->>PixiRenderer: initialize renderer for new generation
Merge Risk: ⚪ Minimal · up to No actionable regression remains; the bounded diagnostics and renderer recovery changes are ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Summary
ModernVideoExporter.export()callscollectRuntimeDiagnostics()as its very first step, before any progress is ever reported. That awaitswindow.electronAPI.getExportHardwareInfo(), whose main-process handler awaitsapp.getGPUInfo("complete")with no timeout — a documented Electron footgun that can hang indefinitely (never resolve or reject) when the GPU process is crashed, disabled, or blocklisted. With no timeout anywhere in that chain, the whole export pipeline silently stalls at "preparing" forever, with no error ever surfaced to the user. Fixed by racingapp.getGPUInfo("complete")against a 3s timeout inelectron/ipc/export/native-video.ts, plus a defense-in-depth timeout around both diagnostics IPC calls on the renderer side inmodernVideoExporter.ts.VideoPlayback.tsx). Windows can reclaim GPU resources from a window that's been minimized/occluded for a long time, silently killing the WebGL context — nothing in the app listened forwebglcontextlost/contextrestoredorvisibilitychange, so the canvas stayed permanently black until the user fully quit and relaunched the app. Added awebglcontextlostlistener that forces a full teardown/recreate of the Pixi renderer (via arendererGenerationcounter, reusing the existing solid cleanup logic), plus avisibilitychangesafety net that does the same after the window was hidden for over a minute, to also cover the WebGPU backend (which doesn't fire that event) and a stalled hidden<video>decoder session.Test plan
npx tsc --noEmitnpx vitest run(1182 passing, 1 skipped, unrelated to this change)Summary by CodeRabbit