From b499f1146fe7bcd4c42ac65a3f12422c284883e0 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Tue, 15 Jul 2014 19:58:09 +0100 Subject: [PATCH 01/43] Add cover art column to librarymodel - it will show the covers --- src/library/basesqltablemodel.cpp | 2 ++ src/library/columncache.cpp | 1 + src/library/columncache.h | 1 + src/library/dao/trackdao.h | 1 + src/library/librarytablemodel.cpp | 4 +++- 5 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/library/basesqltablemodel.cpp b/src/library/basesqltablemodel.cpp index 83df44dc86f..a5df438f920 100644 --- a/src/library/basesqltablemodel.cpp +++ b/src/library/basesqltablemodel.cpp @@ -91,6 +91,8 @@ void BaseSqlTableModel::initHeaderData() { Qt::Horizontal, tr("BPM Lock")); setHeaderData(fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_PREVIEW), Qt::Horizontal, tr("Preview")); + setHeaderData(fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_COVERART), + Qt::Horizontal, tr("Cover Art")); } QSqlDatabase BaseSqlTableModel::database() const { diff --git a/src/library/columncache.cpp b/src/library/columncache.cpp index 5d7a5815e6d..933dbe348c0 100644 --- a/src/library/columncache.cpp +++ b/src/library/columncache.cpp @@ -46,6 +46,7 @@ void ColumnCache::setColumns(const QStringList& columns) { m_columnIndexByEnum[COLUMN_LIBRARYTABLE_KEY_ID] = fieldIndex(LIBRARYTABLE_KEY_ID); m_columnIndexByEnum[COLUMN_LIBRARYTABLE_BPM_LOCK] = fieldIndex(LIBRARYTABLE_BPM_LOCK); m_columnIndexByEnum[COLUMN_LIBRARYTABLE_PREVIEW] = fieldIndex(LIBRARYTABLE_PREVIEW); + m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COVERART] = fieldIndex(LIBRARYTABLE_COVERART); m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COVERART_LOCATION] = fieldIndex(LIBRARYTABLE_COVERART_LOCATION); m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COVERART_MD5] = fieldIndex(LIBRARYTABLE_COVERART_MD5); diff --git a/src/library/columncache.h b/src/library/columncache.h index cb4d8e801e1..ccffa10baa6 100644 --- a/src/library/columncache.h +++ b/src/library/columncache.h @@ -41,6 +41,7 @@ class ColumnCache { COLUMN_LIBRARYTABLE_KEY_ID, COLUMN_LIBRARYTABLE_BPM_LOCK, COLUMN_LIBRARYTABLE_PREVIEW, + COLUMN_LIBRARYTABLE_COVERART, COLUMN_LIBRARYTABLE_COVERART_LOCATION, COLUMN_LIBRARYTABLE_COVERART_MD5, diff --git a/src/library/dao/trackdao.h b/src/library/dao/trackdao.h index 022c294295d..8a5b369f6fc 100644 --- a/src/library/dao/trackdao.h +++ b/src/library/dao/trackdao.h @@ -50,6 +50,7 @@ const QString LIBRARYTABLE_KEY = "key"; const QString LIBRARYTABLE_KEY_ID = "key_id"; const QString LIBRARYTABLE_BPM_LOCK = "bpm_lock"; const QString LIBRARYTABLE_PREVIEW = "preview"; +const QString LIBRARYTABLE_COVERART = "cover"; const QString LIBRARYTABLE_COVERART_LOCATION = "cover_art"; const QString LIBRARYTABLE_COVERART_MD5 = "md5"; diff --git a/src/library/librarytablemodel.cpp b/src/library/librarytablemodel.cpp index 8c9ceaae610..37a3d9b6bb7 100644 --- a/src/library/librarytablemodel.cpp +++ b/src/library/librarytablemodel.cpp @@ -19,7 +19,8 @@ LibraryTableModel::~LibraryTableModel() { void LibraryTableModel::setTableModel(int id) { Q_UNUSED(id); QStringList columns; - columns << "library." + LIBRARYTABLE_ID << "'' as preview"; + columns << "library." + LIBRARYTABLE_ID << "'' AS preview"; + columns << "library." + LIBRARYTABLE_ID << "'' AS cover"; const QString tableName = "library_view"; @@ -37,6 +38,7 @@ void LibraryTableModel::setTableModel(int id) { QStringList tableColumns; tableColumns << LIBRARYTABLE_ID; tableColumns << LIBRARYTABLE_PREVIEW; + tableColumns << LIBRARYTABLE_COVERART; setTable(tableName, LIBRARYTABLE_ID, tableColumns, m_pTrackCollection->getTrackSource()); setSearch(""); From 3380e4cdd5acfe48682e14fde3210366a2d4f63d Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Tue, 15 Jul 2014 22:59:09 +0100 Subject: [PATCH 02/43] Add CoverArtDelegate class - it will be responsible for drawing the cover arts in the tableview --- build/depends.py | 1 + src/library/basesqltablemodel.cpp | 3 +++ src/library/coverartdelegate.cpp | 32 +++++++++++++++++++++++++++++++ src/library/coverartdelegate.h | 22 +++++++++++++++++++++ 4 files changed, 58 insertions(+) create mode 100644 src/library/coverartdelegate.cpp create mode 100644 src/library/coverartdelegate.h diff --git a/build/depends.py b/build/depends.py index 2812158839d..ba964869f5c 100644 --- a/build/depends.py +++ b/build/depends.py @@ -805,6 +805,7 @@ def sources(self, build): "library/bpmdelegate.cpp", "library/bpmeditor.cpp", "library/previewbuttondelegate.cpp", + "library/coverartdelegate.cpp", "audiotagger.cpp", "library/treeitemmodel.cpp", diff --git a/src/library/basesqltablemodel.cpp b/src/library/basesqltablemodel.cpp index a5df438f920..995b22e2990 100644 --- a/src/library/basesqltablemodel.cpp +++ b/src/library/basesqltablemodel.cpp @@ -7,6 +7,7 @@ #include "library/basesqltablemodel.h" +#include "library/coverartdelegate.h" #include "library/stardelegate.h" #include "library/starrating.h" #include "library/bpmdelegate.h" @@ -872,6 +873,8 @@ QAbstractItemDelegate* BaseSqlTableModel::delegateForColumn(const int i, QObject return new BPMDelegate(pParent, i, fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_BPM_LOCK)); } else if (PlayerManager::numPreviewDecks() > 0 && i == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_PREVIEW)) { return new PreviewButtonDelegate(pParent, i); + } else if (i == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_COVERART)) { + return new CoverArtDelegate(pParent); } return NULL; } diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp new file mode 100644 index 00000000000..fe13d5a401c --- /dev/null +++ b/src/library/coverartdelegate.cpp @@ -0,0 +1,32 @@ +#include + +#include "library/coverartdelegate.h" +#include "library/trackmodel.h" +#include "library/dao/trackdao.h" + +CoverArtDelegate::CoverArtDelegate(QObject *parent) + : QStyledItemDelegate(parent), + m_pTableView(NULL) { + if (QTableView *tableView = qobject_cast(parent)) { + m_pTableView = tableView; + } +} + +CoverArtDelegate::~CoverArtDelegate() { +} + +void CoverArtDelegate::paint(QPainter *painter, + const QStyleOptionViewItem &option, + const QModelIndex &index) const { + TrackModel* trackModel = dynamic_cast(m_pTableView->model()); + if (!trackModel) { + return; + } + + QString coverLocation = index.sibling(index.row(), trackModel->fieldIndex( + LIBRARYTABLE_COVERART_LOCATION)).data().toString(); + QString md5Hash = index.sibling(index.row(), trackModel->fieldIndex( + LIBRARYTABLE_COVERART_MD5)).data().toString(); + int trackId = trackModel->getTrackId(index); + +} diff --git a/src/library/coverartdelegate.h b/src/library/coverartdelegate.h new file mode 100644 index 00000000000..b2faf707474 --- /dev/null +++ b/src/library/coverartdelegate.h @@ -0,0 +1,22 @@ +#ifndef COVERARTDELEGATE_H +#define COVERARTDELEGATE_H + +#include +#include + +class CoverArtDelegate : public QStyledItemDelegate { + Q_OBJECT + + public: + explicit CoverArtDelegate(QObject* parent = NULL); + virtual ~CoverArtDelegate(); + + void paint(QPainter *painter, + const QStyleOptionViewItem &option, + const QModelIndex &index) const; + + private: + QTableView* m_pTableView; +}; + +#endif // COVERARTDELEGATE_H From 76a06d5d82fd83b187cd35a397bee4141379af2b Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 16 Jul 2014 05:10:19 +0100 Subject: [PATCH 03/43] CoverArtCache::requestPixmap(..) - returning QPixmap - it will be useful for CoverArtDelegate --- src/library/coverartcache.cpp | 14 ++++++++------ src/library/coverartcache.h | 6 +++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index e88ee01a939..b6b3ade55e6 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -64,23 +64,23 @@ QString CoverArtCache::getHashOfEmbeddedCover(QString trackLocation) { return md5Hash; } -void CoverArtCache::requestPixmap(int trackId, - const QString& coverLocation, - const QString& md5Hash) { +QPixmap CoverArtCache::requestPixmap(int trackId, + const QString& coverLocation, + const QString& md5Hash) { if (trackId < 1) { - return; + return m_defaultCover; } // keep a list of trackIds for which a future is currently running // to avoid loading the same picture again while we are loading it if (m_runningIds.contains(trackId)) { - return; + return m_defaultCover; } QPixmap pixmap; if (QPixmapCache::find(md5Hash, &pixmap)) { emit(pixmapFound(trackId, pixmap)); - return; + return pixmap; } QFuture future; @@ -97,6 +97,8 @@ void CoverArtCache::requestPixmap(int trackId, } m_runningIds.insert(trackId); watcher->setFuture(future); + + return m_defaultCover; } // Load cover from path stored in DB. diff --git a/src/library/coverartcache.h b/src/library/coverartcache.h index dcae4837168..4ac70a03422 100644 --- a/src/library/coverartcache.h +++ b/src/library/coverartcache.h @@ -15,9 +15,9 @@ class CoverArtCache : public QObject, public Singleton Q_OBJECT public: bool changeCoverArt(int trackId, const QString& newCoverLocation); - void requestPixmap(int trackId, - const QString& coverLocation = QString(), - const QString& md5Hash = QString()); + QPixmap requestPixmap(int trackId, + const QString& coverLocation = QString(), + const QString& md5Hash = QString()); void setCoverArtDAO(CoverArtDAO* coverdao); void setTrackDAO(TrackDAO* trackdao); QString getDefaultCoverLocation() { return m_sDefaultCoverLocation; } From 35d84463599810d199f881c47c00bdba6b11a64a Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 16 Jul 2014 07:05:01 +0100 Subject: [PATCH 04/43] CoverCache - new argument to check if it must emit "pixmapFound" signals - CoverDelegate will not use these signals, so it is important not swell up the qt queue. --- src/library/coverartcache.cpp | 34 +++++++++++++++++++++++----------- src/library/coverartcache.h | 10 +++++++--- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index b6b3ade55e6..df81be784af 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -66,7 +66,8 @@ QString CoverArtCache::getHashOfEmbeddedCover(QString trackLocation) { QPixmap CoverArtCache::requestPixmap(int trackId, const QString& coverLocation, - const QString& md5Hash) { + const QString& md5Hash, + const bool emitSignals) { if (trackId < 1) { return m_defaultCover; } @@ -79,7 +80,9 @@ QPixmap CoverArtCache::requestPixmap(int trackId, QPixmap pixmap; if (QPixmapCache::find(md5Hash, &pixmap)) { - emit(pixmapFound(trackId, pixmap)); + if (emitSignals) { + emit(pixmapFound(trackId, pixmap)); + } return pixmap; } @@ -88,11 +91,12 @@ QPixmap CoverArtCache::requestPixmap(int trackId, if (coverLocation.isEmpty() || !QFile::exists(coverLocation)) { CoverArtDAO::CoverArtInfo coverInfo; coverInfo = m_pCoverArtDAO->getCoverArtInfo(trackId); - future = QtConcurrent::run(this, &CoverArtCache::searchImage, coverInfo); + future = QtConcurrent::run(this, &CoverArtCache::searchImage, + coverInfo, emitSignals); connect(watcher, SIGNAL(finished()), this, SLOT(imageFound())); } else { future = QtConcurrent::run(this, &CoverArtCache::loadImage, - trackId, coverLocation, md5Hash); + trackId, coverLocation, md5Hash, emitSignals); connect(watcher, SIGNAL(finished()), this, SLOT(imageLoaded())); } m_runningIds.insert(trackId); @@ -103,14 +107,17 @@ QPixmap CoverArtCache::requestPixmap(int trackId, // Load cover from path stored in DB. // It is executed in a separate thread via QtConcurrent::run -CoverArtCache::FutureResult CoverArtCache::loadImage( - int trackId, const QString& coverLocation, const QString& md5Hash) { +CoverArtCache::FutureResult CoverArtCache::loadImage(int trackId, + const QString& coverLocation, + const QString& md5Hash, + const bool emitSignals) { FutureResult res; res.trackId = trackId; res.coverLocation = coverLocation; res.img = QImage(coverLocation); res.img = rescaleBigImage(res.img); res.md5Hash = md5Hash; + res.emitSignals = emitSignals; return res; } @@ -121,12 +128,14 @@ void CoverArtCache::imageLoaded() { FutureResult res = watcher->result(); QPixmap pixmap; - if (QPixmapCache::find(res.md5Hash, &pixmap)) { + if (QPixmapCache::find(res.md5Hash, &pixmap) && res.emitSignals) { emit(pixmapFound(res.trackId, pixmap)); } else if (!res.img.isNull()) { pixmap.convertFromImage(res.img); if (QPixmapCache::insert(res.md5Hash, pixmap)) { - emit(pixmapFound(res.trackId, pixmap)); + if (res.emitSignals) { + emit(pixmapFound(res.trackId, pixmap)); + } } } m_runningIds.remove(res.trackId); @@ -136,9 +145,10 @@ void CoverArtCache::imageLoaded() { // that could block the main thread. Therefore, this method // is executed in a separate thread via QtConcurrent::run CoverArtCache::FutureResult CoverArtCache::searchImage( - CoverArtDAO::CoverArtInfo coverInfo) { + CoverArtDAO::CoverArtInfo coverInfo, const bool emitSignals) { FutureResult res; res.trackId = coverInfo.trackId; + res.emitSignals = emitSignals; // Looking for embedded cover art. // @@ -244,12 +254,14 @@ void CoverArtCache::imageFound() { FutureResult res = watcher->result(); QPixmap pixmap; - if (QPixmapCache::find(res.md5Hash, &pixmap)) { + if (QPixmapCache::find(res.md5Hash, &pixmap) && res.emitSignals) { emit(pixmapFound(res.trackId, pixmap)); } else if (!res.img.isNull()) { pixmap.convertFromImage(res.img); if (QPixmapCache::insert(res.md5Hash, pixmap)) { - emit(pixmapFound(res.trackId, pixmap)); + if (res.emitSignals) { + emit(pixmapFound(res.trackId, pixmap)); + } } } // update DB diff --git a/src/library/coverartcache.h b/src/library/coverartcache.h index 4ac70a03422..e7b0a5e463d 100644 --- a/src/library/coverartcache.h +++ b/src/library/coverartcache.h @@ -17,7 +17,8 @@ class CoverArtCache : public QObject, public Singleton bool changeCoverArt(int trackId, const QString& newCoverLocation); QPixmap requestPixmap(int trackId, const QString& coverLocation = QString(), - const QString& md5Hash = QString()); + const QString& md5Hash = QString(), + const bool emitSignals = true); void setCoverArtDAO(CoverArtDAO* coverdao); void setTrackDAO(TrackDAO* trackdao); QString getDefaultCoverLocation() { return m_sDefaultCoverLocation; } @@ -41,12 +42,15 @@ class CoverArtCache : public QObject, public Singleton QString coverLocation; QString md5Hash; QImage img; + bool emitSignals; }; - FutureResult searchImage(CoverArtDAO::CoverArtInfo coverInfo); + FutureResult searchImage(CoverArtDAO::CoverArtInfo coverInfo, + const bool emitSignals = true); FutureResult loadImage(int trackId, const QString& coverLocation, - const QString& md5Hash); + const QString& md5Hash, + const bool emitSignals = true); private: static CoverArtCache* m_instance; From 5fd0c7e034a77ce6498481729f6205b10487b495 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 16 Jul 2014 07:40:11 +0100 Subject: [PATCH 05/43] CoverDelegate - requesting pixmap from CoverCache --- src/library/coverartdelegate.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index fe13d5a401c..f99c29bfc25 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -1,5 +1,6 @@ #include +#include "library/coverartcache.h" #include "library/coverartdelegate.h" #include "library/trackmodel.h" #include "library/dao/trackdao.h" @@ -29,4 +30,7 @@ void CoverArtDelegate::paint(QPainter *painter, LIBRARYTABLE_COVERART_MD5)).data().toString(); int trackId = trackModel->getTrackId(index); + QPixmap pixmap = CoverArtCache::instance()->requestPixmap( + trackId, coverLocation, md5Hash, false); + painter->drawPixmap(option.rect, pixmap); } From 4d6ba16e3211b0e6180d137f7a53b3dbe12e3145 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 16 Jul 2014 08:00:21 +0100 Subject: [PATCH 06/43] covercache - wrong order of member initialization --- src/library/coverartcache.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index df81be784af..7d6bac2e4f1 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -10,8 +10,8 @@ CoverArtCache::CoverArtCache() : m_pCoverArtDAO(NULL), m_pTrackDAO(NULL), - m_defaultCover(":/images/library/default_cover.png"), - m_sDefaultCoverLocation(":/images/library/default_cover.png") { + m_sDefaultCoverLocation(":/images/library/default_cover.png"), + m_defaultCover(m_sDefaultCoverLocation) { } CoverArtCache::~CoverArtCache() { From 22681879b888586371293d4339d09d6de319643d Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 16 Jul 2014 15:11:01 +0100 Subject: [PATCH 07/43] CoverDelegate - small fixes + highlight selected rows --- src/library/coverartdelegate.cpp | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index f99c29bfc25..521cb3c1ebf 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -29,8 +29,31 @@ void CoverArtDelegate::paint(QPainter *painter, QString md5Hash = index.sibling(index.row(), trackModel->fieldIndex( LIBRARYTABLE_COVERART_MD5)).data().toString(); int trackId = trackModel->getTrackId(index); - QPixmap pixmap = CoverArtCache::instance()->requestPixmap( trackId, coverLocation, md5Hash, false); + + painter->save(); + + // Populate the correct colors based on the styling + QStyleOptionViewItem newOption = option; + initStyleOption(&newOption, index); + + // Set the palette appropriately based on whether the row is selected or + // not. We also have to check if it is inactive or not and use the + // appropriate ColorGroup. + if (newOption.state & QStyle::State_Selected) { + QPalette::ColorGroup colorGroup = + newOption.state & QStyle::State_Active ? + QPalette::Active : QPalette::Inactive; + painter->fillRect(newOption.rect, + newOption.palette.color(colorGroup, QPalette::Highlight)); + painter->setBrush(newOption.palette.color( + colorGroup, QPalette::HighlightedText)); + } else { + painter->fillRect(newOption.rect, newOption.palette.base()); + } + + painter->setRenderHint(QPainter::Antialiasing, true); painter->drawPixmap(option.rect, pixmap); + painter->restore(); } From a5e1c317424276635621ea57be169ac20ebd45ba Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 16 Jul 2014 15:44:04 +0100 Subject: [PATCH 08/43] Changes return of CoverCache::requestPixmap(..) to null pixmap + do not draw default covers on tableview --- src/library/coverartcache.cpp | 6 +++--- src/library/coverartdelegate.cpp | 5 ++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index 7d6bac2e4f1..c0146ad6fb0 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -69,13 +69,13 @@ QPixmap CoverArtCache::requestPixmap(int trackId, const QString& md5Hash, const bool emitSignals) { if (trackId < 1) { - return m_defaultCover; + return QPixmap(); } // keep a list of trackIds for which a future is currently running // to avoid loading the same picture again while we are loading it if (m_runningIds.contains(trackId)) { - return m_defaultCover; + return QPixmap(); } QPixmap pixmap; @@ -102,7 +102,7 @@ QPixmap CoverArtCache::requestPixmap(int trackId, m_runningIds.insert(trackId); watcher->setFuture(future); - return m_defaultCover; + return QPixmap(); } // Load cover from path stored in DB. diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index 521cb3c1ebf..f6da1b45ecf 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -54,6 +54,9 @@ void CoverArtDelegate::paint(QPainter *painter, } painter->setRenderHint(QPainter::Antialiasing, true); - painter->drawPixmap(option.rect, pixmap); + if (!pixmap.isNull()) { + painter->drawPixmap(option.rect, pixmap); + } + painter->restore(); } From 44717e83d01a949040fca2d84215330c4814d057 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 16 Jul 2014 16:10:14 +0100 Subject: [PATCH 09/43] CoverDelegate - setting render hint to use SmoothPixmapTransform --- src/library/coverartdelegate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index f6da1b45ecf..8864006d68f 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -53,7 +53,7 @@ void CoverArtDelegate::paint(QPainter *painter, painter->fillRect(newOption.rect, newOption.palette.base()); } - painter->setRenderHint(QPainter::Antialiasing, true); + painter->setRenderHint(QPainter::SmoothPixmapTransform, true); if (!pixmap.isNull()) { painter->drawPixmap(option.rect, pixmap); } From ecc2e1f9e2ad8343c99b7bf471a04cc8ddc92622 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 16 Jul 2014 17:24:14 +0100 Subject: [PATCH 10/43] CoverDelegate - avoids drawing default_cover --- src/library/coverartdelegate.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index 8864006d68f..faab863e3b0 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -54,7 +54,8 @@ void CoverArtDelegate::paint(QPainter *painter, } painter->setRenderHint(QPainter::SmoothPixmapTransform, true); - if (!pixmap.isNull()) { + QString defaultLoc = CoverArtCache::instance()->getDefaultCoverLocation(); + if (!pixmap.isNull() && coverLocation != defaultLoc) { painter->drawPixmap(option.rect, pixmap); } From 03781d7b6af9f80f8724bb4ebf8562f542ec4dc5 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 16 Jul 2014 18:00:04 +0100 Subject: [PATCH 11/43] CoverDelegate - avoids calling "requestPixmap(..)" when the cover is default --- src/library/coverartdelegate.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index faab863e3b0..c156cc78809 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -24,13 +24,19 @@ void CoverArtDelegate::paint(QPainter *painter, return; } + QString defaultLoc = CoverArtCache::instance()->getDefaultCoverLocation(); QString coverLocation = index.sibling(index.row(), trackModel->fieldIndex( LIBRARYTABLE_COVERART_LOCATION)).data().toString(); QString md5Hash = index.sibling(index.row(), trackModel->fieldIndex( LIBRARYTABLE_COVERART_MD5)).data().toString(); int trackId = trackModel->getTrackId(index); - QPixmap pixmap = CoverArtCache::instance()->requestPixmap( + bool drawPixmap = coverLocation != defaultLoc; + QPixmap pixmap; + if (drawPixmap) { + pixmap = CoverArtCache::instance()->requestPixmap( trackId, coverLocation, md5Hash, false); + } + drawPixmap = !pixmap.isNull(); painter->save(); @@ -54,8 +60,7 @@ void CoverArtDelegate::paint(QPainter *painter, } painter->setRenderHint(QPainter::SmoothPixmapTransform, true); - QString defaultLoc = CoverArtCache::instance()->getDefaultCoverLocation(); - if (!pixmap.isNull() && coverLocation != defaultLoc) { + if (drawPixmap) { painter->drawPixmap(option.rect, pixmap); } From f8d93c836afda1043c9eed97c1a7a7a2058b50dc Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 17 Jul 2014 11:01:04 +0100 Subject: [PATCH 12/43] tablemodel - using cover_art field as readOnly (disable text edition) --- src/library/basesqltablemodel.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/library/basesqltablemodel.cpp b/src/library/basesqltablemodel.cpp index 995b22e2990..67bee1100b9 100644 --- a/src/library/basesqltablemodel.cpp +++ b/src/library/basesqltablemodel.cpp @@ -640,7 +640,8 @@ Qt::ItemFlags BaseSqlTableModel::readWriteFlags( column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_LOCATION) || column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_DURATION) || column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_BITRATE) || - column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_DATETIMEADDED)) { + column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_DATETIMEADDED) || + column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_COVERART)) { return defaultFlags; } else if (column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_TIMESPLAYED)) { return defaultFlags | Qt::ItemIsUserCheckable; From c6aff88b079ba86cacd10be8ab1fe8e9c4521a0e Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 17 Jul 2014 15:00:01 +0100 Subject: [PATCH 13/43] Add cover_art column to playlistmodel --- src/library/playlisttablemodel.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/library/playlisttablemodel.cpp b/src/library/playlisttablemodel.cpp index 1a71ae9a3e6..96935a0972d 100644 --- a/src/library/playlisttablemodel.cpp +++ b/src/library/playlisttablemodel.cpp @@ -31,7 +31,8 @@ void PlaylistTableModel::setTableModel(int playlistId) { columns << PLAYLISTTRACKSTABLE_TRACKID + " as " + LIBRARYTABLE_ID << PLAYLISTTRACKSTABLE_POSITION << PLAYLISTTRACKSTABLE_DATETIMEADDED - << "'' as preview"; + << "'' as preview" + << "'' AS " + LIBRARYTABLE_COVERART; // We drop files that have been explicitly deleted from mixxx // (mixxx_deleted=0) from the view. There was a bug in <= 1.9.0 where @@ -54,6 +55,7 @@ void PlaylistTableModel::setTableModel(int playlistId) { columns[0] = LIBRARYTABLE_ID; columns[3] = LIBRARYTABLE_PREVIEW; + columns[4] = LIBRARYTABLE_COVERART; setTable(playlistTableName, columns[0], columns, m_pTrackCollection->getTrackSource()); setSearch(""); From abb218f3a284321eb3663f89b2aaa618d1e913b2 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 17 Jul 2014 15:05:51 +0100 Subject: [PATCH 14/43] Cosmetic fixes to playlistmodel - using sql words in capital letters + using constant strings --- src/library/playlisttablemodel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/library/playlisttablemodel.cpp b/src/library/playlisttablemodel.cpp index 96935a0972d..83874e32f15 100644 --- a/src/library/playlisttablemodel.cpp +++ b/src/library/playlisttablemodel.cpp @@ -28,10 +28,10 @@ void PlaylistTableModel::setTableModel(int playlistId) { FieldEscaper escaper(m_database); QStringList columns; - columns << PLAYLISTTRACKSTABLE_TRACKID + " as " + LIBRARYTABLE_ID + columns << PLAYLISTTRACKSTABLE_TRACKID + " AS " + LIBRARYTABLE_ID << PLAYLISTTRACKSTABLE_POSITION << PLAYLISTTRACKSTABLE_DATETIMEADDED - << "'' as preview" + << "'' AS " + LIBRARYTABLE_PREVIEW << "'' AS " + LIBRARYTABLE_COVERART; // We drop files that have been explicitly deleted from mixxx From dd0525ce75ed06c6ffdb285c6f05fbf294f6a1ea Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 17 Jul 2014 15:14:11 +0100 Subject: [PATCH 15/43] librarytablemodel - fixing columns of TEMPORARY VIEW table --- src/library/librarytablemodel.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/library/librarytablemodel.cpp b/src/library/librarytablemodel.cpp index 37a3d9b6bb7..7b20dac063e 100644 --- a/src/library/librarytablemodel.cpp +++ b/src/library/librarytablemodel.cpp @@ -19,8 +19,9 @@ LibraryTableModel::~LibraryTableModel() { void LibraryTableModel::setTableModel(int id) { Q_UNUSED(id); QStringList columns; - columns << "library." + LIBRARYTABLE_ID << "'' AS preview"; - columns << "library." + LIBRARYTABLE_ID << "'' AS cover"; + columns << "library." + LIBRARYTABLE_ID + << "'' AS " + LIBRARYTABLE_PREVIEW + << "'' AS " + LIBRARYTABLE_COVERART; const QString tableName = "library_view"; From 2113213b815fe622b80223f501335bb01241cebe Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 17 Jul 2014 15:20:50 +0100 Subject: [PATCH 16/43] Add cover_art column to cratetablemodel --- src/library/cratetablemodel.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/library/cratetablemodel.cpp b/src/library/cratetablemodel.cpp index acf58cb053e..14c83e5df31 100644 --- a/src/library/cratetablemodel.cpp +++ b/src/library/cratetablemodel.cpp @@ -34,7 +34,8 @@ void CrateTableModel::setTableModel(int crateId) { QString filter = "library.mixxx_deleted = 0"; QStringList columns; columns << "crate_tracks." + CRATETRACKSTABLE_TRACKID + " as " + LIBRARYTABLE_ID - << "'' as preview"; + << "'' as preview" + << "'' AS " + LIBRARYTABLE_COVERART; // We drop files that have been explicitly deleted from mixxx // (mixxx_deleted=0) from the view. There was a bug in <= 1.9.0 where @@ -58,6 +59,7 @@ void CrateTableModel::setTableModel(int crateId) { columns[0] = LIBRARYTABLE_ID; columns[1] = LIBRARYTABLE_PREVIEW; + columns[2] = LIBRARYTABLE_COVERART; setTable(tableName, columns[0], columns, m_pTrackCollection->getTrackSource()); setSearch(""); From 85e7c18c4ef0063381a0de9f1d28fce8c76e227b Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 17 Jul 2014 16:17:52 +0100 Subject: [PATCH 17/43] Cosmetic fixes for cratetablemodel - using sql words in capital letters + using constant strings of librarytable --- src/library/cratetablemodel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/library/cratetablemodel.cpp b/src/library/cratetablemodel.cpp index 14c83e5df31..e2a60ee9a28 100644 --- a/src/library/cratetablemodel.cpp +++ b/src/library/cratetablemodel.cpp @@ -33,8 +33,8 @@ void CrateTableModel::setTableModel(int crateId) { FieldEscaper escaper(m_database); QString filter = "library.mixxx_deleted = 0"; QStringList columns; - columns << "crate_tracks." + CRATETRACKSTABLE_TRACKID + " as " + LIBRARYTABLE_ID - << "'' as preview" + columns << "crate_tracks." + CRATETRACKSTABLE_TRACKID + " AS " + LIBRARYTABLE_ID + << "'' AS " + LIBRARYTABLE_PREVIEW << "'' AS " + LIBRARYTABLE_COVERART; // We drop files that have been explicitly deleted from mixxx From c3b67d609687df1ef7f50d5451a75b9542577346 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Tue, 22 Jul 2014 09:59:01 +0100 Subject: [PATCH 18/43] CoverDeletage - cropping and adjusting the cover to fit in a cell --- src/library/coverartdelegate.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index c156cc78809..03cab962595 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -59,9 +59,14 @@ void CoverArtDelegate::paint(QPainter *painter, painter->fillRect(newOption.rect, newOption.palette.base()); } - painter->setRenderHint(QPainter::SmoothPixmapTransform, true); if (drawPixmap) { - painter->drawPixmap(option.rect, pixmap); + painter->setRenderHint(QPainter::SmoothPixmapTransform, true); + int width = 100; + int height = (float) option.rect.height() * pixmap.width() / width; + QRect target(option.rect.x(), option.rect.y(), + width, option.rect.height()); + QRect source(0, 0, pixmap.width(), height); + painter->drawPixmap(target, pixmap, source); } painter->restore(); From b5f36beb89d19631150cfba3c6a4714f5976f03a Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Tue, 22 Jul 2014 11:00:11 +0100 Subject: [PATCH 19/43] Fix bug when cover column is smaller than cover.width --- src/library/coverartdelegate.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index 03cab962595..a78e04b4368 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -62,6 +62,9 @@ void CoverArtDelegate::paint(QPainter *painter, if (drawPixmap) { painter->setRenderHint(QPainter::SmoothPixmapTransform, true); int width = 100; + if (option.rect.width() < width) { + width = option.rect.width(); + } int height = (float) option.rect.height() * pixmap.width() / width; QRect target(option.rect.x(), option.rect.y(), width, option.rect.height()); From 0933eddacc86f173651e1fd9b1834eb5b1ae5547 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 23 Jul 2014 14:57:22 +0100 Subject: [PATCH 20/43] CoverArtDelegate - drawing rows before doing the coverpixmap stuff --- src/library/coverartdelegate.cpp | 38 ++++++++++++++++---------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index a78e04b4368..aad34200fef 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -19,25 +19,6 @@ CoverArtDelegate::~CoverArtDelegate() { void CoverArtDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const { - TrackModel* trackModel = dynamic_cast(m_pTableView->model()); - if (!trackModel) { - return; - } - - QString defaultLoc = CoverArtCache::instance()->getDefaultCoverLocation(); - QString coverLocation = index.sibling(index.row(), trackModel->fieldIndex( - LIBRARYTABLE_COVERART_LOCATION)).data().toString(); - QString md5Hash = index.sibling(index.row(), trackModel->fieldIndex( - LIBRARYTABLE_COVERART_MD5)).data().toString(); - int trackId = trackModel->getTrackId(index); - bool drawPixmap = coverLocation != defaultLoc; - QPixmap pixmap; - if (drawPixmap) { - pixmap = CoverArtCache::instance()->requestPixmap( - trackId, coverLocation, md5Hash, false); - } - drawPixmap = !pixmap.isNull(); - painter->save(); // Populate the correct colors based on the styling @@ -59,6 +40,25 @@ void CoverArtDelegate::paint(QPainter *painter, painter->fillRect(newOption.rect, newOption.palette.base()); } + TrackModel* trackModel = dynamic_cast(m_pTableView->model()); + if (!trackModel) { + return; + } + + QString defaultLoc = CoverArtCache::instance()->getDefaultCoverLocation(); + QString coverLocation = index.sibling(index.row(), trackModel->fieldIndex( + LIBRARYTABLE_COVERART_LOCATION)).data().toString(); + QString md5Hash = index.sibling(index.row(), trackModel->fieldIndex( + LIBRARYTABLE_COVERART_MD5)).data().toString(); + int trackId = trackModel->getTrackId(index); + bool drawPixmap = coverLocation != defaultLoc; + QPixmap pixmap; + if (drawPixmap) { + pixmap = CoverArtCache::instance()->requestPixmap( + trackId, coverLocation, md5Hash, false); + } + drawPixmap = !pixmap.isNull(); + if (drawPixmap) { painter->setRenderHint(QPainter::SmoothPixmapTransform, true); int width = 100; From ab81473959c0f28e50aceba9f865ed31df2fa3b5 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 23 Jul 2014 19:40:06 +0100 Subject: [PATCH 21/43] CoverArtDelegate - using previous cover_delay to lock/unlock the coverdelegate::paint() If the CoverDelegate is locked, it must not try to load and search covers. It means that in this cases it will just draw covers which are already in the pixmapcache. --- src/library/coverartcache.cpp | 5 +++++ src/library/coverartcache.h | 1 + src/library/coverartdelegate.cpp | 17 +++++++++++++++-- src/library/coverartdelegate.h | 13 +++++++++++++ src/widget/wlibrarytableview.h | 1 + src/widget/wtracktableview.cpp | 3 +++ 6 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index 7c60cc55bf2..1c4f3334652 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -57,6 +57,7 @@ bool CoverArtCache::changeCoverArt(int trackId, QPixmap CoverArtCache::requestPixmap(int trackId, const QString& coverLocation, const QString& md5Hash, + const bool tryLoadAndSearch, const bool emitSignals) { if (trackId < 1) { return QPixmap(); @@ -76,6 +77,10 @@ QPixmap CoverArtCache::requestPixmap(int trackId, return pixmap; } + if (!tryLoadAndSearch) { + return QPixmap(); + } + QFuture future; QFutureWatcher* watcher = new QFutureWatcher(this); if (coverLocation.isEmpty() || !QFile::exists(coverLocation)) { diff --git a/src/library/coverartcache.h b/src/library/coverartcache.h index 3d0cb700daf..c0bf11e100e 100644 --- a/src/library/coverartcache.h +++ b/src/library/coverartcache.h @@ -18,6 +18,7 @@ class CoverArtCache : public QObject, public Singleton QPixmap requestPixmap(int trackId, const QString& coverLocation = QString(), const QString& md5Hash = QString(), + const bool tryLoadAndSearch = true, const bool emitSignals = true); void setCoverArtDAO(CoverArtDAO* coverdao); void setTrackDAO(TrackDAO* trackdao); diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index aad34200fef..5422167098c 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -7,7 +7,12 @@ CoverArtDelegate::CoverArtDelegate(QObject *parent) : QStyledItemDelegate(parent), - m_pTableView(NULL) { + m_pTableView(NULL), + m_bIsLocked(false) { + // This assumes that the parent is wtracktableview + connect(parent, SIGNAL(lockCoverArtDelegate(bool)), + this, SLOT(slotLock(bool))); + if (QTableView *tableView = qobject_cast(parent)) { m_pTableView = tableView; } @@ -16,6 +21,10 @@ CoverArtDelegate::CoverArtDelegate(QObject *parent) CoverArtDelegate::~CoverArtDelegate() { } +void CoverArtDelegate::slotLock(bool lock) { + m_bIsLocked = lock; +} + void CoverArtDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const { @@ -54,8 +63,12 @@ void CoverArtDelegate::paint(QPainter *painter, bool drawPixmap = coverLocation != defaultLoc; QPixmap pixmap; if (drawPixmap) { + // If the CoverDelegate is locked, it must not try + // to load (from coverLocation) and search covers. + // It means that in this cases it will just draw + // covers which are already in the pixmapcache. pixmap = CoverArtCache::instance()->requestPixmap( - trackId, coverLocation, md5Hash, false); + trackId, coverLocation, md5Hash, !m_bIsLocked, false); } drawPixmap = !pixmap.isNull(); diff --git a/src/library/coverartdelegate.h b/src/library/coverartdelegate.h index b2faf707474..c1d5f03cb1b 100644 --- a/src/library/coverartdelegate.h +++ b/src/library/coverartdelegate.h @@ -15,8 +15,21 @@ class CoverArtDelegate : public QStyledItemDelegate { const QStyleOptionViewItem &option, const QModelIndex &index) const; + private slots: + // If the CoverDelegate is locked, it must not try + // to load and search covers. + // It means that in this cases it will just draw + // covers which are already in the pixmapcache. + // It is very important when the user scoll down + // very fast or when they hold an arrow key, because + // in these cases paint() would be called MANY times + // and it would be doing tons of "requestPixmaps", + // which could easily freeze the whole UI. + void slotLock(bool lock); + private: QTableView* m_pTableView; + bool m_bIsLocked; }; #endif // COVERARTDELEGATE_H diff --git a/src/widget/wlibrarytableview.h b/src/widget/wlibrarytableview.h index c412f7d509e..7607e20cafe 100644 --- a/src/widget/wlibrarytableview.h +++ b/src/widget/wlibrarytableview.h @@ -26,6 +26,7 @@ class WLibraryTableView : public QTableView, public virtual LibraryView { void loadTrack(TrackPointer pTrack); void loadTrackToPlayer(TrackPointer pTrack, QString group, bool play = false); void loadCoverArt(const QString& coverLocation, const QString&, int trackId); + void lockCoverArtDelegate(bool); public slots: void saveVScrollBarPos(); diff --git a/src/widget/wtracktableview.cpp b/src/widget/wtracktableview.cpp index 800a097b371..bb6d86092b4 100644 --- a/src/widget/wtracktableview.cpp +++ b/src/widget/wtracktableview.cpp @@ -141,6 +141,8 @@ void WTrackTableView::selectionChanged(const QItemSelection &selected, if (m_bLastCoverLoaded) { // load default cover art emit(loadCoverArt("", "", 0)); + // not draw covers in the tableview (cover_art column) + emit(lockCoverArtDelegate(true)); } m_bLastCoverLoaded = false; m_lastSelection = m_pCOTGuiTickTime->get(); @@ -175,6 +177,7 @@ void WTrackTableView::slotLoadCoverArt() { } } emit(loadCoverArt(coverLocation, md5Hash, trackId)); + emit(lockCoverArtDelegate(false)); update(); } From 8e82305cef7c314708e92be900de19f2ef350602 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 23 Jul 2014 21:15:21 +0100 Subject: [PATCH 22/43] CoverArtDelegate - extending same logic used to delay cover_loadings when the user is scrolling fast --- src/widget/wlibrarytableview.cpp | 3 +++ src/widget/wlibrarytableview.h | 1 + src/widget/wtracktableview.cpp | 11 +++++++++++ src/widget/wtracktableview.h | 1 + 4 files changed, 16 insertions(+) diff --git a/src/widget/wlibrarytableview.cpp b/src/widget/wlibrarytableview.cpp index 8a6fc0ce182..4cb9a2b3150 100644 --- a/src/widget/wlibrarytableview.cpp +++ b/src/widget/wlibrarytableview.cpp @@ -43,6 +43,9 @@ WLibraryTableView::WLibraryTableView(QWidget* parent, loadVScrollBarPosState(); + connect(verticalScrollBar(), SIGNAL(valueChanged(int)), + this, SIGNAL(scrollValueChanged(int))); + setTabKeyNavigation(false); } diff --git a/src/widget/wlibrarytableview.h b/src/widget/wlibrarytableview.h index 7607e20cafe..6ad6981c5a4 100644 --- a/src/widget/wlibrarytableview.h +++ b/src/widget/wlibrarytableview.h @@ -27,6 +27,7 @@ class WLibraryTableView : public QTableView, public virtual LibraryView { void loadTrackToPlayer(TrackPointer pTrack, QString group, bool play = false); void loadCoverArt(const QString& coverLocation, const QString&, int trackId); void lockCoverArtDelegate(bool); + void scrollValueChanged(int); public slots: void saveVScrollBarPos(); diff --git a/src/widget/wtracktableview.cpp b/src/widget/wtracktableview.cpp index bb6d86092b4..5ac0cd854a5 100644 --- a/src/widget/wtracktableview.cpp +++ b/src/widget/wtracktableview.cpp @@ -92,6 +92,8 @@ WTrackTableView::WTrackTableView(QWidget * parent, m_pCOTGuiTickTime = new ControlObjectThread("[Master]", "guiTick50ms"); connect(m_pCOTGuiTickTime, SIGNAL(valueChanged(double)), this, SLOT(slotGuiTickTime(double))); + connect(this, SIGNAL(scrollValueChanged(int)), + this, SLOT(slotScrollValueChanged(int))); } WTrackTableView::~WTrackTableView() { @@ -133,6 +135,15 @@ WTrackTableView::~WTrackTableView() { delete m_pCOTGuiTickTime; } +void WTrackTableView::slotScrollValueChanged(int) { + if (m_bLastCoverLoaded) { + // not draw covers in the tableview (cover_art column) + emit(lockCoverArtDelegate(true)); + } + m_bLastCoverLoaded = false; + m_lastSelection = m_pCOTGuiTickTime->get(); +} + void WTrackTableView::selectionChanged(const QItemSelection &selected, const QItemSelection &deselected) { Q_UNUSED(selected); diff --git a/src/widget/wtracktableview.h b/src/widget/wtracktableview.h index d3a8e29e12e..35d869e2c64 100644 --- a/src/widget/wtracktableview.h +++ b/src/widget/wtracktableview.h @@ -70,6 +70,7 @@ class WTrackTableView : public WLibraryTableView { void slotScaleBpm(int); void slotClearBeats(); void slotGuiTickTime(double); + void slotScrollValueChanged(int); private: void sendToAutoDJ(bool bTop); From 9614f5ecbe1bfea2c5385cbf8223613c8ef7651f Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 24 Jul 2014 11:00:22 +0100 Subject: [PATCH 23/43] Cropped Image - storing cropped images in pixmapcache + cropping them in the working threads (search and load images) For more details, see new commented lines... --- src/library/coverartcache.cpp | 82 +++++++++++++++++++++++++++----- src/library/coverartcache.h | 5 ++ src/library/coverartdelegate.cpp | 33 +++++++------ 3 files changed, 92 insertions(+), 28 deletions(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index 1c4f3334652..9619d25e9a8 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -58,6 +58,7 @@ QPixmap CoverArtCache::requestPixmap(int trackId, const QString& coverLocation, const QString& md5Hash, const bool tryLoadAndSearch, + const bool croppedPixmap, const bool emitSignals) { if (trackId < 1) { return QPixmap(); @@ -69,8 +70,15 @@ QPixmap CoverArtCache::requestPixmap(int trackId, return QPixmap(); } + // If this request comes from CoverDelegate (table view), + // it'll want to get a cropped cover which is ready to be drawn + // in the table view (cover art column). + // It's very important to keep the cropped covers in cache because it avoids + // having to rescale+crop it ALWAYS (which brings a lot of performance issues). + QString cacheKey = croppedPixmap ? md5Hash % "_cropped" : md5Hash; + QPixmap pixmap; - if (QPixmapCache::find(md5Hash, &pixmap)) { + if (QPixmapCache::find(cacheKey, &pixmap)) { if (emitSignals) { emit(pixmapFound(trackId, pixmap)); } @@ -87,11 +95,12 @@ QPixmap CoverArtCache::requestPixmap(int trackId, CoverArtDAO::CoverArtInfo coverInfo; coverInfo = m_pCoverArtDAO->getCoverArtInfo(trackId); future = QtConcurrent::run(this, &CoverArtCache::searchImage, - coverInfo, emitSignals); + coverInfo, croppedPixmap, emitSignals); connect(watcher, SIGNAL(finished()), this, SLOT(imageFound())); } else { future = QtConcurrent::run(this, &CoverArtCache::loadImage, - trackId, coverLocation, md5Hash, emitSignals); + trackId, coverLocation, md5Hash, + croppedPixmap, emitSignals); connect(watcher, SIGNAL(finished()), this, SLOT(imageLoaded())); } m_runningIds.insert(trackId); @@ -105,14 +114,22 @@ QPixmap CoverArtCache::requestPixmap(int trackId, CoverArtCache::FutureResult CoverArtCache::loadImage(int trackId, const QString& coverLocation, const QString& md5Hash, + const bool croppedPixmap, const bool emitSignals) { FutureResult res; res.trackId = trackId; res.coverLocation = coverLocation; res.img = QImage(coverLocation); - res.img = rescaleBigImage(res.img); res.md5Hash = md5Hash; + res.croppedImg = croppedPixmap; res.emitSignals = emitSignals; + + if (res.croppedImg) { + res.img = cropImage(res.img); + } else { + res.img = rescaleBigImage(res.img); + } + return res; } @@ -123,11 +140,12 @@ void CoverArtCache::imageLoaded() { FutureResult res = watcher->result(); QPixmap pixmap; - if (QPixmapCache::find(res.md5Hash, &pixmap) && res.emitSignals) { + QString cacheKey = res.croppedImg ? res.md5Hash % "_cropped" : res.md5Hash; + if (QPixmapCache::find(cacheKey, &pixmap) && res.emitSignals) { emit(pixmapFound(res.trackId, pixmap)); } else if (!res.img.isNull()) { pixmap.convertFromImage(res.img); - if (QPixmapCache::insert(res.md5Hash, pixmap)) { + if (QPixmapCache::insert(cacheKey, pixmap)) { if (res.emitSignals) { emit(pixmapFound(res.trackId, pixmap)); } @@ -140,21 +158,30 @@ void CoverArtCache::imageLoaded() { // that could block the main thread. Therefore, this method // is executed in a separate thread via QtConcurrent::run CoverArtCache::FutureResult CoverArtCache::searchImage( - CoverArtDAO::CoverArtInfo coverInfo, const bool emitSignals) { + CoverArtDAO::CoverArtInfo coverInfo, + const bool croppedPixmap, + const bool emitSignals) { FutureResult res; res.trackId = coverInfo.trackId; + res.croppedImg = croppedPixmap; res.emitSignals = emitSignals; // Looking for embedded cover art. // res.img = extractEmbeddedCover(coverInfo.trackLocation); if (!res.img.isNull()) { - res.img = rescaleBigImage(res.img); + // it is the first time that we are loading the embedded cover, + // so we need to recalculate the md5 hash. if (res.md5Hash.isEmpty()) { - // it is the first time that we are loading the embedded cover, - // so we need to recalculate the md5 hash. res.md5Hash = calculateMD5(res.img); } + + if (res.croppedImg) { + res.img = cropImage(res.img); + } else { + res.img = rescaleBigImage(res.img); + } + return res; } @@ -163,9 +190,15 @@ CoverArtCache::FutureResult CoverArtCache::searchImage( res.coverLocation = searchInTrackDirectory(coverInfo.trackDirectory, coverInfo.trackBaseName, coverInfo.album); + res.img = QImage(res.coverLocation); - res.img = rescaleBigImage(res.img); res.md5Hash = calculateMD5(res.img); + if (res.croppedImg) { + res.img = cropImage(res.img); + } else { + res.img = rescaleBigImage(res.img); + } + return res; } @@ -249,11 +282,12 @@ void CoverArtCache::imageFound() { FutureResult res = watcher->result(); QPixmap pixmap; - if (QPixmapCache::find(res.md5Hash, &pixmap) && res.emitSignals) { + QString cacheKey = res.croppedImg ? res.md5Hash % "_cropped" : res.md5Hash; + if (QPixmapCache::find(cacheKey, &pixmap) && res.emitSignals) { emit(pixmapFound(res.trackId, pixmap)); } else if (!res.img.isNull()) { pixmap.convertFromImage(res.img); - if (QPixmapCache::insert(res.md5Hash, pixmap)) { + if (QPixmapCache::insert(cacheKey, pixmap)) { if (res.emitSignals) { emit(pixmapFound(res.trackId, pixmap)); } @@ -266,6 +300,28 @@ void CoverArtCache::imageFound() { m_runningIds.remove(res.trackId); } +// It will return a cropped cover that is ready to be +// used by the tableview-cover_column (CoverDelegate). +// As QImage is optimized to manipulate images, we will do it here +// instead of rescale it directly on the CoverDelegate::paint() +// because it would be much slower and could easily freeze the UI... +// Also, this method will run in separate thread +// (via Qtconcurrent - called by searchImage() or loadImage()) +QImage CoverArtCache::cropImage(QImage img) { + if (img.isNull()) { + return QImage(); + } + + // it defines the maximum width of the covers displayed + // in the cover art column (tableviews). + // (if you want to increase it - you have to change JUST it.) + const int WIDTH = 100; + const int CELL_HEIGHT = 20; + + img = img.scaledToWidth(WIDTH, Qt::SmoothTransformation); + return img.copy(0, 0, img.width(), CELL_HEIGHT); +} + // if it's too big, we have to scale it. // big images would be quickly removed from cover cache. QImage CoverArtCache::rescaleBigImage(QImage img) { diff --git a/src/library/coverartcache.h b/src/library/coverartcache.h index c0bf11e100e..4d583de7f46 100644 --- a/src/library/coverartcache.h +++ b/src/library/coverartcache.h @@ -19,6 +19,7 @@ class CoverArtCache : public QObject, public Singleton const QString& coverLocation = QString(), const QString& md5Hash = QString(), const bool tryLoadAndSearch = true, + const bool croppedPixmap = false, const bool emitSignals = true); void setCoverArtDAO(CoverArtDAO* coverdao); void setTrackDAO(TrackDAO* trackdao); @@ -42,14 +43,17 @@ class CoverArtCache : public QObject, public Singleton QString coverLocation; QString md5Hash; QImage img; + bool croppedImg; bool emitSignals; }; FutureResult searchImage(CoverArtDAO::CoverArtInfo coverInfo, + const bool croppedPixmap = false, const bool emitSignals = true); FutureResult loadImage(int trackId, const QString& coverLocation, const QString& md5Hash, + const bool croppedPixmap = false, const bool emitSignals = true); private: @@ -61,6 +65,7 @@ class CoverArtCache : public QObject, public Singleton QSet m_runningIds; QString calculateMD5(QImage img); + QImage cropImage(QImage img); QImage rescaleBigImage(QImage img); QImage extractEmbeddedCover(QString trackLocation); QString searchInTrackDirectory(QString directory, diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index 5422167098c..7cbc824a09d 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -60,29 +60,32 @@ void CoverArtDelegate::paint(QPainter *painter, QString md5Hash = index.sibling(index.row(), trackModel->fieldIndex( LIBRARYTABLE_COVERART_MD5)).data().toString(); int trackId = trackModel->getTrackId(index); - bool drawPixmap = coverLocation != defaultLoc; QPixmap pixmap; - if (drawPixmap) { + if (coverLocation != defaultLoc) { // draw cover_art // If the CoverDelegate is locked, it must not try // to load (from coverLocation) and search covers. // It means that in this cases it will just draw // covers which are already in the pixmapcache. pixmap = CoverArtCache::instance()->requestPixmap( - trackId, coverLocation, md5Hash, !m_bIsLocked, false); - } - drawPixmap = !pixmap.isNull(); + trackId, coverLocation, md5Hash, + !m_bIsLocked, true, false); + + if (!pixmap.isNull()) { + // It already got a cropped pixmap (from covercache) + // that fit to the cell. + + // If you want to change the cropped_cover size, + // you MUST do it in CoverArtCache::cropCover() + int width = pixmap.width(); + if (option.rect.width() < width) { + width = option.rect.width(); + } - if (drawPixmap) { - painter->setRenderHint(QPainter::SmoothPixmapTransform, true); - int width = 100; - if (option.rect.width() < width) { - width = option.rect.width(); + QRect target(option.rect.x(), option.rect.y(), + width, option.rect.height()); + QRect source(0, 0, width, pixmap.height()); + painter->drawPixmap(target, pixmap, source); } - int height = (float) option.rect.height() * pixmap.width() / width; - QRect target(option.rect.x(), option.rect.y(), - width, option.rect.height()); - QRect source(0, 0, pixmap.width(), height); - painter->drawPixmap(target, pixmap, source); } painter->restore(); From c447457fed5b7863f1aff31d5c7a0e643b41b3ae Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Mon, 28 Jul 2014 15:33:00 +0100 Subject: [PATCH 24/43] Improving CoverArtDelegate::paint() - remove unnecessary QStyleOptionViewItem copy --- src/library/coverartdelegate.cpp | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index 7cbc824a09d..5dfb003ae58 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -28,25 +28,8 @@ void CoverArtDelegate::slotLock(bool lock) { void CoverArtDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const { - painter->save(); - - // Populate the correct colors based on the styling - QStyleOptionViewItem newOption = option; - initStyleOption(&newOption, index); - - // Set the palette appropriately based on whether the row is selected or - // not. We also have to check if it is inactive or not and use the - // appropriate ColorGroup. - if (newOption.state & QStyle::State_Selected) { - QPalette::ColorGroup colorGroup = - newOption.state & QStyle::State_Active ? - QPalette::Active : QPalette::Inactive; - painter->fillRect(newOption.rect, - newOption.palette.color(colorGroup, QPalette::Highlight)); - painter->setBrush(newOption.palette.color( - colorGroup, QPalette::HighlightedText)); - } else { - painter->fillRect(newOption.rect, newOption.palette.base()); + if (option.state & QStyle::State_Selected) { + painter->fillRect(option.rect, option.palette.highlight()); } TrackModel* trackModel = dynamic_cast(m_pTableView->model()); @@ -87,6 +70,4 @@ void CoverArtDelegate::paint(QPainter *painter, painter->drawPixmap(target, pixmap, source); } } - - painter->restore(); } From c88e16c45c92435c39dea47f67e2d137d074f5b8 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Mon, 28 Jul 2014 17:44:04 +0100 Subject: [PATCH 25/43] Improving CoverArtDelegate::paint() - using trackmodel as a member to get it just once --- src/library/coverartdelegate.cpp | 12 ++++++------ src/library/coverartdelegate.h | 3 +++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index 5dfb003ae58..55bbd8ac060 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -2,12 +2,12 @@ #include "library/coverartcache.h" #include "library/coverartdelegate.h" -#include "library/trackmodel.h" #include "library/dao/trackdao.h" CoverArtDelegate::CoverArtDelegate(QObject *parent) : QStyledItemDelegate(parent), m_pTableView(NULL), + m_pTrackModel(NULL), m_bIsLocked(false) { // This assumes that the parent is wtracktableview connect(parent, SIGNAL(lockCoverArtDelegate(bool)), @@ -15,6 +15,7 @@ CoverArtDelegate::CoverArtDelegate(QObject *parent) if (QTableView *tableView = qobject_cast(parent)) { m_pTableView = tableView; + m_pTrackModel = dynamic_cast(m_pTableView->model()); } } @@ -32,17 +33,16 @@ void CoverArtDelegate::paint(QPainter *painter, painter->fillRect(option.rect, option.palette.highlight()); } - TrackModel* trackModel = dynamic_cast(m_pTableView->model()); - if (!trackModel) { + if (!m_pTrackModel) { return; } QString defaultLoc = CoverArtCache::instance()->getDefaultCoverLocation(); - QString coverLocation = index.sibling(index.row(), trackModel->fieldIndex( + QString coverLocation = index.sibling(index.row(), m_pTrackModel->fieldIndex( LIBRARYTABLE_COVERART_LOCATION)).data().toString(); - QString md5Hash = index.sibling(index.row(), trackModel->fieldIndex( + QString md5Hash = index.sibling(index.row(), m_pTrackModel->fieldIndex( LIBRARYTABLE_COVERART_MD5)).data().toString(); - int trackId = trackModel->getTrackId(index); + int trackId = m_pTrackModel->getTrackId(index); QPixmap pixmap; if (coverLocation != defaultLoc) { // draw cover_art // If the CoverDelegate is locked, it must not try diff --git a/src/library/coverartdelegate.h b/src/library/coverartdelegate.h index c1d5f03cb1b..1623c276154 100644 --- a/src/library/coverartdelegate.h +++ b/src/library/coverartdelegate.h @@ -4,6 +4,8 @@ #include #include +#include "library/trackmodel.h" + class CoverArtDelegate : public QStyledItemDelegate { Q_OBJECT @@ -29,6 +31,7 @@ class CoverArtDelegate : public QStyledItemDelegate { private: QTableView* m_pTableView; + TrackModel* m_pTrackModel; bool m_bIsLocked; }; From 5b8dfc35bdc5eb72121c281868d73fdfea4e6744 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Mon, 28 Jul 2014 18:11:55 +0100 Subject: [PATCH 26/43] Improving CoverArtDelegate::paint() - using defaultCoverLocation as a member to get it just once from CoverCache --- src/library/coverartdelegate.cpp | 9 ++++----- src/library/coverartdelegate.h | 1 + 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index 55bbd8ac060..48aea22908b 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -8,7 +8,8 @@ CoverArtDelegate::CoverArtDelegate(QObject *parent) : QStyledItemDelegate(parent), m_pTableView(NULL), m_pTrackModel(NULL), - m_bIsLocked(false) { + m_bIsLocked(false), + m_sDefaultCover(CoverArtCache::instance()->getDefaultCoverLocation()) { // This assumes that the parent is wtracktableview connect(parent, SIGNAL(lockCoverArtDelegate(bool)), this, SLOT(slotLock(bool))); @@ -37,19 +38,17 @@ void CoverArtDelegate::paint(QPainter *painter, return; } - QString defaultLoc = CoverArtCache::instance()->getDefaultCoverLocation(); QString coverLocation = index.sibling(index.row(), m_pTrackModel->fieldIndex( LIBRARYTABLE_COVERART_LOCATION)).data().toString(); QString md5Hash = index.sibling(index.row(), m_pTrackModel->fieldIndex( LIBRARYTABLE_COVERART_MD5)).data().toString(); int trackId = m_pTrackModel->getTrackId(index); - QPixmap pixmap; - if (coverLocation != defaultLoc) { // draw cover_art + if (coverLocation != m_sDefaultCover) { // draw cover_art // If the CoverDelegate is locked, it must not try // to load (from coverLocation) and search covers. // It means that in this cases it will just draw // covers which are already in the pixmapcache. - pixmap = CoverArtCache::instance()->requestPixmap( + QPixmap pixmap = CoverArtCache::instance()->requestPixmap( trackId, coverLocation, md5Hash, !m_bIsLocked, true, false); diff --git a/src/library/coverartdelegate.h b/src/library/coverartdelegate.h index 1623c276154..d5697f2f707 100644 --- a/src/library/coverartdelegate.h +++ b/src/library/coverartdelegate.h @@ -33,6 +33,7 @@ class CoverArtDelegate : public QStyledItemDelegate { QTableView* m_pTableView; TrackModel* m_pTrackModel; bool m_bIsLocked; + QString m_sDefaultCover; }; #endif // COVERARTDELEGATE_H From 67300c31b8fae04e78d21c2c30a1e1c18e9d7427 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Tue, 29 Jul 2014 12:00:59 +0100 Subject: [PATCH 27/43] Improving CoverArtDelegate::paint() - getting coverLocation and md5 column just once; - ordering calls in a better way; After some analysis with valgrind, I noticed that fieldIndex() takes a long time and that we could call it just once (in the construction). --- src/library/coverartdelegate.cpp | 25 +++++++++++++++++++------ src/library/coverartdelegate.h | 2 ++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index 48aea22908b..a1997497b2b 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -9,7 +9,9 @@ CoverArtDelegate::CoverArtDelegate(QObject *parent) m_pTableView(NULL), m_pTrackModel(NULL), m_bIsLocked(false), - m_sDefaultCover(CoverArtCache::instance()->getDefaultCoverLocation()) { + m_sDefaultCover(CoverArtCache::instance()->getDefaultCoverLocation()), + m_iCoverLocationColumn(-1), + m_iMd5Column(-1) { // This assumes that the parent is wtracktableview connect(parent, SIGNAL(lockCoverArtDelegate(bool)), this, SLOT(slotLock(bool))); @@ -17,6 +19,8 @@ CoverArtDelegate::CoverArtDelegate(QObject *parent) if (QTableView *tableView = qobject_cast(parent)) { m_pTableView = tableView; m_pTrackModel = dynamic_cast(m_pTableView->model()); + m_iCoverLocationColumn = m_pTrackModel->fieldIndex(LIBRARYTABLE_COVERART_LOCATION); + m_iMd5Column = m_pTrackModel->fieldIndex(LIBRARYTABLE_COVERART_MD5); } } @@ -34,16 +38,25 @@ void CoverArtDelegate::paint(QPainter *painter, painter->fillRect(option.rect, option.palette.highlight()); } - if (!m_pTrackModel) { + if (!m_pTrackModel || m_iCoverLocationColumn == -1 || m_iMd5Column == -1) { return; } - QString coverLocation = index.sibling(index.row(), m_pTrackModel->fieldIndex( - LIBRARYTABLE_COVERART_LOCATION)).data().toString(); - QString md5Hash = index.sibling(index.row(), m_pTrackModel->fieldIndex( - LIBRARYTABLE_COVERART_MD5)).data().toString(); int trackId = m_pTrackModel->getTrackId(index); + if (trackId < 1) { + return; + } + + QString coverLocation = index.sibling(index.row(), + m_iCoverLocationColumn + ).data().toString(); + if (coverLocation != m_sDefaultCover) { // draw cover_art + + QString md5Hash = index.sibling(index.row(), + m_iMd5Column + ).data().toString(); + // If the CoverDelegate is locked, it must not try // to load (from coverLocation) and search covers. // It means that in this cases it will just draw diff --git a/src/library/coverartdelegate.h b/src/library/coverartdelegate.h index d5697f2f707..5a71ba45ba2 100644 --- a/src/library/coverartdelegate.h +++ b/src/library/coverartdelegate.h @@ -34,6 +34,8 @@ class CoverArtDelegate : public QStyledItemDelegate { TrackModel* m_pTrackModel; bool m_bIsLocked; QString m_sDefaultCover; + int m_iCoverLocationColumn; + int m_iMd5Column; }; #endif // COVERARTDELEGATE_H From 2b579de1afc43ee555f6559f46596877ca17b0d9 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Tue, 29 Jul 2014 15:11:39 +0100 Subject: [PATCH 28/43] CoverArtCache - checking if the cover found is different from the cover that is already stored in db - it avoids calling 'updates' when there is nothing to update... --- src/library/coverartcache.cpp | 43 ++++++++++++++++++++++------------- src/library/coverartcache.h | 1 + 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index 9619d25e9a8..b15c8d0450b 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -165,6 +165,7 @@ CoverArtCache::FutureResult CoverArtCache::searchImage( res.trackId = coverInfo.trackId; res.croppedImg = croppedPixmap; res.emitSignals = emitSignals; + res.newImgFound = false; // Looking for embedded cover art. // @@ -175,28 +176,34 @@ CoverArtCache::FutureResult CoverArtCache::searchImage( if (res.md5Hash.isEmpty()) { res.md5Hash = calculateMD5(res.img); } + res.newImgFound = true; + } + + // Looking for cover stored in track diretory. + // + if (!res.newImgFound) { + res.coverLocation = searchInTrackDirectory(coverInfo.trackDirectory, + coverInfo.trackBaseName, + coverInfo.album); + res.img = QImage(res.coverLocation); + res.md5Hash = calculateMD5(res.img); + res.newImgFound = true; + } + // adjusting the cover size according to the final purpose + if (res.newImgFound) { if (res.croppedImg) { res.img = cropImage(res.img); } else { res.img = rescaleBigImage(res.img); } - - return res; } - // Looking for cover stored in track diretory. - // - res.coverLocation = searchInTrackDirectory(coverInfo.trackDirectory, - coverInfo.trackBaseName, - coverInfo.album); - - res.img = QImage(res.coverLocation); - res.md5Hash = calculateMD5(res.img); - if (res.croppedImg) { - res.img = cropImage(res.img); - } else { - res.img = rescaleBigImage(res.img); + // check if this image is really new + // (different from the one that we have in db) + if (coverInfo.md5Hash == res.md5Hash) + { + res.newImgFound = false; } return res; @@ -293,9 +300,13 @@ void CoverArtCache::imageFound() { } } } + // update DB - int coverId = m_pCoverArtDAO->saveCoverArt(res.coverLocation, res.md5Hash); - m_pTrackDAO->updateCoverArt(res.trackId, coverId); + if (res.newImgFound) { + int coverId = m_pCoverArtDAO->saveCoverArt(res.coverLocation, + res.md5Hash); + m_pTrackDAO->updateCoverArt(res.trackId, coverId); + } m_runningIds.remove(res.trackId); } diff --git a/src/library/coverartcache.h b/src/library/coverartcache.h index 4d583de7f46..612b6034718 100644 --- a/src/library/coverartcache.h +++ b/src/library/coverartcache.h @@ -45,6 +45,7 @@ class CoverArtCache : public QObject, public Singleton QImage img; bool croppedImg; bool emitSignals; + bool newImgFound; }; FutureResult searchImage(CoverArtDAO::CoverArtInfo coverInfo, From 5fadd655f182fe0451a202ad75c51a53fc8f031c Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Tue, 29 Jul 2014 21:59:33 +0100 Subject: [PATCH 29/43] CoverArtDAO - add method to save multi cover arts at once (from qlist) --- src/library/dao/coverartdao.cpp | 38 +++++++++++++++++++++++++++++++++ src/library/dao/coverartdao.h | 1 + 2 files changed, 39 insertions(+) diff --git a/src/library/dao/coverartdao.cpp b/src/library/dao/coverartdao.cpp index 7126a9c91fe..1adbbaf4d10 100644 --- a/src/library/dao/coverartdao.cpp +++ b/src/library/dao/coverartdao.cpp @@ -45,6 +45,44 @@ int CoverArtDAO::saveCoverArt(QString coverLocation, QString md5Hash) { return coverId; } +QList CoverArtDAO::saveCoverArt(QList > covers) { + if (covers.isEmpty()) { + return QList(); + } + + // it'll be used to avoid writing a new ID for + // rows which have the same md5 (not changed). + QString selectCoverId = QString("SELECT id FROM cover_art WHERE md5='%1'"); + + // preparing query to insert multi rows + QString sQuery; + sQuery = QString("INSERT OR REPLACE INTO cover_art ('id', 'location', 'md5') " + "SELECT (%1) AS 'id', '%2' AS 'location', '%3' AS 'md5' ") + .arg(selectCoverId.arg(covers.first().second)) + .arg(covers.first().first) + .arg(covers.first().second); + for (int i=1; i(); + } + + // getting the last inserted cover id's + QList coverIds; + for (int index=0; index saveCoverArt(QList > covers); struct CoverArtInfo { int trackId; From 9e64660a16719d8e5b03019a41e01bc64d0b5f36 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 30 Jul 2014 10:51:23 +0100 Subject: [PATCH 30/43] TrackDAO - updating multi cover_art rows at once (needs more work) - it should be done in a single query... --- src/library/dao/trackdao.cpp | 25 +++++++++++++++++++++++++ src/library/dao/trackdao.h | 1 + 2 files changed, 26 insertions(+) diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index e763a29481c..3f853368bad 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -1246,6 +1246,31 @@ bool TrackDAO::updateCoverArt(int trackId, int coverId) { return true; } +bool TrackDAO::updateCoverArt(QList trackIds, QList coverIds) { + if (trackIds.isEmpty() || coverIds.isEmpty()) { + return false; + } + + // TODO: it should be done in a single query + QSqlQuery query(m_database); + for (int i=0; i trackIds, QList coverIds); signals: void trackDirty(int trackId); From 806e476db7f0e3d7cfa349aa714a27452737f618 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Wed, 30 Jul 2014 22:00:55 +0100 Subject: [PATCH 31/43] CoverArtCache - accumulate covers for half second and then update the database at once - sqlite can only do about 50 writes per second, so it is important to collect all new covers and write them at once. --- src/library/coverartcache.cpp | 38 ++++++++++++++++++++++++++++++----- src/library/coverartcache.h | 5 +++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index b15c8d0450b..252a23226da 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include "coverartcache.h" #include "soundsourceproxy.h" @@ -11,7 +12,10 @@ CoverArtCache::CoverArtCache() : m_pCoverArtDAO(NULL), m_pTrackDAO(NULL), m_sDefaultCoverLocation(":/images/library/default_cover.png"), - m_defaultCover(m_sDefaultCoverLocation) { + m_defaultCover(m_sDefaultCoverLocation), + m_timer(new QTimer(this)) { + m_timer->setSingleShot(true); + connect(m_timer, SIGNAL(timeout()), SLOT(updateDB())); } CoverArtCache::~CoverArtCache() { @@ -70,6 +74,12 @@ QPixmap CoverArtCache::requestPixmap(int trackId, return QPixmap(); } + // check if we have already found a cover for this track + // and if it is just waiting to be inserted/updated in the DB. + if (m_queueOfUpdates.contains(trackId)) { + return QPixmap(); + } + // If this request comes from CoverDelegate (table view), // it'll want to get a cropped cover which is ready to be drawn // in the table view (cover art column). @@ -302,15 +312,33 @@ void CoverArtCache::imageFound() { } // update DB - if (res.newImgFound) { - int coverId = m_pCoverArtDAO->saveCoverArt(res.coverLocation, - res.md5Hash); - m_pTrackDAO->updateCoverArt(res.trackId, coverId); + if (res.newImgFound && !m_queueOfUpdates.contains(res.trackId)) { + m_queueOfUpdates.insert(res.trackId, + qMakePair(res.coverLocation, res.md5Hash)); + } + + if (m_queueOfUpdates.size() == 1 && !m_timer->isActive()) { + m_timer->start(500); // after 0.5s, it will call `updateDB()` } m_runningIds.remove(res.trackId); } +// sqlite can only do about 50 writes per second, +// so it is important to collect all new covers and write them at once. +void CoverArtCache::updateDB() { + if (m_queueOfUpdates.isEmpty()) { + return; + } + + QList coverIds = m_pCoverArtDAO->saveCoverArt(m_queueOfUpdates.values()); + + Q_ASSERT(m_queueOfUpdates.keys().size() == coverIds.size()); + m_pTrackDAO->updateCoverArt(m_queueOfUpdates.keys(), coverIds); + + m_queueOfUpdates.clear(); +} + // It will return a cropped cover that is ready to be // used by the tableview-cover_column (CoverDelegate). // As QImage is optimized to manipulate images, we will do it here diff --git a/src/library/coverartcache.h b/src/library/coverartcache.h index 612b6034718..479b1fcdf5e 100644 --- a/src/library/coverartcache.h +++ b/src/library/coverartcache.h @@ -30,6 +30,9 @@ class CoverArtCache : public QObject, public Singleton void imageFound(); void imageLoaded(); + private slots: + void updateDB(); + signals: void pixmapFound(int trackId, QPixmap pixmap); @@ -63,7 +66,9 @@ class CoverArtCache : public QObject, public Singleton TrackDAO* m_pTrackDAO; const QString m_sDefaultCoverLocation; const QPixmap m_defaultCover; + QTimer* m_timer; QSet m_runningIds; + QHash > m_queueOfUpdates; QString calculateMD5(QImage img); QImage cropImage(QImage img); From abc114d3b8efb34b5d9c08f57d680240c997a9c1 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 31 Jul 2014 10:55:00 +0100 Subject: [PATCH 32/43] TrackDAO::updateCoverArt() - updating multiple coverIds in a single query --- src/library/dao/trackdao.cpp | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index 3f853368bad..0c0396575a1 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -1251,18 +1251,19 @@ bool TrackDAO::updateCoverArt(QList trackIds, QList coverIds) { return false; } - // TODO: it should be done in a single query - QSqlQuery query(m_database); + QStringList idStringList; + QString sQuery = "UPDATE library SET cover_art = CASE id "; for (int i=0; i Date: Thu, 31 Jul 2014 11:10:00 +0100 Subject: [PATCH 33/43] TrackDAO::updateCoverArt() - trackIds and coverIds must have the same size --- src/library/dao/trackdao.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index 0c0396575a1..27a312a90bb 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -1247,7 +1247,8 @@ bool TrackDAO::updateCoverArt(int trackId, int coverId) { } bool TrackDAO::updateCoverArt(QList trackIds, QList coverIds) { - if (trackIds.isEmpty() || coverIds.isEmpty()) { + if (trackIds.size() != coverIds.size() || + trackIds.isEmpty() || coverIds.isEmpty()) { return false; } From 19fa25e581f7f2006db4f127537cd112e3e6c73d Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 31 Jul 2014 12:33:03 +0100 Subject: [PATCH 34/43] TrackDAO - add new signal to updateTracksInBTC - It is doing the same thing that the signal "tracksAdded" does. - However, it makes the code more readable (considering the purpose)... --- src/library/basetrackcache.cpp | 7 +++++++ src/library/basetrackcache.h | 1 + src/library/dao/trackdao.cpp | 3 +-- src/library/dao/trackdao.h | 1 + src/library/mixxxlibraryfeature.cpp | 2 ++ 5 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/library/basetrackcache.cpp b/src/library/basetrackcache.cpp index a4bd2537e29..bbab972ad4d 100644 --- a/src/library/basetrackcache.cpp +++ b/src/library/basetrackcache.cpp @@ -118,6 +118,13 @@ void BaseTrackCache::slotUpdateTrack(int trackId) { updateTrackInIndex(trackId); } +void BaseTrackCache::slotUpdateTracks(QSet trackIds) { + if (sDebug) { + qDebug() << this << "slotUpdateTracks" << trackIds.size(); + } + updateTracksInIndex(trackIds); +} + bool BaseTrackCache::isCached(int trackId) const { return m_trackInfo.contains(trackId); } diff --git a/src/library/basetrackcache.h b/src/library/basetrackcache.h index 0a0c1fcacf2..6917105a76d 100644 --- a/src/library/basetrackcache.h +++ b/src/library/basetrackcache.h @@ -69,6 +69,7 @@ class BaseTrackCache : public QObject { void slotTrackChanged(int trackId); void slotDbTrackAdded(TrackPointer pTrack); void slotUpdateTrack(int trackId); + void slotUpdateTracks(QSet trackId); private: TrackPointer lookupCachedTrack(int trackId) const; diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index 27a312a90bb..d6bb21c4f90 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -1268,8 +1268,7 @@ bool TrackDAO::updateCoverArt(QList trackIds, QList coverIds) { } // we also need to update the cover_art column in the tablemodel. - // TODO: we should create a new signal with a better name, updateTracksInBTC? - emit(tracksAdded(trackIds.toSet())); + emit(updateTracksInBTC(trackIds.toSet())); return true; } diff --git a/src/library/dao/trackdao.h b/src/library/dao/trackdao.h index 497da094d68..7578af53e9c 100644 --- a/src/library/dao/trackdao.h +++ b/src/library/dao/trackdao.h @@ -127,6 +127,7 @@ class TrackDAO : public QObject, public virtual DAO { void dbTrackAdded(TrackPointer pTrack); void progressVerifyTracksOutside(QString path); void updateTrackInBTC(int trackId); + void updateTracksInBTC(QSet trackIds); public slots: // The public interface to the TrackDAO requires a TrackPointer so that we diff --git a/src/library/mixxxlibraryfeature.cpp b/src/library/mixxxlibraryfeature.cpp index 3a4360f3ac3..f48130ceb79 100644 --- a/src/library/mixxxlibraryfeature.cpp +++ b/src/library/mixxxlibraryfeature.cpp @@ -102,6 +102,8 @@ MixxxLibraryFeature::MixxxLibraryFeature(QObject* parent, pBaseTrackCache, SLOT(slotDbTrackAdded(TrackPointer))); connect(&m_trackDao, SIGNAL(updateTrackInBTC(int)), pBaseTrackCache, SLOT(slotUpdateTrack(int))); + connect(&m_trackDao, SIGNAL(updateTracksInBTC(QSet)), + pBaseTrackCache, SLOT(slotUpdateTracks(QSet))); m_pBaseTrackCache = QSharedPointer(pBaseTrackCache); pTrackCollection->setTrackSource(m_pBaseTrackCache); From afcfa0238084cc97bac148e5851664bdf249f61a Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 31 Jul 2014 14:44:31 +0100 Subject: [PATCH 35/43] CoverCache - set maximum cover size from 400px to 300px --- src/library/coverartcache.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index 252a23226da..59d44699266 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -364,7 +364,7 @@ QImage CoverArtCache::cropImage(QImage img) { // if it's too big, we have to scale it. // big images would be quickly removed from cover cache. QImage CoverArtCache::rescaleBigImage(QImage img) { - const int MAXSIZE = 400; + const int MAXSIZE = 300; QSize size = img.size(); if (size.height() > MAXSIZE || size.width() > MAXSIZE) { return img.scaled(MAXSIZE, MAXSIZE, From 4bfc18c8101fe81dfdabab4db50736fbb16a5ac1 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 31 Jul 2014 16:00:01 +0100 Subject: [PATCH 36/43] CoverCache - rescaling covers before calcule md5 hash - md5 calculation might be a very slow process for big covers --- src/library/coverartcache.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index 59d44699266..c67bb75cf60 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -181,9 +181,10 @@ CoverArtCache::FutureResult CoverArtCache::searchImage( // res.img = extractEmbeddedCover(coverInfo.trackLocation); if (!res.img.isNull()) { - // it is the first time that we are loading the embedded cover, - // so we need to recalculate the md5 hash. + res.img = rescaleBigImage(res.img); if (res.md5Hash.isEmpty()) { + // it is the first time that we are loading the embedded cover, + // so we need to recalculate the md5 hash. res.md5Hash = calculateMD5(res.img); } res.newImgFound = true; @@ -195,21 +196,17 @@ CoverArtCache::FutureResult CoverArtCache::searchImage( res.coverLocation = searchInTrackDirectory(coverInfo.trackDirectory, coverInfo.trackBaseName, coverInfo.album); - res.img = QImage(res.coverLocation); + res.img = rescaleBigImage(QImage(res.coverLocation)); res.md5Hash = calculateMD5(res.img); res.newImgFound = true; } // adjusting the cover size according to the final purpose - if (res.newImgFound) { - if (res.croppedImg) { - res.img = cropImage(res.img); - } else { - res.img = rescaleBigImage(res.img); - } + if (res.newImgFound && res.croppedImg) { + res.img = cropImage(res.img); } - // check if this image is really new + // check if this image is really a new one // (different from the one that we have in db) if (coverInfo.md5Hash == res.md5Hash) { From 961b8a27c187ac49b7d489b591c57dda17f90936 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 31 Jul 2014 16:44:01 +0100 Subject: [PATCH 37/43] Fixing bug in searchImage() - the md5Hash string must start with the value got previously in the db - it will be essential to identify if Mixxx is just searching for an embedded cover that was previously loaded. Avoiding recalculate the md5 hash... --- src/library/coverartcache.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index c67bb75cf60..9937fde4c27 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -173,6 +173,7 @@ CoverArtCache::FutureResult CoverArtCache::searchImage( const bool emitSignals) { FutureResult res; res.trackId = coverInfo.trackId; + res.md5Hash = coverInfo.md5Hash; res.croppedImg = croppedPixmap; res.emitSignals = emitSignals; res.newImgFound = false; From e324d70e412477f16c5488f6d26d7dd2e1d1d3db Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Thu, 31 Jul 2014 21:05:43 +0100 Subject: [PATCH 38/43] cosmetic fix - fixing commented line --- src/library/coverartcache.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index 9937fde4c27..2c77430b618 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -322,7 +322,7 @@ void CoverArtCache::imageFound() { m_runningIds.remove(res.trackId); } -// sqlite can only do about 50 writes per second, +// sqlite can't do a huge number of updates in a very short time, // so it is important to collect all new covers and write them at once. void CoverArtCache::updateDB() { if (m_queueOfUpdates.isEmpty()) { From f662331a886670a4afc9fffe106d856c69fa9c4d Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Fri, 1 Aug 2014 16:44:44 +0100 Subject: [PATCH 39/43] CoverDAO::getCoverInfo() - improving performance by removing "indexOf()" calls The method getCoverInfo() can be called hundreds or even thousands times (correctly - it will depends on the library size) mainly in cases when the user is scrolling fast in a new library. I noticed that it were doing many "indexOf" calls in order to get the column numbers and as each "indexOf" call will do a new loop and this was resulting in a considerable loss of performance... just to do unnecessary things... --- src/library/dao/coverartdao.cpp | 53 ++++++++++++++++----------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/src/library/dao/coverartdao.cpp b/src/library/dao/coverartdao.cpp index 1adbbaf4d10..7b7e8c946cd 100644 --- a/src/library/dao/coverartdao.cpp +++ b/src/library/dao/coverartdao.cpp @@ -147,42 +147,39 @@ CoverArtDAO::CoverArtInfo CoverArtDAO::getCoverArtInfo(int trackId) { return CoverArtInfo(); } - QSqlQuery query(m_database); - query.prepare( - "SELECT album, cover_art.location AS cover, cover_art.md5 AS md5, " - "track_locations.directory AS directory, " - "track_locations.filename AS filename, " - "track_locations.location AS location " - "FROM Library INNER JOIN track_locations " - "ON library.location = track_locations.id " - "LEFT JOIN cover_art ON cover_art.id = library.cover_art " - "WHERE library.id=:id " - ); - query.bindValue(":id", trackId); + // This method can be called a lot of times (by CoverCache), + // if we use functions like "indexOf()" to find the column numbers + // it will do at least one new loop for each column and it can bring + // performance issues... + QString columns = "album," //0 + "cover_art.location AS cover," //1 + "cover_art.md5," //2 + "track_locations.directory," //3 + "track_locations.filename," //4 + "track_locations.location"; //5 + + QString sQuery = QString( + "SELECT " % columns % " FROM Library " + "INNER JOIN track_locations ON library.location = track_locations.id " + "LEFT JOIN cover_art ON cover_art.id = library.cover_art " + "WHERE library.id = %1").arg(trackId); - if (!query.exec()) { + QSqlQuery query(m_database); + if (!query.exec(sQuery)) { LOG_FAILED_QUERY(query); return CoverArtInfo(); } - QSqlRecord queryRecord = query.record(); - const int albumColumn = queryRecord.indexOf("album"); - const int coverColumn = queryRecord.indexOf("cover"); - const int md5Column = queryRecord.indexOf("md5"); - const int directoryColumn = queryRecord.indexOf("directory"); - const int filenameColumn = queryRecord.indexOf("filename"); - const int locationColumn = queryRecord.indexOf("location"); - if (query.next()) { CoverArtInfo coverInfo; coverInfo.trackId = trackId; - coverInfo.album = query.value(albumColumn).toString(); - coverInfo.coverLocation = query.value(coverColumn).toString(); - coverInfo.md5Hash = query.value(md5Column).toString(); - coverInfo.trackDirectory = query.value(directoryColumn).toString(); - coverInfo.trackLocation = query.value(locationColumn).toString(); - coverInfo.trackBaseName = QFileInfo(query.value(filenameColumn) - .toString()).baseName(); + coverInfo.album = query.value(0).toString(); + coverInfo.coverLocation = query.value(1).toString(); + coverInfo.md5Hash = query.value(2).toString(); + coverInfo.trackDirectory = query.value(3).toString(); + QString filename = query.value(4).toString(); + coverInfo.trackLocation = query.value(5).toString(); + coverInfo.trackBaseName = QFileInfo(filename).baseName(); return coverInfo; } From 1239ecae9dcac7c2d2ec26a493e0d60d32579924 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Fri, 1 Aug 2014 21:00:00 +0100 Subject: [PATCH 40/43] WTrackTableView - getting coverLocation and md5 column numbers just once - After some analysis with valgrind, I noticed that fieldIndex() can takes a long time... - (see commented lines) --- src/widget/wtracktableview.cpp | 22 +++++++++++++++++----- src/widget/wtracktableview.h | 4 ++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/widget/wtracktableview.cpp b/src/widget/wtracktableview.cpp index a2a424df6d1..0e4b6b7dc79 100644 --- a/src/widget/wtracktableview.cpp +++ b/src/widget/wtracktableview.cpp @@ -28,7 +28,9 @@ WTrackTableView::WTrackTableView(QWidget * parent, m_pConfig(pConfig), m_pTrackCollection(pTrackCollection), m_DlgTagFetcher(NULL), - m_sorting(sorting) { + m_sorting(sorting), + m_iCoverLocationColumn(-1), + m_iMd5Column(-1) { // Give a NULL parent because otherwise it inherits our style which can make // it unreadable. Bug #673411 m_pTrackInfo = new DlgTrackInfo(NULL,m_DlgTagFetcher); @@ -172,6 +174,10 @@ void WTrackTableView::slotGuiTickTime(double cpuTime) { } void WTrackTableView::slotLoadCoverArt() { + if (m_iCoverLocationColumn < 0 || m_iMd5Column < 0) { + return; + } + QString coverLocation; QString md5Hash; int trackId = 0; @@ -180,11 +186,10 @@ void WTrackTableView::slotLoadCoverArt() { QModelIndex idx = indices[0]; TrackModel* trackModel = getTrackModel(); if (trackModel) { - coverLocation = idx.sibling(idx.row(), trackModel->fieldIndex( - LIBRARYTABLE_COVERART_LOCATION)).data().toString(); - md5Hash = idx.sibling(idx.row(), trackModel->fieldIndex( - LIBRARYTABLE_COVERART_MD5)).data().toString(); + md5Hash = idx.sibling(idx.row(), m_iMd5Column).data().toString(); trackId = trackModel->getTrackId(idx); + coverLocation = idx.sibling(idx.row(), + m_iCoverLocationColumn).data().toString(); } } emit(loadCoverArt(coverLocation, md5Hash, trackId)); @@ -212,6 +217,13 @@ void WTrackTableView::loadTrackModel(QAbstractItemModel *model) { return; } + // The "coverLocation" and "md5" column numbers are very often required + // by slotLoadCoverArt(). As this value will not change when the model + // still the same, we must avoid doing hundreds of "fieldIndex" calls + // when it is completely unnecessary... + m_iCoverLocationColumn = track_model->fieldIndex(LIBRARYTABLE_COVERART_LOCATION); + m_iMd5Column = track_model->fieldIndex(LIBRARYTABLE_COVERART_MD5); + setVisible(false); // Save the previous track model's header state diff --git a/src/widget/wtracktableview.h b/src/widget/wtracktableview.h index 35d869e2c64..db667568d86 100644 --- a/src/widget/wtracktableview.h +++ b/src/widget/wtracktableview.h @@ -150,6 +150,10 @@ class WTrackTableView : public WLibraryTableView { bool m_sorting; + // Column numbers + int m_iCoverLocationColumn; // cover art location + int m_iMd5Column; // cover art md5 hash + // Control the delay to load a cover art. double m_lastSelection; bool m_bLastCoverLoaded; From fde5cd7b1c37004bd6646eb1bd7ed156c9748f50 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Sat, 2 Aug 2014 17:00:40 +0100 Subject: [PATCH 41/43] improving the management of the "covercache::m_queueOfUpdates" member - using qset instead of qlist - removing qassert - changing @param and @return of saveCoverArt() and updateCoverArt() --- src/library/coverartcache.cpp | 9 +++---- src/library/dao/coverartdao.cpp | 48 ++++++++++++++++++++++----------- src/library/dao/coverartdao.h | 5 +++- src/library/dao/trackdao.cpp | 26 +++++++++++------- src/library/dao/trackdao.h | 2 +- 5 files changed, 56 insertions(+), 34 deletions(-) diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index 2c77430b618..e046441202a 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -328,12 +328,9 @@ void CoverArtCache::updateDB() { if (m_queueOfUpdates.isEmpty()) { return; } - - QList coverIds = m_pCoverArtDAO->saveCoverArt(m_queueOfUpdates.values()); - - Q_ASSERT(m_queueOfUpdates.keys().size() == coverIds.size()); - m_pTrackDAO->updateCoverArt(m_queueOfUpdates.keys(), coverIds); - + QSet > covers; + covers = m_pCoverArtDAO->saveCoverArt(m_queueOfUpdates); + m_pTrackDAO->updateCoverArt(covers); m_queueOfUpdates.clear(); } diff --git a/src/library/dao/coverartdao.cpp b/src/library/dao/coverartdao.cpp index 7b7e8c946cd..fd1d80a9b41 100644 --- a/src/library/dao/coverartdao.cpp +++ b/src/library/dao/coverartdao.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -45,42 +46,57 @@ int CoverArtDAO::saveCoverArt(QString coverLocation, QString md5Hash) { return coverId; } -QList CoverArtDAO::saveCoverArt(QList > covers) { +QSet > CoverArtDAO::saveCoverArt( + QHash > covers) { if (covers.isEmpty()) { - return QList(); + return QSet >(); } // it'll be used to avoid writing a new ID for // rows which have the same md5 (not changed). QString selectCoverId = QString("SELECT id FROM cover_art WHERE md5='%1'"); + // + QSet > res; + // preparing query to insert multi rows QString sQuery; + QHashIterator > i(covers); + i.next(); + res.insert(qMakePair(i.key(), -1)); sQuery = QString("INSERT OR REPLACE INTO cover_art ('id', 'location', 'md5') " "SELECT (%1) AS 'id', '%2' AS 'location', '%3' AS 'md5' ") - .arg(selectCoverId.arg(covers.first().second)) - .arg(covers.first().first) - .arg(covers.first().second); - for (int i=1; i(); + return QSet >(); } - // getting the last inserted cover id's - QList coverIds; - for (int index=0; index > set(res); + while (set.hasNext()) { + QPair p = set.next(); + int trackId = p.first; + int coverId = getCoverArtId(covers.value(trackId).second); + if (coverId > 0) { + res.remove(p); + res.insert(qMakePair(trackId, coverId)); + } } - return coverIds; + return res; } void CoverArtDAO::deleteUnusedCoverArts() { diff --git a/src/library/dao/coverartdao.h b/src/library/dao/coverartdao.h index 1e6dec0c658..5c9db453ed6 100644 --- a/src/library/dao/coverartdao.h +++ b/src/library/dao/coverartdao.h @@ -22,7 +22,10 @@ class CoverArtDAO : public DAO { void deleteUnusedCoverArts(); int getCoverArtId(QString md5Hash); int saveCoverArt(QString coverLocation, QString md5Hash); - QList saveCoverArt(QList > covers); + + // @param covers: > + // @return + QSet > saveCoverArt(QHash > covers); struct CoverArtInfo { int trackId; diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index d6bb21c4f90..9e315ed34c3 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -1246,20 +1246,26 @@ bool TrackDAO::updateCoverArt(int trackId, int coverId) { return true; } -bool TrackDAO::updateCoverArt(QList trackIds, QList coverIds) { - if (trackIds.size() != coverIds.size() || - trackIds.isEmpty() || coverIds.isEmpty()) { +// @param +bool TrackDAO::updateCoverArt(QSet > covers) { + if (covers.isEmpty()) { return false; } - QStringList idStringList; + QSet trackIds; + QStringList trackIdsStringList; QString sQuery = "UPDATE library SET cover_art = CASE id "; - for (int i=0; i > set(covers); + while (set.hasNext()) { + QPair p = set.next(); + sQuery = sQuery % QString("WHEN '%1' THEN '%2' ").arg(p.first) + .arg(p.second); + trackIds.insert(p.first); + trackIdsStringList.append(QString::number(p.first)); } - sQuery = sQuery % QString("END WHERE id IN (%1)").arg(idStringList.join(",")); + sQuery = sQuery % QString("END WHERE id IN (%1)") + .arg(trackIdsStringList.join(",")); QSqlQuery query(m_database); if (!query.exec(sQuery)) { @@ -1268,7 +1274,7 @@ bool TrackDAO::updateCoverArt(QList trackIds, QList coverIds) { } // we also need to update the cover_art column in the tablemodel. - emit(updateTracksInBTC(trackIds.toSet())); + emit(updateTracksInBTC(trackIds)); return true; } diff --git a/src/library/dao/trackdao.h b/src/library/dao/trackdao.h index 7578af53e9c..746bab891f7 100644 --- a/src/library/dao/trackdao.h +++ b/src/library/dao/trackdao.h @@ -116,7 +116,7 @@ class TrackDAO : public QObject, public virtual DAO { // it will update the Library.cover_art column in DB bool updateCoverArt(int trackId, int coverId); - bool updateCoverArt(QList trackIds, QList coverIds); + bool updateCoverArt(QSet > covers); signals: void trackDirty(int trackId); From 747ad97a7977a39872a7424488853bef4237aa55 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Sat, 2 Aug 2014 21:01:10 +0100 Subject: [PATCH 42/43] CoverArtDelegate - set initial column width --- src/library/coverartdelegate.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index a1997497b2b..d1b37d72c60 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -19,8 +19,12 @@ CoverArtDelegate::CoverArtDelegate(QObject *parent) if (QTableView *tableView = qobject_cast(parent)) { m_pTableView = tableView; m_pTrackModel = dynamic_cast(m_pTableView->model()); - m_iCoverLocationColumn = m_pTrackModel->fieldIndex(LIBRARYTABLE_COVERART_LOCATION); m_iMd5Column = m_pTrackModel->fieldIndex(LIBRARYTABLE_COVERART_MD5); + m_iCoverLocationColumn = m_pTrackModel->fieldIndex( + LIBRARYTABLE_COVERART_LOCATION); + + int coverColumn = m_pTrackModel->fieldIndex(LIBRARYTABLE_COVERART); + m_pTableView->setColumnWidth(coverColumn, 100); } } From 5dfd3b8344ca991bd02c145dccb6826eb9746c71 Mon Sep 17 00:00:00 2001 From: Marcos CARDINOT Date: Sun, 3 Aug 2014 12:11:00 +0100 Subject: [PATCH 43/43] Remove unnecessary cover_art query from TrackDAO::getTrackFromDB --- src/library/dao/trackdao.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index 9e315ed34c3..70cf964d13f 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -842,7 +842,7 @@ TrackPointer TrackDAO::getTrackFromDB(const int id) const { QSqlQuery query(m_database); query.prepare( - "SELECT library.id, artist, title, album, album_artist, cover_art, year, genre, composer, " + "SELECT library.id, artist, title, album, album_artist, year, genre, composer, " "grouping, tracknumber, filetype, rating, key, track_locations.location as location, " "track_locations.filesize as filesize, comment, url, duration, bitrate, " "samplerate, cuepoint, bpm, replaygain, channels, " @@ -861,7 +861,6 @@ TrackPointer TrackDAO::getTrackFromDB(const int id) const { const int titleColumn = queryRecord.indexOf("title"); const int albumColumn = queryRecord.indexOf("album"); const int albumArtistColumn = queryRecord.indexOf("album_artist"); - const int coverArtColumn = queryRecord.indexOf("cover_art"); const int yearColumn = queryRecord.indexOf("year"); const int genreColumn = queryRecord.indexOf("genre"); const int composerColumn = queryRecord.indexOf("composer"); @@ -897,7 +896,6 @@ TrackPointer TrackDAO::getTrackFromDB(const int id) const { QString title = query.value(titleColumn).toString(); QString album = query.value(albumColumn).toString(); QString albumArtist = query.value(albumArtistColumn).toString(); - int coverArtId = query.value(coverArtColumn).toInt(); QString year = query.value(yearColumn).toString(); QString genre = query.value(genreColumn).toString(); QString composer = query.value(composerColumn).toString();