From 0012f765304a9587d77ccb54bbeeef4e332e5d92 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Caol=C3=A1n=20McNamara?= <caolanm@redhat.com>
Date: Wed, 6 Feb 2019 13:46:24 +0000
Subject: tdf#123165 cache recently scaled bitmaps for reuse
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

dropping the cached scaled bitmap when the bitmap
is accesed via BitmapAccessMode::Write for writing

Reviewed-on: https://gerrit.libreoffice.org/67459
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>
(cherry picked from commit 9f3926df5c2828ad3e8cfce2b4444b1c84352cf4)

Change-Id: Ib6539522944838238bd699ec3531039d21dc0f8b
Reviewed-on: https://gerrit.libreoffice.org/67492
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
---
 vcl/inc/salbmp.hxx                           |  2 ++
 vcl/inc/svdata.hxx                           |  4 ++++
 vcl/source/app/salvtables.cxx                | 11 +++++++++++
 vcl/source/app/svmain.cxx                    |  2 ++
 vcl/source/bitmap/BitmapScaleSuperFilter.cxx | 16 +++++++++++++++-
 vcl/source/gdi/bmpacc.cxx                    | 13 +++++++++----
 6 files changed, 43 insertions(+), 5 deletions(-)

Index: libreoffice-6.1.5.2/vcl/inc/salbmp.hxx
===================================================================
--- libreoffice-6.1.5.2.orig/vcl/inc/salbmp.hxx
+++ libreoffice-6.1.5.2/vcl/inc/salbmp.hxx
@@ -72,6 +72,8 @@ public:
 
     virtual bool            ScalingSupported() const = 0;
     virtual bool            Scale( const double& rScaleX, const double& rScaleY, BmpScaleFlag nScaleFlag ) = 0;
+    void                    DropScaledCache();
+
     virtual bool            Replace( const Color& rSearchColor, const Color& rReplaceColor, sal_uInt8 nTol ) = 0;
 
     virtual bool            ConvertToGreyscale()
Index: libreoffice-6.1.5.2/vcl/inc/svdata.hxx
===================================================================
--- libreoffice-6.1.5.2.orig/vcl/inc/svdata.hxx
+++ libreoffice-6.1.5.2/vcl/inc/svdata.hxx
@@ -22,6 +22,7 @@
 
 #include <config_version.h>
 
+#include <o3tl/lru_map.hxx>
 #include <tools/fldunit.hxx>
 #include <unotools/options.hxx>
 #include <vcl/svapp.hxx>
@@ -168,6 +169,8 @@ struct ImplSVAppData
     DECL_LINK(VclEventTestingHdl, Timer*, void);
 };
 
+typedef o3tl::lru_map<SalBitmap*, BitmapEx> lru_scale_cache;
+
 struct ImplSVGDIData
 {
     ~ImplSVGDIData();
@@ -186,6 +189,7 @@ struct ImplSVGDIData
     ImplPrnQueueList*       mpPrinterQueueList = nullptr;   // List of all printer queue
     PhysicalFontCollection* mpScreenFontList = nullptr;     // Screen-Font-List
     ImplFontCache*          mpScreenFontCache = nullptr;    // Screen-Font-Cache
+    lru_scale_cache maScaleCache = lru_scale_cache(10);     // Cache for scaled images
     ImplDirectFontSubstitution* mpDirectFontSubst = nullptr; // Font-Substitutions defined in Tools->Options->Fonts
     GraphicConverter*       mpGrfConverter = nullptr;       // Converter for graphics
     long                    mnAppFontX = 0;                 // AppFont X-Numenator for 40/tel Width
Index: libreoffice-6.1.5.2/vcl/source/app/salvtables.cxx
===================================================================
--- libreoffice-6.1.5.2.orig/vcl/source/app/salvtables.cxx
+++ libreoffice-6.1.5.2/vcl/source/app/salvtables.cxx
@@ -117,8 +117,19 @@ SalTimer::~SalTimer() COVERITY_NOEXCEPT_
 {
 }
 
+void SalBitmap::DropScaledCache()
+{
+    if (ImplSVData* pSVData = ImplGetSVData())
+    {
+        auto& rCache = pSVData->maGDIData.maScaleCache;
+        rCache.remove_if([this] (const o3tl::lru_map<SalBitmap*, BitmapEx>::key_value_pair_t& rKeyValuePair)
+                         { return rKeyValuePair.first == this; });
+    }
+}
+
 SalBitmap::~SalBitmap()
 {
+    DropScaledCache();
 }
 
 SalI18NImeStatus::~SalI18NImeStatus()
Index: libreoffice-6.1.5.2/vcl/source/app/svmain.cxx
===================================================================
--- libreoffice-6.1.5.2.orig/vcl/source/app/svmain.cxx
+++ libreoffice-6.1.5.2/vcl/source/app/svmain.cxx
@@ -619,6 +619,8 @@ void DeInitVCL()
     pSVData->maGDIData.mpScreenFontList = nullptr;
     delete pSVData->maGDIData.mpScreenFontCache;
     pSVData->maGDIData.mpScreenFontCache = nullptr;
+    pSVData->maGDIData.maScaleCache.remove_if([](const o3tl::lru_map<SalBitmap*, BitmapEx>::key_value_pair_t&)
+                                                { return true; });
 
     // Deinit Sal
     if (pSVData->mpDefInst)
Index: libreoffice-6.1.5.2/vcl/source/bitmap/BitmapScaleSuperFilter.cxx
===================================================================
--- libreoffice-6.1.5.2.orig/vcl/source/bitmap/BitmapScaleSuperFilter.cxx
+++ libreoffice-6.1.5.2/vcl/source/bitmap/BitmapScaleSuperFilter.cxx
@@ -26,6 +26,7 @@
 
 #include <algorithm>
 #include <memory>
+#include <svdata.hxx>
 
 namespace {
 
@@ -932,6 +933,7 @@ BitmapScaleSuperFilter::~BitmapScaleSupe
 BitmapEx BitmapScaleSuperFilter::execute(BitmapEx const& rBitmap)
 {
     Bitmap aBitmap(rBitmap.GetBitmap());
+    SalBitmap* pKey = aBitmap.ImplGetSalBitmap().get();
 
     bool bRet = false;
 
@@ -950,6 +952,17 @@ BitmapEx BitmapScaleSuperFilter::execute
 
     if (nDstW <= 1 || nDstH <= 1)
         return BitmapEx();
+
+    // check cache for a previously scaled version of this
+    ImplSVData* pSVData = ImplGetSVData();
+    auto& rCache = pSVData->maGDIData.maScaleCache;
+    auto aFind = rCache.find(pKey);
+    if (aFind != rCache.end())
+    {
+        if (aFind->second.GetSizePixel().Width() == nDstW && aFind->second.GetSizePixel().Height() == nDstH)
+            return aFind->second;
+    }
+
     {
         Bitmap::ScopedReadAccess pReadAccess(aBitmap);
 
@@ -1070,7 +1083,9 @@ BitmapEx BitmapScaleSuperFilter::execute
     {
         tools::Rectangle aRect(Point(0, 0), Point(nDstW, nDstH));
         aBitmap.Crop(aRect);
-        return BitmapEx(aBitmap);
+        BitmapEx aRet(aBitmap);
+        rCache.insert(std::make_pair(pKey, aRet));
+        return aRet;
     }
 
     return BitmapEx();
Index: libreoffice-6.1.5.2/vcl/source/gdi/bmpacc.cxx
===================================================================
--- libreoffice-6.1.5.2.orig/vcl/source/gdi/bmpacc.cxx
+++ libreoffice-6.1.5.2/vcl/source/gdi/bmpacc.cxx
@@ -34,38 +34,37 @@ BitmapInfoAccess::BitmapInfoAccess( Bitm
 {
     std::shared_ptr<SalBitmap> xImpBmp = rBitmap.ImplGetSalBitmap();
 
-    SAL_WARN_IF( !xImpBmp, "vcl", "Forbidden Access to empty bitmap!" );
+    assert( xImpBmp && "Forbidden Access to empty bitmap!" );
 
-    if( xImpBmp )
+    if( !xImpBmp )
+        return;
+
+    if( mnAccessMode == BitmapAccessMode::Write)
     {
-        if( mnAccessMode == BitmapAccessMode::Write && !maBitmap.ImplGetSalBitmap() )
+        xImpBmp->DropScaledCache();
+
+        if (xImpBmp.use_count() > 2)
         {
             xImpBmp.reset();
             rBitmap.ImplMakeUnique();
             xImpBmp = rBitmap.ImplGetSalBitmap();
         }
-        else
-        {
-            DBG_ASSERT( mnAccessMode != BitmapAccessMode::Write ||
-                        xImpBmp.use_count() == 2,
-                        "Unpredictable results: bitmap is referenced more than once!" );
-        }
+    }
 
-        mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
+    mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
 
-        if( !mpBuffer )
+    if( !mpBuffer )
+    {
+        std::shared_ptr<SalBitmap> xNewImpBmp(ImplGetSVData()->mpDefInst->CreateSalBitmap());
+        if (xNewImpBmp->Create(*xImpBmp, rBitmap.GetBitCount()))
         {
-            std::shared_ptr<SalBitmap> xNewImpBmp(ImplGetSVData()->mpDefInst->CreateSalBitmap());
-            if (xNewImpBmp->Create(*xImpBmp, rBitmap.GetBitCount()))
-            {
-                xImpBmp = xNewImpBmp;
-                rBitmap.ImplSetSalBitmap( xImpBmp );
-                mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
-            }
+            xImpBmp = xNewImpBmp;
+            rBitmap.ImplSetSalBitmap( xImpBmp );
+            mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
         }
-
-        maBitmap = rBitmap;
     }
+
+    maBitmap = rBitmap;
 }
 
 BitmapInfoAccess::~BitmapInfoAccess()
Index: libreoffice-6.1.5.2/include/o3tl/lru_map.hxx
===================================================================
--- libreoffice-6.1.5.2.orig/include/o3tl/lru_map.hxx
+++ libreoffice-6.1.5.2/include/o3tl/lru_map.hxx
@@ -11,6 +11,7 @@
 #ifndef INCLUDED_O3TL_LRU_MAP_HXX
 #define INCLUDED_O3TL_LRU_MAP_HXX
 
+#include <cassert>
 #include <list>
 #include <unordered_map>
 
@@ -30,16 +31,18 @@ namespace o3tl
  * for most of the operations with a combination unordered map and linked list.
  *
  **/
-template<typename Key, typename Value, class KeyHash = std::hash<Key>>
+template<typename Key, typename Value, class KeyHash = std::hash<Key>, class KeyEqual = std::equal_to<Key>>
 class lru_map final
 {
-private:
+public:
     typedef typename std::pair<Key, Value> key_value_pair_t;
+
+private:
     typedef std::list<key_value_pair_t> list_t;
     typedef typename list_t::iterator list_iterator_t;
     typedef typename list_t::const_iterator list_const_iterator_t;
 
-    typedef std::unordered_map<Key, list_iterator_t, KeyHash> map_t;
+    typedef std::unordered_map<Key, list_iterator_t, KeyHash, KeyEqual> map_t;
     typedef typename map_t::iterator map_iterator_t;
     typedef typename map_t::const_iterator map_const_iterator_t;
 
@@ -57,12 +60,14 @@ private:
             mLruList.pop_back();
         }
     }
+
 public:
     typedef list_iterator_t iterator;
     typedef list_const_iterator_t const_iterator;
 
+    // a size of 0 effectively disables the LRU cleanup code
     lru_map(size_t nMaxSize)
-        : mMaxSize(nMaxSize)
+        : mMaxSize(nMaxSize ? nMaxSize : std::min(mLruMap.max_size(), mLruList.max_size()))
     {}
 
     void insert(key_value_pair_t& rPair)
@@ -74,7 +79,8 @@ public:
             // add to front of the list
             mLruList.push_front(rPair);
             // add the list position (iterator) to the map
-            mLruMap[rPair.first] = mLruList.begin();
+            auto it = mLruList.begin();
+            mLruMap[it->first] = it;
             checkLRU();
         }
         else // already exists -> replace value
@@ -95,7 +101,8 @@ public:
             // add to front of the list
             mLruList.push_front(std::move(rPair));
             // add the list position (iterator) to the map
-            mLruMap[rPair.first] = mLruList.begin();
+            auto it = mLruList.begin();
+            mLruMap[it->first] = it;
             checkLRU();
         }
         else // already exists -> replace value
@@ -123,6 +130,28 @@ public:
         }
     }
 
+    // reverse-iterates the list removing all items matching the predicate
+    template<class UnaryPredicate>
+    void remove_if(UnaryPredicate pred)
+    {
+        auto it = mLruList.rbegin();
+        while (it != mLruList.rend())
+        {
+            if (pred(*it))
+            {
+                mLruMap.erase(it->first);
+                it = decltype(it){ mLruList.erase(std::next(it).base()) };
+            }
+            else
+                ++it;
+        }
+    }
+
+    const list_const_iterator_t begin() const
+    {
+        return mLruList.cbegin();
+    }
+
     const list_const_iterator_t end() const
     {
         return mLruList.cend();
@@ -130,7 +159,8 @@ public:
 
     size_t size() const
     {
-        return mLruList.size();
+        assert(mLruMap.size() == mLruList.size());
+        return mLruMap.size();
     }
 
     void clear()
