svg icon caching even for 1x scaling

Previously we cached an icon only if we changed the scaling factor
or re-colored the icon, however if we have svg icons we want to
cache all of the variants, including the unscaled/un-colored one
as svg rendering is slower than loading cached png icons.

Change-Id: Ie2c14687892bbd12b8e04c690a66c5a1a92ac8b0
Reviewed-on: https://gerrit.libreoffice.org/47498Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>
(cherry picked from commit 57d95030)

vcl/inc/implimagetree.hxx

@@ -45,6 +45,7 @@ struct ImageRequestParameters
     BitmapEx& mrBitmap;
     bool mbLocalized;
     ImageLoadFlags meFlags;
+    bool mbWriteImageToCache;
 
     ImageRequestParameters(const OUString & rName, const OUString & rStyle, BitmapEx& rBitmap, bool bLocalized, ImageLoadFlags eFlags)
         : msName(rName)
@@ -52,6 +53,7 @@ struct ImageRequestParameters
         , mrBitmap(rBitmap)
         , mbLocalized(bLocalized)
         , meFlags(eFlags)
+        , mbWriteImageToCache(false)
     {}
 
     bool convertToDarkTheme();

vcl/source/image/ImplImageTree.cxx

@@ -156,6 +156,7 @@ void loadImageFromStream(std::shared_ptr<SvStream> const & xStream, OUString con
     }
     else if (rPath.endsWith(".svg"))
     {
+        rParameters.mbWriteImageToCache = true; // We always want to cache a SVG image
         vcl::bitmap::loadFromSvg(*xStream.get(), rPath, rParameters.mrBitmap, aScalePercentage / 100.0);
         if (bConvertToDarkTheme)
             rParameters.mrBitmap = BitmapProcessor::createLightImage(rParameters.mrBitmap);
@@ -167,10 +168,14 @@ void loadImageFromStream(std::shared_ptr<SvStream> const & xStream, OUString con
     }
 
     if (bConvertToDarkTheme)
+    {
+        rParameters.mbWriteImageToCache = true; // Cache the dark variant
         rParameters.mrBitmap = BitmapProcessor::createLightImage(rParameters.mrBitmap);
+    }
 
     if (aScalePercentage > 100)
     {
+        rParameters.mbWriteImageToCache = true; // Cache the scaled variant
         double aScaleFactor(aScalePercentage / 100.0);
         rParameters.mrBitmap.Scale(aScaleFactor, aScaleFactor, BmpScaleFlag::Fast);
     }
@@ -267,21 +272,21 @@ OUString ImplImageTree::fallbackStyle(const OUString& rsStyle)
     return sResult;
 }
 
-bool ImplImageTree::loadImage(OUString const & name, OUString const & style, BitmapEx & rBitmap, bool localized, const ImageLoadFlags eFlags)
+bool ImplImageTree::loadImage(OUString const & rName, OUString const & rStyle, BitmapEx & rBitmap, bool localized, const ImageLoadFlags eFlags)
 {
-    OUString aStyle(style);
-    while (!aStyle.isEmpty())
+    OUString aCurrentStyle(rStyle);
+    while (!aCurrentStyle.isEmpty())
     {
         try
         {
-            ImageRequestParameters aParameters(name, aStyle, rBitmap, localized, eFlags);
+            ImageRequestParameters aParameters(rName, aCurrentStyle, rBitmap, localized, eFlags);
             if (doLoadImage(aParameters))
                 return true;
         }
         catch (css::uno::RuntimeException &)
         {}
 
-        aStyle = fallbackStyle(aStyle);
+        aCurrentStyle = fallbackStyle(aCurrentStyle);
     }
     return false;
 }
@@ -291,11 +296,7 @@ OUString createVariant(ImageRequestParameters& rParameters)
     bool bConvertToDarkTheme = rParameters.convertToDarkTheme();
     sal_Int32 aScalePercentage = rParameters.scalePercentage();
 
-    OUString aVariant;
-    if (aScalePercentage == 100 && !bConvertToDarkTheme)
-        return aVariant;
-
-    aVariant = OUString::number(aScalePercentage);
+    OUString aVariant = OUString::number(aScalePercentage);
 
     if (bConvertToDarkTheme)
         aVariant += "-dark";
@@ -310,7 +311,7 @@ bool loadDiskCachedVersion(OUString const & sVariant, ImageRequestParameters& rP
         return false;
     SvFileStream aFileStream(sUrl, StreamMode::READ);
     vcl::PNGReader aPNGReader(aFileStream);
-    aPNGReader.SetIgnoreGammaChunk( true );
+    aPNGReader.SetIgnoreGammaChunk(true);
     rParameters.mrBitmap = aPNGReader.Read();
     return true;
 }
@@ -336,18 +337,24 @@ bool ImplImageTree::doLoadImage(ImageRequestParameters& rParameters)
     if (iconCacheLookup(rParameters))
         return true;
 
+    OUString aVariant = createVariant(rParameters);
+    if (loadDiskCachedVersion(aVariant, rParameters))
+    {
+        return true;
+    }
+
     if (!rParameters.mrBitmap.IsEmpty())
         rParameters.mrBitmap.SetEmpty();
 
     LanguageTag aLanguageTag = Application::GetSettings().GetUILanguageTag();
 
-    std::vector<OUString> paths = getPaths(rParameters.msName, aLanguageTag);
+    std::vector<OUString> aPaths = getPaths(rParameters.msName, aLanguageTag);
 
     bool bFound = false;
 
     try
     {
-        bFound = findImage(paths, rParameters);
+        bFound = findImage(aPaths, rParameters);
     }
     catch (css::uno::RuntimeException&)
     {
@@ -355,14 +362,15 @@ bool ImplImageTree::doLoadImage(ImageRequestParameters& rParameters)
     }
     catch (const css::uno::Exception& e)
     {
-        SAL_INFO("vcl", "ImplImageTree::doLoadImage " << e);
+        SAL_INFO("vcl", "ImplImageTree::doLoadImage exception: " << e);
     }
 
     if (bFound)
     {
-        OUString aVariant = createVariant(rParameters);
-        if (!aVariant.isEmpty())
+        if (rParameters.mbWriteImageToCache)
+        {
             cacheBitmapToDisk(aVariant, rParameters);
+        }
         getCurrentIconSet().maIconCache[rParameters.msName] = std::make_pair(rParameters.mbLocalized, rParameters.mrBitmap);
     }
 
@@ -442,18 +450,17 @@ bool ImplImageTree::iconCacheLookup(ImageRequestParameters& rParameters)
         return true;
     }
 
-    OUString aVariant = createVariant(rParameters);
-    return !aVariant.isEmpty() && loadDiskCachedVersion(aVariant, rParameters);
+    return false;
 }
 
-bool ImplImageTree::findImage(std::vector<OUString> const & paths, ImageRequestParameters& rParameters)
+bool ImplImageTree::findImage(std::vector<OUString> const & rPaths, ImageRequestParameters& rParameters)
 {
     if (!checkPathAccess())
         return false;
 
-    const css::uno::Reference<css::container::XNameAccess>& rNameAccess = getCurrentIconSet().maNameAccess;
+    css::uno::Reference<css::container::XNameAccess> const & rNameAccess = getCurrentIconSet().maNameAccess;
 
-    for (const OUString& rPath : paths)
+    for (OUString const & rPath : rPaths)
     {
         if (rNameAccess->hasByName(rPath))
         {