alelievr / HDRP-UI-Camera-Stacking

Optimized implementation of camera stacking for UI only in HDRP.
MIT License
173 stars 22 forks source link

Out of memory when toggling cameras on and off repeatedly #24

Closed FlaxenFlash closed 9 months ago

FlaxenFlash commented 10 months ago

We were getting out of memory issues on xbox when objects with HDCameraUI on them were toggled on and off frequently (on opening and closing our pause menu for example). This doesn't happen in editor seemingly, but maybe I just had more memory available.

I presume the issue is that internalRenderTexture gets newed OnEnable every time, so the old texture is orphaned and wasn't getting cleaned up automatically (at least on xbox). Moving as much of the OnEnable functionality as possible into Start seems to have fixed the issue, however it also doesn't seem ideal that once you turn a HDCameraUI component on then it holds on to a screen sized buffer for the entirety of its lifetime (and then possibly leaks it once it's destroyed if that is what was happening before), so I also made changes to release the RenderTexture on Disable/Destroy.

void Start()
{
    data = GetComponent<HDAdditionalCameraData>();
    attachedCamera = GetComponent<Camera>();

    if (data == null)
        return;

    data.customRender -= StoreHDCamera;
    data.customRender += StoreHDCamera;

    // TODO: Add VR support
    internalRenderTexture = new RenderTexture(1, 1, 0, graphicsFormat, 1);
    internalRenderTexture.dimension = TextureDimension.Tex2DArray;
    internalRenderTexture.volumeDepth = 1;
    internalRenderTexture.depth = 24;
    internalRenderTexture.name = "HDCameraUI Output Target";

    cullingSampler = new ProfilingSampler("UI Culling");
    renderingSampler = new ProfilingSampler("UI Rendering");
    uiCameraStackingSampler = new ProfilingSampler("Render UI Camera Stacking");
    copyToCameraTargetSampler = new ProfilingSampler("Copy To Camera Target");
    initTransparentUIBackgroundSampler = new ProfilingSampler("Init Transparent UI Background");

    if (blitWithBlending == null)
        blitWithBlending = Shader.Find("Hidden/HDRP/UI_Compositing");
    if (blitInitBackground == null)
        blitInitBackground = Shader.Find("Hidden/HDRP/InitTransparentUIBackground");

    CameraStackingCompositing.uiList.Add(this);
}

void OnEnable()
{
    if (data == null)
        return;

    data.customRender -= StoreHDCamera;
    data.customRender += StoreHDCamera;

    CameraStackingCompositing.uiList.Add(this);
}

void OnDisable()
{
    if (data == null)
        return;

    data.customRender -= StoreHDCamera;
    CameraStackingCompositing.uiList.Remove(this);
    if(internalRenderTexture.IsCreated()) internalRenderTexture.Release();
    internalRenderTexture.width = 1;
    internalRenderTexture.height = 1;
}

private void OnDestroy()
{
    OnDisable();
}
alelievr commented 9 months ago

Hello,

That's a very good catch! Indeed adding a Release() of the RenderTexture in the OnDisable should be enough to fix the issue.

I pushed the chages in this commit: https://github.com/alelievr/HDRP-UI-Camera-Stacking/commit/72de05246e5a1b3824e5dab694b28531adf86c1e let me know if that fixes the issue. I've seen a pretty weird behavior where after a Release(), the C# RenderTexture handle is still treated as "alive" but it shouldn't take too much memory since it was release on C++ side which contains the actual data of the RenderTexture. Calling Resources.UnloadUnusedAssets seems to clear these handles.