vcl/inc/win/saldata.hxx | 1 vcl/inc/win/salframe.h | 2 vcl/win/app/salinst.cxx | 1 vcl/win/window/salframe.cxx | 121 ++++++++++++++++++++------------------------ 4 files changed, 56 insertions(+), 69 deletions(-)
New commits: commit 7be45c419d98c4b68bdf359adedd3be6aafc2fcf Author: Noel Grandin <[email protected]> AuthorDate: Mon Jul 14 11:04:49 2025 +0200 Commit: Xisco Fauli <[email protected]> CommitDate: Fri Sep 19 11:08:15 2025 +0200 inline Init/ReleaseFrameGraphicsDC which makes it much more obvious which logic belongs where. Which fixes leaking WinSalGraphics objects on on error paths. Change-Id: Ib08c5b365e993d46fdc38a56e20e12c2833a1f85 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187851 Tested-by: Jenkins Reviewed-by: Noel Grandin <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191182 diff --git a/vcl/inc/win/salframe.h b/vcl/inc/win/salframe.h index 9d356a11bc45..c481cf49187d 100644 --- a/vcl/inc/win/salframe.h +++ b/vcl/inc/win/salframe.h @@ -92,8 +92,6 @@ public: private: void ImplSetParentFrame( HWND hNewParentWnd, bool bAsChild ); - bool InitFrameGraphicsDC( WinSalGraphics *pGraphics, HDC hDC, HWND hWnd ); - bool ReleaseFrameGraphicsDC( WinSalGraphics* pGraphics ); public: WinSalFrame(); diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx index 0557eb208c71..e5a3a38bb4fb 100644 --- a/vcl/win/window/salframe.cxx +++ b/vcl/win/window/salframe.cxx @@ -937,19 +937,6 @@ void WinSalFrame::updateScreenNumber() } } -bool WinSalFrame::ReleaseFrameGraphicsDC( WinSalGraphics* pGraphics ) -{ - assert( pGraphics ); - SalData* pSalData = GetSalData(); - HDC hDC = pGraphics->getHDC(); - if ( !hDC ) - return false; - pGraphics->setHDC(nullptr); - SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_RELEASEDC, - reinterpret_cast<WPARAM>(mhWnd), reinterpret_cast<LPARAM>(hDC) ); - return true; -} - WinSalFrame::~WinSalFrame() { SalData* pSalData = GetSalData(); @@ -967,7 +954,13 @@ WinSalFrame::~WinSalFrame() // destroy the thread SalGraphics if ( mpThreadGraphics ) { - ReleaseFrameGraphicsDC( mpThreadGraphics ); + HDC hDC = mpThreadGraphics->getHDC(); + if (hDC) + { + mpThreadGraphics->setHDC(nullptr); + SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_RELEASEDC, + reinterpret_cast<WPARAM>(mhWnd), reinterpret_cast<LPARAM>(hDC) ); + } delete mpThreadGraphics; mpThreadGraphics = nullptr; } @@ -975,7 +968,9 @@ WinSalFrame::~WinSalFrame() // destroy the local SalGraphics if ( mpLocalGraphics ) { - ReleaseFrameGraphicsDC( mpLocalGraphics ); + HDC hDC = mpLocalGraphics->getHDC(); + mpLocalGraphics->setHDC(nullptr); + ReleaseDC(mhWnd, hDC); delete mpLocalGraphics; mpLocalGraphics = nullptr; } @@ -1005,61 +1000,58 @@ WinSalFrame::~WinSalFrame() } } -bool WinSalFrame::InitFrameGraphicsDC( WinSalGraphics *pGraphics, HDC hDC, HWND hWnd ) -{ - assert( pGraphics ); - pGraphics->setHWND( hWnd ); - - HDC hCurrentDC = pGraphics->getHDC(); - assert( !hCurrentDC || (hCurrentDC == hDC) ); - if ( hCurrentDC ) - return true; - pGraphics->setHDC( hDC ); - - if ( !hDC ) - return false; - return true; -} - SalGraphics* WinSalFrame::AcquireGraphics() { if ( mbGraphicsAcquired || !mhWnd ) return nullptr; SalData* pSalData = GetSalData(); - WinSalGraphics* pGraphics; - HDC hDC; // Other threads get an own DC, because Windows modify in the // other case our DC (changing clip region), when they send a // WM_ERASEBACKGROUND message if ( !pSalData->mpInstance->IsMainThread() ) { + HDC hDC = reinterpret_cast<HDC>(static_cast<sal_IntPtr>(SendMessageW( pSalData->mpInstance->mhComWnd, + SAL_MSG_GETCACHEDDC, reinterpret_cast<WPARAM>(mhWnd), 0 ))); + if ( !hDC ) + return nullptr; + if ( !mpThreadGraphics ) mpThreadGraphics = new WinSalGraphics(WinSalGraphics::WINDOW, true, mhWnd, this); - pGraphics = mpThreadGraphics; - assert( !pGraphics->getHDC() ); - hDC = reinterpret_cast<HDC>(static_cast<sal_IntPtr>(SendMessageW( pSalData->mpInstance->mhComWnd, - SAL_MSG_GETCACHEDDC, reinterpret_cast<WPARAM>(mhWnd), 0 ))); + + assert(!mpThreadGraphics->getHDC() && "this is supposed to be zeroed when ReleaseGraphics is called"); + mpThreadGraphics->setHDC( hDC ); + + mbGraphicsAcquired = true; + return mpThreadGraphics; } else { if ( !mpLocalGraphics ) + { + HDC hDC = GetDC( mhWnd ); + if ( !hDC ) + return nullptr; mpLocalGraphics = new WinSalGraphics(WinSalGraphics::WINDOW, true, mhWnd, this); - pGraphics = mpLocalGraphics; - hDC = pGraphics->getHDC(); - if ( !hDC ) - hDC = GetDC( mhWnd ); + mpLocalGraphics->setHDC( hDC ); + } + mbGraphicsAcquired = true; + return mpLocalGraphics; } - - mbGraphicsAcquired = InitFrameGraphicsDC( pGraphics, hDC, mhWnd ); - return mbGraphicsAcquired ? pGraphics : nullptr; } void WinSalFrame::ReleaseGraphics( SalGraphics* pGraphics ) { if ( mpThreadGraphics == pGraphics ) - ReleaseFrameGraphicsDC( mpThreadGraphics ); + { + SalData* pSalData = GetSalData(); + HDC hDC = mpThreadGraphics->getHDC(); + assert(hDC); + mpThreadGraphics->setHDC(nullptr); + SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_RELEASEDC, + reinterpret_cast<WPARAM>(mhWnd), reinterpret_cast<LPARAM>(hDC) ); + } mbGraphicsAcquired = false; } @@ -1496,20 +1488,28 @@ void WinSalFrame::ImplSetParentFrame( HWND hNewParentWnd, bool bAsChild ) { // save current gdi objects before hdc is gone HDC hDC = mpThreadGraphics->getHDC(); - if ( hDC ) + if (hDC) { hFont = static_cast<HFONT>(GetCurrentObject( hDC, OBJ_FONT )); hPen = static_cast<HPEN>(GetCurrentObject( hDC, OBJ_PEN )); hBrush = static_cast<HBRUSH>(GetCurrentObject( hDC, OBJ_BRUSH )); - } - bHadThreadGraphics = ReleaseFrameGraphicsDC( mpThreadGraphics ); - assert( (bHadThreadGraphics && hDC) || (!bHadThreadGraphics && !hDC) ); + mpThreadGraphics->setHDC(nullptr); + SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_RELEASEDC, + reinterpret_cast<WPARAM>(mhWnd), reinterpret_cast<LPARAM>(hDC) ); + + bHadThreadGraphics = true; + } } // release the local DC if ( mpLocalGraphics ) - bHadLocalGraphics = ReleaseFrameGraphicsDC( mpLocalGraphics ); + { + bHadLocalGraphics = true; + HDC hDC = mpLocalGraphics->getHDC(); + mpLocalGraphics->setHDC(nullptr); + ReleaseDC(mhWnd, hDC); + } // create a new hwnd with the same styles HWND hWndParent = hNewParentWnd; @@ -1524,12 +1524,13 @@ void WinSalFrame::ImplSetParentFrame( HWND hNewParentWnd, bool bAsChild ) // re-create thread DC if( bHadThreadGraphics ) { + mpThreadGraphics->setHWND( hWnd ); HDC hDC = reinterpret_cast<HDC>(static_cast<sal_IntPtr>( SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_GETCACHEDDC, reinterpret_cast<WPARAM>(hWnd), 0 ))); - InitFrameGraphicsDC( mpThreadGraphics, hDC, hWnd ); if ( hDC ) { + mpThreadGraphics->setHDC( hDC ); // re-select saved gdi objects if( hFont ) SelectObject( hDC, hFont ); @@ -1542,7 +1543,12 @@ void WinSalFrame::ImplSetParentFrame( HWND hNewParentWnd, bool bAsChild ) // re-create local DC if( bHadLocalGraphics ) - InitFrameGraphicsDC( mpLocalGraphics, GetDC( hWnd ), hWnd ); + { + mpLocalGraphics->setHWND( hWnd ); + HDC hDC = GetDC( hWnd ); + if (hDC) + mpLocalGraphics->setHDC( hDC ); + } // TODO: add SetParent() call for SalObjects SAL_WARN_IF( !systemChildren.empty(), "vcl", "WinSalFrame::SetParent() parent of living system child window will be destroyed!"); commit 2a93939b3535c6790eb475aab436a188ca294d01 Author: Noel Grandin <[email protected]> AuthorDate: Mon Jul 14 11:12:10 2025 +0200 Commit: Xisco Fauli <[email protected]> CommitDate: Fri Sep 19 11:08:03 2025 +0200 remove mnCacheDCInUse Windows95 has not been supported for some time, we don't need this limitation anymore. Change-Id: I52a476bb1da4db8fc8e7a420f049c3f895549fa1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187891 Tested-by: Jenkins Reviewed-by: Noel Grandin <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191181 diff --git a/vcl/inc/win/saldata.hxx b/vcl/inc/win/saldata.hxx index 6959c7e9c02e..1d003f5129c0 100644 --- a/vcl/inc/win/saldata.hxx +++ b/vcl/inc/win/saldata.hxx @@ -115,7 +115,6 @@ public: sal_uInt16 mnStockPenCount; // count of static pens sal_uInt16 mnStockBrushCount; // count of static brushes WPARAM mnSalObjWantKeyEvt; // KeyEvent that should be processed by SalObj-Hook - BYTE mnCacheDCInUse; // count of CacheDC in use bool mbObjClassInit; // is SALOBJECTCLASS initialised DWORD mnAppThreadId; // Id from Application-Thread SalIcon* mpFirstIcon; // icon cache, points to first icon, NULL if none diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx index a14c8b4feb0c..16e3206f4878 100644 --- a/vcl/win/app/salinst.cxx +++ b/vcl/win/app/salinst.cxx @@ -271,7 +271,6 @@ SalData::SalData() mnStockPenCount = 0; // count of static pens mnStockBrushCount = 0; // count of static brushes mnSalObjWantKeyEvt = 0; // KeyEvent for the SalObj hook - mnCacheDCInUse = 0; // count of CacheDC in use mbObjClassInit = false; // is SALOBJECTCLASS initialised mnAppThreadId = 0; // Id from Application-Thread mpFirstIcon = nullptr; // icon cache, points to first icon, NULL if none diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx index f7cd4abdb0be..0557eb208c71 100644 --- a/vcl/win/window/salframe.cxx +++ b/vcl/win/window/salframe.cxx @@ -947,8 +947,6 @@ bool WinSalFrame::ReleaseFrameGraphicsDC( WinSalGraphics* pGraphics ) pGraphics->setHDC(nullptr); SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_RELEASEDC, reinterpret_cast<WPARAM>(mhWnd), reinterpret_cast<LPARAM>(hDC) ); - if ( pGraphics == mpThreadGraphics ) - pSalData->mnCacheDCInUse--; return true; } @@ -1009,7 +1007,6 @@ WinSalFrame::~WinSalFrame() bool WinSalFrame::InitFrameGraphicsDC( WinSalGraphics *pGraphics, HDC hDC, HWND hWnd ) { - SalData* pSalData = GetSalData(); assert( pGraphics ); pGraphics->setHWND( hWnd ); @@ -1021,9 +1018,6 @@ bool WinSalFrame::InitFrameGraphicsDC( WinSalGraphics *pGraphics, HDC hDC, HWND if ( !hDC ) return false; - - if ( pGraphics == mpThreadGraphics ) - pSalData->mnCacheDCInUse++; return true; } @@ -1041,11 +1035,6 @@ SalGraphics* WinSalFrame::AcquireGraphics() // WM_ERASEBACKGROUND message if ( !pSalData->mpInstance->IsMainThread() ) { - // We use only three CacheDC's for all threads, because W9x is limited - // to max. 5 Cache DC's per thread - if ( pSalData->mnCacheDCInUse >= 3 ) - return nullptr; - if ( !mpThreadGraphics ) mpThreadGraphics = new WinSalGraphics(WinSalGraphics::WINDOW, true, mhWnd, this); pGraphics = mpThreadGraphics; @@ -1502,8 +1491,6 @@ void WinSalFrame::ImplSetParentFrame( HWND hNewParentWnd, bool bAsChild ) HPEN hPen = nullptr; HBRUSH hBrush = nullptr; - int oldCount = pSalData->mnCacheDCInUse; - // release the thread DC if ( mpThreadGraphics ) { @@ -1550,8 +1537,6 @@ void WinSalFrame::ImplSetParentFrame( HWND hNewParentWnd, bool bAsChild ) SelectObject( hDC, hPen ); if( hBrush ) SelectObject( hDC, hBrush ); - - SAL_WARN_IF( oldCount != pSalData->mnCacheDCInUse, "vcl", "WinSalFrame::SetParent() hDC count corrupted"); } }
