Don't crop cached pixmaps anymore.

Scale (preserving aspect ratio) and cache them by (hash, width)
only. Cropping doesn't really save any paint time since
QPainter::drawPixmap is cheap when you don't have to scale the pixmap.

src/library/coverartcache.cpp

@@ -9,8 +9,8 @@
 #include "soundsourceproxy.h"
 
 // Large cover art wastes space in our cache when we typicaly won't show them at
-// their full size. This is the max side length we resize images to.
-const int kMaxCoverSize = 300;
+// their full size. If no width is specified, this is the maximum width cap.
+const int kMaxCoverWidth = 300;
 
 const bool sDebug = false;
 
@@ -30,12 +30,12 @@ CoverArtCache::~CoverArtCache() {
 }
 
 QPixmap CoverArtCache::requestCover(const CoverInfo& requestInfo,
-                                    const QSize& croppedSize,
+                                    const int desiredWidth,
                                     const bool onlyCached,
                                     const bool signalWhenDone) {
     if (sDebug) {
         qDebug() << "CoverArtCache::requestCover"
-                 << requestInfo << croppedSize << onlyCached << signalWhenDone;
+                 << requestInfo << desiredWidth << onlyCached << signalWhenDone;
     }
 
     // TODO(rryan) handle requests for non-library tracks.
@@ -63,7 +63,7 @@ QPixmap CoverArtCache::requestCover(const CoverInfo& requestInfo,
     // having to rescale+crop it ALWAYS (which brings a lot of performance issues).
     if (!requestInfo.hash.isEmpty()) {
         QString cacheKey = CoverArtUtils::pixmapCacheKey(requestInfo.hash,
-                                                         croppedSize);
+                                                         desiredWidth);
 
         QPixmap pixmap;
         if (QPixmapCache::find(cacheKey, &pixmap)) {
@@ -84,7 +84,7 @@ QPixmap CoverArtCache::requestCover(const CoverInfo& requestInfo,
     m_runningIds.insert(requestInfo.trackId);
     QFutureWatcher<FutureResult>* watcher = new QFutureWatcher<FutureResult>(this);
     QFuture<FutureResult> future = QtConcurrent::run(
-            this, &CoverArtCache::loadCover, requestInfo, croppedSize,
+            this, &CoverArtCache::loadCover, requestInfo, desiredWidth,
             signalWhenDone);
     connect(watcher, SIGNAL(finished()), this, SLOT(coverLoaded()));
     watcher->setFuture(future);
@@ -93,16 +93,16 @@ QPixmap CoverArtCache::requestCover(const CoverInfo& requestInfo,
 
 CoverArtCache::FutureResult CoverArtCache::loadCover(
         const CoverInfo& info,
-        const QSize& croppedSize,
+        const int desiredWidth,
         const bool signalWhenDone) {
     if (sDebug) {
         qDebug() << "CoverArtCache::loadCover"
-                 << info << croppedSize << signalWhenDone;
+                 << info << desiredWidth << signalWhenDone;
     }
 
     FutureResult res;
     res.cover.info = info;
-    res.croppedSize = croppedSize;
+    res.desiredWidth = desiredWidth;
     res.signalWhenDone = signalWhenDone;
     res.cover.image = CoverArtUtils::loadCover(res.cover.info);
 
@@ -117,12 +117,12 @@ CoverArtCache::FutureResult CoverArtCache::loadCover(
 
     // Adjust the cover size according to the request or downsize the image for
     // efficiency.
-    if (res.croppedSize.isNull()) {
-        res.cover.image = CoverArtUtils::maybeResizeImage(res.cover.image,
-                                                          kMaxCoverSize);
+    if (res.desiredWidth > 0) {
+        res.cover.image = CoverArtUtils::resizeImage(res.cover.image,
+                                                     res.desiredWidth);
     } else {
-        res.cover.image = CoverArtUtils::cropImage(res.cover.image,
-                                                   res.croppedSize);
+        res.cover.image = CoverArtUtils::maybeResizeImage(res.cover.image,
+                                                          kMaxCoverWidth);
     }
 
     return res;
@@ -139,7 +139,7 @@ void CoverArtCache::coverLoaded() {
     }
 
     QString cacheKey = CoverArtUtils::pixmapCacheKey(res.cover.info.hash,
-                                                     res.croppedSize);
+                                                     res.desiredWidth);
     QPixmap pixmap;
     if (!QPixmapCache::find(cacheKey, &pixmap) && !res.cover.image.isNull()) {
         pixmap.convertFromImage(res.cover.image);

src/library/coverartcache.h

@@ -11,13 +11,6 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
     Q_OBJECT
   public:
     /* This method is used to request a cover art pixmap.
-     *
-     * @param croppedSize : QSize(finalCoverWidth, finalCoverHeight)
-     *      it determines the final cover size.
-     *      Use QSize() to get the original size.
-     *      NOTE!
-     *          the cover will be resized to 'finalCoverWidth' and
-     *          it'll be cropped from the top until the finalCoverHeight' pixel
      *
      * @param onlyCached : if it is 'true', the method will NOT try to load
      *      covers from the given 'coverLocation' and it will also NOT run the
@@ -26,13 +19,13 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
      *      a Pixmap if it is already loaded in the QPixmapCache.
      */
     QPixmap requestCover(const CoverInfo& info,
-                         const QSize& croppedSize = QSize(0,0),
+                         const int desiredWidth = 0,
                          const bool onlyCached = false,
                          const bool signalWhenDone = true);
 
     struct FutureResult {
         CoverArt cover;
-        QSize croppedSize;
+        int desiredWidth;
         bool signalWhenDone;
     };
 
@@ -51,7 +44,7 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
     // Load cover from path indicated in coverInfo. WARNING: This is run in a
     // worker thread.
     FutureResult loadCover(const CoverInfo& coverInfo,
-                           const QSize &croppedSize,
+                           const int desiredWidth,
                            const bool emitSignals);
 
   private:

src/library/coverartdelegate.cpp

@@ -1,6 +1,7 @@
 #include "library/coverartdelegate.h"
 #include "library/coverartcache.h"
 #include "library/dao/trackdao.h"
+#include "util/math.h"
 
 CoverArtDelegate::CoverArtDelegate(QObject *parent)
         : QStyledItemDelegate(parent),
@@ -78,23 +79,15 @@ void CoverArtDelegate::paint(QPainter *painter,
     info.trackId = index.sibling(index.row(), m_iIdColumn).data().toInt();
     info.trackLocation = index.sibling(index.row(), m_iTrackLocationColumn).data().toString();
 
-    QSize coverSize(100, option.rect.height());
-
     // Do not signal when done sine we don't listen to CoverArtCache for updates
     // and instead refresh on a timer in WTrackTableView.
-    QPixmap pixmap = pCache->requestCover(info, coverSize,
-                                          m_bOnlyCachedCover, false);
-
-
+    QPixmap pixmap = pCache->requestCover(info, 100, m_bOnlyCachedCover, false);
     if (!pixmap.isNull()) {
-        int width = pixmap.width();
-        if (option.rect.width() < width) {
-            width = option.rect.width();
-        }
-
+        int width = math_min(pixmap.width(), option.rect.width());
+        int height = math_min(pixmap.height(), option.rect.height());
         QRect target(option.rect.x(), option.rect.y(),
-                     width, option.rect.height());
-        QRect source(0, 0, width, pixmap.height());
+                     width, height);
+        QRect source(0, 0, target.width(), target.height());
         painter->drawPixmap(target, pixmap, source);
     }
 }

src/library/coverartutils.h

@@ -22,14 +22,12 @@ class CoverArtUtils {
     }
 
     static QString pixmapCacheKey(const QString& hash,
-                                  const QSize& size) {
-        if (size.isNull()) {
+                                  const int width) {
+        if (width == 0) {
             return QString("CoverArtCache_%1").arg(hash);
         }
-        return QString("CoverArtCache_%1_%2x%3")
-                .arg(hash)
-                .arg(size.width())
-                .arg(size.height());
+        return QString("CoverArtCache_%1_%2")
+                .arg(hash).arg(width);
     }
 
     // Extracts the first cover art image embedded within the file at
@@ -87,18 +85,6 @@ class CoverArtUtils {
                       image.byteCount()));
     }
 
-    // Crops image to the provided size by first scaling to the appropriate
-    // width and then cropping off the bottom. If size is taller than the image,
-    // black pixels are padded on the bottom.
-    static QImage cropImage(const QImage& image, const QSize& size) {
-        if (image.isNull()) {
-            return QImage();
-        }
-        QImage result = image.scaledToWidth(size.width(),
-                                            Qt::SmoothTransformation);
-        return result.copy(0, 0, image.width(), size.height());
-    }
-
     // Resizes the image (preserving aspect ratio) if it is larger than
     // maxEdgeSize on either side.
     static QImage maybeResizeImage(const QImage& image, int maxEdgeSize) {
@@ -109,6 +95,11 @@ class CoverArtUtils {
         return image;
     }
 
+    // Resizes the image (preserving aspect ratio) to width.
+    static QImage resizeImage(const QImage& image, int width) {
+        return image.scaledToWidth(width, Qt::SmoothTransformation);
+    }
+
     static QStringList supportedCoverArtExtensions() {
         QStringList extensions;
         extensions << "jpg" << "jpeg" << "png" << "gif" << "bmp";

src/test/coverartcache_test.cpp

@@ -71,7 +71,7 @@ TEST_F(CoverArtCacheTest, loadCover) {
     info.hash = "coverhash"; // fake cover hash
 
     CoverArtCache::FutureResult res;
-    res = CoverArtCache::loadCover(info, QSize(0,0), false);
+    res = CoverArtCache::loadCover(info, 0, false);
     EXPECT_EQ(info.trackId, res.cover.info.trackId);
     EXPECT_QSTRING_EQ(info.coverLocation, res.cover.info.coverLocation);
     EXPECT_QSTRING_EQ(info.hash, res.cover.info.hash);
@@ -82,7 +82,7 @@ TEST_F(CoverArtCacheTest, loadCover) {
     info.source = CoverInfo::GUESSED;
     info.coverLocation = QString();
     info.trackLocation = kTrackLocationTest;
-    res = CoverArtCache::loadCover(info, QSize(0,0), false);
+    res = CoverArtCache::loadCover(info, 0, false);
     EXPECT_EQ(info.trackId, res.cover.info.trackId);
     EXPECT_QSTRING_EQ(QString(), res.cover.info.coverLocation);
     EXPECT_QSTRING_EQ(info.hash, res.cover.info.hash);
@@ -267,14 +267,14 @@ TEST_F(CoverArtCacheTest, searchImage) {
     // cLoc_filename = QString(trackdir % "/" % trackBaseName % ".");
     // cLoc_filename.append(format);
     // EXPECT_TRUE(img.save(cLoc_filename, format));
-    // res = CoverArtCache::searchImage(cInfoUtf8, QSize(0,0), false);
+    // res = CoverArtCache::searchImage(cInfoUtf8, 0, false);
     // EXPECT_QSTRING_EQ(cLoc_filename, res.cover.info.coverLocation);
     // QFile::remove(cLoc_filename);
     // // 2. album_name.jpg
     // cLoc_albumName = QString(trackdir % "/" % cInfoUtf8.album % ".");
     // cLoc_albumName.append(format);
     // EXPECT_TRUE(img.save(cLoc_albumName, format));
-    // res = CoverArtCache::searchImage(cInfoUtf8, QSize(0,0), false);
+    // res = CoverArtCache::searchImage(cInfoUtf8, 0, false);
     // EXPECT_QSTRING_EQ(cLoc_albumName, res.cover.info.coverLocation);
     // QFile::remove(cLoc_albumName);