d3d9: Rework the converted vertex declaration management.

dlls/d3d9/d3d9_private.h

@@ -178,10 +178,8 @@ typedef struct IDirect3DDevice9Impl
     /* Avoids recursion with nested ReleaseRef to 0 */
     BOOL                          inDestruction;
 
-    /* A vertex declaration was converted from setFVF.
-     * Keep track of it, so it can be properly freed
-     */
-    IDirect3DVertexDeclaration9  *convertedDecl;
+    IDirect3DVertexDeclaration9  **convertedDecls;
+    unsigned int                 numConvertedDecls, declArraySize;
 
 } IDirect3DDevice9Impl;
 
@@ -485,11 +483,14 @@ typedef struct IDirect3DVertexDeclaration9Impl {
 
   /* IDirect3DVertexDeclaration9 fields */
   IWineD3DVertexDeclaration *wineD3DVertexDeclaration;
-  
+  DWORD convFVF;
+
   /* Parent reference */
   LPDIRECT3DDEVICE9 parentDevice;
 } IDirect3DVertexDeclaration9Impl;
 
+void IDirect3DVertexDeclaration9Impl_Destroy(LPDIRECT3DVERTEXDECLARATION9 iface);
+
 /* ---------------------- */
 /* IDirect3DVertexShader9 */
 /* ---------------------- */

dlls/d3d9/device.c

@@ -61,8 +61,17 @@ static ULONG WINAPI IDirect3DDevice9Impl_Release(LPDIRECT3DDEVICE9 iface) {
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
+      int i;
       This->inDestruction = TRUE;
-      IDirect3DDevice9_SetVertexDeclaration(iface, NULL);
+
+      for(i = 0; i < This->numConvertedDecls; i++) {
+          /* Unless Wine is buggy or the app has a bug the refcount will be 0, because decls hold a reference to the
+           * device
+           */
+          IDirect3DVertexDeclaration9Impl_Destroy(This->convertedDecls[i]);
+      }
+      HeapFree(GetProcessHeap(), 0, This->convertedDecls);
+
       IWineD3DDevice_Uninit3D(This->WineD3DDevice, D3D9CB_DestroyDepthStencilSurface, D3D9CB_DestroySwapChain);
       IWineD3DDevice_Release(This->WineD3DDevice);
       HeapFree(GetProcessHeap(), 0, This);
@@ -769,29 +778,77 @@ static HRESULT  WINAPI  IDirect3DDevice9Impl_ProcessVertices(LPDIRECT3DDEVICE9 i
     return IWineD3DDevice_ProcessVertices(This->WineD3DDevice,SrcStartIndex, DestIndex, VertexCount, ((IDirect3DVertexBuffer9Impl *)pDestBuffer)->wineD3DVertexBuffer, ((IDirect3DVertexBuffer9Impl *)pVertexDecl)->wineD3DVertexBuffer, Flags);       
 }
 
+IDirect3DVertexDeclaration9 *getConvertedDecl(IDirect3DDevice9Impl *This, DWORD fvf) {
+    HRESULT hr;
+    D3DVERTEXELEMENT9* elements = NULL;
+    IDirect3DVertexDeclaration9* pDecl = NULL;
+    int p, low, high; /* deliberately signed */
+    IDirect3DVertexDeclaration9  **convertedDecls = This->convertedDecls;
+
+    TRACE("Searching for declaration for fvf %08x... ", fvf);
+
+    low = 0;
+    high = This->numConvertedDecls - 1;
+    while(low <= high) {
+        p = (low + high) >> 1;
+        TRACE("%d ", p);
+        if(((IDirect3DVertexDeclaration9Impl *) convertedDecls[p])->convFVF == fvf) {
+            TRACE("found %p\n", convertedDecls[p]);
+            return convertedDecls[p];
+        } else if(((IDirect3DVertexDeclaration9Impl *) convertedDecls[p])->convFVF < fvf) {
+            low = p + 1;
+        } else {
+            high = p - 1;
+        }
+    }
+    TRACE("not found. Creating and inserting at position %d.\n", low);
+
+    hr = vdecl_convert_fvf(fvf, &elements);
+    if (hr != S_OK) return NULL;
+
+    hr = IDirect3DDevice9Impl_CreateVertexDeclaration((IDirect3DDevice9 *) This, elements, &pDecl);
+    if (hr != S_OK) return NULL;
+
+    if(This->declArraySize == This->numConvertedDecls) {
+        int grow = max(This->declArraySize / 2, 8);
+        convertedDecls = HeapReAlloc(GetProcessHeap(), 0, convertedDecls,
+                                     sizeof(convertedDecls[0]) * (This->numConvertedDecls + grow));
+        if(!convertedDecls) {
+            /* This will destroy it */
+            IDirect3DVertexDeclaration9_Release(pDecl);
+            return NULL;
+        }
+        This->convertedDecls = convertedDecls;
+        This->declArraySize += grow;
+    }
+
+    memmove(convertedDecls + low + 1, convertedDecls + low, sizeof(IDirect3DVertexDeclaration9Impl *) * (This->numConvertedDecls - low));
+    convertedDecls[low] = pDecl;
+    This->numConvertedDecls++;
+
+    /* Will prevent the decl from beeing destroyed */
+    ((IDirect3DVertexDeclaration9Impl *) pDecl)->convFVF = fvf;
+    IDirect3DVertexDeclaration9_Release(pDecl); /* Does not destroy now */
+
+    TRACE("Returning %p. %d decls in array\n", pDecl, This->numConvertedDecls);
+    return pDecl;
+}
+
 HRESULT  WINAPI  IDirect3DDevice9Impl_SetFVF(LPDIRECT3DDEVICE9 iface, DWORD FVF) {
     IDirect3DDevice9Impl *This = (IDirect3DDevice9Impl *)iface;
     TRACE("(%p) Relay\n" , This);
 
     if (0 != FVF) {
          HRESULT hr;
-         D3DVERTEXELEMENT9* elements = NULL;
-         IDirect3DVertexDeclaration9* pDecl = NULL;
-
-         hr = vdecl_convert_fvf(FVF, &elements);
-         if (hr != S_OK) goto exit;
+         IDirect3DVertexDeclaration9* pDecl = getConvertedDecl(This, FVF);
 
-         hr = IDirect3DDevice9Impl_CreateVertexDeclaration(iface, elements, &pDecl);
-         if (hr != S_OK) goto exit;
+         if(!pDecl) {
+             /* Any situation when this should happen, except out of memory? */
+             ERR("Failed to create a converted vertex declaration\n");
+             return D3DERR_DRIVERINTERNALERROR;
+         }
 
          hr = IDirect3DDevice9Impl_SetVertexDeclaration(iface, pDecl);
-         if (hr != S_OK) goto exit;
-         This->convertedDecl = pDecl;
-
-         exit:
-         HeapFree(GetProcessHeap(), 0, elements);
-         /* If allocated and set correctly, this will reduce the refcount to 0, but not destroy the declaration */
-         if (pDecl) IUnknown_Release(pDecl);
          if (hr != S_OK) return hr;
     }
 

dlls/d3d9/directx.c

@@ -400,6 +400,11 @@ static HRESULT WINAPI IDirect3D9Impl_CreateDevice(LPDIRECT3D9 iface, UINT Adapte
         *ppReturnedDeviceInterface = NULL;
     }
 
+    /* Initialize the converted declaration array. This creates a valid pointer and when adding decls HeapReAlloc
+     * can be used without further checking
+     */
+    object->convertedDecls = HeapAlloc(GetProcessHeap(), 0, 0);
+
     return hr;
 }
 

dlls/d3d9/tests/vertexdeclaration.c

@@ -652,7 +652,8 @@ static void test_fvf_decl_management(
     IDirect3DVertexDeclaration9* result_decl1 = NULL;
     IDirect3DVertexDeclaration9* result_decl2 = NULL;
     IDirect3DVertexDeclaration9* result_decl3 = NULL;
-    int ref1, ref2, ref3;
+    IDirect3DVertexDeclaration9* result_decl4 = NULL;
+    int ref1, ref2, ref3, ref4;
 
     DWORD test_fvf1 = D3DFVF_XYZRHW;
     DWORD test_fvf2 = D3DFVF_NORMAL;
@@ -703,19 +704,35 @@ static void test_fvf_decl_management(
     /* Re-Check if the first decl was overwritten by the new Get() */
     VDECL_CHECK(compare_elements(result_decl1, test_elements1));
 
-    /* The refcounts should all be 1 */
+    hr = IDirect3DDevice9_SetFVF( device, test_fvf1);
+    ok(SUCCEEDED(hr), "SetFVF returned %#x, expected %#x\n", hr, D3D_OK);
+    if (FAILED(hr)) return;
+
+    hr = IDirect3DDevice9_GetVertexDeclaration ( device, &result_decl4);
+    ok(SUCCEEDED(hr), "GetVertexDeclaration returned %#x, expected %#x\n", hr, D3D_OK);
+    if (FAILED(hr)) return;
+
+    ok(result_decl4 == result_decl1, "Setting an already used FVF over results in a different vertexdeclaration\n");
+
     ref1 = get_refcount((IUnknown*) result_decl1);
     ref2 = get_refcount((IUnknown*) result_decl2);
     ref3 = get_refcount((IUnknown*) result_decl3);
-    ok (ref1 == 2, "Refcount #1 is %d, expected 2\n", ref1);
-    ok (ref2 == 2, "Refcount #2 is %d, expected 2\n", ref2);
+    ref4 = get_refcount((IUnknown*) result_decl4);
+    ok (ref1 == 3, "Refcount #1 is %d, expected 3\n", ref1);
+    ok (ref2 == 3, "Refcount #2 is %d, expected 3\n", ref2);
     ok (ref3 == 1, "Refcount #3 is %d, expected 1\n", ref3);
+    ok (ref4 == 3, "Refcount #4 is %d, expected 3\n", ref4);
 
     /* Clear down any current vertex declaration */
     hr = IDirect3DDevice9_SetVertexDeclaration ( device, NULL );
     ok (SUCCEEDED(hr), "SetVertexDeclaration returned %#x, expected %#x\n", hr, D3D_OK);
     if (FAILED(hr)) return;
 
+    IDirect3DVertexDeclaration9_Release(result_decl1);
+    IDirect3DVertexDeclaration9_Release(result_decl2);
+    IDirect3DVertexDeclaration9_Release(result_decl3);
+    IDirect3DVertexDeclaration9_Release(result_decl4);
+
     return;
 }
 

dlls/d3d9/vertexdeclaration.c

@@ -209,6 +209,18 @@ static ULONG WINAPI IDirect3DVertexDeclaration9Impl_AddRef(LPDIRECT3DVERTEXDECLA
     return ref;
 }
 
+void IDirect3DVertexDeclaration9Impl_Destroy(LPDIRECT3DVERTEXDECLARATION9 iface) {
+    IDirect3DVertexDeclaration9Impl *This = (IDirect3DVertexDeclaration9Impl *)iface;
+
+    if(This->ref != 0) {
+        /* Should not happen unless wine has a bug or the application releases references it does not own */
+        ERR("Destroying vdecl with ref != 0\n");
+    }
+    IWineD3DVertexDeclaration_Release(This->wineD3DVertexDeclaration);
+    HeapFree(GetProcessHeap(), 0, This->elements);
+    HeapFree(GetProcessHeap(), 0, This);
+}
+
 static ULONG WINAPI IDirect3DVertexDeclaration9Impl_Release(LPDIRECT3DVERTEXDECLARATION9 iface) {
     IDirect3DVertexDeclaration9Impl *This = (IDirect3DVertexDeclaration9Impl *)iface;
     ULONG ref = InterlockedDecrement(&This->ref);
@@ -217,12 +229,9 @@ static ULONG WINAPI IDirect3DVertexDeclaration9Impl_Release(LPDIRECT3DVERTEXDECL
 
     if (ref == 0) {
         IDirect3DDevice9 *parentDevice = This->parentDevice;
-        BOOL converted = ((IDirect3DDevice9Impl *) parentDevice)->convertedDecl == iface;
 
-        if(!converted) {
-            IWineD3DVertexDeclaration_Release(This->wineD3DVertexDeclaration);
-            HeapFree(GetProcessHeap(), 0, This->elements);
-            HeapFree(GetProcessHeap(), 0, This);
+        if(!This->convFVF) {
+            IDirect3DVertexDeclaration9Impl_Release(iface);
         }
         IUnknown_Release(parentDevice);
     }
@@ -371,17 +380,6 @@ HRESULT  WINAPI  IDirect3DDevice9Impl_SetVertexDeclaration(LPDIRECT3DDEVICE9 ifa
 
     TRACE("(%p) : Relay\n", iface);
 
-    if (This->convertedDecl && This->convertedDecl != pDecl) {
-        IDirect3DVertexDeclaration9Impl *iDecl = (IDirect3DVertexDeclaration9Impl *) This->convertedDecl;
-
-        /* Will need locking once we claim to be thread safe */
-        if(iDecl->ref == 0) {
-            IUnknown_AddRef(This->convertedDecl);
-            IUnknown_Release(This->convertedDecl);
-        }
-        This->convertedDecl = NULL;
-    }
-
     hr = IWineD3DDevice_SetVertexDeclaration(This->WineD3DDevice, pDeclImpl == NULL ? NULL : pDeclImpl->wineD3DVertexDeclaration);
     return hr;
 }