improve font caching in SwFntObj

The code previously cached only SalGlyphItems, but such caching
fails if SalLayout::GetGlyphs() does not return valid glyphs,
which is the case with MultiSalLayout. Worse, it not only fails,
but layout is once computed for caching, that fails, and is computed
a second time for actual use.

This improved cache caches the result of GetTextWidth(), which
itself improves the performance a bit, but it also allows caching
the value for GetGlyph() not returning valid glyphs. Moreover this
also caches failures to get valid glyphs from GetGlyphs().

There are quite some calls to GetTextArray(), I didn't cache those,
but it might be added if needed.

Change-Id: Ia2589fb1b778f4f154c88f65d9906584284239da
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/112588
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

2 files changed, 103 insertions, 56 deletions

diff --git a/sw/source/core/inc/fntcache.hxx b/sw/source/core/inc/fntcache.hxx
index 04cfc4abc219..d8e11701445b 100644
--- a/sw/source/core/inc/fntcache.hxx
+++ b/sw/source/core/inc/fntcache.hxx
@@ -60,19 +60,9 @@ extern SwFntCache *pFntCache;
 extern SwFntObj *pLastFont;
 extern sal_uInt8* mnFontCacheIdCounter;
 
-/**
- * Defines a substring on a given output device, to be used as an std::map<>
- * key.
- */
-struct SwTextGlyphsKey
-{
-    VclPtr<OutputDevice> m_pOutputDevice;
-    OUString m_aText;
-    sal_Int32 m_nIndex;
-    sal_Int32 m_nLength;
-
-};
+struct SwTextGlyphsKey;
 bool operator<(const SwTextGlyphsKey& l, const SwTextGlyphsKey& r);
+struct SwTextGlyphsData;
 
 class SwFntObj : public SwCacheObj
 {
@@ -95,8 +85,8 @@ class SwFntObj : public SwCacheObj
     bool m_bSymbol : 1;
     bool m_bPaintBlank : 1;
 
-    /// Cache of already calculated layout glyphs.
-    std::map<SwTextGlyphsKey, SalLayoutGlyphs> m_aTextGlyphs;
+    /// Cache of already calculated layout glyphs and text widths.
+    std::map<SwTextGlyphsKey, SwTextGlyphsData> m_aTextGlyphs;
 
     static long nPixWidth;
     static MapMode *pPixMap;
@@ -125,7 +115,10 @@ public:
     sal_uInt16   GetZoom() const { return m_nZoom; }
     sal_uInt16   GetPropWidth() const { return m_nPropWidth; }
     bool     IsSymbol() const { return m_bSymbol; }
-    std::map<SwTextGlyphsKey, SalLayoutGlyphs>& GetTextGlyphs() { return m_aTextGlyphs; }
+
+    long GetCachedTextWidth(const SwTextGlyphsKey& key, const vcl::TextLayoutCache* vclCache);
+    SalLayoutGlyphs* GetCachedSalLayoutGlyphs(const SwTextGlyphsKey& key);
+    void ClearCachedTextGlyphs();
 
     void   DrawText( SwDrawTextInfo &rInf );
     /// determine the TextSize (of the printer)
diff --git a/sw/source/core/txtnode/fntcache.cxx b/sw/source/core/txtnode/fntcache.cxx
index 500190bf064c..206e95340a0f 100644
--- a/sw/source/core/txtnode/fntcache.cxx
+++ b/sw/source/core/txtnode/fntcache.cxx
@@ -76,6 +76,28 @@ long SwFntObj::nPixWidth;
 MapMode* SwFntObj::pPixMap = nullptr;
 static vcl::DeleteOnDeinit< VclPtr<OutputDevice> > s_pFntObjPixOut( new VclPtr<OutputDevice> );
 
+/**
+ * Defines a substring on a given output device, to be used as an std::map<>
+ * key.
+ */
+struct SwTextGlyphsKey
+{
+    VclPtr<OutputDevice> m_pOutputDevice;
+    OUString m_aText;
+    sal_Int32 m_nIndex;
+    sal_Int32 m_nLength;
+
+};
+
+/**
+ * Glyphs and text width for the given SwTextGlyphsKey.
+ */
+struct SwTextGlyphsData
+{
+    SalLayoutGlyphs m_aTextGlyphs;
+    long m_nTextWidth = -1; // -1 = not computed yet
+};
+
 namespace
 {
 
@@ -98,36 +120,6 @@ long EvalGridWidthAdd( const SwTextGridItem *const pGrid, const SwDrawTextInfo &
     return nGridWidthAdd;
 }
 
-/**
- * Pre-calculates glyph items for the rendered subset of rKey's text, assuming
- * outdev state does not change between the outdev calls.
- */
-SalLayoutGlyphs* lcl_CreateLayout(SwTextGlyphsKey& rKey, SalLayoutGlyphs& rTextGlyphs)
-{
-    // Use pre-calculated result.
-    if (rTextGlyphs.IsValid())
-        return &rTextGlyphs;
-
-    if (rKey.m_nIndex >= rKey.m_aText.getLength())
-        // Same as in OutputDevice::GetTextArray().
-        return nullptr;
-
-    // Calculate glyph items.
-    std::unique_ptr<SalLayout> pLayout
-        = rKey.m_pOutputDevice->ImplLayout(rKey.m_aText, rKey.m_nIndex, rKey.m_nLength, Point(0, 0), 0,
-                                         nullptr, SalLayoutFlags::GlyphItemsOnly);
-    if (!pLayout)
-        return nullptr;
-
-    const SalLayoutGlyphs* pGlyphs = pLayout->GetGlyphs();
-    if (!pGlyphs)
-        return nullptr;
-
-    // Remember the calculation result.
-    rTextGlyphs = *pGlyphs;
-
-    return &rTextGlyphs;
-}
 }
 
 bool operator<(const SwTextGlyphsKey& l, const SwTextGlyphsKey& r)
@@ -228,6 +220,72 @@ void SwFntObj::CreatePrtFont( const OutputDevice& rPrt )
 
 }
 
+/**
+ * Pre-calculates glyph items for the rendered subset of rKey's text, assuming
+ * outdev state does not change between the outdev calls.
+ */
+static SalLayoutGlyphs* lcl_CreateLayout(const SwTextGlyphsKey& rKey, std::map<SwTextGlyphsKey, SwTextGlyphsData>::iterator it)
+{
+    assert (!it->second.m_aTextGlyphs.IsValid());
+
+    if (rKey.m_nIndex >= rKey.m_aText.getLength())
+        // Same as in OutputDevice::GetTextArray().
+        return nullptr;
+
+    // Calculate glyph items.
+    std::unique_ptr<SalLayout> pLayout
+        = rKey.m_pOutputDevice->ImplLayout(rKey.m_aText, rKey.m_nIndex, rKey.m_nLength, Point(0, 0), 0,
+                                         nullptr, SalLayoutFlags::GlyphItemsOnly);
+    if (!pLayout)
+        return nullptr;
+
+    const SalLayoutGlyphs* pGlyphs = pLayout->GetGlyphs();
+    if (!pGlyphs)
+        return nullptr;
+
+    // Remember the calculation result.
+    it->second.m_aTextGlyphs = *pGlyphs;
+
+    return &it->second.m_aTextGlyphs;
+}
+
+SalLayoutGlyphs* SwFntObj::GetCachedSalLayoutGlyphs(const SwTextGlyphsKey& key)
+{
+    std::map<SwTextGlyphsKey, SwTextGlyphsData>::iterator it = m_aTextGlyphs.find(key);
+    if(it != m_aTextGlyphs.end())
+    {
+        if( it->second.m_aTextGlyphs.IsValid())
+            return &it->second.m_aTextGlyphs;
+        // Do not try to create the layout here. If a cache item exists, it's already
+        // been attempted and the layout was invalid (this happens with MultiSalLayout).
+        // So in that case this is a cached failure.
+        return nullptr;
+    }
+    it = m_aTextGlyphs.emplace( key, SwTextGlyphsData()).first;
+    return lcl_CreateLayout(key, it);
+}
+
+long SwFntObj::GetCachedTextWidth(const SwTextGlyphsKey& key, const vcl::TextLayoutCache* vclCache)
+{
+    std::map<SwTextGlyphsKey, SwTextGlyphsData>::iterator it = m_aTextGlyphs.find(key);
+    if(it != m_aTextGlyphs.end() && it->second.m_nTextWidth >= 0)
+        return it->second.m_nTextWidth;
+    if(it == m_aTextGlyphs.end())
+    {
+        it = m_aTextGlyphs.emplace( key, SwTextGlyphsData()).first;
+        lcl_CreateLayout(key, it);
+    }
+    it->second.m_nTextWidth = key.m_pOutputDevice->GetTextWidth(key.m_aText, key.m_nIndex, key.m_nLength, vclCache,
+        it->second.m_aTextGlyphs.IsValid() ? &it->second.m_aTextGlyphs : nullptr );
+    assert(it->second.m_nTextWidth >= 0);
+    return it->second.m_nTextWidth;
+}
+
+void SwFntObj::ClearCachedTextGlyphs()
+{
+    m_aTextGlyphs.clear();
+}
+
 /*
  *  returns whether we have to adjust the output font to resemble
  *  the formatting font
@@ -1458,7 +1516,7 @@ void SwFntObj::DrawText( SwDrawTextInfo &rInf )
         // get screen array
         std::unique_ptr<long[]> pScrArray(new long[sal_Int32(rInf.GetLen())]);
         SwTextGlyphsKey aGlyphsKey{ &rInf.GetOut(), rInf.GetText(), sal_Int32(rInf.GetIdx()), sal_Int32(rInf.GetLen()) };
-        SalLayoutGlyphs* pGlyphs = lcl_CreateLayout(aGlyphsKey, m_aTextGlyphs[aGlyphsKey]);
+        SalLayoutGlyphs* pGlyphs = GetCachedSalLayoutGlyphs(aGlyphsKey);
         rInf.GetOut().GetTextArray( rInf.GetText(), pScrArray.get(),
                         sal_Int32(rInf.GetIdx()), sal_Int32(rInf.GetLen()), nullptr, pGlyphs);
 
@@ -1473,7 +1531,7 @@ void SwFntObj::DrawText( SwDrawTextInfo &rInf )
                     m_pPrinter->SetFont( *m_pPrtFont );
             }
             aGlyphsKey = SwTextGlyphsKey{ m_pPrinter, rInf.GetText(), sal_Int32(rInf.GetIdx()), sal_Int32(rInf.GetLen()) };
-            pGlyphs = lcl_CreateLayout(aGlyphsKey, m_aTextGlyphs[aGlyphsKey]);
+            pGlyphs = GetCachedSalLayoutGlyphs(aGlyphsKey);
             m_pPrinter->GetTextArray(rInf.GetText(), pKernArray.get(),
                     sal_Int32(rInf.GetIdx()), sal_Int32(rInf.GetLen()), nullptr, pGlyphs);
         }
@@ -1818,7 +1876,7 @@ void SwFntObj::DrawText( SwDrawTextInfo &rInf )
                             : sal_Int32(rInf.GetIdx());
                 aGlyphsKey = SwTextGlyphsKey{ &rInf.GetOut(), *pStr, nTmpIdx, nLen };
                 if (bCacheLayout)
-                    pGlyphs = lcl_CreateLayout(aGlyphsKey, m_aTextGlyphs[aGlyphsKey]);
+                    pGlyphs = GetCachedSalLayoutGlyphs(aGlyphsKey);
                 else
                     pGlyphs = nullptr;
                 rInf.GetOut().DrawTextArray( aTextOriginPos, *pStr, pKernArray.get(),
@@ -2055,10 +2113,7 @@ Size SwFntObj::GetTextSize( SwDrawTextInfo& rInf )
         else
         {
             SwTextGlyphsKey aGlyphsKey{ &rInf.GetOut(), rInf.GetText(), sal_Int32(rInf.GetIdx()), sal_Int32(nLn) };
-            SalLayoutGlyphs* pGlyphs = lcl_CreateLayout(aGlyphsKey, m_aTextGlyphs[aGlyphsKey]);
-            aTextSize.setWidth( rInf.GetOut().GetTextWidth( rInf.GetText(),
-                                   sal_Int32(rInf.GetIdx()), sal_Int32(nLn),
-                                                           rInf.GetVclCache(), pGlyphs) );
+            aTextSize.setWidth( GetCachedTextWidth(aGlyphsKey, rInf.GetVclCache()));
             rInf.SetKanaDiff( 0 );
         }
 
@@ -2092,7 +2147,7 @@ TextFrameIndex SwFntObj::GetCursorOfst(SwDrawTextInfo &rInf)
         m_pPrinter->SetLayoutMode( rInf.GetOut().GetLayoutMode() );
         m_pPrinter->SetDigitLanguage( rInf.GetOut().GetDigitLanguage() );
         SwTextGlyphsKey aGlyphsKey{ m_pPrinter, rInf.GetText(), sal_Int32(rInf.GetIdx()), sal_Int32(rInf.GetLen()) };
-        SalLayoutGlyphs* pGlyphs = lcl_CreateLayout(aGlyphsKey, m_aTextGlyphs[aGlyphsKey]);
+        SalLayoutGlyphs* pGlyphs = GetCachedSalLayoutGlyphs(aGlyphsKey);
         m_pPrinter->GetTextArray( rInf.GetText(), pKernArray.get(),
                 sal_Int32(rInf.GetIdx()), sal_Int32(rInf.GetLen()), nullptr, pGlyphs);
     }
@@ -2506,8 +2561,7 @@ TextFrameIndex SwFont::GetTextBreak(SwDrawTextInfo const & rInf, long nTextWidth
             SwFntAccess aFntAccess(m_aSub[m_nActual].m_nFontCacheId, m_aSub[m_nActual].m_nFontIndex,
                                    &m_aSub[m_nActual], rInf.GetShell());
             SwTextGlyphsKey aGlyphsKey{ &rInf.GetOut(), *pTmpText, sal_Int32(nTmpIdx), sal_Int32(nTmpLen) };
-            SalLayoutGlyphs* pGlyphs
-                = lcl_CreateLayout(aGlyphsKey, aFntAccess.Get()->GetTextGlyphs()[aGlyphsKey]);
+            SalLayoutGlyphs* pGlyphs = aFntAccess.Get()->GetCachedSalLayoutGlyphs(aGlyphsKey);
             nTextBreak = TextFrameIndex(rInf.GetOut().GetTextBreak(
                              *pTmpText, nTextWidth,
                              sal_Int32(nTmpIdx), sal_Int32(nTmpLen),
@@ -2699,7 +2753,7 @@ bool SwDrawTextInfo::ApplyAutoColor( vcl::Font* pFont )
 void SwClearFntCacheTextGlyphs()
 {
     for (SwFntObj* pFntObj = pFntCache->First(); pFntObj; pFntObj = SwFntCache::Next(pFntObj))
-        pFntObj->GetTextGlyphs().clear();
+        pFntObj->ClearCachedTextGlyphs();
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */