Fixed crash caused by implicit release of clippers

See issues #96 and #97.

DDrawCompat/DDraw/DirectDrawClipper.cpp

@@ -6,6 +6,7 @@
 #include <D3dDdi/KernelModeThunks.h>
 #include <DDraw/DirectDrawClipper.h>
 #include <DDraw/ScopedThreadLock.h>
+#include <DDraw/Surfaces/Surface.h>
 #include <DDraw/Visitors/DirectDrawClipperVtblVisitor.h>
 #include <Gdi/Gdi.h>
 #include <Gdi/Region.h>
@@ -15,10 +16,12 @@ namespace
 	struct ClipperData
 	{
 		HWND hwnd;
-		std::vector<unsigned char> oldClipList;
+		DWORD refCount;
+		std::vector<unsigned char> origClipList;
 	};
 
 	std::map<IDirectDrawClipper*, ClipperData> g_clipperData;
+	std::map<DDraw::Surface*, std::map<IDirectDrawClipper*, ClipperData>::iterator> g_surfaceToClipperData;
 	bool g_isInvalidated = false;
 
 	void updateWindowClipList(CompatRef<IDirectDrawClipper> clipper, ClipperData& data);
@@ -28,6 +31,13 @@ namespace
 		g_isInvalidated = true;
 	}
 
+	void restoreOrigClipList(IDirectDrawClipper* clipper, ClipperData& clipperData)
+	{
+		getOrigVtable(clipper).SetClipList(clipper,
+			clipperData.origClipList.empty() ? nullptr : reinterpret_cast<RGNDATA*>(clipperData.origClipList.data()), 0);
+		clipperData.origClipList.clear();
+	}
+
 	void updateWindowClipList(CompatRef<IDirectDrawClipper> clipper, ClipperData& data)
 	{
 		HDC dc = GetDCEx(data.hwnd, nullptr, DCX_CACHE | DCX_USESTYLE);
@@ -46,10 +56,7 @@ namespace
 		GetRegionData(rgn, rgnSize, reinterpret_cast<RGNDATA*>(rgnData.data()));
 
 		clipper->SetHWnd(&clipper, 0, nullptr);
-		if (FAILED(clipper->SetClipList(&clipper, reinterpret_cast<RGNDATA*>(rgnData.data()), 0)))
+		clipper->SetClipList(&clipper, rgnData.empty() ? nullptr : reinterpret_cast<RGNDATA*>(rgnData.data()), 0);
-		{
-			clipper->SetHWnd(&clipper, 0, data.hwnd);
-		}
 	}
 
 	HRESULT STDMETHODCALLTYPE GetHWnd(IDirectDrawClipper* This, HWND* lphWnd)
@@ -57,7 +64,7 @@ namespace
 		if (lphWnd)
 		{
 			auto it = g_clipperData.find(This);
-			if (it != g_clipperData.end())
+			if (it != g_clipperData.end() && it->second.hwnd)
 			{
 				*lphWnd = it->second.hwnd;
 				return DD_OK;
@@ -66,19 +73,10 @@ namespace
 		return getOrigVtable(This).GetHWnd(This, lphWnd);
 	}
 	
-	ULONG STDMETHODCALLTYPE Release(IDirectDrawClipper* This)
-	{
-		ULONG result = getOrigVtable(This).Release(This);
-		if (0 == result)
-		{
-			g_clipperData.erase(This);
-		}
-		return result;
-	}
-
 	HRESULT STDMETHODCALLTYPE SetClipList(IDirectDrawClipper* This, LPRGNDATA lpClipList, DWORD dwFlags)
 	{
-		if (g_clipperData.find(This) != g_clipperData.end())
+		auto it = g_clipperData.find(This);
+		if (it != g_clipperData.end() && it->second.hwnd)
 		{
 			return DDERR_CLIPPERISUSINGHWND;
 		}
@@ -87,31 +85,38 @@ namespace
 
 	HRESULT STDMETHODCALLTYPE SetHWnd(IDirectDrawClipper* This, DWORD dwFlags, HWND hWnd)
 	{
+		auto it = g_clipperData.find(This);
+		if (it == g_clipperData.end())
+		{
+			return getOrigVtable(This).SetHWnd(This, dwFlags, hWnd);
+		}
+
+		std::vector<unsigned char> origClipList;
+		if (hWnd && !it->second.hwnd)
+		{
+			DWORD size = 0;
+			getOrigVtable(This).GetClipList(This, nullptr, nullptr, &size);
+			origClipList.resize(size);
+			getOrigVtable(This).GetClipList(This, nullptr, reinterpret_cast<RGNDATA*>(origClipList.data()), &size);
+		}
+
 		HRESULT result = getOrigVtable(This).SetHWnd(This, dwFlags, hWnd);
 		if (SUCCEEDED(result))
 		{
-			auto it = g_clipperData.find(This);
 			if (hWnd)
 			{
-				if (it == g_clipperData.end())
+				if (!it->second.hwnd)
 				{
-					it = g_clipperData.insert({ This, ClipperData() }).first;
+					it->second.origClipList = origClipList;
-					it->second.hwnd = hWnd;
-
-					DWORD size = 0;
-					getOrigVtable(This).GetClipList(This, nullptr, nullptr, &size);
-					it->second.oldClipList.resize(size);
-					getOrigVtable(This).GetClipList(This, nullptr,
-						reinterpret_cast<RGNDATA*>(it->second.oldClipList.data()), &size);
 				}
+				it->second.hwnd = hWnd;
 				updateWindowClipList(*This, it->second);
 				Gdi::watchWindowPosChanges(&onWindowPosChange);
 			}
-			else if (it != g_clipperData.end())
+			else if (it->second.hwnd)
 			{
-				getOrigVtable(This).SetClipList(This, it->second.oldClipList.empty() ? nullptr :
+				restoreOrigClipList(it->first, it->second);
-					reinterpret_cast<RGNDATA*>(it->second.oldClipList.data()), 0);
+				it->second.hwnd = nullptr;
-				g_clipperData.erase(it);
 			}
 		}
 		return result;
@@ -120,7 +125,6 @@ namespace
 	constexpr void setCompatVtable(IDirectDrawClipperVtbl& vtable)
 	{
 		vtable.GetHWnd = &GetHWnd;
-		vtable.Release = &Release;
 		vtable.SetClipList = &SetClipList;
 		vtable.SetHWnd = &SetHWnd;
 	}
@@ -140,6 +144,47 @@ namespace DDraw
 			return ExtCreateRegion(nullptr, size, reinterpret_cast<RGNDATA*>(rgnData.data()));
 		}
 
+		void setClipper(Surface& surface, IDirectDrawClipper* clipper)
+		{
+			auto it = g_surfaceToClipperData.find(&surface);
+			if (it != g_surfaceToClipperData.end())
+			{
+				auto& [prevClipper, prevClipperData] = *it->second;
+				if (prevClipper == clipper)
+				{
+					return;
+				}
+
+				--prevClipperData.refCount;
+				if (0 == prevClipperData.refCount)
+				{
+					if (prevClipperData.hwnd)
+					{
+						restoreOrigClipList(prevClipper, prevClipperData);
+						getOrigVtable(prevClipper).SetHWnd(prevClipper, 0, prevClipperData.hwnd);
+					}
+					g_clipperData.erase(it->second);
+				}
+				g_surfaceToClipperData.erase(it);
+			}
+
+			if (clipper)
+			{
+				auto [clipperDataIter, inserted] = g_clipperData.insert({ clipper, ClipperData{} });
+				if (inserted)
+				{
+					HWND hwnd = nullptr;
+					getOrigVtable(clipper).GetHWnd(clipper, &hwnd);
+					if (hwnd)
+					{
+						SetHWnd(clipper, 0, hwnd);
+					}
+				}
+				++clipperDataIter->second.refCount;
+				g_surfaceToClipperData[&surface] = clipperDataIter;
+			}
+		}
+
 		HRESULT setClipRgn(CompatRef<IDirectDrawClipper> clipper, HRGN rgn)
 		{
 			std::vector<unsigned char> rgnData;

DDrawCompat/DDraw/DirectDrawClipper.h

@@ -6,9 +6,12 @@
 
 namespace DDraw
 {
+	class Surface;
+
 	namespace DirectDrawClipper
 	{
 		HRGN getClipRgn(CompatRef<IDirectDrawClipper> clipper);
+		void setClipper(Surface& surface, IDirectDrawClipper* clipper);
 		HRESULT setClipRgn(CompatRef<IDirectDrawClipper> clipper, HRGN rgn);
 		void update();
 

DDrawCompat/DDraw/DirectDrawSurface.cpp

@@ -68,6 +68,7 @@ namespace
 		SET_COMPAT_METHOD(QueryInterface);
 		SET_COMPAT_METHOD(ReleaseDC);
 		SET_COMPAT_METHOD(Restore);
+		SET_COMPAT_METHOD(SetClipper);
 		SET_COMPAT_METHOD(SetPalette);
 		SET_COMPAT_METHOD(Unlock);
 	}

DDrawCompat/DDraw/Surfaces/Surface.cpp

@@ -3,6 +3,7 @@
 #include <Common/CompatPtr.h>
 #include <D3dDdi/Device.h>
 #include <D3dDdi/Resource.h>
+#include <DDraw/DirectDrawClipper.h>
 #include <DDraw/DirectDrawSurface.h>
 #include <DDraw/Surfaces/Surface.h>
 #include <DDraw/Surfaces/SurfaceImpl.h>
@@ -43,6 +44,7 @@ namespace DDraw
 
 	Surface::~Surface()
 	{
+		DirectDrawClipper::setClipper(*this, nullptr);
 	}
 
 	void Surface::attach(CompatRef<IDirectDrawSurface7> dds, std::unique_ptr<Surface> privateData)

DDrawCompat/DDraw/Surfaces/SurfaceImpl.cpp

@@ -203,6 +203,17 @@ namespace DDraw
 		return result;
 	}
 
+	template <typename TSurface>
+	HRESULT SurfaceImpl<TSurface>::SetClipper(TSurface* This, LPDIRECTDRAWCLIPPER lpDDClipper)
+	{
+		HRESULT result = getOrigVtable(This).SetClipper(This, lpDDClipper);
+		if (SUCCEEDED(result))
+		{
+			DDraw::DirectDrawClipper::setClipper(*m_data, lpDDClipper);
+		}
+		return result;
+	}
+
 	template <typename TSurface>
 	HRESULT SurfaceImpl<TSurface>::SetPalette(TSurface* This, LPDIRECTDRAWPALETTE lpDDPalette)
 	{

DDrawCompat/DDraw/Surfaces/SurfaceImpl.h

@@ -38,6 +38,7 @@ namespace DDraw
 		virtual HRESULT QueryInterface(TSurface* This, REFIID riid, LPVOID* obp);
 		virtual HRESULT ReleaseDC(TSurface* This, HDC hDC);
 		virtual HRESULT Restore(TSurface* This);
+		virtual HRESULT SetClipper(TSurface* This, LPDIRECTDRAWCLIPPER lpDDClipper);
 		virtual HRESULT SetPalette(TSurface* This, LPDIRECTDRAWPALETTE lpDDPalette);
 		virtual HRESULT Unlock(TSurface* This, TUnlockParam lpRect);