[Libreoffice-commits] core.git: Branch 'libreoffice-6-4' - 2 commits - vcl/headless vcl/inc vcl/qt5 vcl/quartz vcl/source vcl/unx
Jan-Marek Glogowski (via logerrit)
logerrit at kemper.freedesktop.org
Tue Nov 19 11:48:31 UTC 2019
vcl/headless/svpgdi.cxx | 1 +
vcl/inc/textrender.hxx | 2 ++
vcl/inc/unx/cairotextrender.hxx | 4 +---
vcl/inc/unx/glyphcache.hxx | 13 +++++++++++++
vcl/qt5/Qt5Graphics.cxx | 11 +----------
vcl/quartz/salgdi.cxx | 7 +------
vcl/source/font/fontcache.cxx | 9 +++++++++
vcl/source/gdi/salgdilayout.cxx | 1 +
vcl/unx/generic/gdi/cairotextrender.cxx | 12 ++++++------
vcl/unx/generic/glyphs/glyphcache.cxx | 21 ++++++++++++++++++++-
10 files changed, 55 insertions(+), 26 deletions(-)
New commits:
commit a00bdc999344db34d5926dc77ed5ca895295b0ee
Author: Jan-Marek Glogowski <jan-marek.glogowski at extern.cib.de>
AuthorDate: Mon Nov 18 16:04:24 2019 +0000
Commit: Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Tue Nov 19 12:48:03 2019 +0100
tdf#128434 correctly release fonts in destructors
This adds ReleaseFonts() calls to all destructors of SalGraphics
and TextRenderImpl derivated classes, which implement SetFont.
During destruction a base class can't call into derivated classes,
as these are already destructed, so we have to spread these calls
manually.
Change-Id: Ia57db04f7df665e5205212ce512119e2f60e3379
Reviewed-on: https://gerrit.libreoffice.org/82967
Tested-by: Jenkins
Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
(cherry picked from commit f8e1f8652255cadd80a991aa3e059ee631b333b8)
Reviewed-on: https://gerrit.libreoffice.org/83149
diff --git a/vcl/headless/svpgdi.cxx b/vcl/headless/svpgdi.cxx
index 57f66d4fabf6..e34678a93932 100644
--- a/vcl/headless/svpgdi.cxx
+++ b/vcl/headless/svpgdi.cxx
@@ -612,6 +612,7 @@ SvpSalGraphics::SvpSalGraphics()
SvpSalGraphics::~SvpSalGraphics()
{
+ ReleaseFonts();
}
void SvpSalGraphics::setSurface(cairo_surface_t* pSurface, const basegfx::B2IVector& rSize)
diff --git a/vcl/inc/textrender.hxx b/vcl/inc/textrender.hxx
index 742d8445299b..1aec8fba2301 100644
--- a/vcl/inc/textrender.hxx
+++ b/vcl/inc/textrender.hxx
@@ -32,10 +32,12 @@ class PhysicalFontFace;
class TextRenderImpl
{
public:
+ // can't call ReleaseFonts here, as the destructor just calls this classes SetFont (pure virtual)!
virtual ~TextRenderImpl() {}
virtual void SetTextColor( Color nColor ) = 0;
virtual void SetFont(LogicalFontInstance*, int nFallbackLevel) = 0;
+ void ReleaseFonts() { SetFont(nullptr, 0); }
virtual void GetFontMetric( ImplFontMetricDataRef&, int nFallbackLevel ) = 0;
virtual FontCharMapRef GetFontCharMap() const = 0;
virtual bool GetFontCapabilities(vcl::FontCapabilities &rFontCapabilities) const = 0;
diff --git a/vcl/inc/unx/cairotextrender.hxx b/vcl/inc/unx/cairotextrender.hxx
index 33b1a622945e..c0882bc35f86 100644
--- a/vcl/inc/unx/cairotextrender.hxx
+++ b/vcl/inc/unx/cairotextrender.hxx
@@ -38,13 +38,11 @@ protected:
virtual void getSurfaceOffset(double& nDX, double& nDY) = 0;
virtual void releaseCairoContext(cairo_t* cr) = 0;
- void setFont(LogicalFontInstance *pEntry, int nFallbackLevel);
-
virtual void clipRegion(cairo_t* cr) = 0;
public:
CairoTextRender();
-
+ virtual ~CairoTextRender() override;
virtual void SetTextColor( Color nColor ) override;
virtual void SetFont(LogicalFontInstance*, int nFallbackLevel) override;
diff --git a/vcl/qt5/Qt5Graphics.cxx b/vcl/qt5/Qt5Graphics.cxx
index 5192f0b42416..8228699a2602 100644
--- a/vcl/qt5/Qt5Graphics.cxx
+++ b/vcl/qt5/Qt5Graphics.cxx
@@ -44,16 +44,7 @@ Qt5Graphics::Qt5Graphics( Qt5Frame *pFrame, QImage *pQImage )
m_pWidgetDraw.reset(new Qt5Graphics_Controls());
}
-Qt5Graphics::~Qt5Graphics()
-{
- // release the text styles
- for (int i = 0; i < MAX_FALLBACK; ++i)
- {
- if (!m_pTextStyle[i])
- break;
- m_pTextStyle[i].clear();
- }
-}
+Qt5Graphics::~Qt5Graphics() { ReleaseFonts(); }
void Qt5Graphics::ChangeQImage(QImage* pQImage)
{
diff --git a/vcl/quartz/salgdi.cxx b/vcl/quartz/salgdi.cxx
index 8884d0bd9f3b..b6df53319e28 100644
--- a/vcl/quartz/salgdi.cxx
+++ b/vcl/quartz/salgdi.cxx
@@ -222,12 +222,7 @@ AquaSalGraphics::~AquaSalGraphics()
CGPathRelease( mxClipPath );
}
- for (int i = 0; i < MAX_FALLBACK; ++i)
- {
- if (!mpTextStyle[i])
- break;
- mpTextStyle[i].clear();
- }
+ ReleaseFonts();
if( mpXorEmulation )
delete mpXorEmulation;
diff --git a/vcl/source/gdi/salgdilayout.cxx b/vcl/source/gdi/salgdilayout.cxx
index b0fefa665bd5..de11058e4507 100644
--- a/vcl/source/gdi/salgdilayout.cxx
+++ b/vcl/source/gdi/salgdilayout.cxx
@@ -82,6 +82,7 @@ bool SalGraphics::initWidgetDrawBackends(bool bForce)
SalGraphics::~SalGraphics() COVERITY_NOEXCEPT_FALSE
{
+ // can't call ReleaseFonts here, as the destructor just calls this classes SetFont (pure virtual)!
}
#if HAVE_FEATURE_OPENGL
diff --git a/vcl/unx/generic/gdi/cairotextrender.cxx b/vcl/unx/generic/gdi/cairotextrender.cxx
index 3b1c7f24f01a..9610a73bc1d6 100644
--- a/vcl/unx/generic/gdi/cairotextrender.cxx
+++ b/vcl/unx/generic/gdi/cairotextrender.cxx
@@ -81,7 +81,12 @@ CairoTextRender::CairoTextRender()
rp = nullptr;
}
-void CairoTextRender::setFont(LogicalFontInstance *pEntry, int nFallbackLevel)
+CairoTextRender::~CairoTextRender()
+{
+ ReleaseFonts();
+}
+
+void CairoTextRender::SetFont(LogicalFontInstance *pEntry, int nFallbackLevel)
{
// release all no longer needed font resources
for( int i = nFallbackLevel; i < MAX_FALLBACK; ++i )
@@ -380,11 +385,6 @@ bool CairoTextRender::GetFontCapabilities(vcl::FontCapabilities &rGetImplFontCap
// SalGraphics
-void CairoTextRender::SetFont(LogicalFontInstance *pEntry, int nFallbackLevel)
-{
- setFont(pEntry, nFallbackLevel);
-}
-
void
CairoTextRender::SetTextColor( Color nColor )
{
commit 325005697155853891ce4f23e7349931e748d7e7
Author: Jan-Marek Glogowski <jan-marek.glogowski at extern.cib.de>
AuthorDate: Sat Nov 16 02:21:26 2019 +0000
Commit: Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Tue Nov 19 12:47:51 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
diff --git a/vcl/inc/unx/glyphcache.hxx b/vcl/inc/unx/glyphcache.hxx
index 797890a2a345..f369952faac4 100644
--- a/vcl/inc/unx/glyphcache.hxx
+++ b/vcl/inc/unx/glyphcache.hxx
@@ -90,6 +90,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 67217acd0472..f3f87d36616b 100644
--- a/vcl/source/font/fontcache.cxx
+++ b/vcl/source/font/fontcache.cxx
@@ -24,6 +24,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();
@@ -91,7 +95,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 34543b7731a0..9640ea70adcf 100644
--- a/vcl/unx/generic/glyphs/glyphcache.cxx
+++ b/vcl/unx/generic/glyphs/glyphcache.cxx
@@ -199,6 +199,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
@@ -236,7 +251,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);
}
}
More information about the Libreoffice-commits
mailing list