From 83143589eecdb7ea33ce53b39d91374c75dcccc3 Mon Sep 17 00:00:00 2001 From: narzoul Date: Wed, 12 May 2021 21:45:10 +0200 Subject: [PATCH] Fixed crash caused by implicit release of clippers See issues #96 and #97. --- DDrawCompat/DDraw/DirectDrawClipper.cpp | 109 +++++++++++++++------ DDrawCompat/DDraw/DirectDrawClipper.h | 3 + DDrawCompat/DDraw/DirectDrawSurface.cpp | 1 + DDrawCompat/DDraw/Surfaces/Surface.cpp | 2 + DDrawCompat/DDraw/Surfaces/SurfaceImpl.cpp | 11 +++ DDrawCompat/DDraw/Surfaces/SurfaceImpl.h | 1 + 6 files changed, 95 insertions(+), 32 deletions(-) diff --git a/DDrawCompat/DDraw/DirectDrawClipper.cpp b/DDrawCompat/DDraw/DirectDrawClipper.cpp index 0b5c03c..e4e4097 100644 --- a/DDrawCompat/DDraw/DirectDrawClipper.cpp +++ b/DDrawCompat/DDraw/DirectDrawClipper.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -15,10 +16,12 @@ namespace struct ClipperData { HWND hwnd; - std::vector oldClipList; + DWORD refCount; + std::vector origClipList; }; std::map g_clipperData; + std::map::iterator> g_surfaceToClipperData; bool g_isInvalidated = false; void updateWindowClipList(CompatRef 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(clipperData.origClipList.data()), 0); + clipperData.origClipList.clear(); + } + void updateWindowClipList(CompatRef clipper, ClipperData& data) { HDC dc = GetDCEx(data.hwnd, nullptr, DCX_CACHE | DCX_USESTYLE); @@ -46,10 +56,7 @@ namespace GetRegionData(rgn, rgnSize, reinterpret_cast(rgnData.data())); clipper->SetHWnd(&clipper, 0, nullptr); - if (FAILED(clipper->SetClipList(&clipper, reinterpret_cast(rgnData.data()), 0))) - { - clipper->SetHWnd(&clipper, 0, data.hwnd); - } + clipper->SetClipList(&clipper, rgnData.empty() ? nullptr : reinterpret_cast(rgnData.data()), 0); } 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 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(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.hwnd = hWnd; - - DWORD size = 0; - getOrigVtable(This).GetClipList(This, nullptr, nullptr, &size); - it->second.oldClipList.resize(size); - getOrigVtable(This).GetClipList(This, nullptr, - reinterpret_cast(it->second.oldClipList.data()), &size); + it->second.origClipList = origClipList; } + 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 : - reinterpret_cast(it->second.oldClipList.data()), 0); - g_clipperData.erase(it); + restoreOrigClipList(it->first, it->second); + it->second.hwnd = nullptr; } } 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.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 clipper, HRGN rgn) { std::vector rgnData; diff --git a/DDrawCompat/DDraw/DirectDrawClipper.h b/DDrawCompat/DDraw/DirectDrawClipper.h index d4ffbd0..ab8477e 100644 --- a/DDrawCompat/DDraw/DirectDrawClipper.h +++ b/DDrawCompat/DDraw/DirectDrawClipper.h @@ -6,9 +6,12 @@ namespace DDraw { + class Surface; + namespace DirectDrawClipper { HRGN getClipRgn(CompatRef clipper); + void setClipper(Surface& surface, IDirectDrawClipper* clipper); HRESULT setClipRgn(CompatRef clipper, HRGN rgn); void update(); diff --git a/DDrawCompat/DDraw/DirectDrawSurface.cpp b/DDrawCompat/DDraw/DirectDrawSurface.cpp index 350a5d0..5159540 100644 --- a/DDrawCompat/DDraw/DirectDrawSurface.cpp +++ b/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); } diff --git a/DDrawCompat/DDraw/Surfaces/Surface.cpp b/DDrawCompat/DDraw/Surfaces/Surface.cpp index ced596a..05e9b12 100644 --- a/DDrawCompat/DDraw/Surfaces/Surface.cpp +++ b/DDrawCompat/DDraw/Surfaces/Surface.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -43,6 +44,7 @@ namespace DDraw Surface::~Surface() { + DirectDrawClipper::setClipper(*this, nullptr); } void Surface::attach(CompatRef dds, std::unique_ptr privateData) diff --git a/DDrawCompat/DDraw/Surfaces/SurfaceImpl.cpp b/DDrawCompat/DDraw/Surfaces/SurfaceImpl.cpp index 7ef778e..7cba1a4 100644 --- a/DDrawCompat/DDraw/Surfaces/SurfaceImpl.cpp +++ b/DDrawCompat/DDraw/Surfaces/SurfaceImpl.cpp @@ -203,6 +203,17 @@ namespace DDraw return result; } + template + HRESULT SurfaceImpl::SetClipper(TSurface* This, LPDIRECTDRAWCLIPPER lpDDClipper) + { + HRESULT result = getOrigVtable(This).SetClipper(This, lpDDClipper); + if (SUCCEEDED(result)) + { + DDraw::DirectDrawClipper::setClipper(*m_data, lpDDClipper); + } + return result; + } + template HRESULT SurfaceImpl::SetPalette(TSurface* This, LPDIRECTDRAWPALETTE lpDDPalette) { diff --git a/DDrawCompat/DDraw/Surfaces/SurfaceImpl.h b/DDrawCompat/DDraw/Surfaces/SurfaceImpl.h index 27125e4..2ad3e5b 100644 --- a/DDrawCompat/DDraw/Surfaces/SurfaceImpl.h +++ b/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);