ddraw: Keep track of interface attached by AddAttachedSurface and detach correct interface when parent is released.

dlls/ddraw/ddraw_private.h

@@ -165,6 +165,7 @@ struct IDirectDrawSurfaceImpl
     /* This implementation handles attaching surfaces to other surfaces */
     IDirectDrawSurfaceImpl  *next_attached;
     IDirectDrawSurfaceImpl  *first_attached;
+    IUnknown                *attached_iface;
 
     /* Complex surfaces are organized in a tree, although the tree is degenerated to a list in most cases.
      * In mipmap and primary surfaces each level has only one attachment, which is the next surface level.

dlls/ddraw/surface.c

@@ -1350,7 +1350,7 @@ static HRESULT WINAPI ddraw_surface1_Blt(IDirectDrawSurface *iface, RECT *dst_re
  *
  * Attaches a surface to another surface. How the surface attachments work
  * is not totally understood yet, and this method is prone to problems.
- * he surface that is attached is AddRef-ed.
+ * The surface that is attached is AddRef-ed.
  *
  * Tests with complex surfaces suggest that the surface attachments form a
  * tree, but no method to test this has been found yet.
@@ -1449,6 +1449,7 @@ static HRESULT WINAPI ddraw_surface7_AddAttachedSurface(IDirectDrawSurface7 *ifa
         return hr;
     }
     ddraw_surface7_AddRef(attachment);
+    attachment_impl->attached_iface = (IUnknown *)attachment;
     return hr;
 }
 
@@ -1468,6 +1469,7 @@ static HRESULT WINAPI ddraw_surface4_AddAttachedSurface(IDirectDrawSurface4 *ifa
     }
     ddraw_surface4_AddRef(attachment);
     ddraw_surface7_Release(&attachment_impl->IDirectDrawSurface7_iface);
+    attachment_impl->attached_iface = (IUnknown *)attachment;
     return hr;
 }
 static HRESULT WINAPI ddraw_surface3_AddAttachedSurface(IDirectDrawSurface3 *iface, IDirectDrawSurface3 *attachment)
@@ -1512,6 +1514,7 @@ static HRESULT WINAPI ddraw_surface3_AddAttachedSurface(IDirectDrawSurface3 *ifa
         return hr;
     }
     ddraw_surface3_AddRef(attachment);
+    attachment_impl->attached_iface = (IUnknown *)attachment;
     return hr;
 }
 
@@ -1531,6 +1534,7 @@ static HRESULT WINAPI ddraw_surface2_AddAttachedSurface(IDirectDrawSurface2 *ifa
     }
     ddraw_surface2_AddRef(attachment);
     ddraw_surface3_Release(&attachment_impl->IDirectDrawSurface3_iface);
+    attachment_impl->attached_iface = (IUnknown *)attachment;
     return hr;
 }
 
@@ -1550,6 +1554,7 @@ static HRESULT WINAPI ddraw_surface1_AddAttachedSurface(IDirectDrawSurface *ifac
     }
     ddraw_surface1_AddRef(attachment);
     ddraw_surface3_Release(&attachment_impl->IDirectDrawSurface3_iface);
+    attachment_impl->attached_iface = (IUnknown *)attachment;
     return hr;
 }
 
@@ -1569,11 +1574,11 @@ static HRESULT WINAPI ddraw_surface1_AddAttachedSurface(IDirectDrawSurface *ifac
  *
  *****************************************************************************/
 static HRESULT ddraw_surface_delete_attached_surface(IDirectDrawSurfaceImpl *This,
-        IDirectDrawSurfaceImpl *Surf)
+        IDirectDrawSurfaceImpl *Surf, IUnknown *detach_iface)
 {
     IDirectDrawSurfaceImpl *Prev = This;
 
-    TRACE("surface %p, attachment %p.\n", This, Surf);
+    TRACE("surface %p, attachment %p, detach_iface %p.\n", This, Surf, detach_iface);
 
     EnterCriticalSection(&ddraw_cs);
     if (!Surf || (Surf->first_attached != This) || (Surf == This) )
@@ -1582,6 +1587,13 @@ static HRESULT ddraw_surface_delete_attached_surface(IDirectDrawSurfaceImpl *Thi
         return DDERR_CANNOTDETACHSURFACE;
     }
 
+    if (Surf->attached_iface != detach_iface)
+    {
+        WARN("Surf->attach_iface %p != detach_iface %p.\n", Surf->attached_iface, detach_iface);
+        LeaveCriticalSection(&ddraw_cs);
+        return DDERR_SURFACENOTATTACHED;
+    }
+
     /* Remove MIPMAPSUBLEVEL if this seemed to be one */
     if (This->surface_desc.ddsCaps.dwCaps &
         Surf->surface_desc.ddsCaps.dwCaps & DDSCAPS_MIPMAP)
@@ -1612,6 +1624,8 @@ static HRESULT ddraw_surface_delete_attached_surface(IDirectDrawSurfaceImpl *Thi
         IDirect3DDeviceImpl_UpdateDepthStencil(This->ddraw->d3ddevice);
     }
     LeaveCriticalSection(&ddraw_cs);
+    IUnknown_Release(Surf->attached_iface);
+    Surf->attached_iface = NULL;
     return DD_OK;
 }
 
@@ -1620,17 +1634,10 @@ static HRESULT WINAPI ddraw_surface7_DeleteAttachedSurface(IDirectDrawSurface7 *
 {
     IDirectDrawSurfaceImpl *This = impl_from_IDirectDrawSurface7(iface);
     IDirectDrawSurfaceImpl *attachment_impl = unsafe_impl_from_IDirectDrawSurface7(attachment);
-    HRESULT hr;
 
     TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
 
-    hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
+    return ddraw_surface_delete_attached_surface(This, attachment_impl, (IUnknown *)attachment);
-    if (FAILED(hr))
-    {
-        return hr;
-    }
-    ddraw_surface7_Release(attachment);
-    return hr;
 }
 
 static HRESULT WINAPI ddraw_surface4_DeleteAttachedSurface(IDirectDrawSurface4 *iface,
@@ -1638,17 +1645,10 @@ static HRESULT WINAPI ddraw_surface4_DeleteAttachedSurface(IDirectDrawSurface4 *
 {
     IDirectDrawSurfaceImpl *This = impl_from_IDirectDrawSurface4(iface);
     IDirectDrawSurfaceImpl *attachment_impl = unsafe_impl_from_IDirectDrawSurface4(attachment);
-    HRESULT hr;
 
     TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
 
-    hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
+    return ddraw_surface_delete_attached_surface(This, attachment_impl, (IUnknown *)attachment);
-    if (FAILED(hr))
-    {
-        return hr;
-    }
-    ddraw_surface4_Release(attachment);
-    return hr;
 }
 
 static HRESULT WINAPI ddraw_surface3_DeleteAttachedSurface(IDirectDrawSurface3 *iface,
@@ -1656,16 +1656,10 @@ static HRESULT WINAPI ddraw_surface3_DeleteAttachedSurface(IDirectDrawSurface3 *
 {
     IDirectDrawSurfaceImpl *This = impl_from_IDirectDrawSurface3(iface);
     IDirectDrawSurfaceImpl *attachment_impl = unsafe_impl_from_IDirectDrawSurface3(attachment);
-    HRESULT hr;
+
     TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
 
-    hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
+    return ddraw_surface_delete_attached_surface(This, attachment_impl, (IUnknown *)attachment);
-    if (FAILED(hr))
-    {
-        return hr;
-    }
-    ddraw_surface3_Release(attachment);
-    return hr;
 }
 
 static HRESULT WINAPI ddraw_surface2_DeleteAttachedSurface(IDirectDrawSurface2 *iface,
@@ -1673,16 +1667,10 @@ static HRESULT WINAPI ddraw_surface2_DeleteAttachedSurface(IDirectDrawSurface2 *
 {
     IDirectDrawSurfaceImpl *This = impl_from_IDirectDrawSurface2(iface);
     IDirectDrawSurfaceImpl *attachment_impl = unsafe_impl_from_IDirectDrawSurface2(attachment);
-    HRESULT hr;
+
     TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
 
-    hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
+    return ddraw_surface_delete_attached_surface(This, attachment_impl, (IUnknown *)attachment);
-    if (FAILED(hr))
-    {
-        return hr;
-    }
-    ddraw_surface2_Release(attachment);
-    return hr;
 }
 
 static HRESULT WINAPI ddraw_surface1_DeleteAttachedSurface(IDirectDrawSurface *iface,
@@ -1690,16 +1678,10 @@ static HRESULT WINAPI ddraw_surface1_DeleteAttachedSurface(IDirectDrawSurface *i
 {
     IDirectDrawSurfaceImpl *This = impl_from_IDirectDrawSurface(iface);
     IDirectDrawSurfaceImpl *attachment_impl = unsafe_impl_from_IDirectDrawSurface(attachment);
-    HRESULT hr;
+
     TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
 
-    hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
+    return ddraw_surface_delete_attached_surface(This, attachment_impl, (IUnknown *)attachment);
-    if (FAILED(hr))
-    {
-        return hr;
-    }
-    ddraw_surface1_Release(attachment);
-    return hr;
 }
 
 /*****************************************************************************
@@ -5100,9 +5082,6 @@ static void STDMETHODCALLTYPE ddraw_surface_wined3d_object_destroyed(void *paren
     /* Check for attached surfaces and detach them. */
     if (surface->first_attached != surface)
     {
-        IDirectDrawSurface7 *root = &surface->first_attached->IDirectDrawSurface7_iface;
-        IDirectDrawSurface7 *detach = &surface->IDirectDrawSurface7_iface;
-
         /* Well, this shouldn't happen: The surface being attached is
          * referenced in AddAttachedSurface(), so it shouldn't be released
          * until DeleteAttachedSurface() is called, because the refcount is
@@ -5111,18 +5090,14 @@ static void STDMETHODCALLTYPE ddraw_surface_wined3d_object_destroyed(void *paren
         WARN("Surface is still attached to surface %p.\n", surface->first_attached);
 
         /* The refcount will drop to -1 here */
-        if (FAILED(IDirectDrawSurface7_DeleteAttachedSurface(root, 0, detach)))
+        if (FAILED(ddraw_surface_delete_attached_surface(surface->first_attached, surface, surface->attached_iface)))
             ERR("DeleteAttachedSurface failed.\n");
     }
 
     while (surface->next_attached)
-    {
+        if (FAILED(ddraw_surface_delete_attached_surface(surface,
-        IDirectDrawSurface7 *root = &surface->IDirectDrawSurface7_iface;
+                surface->next_attached, surface->next_attached->attached_iface)))
-        IDirectDrawSurface7 *detach = &surface->next_attached->IDirectDrawSurface7_iface;
-
-        if (FAILED(IDirectDrawSurface7_DeleteAttachedSurface(root, 0, detach)))
             ERR("DeleteAttachedSurface failed.\n");
-    }
 
     /* Having a texture handle set implies that the device still exists. */
     if (surface->Handle)

dlls/ddraw/tests/refcount.c

@@ -216,6 +216,14 @@ static void test_ddraw_objects(void)
             {
                 ref = getRefcount( (IUnknown *) stencil);
                 ok(ref == 2, "Got refcount %d, expected 2\n", ref);
+                hr = IDirectDrawSurface7_QueryInterface(surface, &IID_IDirectDrawSurface, (void **) &surface1);
+                ok(hr == DD_OK, "IDirectDrawSurface7_QueryInterface returned %08x\n", hr);
+                hr = IDirectDrawSurface7_QueryInterface(stencil, &IID_IDirectDrawSurface, (void **) &stencil1);
+                ok(hr == DD_OK, "IDirectDrawSurface7_QueryInterface returned %08x\n", hr);
+                hr = IDirectDrawSurface_DeleteAttachedSurface(surface1, 0, stencil1);
+                ok(hr == DDERR_SURFACENOTATTACHED, "DeleteAttachedSurface returned %08x\n", hr);
+                if (stencil1 != NULL) IDirectDrawSurface_Release(stencil1);
+                if (surface1 != NULL) IDirectDrawSurface_Release(surface1);
                 hr = IDirectDrawSurface7_DeleteAttachedSurface(surface, 0, stencil);
                 ok(hr == DD_OK, "DeleteAttachedSurface returned %08x\n", hr);
                 ref = getRefcount( (IUnknown *) stencil);
@@ -270,9 +278,9 @@ static void test_ddraw_objects(void)
             ref = IDirectDrawSurface_Release(surface1);
             ok(!ref, "Got refcount %d, expected 0\n", ref);
             ref = getRefcount( (IUnknown *) stencil1);
-            todo_wine ok(ref == 1, "Got refcount %d, expected 1\n", ref);
+            ok(ref == 1, "Got refcount %d, expected 1\n", ref);
             ref = IDirectDrawSurface_Release(stencil1);
-            todo_wine ok(!ref, "Got refcount %d, expected 0\n", ref);
+            ok(!ref, "Got refcount %d, expected 0\n", ref);
         }
         else
             IDirectDrawSurface_Release(surface1);