Bug 134013

Summary: Tag menu extremly slow
Product: [Applications] digikam Reporter: Thomas McGuire <mcguire>
Component: Usability-MenusAssignee: Digikam Developers <digikam-bugs-null>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: openSUSE   
OS: Linux   
Latest Commit: Version Fixed In: 0.9.0
Sentry Crash Report:

Description Thomas McGuire 2006-09-13 15:09:17 UTC
Version:           0.9.0-beta2 (SVN rev. 583773) (using KDE KDE 3.5.3)
Installed from:    SuSE RPMs
OS:                Linux

With recent versions of digikam SVN, the popup of the tag menu is unacceptable slow. When I right click an image and hover the mouse over the "Assing Tag" menu entry, it takes about 8 (!!) seconds to display the tag submenu. During these 8 seconds, the CPU usage is 100%,
I can reproduce this every time. It does not crash, so there is no backtrace. I do not get an assert or anything strange when running from within Konsole.
I have about 85 different tags, nested as deep as 4 levels. About half of them have thumbnails.

This might be related to bug 123623 or bug 131935, but I do not get crashes.
Comment 1 caulier.gilles 2006-09-13 15:12:29 UTC
Are you some messages from the console ?

Gilles Caulier
Comment 2 Thomas McGuire 2006-09-13 15:21:23 UTC
Actually, I just noticed the following upon startup:

digikam: ScanLib: Finding non-existing Albums: 8 ms
digikam: ScanLib: Finding items not in the database or disk: 775 ms
digikam: ScanLib: Updating items without date: 5 ms
digikam: Found dcraw version: 7.30
digikam: WARNING: Failed to find parent tag for tag Kepler Track with pid 39

No other warnings or anything.

And I miscounted, I have about 150 tags up to 5 levels deep.
Comment 3 Gerhard Kulzer 2006-09-13 15:37:07 UTC
Am Mittwoch, 13. September 2006 15:21 schrieb Thomas McGuire:
[bugs.kde.org quoted mail]
Here it's the same: takes a long time and no message in konsole.
Maybe the addressbooks tags are all read with the digikam tags?
Gerhard
Comment 4 caulier.gilles 2006-09-13 16:11:15 UTC
I cannot reproduce this problem on my computer. 

Marcel, can you reproduce it ?

Gilles
Comment 5 Mikolaj Machowski 2006-09-13 19:36:13 UTC
Cannot confirm for svn ver. 580463 (NOT "stable" beta2 package).

I have also tags from addressbook.
Comment 6 Marcel Wiesweg 2006-09-13 20:52:41 UTC
I assume this is related to bug 124688.
The icons are loaded each time the TagsPopupMenu is created, and loading 40 icons can take 8 s.

Question is: Do we need icons in the submenu? If yes, me must apply some sort of caching.
Comment 7 caulier.gilles 2006-09-13 22:07:21 UTC
Right Marcel,

Well, if we remove the tag icons in pop-up menu, i'm sure that we will have another file in B.K.O about (:=)))...

Doing a tag icons cache is certainly the better way. It can be used to another place to speed-up digiKam where tag icons are used, for example in tag folder view...

Gilles
Comment 8 Marcel Wiesweg 2006-09-16 22:15:18 UTC
SVN commit 585276 by mwiesweg:

Cache tag thumbnails
- add a cache (actually it is a QMap) to AlbumThumbnailLoader
- add a method to SyncJob to load icon for a TAlbum, using the AlbumThumbnailLoader
- use this in TagPopupMenu

BUG: 134013


 M  +59 -0     digikam/albumthumbnailloader.cpp  
 M  +7 -0      digikam/albumthumbnailloader.h  
 M  +61 -1     digikam/syncjob.cpp  
 M  +8 -0      digikam/syncjob.h  
 M  +3 -3      digikam/tagspopupmenu.cpp  
 M  +1 -1      libs/imageproperties/imagedescedittab.cpp  


--- trunk/extragear/graphics/digikam/digikam/albumthumbnailloader.cpp #585275:585276
@@ -43,6 +43,7 @@
 {
 
 typedef QMap<KURL, QValueList<int> > UrlAlbumMap;
+typedef QMap<int, QPixmap> TagThumbnailMap;
 
 class AlbumThumbnailLoaderPrivate
 {
@@ -63,9 +64,23 @@
 
     UrlAlbumMap             urlAlbumMap;
 
+    TagThumbnailMap         tagThumbnailMap;
+
     //QCache<QPixmap>        *m_cache;
 };
 
+class AlbumThumbnailLoaderEvent : public QCustomEvent
+{
+public:
+    AlbumThumbnailLoaderEvent(int albumID, const QPixmap &thumbnail)
+        : QCustomEvent(QEvent::User),
+          albumID(albumID), thumbnail(thumbnail)
+        {};
+
+    int albumID;
+    QPixmap thumbnail;
+};
+
 AlbumThumbnailLoader *AlbumThumbnailLoader::m_instance = 0;
 
 AlbumThumbnailLoader *AlbumThumbnailLoader::instance()
@@ -83,6 +98,12 @@
 AlbumThumbnailLoader::AlbumThumbnailLoader()
 {
     d = new AlbumThumbnailLoaderPrivate;
+
+    connect(AlbumManager::instance(), SIGNAL(signalAlbumIconChanged(Album*)),
+            this, SLOT(slotIconChanged(Album*)));
+
+    connect(AlbumManager::instance(), SIGNAL(signalAlbumDeleted(Album*)),
+            this, SLOT(slotIconChanged(Album*)));
 }
 
 AlbumThumbnailLoader::~AlbumThumbnailLoader()
@@ -190,6 +211,19 @@
     return pix;
     */
 
+    // First check cached thumbnails.
+    // At startup, this is not relevant, as the views will add their requests in a row.
+    // This is to speed up context menu and IE imagedescedit
+    TagThumbnailMap::iterator ttit = d->tagThumbnailMap.find(album->globalID());
+    if (ttit != d->tagThumbnailMap.end())
+    {
+        // It is not necessary to return cached icon asynchronously - they could be
+        // returned by getTagThumbnail already - but this would make the API
+        // less elegant, it feels much better this way.
+        QApplication::postEvent(this, new AlbumThumbnailLoaderEvent(album->globalID(), *ttit));
+        return;
+    }
+
     // Check if the URL has already been added (ThumbnailJob will _not_ check this)
     UrlAlbumMap::iterator it = d->urlAlbumMap.find(url);
 
@@ -291,6 +325,7 @@
                     if (tagThumbnail.isNull())
                     {
                         tagThumbnail = createTagThumbnail(thumbnail);
+                        d->tagThumbnailMap.insert(album->globalID(), tagThumbnail);
                     }
 
                     emit signalThumbnail(album, tagThumbnail);
@@ -307,6 +342,30 @@
 
 }
 
+void AlbumThumbnailLoader::customEvent(QCustomEvent *e)
+{
+    // for cached thumbnails
+
+    AlbumThumbnailLoaderEvent *atle = (AlbumThumbnailLoaderEvent *)e;
+    AlbumManager *manager = AlbumManager::instance();
+    Album *album = manager->findAlbum(atle->albumID);
+    if (album)
+    {
+        if (atle->thumbnail.isNull())
+            emit signalFailed(album);
+        else
+            emit signalThumbnail(album, atle->thumbnail);
+    }
+}
+
+void AlbumThumbnailLoader::slotIconChanged(Album* album)
+{
+    if(!album || album->type() != Album::TAG)
+        return;
+
+    d->tagThumbnailMap.remove(album->globalID());
+}
+
 QPixmap AlbumThumbnailLoader::createTagThumbnail(const QPixmap &albumThumbnail)
 {
     // tag thumbnails are cropped
--- trunk/extragear/graphics/digikam/digikam/albumthumbnailloader.h #585275:585276
@@ -30,6 +30,8 @@
 
 #include <kurl.h>
 
+class QCustomEvent;
+
 namespace Digikam
 {
 
@@ -110,7 +112,12 @@
 
     void slotGotThumbnailFromIcon(const KURL&, const QPixmap&);
     void slotThumbnailLost(const KURL&);
+    void slotIconChanged(Album* album);
 
+protected:
+
+    void customEvent(QCustomEvent *e);
+
 private:
 
     AlbumThumbnailLoader();
--- trunk/extragear/graphics/digikam/digikam/syncjob.cpp #585275:585276
@@ -44,6 +44,8 @@
 #include "syncjob.h"
 #include "thumbnailjob.h"
 #include "thumbnailsize.h"
+#include "albumthumbnailloader.h"
+#include "album.h"
 
 void qt_enter_modal( QWidget *widget );
 void qt_leave_modal( QWidget *widget );
@@ -70,6 +72,12 @@
     return sj.fileMovePriv(src, dest);
 }
 
+QPixmap SyncJob::getTagThumbnail(TAlbum *album)
+{
+    SyncJob sj;
+    return sj.getTagThumbnailPriv(album);
+}
+
 QPixmap SyncJob::getTagThumbnail(const QString &name, int size)
 {
     SyncJob sj;
@@ -79,6 +87,7 @@
 SyncJob::SyncJob()
 {
     thumbnail_ = 0;
+    album_ = 0;
 }
 
 SyncJob::~SyncJob()
@@ -152,6 +161,57 @@
     qApp->exit_loop();
 }
 
+QPixmap SyncJob::getTagThumbnailPriv(TAlbum *album)
+{
+    if (thumbnail_)
+        delete thumbnail_;
+    thumbnail_ = new QPixmap;
+
+    AlbumThumbnailLoader *loader = AlbumThumbnailLoader::instance();
+
+    if (!loader->getTagThumbnail(album, *thumbnail_))
+    {
+        if (thumbnail_->isNull())
+        {
+            return loader->getStandardTagIcon(album);
+        }
+        else
+        {
+            return loader->blendIcons(loader->getStandardTagIcon(), *thumbnail_);
+        }
+    }
+    else
+    {
+        connect(loader, SIGNAL(signalThumbnail(Album *, const QPixmap&)),
+                this, SLOT(slotGotThumbnailFromIcon(Album *, const QPixmap&)));
+
+        connect(loader, SIGNAL(signalFailed(Album *)),
+                this, SLOT(slotLoadThumbnailFailed(Album *)));
+
+        album_ = album;
+        enter_loop();
+    }
+    return *thumbnail_;
+}
+
+void SyncJob::slotLoadThumbnailFailed(Album *album)
+{
+    // TODO: setting _lastError*
+    if (album == album_)
+    {
+        qApp->exit_loop();
+    }
+}
+
+void SyncJob::slotGotThumbnailFromIcon(Album *album, const QPixmap& pix)
+{
+    if (album == album_)
+    {
+        *thumbnail_ = pix;
+        qApp->exit_loop();
+    }
+}
+
 QPixmap SyncJob::getTagThumbnailPriv(const QString &name, int size)
 {
     thumbnailSize_ = size;
@@ -181,7 +241,7 @@
     {
         KIconLoader *iconLoader = KApplication::kApplication()->iconLoader();
         *thumbnail_ = iconLoader->loadIcon(name, KIcon::NoGroup, thumbnailSize_,
-                                          KIcon::DefaultState, 0, true);
+                                           KIcon::DefaultState, 0, true);
     }
     return *thumbnail_;
 }
--- trunk/extragear/graphics/digikam/digikam/syncjob.h #585275:585276
@@ -46,6 +46,9 @@
 namespace Digikam
 {
 
+class Album;
+class TAlbum;
+
 class SyncJob : public QObject
 {
     Q_OBJECT
@@ -59,6 +62,7 @@
     static bool file_move(const KURL &src, const KURL &dest);
 
     /* Load the image or icon for the tag thumbnail */    
+    static QPixmap getTagThumbnail(TAlbum *album);
     static QPixmap getTagThumbnail(const QString &name, int size);
 
     static QString lastErrorMsg();
@@ -74,6 +78,7 @@
 
     bool fileMovePriv(const KURL &src, const KURL &dest);
     
+    QPixmap getTagThumbnailPriv(TAlbum *album);
     QPixmap getTagThumbnailPriv(const QString &name, int size);
 
     void enter_loop();
@@ -83,11 +88,14 @@
     bool            success_;
     
     QPixmap         *thumbnail_;
+    Album           *album_;
     int             thumbnailSize_;
 
 private slots:
 
     void slotResult( KIO::Job * job );
+    void slotGotThumbnailFromIcon(Album *album, const QPixmap& pix);
+    void slotLoadThumbnailFailed(Album *album);
     void slotGotThumbnailFromIcon(const KURL& url, const QPixmap& pix);
     void slotLoadThumbnailFailed();
 };
--- trunk/extragear/graphics/digikam/digikam/tagspopupmenu.cpp #585275:585276
@@ -155,7 +155,7 @@
                           ADDTAGID + album->id());
         popup->insertSeparator();
                 
-        QPixmap pix = SyncJob::getTagThumbnail(album->icon(), KIcon::SizeSmall);
+        QPixmap pix = SyncJob::getTagThumbnail(album);
         if ((m_mode == ASSIGN) && (m_assignedTags.contains(album->id())))
         {
             popup->insertItem(new TagsPopupCheckedMenuItem(popup, album->title(), pix),
@@ -175,7 +175,7 @@
     {
         if (!album->isRoot())
         {
-            QPixmap pix = SyncJob::getTagThumbnail(album->icon(), KIcon::SizeSmall);
+            QPixmap pix = SyncJob::getTagThumbnail(album);
             popup->insertItem(pix, album->title(), m_addToID + album->id());
             popup->insertSeparator();
         }
@@ -280,7 +280,7 @@
                 continue;
         }
         
-        QPixmap pix = SyncJob::getTagThumbnail(((TAlbum*)a)->icon(), KIcon::SizeSmall);
+        QPixmap pix = SyncJob::getTagThumbnail((TAlbum*)a);
         if (a->firstChild())
         {
             menu->insertItem(pix, a->title(), buildSubMenu(a->id()));
--- trunk/extragear/graphics/digikam/libs/imageproperties/imagedescedittab.cpp #585275:585276
@@ -915,7 +915,7 @@
             TAlbum* album = albumMan->findTAlbum(*it);
             if (album)
             {
-                QPixmap pix = SyncJob::getTagThumbnail(album->icon(), KIcon::SizeSmall);
+                QPixmap pix = SyncJob::getTagThumbnail(album);
                 QString text = album->title() + " (" + ((TAlbum*)album->parent())->prettyURL() + ")";
                 menu->insertItem(pix, text, album->id());
             }