DDraw: Palette refcounting fix
Stefan Dösinger
stefandoesinger at gmx.at
Wed Jul 5 11:29:08 CDT 2006
Am Mittwoch 05 Juli 2006 02:34 schrieb Stefan Dösinger:
> As shown in a test I sent with the first palette refcounting fix
> IDirectDraw7::CreatePalette addrefs the IDirectDraw7 interface to hold a
> reference for the palette. The same applies to IDirectDraw4::CreatePalette,
> but NOT IDirectDraw2::CreatePalette and IDirectDraw::CreatePalette. Don't
> ask me why ;-)
>
> This patch adds a test which checks each DirectDraw interface version for
> this behavior and fixes the CreatePalette functions accordingly. It depends
> on the refcount split patch, as well as the refcount split patch depends on
> this patch for correct releasing of the held reference.
>
> This patch fixes the issue in C&C Red Alert where the game complained about
> an unexpected return value from IDirectDraw_Release
>
> ChangeLog:
> Stefan Dösinger: Add a test for more details of palette refcounting and fix
> the code.
Here is an updated patch which just has updated lines due to the modifications
in the refcount split patch. Otherwise it is unchanged.
-------------- next part --------------
From nobody Mon Sep 17 00:00:00 2001
From: Stefan Dösinger <stefan at codeweavers.com>
Date: Wed Jul 5 18:21:27 2006 +0200
Subject: [PATCH] DDraw: Another palette refcounting fix
---
dlls/ddraw/ddraw.c | 1 +
dlls/ddraw/ddraw_private.h | 1 +
dlls/ddraw/ddraw_thunks.c | 18 ++++++++++--
dlls/ddraw/palette.c | 7 ++++-
dlls/ddraw/tests/refcount.c | 62 +++++++++++++++++++++++++++++++++----------
5 files changed, 70 insertions(+), 19 deletions(-)
055b284f32824e79c2fddee9e4aff4e2888970d9
diff --git a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
index 83d302e..30c45c3 100644
--- a/dlls/ddraw/ddraw.c
+++ b/dlls/ddraw/ddraw.c
@@ -3012,6 +3012,7 @@ IDirectDrawImpl_CreatePalette(IDirectDra
}
IDirectDraw7_AddRef(iface);
+ object->ifaceToRelease = (IUnknown *) iface;
*Palette = ICOM_INTERFACE(object, IDirectDrawPalette);
return DD_OK;
}
diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
index c2336c8..2593cd3 100644
--- a/dlls/ddraw/ddraw_private.h
+++ b/dlls/ddraw/ddraw_private.h
@@ -394,6 +394,7 @@ struct IDirectDrawPaletteImpl
/* IDirectDrawPalette fields */
IDirectDrawImpl *ddraw_owner;
+ IUnknown *ifaceToRelease;
};
const IDirectDrawPaletteVtbl IDirectDrawPalette_Vtbl;
diff --git a/dlls/ddraw/ddraw_thunks.c b/dlls/ddraw/ddraw_thunks.c
index b35abc5..054ca56 100644
--- a/dlls/ddraw/ddraw_thunks.c
+++ b/dlls/ddraw/ddraw_thunks.c
@@ -227,10 +227,13 @@ IDirectDrawImpl_CreatePalette(LPDIRECTDR
dwFlags, pEntries, ppPalette, pUnkOuter);
if(SUCCEEDED(hr) && *ppPalette)
{
+ IDirectDrawPaletteImpl *impl = ICOM_OBJECT(IDirectDrawPaletteImpl, IDirectDrawPalette, *ppPalette);
IDirectDraw7_Release(COM_INTERFACE_CAST(IDirectDrawImpl,
IDirectDraw,
IDirectDraw7,
This));
+ impl->ifaceToRelease = NULL;
+
}
return hr;
}
@@ -242,18 +245,21 @@ IDirectDraw2Impl_CreatePalette(LPDIRECTD
IUnknown *pUnkOuter)
{
HRESULT hr;
- return IDirectDraw7_CreatePalette(COM_INTERFACE_CAST(IDirectDrawImpl,
+ hr = IDirectDraw7_CreatePalette(COM_INTERFACE_CAST(IDirectDrawImpl,
IDirectDraw2,
IDirectDraw7,
This),
dwFlags, pEntries, ppPalette, pUnkOuter);
if(SUCCEEDED(hr) && *ppPalette)
{
+ IDirectDrawPaletteImpl *impl = ICOM_OBJECT(IDirectDrawPaletteImpl, IDirectDrawPalette, *ppPalette);
IDirectDraw7_Release(COM_INTERFACE_CAST(IDirectDrawImpl,
- IDirectDraw,
+ IDirectDraw2,
IDirectDraw7,
This));
+ impl->ifaceToRelease = NULL;
}
+ return hr;
}
static HRESULT WINAPI
@@ -263,18 +269,22 @@ IDirectDraw4Impl_CreatePalette(LPDIRECTD
IUnknown *pUnkOuter)
{
HRESULT hr;
- return IDirectDraw7_CreatePalette(COM_INTERFACE_CAST(IDirectDrawImpl,
+ hr = IDirectDraw7_CreatePalette(COM_INTERFACE_CAST(IDirectDrawImpl,
IDirectDraw4,
IDirectDraw7,
This),
dwFlags, pEntries, ppPalette, pUnkOuter);
if(SUCCEEDED(hr) && *ppPalette)
{
+ IDirectDrawPaletteImpl *impl = ICOM_OBJECT(IDirectDrawPaletteImpl, IDirectDrawPalette, *ppPalette);
IDirectDraw7_Release(COM_INTERFACE_CAST(IDirectDrawImpl,
- IDirectDraw,
+ IDirectDraw4,
IDirectDraw7,
This));
+ IDirectDraw4_AddRef(This);
+ impl->ifaceToRelease = (IUnknown *) This;
}
+ return hr;
}
static HRESULT WINAPI
diff --git a/dlls/ddraw/palette.c b/dlls/ddraw/palette.c
index 97a9ef4..03bdb3a 100644
--- a/dlls/ddraw/palette.c
+++ b/dlls/ddraw/palette.c
@@ -21,6 +21,8 @@
#include "winerror.h"
#include "wine/debug.h"
+#define COBJMACROS
+
#include <assert.h>
#include <string.h>
@@ -104,7 +106,10 @@ IDirectDrawPaletteImpl_Release(IDirectDr
if (ref == 0)
{
IWineD3DPalette_Release(This->wineD3DPalette);
- IDirectDraw7_Release(ICOM_INTERFACE(This->ddraw_owner, IDirectDraw7));
+ if(This->ifaceToRelease)
+ {
+ IUnknown_Release(This->ifaceToRelease);
+ }
HeapFree(GetProcessHeap(), 0, This);
}
diff --git a/dlls/ddraw/tests/refcount.c b/dlls/ddraw/tests/refcount.c
index 12d7ec8..b1e5472 100644
--- a/dlls/ddraw/tests/refcount.c
+++ b/dlls/ddraw/tests/refcount.c
@@ -47,32 +47,42 @@ static void test_ddraw_objects(void)
{
HRESULT hr;
unsigned long ref;
- IDirectDraw7 *DDraw;
+ IDirectDraw7 *DDraw7;
+ IDirectDraw4 *DDraw4;
+ IDirectDraw4 *DDraw2;
+ IDirectDraw4 *DDraw1;
IDirectDrawPalette *palette;
IDirectDrawSurface7 *surface;
PALETTEENTRY Table[256];
DDSURFACEDESC2 ddsd;
- hr = pDirectDrawCreateEx(NULL, (void **) &DDraw, &IID_IDirectDraw7, NULL);
+ hr = pDirectDrawCreateEx(NULL, (void **) &DDraw7, &IID_IDirectDraw7, NULL);
ok(hr == DD_OK || hr==DDERR_NODIRECTDRAWSUPPORT, "DirectDrawCreateEx returned: %lx\n", hr);
- if(!DDraw)
+ if(!DDraw7)
{
trace("Couldn't create DDraw interface, skipping tests\n");
return;
}
- ref = getRefcount( (IUnknown *) DDraw);
+ hr = IDirectDraw7_QueryInterface(DDraw7, &IID_IDirectDraw4, (void **) &DDraw4);
+ ok(hr == DD_OK, "IDirectDraw7_QueryInterface returned %08lx\n", hr);
+ hr = IDirectDraw7_QueryInterface(DDraw7, &IID_IDirectDraw2, (void **) &DDraw2);
+ ok(hr == DD_OK, "IDirectDraw7_QueryInterface returned %08lx\n", hr);
+ hr = IDirectDraw7_QueryInterface(DDraw7, &IID_IDirectDraw, (void **) &DDraw1);
+ ok(hr == DD_OK, "IDirectDraw7_QueryInterface returned %08lx\n", hr);
+
+ ref = getRefcount( (IUnknown *) DDraw7);
ok(ref == 1, "Got refcount %ld, expected 1\n", ref);
/* Fails without a cooplevel */
- hr = IDirectDraw7_CreatePalette(DDraw, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, &palette, NULL);
+ hr = IDirectDraw7_CreatePalette(DDraw7, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, &palette, NULL);
ok(hr == DDERR_NOCOOPERATIVELEVELSET, "CreatePalette returned %08lx\n", hr);
/* This check is before the cooplevel check */
- hr = IDirectDraw7_CreatePalette(DDraw, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, &palette, (void *) 0xdeadbeef);
+ hr = IDirectDraw7_CreatePalette(DDraw7, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, &palette, (void *) 0xdeadbeef);
ok(hr == CLASS_E_NOAGGREGATION, "CreatePalette returned %08lx\n", hr);
- hr = IDirectDraw7_SetCooperativeLevel(DDraw, 0, DDSCL_NORMAL);
+ hr = IDirectDraw7_SetCooperativeLevel(DDraw7, 0, DDSCL_NORMAL);
ok(hr == DD_OK, "SetCooperativeLevel failed with %08lx\n", hr);
memset(&ddsd, 0, sizeof(ddsd));
@@ -85,22 +95,22 @@ static void test_ddraw_objects(void)
U4(ddsd).ddpfPixelFormat.dwFlags = DDPF_PALETTEINDEXED8 | DDPF_RGB;
U1(U4(ddsd).ddpfPixelFormat).dwRGBBitCount = 8;
- hr = IDirectDraw7_CreateSurface(DDraw, &ddsd, &surface, NULL);
+ hr = IDirectDraw7_CreateSurface(DDraw7, &ddsd, &surface, NULL);
ok(hr == DD_OK, "CreateSurface failed with %08lx\n", hr);
/* DDraw refcount increased by 1 */
- ref = getRefcount( (IUnknown *) DDraw);
+ ref = getRefcount( (IUnknown *) DDraw7);
ok(ref == 2, "Got refcount %ld, expected 2\n", ref);
/* Surface refcount starts with 1 */
ref = getRefcount( (IUnknown *) surface);
ok(ref == 1, "Got refcount %ld, expected 1\n", ref);
- hr = IDirectDraw7_CreatePalette(DDraw, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, &palette, NULL);
+ hr = IDirectDraw7_CreatePalette(DDraw7, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, &palette, NULL);
ok(hr == DD_OK, "CreatePalette returned %08lx\n", hr);
/* DDraw refcount increased by 1 */
- ref = getRefcount( (IUnknown *) DDraw);
+ ref = getRefcount( (IUnknown *) DDraw7);
ok(ref == 3, "Got refcount %ld, expected 3\n", ref);
/* Palette starts with 1 */
@@ -119,7 +129,7 @@ static void test_ddraw_objects(void)
IDirectDrawSurface7_Release(surface);
/* Incresed before - decrease now */
- ref = getRefcount( (IUnknown *) DDraw);
+ ref = getRefcount( (IUnknown *) DDraw7);
ok(ref == 2, "Got refcount %ld, expected 2\n", ref);
/* Releasing the surface detaches the palette */
@@ -129,10 +139,34 @@ static void test_ddraw_objects(void)
IDirectDrawPalette_Release(palette);
/* Incresed before - decrease now */
- ref = getRefcount( (IUnknown *) DDraw);
+ ref = getRefcount( (IUnknown *) DDraw7);
ok(ref == 1, "Got refcount %ld, expected 1\n", ref);
- IDirectDraw7_Release(DDraw);
+ /* Not all interfaces are AddRefed when a palette is created */
+ hr = IDirectDraw4_CreatePalette(DDraw4, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, &palette, NULL);
+ ok(hr == DD_OK, "CreatePalette returned %08lx\n", hr);
+ ref = getRefcount( (IUnknown *) DDraw4);
+ ok(ref == 2, "Got refcount %ld, expected 2\n", ref);
+ IDirectDrawPalette_Release(palette);
+
+ /* No addref here */
+ hr = IDirectDraw2_CreatePalette(DDraw2, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, &palette, NULL);
+ ok(hr == DD_OK, "CreatePalette returned %08lx\n", hr);
+ ref = getRefcount( (IUnknown *) DDraw2);
+ ok(ref == 1, "Got refcount %ld, expected 1\n", ref);
+ IDirectDrawPalette_Release(palette);
+
+ /* No addref here */
+ hr = IDirectDraw_CreatePalette(DDraw1, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, &palette, NULL);
+ ok(hr == DD_OK, "CreatePalette returned %08lx\n", hr);
+ ref = getRefcount( (IUnknown *) DDraw1);
+ ok(ref == 1, "Got refcount %ld, expected 1\n", ref);
+ IDirectDrawPalette_Release(palette);
+
+ IDirectDraw7_Release(DDraw7);
+ IDirectDraw4_Release(DDraw4);
+ IDirectDraw2_Release(DDraw2);
+ IDirectDraw_Release(DDraw1);
}
static void test_iface_refcnt()
--
1.2.4
More information about the wine-patches
mailing list