Properly dispose renderer to stop the animation loop #5112
Merged
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:
![Capture d’écran du 2022-09-10 17-42-52](https://user-images.githubusercontent.com/112249/189492806-df4bac5f-b958-4e53-b931-001bf9d5ed0a.png)
In aframe 1.3.0 and master (three r141), when you are removing the a-scene from the DOM, it disposes the WebXRManager/WebVRManager (
renderer.xr
) ondetachedCallback
but not the renderer itself (WebGLRenderer), so it doesn't stop properly the animation loop and it continues executing some tick methods.You can test with
document.body.removeChild(AFRAME.scenes[0])
Before the PR:
After the PR:
![Capture d’écran du 2022-09-10 18-33-41](https://user-images.githubusercontent.com/112249/189492878-20ee9cbf-3914-4f66-b5dc-8dd4855e3e33.png)
This is known issue since a long time, see #3146 (comment) and https://stackoverflow.com/questions/54930875/aframejs-how-to-completely-destroy-a-scene and I also documented it in https://aframe.wiki/en/memory
Changes proposed:
renderer.dispose()
that executerenderer.xr.dispose()
andanimation.stop()
and also dispose other things, see https://github.com/supermedium/three.js/blob/super-r141/src/renderers/WebGLRenderer.js#L592-L621Properly disposing the renderer in
detachedCallback
seems right because inattachedCallback
that is callingthis.setupRenderer()
we're creating a new WebGLRenderer always.But note that detaching or reattaching the scene is not a supported use case, it gives currently an error before and after this PR anyway on the
/examples/boilerplate/3d-model/
example where I tested that and I'm no gonna investigate that.