FrameBuffer depthStencil texture destruction fix (#1538)

- [JS thread] engine is disposed from 
- [main thread] resetView is called 
- [main thread] DisableRendering -> bgfx context is destroyed
- [JS thread] ExternalWithFinalizer for the `depthStencilTextureHandle`

---------

Co-authored-by: Gary Hsu <[email protected]>

Author: CedricGuillemet

.github/jobs/test_install_win32.yml

@@ -42,7 +42,7 @@ jobs:
       # BGFX_CONFIG_MAX_FRAME_BUFFERS is set so enough Framebuffers are available before V8 starts disposing unused ones
       - script: |
           # BGFX_CONFIG_MAX_FRAME_BUFFERS is set so enough Framebuffers are available before V8 starts disposing unused ones
-          cmake -B build${{ variables.solutionName }} -A ${{ parameters.platform }} ${{ variables.jsEngineDefine }} -D BX_CONFIG_DEBUG=ON -D GRAPHICS_API=${{ parameters.graphics_api }} -D CMAKE_UNITY_BUILD=$(UNITY_BUILD) -D BGFX_CONFIG_MAX_FRAME_BUFFERS=256 -D BABYLON_DEBUG_TRACE=ON
+          cmake -G "Visual Studio 17 2022" -B build${{ variables.solutionName }} -A ${{ parameters.platform }} ${{ variables.jsEngineDefine }} -D BX_CONFIG_DEBUG=ON -D GRAPHICS_API=${{ parameters.graphics_api }} -D CMAKE_UNITY_BUILD=$(UNITY_BUILD) -D BGFX_CONFIG_MAX_FRAME_BUFFERS=256 -D BABYLON_DEBUG_TRACE=ON
         displayName: 'Generate ${{ variables.solutionName }} solution'
     
       - task: MSBuild@1

.github/jobs/uwp.yml

@@ -32,7 +32,7 @@ jobs:
         vmImage: ${{ parameters.vmImage }}
 
     - script: |
-        cmake -B build${{ variables.solutionName }} -D CMAKE_SYSTEM_NAME=WindowsStore -D CMAKE_SYSTEM_VERSION=10.0 ${{ variables.jsEngineDefine }} -A ${{ parameters.platform }} -D CMAKE_UNITY_BUILD=$(UNITY_BUILD)  -D BABYLON_DEBUG_TRACE=ON
+        cmake -G "Visual Studio 17 2022" -B build${{ variables.solutionName }} -D CMAKE_SYSTEM_NAME=WindowsStore -D CMAKE_SYSTEM_VERSION=10.0 ${{ variables.jsEngineDefine }} -A ${{ parameters.platform }} -D CMAKE_UNITY_BUILD=$(UNITY_BUILD)  -D BABYLON_DEBUG_TRACE=ON
       displayName: 'Generate ${{ variables.solutionName }} solution'
   
     - task: VSBuild@1

.github/jobs/win32.yml

@@ -40,7 +40,7 @@ jobs:
 
       # BGFX_CONFIG_MAX_FRAME_BUFFERS is set so enough Framebuffers are available before V8 starts disposing unused ones
       - script: |
-          cmake -B build${{ variables.solutionName }} -A ${{ parameters.platform }} ${{ variables.jsEngineDefine }} -D BX_CONFIG_DEBUG=ON -D GRAPHICS_API=${{ parameters.graphics_api }} -D CMAKE_UNITY_BUILD=$(UNITY_BUILD) -D BGFX_CONFIG_MAX_FRAME_BUFFERS=256 -D BABYLON_DEBUG_TRACE=ON
+          cmake -G "Visual Studio 17 2022" -B build${{ variables.solutionName }} -A ${{ parameters.platform }} ${{ variables.jsEngineDefine }} -D BX_CONFIG_DEBUG=ON -D GRAPHICS_API=${{ parameters.graphics_api }} -D CMAKE_UNITY_BUILD=$(UNITY_BUILD) -D BGFX_CONFIG_MAX_FRAME_BUFFERS=256 -D BABYLON_DEBUG_TRACE=ON
         displayName: 'Generate ${{ variables.solutionName }} solution'
     
       - task: MSBuild@1

Core/Graphics/InternalInclude/Babylon/Graphics/FrameBuffer.h

@@ -20,7 +20,7 @@ namespace Babylon::Graphics
     class FrameBuffer final
     {
     public:
-        FrameBuffer(DeviceContext& context, bgfx::FrameBufferHandle handle, uint16_t width, uint16_t height, bool defaultBackBuffer, bool hasDepth, bool hasStencil);
+        FrameBuffer(DeviceContext& context, bgfx::FrameBufferHandle handle, uint16_t width, uint16_t height, bool defaultBackBuffer, bool hasDepth, bool hasStencil, int8_t depthStencilAttachmentIndex = -1);
         ~FrameBuffer();
 
         FrameBuffer(const FrameBuffer&) = delete;
@@ -69,5 +69,6 @@ namespace Babylon::Graphics
         Rect m_desiredScissor{};
 
         bool m_disposed{};
+        int8_t m_depthStencilAttachmentIndex{-1};
     };
 }

Core/Graphics/Source/DeviceImpl.h

@@ -109,7 +109,6 @@ namespace Babylon::Graphics
 
         void UpdateBgfxState();
         void UpdateBgfxResolution();
-        void DiscardIfDirty();
         void RequestScreenShots();
         void Frame();
         bgfx::Encoder* GetEncoderForThread();

Core/Graphics/Source/FrameBuffer.cpp

@@ -5,7 +5,7 @@
 
 namespace Babylon::Graphics
 {
-    FrameBuffer::FrameBuffer(DeviceContext& deviceContext, bgfx::FrameBufferHandle handle, uint16_t width, uint16_t height, bool defaultBackBuffer, bool hasDepth, bool hasStencil)
+    FrameBuffer::FrameBuffer(DeviceContext& deviceContext, bgfx::FrameBufferHandle handle, uint16_t width, uint16_t height, bool defaultBackBuffer, bool hasDepth, bool hasStencil, int8_t depthStencilAttachmentIndex)
         : m_deviceContext{deviceContext}
         , m_deviceID{deviceContext.GetDeviceId()}
         , m_handle{handle}
@@ -15,6 +15,7 @@ namespace Babylon::Graphics
         , m_hasDepth{hasDepth}
         , m_hasStencil{hasStencil}
         , m_disposed{false}
+        , m_depthStencilAttachmentIndex{depthStencilAttachmentIndex}
     {
     }
 
@@ -30,10 +31,19 @@ namespace Babylon::Graphics
             return;
         }
 
-        if (bgfx::isValid(m_handle) && m_deviceID == m_deviceContext.GetDeviceId())
+        if (m_deviceID == m_deviceContext.GetDeviceId())
         {
-            bgfx::destroy(m_handle);
-            m_handle = BGFX_INVALID_HANDLE;
+            if (m_depthStencilAttachmentIndex >= 0)
+            {
+                bgfx::destroy(bgfx::getTexture(m_handle, m_depthStencilAttachmentIndex));
+                m_depthStencilAttachmentIndex = -1;
+            }
+
+            if (bgfx::isValid(m_handle))
+            {
+                bgfx::destroy(m_handle);
+                m_handle = BGFX_INVALID_HANDLE;
+            }
         }
 
         m_disposed = true;

Plugins/NativeEngine/Source/NativeEngine.cpp

@@ -1850,6 +1850,7 @@ namespace Babylon
         }
 
         bgfx::TextureHandle depthStencilTextureHandle = BGFX_INVALID_HANDLE;
+        int8_t depthStencilAttachmentIndex = -1;
         if (generateStencilBuffer || generateDepth)
         {
             if (generateStencilBuffer && !generateDepth)
@@ -1874,6 +1875,7 @@ namespace Babylon
             // And not sure it makes sense to generate mipmaps from a depth buffer with exponential values.
             // only allows mipmaps resolve step when mipmapping is asked and for the color texture, not the depth.
             // https://github.com/bkaradzic/bgfx/blob/2c21f68998595fa388e25cb6527e82254d0e9bff/src/renderer_d3d11.cpp#L4525
+            depthStencilAttachmentIndex = numAttachments;
             attachments[numAttachments++].init(depthStencilTextureHandle);
         }
 
@@ -1888,15 +1890,8 @@ namespace Babylon
             throw Napi::Error::New(info.Env(), "Failed to create frame buffer");
         }
 
-        Graphics::FrameBuffer* frameBuffer = new Graphics::FrameBuffer(m_deviceContext, frameBufferHandle, width, height, false, generateDepth, generateStencilBuffer);
-        return Napi::Pointer<Graphics::FrameBuffer>::Create(info.Env(), frameBuffer, [frameBuffer, depthStencilTextureHandle]() {
-            if (bgfx::isValid(depthStencilTextureHandle))
-            {
-                bgfx::destroy(depthStencilTextureHandle);
-            }
-
-            delete frameBuffer;
-        });
+        Graphics::FrameBuffer* frameBuffer = new Graphics::FrameBuffer(m_deviceContext, frameBufferHandle, width, height, false, generateDepth, generateStencilBuffer, depthStencilAttachmentIndex);
+        return Napi::Pointer<Graphics::FrameBuffer>::Create(info.Env(), frameBuffer, Napi::NapiPointerDeleter(frameBuffer));
     }
 
     // TODO: This doesn't get called when an Engine instance is disposed.

nightly.yml

@@ -21,7 +21,7 @@ jobs:
   - checkout: self
 
   - script: |
-      cmake -B build -A x64 -D BX_CONFIG_DEBUG=ON -D BGFX_CONFIG_MAX_FRAME_BUFFERS=256 -D BABYLON_DEBUG_TRACE=ON
+      cmake -G "Visual Studio 17 2022" -B build -A x64 -D BX_CONFIG_DEBUG=ON -D BGFX_CONFIG_MAX_FRAME_BUFFERS=256 -D BABYLON_DEBUG_TRACE=ON
     displayName: 'Generate solution'
 
   - script: |