-
-
Notifications
You must be signed in to change notification settings - Fork 36.1k
TSL: ShadowNode: prevent memory leaks. Add webgpu_test_memory #32395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses memory leaks in the WebGPU renderer related to LightNode instances and their associated shadow resources. The changes ensure proper disposal of shadow materials, textures, and render objects when lights are disposed or modified, preventing memory accumulation in long-running applications.
Key Changes:
- Added disposal mechanism for shadow materials through
disposeShadowMaterialfunction - Implemented event-driven cleanup where
AnalyticLightNoderesponds to shadow disposal events - Extended
LightShadowto dispatch disposal events viaEventDispatcher
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/nodes/lighting/ShadowFilterNode.js | Added disposeShadowMaterial function to properly dispose shadow materials and remove them from the material library |
| src/nodes/lighting/ShadowNode.js | Integrated disposeShadowMaterial call in the _reset() method to clean up shadow materials during node disposal |
| src/nodes/lighting/AnalyticLightNode.js | Added event listener for shadow disposal and disposeShadow() method to clean up shadow-related nodes and restore color nodes |
| src/lights/LightShadow.js | Extended EventDispatcher to enable disposal event dispatching, allowing dependent nodes to respond to shadow cleanup |
| examples/webgpu_test_memory.html | Added memory test example demonstrating proper disposal patterns for meshes, lights, and post-processing effects |
| examples/screenshots/webgpu_test_memory.jpg | Screenshot for the new memory test example |
| examples/files.json | Registered the new memory test example in the examples list |
| test/e2e/puppeteer.js | Added the new example to the test exception list for investigation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
@sunag I’ve just updated the PR. The next video was recorded inside the dev branch. As you can see, there’s no way for the GC to collect the unused memory after enabling and disabling shadows, then recreating the shadow light. NOTE: RenderObject are not cleared until I disable PP and shadows Video (dev branch): output2.mp4Here is the behavior in the PR branch: output1.mp4I'll create an issue for this. It would be great to have a way to notify |
|
@cmhhelgeson The PR does not include the engine build. So, we can treat your link as the dev branch, and this link as the PR version (it is a branch with the built engine): |
Summary
Prevent memory leaks caused by
ShadowNodeinstances and their associated resources by:LightsNodedisposes obsoleteShadowNodeinstances when lights change and when the node itself is disposed.RenderObjects and their references to meshes can be garbage collected.The PR partially resolves the issue. At least for now, the GC successfully deallocates meshes when I recreate the light sources.
Background / Problem
A
ShadowNodecan have associated runtime resources (shadow materials, textures, RenderObject's created for different light nodes)If we do not dispose
ShadowNodeinstances:renderer._nodes, those meshes remain indirectly reachable vianodes.nodeBuilderCache.So, as long as a
ShadowNodesticks around, the JS GC cannot reclaim:This makes long-running apps slowly accumulate “dead” meshes, materials, geometries in memory.
There is a bigger problem
WebGPURenderer: Right now it’s too hard to clean memory of meshes, even if the user explicitly disposes geometry, materials and textures and remove references.
The reason is:
scene.overrideMaterial)PassNode/ override material can create one or moreRenderObjects behind the scenes.RenderObjects can only be destroyed when the corresponding material is disposed.You must also dispose all nodes that use
overrideMaterial(includingShadowNodeand PP nodes), orRenderObjects will stay alive and keep meshes strongly referenced.There is still a problem that a user have to recreate PP and light sources to clean references on removed meshes so GC can deallocate removed meshes.
@sunag @Mugen87 Should I create an issue?