fix: prevent crash when GPU is unavailable#772
fix: prevent crash when GPU is unavailable#772Bishtrahulsingh wants to merge 1 commit intoOpenCut-app:mainfrom
Conversation
|
@Bishtrahulsingh is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThese changes add GPU availability checks to the compositor's canvas operations, update canvas rendering methods to handle null values, and enforce memory alignment for the LayerUniformBuffer struct by adding explicit padding fields in both Rust and shader code. A development dependency ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/package.json (1)
6-19:⚠️ Potential issue | 🟡 MinorAdd
postinstallscript to enablepatch-packagefunctionality.
patch-packageonly works when invoked via apostinstallscript. Without it, any patches in apatches/directory will be ignored on install. Since nopatches/directory or patched dependencies are visible in this PR, confirm this dependency is actually needed or remove it.Proposed addition
"scripts": { "dev": "next dev --turbopack", + "postinstall": "patch-package", "build": "next build",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/package.json` around lines 6 - 19, The package.json currently includes patch-package as a dependency but lacks a postinstall script to apply patches; add a "postinstall" entry under "scripts" that runs "patch-package" (or remove the patch-package dependency if no patches exist) so that patches in the patches/ directory are applied after install; look for the "scripts" object near existing entries like "dev" and "build" to insert the "postinstall" script or remove the dependency if it's unused.
🧹 Nitpick comments (2)
rust/crates/compositor/src/compositor.rs (1)
60-66: LGTM — explicit padding + compile-time alignment check is solid.The
_padding: [f32; 2]correctly mirrors the WGSL_padding: vec2fand theconst _: () = assert!(...)guarantees any future field addition that breaks 16-byte alignment will fail at compile time rather than silently producing GPU layout bugs at runtime. Consider applying the sameconst _alignment assert toBlendUniformBufferandMaskUniformBuffer(lines 68–80) for consistency — purely optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/crates/compositor/src/compositor.rs` around lines 60 - 66, Add the same compile-time 16-byte alignment assertion used for LayerUniformBuffer to the other uniform structs so future changes can't break GPU layout: locate the BlendUniformBuffer and MaskUniformBuffer declarations and add a const _: () = assert!(std::mem::size_of::<BlendUniformBuffer>() % 16 == 0, "BlendUniformBuffer must be 16-byte aligned for cross-device wgpu compatibility") and similarly for MaskUniformBuffer to enforce the same alignment check.apps/web/src/services/renderer/canvas-renderer.ts (1)
71-83: Silent no-oprender()in degraded mode — consider signalling state to UI.When GPU is unavailable,
wasmCompositor.ensureInitialized/syncTextures/renderall early-exit, sorender()resolves successfully without producing any frame. The preview canvas will just stay blank with no indication to the user about why. Consider exposing a degraded-mode flag (e.g. viaisGpuAvailable()) the editor can surface as a banner/toast, so users understand why the preview is empty rather than thinking the editor is broken.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/services/renderer/canvas-renderer.ts` around lines 71 - 83, The render method can silently no-op when the GPU is unavailable; detect that by checking the compositor's initialization/availability (use wasmCompositor.ensureInitialized's return or add/use wasmCompositor.isGpuAvailable()), then set or expose a flag on the renderer (e.g. this.degradedMode or add an isGpuAvailable() accessor on the class) and update render to flip that flag (or emit a 'degraded' event) before returning so the UI/editor can show a banner/toast; change references inside render where you call wasmCompositor.ensureInitialized, wasmCompositor.syncTextures, and wasmCompositor.render to early-check availability and mark the state accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/services/renderer/canvas-renderer.ts`:
- Around line 43-49: The export path currently assumes getOutputCanvas() returns
a non-null HTMLCanvasElement; add a null check before calling new
CanvasSource(...) in the export method where CanvasSource is instantiated (the
code that calls getOutputCanvas()). If getOutputCanvas() returns null (GPU
unavailable via wasmCompositor.getCanvas/getOutputCanvas), either return null
with a clear user-facing message like "Export failed: GPU unavailable — export
requires GPU" or throw a descriptive Error immediately to fail fast; update the
code that calls new CanvasSource(...) to handle the chosen behavior.
---
Outside diff comments:
In `@apps/web/package.json`:
- Around line 6-19: The package.json currently includes patch-package as a
dependency but lacks a postinstall script to apply patches; add a "postinstall"
entry under "scripts" that runs "patch-package" (or remove the patch-package
dependency if no patches exist) so that patches in the patches/ directory are
applied after install; look for the "scripts" object near existing entries like
"dev" and "build" to insert the "postinstall" script or remove the dependency if
it's unused.
---
Nitpick comments:
In `@apps/web/src/services/renderer/canvas-renderer.ts`:
- Around line 71-83: The render method can silently no-op when the GPU is
unavailable; detect that by checking the compositor's
initialization/availability (use wasmCompositor.ensureInitialized's return or
add/use wasmCompositor.isGpuAvailable()), then set or expose a flag on the
renderer (e.g. this.degradedMode or add an isGpuAvailable() accessor on the
class) and update render to flip that flag (or emit a 'degraded' event) before
returning so the UI/editor can show a banner/toast; change references inside
render where you call wasmCompositor.ensureInitialized,
wasmCompositor.syncTextures, and wasmCompositor.render to early-check
availability and mark the state accordingly.
In `@rust/crates/compositor/src/compositor.rs`:
- Around line 60-66: Add the same compile-time 16-byte alignment assertion used
for LayerUniformBuffer to the other uniform structs so future changes can't
break GPU layout: locate the BlendUniformBuffer and MaskUniformBuffer
declarations and add a const _: () =
assert!(std::mem::size_of::<BlendUniformBuffer>() % 16 == 0, "BlendUniformBuffer
must be 16-byte aligned for cross-device wgpu compatibility") and similarly for
MaskUniformBuffer to enforce the same alignment check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a3ce8a41-2ee2-43d7-acbf-f2d3b5f3b9d8
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
apps/web/package.jsonapps/web/src/services/renderer/canvas-renderer.tsapps/web/src/services/renderer/compositor/wasm-compositor.tsrust/crates/compositor/src/compositor.rsrust/crates/compositor/src/shaders/layer.wgsl
| getOutputCanvas(): HTMLCanvasElement | null { | ||
| wasmCompositor.ensureInitialized({ | ||
| width: this.width, | ||
| height: this.height, | ||
| }); | ||
| return wasmCompositor.getCanvas(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\bgetOutputCanvas\s*\(' -C3Repository: OpenCut-app/OpenCut
Length of output: 1266
🏁 Script executed:
#!/bin/bash
# Check the full context around scene-exporter.ts line 100
cat -n apps/web/src/services/renderer/scene-exporter.ts | sed -n '85,115p'Repository: OpenCut-app/OpenCut
Length of output: 1386
🏁 Script executed:
#!/bin/bash
# Find CanvasSource type definition and constructor signature
rg -n "class CanvasSource|interface CanvasSource|type CanvasSource" -A5Repository: OpenCut-app/OpenCut
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check the full implementation of getOutputCanvas
cat -n apps/web/src/services/renderer/canvas-renderer.ts | sed -n '38,55p'Repository: OpenCut-app/OpenCut
Length of output: 837
🏁 Script executed:
#!/bin/bash
# Check wasmCompositor.getCanvas() return type
rg -n "getCanvas\s*\(" -B2 -A3 apps/web/src/services/renderer/Repository: OpenCut-app/OpenCut
Length of output: 1680
🏁 Script executed:
#!/bin/bash
# Find CanvasSource definition - could be imported from a library or defined locally
rg "CanvasSource" -B2 -A8 apps/web/src/services/renderer/scene-exporter.ts | head -30Repository: OpenCut-app/OpenCut
Length of output: 663
🏁 Script executed:
#!/bin/bash
# Search for CanvasSource import/definition across the codebase
rg "class CanvasSource|from.*CanvasSource" -B1 -A1Repository: OpenCut-app/OpenCut
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check mediabunny version and any type definitions
cat -n package.json | grep -i mediabunny -B2 -A2Repository: OpenCut-app/OpenCut
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Search for all uses of getOutputCanvas in the codebase
rg "getOutputCanvas" -B3 -A3Repository: OpenCut-app/OpenCut
Length of output: 1220
🏁 Script executed:
#!/bin/bash
# Check if there's a try-catch around the export that might handle this
cat -n apps/web/src/services/renderer/scene-exporter.ts | sed -n '75,120p'Repository: OpenCut-app/OpenCut
Length of output: 1746
Handle null return from getOutputCanvas() in export path.
getOutputCanvas() returns HTMLCanvasElement | null and can return null when GPU is unavailable (see wasm-compositor.ts:74-75). However, scene-exporter.ts:100 passes the result directly to new CanvasSource(...) without a null check. When GPU is unavailable, this will fail during export without a user-friendly error message.
Add a null check in the export method before instantiating CanvasSource, and either:
- Return
nullwith a user-friendly error (export requires GPU), or - Throw a descriptive error immediately for faster failure with clear messaging
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/services/renderer/canvas-renderer.ts` around lines 43 - 49, The
export path currently assumes getOutputCanvas() returns a non-null
HTMLCanvasElement; add a null check before calling new CanvasSource(...) in the
export method where CanvasSource is instantiated (the code that calls
getOutputCanvas()). If getOutputCanvas() returns null (GPU unavailable via
wasmCompositor.getCanvas/getOutputCanvas), either return null with a clear
user-facing message like "Export failed: GPU unavailable — export requires GPU"
or throw a descriptive Error immediately to fail fast; update the code that
calls new CanvasSource(...) to handle the chosen behavior.
1 participant
Fix: prevent "Failed to Load Project" when GPU is unavailable
Fixes #771
The editor fails to load projects across all browsers and platforms, showing the error "Failed to Load Project".
This happens when GPU initialization fails (for example, in environments without WebGPU support). Even in this state, the compositor continues to call WASM functions that depend on a valid GPU runtime, which results in runtime errors and breaks project loading.
Added guards to ensure compositor methods are only executed when GPU is available.
Changes include:
Added GPU availability checks before calling WASM compositor methods
Updated:
ensureInitializedsyncTexturesrendergetCanvasResult
Testing
Tested in environments without WebGPU support:
All scenarios worked without triggering the error.
Summary by CodeRabbit
Bug Fixes
Chores