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

Jan-Marek Glogowski (via logerrit) logerrit at kemper.freedesktop.org
Mon Sep 27 09:46:52 UTC 2021


 include/vcl/print.hxx    |    1 
 vcl/inc/win/salprn.h     |   13 +++++++----
 vcl/source/gdi/print.cxx |   15 +++++-------
 vcl/win/gdi/salprn.cxx   |   55 ++++++++++++++++++++++++-----------------------
 4 files changed, 46 insertions(+), 38 deletions(-)

New commits:
commit 1b7c53db87bb67eeb2591fbb186f7ac20eb00c68
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Mon Sep 20 17:55:37 2021 +0200
Commit:     Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Mon Sep 27 11:46:18 2021 +0200

    WIN lazy init WinSalInfoPrinter graphics
    
    ... and while at it make stuff private and add _ to the newly
    private member variables.
    
    The new assert revealed a bug in the SalGraphics refcounting,
    because the virtual ReleaseGraphics is called from the
    destructor, which was probably also the reason for the HACK
    and comment in Printer::ImplReleaseFonts.
    
    Change-Id: I7af0bda19be6810dd8c0ea5b74604381e2047407
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122371
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>

diff --git a/include/vcl/print.hxx b/include/vcl/print.hxx
index e487e8aa82b5..b6ea7000436c 100644
--- a/include/vcl/print.hxx
+++ b/include/vcl/print.hxx
@@ -124,6 +124,7 @@ public:
 protected:
     virtual bool                AcquireGraphics() const override;
     virtual void                ReleaseGraphics( bool bRelease = true ) override;
+    void ImplReleaseGraphics(bool bRelease = true);
     virtual void                ImplReleaseFonts() override;
 
     virtual tools::Long                GetGradientStepCount( tools::Long nMinRect ) override;
diff --git a/vcl/inc/win/salprn.h b/vcl/inc/win/salprn.h
index 209f064fd157..43eaa520c23d 100644
--- a/vcl/inc/win/salprn.h
+++ b/vcl/inc/win/salprn.h
@@ -40,19 +40,24 @@ struct SalDriverData
 
 class WinSalGraphics;
 
-class WinSalInfoPrinter : public SalInfoPrinter
+class WinSalInfoPrinter final : public SalInfoPrinter
 {
 public:
-    WinSalGraphics*        mpGraphics;             // current Printer graphics
     OUString               maDriverName;           // printer driver name
     OUString               maDeviceName;           // printer device name
     OUString               maPortName;             // printer port name
-    HDC                    mhDC;                   // printer hdc
-    bool                   mbGraphics;             // is Graphics used
+
+private:
+    HDC m_hDC;                    ///< printer hdc
+    WinSalGraphics* m_pGraphics;  ///< current Printer graphics
+    bool m_bGraphics;             ///< is Graphics used
+
 public:
     WinSalInfoPrinter();
     virtual ~WinSalInfoPrinter() override;
 
+    void setHDC(HDC);
+
     virtual SalGraphics*            AcquireGraphics() override;
     virtual void                    ReleaseGraphics( SalGraphics* pGraphics ) override;
     virtual bool                    Setup( weld::Window* pFrame, ImplJobSetup* pSetupData ) override;
diff --git a/vcl/source/gdi/print.cxx b/vcl/source/gdi/print.cxx
index 36aa93f6ec37..df685efbcb13 100644
--- a/vcl/source/gdi/print.cxx
+++ b/vcl/source/gdi/print.cxx
@@ -550,13 +550,7 @@ bool Printer::AcquireGraphics() const
 
 void Printer::ImplReleaseFonts()
 {
-#ifdef UNX
-    // HACK to fix an urgent P1 printing issue fast
-    // WinSalPrinter does not respect GetGraphics/ReleaseGraphics conventions
-    // so Printer::mpGraphics often points to a dead WinSalGraphics
-    // TODO: fix WinSalPrinter's GetGraphics/ReleaseGraphics handling
     mpGraphics->ReleaseFonts();
-#endif
     mbNewFont = true;
     mbInitFont = true;
 
@@ -565,7 +559,7 @@ void Printer::ImplReleaseFonts()
     mpDeviceFontSizeList.reset();
 }
 
-void Printer::ReleaseGraphics( bool bRelease )
+void Printer::ImplReleaseGraphics(bool bRelease)
 {
     DBG_TESTSOLARMUTEX();
 
@@ -618,6 +612,11 @@ void Printer::ReleaseGraphics( bool bRelease )
     mpNextGraphics  = nullptr;
 }
 
+void Printer::ReleaseGraphics(bool bRelease)
+{
+    ImplReleaseGraphics(bRelease);
+}
+
 void Printer::ImplInit( SalPrinterQueueInfo* pInfo )
 {
     ImplSVData* pSVData = ImplGetSVData();
@@ -912,7 +911,7 @@ void Printer::dispose()
 
     mpPrinterOptions.reset();
 
-    ReleaseGraphics();
+    ImplReleaseGraphics();
     if ( mpInfoPrinter )
         ImplGetSVData()->mpDefInst->DestroyInfoPrinter( mpInfoPrinter );
     if ( mpDisplayDev )
diff --git a/vcl/win/gdi/salprn.cxx b/vcl/win/gdi/salprn.cxx
index 19e61d08ed03..21e779eb6156 100644
--- a/vcl/win/gdi/salprn.cxx
+++ b/vcl/win/gdi/salprn.cxx
@@ -1041,16 +1041,7 @@ static bool ImplUpdateSalPrnIC( WinSalInfoPrinter* pPrinter, const ImplJobSetup*
     if ( !hNewDC )
         return false;
 
-    if ( pPrinter->mpGraphics )
-    {
-        assert(pPrinter->mpGraphics->getHDC() == pPrinter->mhDC);
-        delete pPrinter->mpGraphics;
-        DeleteDC(pPrinter->mhDC);
-    }
-
-    pPrinter->mpGraphics = ImplCreateSalPrnGraphics( hNewDC );
-    pPrinter->mhDC      = hNewDC;
-
+    pPrinter->setHDC(hNewDC);
     return true;
 }
 
@@ -1075,8 +1066,7 @@ SalInfoPrinter* WinSalInstance::CreateInfoPrinter( SalPrinterQueueInfo* pQueueIn
         return nullptr;
     }
 
-    pPrinter->mpGraphics = ImplCreateSalPrnGraphics( hDC );
-    pPrinter->mhDC      = hDC;
+    pPrinter->setHDC(hDC);
     if ( !pSetupData->GetDriverData() )
         ImplUpdateSalJobSetup( pPrinter, pSetupData, false, nullptr );
     ImplDevModeToJobSetup( pPrinter, pSetupData, JobSetFlags::ALL );
@@ -1092,23 +1082,33 @@ void WinSalInstance::DestroyInfoPrinter( SalInfoPrinter* pPrinter )
 
 
 WinSalInfoPrinter::WinSalInfoPrinter() :
-    mpGraphics( nullptr ),
-    mhDC( nullptr ),
-    mbGraphics( false )
+    m_hDC(nullptr),
+    m_pGraphics(nullptr),
+    m_bGraphics(false)
 {
     m_bPapersInit = false;
 }
 
 WinSalInfoPrinter::~WinSalInfoPrinter()
 {
-    if ( mpGraphics )
+    setHDC(nullptr);
+}
+
+void WinSalInfoPrinter::setHDC(HDC hNewDC)
+{
+    assert(!m_bGraphics);
+
+    if (m_hDC)
     {
+        assert(!m_pGraphics || m_hDC == m_pGraphics->getHDC());
         // 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());
-        assert(mpGraphics->getHDC() == mhDC);
-        delete mpGraphics;
-        DeleteDC(mhDC);
+        SAL_WARN_IF(!hNewDC, "vcl", "Graphics DC " << m_hDC);
+        delete m_pGraphics;
+        m_pGraphics = nullptr;
+        DeleteDC(m_hDC);
     }
+
+    m_hDC = hNewDC;
 }
 
 void WinSalInfoPrinter::InitPaperFormats( const ImplJobSetup* pSetupData )
@@ -1149,18 +1149,21 @@ int WinSalInfoPrinter::GetLandscapeAngle( const ImplJobSetup* pSetupData )
 
 SalGraphics* WinSalInfoPrinter::AcquireGraphics()
 {
-    if ( mbGraphics )
+    assert(m_hDC);
+    if (m_bGraphics)
         return nullptr;
 
-    if ( mpGraphics )
-        mbGraphics = true;
+    if (!m_pGraphics)
+        m_pGraphics = ImplCreateSalPrnGraphics(m_hDC);
+    if (m_pGraphics)
+        m_bGraphics = true;
 
-    return mpGraphics;
+    return m_pGraphics;
 }
 
 void WinSalInfoPrinter::ReleaseGraphics( SalGraphics* )
 {
-    mbGraphics = false;
+    m_bGraphics = false;
 }
 
 bool WinSalInfoPrinter::Setup(weld::Window* pFrame, ImplJobSetup* pSetupData)
@@ -1266,7 +1269,7 @@ void WinSalInfoPrinter::GetPageInfo( const ImplJobSetup*,
                                   Point& rPageOffset,
                                   Size& rPaperSize )
 {
-    HDC hDC = mhDC;
+    HDC hDC = m_hDC;
 
     rOutWidth   = GetDeviceCaps( hDC, HORZRES );
     rOutHeight  = GetDeviceCaps( hDC, VERTRES );


More information about the Libreoffice-commits mailing list