mkkellogg / GaussianSplats3D

Three.js-based implementation of 3D Gaussian splatting
MIT License
1.53k stars 198 forks source link

dispose failed due to Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node. #329

Closed v2img closed 2 months ago

v2img commented 2 months ago

A little context: my application renders 3d splat model inside a div in a component. When i navigate away from the div and unmounts the component, i do a viewer.dispose but it always fails due to that error.

My canvas is not the child of the document.

Also, this stemmed from another issue. The event listeners WASD, was not cleaned up properly and after i load the gaussian 3D model from the .ply file using a normal Viewer (not drop in), my WASD are disabled through my application permenantly until a refresh of the tab.

This is my code to initialise the viewer: viewer = new GaussianSplats3D.Viewer({ cameraUp: [0, 1, 0], initialCameraPosition: [0, 0, 7], initialCameraLookAt: [0, 0, 0], rootElement: containerRef.current, sharedMemoryForWorkers: false, dynamicScene: true, });

this is how i dispose it: viewer .dispose() .then(() => { console.log('viewer disposed'); }) .catch((err: any) => { console.log("Didn't successfully dispose viewer", err); });

v2img commented 2 months ago

So my question is, i wonder why is the event handlers not cleaned up properly, and how do i properly dispose it?

Im wondering when your dispose function promises catches an error, does the keyboard listener still get removed?

v2img commented 2 months ago

After investigation, it seems like it is orbital controls fault that the event listeners are not being cleaned up. Any suggestions on how to clean it up?

v2img commented 2 months ago

Ok i found the fix. in your viewer's dispose, add in this.orthographicControls.dispose(); this.perspectiveControls.dispose();

mkkellogg commented 2 months ago

Ah, good catch! I'll add this fix to the next release.

mkkellogg commented 2 months ago

I pushed an update to the memory-optimizations branch, could you see if that fixes it for you?

v2img commented 2 months ago

ok! I dont see any other push, other than the ReadMe change, can i know which commit is it?

mkkellogg commented 2 months ago

So apparently I lied earlier 😄 The change should be in there now.

v2img commented 2 months ago

HAHA xD Yes it works! Thanks :D Looking forward to next release

mkkellogg commented 2 months ago

Great! The next release should hopefully go out in the next couple of days.

mkkellogg commented 2 months ago

These changes are all now in the latest release, v0.4.5

v2img commented 2 months ago

Thanks for your amazing talent and contribution ser