simplify ImageList

we don't need an Impl pattern here

Change-Id: I93d0c5904a3aee8844987c7e36c19319d071d2f4
Reviewed-on: https://gerrit.libreoffice.org/75960
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

1 files changed, 80 insertions, 77 deletions

diff --git a/framework/source/uiconfiguration/ImageList.cxx b/framework/source/uiconfiguration/ImageList.cxx
index 0bb10d3d2bdb..100e152b17fc 100644
--- a/framework/source/uiconfiguration/ImageList.cxx
+++ b/framework/source/uiconfiguration/ImageList.cxx
@@ -29,7 +29,6 @@
 #include <vcl/image.hxx>
 #include <vcl/imagerepository.hxx>
 #include <vcl/ImageTree.hxx>
-#include "image.h"
 #include "ImageList.hxx"
 
 ImageList::ImageList()
@@ -41,48 +40,40 @@ ImageList::ImageList(const std::vector< OUString >& rNameVector,
 {
     SAL_INFO( "vcl", "vcl: ImageList::ImageList(const vector< OUString >& ..." );
 
-    ImplInit( sal::static_int_cast< sal_uInt16 >( rNameVector.size() ), Size() );
+    maImages.reserve( rNameVector.size() );
 
-    mpImplData->maPrefix = rPrefix;
+    maPrefix = rPrefix;
     for( size_t i = 0; i < rNameVector.size(); ++i )
     {
-        mpImplData->AddImage( rNameVector[ i ], static_cast< sal_uInt16 >( i ) + 1, BitmapEx() );
+        ImplAddImage( rNameVector[ i ], static_cast< sal_uInt16 >( i ) + 1, BitmapEx() );
     }
 }
 
-void ImageList::ImplInit( sal_uInt16 nItems, const Size &rSize )
-{
-    mpImplData.reset(new ImplImageList);
-    mpImplData->maImages.reserve( nItems );
-    mpImplData->maImageSize = rSize;
-}
-
 // FIXME: Rather a performance hazard
 BitmapEx ImageList::GetAsHorizontalStrip() const
 {
-    Size aSize( mpImplData->maImageSize );
-    sal_uInt16 nCount = GetImageCount();
+    sal_uInt16 nCount = maImages.size();
     if( !nCount )
         return BitmapEx();
-    aSize.setWidth( aSize.Width() * nCount );
+    Size aSize( maImageSize.Width() * nCount, maImageSize.Height() );
 
     // Load any stragglers
     for (sal_uInt16 nIdx = 0; nIdx < nCount; nIdx++)
     {
-        ImageAryData *pData = mpImplData->maImages[ nIdx ].get();
+        ImageAryData *pData = maImages[ nIdx ].get();
         if( pData->IsLoadable() )
-            pData->Load( mpImplData->maPrefix );
+            ImplLoad(*pData);
     }
 
-    BitmapEx aTempl = mpImplData->maImages[ 0 ]->maBitmapEx;
+    BitmapEx aTempl = maImages[ 0 ]->maBitmapEx;
     BitmapEx aResult( aTempl, Point(), aSize );
 
-    tools::Rectangle aSrcRect( Point( 0, 0 ), mpImplData->maImageSize );
+    tools::Rectangle aSrcRect( Point( 0, 0 ), maImageSize );
     for (sal_uInt16 nIdx = 0; nIdx < nCount; nIdx++)
     {
-        tools::Rectangle aDestRect( Point( nIdx * mpImplData->maImageSize.Width(), 0 ),
-                             mpImplData->maImageSize );
-        ImageAryData *pData = mpImplData->maImages[ nIdx ].get();
+        tools::Rectangle aDestRect( Point( nIdx * maImageSize.Width(), 0 ),
+                             maImageSize );
+        ImageAryData *pData = maImages[ nIdx ].get();
         aResult.CopyPixel( aDestRect, aSrcRect, &pData->maBitmapEx);
     }
 
@@ -101,32 +92,32 @@ void ImageList::InsertFromHorizontalStrip( const BitmapEx &rBitmapEx,
     DBG_ASSERT (rBitmapEx.GetSizePixel().Width() % nItems == 0,
                 "ImageList::InsertFromHorizontalStrip - very odd size");
     aSize.setWidth( aSize.Width() / nItems );
-    ImplInit( nItems, aSize );
+    maImages.clear();
+    maNameHash.clear();
+    maImages.reserve( nItems );
+    maImageSize = aSize;
+    maPrefix.clear();
 
     for (sal_uInt16 nIdx = 0; nIdx < nItems; nIdx++)
     {
         BitmapEx aBitmap( rBitmapEx, Point( nIdx * aSize.Width(), 0 ), aSize );
-        mpImplData->AddImage( rNameVector[ nIdx ], nIdx + 1, aBitmap );
+        ImplAddImage( rNameVector[ nIdx ], nIdx + 1, aBitmap );
     }
 }
 
 sal_uInt16 ImageList::ImplGetImageId( const OUString& rImageName ) const
 {
-    ImageAryData *pImg = mpImplData->maNameHash[ rImageName ];
-    if( pImg )
-        return pImg->mnId;
-    else
+    auto it = maNameHash.find( rImageName );
+    if (it == maNameHash.end())
         return 0;
+    return it->second->mnId;
 }
 
 void ImageList::AddImage( const OUString& rImageName, const Image& rImage )
 {
     SAL_WARN_IF( GetImagePos( rImageName ) != IMAGELIST_IMAGE_NOTFOUND, "vcl", "ImageList::AddImage() - ImageName already exists" );
 
-    if( !mpImplData )
-        ImplInit( 0, rImage.GetSizePixel() );
-
-    mpImplData->AddImage( rImageName, GetImageCount() + 1,
+    ImplAddImage( rImageName, GetImageCount() + 1,
                           rImage.GetBitmapEx() );
 }
 
@@ -138,18 +129,18 @@ void ImageList::ReplaceImage( const OUString& rImageName, const Image& rImage )
     {
         //Just replace the bitmap rather than doing RemoveImage / AddImage
         //which breaks index-based iteration.
-        ImageAryData *pImg = mpImplData->maNameHash[ rImageName ];
+        ImageAryData *pImg = maNameHash[ rImageName ];
         pImg->maBitmapEx = rImage.GetBitmapEx();
     }
 }
 
 void ImageList::RemoveImage( sal_uInt16 nId )
 {
-    for( size_t i = 0; i < mpImplData->maImages.size(); ++i )
+    for( size_t i = 0; i < maImages.size(); ++i )
     {
-        if( mpImplData->maImages[ i ]->mnId == nId )
+        if( maImages[ i ]->mnId == nId )
         {
-            mpImplData->RemoveImage( static_cast< sal_uInt16 >( i ) );
+            ImplRemoveImage( static_cast< sal_uInt16 >( i ) );
             break;
         }
     }
@@ -157,33 +148,27 @@ void ImageList::RemoveImage( sal_uInt16 nId )
 
 Image ImageList::GetImage( const OUString& rImageName ) const
 {
-    if( mpImplData )
-    {
-        ImageAryData *pImg = mpImplData->maNameHash[ rImageName ];
-
-        if( pImg )
-        {
-            if( pImg->IsLoadable() )
-                pImg->Load( mpImplData->maPrefix );
-            return Image( pImg->maBitmapEx );
-        }
-    }
-
-    return Image();
+    auto it = maNameHash.find( rImageName );
+    if (it == maNameHash.end())
+        return Image();
+    ImageAryData *pImg = it->second;
+    if( pImg->IsLoadable() )
+        ImplLoad( *pImg );
+    return Image( pImg->maBitmapEx );
 }
 
 sal_uInt16 ImageList::GetImageCount() const
 {
-    return mpImplData ? static_cast< sal_uInt16 >( mpImplData->maImages.size() ) : 0;
+    return static_cast< sal_uInt16 >( maImages.size() );
 }
 
 sal_uInt16 ImageList::GetImagePos( const OUString& rImageName ) const
 {
-    if( mpImplData && !rImageName.isEmpty() )
+    if( !rImageName.isEmpty() )
     {
-        for( size_t i = 0; i < mpImplData->maImages.size(); i++ )
+        for( size_t i = 0; i < maImages.size(); i++ )
         {
-            if (mpImplData->maImages[i]->maName == rImageName)
+            if (maImages[i]->maName == rImageName)
                 return static_cast< sal_uInt16 >( i );
         }
     }
@@ -193,18 +178,12 @@ sal_uInt16 ImageList::GetImagePos( const OUString& rImageName ) const
 
 sal_uInt16 ImageList::GetImageId( sal_uInt16 nPos ) const
 {
-    if( mpImplData && (nPos < GetImageCount()) )
-        return mpImplData->maImages[ nPos ]->mnId;
-
-    return 0;
+    return maImages[ nPos ]->mnId;
 }
 
 OUString ImageList::GetImageName( sal_uInt16 nPos ) const
 {
-    if( mpImplData && (nPos < GetImageCount()) )
-        return mpImplData->maImages[ nPos ]->maName;
-
-    return OUString();
+    return maImages[ nPos ]->maName;
 }
 
 void ImageList::GetImageNames( std::vector< OUString >& rNames ) const
@@ -213,30 +192,54 @@ void ImageList::GetImageNames( std::vector< OUString >& rNames ) const
 
     rNames = std::vector< OUString >();
 
-    if( mpImplData )
+    for(auto const & pImage : maImages)
     {
-        for(auto const & pImage : mpImplData->maImages)
-        {
-            const OUString& rName( pImage->maName );
-            if( !rName.isEmpty())
-                rNames.push_back( rName );
-        }
+        const OUString& rName( pImage->maName );
+        if( !rName.isEmpty())
+            rNames.push_back( rName );
     }
 }
 
-bool ImageList::operator==( const ImageList& rImageList ) const
+void ImageList::ImplAddImage( const OUString &aName,
+                              sal_uInt16 nId, const BitmapEx &aBitmapEx )
+{
+    ImageAryData *pImg = new ImageAryData{ aName, nId, aBitmapEx };
+    maImages.emplace_back( pImg );
+    if( !aName.isEmpty() )
+        maNameHash [ aName ] = pImg;
+}
+
+void ImageList::ImplRemoveImage( sal_uInt16 nPos )
+{
+    ImageAryData *pImg = maImages[ nPos ].get();
+    if( !pImg->maName.isEmpty() )
+        maNameHash.erase( pImg->maName );
+    maImages.erase( maImages.begin() + nPos );
+}
+
+void ImageList::ImplLoad(ImageAryData& rImageData) const
 {
-    bool bRet = false;
+    OUString aIconTheme = Application::GetSettings().GetStyleSettings().DetermineIconTheme();
 
-    if( rImageList.mpImplData == mpImplData )
-        bRet = true;
-    else if( !rImageList.mpImplData || !mpImplData )
-        bRet = false;
-    else if( rImageList.GetImageCount() == GetImageCount() &&
-              rImageList.mpImplData->maImageSize == mpImplData->maImageSize )
-        bRet = true; // strange semantic
+    OUString aFileName = maPrefix + rImageData.maName;
 
-    return bRet;
+    bool bSuccess = ImageTree::get().loadImage(aFileName, aIconTheme, rImageData.maBitmapEx, true);
+
+    /* If the uno command has parameters, passed in from a toolbar,
+     * recover from failure by removing the parameters from the file name
+     */
+    if (!bSuccess && aFileName.indexOf("%3f") > 0)
+    {
+        sal_Int32 nStart = aFileName.indexOf("%3f");
+        sal_Int32 nEnd = aFileName.lastIndexOf(".");
+
+        aFileName = aFileName.replaceAt(nStart, nEnd - nStart, "");
+        bSuccess = ImageTree::get().loadImage(aFileName, aIconTheme, rImageData.maBitmapEx, true);
+    }
+
+    SAL_WARN_IF(!bSuccess, "fwk.uiconfiguration", "Failed to load image '" << aFileName
+              << "' from icon theme '" << aIconTheme << "'");
 }
 
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */