fix(馃寪): don't lose the canvas WebGL context on layout-effect cleanup - #4041
Closed
wcandillon wants to merge 3 commits into
Closed
wcandillon wants to merge 3 commits into
wcandillon wants to merge 3 commits into
Conversation
WebGLRenderer.dispose() runs from the cleanup of a layout effect, and effect cleanup does not imply the <canvas> host node is gone: React re-runs layout effects on a preserved element under StrictMode's DEV double-invoke and on Activity/offscreen reveal. WEBGL_lose_context.loseContext() is permanent for that element, so the next renderer built on it got a non-zero handle back from GetWebGLContext (the guard never fired) but could not build a GrDirectContext on the dead context, and threw. CanvasKit.deleteContext() is kept: it is what unregisters the context from the Emscripten GL registry and drops the reference to the canvas element, so the leak fixed in #3924 stays fixed. Fixes #3976
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
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.
Description
Fixes #3976. Builds on #4002 by @giaBaoJS, whose analysis and StrictMode test this PR carries forward (the original commit is kept as-is on this branch).
On web, a
<Canvas>throws whenever React re-runs its layout effects on the same DOM node, becauseWebGLRenderer.dispose()callsloseContext()on the<canvas>element's WebGL context. Effect cleanup does not imply the host node is gone: StrictMode's DEV double-invoke and an Activity/offscreen reveal (how React Navigation / Expo Router keep inactive routes mounted) both re-run the layout effect against a preserved element. The re-created renderer then gets a non-zero handle fromGetWebGLContexton the dead canvas andMakeWebGLContextfaults inside wasm.Change
#4002 removed the
loseContext()call. This PR goes one step further and makes the lifetimes explicit: the renderer belongs to the layout effect, the WebGL context belongs to the<canvas>element, andWebGLRendererreconciles the two.WeakMap<HTMLCanvasElement, CanvasWebGL>records the element's context, itsWEBGL_lose_contextextension (captured while the context is healthy, sincegetExtension()returns null on a lost one), whether the loss has been announced, and the currently attached renderer.webglcontextrestored, then requests a redraw.webglcontextlostispreventDefault()ed by element-lifetime listeners so the browser is allowed to restore an evicted context even while no renderer is attached (hidden Activity).dispose()queues a microtask that callsloseContext()only if no new renderer took the element and it has left the document. React runs the cleanup before removing the node, so this is the earliest point at which the two cases can be told apart. This matters because browsers cap live contexts per page (16 in Chrome) and evict the oldest one.onResize()sizes it again.draw,onResize, andmakeImageSnapshotno-op on a lost context instead of touching freed GL objects.MakeWebGLContextfailure always unregisters the handle (try/finally), so a throw no longer leaves CanvasKit's registry retaining the canvas ([Web] Canvas unmount doesn't release WebGL context (retains whole DOM subtree); SkiaPictureView also leaks a new context on every relayout聽#3924).<canvas>gets akeyper renderer kind. A canvas element is bound to one context kind for life, so switching between the static (2D) and live (WebGL) renderer needs a fresh element.Tests
packages/skia/src/views/__tests__/SkiaPictureView.web.spec.tsxgrows from 4 to 10 tests. The CanvasKit mock models a per-element context that can be lost and restored, andGetWebGLContextreturning a non-zero handle on a lost context (the reason the existing guard never fired). New cases:馃 Generated with Claude Code
https://claude.ai/code/session_015BrCh6yHU4HvK1QSHoVWtk