BabylonJS / Babylon.js

Babylon.js is a powerful, beautiful, simple, and open game and rendering engine packed into a friendly JavaScript framework.
http://www.babylonjs.com
Apache License 2.0
23.09k stars 3.41k forks source link

Fix WebGLRenderbuffer memory leak from MSAA RenderTargetTextures #15184

Closed rapid-images-tore-levenstam closed 3 months ago

rapid-images-tore-levenstam commented 3 months ago

Forum thread with more info: https://forum.babylonjs.com/t/babylon-leaks-msaa-renderbuffers-on-resize-dispose-with-webgl/51228

The short version is that renderbuffers created by MSAA RenderTargetTextures were never deleted, causing a (sometimes big) memory leak. In our project it could be several gigabytes of leaked memory in a few seconds. The proposed solution here is that thinEngine should use the webGLHardwareTexture's function release to let it delete its resources, instead of directly deleting (only) the WebGLTexture from the context directly.

bjsplat commented 3 months ago

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). To prevent this PR from going to the changelog marked it with the "skip changelog" label.

rapid-images-tore-levenstam commented 3 months ago

I want to note here that this fix seemed to work in our project which runs an earlier version of Babylon (6.31.0). I'm not sure if this change has other implications, but I couldn't find any direct issues from my limited testing.

bjsplat commented 3 months ago

Snapshot stored with reference name: refs/pull/15184/merge

Test environment: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15184/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15184/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/15184/merge https://sandbox.babylonjs.com/?snapshot=refs/pull/15184/merge https://gui.babylonjs.com/?snapshot=refs/pull/15184/merge https://nme.babylonjs.com/?snapshot=refs/pull/15184/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/15184/merge#BCU1XR#0

bjsplat commented 3 months ago

Visualization tests for WebGPU (Experimental)

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15184/merge/testResults/webgpuplaywright/index.html

bjsplat commented 3 months ago

WebGL2 visualization test reporter:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15184/merge/testResults/webgl2playwright/index.html

bjsplat commented 3 months ago

WebGL2 visualization test reporter:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15184/merge/testResults/webgl2playwright/index.html

bjsplat commented 3 months ago

Visualization tests for WebGPU (Experimental)

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15184/merge/testResults/webgpuplaywright/index.html

bjsplat commented 3 months ago

Visualization tests for WebGL 1 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15184/merge/testResults/webgl1/index.html

If tests were successful afterwards, this report might not be available anymore.

rapid-images-tore-levenstam commented 3 months ago

Not sure why the formatting fails, or the WebGL1 visualization tests fail. Any ideas?

RaananW commented 3 months ago

Not sure why the formatting fails, or the WebGL1 visualization tests fail. Any ideas?

Formatting fails because something is off :-)

Run npm run format:fix, it will fix everything for you. Probably some whitespace issue.

About WebGL1 - I am running the tests again just to be sure it is related to this PR.

bjsplat commented 3 months ago

Visualization tests for WebGL 1 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15184/merge/testResults/webgl1/index.html

If tests were successful afterwards, this report might not be available anymore.

rapid-images-tore-levenstam commented 3 months ago

Not sure why the formatting fails, or the WebGL1 visualization tests fail. Any ideas?

Formatting fails because something is off :-)

Run npm run format:fix, it will fix everything for you. Probably some whitespace issue.

About WebGL1 - I am running the tests again just to be sure it is related to this PR.

Alright, thanks for the tip, trying it :)

bjsplat commented 3 months ago

Visualization tests for WebGL 1 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15184/merge/testResults/webgl1/index.html

If tests were successful afterwards, this report might not be available anymore.

RaananW commented 3 months ago

You can ignore the webgl1 error for now, it is related to some other PR (and webgl1 tests are optional)