diff --git a/include/AvatarProvider.h b/include/AvatarProvider.h index f3441290..95a5765d 100644 --- a/include/AvatarProvider.h +++ b/include/AvatarProvider.h @@ -25,9 +25,12 @@ class MatrixClient; class TimelineItem; +//! Saved cache data per user. struct AvatarData { + //! The avatar image of the user. QImage img; + //! The url that was used to download the avatar. QUrl url; }; @@ -37,17 +40,22 @@ class AvatarProvider : public QObject public: static void init(QSharedPointer client); - static void resolve(const QString &userId, std::function callback); + //! The callback is called with the downloaded avatar for the given user + //! or the avatar is downloaded first and then saved for re-use. + static void resolve(const QString &userId, + QObject *receiver, + std::function callback); + //! Used to initialize the mapping user -> avatar url. static void setAvatarUrl(const QString &userId, const QUrl &url); - - static void clear(); + //! Remove all saved data. + static void clear() { avatars_.clear(); }; private: + //! Update the cache with the downloaded avatar. static void updateAvatar(const QString &uid, const QImage &img); static QSharedPointer client_; using UserID = QString; static std::map avatars_; - static std::map>> toBeResolved_; }; diff --git a/include/MatrixClient.h b/include/MatrixClient.h index 3052a118..0e15dc2d 100644 --- a/include/MatrixClient.h +++ b/include/MatrixClient.h @@ -29,6 +29,7 @@ class DownloadMediaProxy : public QObject signals: void imageDownloaded(const QPixmap &data); void fileDownloaded(const QByteArray &data); + void avatarDownloaded(const QImage &img); }; /* @@ -59,14 +60,12 @@ public: void versions() noexcept; void fetchRoomAvatar(const QString &roomid, const QUrl &avatar_url); //! Download user's avatar. - void fetchUserAvatar(const QUrl &avatarUrl, - std::function onSuccess, - std::function onError); + QSharedPointer fetchUserAvatar(const QUrl &avatarUrl); void fetchCommunityAvatar(const QString &communityId, const QUrl &avatarUrl); void fetchCommunityProfile(const QString &communityId); void fetchCommunityRooms(const QString &communityId); - DownloadMediaProxy *downloadImage(const QUrl &url); - DownloadMediaProxy *downloadFile(const QUrl &url); + QSharedPointer downloadImage(const QUrl &url); + QSharedPointer downloadFile(const QUrl &url); void messages(const QString &room_id, const QString &from_token, int limit = 30) noexcept; void uploadImage(const QString &roomid, const QString &filename, diff --git a/include/timeline/TimelineItem.h b/include/timeline/TimelineItem.h index ade2f834..b7a5623f 100644 --- a/include/timeline/TimelineItem.h +++ b/include/timeline/TimelineItem.h @@ -182,7 +182,8 @@ TimelineItem::setupLocalWidgetLayout(Widget *widget, headerLayout_->addLayout(widgetLayout_); messageLayout_->addLayout(headerLayout_, 1); - AvatarProvider::resolve(userid, [this](const QImage &img) { setUserAvatar(img); }); + AvatarProvider::resolve( + userid, this, [this](const QImage &img) { setUserAvatar(img); }); } else { setupSimpleLayout(); @@ -230,7 +231,8 @@ TimelineItem::setupWidgetLayout(Widget *widget, headerLayout_->addLayout(widgetLayout_); messageLayout_->addLayout(headerLayout_, 1); - AvatarProvider::resolve(sender, [this](const QImage &img) { setUserAvatar(img); }); + AvatarProvider::resolve( + sender, this, [this](const QImage &img) { setUserAvatar(img); }); } else { setupSimpleLayout(); diff --git a/src/AvatarProvider.cc b/src/AvatarProvider.cc index 1c509d0a..e6e98886 100644 --- a/src/AvatarProvider.cc +++ b/src/AvatarProvider.cc @@ -21,7 +21,6 @@ QSharedPointer AvatarProvider::client_; std::map AvatarProvider::avatars_; -std::map>> AvatarProvider::toBeResolved_; void AvatarProvider::init(QSharedPointer client) @@ -32,22 +31,14 @@ AvatarProvider::init(QSharedPointer client) void AvatarProvider::updateAvatar(const QString &uid, const QImage &img) { - if (toBeResolved_.find(uid) != toBeResolved_.end()) { - auto callbacks = toBeResolved_[uid]; - - // Update all the timeline items with the resolved avatar. - for (const auto &callback : callbacks) - callback(img); - - toBeResolved_.erase(uid); - } - auto avatarData = &avatars_[uid]; avatarData->img = img; } void -AvatarProvider::resolve(const QString &userId, std::function callback) +AvatarProvider::resolve(const QString &userId, + QObject *receiver, + std::function callback) { if (avatars_.find(userId) == avatars_.end()) return; @@ -59,23 +50,19 @@ AvatarProvider::resolve(const QString &userId, std::function callb return; } - // Add the current timeline item to the waiting list for this avatar. - if (toBeResolved_.find(userId) == toBeResolved_.end()) { - client_->fetchUserAvatar(avatars_[userId].url, - [userId](QImage image) { updateAvatar(userId, image); }, - [userId](QString error) { - qWarning() - << error << ": failed to retrieve user avatar" - << userId; - }); + auto proxy = client_->fetchUserAvatar(avatars_[userId].url); - std::vector> items; - items.emplace_back(callback); + if (proxy == nullptr) + return; - toBeResolved_.emplace(userId, items); - } else { - toBeResolved_[userId].emplace_back(callback); - } + connect(proxy.data(), + &DownloadMediaProxy::avatarDownloaded, + receiver, + [userId, proxy, callback](const QImage &img) { + proxy->deleteLater(); + updateAvatar(userId, img); + callback(img); + }); } void @@ -86,10 +73,3 @@ AvatarProvider::setAvatarUrl(const QString &userId, const QUrl &url) avatars_.emplace(userId, data); } - -void -AvatarProvider::clear() -{ - avatars_.clear(); - toBeResolved_.clear(); -} diff --git a/src/ChatPage.cc b/src/ChatPage.cc index b49fb6a2..0d69c51f 100644 --- a/src/ChatPage.cc +++ b/src/ChatPage.cc @@ -579,11 +579,20 @@ ChatPage::updateOwnProfileInfo(const QUrl &avatar_url, const QString &display_na user_info_widget_->setUserId(userid); user_info_widget_->setDisplayName(display_name); - if (avatar_url.isValid()) - client_->fetchUserAvatar( - avatar_url, - [this](QImage img) { user_info_widget_->setAvatar(img); }, - [](QString error) { qWarning() << error << ": failed to fetch own avatar"; }); + if (avatar_url.isValid()) { + auto proxy = client_->fetchUserAvatar(avatar_url); + if (proxy == nullptr) + return; + + proxy->setParent(this); + connect(proxy.data(), + &DownloadMediaProxy::avatarDownloaded, + this, + [this, proxy](const QImage &img) { + proxy->deleteLater(); + user_info_widget_->setAvatar(img); + }); + } } void diff --git a/src/MatrixClient.cc b/src/MatrixClient.cc index dff7b825..60feefde 100644 --- a/src/MatrixClient.cc +++ b/src/MatrixClient.cc @@ -709,16 +709,14 @@ MatrixClient::fetchCommunityRooms(const QString &communityId) }); } -void -MatrixClient::fetchUserAvatar(const QUrl &avatarUrl, - std::function onSuccess, - std::function onError) +QSharedPointer +MatrixClient::fetchUserAvatar(const QUrl &avatarUrl) { QList url_parts = avatarUrl.toString().split("mxc://"); if (url_parts.size() != 2) { - qDebug() << "Invalid format for user avatar " << avatarUrl.toString(); - return; + qDebug() << "Invalid format for user avatar:" << avatarUrl.toString(); + return nullptr; } QUrlQuery query; @@ -735,33 +733,42 @@ MatrixClient::fetchUserAvatar(const QUrl &avatarUrl, QNetworkRequest avatar_request(endpoint); auto reply = get(avatar_request); - connect(reply, &QNetworkReply::finished, this, [reply, onSuccess, onError]() { + auto proxy = QSharedPointer( + new DownloadMediaProxy, [this](auto proxy) { proxy->deleteLater(); }); + connect(reply, &QNetworkReply::finished, this, [reply, proxy, avatarUrl]() { reply->deleteLater(); int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); - if (status == 0 || status >= 400) - return onError(reply->errorString()); + if (status == 0 || status >= 400) { + qWarning() << reply->errorString() << avatarUrl; + return; + } auto data = reply->readAll(); - if (data.size() == 0) - return onError("received avatar with no data"); + if (data.size() == 0) { + qWarning() << "received avatar with no data:" << avatarUrl; + return; + } QImage img; img.loadFromData(data); - onSuccess(std::move(img)); + emit proxy->avatarDownloaded(img); }); + + return proxy; } -DownloadMediaProxy * +QSharedPointer MatrixClient::downloadImage(const QUrl &url) { QNetworkRequest image_request(url); auto reply = get(image_request); - auto proxy = new DownloadMediaProxy; + auto proxy = QSharedPointer( + new DownloadMediaProxy, [this](auto proxy) { proxy->deleteLater(); }); connect(reply, &QNetworkReply::finished, this, [reply, proxy]() { reply->deleteLater(); @@ -786,13 +793,14 @@ MatrixClient::downloadImage(const QUrl &url) return proxy; } -DownloadMediaProxy * +QSharedPointer MatrixClient::downloadFile(const QUrl &url) { QNetworkRequest fileRequest(url); auto reply = get(fileRequest); - auto proxy = new DownloadMediaProxy; + auto proxy = QSharedPointer( + new DownloadMediaProxy, [this](auto proxy) { proxy->deleteLater(); }); connect(reply, &QNetworkReply::finished, this, [reply, proxy]() { reply->deleteLater(); diff --git a/src/SuggestionsPopup.cpp b/src/SuggestionsPopup.cpp index 3a7b3852..8a900790 100644 --- a/src/SuggestionsPopup.cpp +++ b/src/SuggestionsPopup.cpp @@ -44,8 +44,8 @@ PopupItem::PopupItem(QWidget *parent, const QString &user_id) topLayout_->addWidget(avatar_); topLayout_->addWidget(userName_, 1); - /* AvatarProvider::resolve(user_id, [this](const QImage &img) { avatar_->setImage(img); }); - */ + AvatarProvider::resolve( + user_id, this, [this](const QImage &img) { avatar_->setImage(img); }); } void diff --git a/src/dialogs/ReadReceipts.cc b/src/dialogs/ReadReceipts.cc index 839871be..63ce68e6 100644 --- a/src/dialogs/ReadReceipts.cc +++ b/src/dialogs/ReadReceipts.cc @@ -51,7 +51,8 @@ ReceiptItem::ReceiptItem(QWidget *parent, const QString &user_id, uint64_t times topLayout_->addWidget(avatar_); topLayout_->addLayout(textLayout_, 1); - AvatarProvider::resolve(user_id, [this](const QImage &img) { avatar_->setImage(img); }); + AvatarProvider::resolve( + user_id, this, [this](const QImage &img) { avatar_->setImage(img); }); } QString diff --git a/src/timeline/TimelineItem.cc b/src/timeline/TimelineItem.cc index 326b2c14..0296c6cd 100644 --- a/src/timeline/TimelineItem.cc +++ b/src/timeline/TimelineItem.cc @@ -126,7 +126,8 @@ TimelineItem::TimelineItem(mtx::events::MessageType ty, messageLayout_->addLayout(headerLayout_, 1); - AvatarProvider::resolve(userid, [this](const QImage &img) { setUserAvatar(img); }); + AvatarProvider::resolve( + userid, this, [this](const QImage &img) { setUserAvatar(img); }); } else { generateBody(body); setupSimpleLayout(); @@ -259,7 +260,8 @@ TimelineItem::TimelineItem(const mtx::events::RoomEventaddLayout(headerLayout_, 1); - AvatarProvider::resolve(sender, [this](const QImage &img) { setUserAvatar(img); }); + AvatarProvider::resolve( + sender, this, [this](const QImage &img) { setUserAvatar(img); }); } else { generateBody(body); setupSimpleLayout(); @@ -303,7 +305,8 @@ TimelineItem::TimelineItem(const mtx::events::RoomEvent messageLayout_->addLayout(headerLayout_, 1); - AvatarProvider::resolve(sender, [this](const QImage &img) { setUserAvatar(img); }); + AvatarProvider::resolve( + sender, this, [this](const QImage &img) { setUserAvatar(img); }); } else { generateBody(emoteMsg); setupSimpleLayout(); @@ -352,7 +355,8 @@ TimelineItem::TimelineItem(const mtx::events::RoomEvent messageLayout_->addLayout(headerLayout_, 1); - AvatarProvider::resolve(sender, [this](const QImage &img) { setUserAvatar(img); }); + AvatarProvider::resolve( + sender, this, [this](const QImage &img) { setUserAvatar(img); }); } else { generateBody(body); setupSimpleLayout(); @@ -562,5 +566,5 @@ TimelineItem::addAvatar() messageLayout_->addWidget(checkmark_); messageLayout_->addWidget(timestamp_); - AvatarProvider::resolve(userid, [this](const QImage &img) { setUserAvatar(img); }); + AvatarProvider::resolve(userid, this, [this](const QImage &img) { setUserAvatar(img); }); } diff --git a/src/timeline/widgets/AudioItem.cc b/src/timeline/widgets/AudioItem.cc index 9f8b5dd1..79f944ff 100644 --- a/src/timeline/widgets/AudioItem.cc +++ b/src/timeline/widgets/AudioItem.cc @@ -135,7 +135,7 @@ AudioItem::mousePressEvent(QMouseEvent *event) return; auto proxy = client_->downloadFile(url_); - connect(proxy, + connect(proxy.data(), &DownloadMediaProxy::fileDownloaded, this, [proxy, this](const QByteArray &data) { diff --git a/src/timeline/widgets/FileItem.cc b/src/timeline/widgets/FileItem.cc index d11ebe91..7445eb0f 100644 --- a/src/timeline/widgets/FileItem.cc +++ b/src/timeline/widgets/FileItem.cc @@ -121,7 +121,7 @@ FileItem::mousePressEvent(QMouseEvent *event) return; auto proxy = client_->downloadFile(url_); - connect(proxy, + connect(proxy.data(), &DownloadMediaProxy::fileDownloaded, this, [proxy, this](const QByteArray &data) { diff --git a/src/timeline/widgets/ImageItem.cc b/src/timeline/widgets/ImageItem.cc index fc1e46f5..3aae63c8 100644 --- a/src/timeline/widgets/ImageItem.cc +++ b/src/timeline/widgets/ImageItem.cc @@ -56,11 +56,13 @@ ImageItem::ImageItem(QSharedPointer client, auto proxy = client_.data()->downloadImage(url_); - connect( - proxy, &DownloadMediaProxy::imageDownloaded, this, [this, proxy](const QPixmap &img) { - proxy->deleteLater(); - setImage(img); - }); + connect(proxy.data(), + &DownloadMediaProxy::imageDownloaded, + this, + [this, proxy](const QPixmap &img) { + proxy->deleteLater(); + setImage(img); + }); } ImageItem::ImageItem(QSharedPointer client, @@ -92,11 +94,13 @@ ImageItem::ImageItem(QSharedPointer client, auto proxy = client_.data()->downloadImage(url_); - connect( - proxy, &DownloadMediaProxy::imageDownloaded, this, [proxy, this](const QPixmap &img) { - proxy->deleteLater(); - setImage(img); - }); + connect(proxy.data(), + &DownloadMediaProxy::imageDownloaded, + this, + [proxy, this](const QPixmap &img) { + proxy->deleteLater(); + setImage(img); + }); } void @@ -230,7 +234,7 @@ ImageItem::saveAs() return; auto proxy = client_->downloadFile(url_); - connect(proxy, + connect(proxy.data(), &DownloadMediaProxy::fileDownloaded, this, [proxy, filename](const QByteArray &data) {