From c73828d6055db664354ae5d1b2637d379294ef8a Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 10 Dec 2017 00:07:46 +0100 Subject: [PATCH] Simplify DivePictureModel The code of DivePictureModel used a QHash to keep track of thumbnails. Not only was the code rather complex - it also had the consequence that pictures are sorted according to the hash function, i.e. seemingly random. This commit replaces the QHash by a simple QList which keeps track of thumbnails and some meta-data. Signed-off-by: Berthold Stoeger --- qt-models/divepicturemodel.cpp | 57 ++++++++++++----------------- qt-models/divepicturemodel.h | 13 +++---- qt-models/divesitepicturesmodel.cpp | 24 ++++-------- 3 files changed, 35 insertions(+), 59 deletions(-) diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp index e5b40b748..d135e0faa 100644 --- a/qt-models/divepicturemodel.cpp +++ b/qt-models/divepicturemodel.cpp @@ -9,22 +9,19 @@ extern QHash thumbnailCache; -SPixmap scaleImages(picturepointer picture) +void scaleImages(PictureEntry &entry) { - SPixmap ret; - ret.first = picture; - if (thumbnailCache.contains(picture->filename) && !thumbnailCache.value(picture->filename).isNull()) { - ret.second = thumbnailCache.value(picture->filename); + if (thumbnailCache.contains(entry.filename) && !thumbnailCache.value(entry.filename).isNull()) { + entry.image = thumbnailCache.value(entry.filename); } else { int dim = defaultIconMetrics().sz_pic; - QImage p = SHashedImage(picture); + QImage p = SHashedImage(entry.picture); if(!p.isNull()) { p = p.scaled(dim, dim, Qt::KeepAspectRatio); - thumbnailCache.insert(picture->filename, p); + thumbnailCache.insert(entry.filename, p); } - ret.second = p; + entry.image = p; } - return ret; } DivePictureModel *DivePictureModel::instance() @@ -33,7 +30,7 @@ DivePictureModel *DivePictureModel::instance() return self; } -DivePictureModel::DivePictureModel() : numberOfPictures(0) +DivePictureModel::DivePictureModel() { } @@ -47,30 +44,22 @@ void DivePictureModel::updateDivePicturesWhenDone(QList> futures) void DivePictureModel::updateDivePictures() { - if (numberOfPictures != 0) { - beginRemoveRows(QModelIndex(), 0, numberOfPictures - 1); - numberOfPictures = 0; + if (!pictures.isEmpty()) { + beginRemoveRows(QModelIndex(), 0, pictures.count() - 1); + pictures.clear(); endRemoveRows(); } // if the dive_table is empty, ignore the displayed_dive - numberOfPictures = dive_table.nr == 0 ? 0 : dive_get_picture_count(&displayed_dive); - if (numberOfPictures == 0) { + if (dive_table.nr == 0 || dive_get_picture_count(&displayed_dive) == 0) return; - } - stringPixmapCache.clear(); - SPictureList pictures; - FOR_EACH_PICTURE_NON_PTR(displayed_dive) { - stringPixmapCache[QString(picture->filename)].offsetSeconds = picture->offset.seconds; - pictures.push_back(picture); - } + FOR_EACH_PICTURE_NON_PTR(displayed_dive) + pictures.push_back({picture, picture->filename, QImage(), picture->offset.seconds}); - QList list = QtConcurrent::blockingMapped(pictures, scaleImages); - Q_FOREACH (const SPixmap &pixmap, list) - stringPixmapCache[pixmap.first->filename].image = pixmap.second; + QtConcurrent::blockingMap(pictures, scaleImages); - beginInsertRows(QModelIndex(), 0, numberOfPictures - 1); + beginInsertRows(QModelIndex(), 0, pictures.count() - 1); endInsertRows(); } @@ -86,28 +75,28 @@ QVariant DivePictureModel::data(const QModelIndex &index, int role) const if (!index.isValid()) return ret; - QString key = stringPixmapCache.keys().at(index.row()); + const PictureEntry &entry = pictures.at(index.row()); if (index.column() == 0) { switch (role) { case Qt::ToolTipRole: - ret = key; + ret = entry.filename; break; case Qt::DecorationRole: - ret = stringPixmapCache[key].image; + ret = entry.image; break; case Qt::DisplayRole: - ret = QFileInfo(key).fileName(); + ret = QFileInfo(entry.filename).fileName(); break; case Qt::DisplayPropertyRole: - ret = QFileInfo(key).filePath(); + ret = QFileInfo(entry.filename).filePath(); } } else if (index.column() == 1) { switch (role) { case Qt::UserRole: - ret = QVariant::fromValue((int)stringPixmapCache[key].offsetSeconds); + ret = QVariant::fromValue(entry.offsetSeconds); break; case Qt::DisplayRole: - ret = key; + ret = entry.filename; } } return ret; @@ -126,5 +115,5 @@ void DivePictureModel::removePicture(const QString &fileUrl, bool last) int DivePictureModel::rowCount(const QModelIndex &parent) const { Q_UNUSED(parent); - return numberOfPictures; + return pictures.count(); } diff --git a/qt-models/divepicturemodel.h b/qt-models/divepicturemodel.h index b0494d355..bbb82293e 100644 --- a/qt-models/divepicturemodel.h +++ b/qt-models/divepicturemodel.h @@ -6,17 +6,15 @@ #include #include -struct PhotoHelper { +struct PictureEntry { + struct picture *picture; + QString filename; QImage image; int offsetSeconds; }; -typedef QList SPictureList; -typedef struct picture *picturepointer; -typedef QPair SPixmap; - // function that will scale the pixmap, used inside the QtConcurrent thread. -SPixmap scaleImages(picturepointer picture); +void scaleImages(PictureEntry &entry); class DivePictureModel : public QAbstractTableModel { Q_OBJECT @@ -31,11 +29,10 @@ public: protected: DivePictureModel(); - int numberOfPictures; // Currently, load the images on the fly // Later, use a thread to load the images // Later, save the thumbnails so we don't need to reopen every time. - QHash stringPixmapCache; + QList pictures; }; #endif diff --git a/qt-models/divesitepicturesmodel.cpp b/qt-models/divesitepicturesmodel.cpp index 0cd444ca8..b83d616c0 100644 --- a/qt-models/divesitepicturesmodel.cpp +++ b/qt-models/divesitepicturesmodel.cpp @@ -17,30 +17,20 @@ DiveSitePicturesModel::DiveSitePicturesModel() { void DiveSitePicturesModel::updateDivePictures() { beginResetModel(); - numberOfPictures = 0; + pictures.clear(); endResetModel(); const uint32_t ds_uuid = displayed_dive_site.uuid; struct dive *d; int i; - stringPixmapCache.clear(); - SPictureList pictures; + for_each_dive (i, d) + if (d->dive_site_uuid == ds_uuid && dive_get_picture_count(d)) + FOR_EACH_PICTURE(d) + pictures.push_back({picture, picture->filename, QImage(), picture->offset.seconds}); - for_each_dive (i, d) { - if (d->dive_site_uuid == ds_uuid && dive_get_picture_count(d)) { - FOR_EACH_PICTURE(d) { - stringPixmapCache[QString(picture->filename)].offsetSeconds = picture->offset.seconds; - pictures.push_back(picture); - } - } - } + QtConcurrent::blockingMap(pictures, scaleImages); - QList list = QtConcurrent::blockingMapped(pictures, scaleImages); - Q_FOREACH (const SPixmap &pixmap, list) - stringPixmapCache[pixmap.first->filename].image = pixmap.second; - - numberOfPictures = list.count(); - beginInsertRows(QModelIndex(), 0, numberOfPictures - 1); + beginInsertRows(QModelIndex(), 0, pictures.count() - 1); endInsertRows(); }