[Libreoffice-commits] core.git: Branch 'libreoffice-6-3' - 2 commits - vcl/inc vcl/source vcl/unx

Jan-Marek Glogowski (via logerrit) logerrit at kemper.freedesktop.org
Thu Nov 21 14:22:02 UTC 2019


 vcl/inc/unx/freetype_glyphcache.hxx            |   16 +++----
 vcl/inc/unx/glyphcache.hxx                     |   56 ++++++++++++++++++++++---
 vcl/source/font/fontcache.cxx                  |    9 ++++
 vcl/unx/generic/app/gendata.cxx                |    1 
 vcl/unx/generic/glyphs/freetype_glyphcache.cxx |   28 +-----------
 vcl/unx/generic/glyphs/glyphcache.cxx          |   36 +++++++++++++++-
 6 files changed, 105 insertions(+), 41 deletions(-)

New commits:
commit 083d98d73379f864a0d3cb9695e7c7ad0febce1c
Author:     Jan-Marek Glogowski <jan-marek.glogowski at extern.cib.de>
AuthorDate: Sat Nov 16 02:21:26 2019 +0000
Commit:     Michael Weghorn <m.weghorn at posteo.de>
CommitDate: Thu Nov 21 15:21:05 2019 +0100

    tdf#128434 try garbage collect ImplFontCache fonts
    
    Instead of changing the harfbuzz caching, for this use case it's
    enough to queue all per-OutputDevice fonts for garbage collection
    (GC), which are managed by the OutputDevices ImplFontCache. So
    just try to GC all the fonts in the ImplFontCache destructor.
    
    There is no point keeping these LogicalFontInstances alive, after
    the OutputDevice font cache is destroyed, as these fonts aren't
    shared and can't be accessed later. But the main problem is still
    some correct accounting of harfbuzz resources and eventual even
    the Freetype ones, so this cleanup would not really be needed.
    
    AFAI can tell, this plugs the remaining per-document leaks of the
    PDF generation, except for a 72 byte basic listener leak from:
    
    basic::ImplRepository::impl_createManagerForModel(...)
        basicmanagerrepository.cxx:480
    
    Change-Id: I3155be59a2bdcd85e01f6f048b990db04d88c94f
    Reviewed-on: https://gerrit.libreoffice.org/82968
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
    (cherry picked from commit e6aac0b637d583d3cfb893276f813ff5aa1ade17)
    Reviewed-on: https://gerrit.libreoffice.org/83148
    (cherry picked from commit 325005697155853891ce4f23e7349931e748d7e7)
    Reviewed-on: https://gerrit.libreoffice.org/83196
    Reviewed-by: Michael Weghorn <m.weghorn at posteo.de>

diff --git a/vcl/inc/unx/glyphcache.hxx b/vcl/inc/unx/glyphcache.hxx
index cf225ed6b91b..f2d9c771e162 100644
--- a/vcl/inc/unx/glyphcache.hxx
+++ b/vcl/inc/unx/glyphcache.hxx
@@ -93,6 +93,19 @@ public:
 
     FreetypeFont*           CacheFont(LogicalFontInstance* pFontInstance);
     void                    UncacheFont( FreetypeFont& );
+
+    /** Try to GarbageCollect an explicit logical font
+     *
+     * This should just be called from the ~ImplFontCache destructor, which holds the mapping of the
+     * FontSelectPattern to the LogicalFontInstance per OutputDevice. All other users should just
+     * call CacheFont and UncacheFont correctly. When the ImplFontCache is destroyed with its
+     * OutputDevice, we can safely garbage collection its unused entries, as these can't be reused.
+     *
+     * It's always safe to call this, as it just ignores the used bytes when considering a font for
+     * garbage collection, which normally keeps unreferenced fonts alive.
+     **/
+    void TryGarbageCollectFont(LogicalFontInstance*);
+
     void                    ClearFontCache();
     void                    ClearFontOptions();
 
diff --git a/vcl/source/font/fontcache.cxx b/vcl/source/font/fontcache.cxx
index abcee2358c4b..dccce0cac7c4 100644
--- a/vcl/source/font/fontcache.cxx
+++ b/vcl/source/font/fontcache.cxx
@@ -26,6 +26,10 @@
 #include <PhysicalFontFamily.hxx>
 #include <sal/log.hxx>
 
+#if !(defined(_WIN32) || defined(MACOSX) || defined(IOS))
+#include <unx/glyphcache.hxx>
+#endif
+
 size_t ImplFontCache::IFSD_Hash::operator()( const FontSelectPattern& rFSD ) const
 {
     return rFSD.hashCode();
@@ -93,7 +97,12 @@ ImplFontCache::ImplFontCache()
 ImplFontCache::~ImplFontCache()
 {
     for (const auto & rLFI : maFontInstanceList)
+    {
+#if !(defined(_WIN32) || defined(MACOSX) || defined(IOS))
+        GlyphCache::GetInstance().TryGarbageCollectFont(rLFI.second.get());
+#endif
         rLFI.second->mpFontCache = nullptr;
+    }
 }
 
 rtl::Reference<LogicalFontInstance> ImplFontCache::GetFontInstance( PhysicalFontCollection const * pFontList,
diff --git a/vcl/unx/generic/glyphs/glyphcache.cxx b/vcl/unx/generic/glyphs/glyphcache.cxx
index 18fb60b363a5..28fdaa8c4cb0 100644
--- a/vcl/unx/generic/glyphs/glyphcache.cxx
+++ b/vcl/unx/generic/glyphs/glyphcache.cxx
@@ -204,6 +204,21 @@ void GlyphCache::UncacheFont( FreetypeFont& rFreetypeFont )
     }
 }
 
+void GlyphCache::TryGarbageCollectFont(LogicalFontInstance *pFontInstance)
+{
+    if (maFontList.empty() || !pFontInstance)
+        return;
+    FreetypeFontInstance* pFFI = dynamic_cast<FreetypeFontInstance*>(pFontInstance);
+    if (!pFFI)
+        return;
+    FreetypeFont* pFreetypeFont = pFFI->GetFreetypeFont();
+    if (pFreetypeFont && (pFreetypeFont->GetRefCount() <= 0))
+    {
+        mpCurrentGCFont = pFreetypeFont;
+        GarbageCollect();
+    }
+}
+
 void GlyphCache::GarbageCollect()
 {
     // when current GC font has been destroyed get another one
@@ -241,7 +256,11 @@ void GlyphCache::GarbageCollect()
         if( pFreetypeFont == mpCurrentGCFont )
             mpCurrentGCFont = nullptr;
 
-        maFontList.erase(pFreetypeFont->GetFontInstance());
+#ifndef NDEBUG
+        int nErased =
+#endif
+            maFontList.erase(pFreetypeFont->GetFontInstance());
+        assert(1 == nErased);
     }
 }
 
commit d7680512046b41bcf57e4f1b21420a3ea0190bf4
Author:     Jan-Marek Glogowski <jan-marek.glogowski at extern.cib.de>
AuthorDate: Fri Nov 15 16:19:31 2019 +0000
Commit:     Michael Weghorn <m.weghorn at posteo.de>
CommitDate: Thu Nov 21 15:20:46 2019 +0100

    Move static aFontFileList into GlyphCache
    
    GlyphCache is already a global in the unix SalData class, so we
    can drop one more global static variable. and the FontFile map
    values aren't shared, so just use std::unique_ptr, like the two
    other maps, which form the GlyphCache class.
    
    While at it finalize the classes and hide their constructors.
    
    Reviewed-on: https://gerrit.libreoffice.org/82966
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
    (cherry picked from commit 578f97b4b2d06d3d5d5db7fd70066ba8ca665abc)
    Reviewed-on: https://gerrit.libreoffice.org/83147
    (cherry picked from commit aac19da146d19033964d7c6ed608cfc4786f88c5)
    
    Change-Id: Iaac8cd9905e9b4025707a17f62d8961ccfa5d0fb
    Reviewed-on: https://gerrit.libreoffice.org/83195
    Tested-by: Jenkins
    Reviewed-by: Michael Weghorn <m.weghorn at posteo.de>

diff --git a/vcl/inc/unx/freetype_glyphcache.hxx b/vcl/inc/unx/freetype_glyphcache.hxx
index 6fd0154d98a7..1417f4a4c687 100644
--- a/vcl/inc/unx/freetype_glyphcache.hxx
+++ b/vcl/inc/unx/freetype_glyphcache.hxx
@@ -30,11 +30,9 @@ class CmapResult;
 // FreetypeFontFile has the responsibility that a font file is only mapped once.
 // (#86621#) the old directly ft-managed solution caused it to be mapped
 // in up to nTTC*nSizes*nOrientation*nSynthetic times
-class FreetypeFontFile
+class FreetypeFontFile final
 {
 public:
-    static FreetypeFontFile*      FindFontFile( const OString& rNativeFileName );
-
     bool                    Map();
     void                    Unmap();
 
@@ -44,6 +42,7 @@ public:
     int                     GetLangBoost() const { return mnLangBoost; }
 
 private:
+    friend class GlyphCache;
     explicit                FreetypeFontFile( const OString& rNativeFileName );
 
     const OString    maNativeFileName;
@@ -54,11 +53,9 @@ private:
 };
 
 // FreetypeFontInfo corresponds to an unscaled font face
-class FreetypeFontInfo
+class FreetypeFontInfo final
 {
 public:
-    FreetypeFontInfo(const FontAttributes&, const OString& rNativeFileName,
-                     int nFaceNum, int nFaceVariation, sal_IntPtr nFontId);
     ~FreetypeFontInfo();
 
     const unsigned char*  GetTable( const char*, sal_uLong* pLength) const;
@@ -79,6 +76,10 @@ public:
     const FontCharMapRef& GetFontCharMap();
 
 private:
+    friend class GlyphCache;
+    explicit FreetypeFontInfo(const FontAttributes&, FreetypeFontFile* const pFontFile,
+                              int nFaceNum, int nFaceVariation, sal_IntPtr nFontId);
+
     FT_FaceRec_*    maFaceFT;
     FreetypeFontFile* const mpFontFile;
     const int       mnFaceNum;
@@ -102,8 +103,7 @@ public:
     virtual sal_IntPtr      GetFontId() const override { return mpFreetypeFontInfo->GetFontId(); }
 };
 
-// a class for cache entries for physical font instances that are based on serverfonts
-class VCL_DLLPUBLIC FreetypeFontInstance : public LogicalFontInstance
+class FreetypeFontInstance : public LogicalFontInstance
 {
     friend rtl::Reference<LogicalFontInstance> FreetypeFontFace::CreateFontInstance(const FontSelectPattern&) const;
 
diff --git a/vcl/inc/unx/glyphcache.hxx b/vcl/inc/unx/glyphcache.hxx
index 91881688fa22..cf225ed6b91b 100644
--- a/vcl/inc/unx/glyphcache.hxx
+++ b/vcl/inc/unx/glyphcache.hxx
@@ -40,6 +40,7 @@
 #include <unordered_map>
 
 class FreetypeFont;
+class FreetypeFontFile;
 class FreetypeFontInstance;
 class FreetypeFontInfo;
 class FontConfigFontOptions;
@@ -50,11 +51,36 @@ class SvpGcpHelper;
 namespace basegfx { class B2DPolyPolygon; }
 namespace vcl { struct FontCapabilities; }
 
+ /**
+  * The GlyphCache caches various aspects of Freetype fonts
+  *
+  * It mainly consists of three std::unordered_map lists, which hold the items of the cache.
+  *
+  * They form kind of a tree, with FreetypeFontFile as the roots, referenced by multiple FreetypeFontInfo
+  * entries, which are referenced by the FreetypeFont items.
+  *
+  * All of these items have reference counters, but these don't control the items life-cycle, but that of
+  * the managed resources.
+  *
+  * The respective resources are:
+  *   FreetypeFontFile = holds the mmapped font file, as long as it's used by any FreetypeFontInfo.
+  *   FreetypeFontInfo = holds the FT_FaceRec_ object, as long as it's used by any FreetypeFont.
+  *   FreetypeFont     = holds the FT_SizeRec_.
+  *
+  * FreetypeFontInfo therefore is embedded in the Freetype subclass of PhysicalFontFace.
+  * FreetypeFont is embedded in the Freetype subclass of LogicalFontInstance.
+  *
+  * Nowadays there is not really a reason to have seperate files for the classes, as the GlyphCache is
+  * just about handling of Freetype based fonts, not some abstract glyphs.
+  *
+  * One additional note: the byte-size based garbage collection of unused fonts can currently be assumed
+  * to be broken. Since the move of the glyph rect cache into the ImplFontCache, so it can be used by all
+  * platforms, it just takes too long to kick-in, as there is no real accounting left.
+  **/
 class VCL_DLLPUBLIC GlyphCache final
 {
 public:
-    explicit                GlyphCache();
-    virtual                 ~GlyphCache();
+    ~GlyphCache();
 
     static GlyphCache&      GetInstance();
 
@@ -71,9 +97,14 @@ public:
     void                    ClearFontOptions();
 
 private:
+    // to access the constructor (can't use InitFreetypeManager function, because it's private?!)
+    friend class GenericUnixSalData;
+    explicit GlyphCache();
+
     static void             InitFreetype();
     void                    GarbageCollect();
     FreetypeFont*           CreateFont(LogicalFontInstance* pLogicalFont);
+    FreetypeFontFile* FindFontFile(const OString& rNativeFileName);
 
     // the GlyphCache's FontList matches a font request to a serverfont instance
     // the FontList key's mpFontData member is reinterpreted as integer font id
@@ -81,6 +112,7 @@ private:
     struct IFSD_Hash{ size_t operator()( const rtl::Reference<LogicalFontInstance>& ) const; };
     typedef std::unordered_map<rtl::Reference<LogicalFontInstance>,std::unique_ptr<FreetypeFont>,IFSD_Hash,IFSD_Equal > FontList;
     typedef std::unordered_map<sal_IntPtr, std::unique_ptr<FreetypeFontInfo>> FontInfoList;
+    typedef std::unordered_map<const char*, std::unique_ptr<FreetypeFontFile>, rtl::CStringHash, rtl::CStringEqual> FontFileList;
 
     FontList                maFontList;
     static constexpr sal_uLong gnMaxSize = 1500000;  // max overall cache size in bytes
@@ -89,12 +121,13 @@ private:
 
     FontInfoList            m_aFontInfoList;
     sal_IntPtr              m_nMaxFontId;
+
+    FontFileList            m_aFontFileList;
 };
 
 class VCL_DLLPUBLIC FreetypeFont final
 {
 public:
-                            FreetypeFont(LogicalFontInstance* pFontInstance, FreetypeFontInfo*);
                             ~FreetypeFont();
 
     const OString&          GetFontFileName() const;
@@ -129,9 +162,7 @@ public:
 
 private:
     friend class GlyphCache;
-    friend class FreetypeFontInstance;
-    friend class X11SalGraphics;
-    friend class CairoTextRender;
+    explicit FreetypeFont(LogicalFontInstance*, FreetypeFontInfo*);
 
     void                    AddRef() const      { ++mnRefCount; }
     long                    GetRefCount() const { return mnRefCount; }
diff --git a/vcl/unx/generic/app/gendata.cxx b/vcl/unx/generic/app/gendata.cxx
index 2e376cc2e3f3..ea091105529a 100644
--- a/vcl/unx/generic/app/gendata.cxx
+++ b/vcl/unx/generic/app/gendata.cxx
@@ -25,7 +25,6 @@
 GenericUnixSalData::GenericUnixSalData(GenericUnixSalDataType const t, SalInstance* const pInstance)
     : m_eType(t)
     , m_pDisplay(nullptr)
-    , m_pGlyphCache(new GlyphCache)
 {
     m_pInstance = pInstance;
     SetSalData(this);
diff --git a/vcl/unx/generic/glyphs/freetype_glyphcache.cxx b/vcl/unx/generic/glyphs/freetype_glyphcache.cxx
index 0b65a8cb78e3..a97a4bbccda7 100644
--- a/vcl/unx/generic/glyphs/freetype_glyphcache.cxx
+++ b/vcl/unx/generic/glyphs/freetype_glyphcache.cxx
@@ -71,10 +71,6 @@
 
 static FT_Library aLibFT = nullptr;
 
-typedef std::unordered_map<const char*, std::shared_ptr<FreetypeFontFile>, rtl::CStringHash, rtl::CStringEqual> FontFileList;
-
-namespace { struct vclFontFileList : public rtl::Static< FontFileList, vclFontFileList > {}; }
-
 // TODO: remove when the priorities are selected by UI
 // if (AH==0) => disable autohinting
 // if (AA==0) => disable antialiasing
@@ -111,22 +107,6 @@ FreetypeFontFile::FreetypeFontFile( const OString& rNativeFileName )
     }
 }
 
-FreetypeFontFile* FreetypeFontFile::FindFontFile( const OString& rNativeFileName )
-{
-    // font file already known? (e.g. for ttc, synthetic, aliased fonts)
-    const char* pFileName = rNativeFileName.getStr();
-    FontFileList &rFontFileList = vclFontFileList::get();
-    FontFileList::const_iterator it = rFontFileList.find( pFileName );
-    if( it != rFontFileList.end() )
-        return it->second.get();
-
-    // no => create new one
-    FreetypeFontFile* pFontFile = new FreetypeFontFile( rNativeFileName );
-    pFileName = pFontFile->maNativeFileName.getStr();
-    rFontFileList[pFileName].reset(pFontFile);
-    return pFontFile;
-}
-
 bool FreetypeFontFile::Map()
 {
     if( mnRefCount++ <= 0 )
@@ -164,10 +144,10 @@ void FreetypeFontFile::Unmap()
 }
 
 FreetypeFontInfo::FreetypeFontInfo( const FontAttributes& rDevFontAttributes,
-    const OString& rNativeFileName, int nFaceNum, int nFaceVariation, sal_IntPtr nFontId)
+    FreetypeFontFile* const pFontFile, int nFaceNum, int nFaceVariation, sal_IntPtr nFontId)
 :
     maFaceFT( nullptr ),
-    mpFontFile( FreetypeFontFile::FindFontFile( rNativeFileName ) ),
+    mpFontFile(pFontFile),
     mnFaceNum( nFaceNum ),
     mnFaceVariation( nFaceVariation ),
     mnRefCount( 0 ),
@@ -320,8 +300,6 @@ void GlyphCache::InitFreetype()
     pEnv = ::getenv( "SAL_ANTIALIASED_TEXT_PRIORITY" );
     if( pEnv )
         nDefaultPrioAntiAlias = pEnv[0] - '0';
-
-    (void)vclFontFileList::get();
 }
 
 namespace
@@ -361,7 +339,7 @@ void GlyphCache::AddFontFile(const OString& rNormalizedName,
         return;
 
     FreetypeFontInfo* pFontInfo = new FreetypeFontInfo( rDevFontAttr,
-        rNormalizedName, nFaceNum, nVariantNum, nFontId);
+        FindFontFile(rNormalizedName), nFaceNum, nVariantNum, nFontId);
     m_aFontInfoList[ nFontId ].reset(pFontInfo);
     if( m_nMaxFontId < nFontId )
         m_nMaxFontId = nFontId;
diff --git a/vcl/unx/generic/glyphs/glyphcache.cxx b/vcl/unx/generic/glyphs/glyphcache.cxx
index 558e3d8c3323..18fb60b363a5 100644
--- a/vcl/unx/generic/glyphs/glyphcache.cxx
+++ b/vcl/unx/generic/glyphs/glyphcache.cxx
@@ -245,6 +245,21 @@ void GlyphCache::GarbageCollect()
     }
 }
 
+FreetypeFontFile* GlyphCache::FindFontFile(const OString& rNativeFileName)
+{
+    // font file already known? (e.g. for ttc, synthetic, aliased fonts)
+    const char* pFileName = rNativeFileName.getStr();
+    FontFileList::const_iterator it = m_aFontFileList.find(pFileName);
+    if (it != m_aFontFileList.end())
+        return it->second.get();
+
+    // no => create new one
+    FreetypeFontFile* pFontFile = new FreetypeFontFile(rNativeFileName);
+    pFileName = pFontFile->maNativeFileName.getStr();
+    m_aFontFileList[pFileName].reset(pFontFile);
+    return pFontFile;
+}
+
 void FreetypeFont::ReleaseFromGarbageCollect()
 {
     // remove from GC list


More information about the Libreoffice-commits mailing list