[Libreoffice-commits] core.git: vcl/inc vcl/win

Jan-Marek Glogowski (via logerrit) logerrit at kemper.freedesktop.org
Sat Sep 25 13:48:28 UTC 2021


 vcl/inc/win/salgdi.h        |    8 +++++---
 vcl/win/gdi/salgdi.cxx      |   24 ++++++++++++++++++++++++
 vcl/win/gdi/salprn.cxx      |   34 ++++++++++------------------------
 vcl/win/gdi/salvd.cxx       |   13 +++++++------
 vcl/win/window/salframe.cxx |    4 +---
 5 files changed, 47 insertions(+), 36 deletions(-)

New commits:
commit 42f9d4335bfa4b7299224801fd7ccdd97ae92fbf
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Wed Sep 15 17:24:48 2021 +0200
Commit:     Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Sat Sep 25 15:47:55 2021 +0200

    WIN always (de-)init WinSalGraphics
    
    Originally I thought the whole (de-)initialization was some kind
    of lazy SalGraphics init stuff. But reading the existing code
    proved me wrong and every InitGraphics came a few lines after a
    setHDC call and DeInitGraphics before deletion or setHDC(nullptr).
    
    $ git grep -n "delete.*pGraph\|setHDC\|InitGraphics" vcl/win/
    
    So just make (De-)Init part of setHDC and drop all the other calls
    to (De-)InitGraphics, adding a setHDC(nullptr) to the destructor.
    
    I've also added some questionable asserts, like
    
    assert(pPrinter->mpGraphics->getHDC() == pPrinter->mhDC);
    
    As I read the code, you can have a printer object initialized with
    a DC but without graphics, which will be initialized on demand.
    
    But AFAIK it's invalid to have a printer DC != graphics DC.
    
    Then there was the hDC check in WinSalPrinter::EndPage, which is
    IMHO not needed. AFAIK there is no way the graphics DC might be
    "magically" invalidated, so restoring the old DC values should
    always work.
    
    Change-Id: I1b961cfa733263ce773575a728bcce5c7d3e97ad
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122157
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>

diff --git a/vcl/inc/win/salgdi.h b/vcl/inc/win/salgdi.h
index 3e1cac3a5634..f43224a861a4 100644
--- a/vcl/inc/win/salgdi.h
+++ b/vcl/inc/win/salgdi.h
@@ -166,19 +166,21 @@ private:
     RGNDATA*                mpStdClipRgnData;   // Cache Standard-ClipRegion-Data
     int                     mnPenWidth;         // line width
 
+    // just call both from setHDC!
+    void InitGraphics();
+    void DeInitGraphics();
+
 public:
     HFONT ImplDoSetFont(FontSelectPattern const & i_rFont, const PhysicalFontFace * i_pFontFace, HFONT& o_rOldFont);
 
     HDC getHDC() const { return mhLocalDC; }
-    void setHDC(HDC aNew) { mhLocalDC = aNew; }
+    void setHDC(HDC aNew);
 
     HPALETTE getDefPal() const;
     void setDefPal(HPALETTE hDefPal);
 
     HRGN getRegion() const;
 
-    void InitGraphics();
-    void DeInitGraphics();
 
     enum Type
     {
diff --git a/vcl/win/gdi/salgdi.cxx b/vcl/win/gdi/salgdi.cxx
index 0e868e516b60..43536de1fb01 100644
--- a/vcl/win/gdi/salgdi.cxx
+++ b/vcl/win/gdi/salgdi.cxx
@@ -452,6 +452,9 @@ void ImplUpdateSysColorEntries()
 
 void WinSalGraphics::InitGraphics()
 {
+    if (!getHDC())
+        return;
+
     // calculate the minimal line width for the printer
     if ( isPrinter() )
     {
@@ -471,19 +474,38 @@ void WinSalGraphics::InitGraphics()
 
 void WinSalGraphics::DeInitGraphics()
 {
+    if (!getHDC())
+        return;
+
     // clear clip region
     SelectClipRgn( getHDC(), nullptr );
     // select default objects
     if ( mhDefPen )
+    {
         SelectPen( getHDC(), mhDefPen );
+        mhDefPen = nullptr;
+    }
     if ( mhDefBrush )
+    {
         SelectBrush( getHDC(), mhDefBrush );
+        mhDefBrush = nullptr;
+    }
     if ( mhDefFont )
+    {
         SelectFont( getHDC(), mhDefFont );
+        mhDefFont = nullptr;
+    }
 
     mpImpl->DeInit();
 }
 
+void WinSalGraphics::setHDC(HDC aNew)
+{
+    DeInitGraphics();
+    mhLocalDC = aNew;
+    InitGraphics();
+}
+
 HDC ImplGetCachedDC( sal_uLong nID, HBITMAP hBmp )
 {
     SalData*    pSalData = GetSalData();
@@ -637,6 +659,8 @@ WinSalGraphics::~WinSalGraphics()
 
     // delete cache data
     delete [] reinterpret_cast<BYTE*>(mpStdClipRgnData);
+
+    setHDC(nullptr);
 }
 
 SalGraphicsImpl* WinSalGraphics::GetImpl() const
diff --git a/vcl/win/gdi/salprn.cxx b/vcl/win/gdi/salprn.cxx
index 0c5057797682..9e20e0dbb4c9 100644
--- a/vcl/win/gdi/salprn.cxx
+++ b/vcl/win/gdi/salprn.cxx
@@ -1032,7 +1032,6 @@ static WinSalGraphics* ImplCreateSalPrnGraphics( HDC hDC )
     WinSalGraphics* pGraphics = new WinSalGraphics(WinSalGraphics::PRINTER, false, nullptr, /* CHECKME */ nullptr);
     pGraphics->SetLayout( SalLayoutFlags::NONE );
     pGraphics->setHDC(hDC);
-    pGraphics->InitGraphics();
     return pGraphics;
 }
 
@@ -1044,9 +1043,9 @@ static bool ImplUpdateSalPrnIC( WinSalInfoPrinter* pPrinter, const ImplJobSetup*
 
     if ( pPrinter->mpGraphics )
     {
-        pPrinter->mpGraphics->DeInitGraphics();
-        DeleteDC( pPrinter->mpGraphics->getHDC() );
+        assert(pPrinter->mpGraphics->getHDC() == pPrinter->mhDC);
         delete pPrinter->mpGraphics;
+        DeleteDC(pPrinter->mhDC);
     }
 
     pPrinter->mpGraphics = ImplCreateSalPrnGraphics( hNewDC );
@@ -1104,11 +1103,11 @@ WinSalInfoPrinter::~WinSalInfoPrinter()
 {
     if ( mpGraphics )
     {
-        mpGraphics->DeInitGraphics();
         // we get intermittent crashes on the Windows jenkins box around here, let us see if there is something weird about the DC
         SAL_WARN("vcl", "Graphics DC " << mpGraphics->getHDC());
-        DeleteDC( mpGraphics->getHDC() );
+        assert(mpGraphics->getHDC() == mhDC);
         delete mpGraphics;
+        DeleteDC(mhDC);
     }
 }
 
@@ -1372,12 +1371,7 @@ WinSalPrinter::~WinSalPrinter()
     HDC hDC = mhDC;
     if ( hDC )
     {
-        if ( mpGraphics )
-        {
-            mpGraphics->DeInitGraphics();
-            delete mpGraphics;
-        }
-
+        delete mpGraphics;
         DeleteDC( hDC );
     }
 
@@ -1535,12 +1529,8 @@ bool WinSalPrinter::EndJob()
     HDC hDC = mhDC;
     if ( isValid() && hDC )
     {
-        if ( mpGraphics )
-        {
-            mpGraphics->DeInitGraphics();
-            delete mpGraphics;
-            mpGraphics = nullptr;
-        }
+        delete mpGraphics;
+        mpGraphics = nullptr;
 
         // #i54419# Windows fax printer brings up a dialog in EndDoc
         // which text previously copied in soffice process can be
@@ -1601,17 +1591,13 @@ SalGraphics* WinSalPrinter::StartPage( ImplJobSetup* pSetupData, bool bNewJobDat
 
 void WinSalPrinter::EndPage()
 {
-    HDC hDC = mhDC;
-    if ( hDC && mpGraphics )
-    {
-        mpGraphics->DeInitGraphics();
-        delete mpGraphics;
-        mpGraphics = nullptr;
-    }
+    delete mpGraphics;
+    mpGraphics = nullptr;
 
     if( ! isValid() )
         return;
 
+    HDC hDC = mhDC;
     volatile int nRet = 0;
     CATCH_DRIVER_EX_BEGIN;
     nRet = ::EndPage( hDC );
diff --git a/vcl/win/gdi/salvd.cxx b/vcl/win/gdi/salvd.cxx
index 74e458dc7244..679a32a576cd 100644
--- a/vcl/win/gdi/salvd.cxx
+++ b/vcl/win/gdi/salvd.cxx
@@ -133,7 +133,6 @@ std::unique_ptr<SalVirtualDevice> WinSalInstance::CreateVirtualDevice( SalGraphi
         RealizePalette( hDC );
     }
 
-    pVirGraphics->InitGraphics();
     pVDev->setGraphics(pVirGraphics);
 
     return std::unique_ptr<SalVirtualDevice>(pVDev);
@@ -169,14 +168,16 @@ WinSalVirtualDevice::~WinSalVirtualDevice()
     if( *ppVirDev )
         *ppVirDev = mpNext;
 
-    // destroy saved DC
+    HDC hDC = mpGraphics->getHDC();
+    // restore the mpGraphics' original HDC values, so the HDC can be deleted in the !mbForeignDC case
+    mpGraphics->setHDC(nullptr);
+
     if( mpGraphics->getDefPal() )
-        SelectPalette( mpGraphics->getHDC(), mpGraphics->getDefPal(), TRUE );
-    mpGraphics->DeInitGraphics();
+        SelectPalette(hDC, mpGraphics->getDefPal(), TRUE);
     if( mhDefBmp )
-        SelectBitmap( mpGraphics->getHDC(), mhDefBmp );
+        SelectBitmap(hDC, mhDefBmp);
     if( !mbForeignDC )
-        DeleteDC( mpGraphics->getHDC() );
+        DeleteDC(hDC);
 }
 
 SalGraphics* WinSalVirtualDevice::AcquireGraphics()
diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx
index 89d1d10c6961..d38b0296f3e3 100644
--- a/vcl/win/window/salframe.cxx
+++ b/vcl/win/window/salframe.cxx
@@ -915,12 +915,11 @@ bool WinSalFrame::ReleaseFrameGraphicsDC( WinSalGraphics* pGraphics )
         return false;
     if ( pGraphics->getDefPal() )
         SelectPalette( hDC, pGraphics->getDefPal(), TRUE );
-    pGraphics->DeInitGraphics();
+    pGraphics->setHDC(nullptr);
     SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_RELEASEDC,
         reinterpret_cast<WPARAM>(mhWnd), reinterpret_cast<LPARAM>(hDC) );
     if ( pGraphics == mpThreadGraphics )
         pSalData->mnCacheDCInUse--;
-    pGraphics->setHDC(nullptr);
     return true;
 }
 
@@ -999,7 +998,6 @@ bool WinSalFrame::InitFrameGraphicsDC( WinSalGraphics *pGraphics, HDC hDC, HWND
         pGraphics->setDefPal(SelectPalette( hDC, pSalData->mhDitherPal, TRUE ));
         RealizePalette( hDC );
     }
-    pGraphics->InitGraphics();
 
     if ( pGraphics == mpThreadGraphics )
         pSalData->mnCacheDCInUse++;


More information about the Libreoffice-commits mailing list