From 7eac2e5783e873415dfdad861e8a0f47c2307481 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 16 Oct 2023 20:16:38 +0200 Subject: [PATCH] Better ImageProvider->ThumbnailProvider The class handles avatars in a special way now - when passed a room id and, optionally, a user id for a member of that room, it spawns a newly introduced AvatarResponse that can retrieve the possibly already cached avatar from the respective Room/User object. This required some changes in QML, with common code moved out to the special Avatar specialisation of Image component. What warranted a dedicated QML component is that Quotient::Avatar returns the first (possibly empty or smaller resolution) image immediately while requesting the right one behind the scenes; so QML Avatar listens on the respective signals from the library and updates the image when the new avatar arrives - which (i.e. updating) is in turn somewhat "idiomatic" because of QTBUG-14900. QML Avatar encapsulates these two peculiarities. --- CMakeLists.txt | 2 +- client/imageprovider.cpp | 160 ------------------------- client/imageprovider.h | 37 ------ client/logging_categories.h | 2 +- client/qml/Avatar.qml | 27 +++++ client/qml/Timeline.qml | 6 +- client/qml/TimelineItem.qml | 10 +- client/resources.qrc | 1 + client/thumbnailprovider.cpp | 218 +++++++++++++++++++++++++++++++++++ client/thumbnailprovider.h | 35 ++++++ client/timelinewidget.cpp | 8 +- client/timelinewidget.h | 4 +- 12 files changed, 294 insertions(+), 216 deletions(-) delete mode 100644 client/imageprovider.cpp delete mode 100644 client/imageprovider.h create mode 100644 client/qml/Avatar.qml create mode 100644 client/thumbnailprovider.cpp create mode 100644 client/thumbnailprovider.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 6db7b103..2259f94b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -155,7 +155,7 @@ message( STATUS ) set(quaternion_SRCS client/quaternionroom.cpp client/htmlfilter.cpp - client/imageprovider.cpp + client/thumbnailprovider.cpp client/activitydetector.cpp client/dialog.cpp client/logindialog.cpp diff --git a/client/imageprovider.cpp b/client/imageprovider.cpp deleted file mode 100644 index 8c116d22..00000000 --- a/client/imageprovider.cpp +++ /dev/null @@ -1,160 +0,0 @@ -/************************************************************************** - * * - * SPDX-FileCopyrightText: 2016 Felix Rohrbach * - * * - * SPDX-License-Identifier: GPL-3.0-or-later - * * - **************************************************************************/ - -#include "imageprovider.h" - -#include "logging_categories.h" - -#include -#include - -#include -#include -#include - -using Quotient::Connection; -using Quotient::BaseJob; - -class ThumbnailResponse : public QQuickImageResponse -{ - Q_OBJECT - public: - ThumbnailResponse(Connection* c, QString id, QSize size) - : c(c), mediaId(std::move(id)), requestedSize(size) - { - if (!c) - errorStr = tr("No connection to perform image request"); - else if (mediaId.count('/') != 1) - errorStr = - tr("Media id '%1' doesn't follow server/mediaId pattern") - .arg(mediaId); - else if (requestedSize.isEmpty()) { - qCDebug(IMAGEPROVIDER) - << "Returning an empty image for" << mediaId - << "due to empty" << requestedSize; - image = {requestedSize, QImage::Format_Invalid}; - } - if (!errorStr.isEmpty() || requestedSize.isEmpty()) { - emit finished(); - return; - } - // We are good to go - qCDebug(IMAGEPROVIDER).nospace() - << "Requesting " << mediaId << ", " << size; - errorStr = tr("Image request is pending"); - - // Execute a request on the main thread asynchronously - moveToThread(c->thread()); - QMetaObject::invokeMethod(this, &ThumbnailResponse::startRequest); - } - ~ThumbnailResponse() override = default; - - private slots: - // All these run in the main thread, not QML thread - - void startRequest() - { - Q_ASSERT(QThread::currentThread() == c->thread()); - - job = c->getThumbnail(mediaId, requestedSize); - // Connect to any possible outcome including abandonment - // to make sure the QML thread is not left stuck forever. - connect(job, &BaseJob::finished, - this, &ThumbnailResponse::prepareResult); - } - - void prepareResult() - { - Q_ASSERT(QThread::currentThread() == job->thread()); - Q_ASSERT(job->error() != BaseJob::Pending); - { - QWriteLocker _(&lock); - if (job->error() == BaseJob::Success) - { - image = job->thumbnail(); - errorStr.clear(); - qCDebug(IMAGEPROVIDER).nospace() - << "Image ready for " << mediaId << ", " - << image.size(); - } else if (job->error() == BaseJob::Abandoned) { - errorStr = tr("Image request has been cancelled"); - qCDebug(IMAGEPROVIDER) - << "Request cancelled for" << mediaId; - } else { - errorStr = job->errorString(); - qCWarning(IMAGEPROVIDER).nospace() - << "No valid image for" << mediaId << ": " << errorStr; - } - } - job = nullptr; - emit finished(); - } - - void doCancel() - { - if (job) - { - Q_ASSERT(QThread::currentThread() == job->thread()); - job->abandon(); - } - } - - private: - Connection* c; - const QString mediaId; - const QSize requestedSize; - QPointer job = nullptr; - - QImage image; - QString errorStr; - mutable QReadWriteLock lock; // Guards ONLY these two above - - // The following overrides run in QML thread - - QQuickTextureFactory *textureFactory() const override - { - QReadLocker _(&lock); - return QQuickTextureFactory::textureFactoryForImage(image); - } - - QString errorString() const override - { - QReadLocker _(&lock); - return errorStr; - } - - void cancel() override - { - // Flip from QML thread to the main thread - QMetaObject::invokeMethod(this, &ThumbnailResponse::doCancel); - } -}; - -#include "imageprovider.moc" // Because we define a Q_OBJECT in the cpp file - -ImageProvider::ImageProvider(Connection* connection) - : m_connection(connection) -{ } - -QQuickImageResponse* ImageProvider::requestImageResponse( - const QString& id, const QSize& requestedSize) -{ - auto size = requestedSize; - // Force integer overflow if the value is -1 - may cause issues when - // screens resolution becomes 100K+ each dimension :-D - if (size.width() == -1) - size.setWidth(ushort(-1)); - if (size.height() == -1) - size.setHeight(ushort(-1)); - return new ThumbnailResponse(m_connection.loadRelaxed(), id, size); -} - -void ImageProvider::setConnection(Connection* connection) -{ - m_connection.storeRelaxed(connection); -} diff --git a/client/imageprovider.h b/client/imageprovider.h deleted file mode 100644 index f1502660..00000000 --- a/client/imageprovider.h +++ /dev/null @@ -1,37 +0,0 @@ -/************************************************************************** - * * - * SPDX-FileCopyrightText: 2016 Felix Rohrbach * - * * - * SPDX-License-Identifier: GPL-3.0-or-later - * * - **************************************************************************/ - -#pragma once - -#include -#include - -namespace Quotient { - class Connection; -} - -// FIXME: It's actually ThumbnailProvider, not ImageProvider, because internally -// it calls MediaThumbnailJob. Trying to get a full-size image using this -// provider may bring suboptimal results (and generally you shouldn't do it -// because images loaded by QML are not necessarily cached to disk, so it's a -// waste of bandwidth). - -class ImageProvider: public QQuickAsyncImageProvider -{ - public: - explicit ImageProvider(Quotient::Connection* connection = nullptr); - - QQuickImageResponse* requestImageResponse( - const QString& id, const QSize& requestedSize) override; - - void setConnection(Quotient::Connection* connection); - - private: - QAtomicPointer m_connection; - Q_DISABLE_COPY(ImageProvider) -}; diff --git a/client/logging_categories.h b/client/logging_categories.h index 974aaedf..9f3f0a25 100644 --- a/client/logging_categories.h +++ b/client/logging_categories.h @@ -16,7 +16,7 @@ QUO_LOGGING_CATEGORY(EVENTMODEL, "quaternion.models.events") QUO_LOGGING_CATEGORY(TIMELINE, "quaternion.timeline") QUO_LOGGING_CATEGORY(HTMLFILTER, "quaternion.htmlfilter") QUO_LOGGING_CATEGORY(MSGINPUT, "quaternion.messageinput") -QUO_LOGGING_CATEGORY(IMAGEPROVIDER, "quaternion.imageprovider") +QUO_LOGGING_CATEGORY(THUMBNAILS, "quaternion.thumbnails") // Only to be used in QML; shows up here for documentation purpose only [[maybe_unused]] QUO_LOGGING_CATEGORY(TIMELINEQML, "quaternion.timeline.qml") diff --git a/client/qml/Avatar.qml b/client/qml/Avatar.qml new file mode 100644 index 00000000..1acb9938 --- /dev/null +++ b/client/qml/Avatar.qml @@ -0,0 +1,27 @@ +import QtQuick 2.15 + +Image { + readonly property var forRoom: root.room + /* readonly */ property var forMember + + property string sourceId: forRoom ? "image://thumbnail/" + forRoom.id + + (forMember ? '/' + forMember.id : "") + : "" + source: sourceId + cache: false // Quotient::Avatar takes care of caching + fillMode: Image.PreserveAspectFit + + function reload() { + source = "" + source = Qt.binding(function() { return sourceId }) + } + + Connections { + target: forRoom + function onAvatarChanged() { parent.reload() } + function onMemberAvatarChanged(member) { + if (member === parent.forMember) + parent.reload() + } + } +} diff --git a/client/qml/Timeline.qml b/client/qml/Timeline.qml index 094e8f86..05d46b98 100644 --- a/client/qml/Timeline.qml +++ b/client/qml/Timeline.qml @@ -42,7 +42,7 @@ Page { property bool showTopic: true - Image { + Avatar { id: roomAvatar anchors.verticalCenter: headerText.verticalCenter anchors.left: parent.left @@ -53,13 +53,9 @@ Page { width: Math.min(headerText.height / implicitHeight * implicitWidth, parent.width / 2.618) // Golden ratio - just for fun - source: room && room.avatarMediaId - ? "image://mtx/" + room.avatarMediaId : "" // Safe upper limit (see also topicField) sourceSize: Qt.size(-1, settings.lineSpacing * 9) - fillMode: Image.PreserveAspectFit - AnimationBehavior on width { NormalNumberAnimation { easing.type: Easing.OutQuad } } diff --git a/client/qml/TimelineItem.qml b/client/qml/TimelineItem.qml index 7254983d..6b6021b9 100644 --- a/client/qml/TimelineItem.qml +++ b/client/qml/TimelineItem.qml @@ -216,10 +216,10 @@ Item { text: "<" + time + ">" } - Image { + Avatar { id: authorAvatar visible: (authorSectionVisible || settings.timelineStyleIsXChat) - && settings.show_author_avatars && author.avatarMediaId + && settings.show_author_avatars && paintedHeight > 0 anchors.left: timelabel.right anchors.leftMargin: 3 height: visible ? settings.minimalTimelineItemHeight @@ -228,11 +228,9 @@ Item { width: settings.show_author_avatars * settings.minimalTimelineItemHeight - fillMode: Image.PreserveAspectFit horizontalAlignment: Image.AlignRight - source: author.avatarMediaId - ? "image://mtx/" + author.avatarMediaId : "" + forMember: author sourceSize: Qt.size(width, -1) AuthorInteractionArea { } @@ -427,7 +425,7 @@ Item { ? "" : content.info && content.info.thumbnail_info && !autoload - ? "image://mtx/" + content.thumbnailMediaId + ? "image://thumbnail/" + content.thumbnailMediaId : "" maxHeight: chatView.height - textField.height - authorLabel.height * !settings.timelineStyleIsXChat diff --git a/client/resources.qrc b/client/resources.qrc index 92513fe7..7ead8d90 100644 --- a/client/resources.qrc +++ b/client/resources.qrc @@ -22,5 +22,6 @@ qml/AnimatedTransition.qml qml/ScrollFinisher.qml qml/Logger.qml + qml/Avatar.qml diff --git a/client/thumbnailprovider.cpp b/client/thumbnailprovider.cpp new file mode 100644 index 00000000..d7302f4f --- /dev/null +++ b/client/thumbnailprovider.cpp @@ -0,0 +1,218 @@ +/************************************************************************** + * * + * SPDX-FileCopyrightText: 2016 Felix Rohrbach * + * * + * SPDX-License-Identifier: GPL-3.0-or-later + * * + **************************************************************************/ + +#include "thumbnailprovider.h" + +#include "logging_categories.h" + +#include +#include +#include + +#include +#include +#include + +using Quotient::Connection; +using Quotient::BaseJob; + +inline QDebug operator<<(QDebug dbg, const auto&& size) + requires std::is_same_v, QSize> +{ + QDebugStateSaver _(dbg); + return dbg.nospace() << size.width() << 'x' << size.height(); +} + +class AbstractThumbnailResponse : public QQuickImageResponse { + Q_OBJECT +public: + AbstractThumbnailResponse(Connection* c, QString id, QSize size) + : connection(c), mediaId(std::move(id)), requestedSize(size) + { + qCDebug(THUMBNAILS).noquote() + << mediaId << '@' << requestedSize << "requested"; + if (!c) + errorStr = tr("No connection to perform image request"); + else if (mediaId.isEmpty() || size.isEmpty()) { + qCDebug(THUMBNAILS) << "Returning an empty thumbnail"; + image = { requestedSize, QImage::Format_Invalid }; + } else if (!mediaId.startsWith('!') && mediaId.count('/') != 1) + errorStr = + tr("Media id '%1' doesn't follow server/mediaId pattern") + .arg(mediaId); + else { + errorStr = tr("Image request is pending"); + // Start a request on the main thread, concluding the initialisation + moveToThread(connection->thread()); + QMetaObject::invokeMethod(this, + &AbstractThumbnailResponse::startRequest); + // From this point, access to `image` and `errorStr` must be guarded + // by `lock` + return; + } + emit finished(); + } + +protected: + // The two below run in the main thread, not QML thread + virtual void startRequest() = 0; + virtual void doCancel() {} + + void finish(const QImage& result, const QString& error = {}) + { + { + QWriteLocker _(&lock); + image = result; + errorStr = error; + } + emit finished(); + } + + Connection* const connection = nullptr; + const QString mediaId{}; + const QSize requestedSize{}; + +private: + QImage image{}; + QString errorStr{}; + mutable QReadWriteLock lock{}; // Guards ONLY these two above + + // The following overrides run in QML thread + + QQuickTextureFactory* textureFactory() const override + { + QReadLocker _(&lock); + return QQuickTextureFactory::textureFactoryForImage(image); + } + + QString errorString() const override + { + QReadLocker _(&lock); + return errorStr; + } + + void cancel() override + { + // Flip from QML thread to the main thread + QMetaObject::invokeMethod(this, &AbstractThumbnailResponse::doCancel); + } +}; + +class ThumbnailResponse : public AbstractThumbnailResponse { + Q_OBJECT +public: + using AbstractThumbnailResponse::AbstractThumbnailResponse; + + ~ThumbnailResponse() override = default; + +private slots: + void startRequest() override + { + Q_ASSERT(QThread::currentThread() == connection->thread()); + + job = connection->getThumbnail(mediaId, requestedSize); + // Connect to any possible outcome including abandonment + // to make sure the QML thread is not left stuck forever. + connect(job, &BaseJob::finished, this, [this] { + Q_ASSERT(job->error() != BaseJob::Pending); + if (job->error() == BaseJob::Success) { + qCDebug(THUMBNAILS).noquote() + << "Thumbnail for" << mediaId + << "ready, actual size:" << job->thumbnail().size(); + finish(job->thumbnail()); + } else if (job->error() == BaseJob::Abandoned) { + qCDebug(THUMBNAILS) << "Request cancelled for" << mediaId; + finish({}, tr("Image request has been cancelled")); + } else { + qCWarning(THUMBNAILS).nospace() + << "No valid thumbnail for" << mediaId << ": " + << job->errorString(); + finish({}, job->errorString()); + } + job = nullptr; + }); + } + + void doCancel() override + { + if (job) { + Q_ASSERT(QThread::currentThread() == job->thread()); + job->abandon(); + } + } + +private: + QPointer job = nullptr; +}; + +class AvatarResponse : public AbstractThumbnailResponse { + Q_OBJECT +public: + using AbstractThumbnailResponse::AbstractThumbnailResponse; + +private: + Quotient::Room* room = nullptr; + + void startRequest() override + { + Q_ASSERT(QThread::currentThread() == connection->thread()); + const auto parts = mediaId.split(u'/'); + if (parts.size() > 2) { // Not tr() because it's internal error + qCCritical(THUMBNAILS) << "Avatar reference '" << mediaId + << "' must look like 'roomid[/userid]'"; + Q_ASSERT(parts.size() <= 2); + } + room = connection->room(parts.front()); + Q_ASSERT(room != nullptr); + + // NB: both Room:avatar() and User::avatar() invocations return an image + // available right now and, if needed, request one with the better + // resolution asynchronously. To get this better resolution image, + // Avatar elements in QML should call Avatar.reload() in response to + // Room::avatarChanged() and Room::memberAvatarChanged() (sic!) + // respectively. + const auto& w = requestedSize.width(); + const auto& h = requestedSize.height(); + if (parts.size() == 1) { + // As of libQuotient 0.8, Room::avatar() is the only call in the + // Room::avatar*() family that substitutes the counterpart's + // avatar for a direct chat avatar. + prepareResult(room->avatar(w, h)); + return; + } + + auto* user = room->user(parts.back()); + Q_ASSERT(user != nullptr); + prepareResult(user->avatar(w, h, room)); + } + + void prepareResult(const QImage& avatar) + { + qCDebug(THUMBNAILS).noquote() << "Returning avatar for" << mediaId + << "with size:" << avatar.size(); + finish(avatar); + } +}; + +#include "thumbnailprovider.moc" // Because we define a Q_OBJECT in the cpp file + +QQuickImageResponse* ThumbnailProvider::requestImageResponse( + const QString& id, const QSize& requestedSize) +{ + auto size = requestedSize; + // Force integer overflow if the value is -1 - may cause issues when + // screens resolution becomes 100K+ each dimension :-D + if (size.width() == -1) + size.setWidth(ushort(-1)); + if (size.height() == -1) + size.setHeight(ushort(-1)); + auto* const c = m_connection.loadRelaxed(); + if (id.startsWith(u'!')) + return new AvatarResponse(c, id, size); + return new ThumbnailResponse(c, id, size); +} diff --git a/client/thumbnailprovider.h b/client/thumbnailprovider.h new file mode 100644 index 00000000..d0b8bdab --- /dev/null +++ b/client/thumbnailprovider.h @@ -0,0 +1,35 @@ +/************************************************************************** + * * + * SPDX-FileCopyrightText: 2016 Felix Rohrbach * + * * + * SPDX-License-Identifier: GPL-3.0-or-later + * * + **************************************************************************/ + +#pragma once + +#include +#include + +namespace Quotient { + class Connection; +} + +class ThumbnailProvider : public QQuickAsyncImageProvider { +public: + explicit ThumbnailProvider(Quotient::Connection* connection = nullptr) + : m_connection(connection) + { } + + QQuickImageResponse* requestImageResponse( + const QString& id, const QSize& requestedSize) override; + + void setConnection(Quotient::Connection* connection) + { + m_connection.storeRelaxed(connection); + } + +private: + QAtomicPointer m_connection; + Q_DISABLE_COPY(ThumbnailProvider) +}; diff --git a/client/timelinewidget.cpp b/client/timelinewidget.cpp index 962361bc..68e7aecd 100644 --- a/client/timelinewidget.cpp +++ b/client/timelinewidget.cpp @@ -2,7 +2,7 @@ #include "chatroomwidget.h" #include "models/messageeventmodel.h" -#include "imageprovider.h" +#include "thumbnailprovider.h" #include "logging_categories.h" #include @@ -25,7 +25,7 @@ using Quotient::operator""_ls; TimelineWidget::TimelineWidget(ChatRoomWidget* chatRoomWidget) : QQuickWidget(chatRoomWidget) , m_messageModel(new MessageEventModel(this)) - , m_imageProvider(new ImageProvider()) + , m_thumbnailProvider(new ThumbnailProvider()) , indexToMaybeRead(-1) , readMarkerOnScreen(false) , roomWidget(chatRoomWidget) @@ -45,7 +45,7 @@ TimelineWidget::TimelineWidget(ChatRoomWidget* chatRoomWidget) setResizeMode(SizeRootObjectToView); - engine()->addImageProvider("mtx"_ls, m_imageProvider); + engine()->addImageProvider("thumbnail"_ls, m_thumbnailProvider); auto* ctxt = rootContext(); ctxt->setContextProperty("messageModel"_ls, m_messageModel); @@ -85,7 +85,7 @@ void TimelineWidget::setRoom(QuaternionRoom* newRoom) // Update the image provider upfront to allow image requests from // QML bindings to MessageEventModel::roomChanged - m_imageProvider->setConnection(newRoom ? newRoom->connection() : nullptr); + m_thumbnailProvider->setConnection(newRoom ? newRoom->connection() : nullptr); m_messageModel->changeRoom(newRoom); if (newRoom) { connect(newRoom, &Quotient::Room::fullyReadMarkerMoved, this, [this] { diff --git a/client/timelinewidget.h b/client/timelinewidget.h index 1940ae7a..c412b038 100644 --- a/client/timelinewidget.h +++ b/client/timelinewidget.h @@ -9,7 +9,7 @@ class ChatRoomWidget; class MessageEventModel; -class ImageProvider; +class ThumbnailProvider; class QuaternionRoom; class TimelineWidget : public QQuickWidget { @@ -46,7 +46,7 @@ public slots: private: MessageEventModel* m_messageModel; - ImageProvider* m_imageProvider; + ThumbnailProvider* m_thumbnailProvider; QString m_selectedText; using timeline_index_t = Quotient::TimelineItem::index_t;