d3d9: Fix implicit surface refcounting.

dlls/d3d9/d3d9_private.h

@@ -176,6 +176,9 @@ typedef struct IDirect3DDevice9Impl
     /* IDirect3DDevice9 fields */
     IWineD3DDevice               *WineD3DDevice;
 
+    /* Avoids recursion with nested ReleaseRef to 0 */
+    BOOL                          inDestruction;
+
 } IDirect3DDevice9Impl;
 
 
@@ -306,6 +309,9 @@ typedef struct IDirect3DSurface9Impl
 
     /* If set forward refcounting to this object */
     IUnknown                    *forwardReference;
+
+    /* Flags an implicit surface */
+    BOOL                        isImplicit;
 } IDirect3DSurface9Impl;
 
 /* ---------------------- */
@@ -548,6 +554,8 @@ extern HRESULT WINAPI D3D9CB_CreateRenderTarget(IUnknown *device, IUnknown *pSup
 
 extern ULONG WINAPI D3D9CB_DestroyDepthStencilSurface (IWineD3DSurface *pSurface);
 
+extern ULONG WINAPI D3D9CB_DestroyRenderTarget (IWineD3DSurface *pSurface);
+
 extern ULONG WINAPI D3D9CB_DestroySurface(IWineD3DSurface *pSurface);
 
 extern ULONG WINAPI D3D9CB_DestroyVolume(IWineD3DVolume *pVolume);

dlls/d3d9/device.c

@@ -53,11 +53,15 @@ static ULONG WINAPI IDirect3DDevice9Impl_AddRef(LPDIRECT3DDEVICE9 iface) {
 
 static ULONG WINAPI IDirect3DDevice9Impl_Release(LPDIRECT3DDEVICE9 iface) {
     IDirect3DDevice9Impl *This = (IDirect3DDevice9Impl *)iface;
-    ULONG ref = InterlockedDecrement(&This->ref);
+    ULONG ref;
+
+    if (This->inDestruction) return 0;
+    ref = InterlockedDecrement(&This->ref);
 
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
+      This->inDestruction = TRUE;
       IWineD3DDevice_Uninit3D(This->WineD3DDevice, D3D9CB_DestroyDepthStencilSurface);
       IWineD3DDevice_Release(This->WineD3DDevice);
       HeapFree(GetProcessHeap(), 0, This);

dlls/d3d9/directx.c

@@ -189,14 +189,25 @@ HRESULT WINAPI D3D9CB_CreateRenderTarget(IUnknown *device, IUnknown *pSuperior,
 
     if (SUCCEEDED(res)) {
         *ppSurface = d3dSurface->wineD3DSurface;
-        IUnknown_Release(d3dSurface->parentDevice);
-        d3dSurface->parentDevice = NULL;
+        d3dSurface->isImplicit = TRUE;
+        /* Implicit surfaces are created with an refcount of 0 */
+        IUnknown_Release((IUnknown *)d3dSurface);
     } else {
         *ppSurface = NULL;
     }
     return res;
 }
 
+ULONG WINAPI D3D9CB_DestroyRenderTarget(IWineD3DSurface *pSurface) {
+    IDirect3DSurface9Impl* surfaceParent;
+    TRACE("(%p) call back\n", pSurface);
+
+    IWineD3DSurface_GetParent(pSurface, (IUnknown **) &surfaceParent);
+    surfaceParent->isImplicit = FALSE;
+    /* Surface had refcount of 0 GetParent addrefed to 1, so 1 Release is enough */
+    return IDirect3DSurface9_Release((IDirect3DSurface9*) surfaceParent);
+}
+
 HRESULT WINAPI D3D9CB_CreateAdditionalSwapChain(IUnknown *device,
                                                 WINED3DPRESENT_PARAMETERS* pPresentationParameters,
                                                 IWineD3DSwapChain ** ppSwapChain) {
@@ -263,8 +274,9 @@ HRESULT WINAPI D3D9CB_CreateDepthStencilSurface(IUnknown *device, IUnknown *pSup
                                          (IDirect3DSurface9 **)&d3dSurface, pSharedHandle);
     if (SUCCEEDED(res)) {
         *ppSurface = d3dSurface->wineD3DSurface;
-        IUnknown_Release(d3dSurface->parentDevice);
-        d3dSurface->parentDevice = NULL;
+        d3dSurface->isImplicit = TRUE;
+        /* Implicit surfaces are created with an refcount of 0 */
+        IUnknown_Release((IUnknown *)d3dSurface);
     }
     return res;
 }
@@ -274,7 +286,8 @@ ULONG WINAPI D3D9CB_DestroyDepthStencilSurface(IWineD3DSurface *pSurface) {
     TRACE("(%p) call back\n", pSurface);
 
     IWineD3DSurface_GetParent(pSurface, (IUnknown **) &surfaceParent);
-    IDirect3DSurface9_Release((IDirect3DSurface9*) surfaceParent);
+    surfaceParent->isImplicit = FALSE;
+    /* Surface had refcount of 0 GetParent addrefed to 1, so 1 Release is enough */
     return IDirect3DSurface9_Release((IDirect3DSurface9*) surfaceParent);
 }
 

dlls/d3d9/surface.c

@@ -53,6 +53,7 @@ static ULONG WINAPI IDirect3DSurface9Impl_AddRef(LPDIRECT3DSURFACE9 iface) {
     } else {
         /* No container, handle our own refcounting */
         ULONG ref = InterlockedIncrement(&This->ref);
+        if(ref == 1 && This->parentDevice) IUnknown_AddRef(This->parentDevice);
         TRACE("(%p) : AddRef from %d\n", This, ref - 1);
 
         return ref;
@@ -75,9 +76,11 @@ static ULONG WINAPI IDirect3DSurface9Impl_Release(LPDIRECT3DSURFACE9 iface) {
         TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
         if (ref == 0) {
-            IWineD3DSurface_Release(This->wineD3DSurface);
             if (This->parentDevice) IUnknown_Release(This->parentDevice);
-            HeapFree(GetProcessHeap(), 0, This);
+            if (!This->isImplicit) {
+                IWineD3DSurface_Release(This->wineD3DSurface);
+                HeapFree(GetProcessHeap(), 0, This);
+            }
         }
 
         return ref;

dlls/d3d9/swapchain.c

@@ -58,7 +58,7 @@ static ULONG WINAPI IDirect3DSwapChain9Impl_Release(LPDIRECT3DSWAPCHAIN9 iface)
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
-        IWineD3DSwapChain_Release(This->wineD3DSwapChain);
+        IWineD3DSwapChain_Destroy(This->wineD3DSwapChain, D3D9CB_DestroyRenderTarget);
         if (This->parentDevice) IUnknown_Release(This->parentDevice);
         HeapFree(GetProcessHeap(), 0, This);
     }

dlls/d3d9/tests/device.c

@@ -369,24 +369,24 @@ static void test_refcount(void)
         if(pRenderTarget)
         {
             CHECK_SURFACE_CONTAINER( pRenderTarget, IID_IDirect3DSwapChain9, pSwapChain);
-            todo_wine CHECK_REFCOUNT( pRenderTarget, 1);
+            CHECK_REFCOUNT( pRenderTarget, 1);
 
-            todo_wine CHECK_ADDREF_REFCOUNT(pRenderTarget, 2);
+            CHECK_ADDREF_REFCOUNT(pRenderTarget, 2);
             todo_wine CHECK_REFCOUNT(pDevice, refcount);
-            todo_wine CHECK_RELEASE_REFCOUNT(pRenderTarget, 1);
+            CHECK_RELEASE_REFCOUNT(pRenderTarget, 1);
             todo_wine CHECK_REFCOUNT(pDevice, refcount);
 
             hr = IDirect3DDevice9_GetRenderTarget(pDevice, 0, &pRenderTarget);
             todo_wine CHECK_CALL( hr, "GetRenderTarget", pDevice, refcount);
-            todo_wine CHECK_REFCOUNT( pRenderTarget, 2);
-            todo_wine CHECK_RELEASE_REFCOUNT( pRenderTarget, 1);
-            todo_wine CHECK_RELEASE_REFCOUNT( pRenderTarget, 0);
+            CHECK_REFCOUNT( pRenderTarget, 2);
+            CHECK_RELEASE_REFCOUNT( pRenderTarget, 1);
+            CHECK_RELEASE_REFCOUNT( pRenderTarget, 0);
             todo_wine CHECK_REFCOUNT( pDevice, --refcount);
 
             /* The render target is released with the device, so AddRef with refcount=0 is fine here. */
-            todo_wine CHECK_ADDREF_REFCOUNT(pRenderTarget, 1);
+            CHECK_ADDREF_REFCOUNT(pRenderTarget, 1);
             todo_wine CHECK_REFCOUNT(pDevice, ++refcount);
-            todo_wine CHECK_RELEASE_REFCOUNT(pRenderTarget, 0);
+            CHECK_RELEASE_REFCOUNT(pRenderTarget, 0);
             todo_wine CHECK_REFCOUNT(pDevice, --refcount);
         }
 
@@ -395,7 +395,7 @@ static void test_refcount(void)
         todo_wine CHECK_CALL( hr, "GetBackBuffer", pDevice, ++refcount);
         if(pBackBuffer)
         {
-            todo_wine CHECK_RELEASE_REFCOUNT(pBackBuffer, 0);
+            CHECK_RELEASE_REFCOUNT(pBackBuffer, 0);
             ok(pRenderTarget == pBackBuffer, "RenderTarget=%p and BackBuffer=%p should be the same.\n",
             pRenderTarget, pBackBuffer);
             pBackBuffer = NULL;
@@ -408,20 +408,20 @@ static void test_refcount(void)
         if(pStencilSurface)
         {
             CHECK_SURFACE_CONTAINER( pStencilSurface, IID_IDirect3DDevice9, pDevice);
-            todo_wine CHECK_REFCOUNT( pStencilSurface, 1);
+            CHECK_REFCOUNT( pStencilSurface, 1);
 
-            todo_wine CHECK_ADDREF_REFCOUNT(pStencilSurface, 2);
+            CHECK_ADDREF_REFCOUNT(pStencilSurface, 2);
             todo_wine CHECK_REFCOUNT(pDevice, refcount);
-            todo_wine CHECK_RELEASE_REFCOUNT(pStencilSurface, 1);
+            CHECK_RELEASE_REFCOUNT(pStencilSurface, 1);
             todo_wine CHECK_REFCOUNT(pDevice, refcount);
 
-            todo_wine CHECK_RELEASE_REFCOUNT( pStencilSurface, 0);
+            CHECK_RELEASE_REFCOUNT( pStencilSurface, 0);
             todo_wine CHECK_REFCOUNT( pDevice, --refcount);
 
             /* The stencil surface is released with the device, so AddRef with refcount=0 is fine here. */
-            todo_wine CHECK_ADDREF_REFCOUNT(pStencilSurface, 1);
+            CHECK_ADDREF_REFCOUNT(pStencilSurface, 1);
             todo_wine CHECK_REFCOUNT(pDevice, ++refcount);
-            todo_wine CHECK_RELEASE_REFCOUNT(pStencilSurface, 0);
+            CHECK_RELEASE_REFCOUNT(pStencilSurface, 0);
             todo_wine CHECK_REFCOUNT(pDevice, --refcount);
             pStencilSurface = NULL;
         }
@@ -526,10 +526,13 @@ static void test_refcount(void)
     /* Surfaces */
     hr = IDirect3DDevice9_CreateDepthStencilSurface( pDevice, 32, 32, D3DFMT_D24S8, D3DMULTISAMPLE_NONE, 0, TRUE, &pStencilSurface, NULL );
     CHECK_CALL( hr, "CreateDepthStencilSurface", pDevice, ++refcount );
+    CHECK_REFCOUNT( pStencilSurface, 1 );
     hr = IDirect3DDevice9_CreateOffscreenPlainSurface( pDevice, 32, 32, D3DFMT_X8R8G8B8, D3DPOOL_DEFAULT, &pOffscreenSurface, NULL );
     CHECK_CALL( hr, "CreateOffscreenPlainSurface", pDevice, ++refcount );
+    CHECK_REFCOUNT( pOffscreenSurface, 1 );
     hr = IDirect3DDevice9_CreateRenderTarget( pDevice, 32, 32, D3DFMT_X8R8G8B8, D3DMULTISAMPLE_NONE, 0, TRUE, &pRenderTarget3, NULL );
     CHECK_CALL( hr, "CreateRenderTarget", pDevice, ++refcount );
+    CHECK_REFCOUNT( pRenderTarget3, 1 );
     /* Misc */
     hr = IDirect3DDevice9_CreateStateBlock( pDevice, D3DSBT_ALL, &pStateBlock );
     CHECK_CALL( hr, "CreateStateBlock", pDevice, ++refcount );
@@ -539,19 +542,19 @@ static void test_refcount(void)
     {
         /* check implicit back buffer */
         hr = IDirect3DSwapChain9_GetBackBuffer(pSwapChain, 0, 0, &pBackBuffer);
-        todo_wine CHECK_CALL( hr, "GetBackBuffer", pDevice, ++refcount);
+        CHECK_CALL( hr, "GetBackBuffer", pDevice, ++refcount);
         CHECK_REFCOUNT( pSwapChain, 1);
         if(pBackBuffer)
         {
             CHECK_SURFACE_CONTAINER( pBackBuffer, IID_IDirect3DSwapChain9, pSwapChain);
-            todo_wine CHECK_REFCOUNT( pBackBuffer, 1);
-            todo_wine CHECK_RELEASE_REFCOUNT( pBackBuffer, 0);
+            CHECK_REFCOUNT( pBackBuffer, 1);
+            CHECK_RELEASE_REFCOUNT( pBackBuffer, 0);
             CHECK_REFCOUNT( pDevice, --refcount);
 
             /* The back buffer is released with the swapchain, so AddRef with refcount=0 is fine here. */
-            todo_wine CHECK_ADDREF_REFCOUNT(pBackBuffer, 1);
-            todo_wine CHECK_REFCOUNT(pDevice, ++refcount);
-            todo_wine CHECK_RELEASE_REFCOUNT(pBackBuffer, 0);
+            CHECK_ADDREF_REFCOUNT(pBackBuffer, 1);
+            CHECK_REFCOUNT(pDevice, ++refcount);
+            CHECK_RELEASE_REFCOUNT(pBackBuffer, 0);
             CHECK_REFCOUNT(pDevice, --refcount);
             pBackBuffer = NULL;
         }
@@ -568,10 +571,10 @@ static void test_refcount(void)
     /* The implicit render target is not freed if refcount reaches 0.
      * Otherwise GetRenderTarget would re-allocate it and the pointer would change.*/
     hr = IDirect3DDevice9_GetRenderTarget(pDevice, 0, &pRenderTarget2);
-    todo_wine CHECK_CALL( hr, "GetRenderTarget", pDevice, ++refcount);
+    CHECK_CALL( hr, "GetRenderTarget", pDevice, ++refcount);
     if(pRenderTarget2)
     {
-        todo_wine CHECK_RELEASE_REFCOUNT(pRenderTarget2, 0);
+        CHECK_RELEASE_REFCOUNT(pRenderTarget2, 0);
         ok(pRenderTarget == pRenderTarget2, "RenderTarget=%p and RenderTarget2=%p should be the same.\n",
            pRenderTarget, pRenderTarget2);
         CHECK_REFCOUNT( pDevice, --refcount);