tdf#118555 fix HFONT fallback handing / lifecycle

Instead of storing the never changing DC in the WinFontInstance
store the HFONT, which is Windows logical font instance.

Then set the correct HFONT instance from the layout when rendering
its text.

This also changes the HFONT ownership and lifecycle. The HFONT
is moved from the mhFonts to the WinFontInstance, if available,
so it has a proper referenced lifecycle. The mhFonts is still
needed, as embedded font just supply an HFONT and no
WinFontInstance.

Change-Id: Iba62281c710290276f004f0c0177e6d37c849d2c
Reviewed-on: https://gerrit.libreoffice.org/57101
Tested-by: Jenkins
Reviewed-by: Khaled Hosny <khaledhosny@eglug.org>

4 files changed, 90 insertions, 41 deletions

diff --git a/vcl/inc/win/salgdi.h b/vcl/inc/win/salgdi.h
index 6b2e464bb0a1..85b92946226e 100644
--- a/vcl/inc/win/salgdi.h
+++ b/vcl/inc/win/salgdi.h
@@ -156,6 +156,18 @@ private:
     bool                    mbScreen : 1;           // is Screen compatible
     HWND                    mhWnd;              // Window-Handle, when Window-Graphics
 
+    /** HFONT lifecycle
+     *
+     * The HFONT has to be shared between mhFonts and mpWinFontEntry.
+     * As mpWinFontEntry is reference counted and just freed in SetFont, the HFONT is
+     * transferred from mhFonts to the mpWinFontEntry.
+     *
+     * We need the mhFonts list, as embedded fonts don't have a corresponding WinFontInstance
+     * so for these there is just the mhFonts entry.
+     *
+     * The HFONT object can just be assigned to mhFonts _or_ mpWinFontEntry!
+     **/
+
     HFONT                   mhFonts[ MAX_FALLBACK ];        // Font + Fallbacks
     rtl::Reference<WinFontInstance>
                             mpWinFontEntry[ MAX_FALLBACK ]; // pointer to the most recent font instance
@@ -173,6 +185,7 @@ private:
 
     bool CacheGlyphs(const GenericSalLayout& rLayout);
     bool DrawCachedGlyphs(const GenericSalLayout& rLayout);
+    HFONT ImplDoSetFont(FontSelectPattern const * i_pFont, const PhysicalFontFace * i_pFontFace, float& o_rFontScale, HFONT& o_rOldFont);
 
 public:
     HDC getHDC() const { return mhLocalDC; }
@@ -198,7 +211,6 @@ public:
 
     HWND gethWnd();
 
-    HFONT                   ImplDoSetFont( FontSelectPattern const * i_pFont, const PhysicalFontFace * i_pFontFace, float& o_rFontScale, HFONT& o_rOldFont );
 
 public:
     explicit WinSalGraphics(WinSalGraphics::Type eType, bool bScreen, HWND hWnd,
diff --git a/vcl/inc/win/winlayout.hxx b/vcl/inc/win/winlayout.hxx
index d15b3d124472..0b7c23bfc9a5 100644
--- a/vcl/inc/win/winlayout.hxx
+++ b/vcl/inc/win/winlayout.hxx
@@ -148,21 +148,24 @@ class WinFontInstance : public LogicalFontInstance
     friend rtl::Reference<LogicalFontInstance> WinFontFace::CreateFontInstance(const FontSelectPattern&) const;
 
 public:
-    virtual                 ~WinFontInstance() override;
+    virtual ~WinFontInstance() override;
 
     bool CacheGlyphToAtlas(HDC hDC, int nGlyphIndex, SalGraphics& rGraphics);
     GlyphCache& GetGlyphCache() { return maGlyphCache; }
     bool hasHScale() const;
 
-    void SetHDC(const HDC);
+    void SetHFONT(const HFONT);
     HFONT GetHFONT() const { return m_hFont; }
 
+    // Prevend deletion of the HFONT in the WinFontInstance destructor
+    // Used for the ScopedFont handling
+    void UnsetHFONT() { m_hFont = nullptr; }
+
 private:
     explicit WinFontInstance(const PhysicalFontFace&, const FontSelectPattern&);
 
     virtual hb_font_t* ImplInitHbFont() override;
 
-    HDC m_hDC;
     HFONT m_hFont;
     GlyphCache maGlyphCache;
 };
diff --git a/vcl/win/gdi/salfont.cxx b/vcl/win/gdi/salfont.cxx
index 2796f8686255..5cf7f4354480 100644
--- a/vcl/win/gdi/salfont.cxx
+++ b/vcl/win/gdi/salfont.cxx
@@ -884,6 +884,7 @@ HFONT WinSalGraphics::ImplDoSetFont(FontSelectPattern const * i_pFont,
     if( hdcScreen )
     {
         // select font into screen hdc first to get an antialiased font
+        // and instantly restore the default font!
         // see knowledge base article 305290:
         // "PRB: Fonts Not Drawn Antialiased on Device Context for DirectDraw Surface"
         SelectFont( hdcScreen, SelectFont( hdcScreen , hNewFont ) );
@@ -915,18 +916,22 @@ void WinSalGraphics::SetFont( const FontSelectPattern* pFont, int nFallbackLevel
     if( !pFont )
     {
         // deselect still active font
-        if( mhDefFont )
-            ::SelectFont( getHDC(), mhDefFont );
+        if (mhDefFont)
+        {
+            ::SelectFont(getHDC(), mhDefFont);
+            mhDefFont = nullptr;
+        }
         mfCurrentFontScale = mfFontScale[nFallbackLevel];
         // release no longer referenced font handles
         for( int i = nFallbackLevel; i < MAX_FALLBACK; ++i )
         {
             if( mhFonts[i] )
+            {
                 ::DeleteFont( mhFonts[i] );
-            mhFonts[ i ] = nullptr;
+                mhFonts[ i ] = nullptr;
+            }
             mpWinFontEntry[i] = nullptr;
         }
-        mhDefFont = nullptr;
         return;
     }
 
@@ -953,25 +958,31 @@ void WinSalGraphics::SetFont( const FontSelectPattern* pFont, int nFallbackLevel
                 ::DeleteFont( mhFonts[i] );
                 mhFonts[i] = nullptr;
             }
-            // note: removing mpWinFontEntry[i] here has obviously bad effects
+            if (i > nFallbackLevel)
+                mpWinFontEntry[i] = nullptr;
         }
     }
 
     // store new font in correct layer
-    mhFonts[ nFallbackLevel ] = hNewFont;
-
-    // now the font is live => update font face
     if (mpWinFontEntry[nFallbackLevel])
     {
+        mpWinFontEntry[nFallbackLevel]->SetHFONT(hNewFont);
+        // now the font is live => update font face
         const WinFontFace* pFontFace = static_cast<const WinFontFace*>(mpWinFontEntry[nFallbackLevel]->GetFontFace());
         pFontFace->UpdateFromHDC(getHDC());
     }
+    else
+        mhFonts[ nFallbackLevel ] = hNewFont;
 }
 
 void WinSalGraphics::GetFontMetric( ImplFontMetricDataRef& rxFontMetric, int nFallbackLevel )
 {
     // temporarily change the HDC to the font in the fallback level
-    HFONT hOldFont = SelectFont( getHDC(), mhFonts[nFallbackLevel] );
+    const HFONT hFallbackFont = mhFonts[nFallbackLevel] ? mhFonts[nFallbackLevel]
+                                                        : mpWinFontEntry[nFallbackLevel]->GetHFONT();
+    assert((mhFonts[nFallbackLevel] && !mpWinFontEntry[nFallbackLevel]) ||
+           (!mhFonts[nFallbackLevel] && mpWinFontEntry[nFallbackLevel]));
+    const HFONT hOldFont = SelectFont(getHDC(), hFallbackFont);
 
     wchar_t aFaceName[LF_FACESIZE+60];
     if( GetTextFaceW( getHDC(), SAL_N_ELEMENTS(aFaceName), aFaceName ) )
@@ -982,8 +993,21 @@ void WinSalGraphics::GetFontMetric( ImplFontMetricDataRef& rxFontMetric, int nFa
     const RawFontData aHheaRawData(getHDC(), nHheaTag);
     const RawFontData aOS2RawData(getHDC(), nOS2Tag);
 
-    mpWinFontEntry[nFallbackLevel]->SetHDC(getHDC());
-    rxFontMetric->SetMinKashida(mpWinFontEntry[nFallbackLevel]->GetKashidaWidth());
+    if (mpWinFontEntry[nFallbackLevel])
+        rxFontMetric->SetMinKashida(mpWinFontEntry[nFallbackLevel]->GetKashidaWidth());
+    else
+    {
+        // Calculate Kashida width without mpWinFontEntry for embedded fonts
+        WCHAR nKashidaCh = 0x0640;
+        WORD nKashidaGid;
+        DWORD ret = GetGlyphIndicesW(getHDC(), &nKashidaCh, 1, &nKashidaGid, GGI_MARK_NONEXISTING_GLYPHS);
+        if (ret != GDI_ERROR && nKashidaGid != 0xFFFF)
+        {
+            int nKashidaWidth = 0;
+            if (GetCharWidthI(getHDC(), nKashidaGid, 1, nullptr, &nKashidaWidth))
+                rxFontMetric->SetMinKashida(static_cast<int>(mfFontScale[nFallbackLevel] * nKashidaWidth));
+        }
+    }
 
     // get the font metric
     OUTLINETEXTMETRICW aOutlineMetric;
@@ -1580,8 +1604,16 @@ private:
 
 ScopedFont::ScopedFont(WinSalGraphics & rData): m_rData(rData)
 {
-    m_hOrigFont = m_rData.mhFonts[0];
-    m_rData.mhFonts[0] = nullptr; // avoid deletion of current font
+    if (m_rData.mpWinFontEntry[0])
+    {
+        m_hOrigFont = m_rData.mpWinFontEntry[0]->GetHFONT();
+        m_rData.mpWinFontEntry[0]->UnsetHFONT();
+    }
+    else
+    {
+        m_hOrigFont = m_rData.mhFonts[0];
+        m_rData.mhFonts[0] = nullptr; // avoid deletion of current font
+    }
 }
 
 ScopedFont::~ScopedFont()
@@ -1590,7 +1622,10 @@ ScopedFont::~ScopedFont()
     {
         // restore original font, destroy temporary font
         HFONT hTempFont = m_rData.mhFonts[0];
-        m_rData.mhFonts[0] = m_hOrigFont;
+        if (m_rData.mpWinFontEntry[0])
+            m_rData.mpWinFontEntry[0]->SetHFONT(m_hOrigFont);
+        else
+            m_rData.mhFonts[0] = m_hOrigFont;
         SelectObject( m_rData.getHDC(), m_hOrigFont );
         DeleteObject( hTempFont );
     }
diff --git a/vcl/win/gdi/winlayout.cxx b/vcl/win/gdi/winlayout.cxx
index 8af8bbd84167..0cd916952b3e 100644
--- a/vcl/win/gdi/winlayout.cxx
+++ b/vcl/win/gdi/winlayout.cxx
@@ -289,18 +289,20 @@ std::unique_ptr<SalLayout> WinSalGraphics::GetTextLayout(ImplLayoutArgs& /*rArgs
 
     assert(mpWinFontEntry[nFallbackLevel]->GetFontFace());
 
-    mpWinFontEntry[nFallbackLevel]->SetHDC(getHDC());
     GenericSalLayout *aLayout = new GenericSalLayout(*mpWinFontEntry[nFallbackLevel]);
     return std::unique_ptr<SalLayout>(aLayout);
 }
 
 WinFontInstance::WinFontInstance(const PhysicalFontFace& rPFF, const FontSelectPattern& rFSP)
     : LogicalFontInstance(rPFF, rFSP)
+    , m_hFont(nullptr)
 {
 }
 
 WinFontInstance::~WinFontInstance()
 {
+    if (m_hFont)
+        ::DeleteFont(m_hFont);
 }
 
 bool WinFontInstance::hasHScale() const
@@ -336,8 +338,7 @@ static hb_blob_t* getFontTable(hb_face_t* /*face*/, hb_tag_t nTableTag, void* pU
 
 hb_font_t* WinFontInstance::ImplInitHbFont()
 {
-    assert(m_hDC);
-    m_hFont = static_cast<HFONT>(GetCurrentObject(m_hDC, OBJ_FONT));
+    assert(m_hFont);
     hb_font_t* pHbFont = InitHbFont(hb_face_create_for_tables(getFontTable, m_hFont, nullptr));
 
     // Calculate the AverageWidthFactor, see LogicalFontInstance::GetScale().
@@ -355,11 +356,13 @@ hb_font_t* WinFontInstance::ImplInitHbFont()
 
         // Get the font metrics.
         HFONT hNewFont = CreateFontIndirectW(&aLogFont);
-        HFONT hOldFont = static_cast<HFONT>(SelectObject(m_hDC, hNewFont));
+        HDC hDC = GetDC(nullptr);
+        HGDIOBJ hOrigFont = SelectObject(hDC, hNewFont);
         TEXTMETRICW aFontMetric;
-        GetTextMetricsW(m_hDC, &aFontMetric);
-        SelectObject(m_hDC, hOldFont);
+        GetTextMetricsW(hDC, &aFontMetric);
+        SelectObject(hDC, hOrigFont);
         DeleteObject(hNewFont);
+        ReleaseDC(nullptr, hDC);
 
         SetAverageWidthFactor(nUPEM / aFontMetric.tmAveCharWidth);
     }
@@ -367,12 +370,12 @@ hb_font_t* WinFontInstance::ImplInitHbFont()
     return pHbFont;
 }
 
-void WinFontInstance::SetHDC(const HDC hDC)
+void WinFontInstance::SetHFONT(const HFONT hFont)
 {
-    if (m_hDC == hDC)
-        return;
     ReleaseHbFont();
-    m_hDC = hDC;
+    if (m_hFont)
+        ::DeleteFont(m_hFont);
+    m_hFont = hFont;
 }
 
 bool WinSalGraphics::CacheGlyphs(const GenericSalLayout& rLayout)
@@ -448,25 +451,23 @@ void WinSalGraphics::DrawTextLayout(const GenericSalLayout& rLayout)
 {
     HDC hDC = getHDC();
 
-    HFONT hFont = static_cast<const WinFontInstance*>(&rLayout.GetFont())->GetHFONT();
-    HGDIOBJ hOrigFont = SelectObject(hDC, hFont);
+    const WinFontInstance* pWinFont = static_cast<const WinFontInstance*>(&rLayout.GetFont());
+    const HFONT hLayoutFont = pWinFont->GetHFONT();
 
     // Our DirectWrite renderer is incomplete, skip it for non-horizontal or
     // stretched text.
-    bool bForceGDI = rLayout.GetOrientation() || static_cast<const WinFontInstance*>(&rLayout.GetFont())->hasHScale();
+    bool bForceGDI = rLayout.GetOrientation() || pWinFont->hasHScale();
 
     bool bUseOpenGL = OpenGLHelper::isVCLOpenGLEnabled() && !mbPrinter;
     if (!bUseOpenGL)
     {
         // no OpenGL, just classic rendering
+        const HFONT hOrigFont = ::SelectFont(hDC, hLayoutFont);
         DrawTextLayout(rLayout, hDC, false);
+        ::SelectFont(hDC, hOrigFont);
     }
-    else if (!bForceGDI && CacheGlyphs(rLayout) &&
-             DrawCachedGlyphs(rLayout))
-    {
-        // Nothing
-    }
-    else
+    // if we can't draw the cached OpenGL glyphs, try to draw a full OpenGL layout
+    else if (bForceGDI || !CacheGlyphs(rLayout) || !DrawCachedGlyphs(rLayout))
     {
         // We have to render the text to a hidden texture, and draw it.
         //
@@ -516,7 +517,7 @@ void WinSalGraphics::DrawTextLayout(const GenericSalLayout& rLayout)
 
             // setup the hidden DC with black color and white background, we will
             // use the result of the text drawing later as a mask only
-            HFONT hOFont = ::SelectFont(aDC.getCompatibleHDC(), hFont);
+            const HFONT hOrigFont = ::SelectFont(aDC.getCompatibleHDC(), hLayoutFont);
 
             ::SetTextColor(aDC.getCompatibleHDC(), RGB(0, 0, 0));
             ::SetBkColor(aDC.getCompatibleHDC(), RGB(255, 255, 255));
@@ -534,13 +535,11 @@ void WinSalGraphics::DrawTextLayout(const GenericSalLayout& rLayout)
             if (xTexture)
                 pImpl->DrawMask(*xTexture, salColor, aDC.getTwoRect());
 
-            ::SelectFont(aDC.getCompatibleHDC(), hOFont);
+            ::SelectFont(aDC.getCompatibleHDC(), hOrigFont);
 
             pImpl->PostDraw();
         }
     }
-
-    SelectObject(hDC, hOrigFont);
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */