Repository navigation
fix: let renderers own their canvas so contexts are released - #11
Merged
Merged
Conversation
Model and Carousel rendered a <canvas> in JSX and handed it to a WebGLRenderer built in an effect. Those are two different lifetimes — the renderer lives and dies with the effect, the canvas with the component's render output — and cleanRenderer was left unable to release the WebGL context because of it: forcing the loss on teardown would leave StrictMode's second effect run holding a canvas that can never hand out another context, where three throws on the null context that comes back. So cleanRenderer only called dispose(), which frees three's GPU objects but leaves the context alive until the canvas is collected. Browsers cap live contexts (~16 in Chrome) and evict the *oldest* past that cap, so mounting and unmounting models while scrolling leaked one per unmount until an unrelated, still-visible canvas elsewhere on the page was killed — in the portfolio, the home page hero sphere, which Chrome then painted its dead-canvas glyph over. mountRenderer has the renderer create and own its canvas instead, the way three is meant to be used. Every effect run starts from a fresh element, so cleanRenderer can dispose, force the context loss, and remove the canvas unconditionally, with nothing to detect. Carousel's viewport observer and measurement move onto canvasWrapper, the element that now outlives the renderer. The new tests record which canvases have had their context released, so they can state the invariant directly: everything torn down is released, and nothing still on the page is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
The bug
ModelandCarouseleach render a<canvas>in JSX and hand it to aWebGLRendererbuilt in an effect. Those are two different lifetimes — the renderer lives and dies with the effect, the canvas with the component's render output — and that mismatch is whycleanRenderercould only calldispose().dispose()frees three's own GPU objects but leaves the underlying WebGL context alive until the canvas is garbage collected. Browsers cap live contexts (~16 in Chrome) and evict the oldest once that cap is passed. So mounting and unmounting models while scrolling leaked one context per unmount until an unrelated, still-visible canvas elsewhere on the page was killed.Seen in parammehta/portfolio: scrolling the home page's experience timeline down and back up a few times killed the hero sphere's context — Chrome logs
WARNING: Too many active WebGL contexts. Oldest context will be lost.and paints its dead-canvas placeholder (a small broken-image glyph) over the hero.Why not just add
forceContextLoss()Tried that first; it breaks the page. StrictMode tears an effect down and re-runs it against the same element, and a canvas whose context has been lost can never hand out another one — the second run builds a renderer on a dead canvas and three throws
Cannot read properties of null (reading 'precision').Detecting which case you're in (
domElement.isConnected) works, but it infers undocumented React commit ordering, and if that ever changed the leak would come back silently.The fix
mountRendererhas the renderer create and own its canvas, mounting it into a container — the way three is meant to be used. Every effect run starts from a fresh element, socleanRenderercan dispose, force the context loss, and remove the canvas unconditionally. Nothing to detect.Carousel's viewport observer andgetBoundingClientRect()move ontocanvasWrapper, the element that now outlives the renderer.Verification
The new tests record which canvases have had their context released, so they can state the invariant rather than the mechanism: everything torn down is released, nothing still on the page is. Mutation-tested — reverting
cleanRenderertodispose()only fails 3 tests; makingmountRendererreuse a shared canvas fails 2, including the StrictMode one that catches the crash above.Built this branch against the portfolio and ran the original repro — 6 full scroll cycles (roughly triple what reproduced it), zero context-loss warnings, 3 healthy canvases, device models rendering correctly.
tsc, eslint, stylelint, and 186 tests pass.🤖 Generated with Claude Code