RFC: d3d8 refcount implementation

Markus Amsler markus.amsler at oribi.org
Mon Nov 13 18:46:22 CST 2006


After spamming wine-patches with d3d tests, I tried to implement the
probed behaviour. Attached are two patches, which makes all d3d8
refcount test pass. They are small and ugly. The problems are:

The implicit surfaces now gets released if the refcount is -1. Somehow 
we have to force releasing d3d8 sufaces from wined3d. No idea how.

I added a flag to mark the implicit surfaces. It's redundant, because 
wined3d knows the implicit surfaces. The other idea is calling 
GetRenderTarget and friends in Surface_(AddRef/Release). Besides the 
problem of an endless recursion, it would be quite an overhead.

Another problem is, that IWineD3DDeviceImpl_Uninit3D calls via GetParent 
AddRef/Release on the d3d8 surface. This causes the device to be 
released again(=>endless recursion). I "solved" it with ignoring  an 
addref from 0 in the d3d8 Device, which causes the release to be -1 => 
no wined3d device releasing. Works but ugly.

Ideas, suggestions?






-------------- next part --------------
>From 1a73ec5958e2e4c7fd076184c9e1f1cf7427d936 Mon Sep 17 00:00:00 2001
From: Markus Amsler <markus.amsler at oribi.org>
Date: Mon, 13 Nov 2006 22:29:12 +0100
Subject: [PATCH] d3d8: Force implicit surfaces to handle their own refcount.
To: wine-patches <wine-patches at winehq.org>

---
 dlls/d3d8/d3d8_private.h |    3 +++
 dlls/d3d8/directx.c      |    2 ++
 dlls/d3d8/surface.c      |    4 ++--
 dlls/d3d8/tests/device.c |    6 +++---
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/dlls/d3d8/d3d8_private.h b/dlls/d3d8/d3d8_private.h
index b32033f..9ca9989 100644
--- a/dlls/d3d8/d3d8_private.h
+++ b/dlls/d3d8/d3d8_private.h
@@ -246,6 +246,9 @@ struct IDirect3DSurface8Impl
 
     /* Parent reference */
     LPDIRECT3DDEVICE8                  parentDevice;
+
+    /* Flags an implicit surface */
+    BOOL                      implicit;
 };
 
 /* ------------------ */
diff --git a/dlls/d3d8/directx.c b/dlls/d3d8/directx.c
index 42ae40d..b51181e 100644
--- a/dlls/d3d8/directx.c
+++ b/dlls/d3d8/directx.c
@@ -202,6 +202,7 @@ HRESULT WINAPI D3D8CB_CreateRenderTarget
         *ppSurface = d3dSurface->wineD3DSurface;
         IUnknown_Release(d3dSurface->parentDevice);
         d3dSurface->parentDevice = NULL;
+        d3dSurface->implicit = TRUE;
     } else {
         *ppSurface = NULL;
     }
@@ -281,6 +282,7 @@ HRESULT WINAPI D3D8CB_CreateDepthStencil
         *ppSurface = d3dSurface->wineD3DSurface;
         IUnknown_Release(d3dSurface->parentDevice);
         d3dSurface->parentDevice = NULL;
+        d3dSurface->implicit = TRUE;
     }
     return res;
 }
diff --git a/dlls/d3d8/surface.c b/dlls/d3d8/surface.c
index 8ab312e..83265e9 100644
--- a/dlls/d3d8/surface.c
+++ b/dlls/d3d8/surface.c
@@ -46,7 +46,7 @@ static ULONG WINAPI IDirect3DSurface8Imp
 
     TRACE("(%p)\n", This);
 
-    IWineD3DSurface_GetContainerParent(This->wineD3DSurface, &containerParent);
+    if (!This->implicit) IWineD3DSurface_GetContainerParent(This->wineD3DSurface, &containerParent);
     if (containerParent) {
         /* Forward to the containerParent */
         TRACE("(%p) : Forwarding to %p\n", This, containerParent);
@@ -65,7 +65,7 @@ static ULONG WINAPI IDirect3DSurface8Imp
 
     TRACE("(%p)\n", This);
 
-    IWineD3DSurface_GetContainerParent(This->wineD3DSurface, &containerParent);
+    if (!This->implicit) IWineD3DSurface_GetContainerParent(This->wineD3DSurface, &containerParent);
     if (containerParent) {
         /* Forward to the containerParent */
         TRACE("(%p) : Forwarding to %p\n", This, containerParent);
diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c
index 06f8aab..86e10af 100644
--- a/dlls/d3d8/tests/device.c
+++ b/dlls/d3d8/tests/device.c
@@ -429,17 +429,17 @@ static void test_refcount(void)
         /* check implicit back buffer */
         hr = IDirect3DSwapChain8_GetBackBuffer(pSwapChain, 0, 0, &pBackBuffer);
         todo_wine CHECK_CALL( hr, "GetBackBuffer", pDevice, ++refcount);
-        todo_wine CHECK_REFCOUNT( pSwapChain, 1);
+        CHECK_REFCOUNT( pSwapChain, 1);
         if(pBackBuffer)
         {
             todo_wine CHECK_CONTAINER( Surface, pBackBuffer, IID_IDirect3DDevice8, pDevice, S_OK);
             todo_wine CHECK_REFCOUNT( pBackBuffer, 1);
 
             todo_wine CHECK_ADDREF_REFCOUNT(pBackBuffer, 2);
-            todo_wine CHECK_REFCOUNT(pSwapChain, 1);
+            CHECK_REFCOUNT(pSwapChain, 1);
             todo_wine CHECK_REFCOUNT(pDevice, refcount);
             todo_wine CHECK_RELEASE_REFCOUNT(pBackBuffer, 1);
-            todo_wine CHECK_REFCOUNT(pSwapChain, 1);
+            CHECK_REFCOUNT(pSwapChain, 1);
             todo_wine CHECK_REFCOUNT(pDevice, refcount);
 
             todo_wine CHECK_RELEASE_REFCOUNT( pBackBuffer, 0);
-- 
1.4.3.3


-------------- next part --------------
>From 99bea2c16f008c412d578c81919bc52985396145 Mon Sep 17 00:00:00 2001
From: Markus Amsler <markus.amsler at oribi.org>
Date: Tue, 14 Nov 2006 00:15:31 +0100
Subject: [PATCH] d3d8: Hack to fix all refcount issues.
To: wine-patches <wine-patches at winehq.org>

---
 dlls/d3d8/device.c       |    1 +
 dlls/d3d8/directx.c      |    6 +--
 dlls/d3d8/surface.c      |    7 ++--
 dlls/d3d8/tests/device.c |   70 +++++++++++++++++++++++-----------------------
 dlls/wined3d/device.c    |    2 +-
 5 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/dlls/d3d8/device.c b/dlls/d3d8/device.c
index 1fa10da..04d37f6 100644
--- a/dlls/d3d8/device.c
+++ b/dlls/d3d8/device.c
@@ -80,6 +80,7 @@ static HRESULT WINAPI IDirect3DDevice8Im
 
 static ULONG WINAPI IDirect3DDevice8Impl_AddRef(LPDIRECT3DDEVICE8 iface) {
     IDirect3DDevice8Impl *This = (IDirect3DDevice8Impl *)iface;
+    if (This->ref == 0) return 0;
     ULONG ref = InterlockedIncrement(&This->ref);
 
     TRACE("(%p) : AddRef from %d\n", This, ref - 1);
diff --git a/dlls/d3d8/directx.c b/dlls/d3d8/directx.c
index b51181e..ba6eec5 100644
--- a/dlls/d3d8/directx.c
+++ b/dlls/d3d8/directx.c
@@ -200,9 +200,8 @@ HRESULT WINAPI D3D8CB_CreateRenderTarget
 
     if (SUCCEEDED(res)) {
         *ppSurface = d3dSurface->wineD3DSurface;
-        IUnknown_Release(d3dSurface->parentDevice);
-        d3dSurface->parentDevice = NULL;
         d3dSurface->implicit = TRUE;
+        IDirect3DSurface8_Release((IDirect3DSurface8 *)d3dSurface);
     } else {
         *ppSurface = NULL;
     }
@@ -280,9 +279,8 @@ HRESULT WINAPI D3D8CB_CreateDepthStencil
                                          (D3DFORMAT)Format, MultiSample, (IDirect3DSurface8 **)&d3dSurface);
     if (SUCCEEDED(res)) {
         *ppSurface = d3dSurface->wineD3DSurface;
-        IUnknown_Release(d3dSurface->parentDevice);
-        d3dSurface->parentDevice = NULL;
         d3dSurface->implicit = TRUE;
+        IDirect3DSurface8_Release((IDirect3DSurface8 *)d3dSurface);
     }
     return res;
 }
diff --git a/dlls/d3d8/surface.c b/dlls/d3d8/surface.c
index 83265e9..f15a5fa 100644
--- a/dlls/d3d8/surface.c
+++ b/dlls/d3d8/surface.c
@@ -55,6 +55,7 @@ static ULONG WINAPI IDirect3DSurface8Imp
         /* No container, handle our own refcounting */
         ULONG ref = InterlockedIncrement(&This->ref);
         TRACE("(%p) : AddRef from %d\n", This, ref - 1);
+        if (This->parentDevice && ref == 1) IUnknown_AddRef(This->parentDevice);
         return ref;
     }
 }
@@ -75,12 +76,12 @@ static ULONG WINAPI IDirect3DSurface8Imp
         ULONG ref = InterlockedDecrement(&This->ref);
         TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
-        if (ref == 0) {
+        if (This->parentDevice && ref == 0) IUnknown_Release(This->parentDevice);
+        if ((ref == 0 && !This->implicit) || ref == -1) {
             IWineD3DSurface_Release(This->wineD3DSurface);
-            if (This->parentDevice) IUnknown_Release(This->parentDevice);
             HeapFree(GetProcessHeap(), 0, This);
         }
-
+        if (ref==-1) ref = 0;
         return ref;
     }
 }
diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c
index 86e10af..d2cf7b5 100644
--- a/dlls/d3d8/tests/device.c
+++ b/dlls/d3d8/tests/device.c
@@ -297,38 +297,38 @@ static void test_refcount(void)
      *   - the refcount is not forwarded to the container.
      */
     hr = IDirect3DDevice8_GetRenderTarget(pDevice, &pRenderTarget);
-    todo_wine CHECK_CALL( hr, "GetRenderTarget", pDevice, ++refcount);
+    CHECK_CALL( hr, "GetRenderTarget", pDevice, ++refcount);
     if(pRenderTarget)
     {
         todo_wine CHECK_CONTAINER( Surface, pRenderTarget, IID_IDirect3DDevice8, pDevice, S_OK);
-        todo_wine CHECK_REFCOUNT( pRenderTarget, 1);
+        CHECK_REFCOUNT( pRenderTarget, 1);
 
-        todo_wine CHECK_ADDREF_REFCOUNT(pRenderTarget, 2);
-        todo_wine CHECK_REFCOUNT(pDevice, refcount);
-        todo_wine CHECK_RELEASE_REFCOUNT(pRenderTarget, 1);
-        todo_wine CHECK_REFCOUNT(pDevice, refcount);
+        CHECK_ADDREF_REFCOUNT(pRenderTarget, 2);
+        CHECK_REFCOUNT(pDevice, refcount);
+        CHECK_RELEASE_REFCOUNT(pRenderTarget, 1);
+        CHECK_REFCOUNT(pDevice, refcount);
 
         hr = IDirect3DDevice8_GetRenderTarget(pDevice, &pRenderTarget);
-        todo_wine CHECK_CALL( hr, "GetRenderTarget", pDevice, refcount);
-        todo_wine CHECK_REFCOUNT( pRenderTarget, 2);
-        todo_wine CHECK_RELEASE_REFCOUNT( pRenderTarget, 1);
+        CHECK_CALL( hr, "GetRenderTarget", pDevice, refcount);
+        CHECK_REFCOUNT( pRenderTarget, 2);
+        CHECK_RELEASE_REFCOUNT( pRenderTarget, 1);
 
-        todo_wine CHECK_RELEASE_REFCOUNT( pRenderTarget, 0);
+        CHECK_RELEASE_REFCOUNT( pRenderTarget, 0);
         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);
-        todo_wine CHECK_REFCOUNT(pDevice, ++refcount);
-        todo_wine CHECK_RELEASE_REFCOUNT(pRenderTarget, 0);
+        CHECK_ADDREF_REFCOUNT(pRenderTarget, 1);
+        CHECK_REFCOUNT(pDevice, ++refcount);
+        CHECK_RELEASE_REFCOUNT(pRenderTarget, 0);
         CHECK_REFCOUNT(pDevice, --refcount);
     }
 
     /* Render target and back buffer are the same. */
     hr = IDirect3DDevice8_GetBackBuffer(pDevice, 0, 0, &pBackBuffer);
-    todo_wine CHECK_CALL( hr, "GetBackBuffer", pDevice, ++refcount);
+    CHECK_CALL( hr, "GetBackBuffer", pDevice, ++refcount);
     if(pBackBuffer)
     {
-        todo_wine CHECK_RELEASE_REFCOUNT(pBackBuffer, 0);
+        CHECK_RELEASE_REFCOUNT(pBackBuffer, 0);
         CHECK_REFCOUNT( pDevice, --refcount);
         ok(pRenderTarget == pBackBuffer, "RenderTarget=%p and BackBuffer=%p should be the same.\n",
            pRenderTarget, pBackBuffer);
@@ -337,24 +337,24 @@ static void test_refcount(void)
     pRenderTarget = NULL;
 
     hr = IDirect3DDevice8_GetDepthStencilSurface(pDevice, &pStencilSurface);
-    todo_wine CHECK_CALL( hr, "GetDepthStencilSurface", pDevice, ++refcount);
+    CHECK_CALL( hr, "GetDepthStencilSurface", pDevice, ++refcount);
     if(pStencilSurface)
     {
         CHECK_CONTAINER( Surface, pStencilSurface, IID_IDirect3DDevice8, pDevice, S_OK);
-        todo_wine CHECK_REFCOUNT( pStencilSurface, 1);
+        CHECK_REFCOUNT( pStencilSurface, 1);
 
-        todo_wine CHECK_ADDREF_REFCOUNT(pStencilSurface, 2);
-        todo_wine CHECK_REFCOUNT(pDevice, refcount);
-        todo_wine CHECK_RELEASE_REFCOUNT(pStencilSurface, 1);
-        todo_wine CHECK_REFCOUNT(pDevice, refcount);
+        CHECK_ADDREF_REFCOUNT(pStencilSurface, 2);
+        CHECK_REFCOUNT(pDevice, refcount);
+        CHECK_RELEASE_REFCOUNT(pStencilSurface, 1);
+        CHECK_REFCOUNT(pDevice, refcount);
 
-        todo_wine CHECK_RELEASE_REFCOUNT( pStencilSurface, 0);
+        CHECK_RELEASE_REFCOUNT( pStencilSurface, 0);
         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);
-        todo_wine CHECK_REFCOUNT(pDevice, ++refcount);
-        todo_wine CHECK_RELEASE_REFCOUNT(pStencilSurface, 0);
+        CHECK_ADDREF_REFCOUNT(pStencilSurface, 1);
+        CHECK_REFCOUNT(pDevice, ++refcount);
+        CHECK_RELEASE_REFCOUNT(pStencilSurface, 0);
         CHECK_REFCOUNT(pDevice, --refcount);
 
     }
@@ -428,27 +428,27 @@ static void test_refcount(void)
     {
         /* check implicit back buffer */
         hr = IDirect3DSwapChain8_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)
         {
             todo_wine CHECK_CONTAINER( Surface, pBackBuffer, IID_IDirect3DDevice8, pDevice, S_OK);
-            todo_wine CHECK_REFCOUNT( pBackBuffer, 1);
+            CHECK_REFCOUNT( pBackBuffer, 1);
 
-            todo_wine CHECK_ADDREF_REFCOUNT(pBackBuffer, 2);
+            CHECK_ADDREF_REFCOUNT(pBackBuffer, 2);
             CHECK_REFCOUNT(pSwapChain, 1);
-            todo_wine CHECK_REFCOUNT(pDevice, refcount);
-            todo_wine CHECK_RELEASE_REFCOUNT(pBackBuffer, 1);
+            CHECK_REFCOUNT(pDevice, refcount);
+            CHECK_RELEASE_REFCOUNT(pBackBuffer, 1);
             CHECK_REFCOUNT(pSwapChain, 1);
-            todo_wine CHECK_REFCOUNT(pDevice, refcount);
+            CHECK_REFCOUNT(pDevice, refcount);
 
-            todo_wine CHECK_RELEASE_REFCOUNT( pBackBuffer, 0);
+            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;
diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 794cba3..12dcf95 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -2170,7 +2170,7 @@ static HRESULT WINAPI IWineD3DDeviceImpl
     This->stencilBufferTarget = NULL;
 
     TRACE("Releasing the render target at %p\n", This->renderTarget);
-    if(IWineD3DSurface_Release(This->renderTarget) >0){
+    if(This->renderTarget && IWineD3DSurface_Release(This->renderTarget) >0){
           /* This check is a bit silly, itshould be in swapchain_release FIXME("(%p) Something's still holding the renderTarget\n",This); */
     }
     TRACE("Setting rendertarget to NULL\n");
-- 
1.4.3.3




More information about the wine-devel mailing list