wined3d: Destroying the resource frees all private data.

Also adds a test for ddraw, d3d8 and d3d9.

dlls/d3d8/tests/surface.c

@@ -195,6 +195,70 @@ static void test_lockrect_invalid(IDirect3DDevice8 *device)
     IDirect3DSurface8_Release(surface);
 }
 
+static unsigned long getref(IUnknown *iface)
+{
+    IUnknown_AddRef(iface);
+    return IUnknown_Release(iface);
+}
+
+static void test_private_data(IDirect3DDevice8 *device)
+{
+    HRESULT hr;
+    IDirect3DSurface8 *surface;
+    ULONG ref, ref2;
+    IUnknown *ptr;
+    DWORD size = sizeof(IUnknown *);
+
+    hr = IDirect3DDevice8_CreateImageSurface(device, 4, 4, D3DFMT_A8R8G8B8, &surface);
+    ok(SUCCEEDED(hr), "CreateImageSurface failed (0x%08x)\n", hr);
+    if(!surface)
+    {
+        return;
+    }
+
+    /* This fails */
+    hr = IDirect3DSurface8_SetPrivateData(surface, &IID_IDirect3DSurface8 /* Abuse this tag */, device, 0, D3DSPD_IUNKNOWN);
+    ok(hr == D3DERR_INVALIDCALL, "IDirect3DSurface8_SetPrivateData failed with %08x\n", hr);
+    hr = IDirect3DSurface8_SetPrivateData(surface, &IID_IDirect3DSurface8 /* Abuse this tag */, device, 5, D3DSPD_IUNKNOWN);
+    ok(hr == D3DERR_INVALIDCALL, "IDirect3DSurface8_SetPrivateData failed with %08x\n", hr);
+    hr = IDirect3DSurface8_SetPrivateData(surface, &IID_IDirect3DSurface8 /* Abuse this tag */, device, sizeof(IUnknown *) * 2, D3DSPD_IUNKNOWN);
+    ok(hr == D3DERR_INVALIDCALL, "IDirect3DSurface8_SetPrivateData failed with %08x\n", hr);
+
+    ref = getref((IUnknown *) device);
+    hr = IDirect3DSurface8_SetPrivateData(surface, &IID_IDirect3DSurface8 /* Abuse this tag */, device, sizeof(IUnknown *), D3DSPD_IUNKNOWN);
+    ok(hr == D3D_OK, "IDirect3DSurface8_SetPrivateData failed with %08x\n", hr);
+    ref2 = getref((IUnknown *) device);
+    ok(ref2 == ref + 1, "Object reference is %d, expected %d\n", ref2, ref + 1);
+    hr = IDirect3DSurface8_FreePrivateData(surface, &IID_IDirect3DSurface8);
+    ok(hr == D3D_OK, "IDirect3DSurface8_FreePrivateData returned %08x\n", hr);
+    ref2 = getref((IUnknown *) device);
+    ok(ref2 == ref, "Object reference is %d, expected %d\n", ref2, ref);
+
+    hr = IDirect3DSurface8_SetPrivateData(surface, &IID_IDirect3DSurface8, device, sizeof(IUnknown *), D3DSPD_IUNKNOWN);
+    ok(hr == D3D_OK, "IDirect3DSurface8_SetPrivateData failed with %08x\n", hr);
+    hr = IDirect3DSurface8_SetPrivateData(surface, &IID_IDirect3DSurface8, surface, sizeof(IUnknown *), D3DSPD_IUNKNOWN);
+    ok(hr == D3D_OK, "IDirect3DSurface8_SetPrivateData failed with %08x\n", hr);
+    ref2 = getref((IUnknown *) device);
+    ok(ref2 == ref, "Object reference is %d, expected %d\n", ref2, ref);
+
+    hr = IDirect3DSurface8_SetPrivateData(surface, &IID_IDirect3DSurface8, device, sizeof(IUnknown *), D3DSPD_IUNKNOWN);
+    ok(hr == D3D_OK, "IDirect3DSurface8_SetPrivateData failed with %08x\n", hr);
+    hr = IDirect3DSurface8_GetPrivateData(surface, &IID_IDirect3DSurface8, &ptr, &size);
+    ok(hr == D3D_OK, "IDirect3DSurface8_GetPrivateData failed with %08x\n", hr);
+    ref2 = getref((IUnknown *) device);
+    /* Object is NOT beein addrefed */
+    ok(ptr == (IUnknown *) device, "Returned interface pointer is %p, expected %p\n", ptr, device);
+    ok(ref2 == ref + 2, "Object reference is %d, expected %d. ptr at %p, orig at %p\n", ref2, ref + 2, ptr, device);
+    IUnknown_Release(ptr);
+
+    IDirect3DSurface8_Release(surface);
+
+    /* Destroying the surface frees the held reference */
+    ref2 = getref((IUnknown *) device);
+    /* -1 because the surface was released and held a reference before */
+    ok(ref2 == (ref - 1), "Object reference is %d, expected %d\n", ref2, ref - 1);
+}
+
 START_TEST(surface)
 {
     HMODULE d3d8_handle;
@@ -212,4 +276,5 @@ START_TEST(surface)
 
     test_surface_get_container(device_ptr);
     test_lockrect_invalid(device_ptr);
+    test_private_data(device_ptr);
 }

dlls/d3d9/tests/surface.c

@@ -286,6 +286,70 @@ static void test_lockrect_invalid(IDirect3DDevice9 *device)
     IDirect3DSurface9_Release(surface);
 }
 
+static unsigned long getref(IUnknown *iface)
+{
+    IUnknown_AddRef(iface);
+    return IUnknown_Release(iface);
+}
+
+static void test_private_data(IDirect3DDevice9 *device)
+{
+    HRESULT hr;
+    IDirect3DSurface9 *surface;
+    ULONG ref, ref2;
+    IUnknown *ptr;
+    DWORD size = sizeof(IUnknown *);
+
+    hr = IDirect3DDevice9_CreateOffscreenPlainSurface(device, 4, 4, D3DFMT_A8R8G8B8, D3DPOOL_SCRATCH, &surface, 0);
+    ok(SUCCEEDED(hr), "CreateImageSurface failed (0x%08x)\n", hr);
+    if(!surface)
+    {
+        return;
+    }
+
+    /* This fails */
+    hr = IDirect3DSurface9_SetPrivateData(surface, &IID_IDirect3DSurface9 /* Abuse this tag */, device, 0, D3DSPD_IUNKNOWN);
+    ok(hr == D3DERR_INVALIDCALL, "IDirect3DSurface9_SetPrivateData failed with %08x\n", hr);
+    hr = IDirect3DSurface9_SetPrivateData(surface, &IID_IDirect3DSurface9 /* Abuse this tag */, device, 5, D3DSPD_IUNKNOWN);
+    ok(hr == D3DERR_INVALIDCALL, "IDirect3DSurface9_SetPrivateData failed with %08x\n", hr);
+    hr = IDirect3DSurface9_SetPrivateData(surface, &IID_IDirect3DSurface9 /* Abuse this tag */, device, sizeof(IUnknown *) * 2, D3DSPD_IUNKNOWN);
+    ok(hr == D3DERR_INVALIDCALL, "IDirect3DSurface9_SetPrivateData failed with %08x\n", hr);
+
+    ref = getref((IUnknown *) device);
+    hr = IDirect3DSurface9_SetPrivateData(surface, &IID_IDirect3DSurface9 /* Abuse this tag */, device, sizeof(IUnknown *), D3DSPD_IUNKNOWN);
+    ok(hr == D3D_OK, "IDirect3DSurface9_SetPrivateData failed with %08x\n", hr);
+    ref2 = getref((IUnknown *) device);
+    ok(ref2 == ref + 1, "Object reference is %d, expected %d\n", ref2, ref + 1);
+    hr = IDirect3DSurface9_FreePrivateData(surface, &IID_IDirect3DSurface9);
+    ok(hr == D3D_OK, "IDirect3DSurface9_FreePrivateData returned %08x\n", hr);
+    ref2 = getref((IUnknown *) device);
+    ok(ref2 == ref, "Object reference is %d, expected %d\n", ref2, ref);
+
+    hr = IDirect3DSurface9_SetPrivateData(surface, &IID_IDirect3DSurface9, device, sizeof(IUnknown *), D3DSPD_IUNKNOWN);
+    ok(hr == D3D_OK, "IDirect3DSurface9_SetPrivateData failed with %08x\n", hr);
+    hr = IDirect3DSurface9_SetPrivateData(surface, &IID_IDirect3DSurface9, surface, sizeof(IUnknown *), D3DSPD_IUNKNOWN);
+    ok(hr == D3D_OK, "IDirect3DSurface9_SetPrivateData failed with %08x\n", hr);
+    ref2 = getref((IUnknown *) device);
+    ok(ref2 == ref, "Object reference is %d, expected %d\n", ref2, ref);
+
+    hr = IDirect3DSurface9_SetPrivateData(surface, &IID_IDirect3DSurface9, device, sizeof(IUnknown *), D3DSPD_IUNKNOWN);
+    ok(hr == D3D_OK, "IDirect3DSurface9_SetPrivateData failed with %08x\n", hr);
+    hr = IDirect3DSurface9_GetPrivateData(surface, &IID_IDirect3DSurface9, &ptr, &size);
+    ok(hr == D3D_OK, "IDirect3DSurface9_GetPrivateData failed with %08x\n", hr);
+    ref2 = getref((IUnknown *) device);
+    /* Object is NOT beein addrefed */
+    ok(ptr == (IUnknown *) device, "Returned interface pointer is %p, expected %p\n", ptr, device);
+    ok(ref2 == ref + 2, "Object reference is %d, expected %d. ptr at %p, orig at %p\n", ref2, ref + 2, ptr, device);
+    IUnknown_Release(ptr);
+
+    IDirect3DSurface9_Release(surface);
+
+    /* Destroying the surface frees the held reference */
+    ref2 = getref((IUnknown *) device);
+    /* -1 because the surface was released and held a reference before */
+    ok(ref2 == (ref - 1), "Object reference is %d, expected %d\n", ref2, (ref - 1));
+}
+
 START_TEST(surface)
 {
     HMODULE d3d9_handle;
@@ -305,4 +369,5 @@ START_TEST(surface)
     test_surface_alignment(device_ptr);
     test_lockrect_offset(device_ptr);
     test_lockrect_invalid(device_ptr);
+    test_private_data(device_ptr);
 }

dlls/ddraw/surface.c

@@ -1105,13 +1105,19 @@ IDirectDrawSurfaceImpl_SetPrivateData(IDirectDrawSurface7 *iface,
                                       DWORD Flags)
 {
     ICOM_THIS_FROM(IDirectDrawSurfaceImpl, IDirectDrawSurface7, iface);
+    HRESULT hr;
     TRACE("(%p)->(%s,%p,%d,%x): Relay\n", This, debugstr_guid(tag), Data, Size, Flags);
 
-    return IWineD3DSurface_SetPrivateData(This->WineD3DSurface,
+    hr = IWineD3DSurface_SetPrivateData(This->WineD3DSurface,
-                                          tag,
+                                        tag,
-                                          Data,
+                                        Data,
-                                          Size,
+                                        Size,
-                                          Flags);
+                                        Flags);
+    switch(hr)
+    {
+        case WINED3DERR_INVALIDCALL:        return DDERR_INVALIDPARAMS;
+        default:                            return hr;
+    }
 }
 
 /*****************************************************************************

dlls/ddraw/tests/dsurface.c

@@ -2083,6 +2083,77 @@ static void SizeTest(void)
     ok(ret == DD_OK, "SetCooperativeLevel failed with %08x\n", ret);
 }
 
+static void PrivateDataTest(void)
+{
+    HRESULT hr;
+    IDirectDrawSurface7 *surface7;
+    IDirectDrawSurface *surface;
+    DDSURFACEDESC desc;
+    ULONG ref, ref2;
+    IUnknown *ptr;
+    DWORD size = sizeof(IUnknown *);
+
+    ZeroMemory(&desc, sizeof(desc));
+    desc.dwSize = sizeof(desc);
+    desc.dwFlags = DDSD_CAPS | DDSD_HEIGHT | DDSD_WIDTH;
+    desc.ddsCaps.dwCaps |= DDSCAPS_OFFSCREENPLAIN;
+    desc.dwHeight = 128;
+    desc.dwWidth = 128;
+    hr = IDirectDraw_CreateSurface(lpDD, &desc, &surface, NULL);
+    ok(hr == DD_OK, "Creating an offscreen plain surface failed with %08x\n", hr);
+    if(!surface)
+    {
+        return;
+    }
+    hr = IDirectDrawSurface_QueryInterface(surface, &IID_IDirectDrawSurface7, (void **) &surface7);
+    ok(hr == DD_OK, "IDirectDrawSurface_QueryInterface failed with %08x\n", hr);
+    if(!surface)
+    {
+        IDirectDrawSurface_Release(surface);
+        return;
+    }
+
+    /* This fails */
+    hr = IDirectDrawSurface7_SetPrivateData(surface7, &IID_IDirectDrawSurface7 /* Abuse this tag */, lpDD, 0, DDSPD_IUNKNOWNPTR);
+    ok(hr == DDERR_INVALIDPARAMS, "IDirectDrawSurface7_SetPrivateData failed with %08x\n", hr);
+    hr = IDirectDrawSurface7_SetPrivateData(surface7, &IID_IDirectDrawSurface7 /* Abuse this tag */, lpDD, 5, DDSPD_IUNKNOWNPTR);
+    ok(hr == DDERR_INVALIDPARAMS, "IDirectDrawSurface7_SetPrivateData failed with %08x\n", hr);
+    hr = IDirectDrawSurface7_SetPrivateData(surface7, &IID_IDirectDrawSurface7 /* Abuse this tag */, lpDD, sizeof(IUnknown *) * 2, DDSPD_IUNKNOWNPTR);
+    ok(hr == DDERR_INVALIDPARAMS, "IDirectDrawSurface7_SetPrivateData failed with %08x\n", hr);
+
+    ref = getref((IUnknown *) lpDD);
+    hr = IDirectDrawSurface7_SetPrivateData(surface7, &IID_IDirectDrawSurface7 /* Abuse this tag */, lpDD, sizeof(IUnknown *), DDSPD_IUNKNOWNPTR);
+    ok(hr == DD_OK, "IDirectDrawSurface7_SetPrivateData failed with %08x\n", hr);
+    ref2 = getref((IUnknown *) lpDD);
+    ok(ref2 == ref + 1, "Object reference is %d, expected %d\n", ref2, ref + 1);
+    hr = IDirectDrawSurface7_FreePrivateData(surface7, &IID_IDirectDrawSurface7);
+    ref2 = getref((IUnknown *) lpDD);
+    ok(ref2 == ref, "Object reference is %d, expected %d\n", ref2, ref);
+
+    hr = IDirectDrawSurface7_SetPrivateData(surface7, &IID_IDirectDrawSurface7, lpDD, sizeof(IUnknown *), DDSPD_IUNKNOWNPTR);
+    ok(hr == DD_OK, "IDirectDrawSurface7_SetPrivateData failed with %08x\n", hr);
+    hr = IDirectDrawSurface7_SetPrivateData(surface7, &IID_IDirectDrawSurface7, surface7, sizeof(IUnknown *), DDSPD_IUNKNOWNPTR);
+    ok(hr == DD_OK, "IDirectDrawSurface7_SetPrivateData failed with %08x\n", hr);
+    ref2 = getref((IUnknown *) lpDD);
+    ok(ref2 == ref, "Object reference is %d, expected %d\n", ref2, ref);
+
+    hr = IDirectDrawSurface7_SetPrivateData(surface7, &IID_IDirectDrawSurface7, lpDD, sizeof(IUnknown *), DDSPD_IUNKNOWNPTR);
+    ok(hr == DD_OK, "IDirectDrawSurface7_SetPrivateData failed with %08x\n", hr);
+    hr = IDirectDrawSurface7_GetPrivateData(surface7, &IID_IDirectDrawSurface7, &ptr, &size);
+    ok(hr == DD_OK, "IDirectDrawSurface7_GetPrivateData failed with %08x\n", hr);
+    ref2 = getref((IUnknown *) lpDD);
+    /* Object is NOT beein addrefed */
+    ok(ptr == (IUnknown *) lpDD, "Returned interface pointer is %p, expected %p\n", ptr, lpDD);
+    ok(ref2 == ref + 1, "Object reference is %d, expected %d. ptr at %p, orig at %p\n", ref2, ref + 1, ptr, lpDD);
+
+    IDirectDrawSurface_Release(surface);
+    IDirectDrawSurface7_Release(surface7);
+
+    /* Destroying the surface frees the held reference */
+    ref2 = getref((IUnknown *) lpDD);
+    ok(ref2 == ref, "Object reference is %d, expected %d\n", ref2, ref);
+}
+
 START_TEST(dsurface)
 {
     if (!CreateDirectDraw())
@@ -2101,5 +2172,6 @@ START_TEST(dsurface)
     test_lockrect_invalid();
     CompressedTest();
     SizeTest();
+    PrivateDataTest();
     ReleaseDirectDraw();
 }

dlls/wined3d/resource.c

@@ -64,12 +64,24 @@ ULONG WINAPI IWineD3DResourceImpl_Release(IWineD3DResource *iface) {
 /* class static (not in vtable) */
 void IWineD3DResourceImpl_CleanUp(IWineD3DResource *iface){
     IWineD3DResourceImpl *This = (IWineD3DResourceImpl *)iface;
+    struct list *e1, *e2;
+    PrivateData *data;
+    HRESULT hr;
+
     TRACE("(%p) Cleaning up resource\n", This);
     if (This->resource.pool == WINED3DPOOL_DEFAULT) {
         TRACE("Decrementing device memory pool by %u\n", This->resource.size);
         globalChangeGlRam(-This->resource.size);
     }
 
+    LIST_FOR_EACH_SAFE(e1, e2, &This->resource.privateData) {
+        data = LIST_ENTRY(e1, PrivateData, entry);
+        hr = IWineD3DResourceImpl_FreePrivateData(iface, &data->tag);
+        if(hr != WINED3D_OK) {
+            ERR("Failed to free private data when destroying resource %p, hr = %08x\n", This, hr);
+        }
+    }
+
     HeapFree(GetProcessHeap(), 0, This->resource.allocatedMemory);
     This->resource.allocatedMemory = 0;
 
@@ -112,45 +124,36 @@ HRESULT WINAPI IWineD3DResourceImpl_SetPrivateData(IWineD3DResource *iface, REFG
     PrivateData *data;
 
     TRACE("(%p) : %s %p %d %d\n", This, debugstr_guid(refguid), pData, SizeOfData, Flags);
-    data = IWineD3DResourceImpl_FindPrivateData(This, refguid);
+    IWineD3DResourceImpl_FreePrivateData(iface, refguid);
-    if (data == NULL)
-    {
-        data = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*data));
-        if (NULL == data) return E_OUTOFMEMORY;
 
-        data->tag = *refguid;
+    data = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*data));
-        data->flags = Flags;
+    if (NULL == data) return E_OUTOFMEMORY;
+
+    data->tag = *refguid;
+    data->flags = Flags;
 #if 0
         (*data)->uniquenessValue = This->uniquenessValue;
 #endif
-        if (Flags & WINED3DSPD_IUNKNOWN) {
+    if (Flags & WINED3DSPD_IUNKNOWN) {
-            data->ptr.object = (LPUNKNOWN)pData;
+        if(SizeOfData != sizeof(IUnknown *)) {
-            data->size = sizeof(LPUNKNOWN);
+            WARN("IUnknown data with size %d, returning WINED3DERR_INVALIDCALL\n", SizeOfData);
-            IUnknown_AddRef(data->ptr.object);
+            return WINED3DERR_INVALIDCALL;
         }
-        else
+        data->ptr.object = (LPUNKNOWN)pData;
-        {
+        data->size = sizeof(LPUNKNOWN);
-            data->ptr.data = HeapAlloc(GetProcessHeap(), 0, SizeOfData);
+        IUnknown_AddRef(data->ptr.object);
-            if (NULL == data->ptr.data) {
-                HeapFree(GetProcessHeap(), 0, data);
-                return E_OUTOFMEMORY;
-            }
-            data->size = SizeOfData;
-            memcpy(data->ptr.data, pData, SizeOfData);
-        }
-        list_add_tail(&This->resource.privateData, &data->entry);
-        return WINED3D_OK;
-
-    } else {
-        /* I don't actually know how windows handles this case. The only
-         * reason I don't just call FreePrivateData is because I want to
-         * guarantee SetPrivateData working when using LPUNKNOWN or data
-         * that is no larger than the old data.
-         */
-        FIXME("Handle overwriting private data in SetPrivateData\n");
-        return E_FAIL;
-
     }
+    else
+    {
+        data->ptr.data = HeapAlloc(GetProcessHeap(), 0, SizeOfData);
+        if (NULL == data->ptr.data) {
+            HeapFree(GetProcessHeap(), 0, data);
+            return E_OUTOFMEMORY;
+        }
+        data->size = SizeOfData;
+        memcpy(data->ptr.data, pData, SizeOfData);
+    }
+    list_add_tail(&This->resource.privateData, &data->entry);
 
     return WINED3D_OK;
 }
@@ -176,7 +179,13 @@ HRESULT WINAPI IWineD3DResourceImpl_GetPrivateData(IWineD3DResource *iface, REFG
 
     if (data->flags & WINED3DSPD_IUNKNOWN) {
         *(LPUNKNOWN *)pData = data->ptr.object;
-        IUnknown_AddRef(data->ptr.object);
+        if(((IWineD3DImpl *) This->resource.wineD3DDevice->wineD3D)->dxVersion != 7) {
+            /* D3D8 and D3D9 addref the private data, DDraw does not. This can't be handled in
+             * ddraw because it doesn't know if the pointer returned is an IUnknown * or just a
+             * Blob
+             */
+            IUnknown_AddRef(data->ptr.object);
+        }
     }
     else {
         memcpy(pData, data->ptr.data, data->size);

include/d3d8.h

@@ -45,6 +45,11 @@
 #define D3DCREATE_HARDWARE_VERTEXPROCESSING     0x00000040L
 #define D3DCREATE_MIXED_VERTEXPROCESSING        0x00000080L
 
+/*****************************************************************************
+ * Flags for SetPrivateData
+ */
+#define D3DSPD_IUNKNOWN                         0x00000001L
+
 /*****************************************************************************
  * #defines and error codes
  */