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.

This patch also includes the follow up patches:

WIN GetFontMetric doesn't matter for GetEmbedFontData
(cherry picked from commit b444422244a2eb8d558499d2ffdb2cca5ddb44f3)

Revert "tdf#117517: Fix OpenGL text rendering on Windows"
(cherry picked from commit ca4e75d694a5fb41a1c800146319aa6ba34d8bab)

Conflicts:
	vcl/win/gdi/salfont.cxx

Reviewed-on: https://gerrit.libreoffice.org/57101
Tested-by: Jenkins
Reviewed-by: Khaled Hosny <khaledhosny@eglug.org>
(cherry picked from commit fad862e290d727fc9fefe206f6e4b807482c4175)

Change-Id: Iba62281c710290276f004f0c0177e6d37c849d2c
Reviewed-on: https://gerrit.libreoffice.org/57234
Tested-by: Jenkins
Reviewed-by: Khaled Hosny <khaledhosny@eglug.org>
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>

4 files changed, 100 insertions, 45 deletions

diff --git a/vcl/inc/win/salgdi.h b/vcl/inc/win/salgdi.h
index f2580aa05d5d..fe4f4b8dc8eb 100644
--- a/vcl/inc/win/salgdi.h
+++ b/vcl/inc/win/salgdi.h
@@ -157,6 +157,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
     WinFontInstance*       mpWinFontEntry[ MAX_FALLBACK ]; // pointer to the most recent font instance
     float                   mfFontScale[ MAX_FALLBACK ];        // allows metrics emulation of huge font sizes
@@ -175,6 +187,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; }
@@ -200,7 +213,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 a3229a4010b6..2e22834c891e 100644
--- a/vcl/inc/win/winlayout.hxx
+++ b/vcl/inc/win/winlayout.hxx
@@ -148,21 +148,24 @@ class WinFontInstance : public LogicalFontInstance
     friend LogicalFontInstance* WinFontFace::CreateFontInstance(const FontSelectPattern&) const;
 
 public:
-    virtual                 ~WinFontInstance() override;
+    virtual ~WinFontInstance() override;
 
-    bool CacheGlyphToAtlas(HDC hDC, int nGlyphIndex, SalGraphics& rGraphics);
+    bool CacheGlyphToAtlas(HDC hDC, HFONT hFont, 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 2d6512154e2a..ae0042706e04 100644
--- a/vcl/win/gdi/salfont.cxx
+++ b/vcl/win/gdi/salfont.cxx
@@ -889,6 +889,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 ) );
@@ -920,20 +921,26 @@ 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;
+            }
             if (mpWinFontEntry[i])
+            {
                 GetWinFontEntry(i)->Release();
-            mpWinFontEntry[i] = nullptr;
+                mpWinFontEntry[i] = nullptr;
+            }
         }
-        mhDefFont = nullptr;
         return;
     }
 
@@ -968,25 +975,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])
+            {
+                GetWinFontEntry(i)->Release();
+                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] );
+    assert(!mhFonts[nFallbackLevel] && mpWinFontEntry[nFallbackLevel]);
+    const HFONT hOldFont = SelectFont(getHDC(), mpWinFontEntry[nFallbackLevel]->GetHFONT());
 
     wchar_t aFaceName[LF_FACESIZE+60];
     if( GetTextFaceW( getHDC(), SAL_N_ELEMENTS(aFaceName), aFaceName ) )
@@ -997,7 +1010,6 @@ 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());
 
     // get the font metric
@@ -1590,8 +1602,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()
@@ -1600,7 +1620,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 95034c97372b..392a6abef02a 100644
--- a/vcl/win/gdi/winlayout.cxx
+++ b/vcl/win/gdi/winlayout.cxx
@@ -55,10 +55,24 @@ GlobalGlyphCache * GlobalGlyphCache::get() {
     return data->m_pGlobalGlyphCache.get();
 }
 
-bool WinFontInstance::CacheGlyphToAtlas(HDC hDC, int nGlyphIndex, SalGraphics& rGraphics)
+bool WinFontInstance::CacheGlyphToAtlas(HDC hDC, HFONT hFont, int nGlyphIndex, SalGraphics& rGraphics)
 {
     OpenGLGlyphDrawElement aElement;
 
+    ScopedHDC aHDC(CreateCompatibleDC(hDC));
+
+    if (!aHDC)
+    {
+        SAL_WARN("vcl.gdi", "CreateCompatibleDC failed: " << WindowsErrorString(GetLastError()));
+        return false;
+    }
+    HFONT hOrigFont = static_cast<HFONT>(SelectObject(aHDC.get(), hFont));
+    if (hOrigFont == nullptr)
+    {
+        SAL_WARN("vcl.gdi", "SelectObject failed: " << WindowsErrorString(GetLastError()));
+        return false;
+    }
+
     // For now we assume DWrite is present and we won't bother with fallback paths.
     D2DWriteTextOutRenderer * pTxt = dynamic_cast<D2DWriteTextOutRenderer *>(&TextOutRenderer::get(true));
     if (!pTxt)
@@ -66,7 +80,7 @@ bool WinFontInstance::CacheGlyphToAtlas(HDC hDC, int nGlyphIndex, SalGraphics& r
 
     pTxt->changeTextAntiAliasMode(D2DTextAntiAliasMode::AntiAliased);
 
-    if (!pTxt->BindFont(hDC))
+    if (!pTxt->BindFont(aHDC.get()))
     {
         SAL_WARN("vcl.gdi", "Binding of font failed. The font might not be supported by DirectWrite.");
         return false;
@@ -179,6 +193,7 @@ bool WinFontInstance::CacheGlyphToAtlas(HDC hDC, int nGlyphIndex, SalGraphics& r
         break;
     default:
         SAL_WARN("vcl.gdi", "DrawGlyphRun-EndDraw failed: " << WindowsErrorString(GetLastError()));
+        SelectFont(aDC.getCompatibleHDC(), hOrigFont);
         return false;
     }
 
@@ -191,6 +206,8 @@ bool WinFontInstance::CacheGlyphToAtlas(HDC hDC, int nGlyphIndex, SalGraphics& r
 
     maGlyphCache.PutDrawElementInCache(aElement, nGlyphIndex);
 
+    SelectFont(aDC.getCompatibleHDC(), hOrigFont);
+
     return true;
 }
 
@@ -289,7 +306,6 @@ 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);
 }
@@ -301,11 +317,14 @@ LogicalFontInstance * WinSalGraphics::GetWinFontEntry(int const nFallbackLevel)
 
 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
@@ -341,8 +360,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().
@@ -360,11 +378,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);
     }
@@ -372,12 +392,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)
@@ -388,6 +408,7 @@ bool WinSalGraphics::CacheGlyphs(const GenericSalLayout& rLayout)
 
     HDC hDC = getHDC();
     WinFontInstance& rFont = *static_cast<WinFontInstance*>(&rLayout.GetFont());
+    HFONT hFONT = rFont.GetHFONT();
 
     int nStart = 0;
     Point aPos(0, 0);
@@ -396,7 +417,7 @@ bool WinSalGraphics::CacheGlyphs(const GenericSalLayout& rLayout)
     {
         if (!rFont.GetGlyphCache().IsGlyphCached(pGlyph->maGlyphId))
         {
-            if (!rFont.CacheGlyphToAtlas(hDC, pGlyph->maGlyphId, *this))
+            if (!rFont.CacheGlyphToAtlas(hDC, hFONT, pGlyph->maGlyphId, *this))
                 return false;
         }
     }
@@ -453,25 +474,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.
         //
@@ -521,7 +540,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));
@@ -539,13 +558,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: */