Move PhysicalFontFace member of FontSelectPattern

A FontSelectPattern describes a general font request. It can be
used to find the best matching LogicalFontInstance. The instance
will be created based on a PhysicalFontFace, which is really a
factory since commit 8b700794b2746070814e9ff416ecd7bbb1c902e7.
Following this workflow, this moves the PhysicalFontFace pointer
to the instance and makes it constant.

Which leaves some special symbol font handling code in the hash
and instance lookup code path. It used to query the font face
directly from the instance.
I'm not sure of the correct handling. The related commits where
made to fix #i89002#, which has an attached test document.

1. commit 849f618270da313f9339dda29a9f35938434c91d
2. commit 8c9823d311fdf8092cc75873e4565325d204a658

The document is as broken as it was before the patch. The symbol
substitution still works, but the 'Q's are missing when displaying
a symbol font.

I also don't understand all the reinterpret_casts for fake font
ids. I guess this was used to prevent the crashes I see, where a
PhysicalFontFace referenced in a valid LogicalFontInstance is
freed and a later FontId check in the GlyphCache crashes. So this
now checks for a valid cache instead.

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

4 files changed, 43 insertions, 38 deletions

diff --git a/vcl/win/gdi/salfont.cxx b/vcl/win/gdi/salfont.cxx
index 11cc5e2d9115..2c96edc91fbe 100644
--- a/vcl/win/gdi/salfont.cxx
+++ b/vcl/win/gdi/salfont.cxx
@@ -52,6 +52,7 @@
 #include <sft.hxx>
 #include <win/saldata.hxx>
 #include <win/salgdi.h>
+#include <win/winlayout.hxx>
 #include <impfontcharmap.hxx>
 #include <impfontmetricdata.hxx>
 
@@ -182,7 +183,7 @@ bool WinGlyphFallbackSubstititution::HasMissingChars(PhysicalFontFace* pFace, OU
         const FontSelectPattern aFSD( *pFace, aSize, static_cast<float>(aSize.Height()), 0, false );
         // construct log font
         LOGFONTW aLogFont;
-        ImplGetLogFontFromFontSelect( mhDC, &aFSD, aLogFont, true );
+        ImplGetLogFontFromFontSelect( mhDC, &aFSD, pFace, aLogFont );
 
         // create HFONT from log font
         HFONT hNewFont = ::CreateFontIndirectW( &aLogFont );
@@ -757,12 +758,15 @@ int CALLBACK SalEnumQueryFontProcExW( const LOGFONTW*,
 
 void ImplGetLogFontFromFontSelect( HDC hDC,
                                    const FontSelectPattern* pFont,
-                                   LOGFONTW& rLogFont,
-                                   bool /*bTestVerticalAvail*/ )
+                                   const PhysicalFontFace* pFontFace,
+                                   LOGFONTW& rLogFont )
 {
-    OUString   aName;
-    if ( pFont->mpFontData )
-        aName = pFont->mpFontData->GetFamilyName();
+    OUString aName;
+    if (!pFontFace && pFont->mpFontInstance)
+        pFontFace = pFont->mpFontInstance->GetFontFace();
+
+    if (pFontFace)
+        aName = pFontFace->GetFamilyName();
     else
         aName = pFont->GetFamilyName().getToken( 0, ';' );
 
@@ -772,17 +776,17 @@ void ImplGetLogFontFromFontSelect( HDC hDC,
     memcpy( rLogFont.lfFaceName, aName.getStr(), nNameLen*sizeof( wchar_t ) );
     rLogFont.lfFaceName[nNameLen] = 0;
 
-    if( !pFont->mpFontData )
+    if  (pFontFace)
     {
-        rLogFont.lfCharSet = pFont->IsSymbolFont() ? SYMBOL_CHARSET : DEFAULT_CHARSET;
-        rLogFont.lfPitchAndFamily = ImplPitchToWin( pFont->GetPitch() )
-                                  | ImplFamilyToWin( pFont->GetFamilyType() );
+        const WinFontFace* pWinFontData = static_cast<const WinFontFace*>(pFontFace);
+        rLogFont.lfCharSet = pWinFontData->GetCharSet();
+        rLogFont.lfPitchAndFamily = pWinFontData->GetPitchAndFamily();
     }
     else
     {
-        const WinFontFace* pWinFontData = static_cast<const WinFontFace*>( pFont->mpFontData );
-        rLogFont.lfCharSet        = pWinFontData->GetCharSet();
-        rLogFont.lfPitchAndFamily = pWinFontData->GetPitchAndFamily();
+        rLogFont.lfCharSet = pFont->IsSymbolFont() ? SYMBOL_CHARSET : DEFAULT_CHARSET;
+        rLogFont.lfPitchAndFamily = ImplPitchToWin( pFont->GetPitch() )
+                                  | ImplFamilyToWin( pFont->GetFamilyType() );
     }
 
     static BYTE nDefaultQuality = NONANTIALIASED_QUALITY;
@@ -838,7 +842,10 @@ void ImplGetLogFontFromFontSelect( HDC hDC,
     }
 }
 
-HFONT WinSalGraphics::ImplDoSetFont(FontSelectPattern const * i_pFont, float& o_rFontScale, HFONT& o_rOldFont)
+HFONT WinSalGraphics::ImplDoSetFont(FontSelectPattern const * i_pFont,
+                                    const PhysicalFontFace * i_pFontFace,
+                                    float& o_rFontScale,
+                                    HFONT& o_rOldFont)
 {
     // clear the cache on font change
     g_BoundRectCache.clear();
@@ -850,7 +857,7 @@ HFONT WinSalGraphics::ImplDoSetFont(FontSelectPattern const * i_pFont, float& o_
         hdcScreen = GetDC(nullptr);
 
     LOGFONTW aLogFont;
-    ImplGetLogFontFromFontSelect( getHDC(), i_pFont, aLogFont, true );
+    ImplGetLogFontFromFontSelect( getHDC(), i_pFont, i_pFontFace, aLogFont );
 
     // #i47675# limit font requests to MAXFONTHEIGHT
     // TODO: share MAXFONTHEIGHT font instance
@@ -917,17 +924,13 @@ void WinSalGraphics::SetFont( const FontSelectPattern* pFont, int nFallbackLevel
                 ::DeleteFont( mhFonts[i] );
             mhFonts[ i ] = nullptr;
             if (mpWinFontEntry[i])
-            {
                 GetWinFontEntry(i)->Release();
-            }
             mpWinFontEntry[i] = nullptr;
-            mpWinFontData[i] = nullptr;
         }
         mhDefFont = nullptr;
         return;
     }
 
-    assert(pFont->mpFontData);
     if (mpWinFontEntry[nFallbackLevel])
     {
         GetWinFontEntry(nFallbackLevel)->Release();
@@ -939,10 +942,9 @@ void WinSalGraphics::SetFont( const FontSelectPattern* pFont, int nFallbackLevel
         pFont->mpFontInstance->Acquire();
     }
     mpWinFontEntry[ nFallbackLevel ] = reinterpret_cast<WinFontInstance*>( pFont->mpFontInstance );
-    mpWinFontData[ nFallbackLevel ] = static_cast<const WinFontFace*>( pFont->mpFontData );
 
     HFONT hOldFont = nullptr;
-    HFONT hNewFont = ImplDoSetFont(pFont, mfFontScale[ nFallbackLevel ], hOldFont);
+    HFONT hNewFont = ImplDoSetFont(pFont, nullptr, mfFontScale[ nFallbackLevel ], hOldFont);
     mfCurrentFontScale = mfFontScale[nFallbackLevel];
 
     if( !mhDefFont )
@@ -966,9 +968,13 @@ void WinSalGraphics::SetFont( const FontSelectPattern* pFont, int nFallbackLevel
 
     // store new font in correct layer
     mhFonts[ nFallbackLevel ] = hNewFont;
+
     // now the font is live => update font face
-    if( mpWinFontData[ nFallbackLevel ] )
-        mpWinFontData[ nFallbackLevel ]->UpdateFromHDC( getHDC() );
+    if (mpWinFontEntry[nFallbackLevel])
+    {
+        const WinFontFace* pFontFace = static_cast<const WinFontFace*>(mpWinFontEntry[nFallbackLevel]->GetFontFace());
+        pFontFace->UpdateFromHDC(getHDC());
+    }
 }
 
 void WinSalGraphics::GetFontMetric( ImplFontMetricDataRef& rxFontMetric, int nFallbackLevel )
@@ -1023,19 +1029,19 @@ void WinSalGraphics::GetFontMetric( ImplFontMetricDataRef& rxFontMetric, int nFa
 
 const FontCharMapRef WinSalGraphics::GetFontCharMap() const
 {
-    if( !mpWinFontData[0] )
+    if (!mpWinFontEntry[0])
     {
         FontCharMapRef xDefFontCharMap( new FontCharMap() );
         return xDefFontCharMap;
     }
-    return mpWinFontData[0]->GetFontCharMap();
+    return static_cast<const WinFontFace*>(mpWinFontEntry[0]->GetFontFace())->GetFontCharMap();
 }
 
 bool WinSalGraphics::GetFontCapabilities(vcl::FontCapabilities &rFontCapabilities) const
 {
-    if( !mpWinFontData[0] )
+    if (!mpWinFontEntry[0])
         return false;
-    return mpWinFontData[0]->GetFontCapabilities(rFontCapabilities);
+    return static_cast<const WinFontFace*>(mpWinFontEntry[0]->GetFontFace())->GetFontCapabilities(rFontCapabilities);
 }
 
 int CALLBACK SalEnumFontsProcExW( const LOGFONTW* lpelfe,
@@ -1644,9 +1650,9 @@ bool WinSalGraphics::CreateFontSubset( const OUString& rToFile,
     ScopedFont aOldFont(*this);
     float fScale = 1.0;
     HFONT hOldFont = nullptr;
-    ImplDoSetFont(&aIFSD, fScale, hOldFont);
+    ImplDoSetFont(&aIFSD, pFont, fScale, hOldFont);
 
-    WinFontFace const * pWinFontData = static_cast<WinFontFace const *>(aIFSD.mpFontData);
+    WinFontFace const * pWinFontData = static_cast<WinFontFace const *>(pFont);
 
 #if OSL_DEBUG_LEVEL > 1
     // get font metrics
@@ -1793,7 +1799,7 @@ void WinSalGraphics::GetGlyphWidths( const PhysicalFontFace* pFont,
 
     float fScale = 0.0;
     HFONT hOldFont = nullptr;
-    ImplDoSetFont(&aIFSD, fScale, hOldFont);
+    ImplDoSetFont(&aIFSD, pFont, fScale, hOldFont);
 
     // get raw font file data
     const RawFontData xRawFontData( getHDC() );
diff --git a/vcl/win/gdi/salgdi.cxx b/vcl/win/gdi/salgdi.cxx
index 8aa601a461b2..d38337b6d546 100644
--- a/vcl/win/gdi/salgdi.cxx
+++ b/vcl/win/gdi/salgdi.cxx
@@ -624,7 +624,6 @@ WinSalGraphics::WinSalGraphics(WinSalGraphics::Type eType, bool bScreen, HWND hW
     for( int i = 0; i < MAX_FALLBACK; ++i )
     {
         mhFonts[ i ] = nullptr;
-        mpWinFontData[ i ]  = nullptr;
         mpWinFontEntry[ i ] = nullptr;
         mfFontScale[ i ] = 1.0;
     }
diff --git a/vcl/win/gdi/winlayout.cxx b/vcl/win/gdi/winlayout.cxx
index 94eb47dddebb..cc37b866848d 100644
--- a/vcl/win/gdi/winlayout.cxx
+++ b/vcl/win/gdi/winlayout.cxx
@@ -305,9 +305,9 @@ std::unique_ptr<SalLayout> WinSalGraphics::GetTextLayout(ImplLayoutArgs& /*rArgs
     if (!mpWinFontEntry[nFallbackLevel])
         return nullptr;
 
-    assert(mpWinFontData[nFallbackLevel]);
+    assert(mpWinFontEntry[nFallbackLevel]->GetFontFace());
 
-    return std::unique_ptr<SalLayout>(new CommonSalLayout(getHDC(), *mpWinFontEntry[nFallbackLevel], *mpWinFontData[nFallbackLevel]));
+    return std::unique_ptr<SalLayout>(new CommonSalLayout(getHDC(), *mpWinFontEntry[nFallbackLevel]));
 }
 
 LogicalFontInstance * WinSalGraphics::GetWinFontEntry(int const nFallbackLevel)
@@ -315,8 +315,8 @@ LogicalFontInstance * WinSalGraphics::GetWinFontEntry(int const nFallbackLevel)
     return mpWinFontEntry[nFallbackLevel];
 }
 
-WinFontInstance::WinFontInstance( FontSelectPattern const & rFSD )
-:   LogicalFontInstance( rFSD )
+WinFontInstance::WinFontInstance(const PhysicalFontFace& rPFF, const FontSelectPattern& rFSP)
+    : LogicalFontInstance(rPFF, rFSP)
 {
 }
 
@@ -335,7 +335,7 @@ PhysicalFontFace* WinFontFace::Clone() const
 
 LogicalFontInstance* WinFontFace::CreateFontInstance(const FontSelectPattern& rFSD) const
 {
-    return new WinFontInstance(rFSD);
+    return new WinFontInstance(*this, rFSD);
 }
 
 bool WinSalGraphics::CacheGlyphs(const CommonSalLayout& rLayout)
diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx
index 864c0dd45f03..ed65a8698d70 100644
--- a/vcl/win/window/salframe.cxx
+++ b/vcl/win/window/salframe.cxx
@@ -2184,8 +2184,8 @@ static void ImplSalFrameSetInputContext( HWND hWnd, const SalInputContext* pCont
                 // specified by this font name; but it seems to decide whether
                 // to use that font's horizontal or vertical variant based on a
                 // '@' in front of this font name.
-                ImplGetLogFontFromFontSelect( hDC, pContext->mpFont, aLogFont,
-                                              false );
+                ImplGetLogFontFromFontSelect(hDC, pContext->mpFont,
+                                             nullptr, aLogFont);
                 ReleaseDC( pFrame->mhWnd, hDC );
                 ImmSetCompositionFontW( hIMC, &aLogFont );
                 ImmReleaseContext( pFrame->mhWnd, hIMC );