RenderTexture#destroy destroys the depth texture as well.

[ShukantPal]

#6556

[ivanpopelyshev]

Maybe it would be better to move it to dispose(). Technically its the same thing for Framebuffer, because its never recovering.. hm...

[ivanpopelyshev]

OK, good enough for a fix. we can refactor it later if someone gets idea how

[ShukantPal]

@ivanpopelyshev Dispose just removes the framebuffer from the GPU. Destroy will destroy the data itself.

We shouldn't move this to dispose. Framebuffer#dispose disposes the depth texture already by disposing the framebuffer-object, which holds the depth texture on the GPU.

[ivanpopelyshev]

We shouldn't move this to dispose. Framebuffer#dispose disposes the depth texture already by disposing the framebuffer-object, which holds the depth texture on the GPU.

Nope, that's the point - depthTexture is not disposed on the GPU automatically. Framebuffer disposes itself and renderbuffer if mounted, but not depthTexture.

            gl.deleteFramebuffer(fbo.framebuffer);
            if (fbo.stencil)
            {
                gl.deleteRenderbuffer(fbo.stencil);
            }

Oh, I feel that we are missing MSAA case here: fboFramebuffer should be also disposed automatically :(

[ShukantPal]

@ivanpopelyshev Correct, because the depth texture was not added internally by the framebuffer system. This is way you cannot run gl.deleteTexture on the depth-texture - this is equivalent of destroying (not disposing) the depth texture: https://github.com/pixijs/pixi.js/blob/9cdb526a9bfbc4643d36a106be3f2b16981e516d/packages/core/src/textures/TextureSystem.ts#L353

[ShukantPal]

The user might not want to destroy the depth texture and may reuse it after disposing the framebuffer. That is not the case for the stencil renderbuffer, because it is never manually added to the framebuffer.

[ivanpopelyshev]

Agree. But then it might be user problem to destroy the depth texture when he destroys renderTexture and we dont need this PR.

I don't think it matters, we can change it later, lets merge this one.

[ShukantPal]

@ivanpopelyshev Agreed, it is all semantics. Putting it in render-texture makes more sense for now, as it is a higher-level API and users might face problems like with 6556.

The user can still "snatch" the depth texture before it gets destroyed by listening to "dispose" on the base render-texture.

[bigtimebuddy]

Looks good. One small request: could you add a simple unit-test check for this? Want to make sure we capture the behavior that destroying a BaseRenderTexture also destroys the depthTexture.

[ShukantPal]

@bigtimebuddy Done!

[bigtimebuddy]

Did you forget to commit RenderTexture? Error: Cannot find module './RenderTexture'

[ShukantPal]

@bigtimebuddy Yup, I forgot a git add .

[bigtimebuddy]

Good work @SukantPal!