Merge pull request #12087 from daschuer/gh11131_3

Fix coverart tootip if cover is not cached
mixxxdj · Nov 18, 2023 · 9c8154b · 9c8154b
2 parents 99a6f83 + 6ba80e8
commit 9c8154b
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 5 deletions.
diff --git a/src/library/basetracktablemodel.cpp b/src/library/basetracktablemodel.cpp
@@ -133,6 +133,13 @@ BaseTrackTableModel::BaseTrackTableModel(
             &PlayerInfo::trackChanged,
             this,
             &BaseTrackTableModel::slotTrackChanged);
+    CoverArtCache* pCache = CoverArtCache::instance();
+    if (pCache) {
+        connect(pCache,
+                &CoverArtCache::coverFound,
+                this,
+                &BaseTrackTableModel::slotCoverFound);
+    }
 }
 
 void BaseTrackTableModel::initTableColumnsAndHeaderProperties(
@@ -550,17 +557,19 @@ QVariant BaseTrackTableModel::composeCoverArtToolTipHtml(
     if (!coverInfo.hasImage()) {
         return QPixmap();
     }
+    m_toolTipIndex = index;
     QPixmap pixmap = CoverArtCache::getCachedCover(
             coverInfo,
             absoluteHeightOfCoverartToolTip);
     if (pixmap.isNull()) {
         // Cache miss -> Don't show a tooltip, refresh cache
         // Height used for the width, in assumption that covers are squares
         CoverArtCache::requestUncachedCover(
-                nullptr,
+                this,
                 coverInfo,
                 absoluteHeightOfCoverartToolTip);
-        return QVariant();
+        //: Tooltip text on the cover art column shown when the cover is read from disk
+        return tr("Fetching image ...");
     }
     QByteArray data;
     QBuffer buffer(&data);
@@ -1109,3 +1118,15 @@ bool BaseTrackTableModel::updateTrackMood(
     return m_pTrackCollectionManager->updateTrackMood(pTrack, mood);
 }
 #endif // __EXTRA_METADATA__
+
+void BaseTrackTableModel::slotCoverFound(
+        const QObject* pRequester,
+        const CoverInfo& coverInfo,
+        const QPixmap& pixmap) {
+    Q_UNUSED(pixmap);
+    if (pRequester != this ||
+            getTrackLocation(m_toolTipIndex) != coverInfo.trackLocation) {
+        return;
+    }
+    emit dataChanged(m_toolTipIndex, m_toolTipIndex, {Qt::ToolTipRole});
+}
diff --git a/src/library/basetracktablemodel.h b/src/library/basetracktablemodel.h
@@ -244,6 +244,11 @@ class BaseTrackTableModel : public QAbstractTableModel, public TrackModel {
 
     void slotRefreshAllRows();
 
+    void slotCoverFound(
+            const QObject* pRequester,
+            const CoverInfo& coverInfo,
+            const QPixmap& pixmap);
+
   private:
     // Track models may reference tracks by an external id
     // TODO: TrackId should only be used for tracks from
@@ -276,5 +281,7 @@ class BaseTrackTableModel : public QAbstractTableModel, public TrackModel {
 
     TrackId m_previewDeckTrackId;
 
+    mutable QModelIndex m_toolTipIndex;
+
     static int s_bpmColumnPrecision;
 };
diff --git a/src/sources/metadatasourcetaglib.cpp b/src/sources/metadatasourcetaglib.cpp
@@ -91,14 +91,17 @@ MetadataSourceTagLib::importTrackMetadataAndCoverImage(
         kLogger.warning()
                 << "Nothing to import"
                 << "from file" << m_fileName
-                << "with type" << m_fileType;
+                << "of type" << m_fileType;
         return afterImport(ImportResult::Unavailable);
     }
     if (kLogger.traceEnabled()) {
         kLogger.trace() << "Importing"
-                        << ((pTrackMetadata && pCoverImage) ? "track metadata and cover art" : (pTrackMetadata ? "track metadata" : "cover art"))
+                        << ((pTrackMetadata && pCoverImage)
+                                           ? "track metadata and cover art"
+                                           : (pTrackMetadata ? "track metadata"
+                                                             : "cover art"))
                         << "from file" << m_fileName
-                        << "with type" << m_fileType;
+                        << "of type" << m_fileType;
     }
 
     // Rationale: If a file contains different types of tags only

diff --git a/src/test/librarytest.cpp b/src/test/librarytest.cpp
@@ -1,5 +1,6 @@
 #include "test/librarytest.h"
 
+#include "library/coverartcache.h"
 #include "library/library_prefs.h"
 #include "track/track.h"
 
@@ -33,6 +34,7 @@ LibraryTest::LibraryTest()
         : MixxxDbTest(kInMemoryDbConnection),
           m_pTrackCollectionManager(newTrackCollectionManager(config(), dbConnectionPooler())),
           m_keyNotationCO(mixxx::library::prefs::kKeyNotationConfigKey) {
+    CoverArtCache::createInstance();
 }
 
 TrackPointer LibraryTest::getOrAddTrackByLocation(

diff --git a/src/test/playermanagertest.cpp b/src/test/playermanagertest.cpp
@@ -8,6 +8,7 @@
 #include "engine/channels/enginedeck.h"
 #include "engine/enginebuffer.h"
 #include "engine/enginemixer.h"
+#include "library/coverartcache.h"
 #include "library/library.h"
 #include "library/trackcollectionmanager.h"
 #include "mixer/basetrackplayer.h"
@@ -61,6 +62,9 @@ class PlayerManagerTest : public MixxxDbTest, SoundSourceProviderRegistration {
         m_pSoundManager = std::make_shared<SoundManager>(m_pConfig, m_pEngine.get());
         m_pControlIndicatorTimer = std::make_shared<mixxx::ControlIndicatorTimer>(nullptr);
         m_pEngine->registerNonEngineChannelSoundIO(m_pSoundManager.get());
+
+        CoverArtCache::createInstance();
+
         m_pPlayerManager = std::make_shared<PlayerManager>(m_pConfig,
                 m_pSoundManager.get(),
                 m_pEffectsManager.get(),

diff --git a/src/widget/wlibrarytableview.cpp b/src/widget/wlibrarytableview.cpp
@@ -1,9 +1,12 @@
 #include "widget/wlibrarytableview.h"
 
+#include <QApplication>
 #include <QFocusEvent>
 #include <QFontMetrics>
 #include <QHeaderView>
+#include <QHelpEvent>
 #include <QScrollBar>
+#include <QToolTip>
 
 #include "moc_wlibrarytableview.cpp"
 #include "util/math.h"
@@ -389,3 +392,28 @@ QModelIndex WLibraryTableView::moveCursor(CursorAction cursorAction,
 
     return QTableView::moveCursor(cursorAction, modifiers);
 }
+
+void WLibraryTableView::dataChanged(
+        const QModelIndex& topLeft,
+        const QModelIndex& bottomRight,
+        const QVector<int>& roles) {
+    for (const auto& role : roles) {
+        // Note: At this point the tooltip is already showing
+        // "Fetching image ..." or still in an effect progress.
+        // QToolTip::isVisible() is false for the later.
+        if (role == Qt::ToolTipRole) {
+            QPoint globalPos = QCursor::pos();
+            QWidget* pViewPort = QApplication::widgetAt(globalPos);
+            if (pViewPort) {
+                QPoint viewPortPos = pViewPort->mapFromGlobal(globalPos);
+                if (indexAt(viewPortPos) == topLeft) {
+                    QHelpEvent toolTipEvent(QEvent::ToolTip,
+                            pViewPort->mapFromGlobal(globalPos),
+                            globalPos);
+                    viewportEvent(&toolTipEvent);
+                }
+            }
+        }
+    }
+    QAbstractItemView::dataChanged(topLeft, bottomRight, roles);
+};
diff --git a/src/widget/wlibrarytableview.h b/src/widget/wlibrarytableview.h
@@ -50,6 +50,11 @@ class WLibraryTableView : public QTableView, public virtual LibraryView {
     /// @param optional: index, otherwise row/column member vars are used
     void restoreCurrentIndex(const QModelIndex& index = QModelIndex());
 
+    void dataChanged(
+            const QModelIndex& topLeft,
+            const QModelIndex& bottomRight,
+            const QVector<int>& roles = QVector<int>()) override;
+
   signals:
     void loadTrack(TrackPointer pTrack);
     void loadTrackToPlayer(TrackPointer pTrack, const QString& group, bool play = false);