search

pmndrs / react-postprocessing

📬 postprocessing for react-three-fiber
https://docs.pmnd.rs/react-postprocessing
MIT License
1.1k stars 106 forks source link

Memory leak when switching cameras #270

Open Pinpickle opened 6 months ago

Pinpickle commented 6 months ago

When switching cameras, a small amount of memory is leaked. This is enough to get Safari to crash on an iPhone 13 Pro after enough switches (somewhere between 500 and 1,000) on a simple scene.

Recording from an iPhone

It crashes about halfway through, auto-reloads, then crashes again and refuses to reload. This is due to running out of memory.

Warning: the video and reproduction strobe a bit as it changes the camera 100 times per second to speed up the crash.

https://github.com/pmndrs/react-postprocessing/assets/3238878/23da9698-08ee-4e4a-ac62-0c9493009686

Reproduction

Here's the reproduction: https://stackblitz.com/edit/vitejs-vite-khvsd4?file=src%2FApp.tsx

To run it on your phone, you'll need to pull the stackblitz down and run it locally (npm install then npm run dev), then open it on your phone via the local network.

Misc comments

With a more complex app, I am able to crash the app in ~10 camera switches with an empty EffectComposer. I can't work out exactly what is causing the increase in memory consumption in the more complex case.

I think this is due to the fact that for every camera/scene change, the useMemo call in EffectComposer creates a new instance without disposing the old one, thus keeping all of the textures for the passes around.

https://github.com/pmndrs/react-postprocessing/blob/29fda1b9d5f3ecfb5a5aa8b376569cc96aea2588/src/EffectComposer.tsx#L105-L115

Pinpickle commented 6 months ago

Submitted without finishing, oops. I'll edit with the rest :)

Done!

davcri commented 6 months ago

Hi @Pinpickle the issue is happening to me as well.

I made a PR in which I tried to use refs and recreate/dispose objects whenever the effect triggered, however it doesn't seem to solve the problem.

I also noticed that many effects use <primitive dispose={null} /> which overrides the dispose() method, preventing it from being called.

However this is probably a common desired behaviour? Probably not, as Cody commented on Discord with:

Right, forgot that's an issue in R3F currently. Which might explain bugs in other places where we call effect.dispose() in useEffect.