From 542feef77d41fe1c257643a43e6d6fe5eb4ba533 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles@charleskerr.com>
Date: Tue, 8 Sep 2020 11:51:33 -0500
Subject: [PATCH] refactor: make torrents' mime icons lazy-loaded. (#1433)

* refactor: make torrents' mime icons lazy-loaded.

Another "small wins" patch, this time on CPU: icon loading is expensive,
so calculating them for every torrent all at once on startup isn't the
smartest way to do it.
---
 qt/FileTreeModel.cc | 20 ++++++++------------
 qt/Torrent.cc       | 39 ++++++++++++++++++++-------------------
 qt/Torrent.h        | 10 +++-------
 qt/TorrentModel.cc  | 10 ++++------
 4 files changed, 35 insertions(+), 44 deletions(-)

diff --git a/qt/FileTreeModel.cc b/qt/FileTreeModel.cc
index 3453d06b5..6b3c1e39c 100644
--- a/qt/FileTreeModel.cc
+++ b/qt/FileTreeModel.cc
@@ -152,14 +152,12 @@ QModelIndexList FileTreeModel::getOrphanIndices(QModelIndexList const& indices)
 
 QVariant FileTreeModel::data(QModelIndex const& index, int role) const
 {
-    QVariant value;
-
     if (index.isValid())
     {
-        value = itemFromIndex(index)->data(index.column(), role);
+        return itemFromIndex(index)->data(index.column(), role);
     }
 
-    return value;
+    return {};
 }
 
 Qt::ItemFlags FileTreeModel::flags(QModelIndex const& index) const
@@ -193,30 +191,28 @@ bool FileTreeModel::setData(QModelIndex const& index, QVariant const& newname, i
 
 QVariant FileTreeModel::headerData(int column, Qt::Orientation orientation, int role) const
 {
-    QVariant data;
-
     if (orientation == Qt::Horizontal && role == Qt::DisplayRole)
     {
         switch (column)
         {
         case COL_NAME:
-            data.setValue(tr("File"));
+            return tr("File");
             break;
 
         case COL_SIZE:
-            data.setValue(tr("Size"));
+            return tr("Size");
             break;
 
         case COL_PROGRESS:
-            data.setValue(tr("Progress"));
+            return tr("Progress");
             break;
 
         case COL_WANTED:
-            data.setValue(tr("Download"));
+            return tr("Download");
             break;
 
         case COL_PRIORITY:
-            data.setValue(tr("Priority"));
+            return tr("Priority");
             break;
 
         default:
@@ -224,7 +220,7 @@ QVariant FileTreeModel::headerData(int column, Qt::Orientation orientation, int
         }
     }
 
-    return data;
+    return {};
 }
 
 QModelIndex FileTreeModel::index(int row, int column, QModelIndex const& parent) const
diff --git a/qt/Torrent.cc b/qt/Torrent.cc
index f55c15f58..790e57958 100644
--- a/qt/Torrent.cc
+++ b/qt/Torrent.cc
@@ -161,27 +161,28 @@ int Torrent::compareETA(Torrent const& that) const
 ****
 ***/
 
-void Torrent::updateMimeIcon()
+QIcon Torrent::getMimeTypeIcon() const
 {
-    auto const& files = files_;
-    auto const& icon_cache = IconCache::get();
+    if (icon_.isNull())
+    {
+        auto const& files = files_;
+        auto const& icon_cache = IconCache::get();
 
-    QIcon icon;
-
-    if (files.size() > 1)
-    {
-        icon = icon_cache.folderIcon();
-    }
-    else if (files.size() == 1)
-    {
-        icon = icon_cache.guessMimeIcon(files.at(0).filename, icon_cache.fileIcon());
-    }
-    else
-    {
-        icon = icon_cache.guessMimeIcon(name(), icon_cache.folderIcon());
+        if (files.size() > 1)
+        {
+            icon_ = icon_cache.folderIcon();
+        }
+        else if (files.size() == 1)
+        {
+            icon_ = icon_cache.guessMimeIcon(files.at(0).filename, icon_cache.fileIcon());
+        }
+        else
+        {
+            icon_ = icon_cache.guessMimeIcon(name(), icon_cache.folderIcon());
+        }
     }
 
-    icon_ = icon;
+    return icon_;
 }
 
 /***
@@ -283,13 +284,13 @@ Torrent::fields_t Torrent::update(tr_quark const* keys, tr_variant const* const*
             {
             case TR_KEY_name:
                 {
-                    updateMimeIcon();
+                    icon_ = {};
                     break;
                 }
 
             case TR_KEY_files:
                 {
-                    updateMimeIcon();
+                    icon_ = {};
                     for (int i = 0; i < files_.size(); ++i)
                     {
                         files_[i].index = i;
diff --git a/qt/Torrent.h b/qt/Torrent.h
index b56ab155e..aaea860d5 100644
--- a/qt/Torrent.h
+++ b/qt/Torrent.h
@@ -540,10 +540,7 @@ public:
         return isWaitingToDownload() || isWaitingToSeed();
     }
 
-    QIcon getMimeTypeIcon() const
-    {
-        return icon_;
-    }
+    QIcon getMimeTypeIcon() const;
 
     enum Field
     {
@@ -609,8 +606,6 @@ public:
     fields_t update(tr_quark const* keys, tr_variant const* const* values, size_t n);
 
 private:
-    void updateMimeIcon();
-
     int const id_;
 
     bool download_limited_ = {};
@@ -666,7 +661,8 @@ private:
     QString error_string_;
     QString name_;
 
-    QIcon icon_;
+    // mutable because it's a lazy lookup
+    mutable QIcon icon_;
 
     PeerList peers_;
     FileList files_;
diff --git a/qt/TorrentModel.cc b/qt/TorrentModel.cc
index 855400678..576e44d51 100644
--- a/qt/TorrentModel.cc
+++ b/qt/TorrentModel.cc
@@ -93,8 +93,6 @@ int TorrentModel::rowCount(QModelIndex const& parent) const
 
 QVariant TorrentModel::data(QModelIndex const& index, int role) const
 {
-    QVariant var;
-
     Torrent const* t = torrents_.value(index.row(), nullptr);
 
     if (t != nullptr)
@@ -102,15 +100,15 @@ QVariant TorrentModel::data(QModelIndex const& index, int role) const
         switch (role)
         {
         case Qt::DisplayRole:
-            var.setValue(t->name());
+            return t->name();
             break;
 
         case Qt::DecorationRole:
-            var.setValue(t->getMimeTypeIcon());
+            return t->getMimeTypeIcon();
             break;
 
         case TorrentRole:
-            var = QVariant::fromValue(t);
+            return QVariant::fromValue(t);
             break;
 
         default:
@@ -119,7 +117,7 @@ QVariant TorrentModel::data(QModelIndex const& index, int role) const
         }
     }
 
-    return var;
+    return {};
 }
 
 /***