From bd3569ff9557b6ada3d1e24ffd0a9d70e044a2d0 Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Tue, 16 Jul 2019 21:17:35 +0100 Subject: [PATCH 1/3] Workaround & LOG_TEST_CASE for missing DestroyResource patch --- src/core/hle/D3D8/Direct3D9/Direct3D9.cpp | 31 +++++++++++++++++------ 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp b/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp index 0b0df6a15..235057872 100644 --- a/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp +++ b/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp @@ -5576,33 +5576,48 @@ ULONG WINAPI XTL::EMUPATCH(D3DResource_Release) // Was the Xbox resource freed? if (uRet == 0) { - // If this was a cached render target or depth surface, clear the cache variable too! + // We should free any variables storing the resource when freed + // HACK: For now, we skip the release and issue a LOG_TEST_CASE + // This is a (small) memory leak, but prevents incorrectly freeing an in-use resource + // This is a *temporary* work around until solving + // https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/issues/1665 if (pThis == g_pXboxRenderTarget) { - g_pXboxRenderTarget = nullptr; + LOG_TEST_CASE("Skipping release of active Xbox Render Target"); + RETURN(uRet); + //g_pXboxRenderTarget = nullptr; } if (pThis == g_pXboxDepthStencil) { - g_pXboxDepthStencil = nullptr; + LOG_TEST_CASE("Skipping release of active Xbox Depth Stencil"); + RETURN(uRet); + //g_pXboxDepthStencil = nullptr; } if (pThis == g_XboxBackBufferSurface) { - g_XboxBackBufferSurface = nullptr; + LOG_TEST_CASE("Skipping release of active Xbox Render Target"); + RETURN(uRet); + //g_XboxBackBufferSurface = nullptr; } if (pThis == g_XboxDefaultDepthStencilSurface) { - g_XboxDefaultDepthStencilSurface = nullptr; + LOG_TEST_CASE("Skipping release of default Xbox Depth Stencil"); + RETURN(uRet); + //g_XboxDefaultDepthStencilSurface = nullptr; } for (int i = 0; i < TEXTURE_STAGES; i++) { - if (pThis == EmuD3DActiveTexture[i]) - EmuD3DActiveTexture[i] = nullptr; + if (pThis == EmuD3DActiveTexture[i]) { + LOG_TEST_CASE("Skipping release of active Xbox Texture"); + RETURN(uRet); + //EmuD3DActiveTexture[i] = nullptr; + } } // Also release the host copy (if it exists!) FreeHostResource(key); } - return uRet; + RETURN(uRet); } // ****************************************************************** From 4190b81431d10cd5316df547aabcc989c3613e30 Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Wed, 17 Jul 2019 10:10:51 +0100 Subject: [PATCH 2/3] Add an early-out for non-zero internal reference count --- src/core/hle/D3D8/Direct3D9/Direct3D9.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp b/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp index 235057872..f7ba43a62 100644 --- a/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp +++ b/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp @@ -5577,10 +5577,15 @@ ULONG WINAPI XTL::EMUPATCH(D3DResource_Release) // Was the Xbox resource freed? if (uRet == 0) { // We should free any variables storing the resource when freed - // HACK: For now, we skip the release and issue a LOG_TEST_CASE + // HACK: For now, we skip the release when the resource has a non-zero internal ref count + // We also skip the release if we still have a cached variable containing this value // This is a (small) memory leak, but prevents incorrectly freeing an in-use resource - // This is a *temporary* work around until solving - // https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/issues/1665 + // This is a *temporary* work around until solving https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/issues/1665 + if ((pThis->Common & X_D3DCOMMON_INTREFCOUNT_MASK) != 0) { + LOG_TEST_CASE("Skipping Release of resource with a non-zero internal reference count"); + RETURN(uRet); + } + if (pThis == g_pXboxRenderTarget) { LOG_TEST_CASE("Skipping release of active Xbox Render Target"); RETURN(uRet); From abe82f0336bb379d14f386a78c77f51a39b86979 Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Wed, 17 Jul 2019 14:58:51 +0100 Subject: [PATCH 3/3] Update to be informational only --- src/core/hle/D3D8/Direct3D9/Direct3D9.cpp | 36 +++++++++-------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp b/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp index f7ba43a62..e1eca93fb 100644 --- a/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp +++ b/src/core/hle/D3D8/Direct3D9/Direct3D9.cpp @@ -5576,45 +5576,37 @@ ULONG WINAPI XTL::EMUPATCH(D3DResource_Release) // Was the Xbox resource freed? if (uRet == 0) { - // We should free any variables storing the resource when freed - // HACK: For now, we skip the release when the resource has a non-zero internal ref count - // We also skip the release if we still have a cached variable containing this value - // This is a (small) memory leak, but prevents incorrectly freeing an in-use resource - // This is a *temporary* work around until solving https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/issues/1665 + + // Generate some test cases so we know what to investigate/re-test after + // solving https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/issues/1665 if ((pThis->Common & X_D3DCOMMON_INTREFCOUNT_MASK) != 0) { - LOG_TEST_CASE("Skipping Release of resource with a non-zero internal reference count"); - RETURN(uRet); + LOG_TEST_CASE("Release of resource with a non-zero internal reference count"); } if (pThis == g_pXboxRenderTarget) { - LOG_TEST_CASE("Skipping release of active Xbox Render Target"); - RETURN(uRet); - //g_pXboxRenderTarget = nullptr; + LOG_TEST_CASE("Release of active Xbox Render Target"); + g_pXboxRenderTarget = nullptr; } if (pThis == g_pXboxDepthStencil) { - LOG_TEST_CASE("Skipping release of active Xbox Depth Stencil"); - RETURN(uRet); - //g_pXboxDepthStencil = nullptr; + LOG_TEST_CASE("Release of active Xbox Depth Stencil"); + g_pXboxDepthStencil = nullptr; } if (pThis == g_XboxBackBufferSurface) { - LOG_TEST_CASE("Skipping release of active Xbox Render Target"); - RETURN(uRet); - //g_XboxBackBufferSurface = nullptr; + LOG_TEST_CASE("Release of active Xbox Render Target"); + g_XboxBackBufferSurface = nullptr; } if (pThis == g_XboxDefaultDepthStencilSurface) { - LOG_TEST_CASE("Skipping release of default Xbox Depth Stencil"); - RETURN(uRet); - //g_XboxDefaultDepthStencilSurface = nullptr; + LOG_TEST_CASE("Release of default Xbox Depth Stencil"); + g_XboxDefaultDepthStencilSurface = nullptr; } for (int i = 0; i < TEXTURE_STAGES; i++) { if (pThis == EmuD3DActiveTexture[i]) { - LOG_TEST_CASE("Skipping release of active Xbox Texture"); - RETURN(uRet); - //EmuD3DActiveTexture[i] = nullptr; + LOG_TEST_CASE("Release of active Xbox Texture"); + EmuD3DActiveTexture[i] = nullptr; } }