From bb9b50ef99bf322922705873b55117eaf54a4598 Mon Sep 17 00:00:00 2001
From: Nicolas Vengeon <nicolas.vengeon@savoirfairelinux.com>
Date: Wed, 1 Feb 2023 17:33:24 -0500
Subject: [PATCH] messaging: revert to using QSortFilterProxyModel for the
message list
SFPM performs poorly compared to its C++ counterpart in cases where re-filtering often is required. Tests show SFPM taking roughly 10x that of QSFPM in certain cases when the entire conversation needs to be loaded.
Change-Id: I25792f0ab376ae11d42073a548934b743e59fa3e
---
src/app/commoncomponents/TimestampInfo.qml | 2 +-
.../mainview/components/MessageListView.qml | 26 ++-------------
src/app/messagesadapter.cpp | 25 +++++++++++---
src/app/messagesadapter.h | 33 +++++++++++++++++++
src/libclient/messagelistmodel.cpp | 7 ++--
src/libclient/messagelistmodel.h | 5 ---
6 files changed, 59 insertions(+), 39 deletions(-)
diff --git a/src/app/commoncomponents/TimestampInfo.qml b/src/app/commoncomponents/TimestampInfo.qml
index 584596848..db6f79027 100644
--- a/src/app/commoncomponents/TimestampInfo.qml
+++ b/src/app/commoncomponents/TimestampInfo.qml
@@ -36,7 +36,7 @@ ColumnLayout {
spacing: 0
Connections {
- target: MessagesAdapter.messageListModel
+ target: MessagesAdapter
function onTimestampUpdated() {
if ((showTime || showDay) && Timestamp !== undefined) {
formattedTime = MessagesAdapter.getFormattedTime(Timestamp)
diff --git a/src/app/mainview/components/MessageListView.qml b/src/app/mainview/components/MessageListView.qml
index 009337b14..2eb6a94d5 100644
--- a/src/app/mainview/components/MessageListView.qml
+++ b/src/app/mainview/components/MessageListView.qml
@@ -22,8 +22,6 @@ import QtQuick.Controls
import QtQuick.Layouts
import Qt.labs.qmlmodels
-import SortFilterProxyModel 0.2
-
import net.jami.Models 1.1
import net.jami.Adapters 1.1
import net.jami.Constants 1.1
@@ -191,7 +189,7 @@ JamiListView {
roles.includes(MessageList.PreviousBodies))) {
return
}
- const staleIndex = proxyModel.mapFromSource(tl).row
+ const staleIndex = tl.row
var pItem = root.itemAtIndex(staleIndex - 1)
var nItem = root.itemAtIndex(staleIndex + 1)
var ppItem = root.itemAtIndex(staleIndex + 2)
@@ -203,27 +201,7 @@ JamiListView {
}
}
- model: SortFilterProxyModel {
- id: proxyModel
- // There doesn't seem to a subscription to property change
- // events in the expression for sourceModel. This was originally
- // masked behind an unchanging QSortFilterProxyModel object that
- // just reset it's sourceModel in MessagesAdapter.
- property var messageListModel: MessagesAdapter.messageListModel
- onMessageListModelChanged: sourceModel = messageListModel
- filters: ExpressionFilter {
- readonly property int mergeType: Interaction.Type.MERGE
- readonly property int editedType: Interaction.Type.EDITED
- readonly property int reactionType: Interaction.Type.REACTION
- expression: Type !== mergeType
- && Type !== editedType
- && Type !== reactionType
- }
- sorters: ExpressionSorter {
- expression: modelLeft.index > modelRight.index
- }
- }
-
+ model: MessagesAdapter.messageListModel
delegate: DelegateChooser {
id: delegateChooser
diff --git a/src/app/messagesadapter.cpp b/src/app/messagesadapter.cpp
index a56554abe..85bdd0094 100644
--- a/src/app/messagesadapter.cpp
+++ b/src/app/messagesadapter.cpp
@@ -49,19 +49,28 @@ MessagesAdapter::MessagesAdapter(AppSettingsManager* settingsManager,
: QmlAdapterBase(instance, parent)
, settingsManager_(settingsManager)
, previewEngine_(previewEngine)
+ , filteredMsgListModel_(new FilteredMsgListModel(this))
, mediaInteractions_(std::make_unique<MessageListModel>())
+ , timestampTimer_(new QTimer(this))
{
connect(lrcInstance_, &LRCInstance::selectedConvUidChanged, [this]() {
set_replyToId("");
set_editId("");
const QString& convId = lrcInstance_->get_selectedConvUid();
const auto& conversation = lrcInstance_->getConversationFromConvUid(convId);
- set_messageListModel(QVariant::fromValue(conversation.interactions.get()));
+
+ // Reset the source model for the proxy model.
+ filteredMsgListModel_->setSourceModel(conversation.interactions.get());
+ set_messageListModel(QVariant::fromValue(filteredMsgListModel_));
+
set_currentConvComposingList(conversationTypersUrlToName(conversation.typers));
});
connect(previewEngine_, &PreviewEngine::infoReady, this, &MessagesAdapter::onPreviewInfoReady);
connect(previewEngine_, &PreviewEngine::linkified, this, &MessagesAdapter::onMessageLinkified);
+
+ connect(timestampTimer_, &QTimer::timeout, this, &MessagesAdapter::timestampUpdated);
+ timestampTimer_->start(timestampUpdateIntervalMs_);
}
void
@@ -106,7 +115,7 @@ void
MessagesAdapter::loadConversationUntil(const QString& to)
{
try {
- if (auto* model = messageListModel_.value<MessageListModel*>()) {
+ if (auto* model = getMsgListSourceModel()) {
auto idx = model->indexOfMessage(to);
if (idx == -1) {
auto accountId = lrcInstance_->get_currentAccountId();
@@ -393,7 +402,7 @@ MessagesAdapter::getTransferStats(const QString& msgId, int status)
QVariant
MessagesAdapter::dataForInteraction(const QString& interactionId, int role) const
{
- if (auto* model = messageListModel_.value<MessageListModel*>()) {
+ if (auto* model = getMsgListSourceModel()) {
auto idx = model->indexOfMessage(interactionId);
if (idx != -1)
return model->data(idx, role);
@@ -404,7 +413,7 @@ MessagesAdapter::dataForInteraction(const QString& interactionId, int role) cons
int
MessagesAdapter::getIndexOfMessage(const QString& interactionId) const
{
- if (auto* model = messageListModel_.value<MessageListModel*>()) {
+ if (auto* model = getMsgListSourceModel()) {
return model->indexOfMessage(interactionId);
}
return {};
@@ -741,3 +750,11 @@ MessagesAdapter::getConvMedias()
qDebug() << "Exception during getConvMedia:";
}
}
+
+MessageListModel*
+MessagesAdapter::getMsgListSourceModel() const
+{
+ // We are certain that filteredMsgListModel_'s source model is a MessageListModel,
+ // However it may be a nullptr if not yet set.
+ return static_cast<MessageListModel*>(filteredMsgListModel_->sourceModel());
+}
diff --git a/src/app/messagesadapter.h b/src/app/messagesadapter.h
index b19193e50..6cb8029b2 100644
--- a/src/app/messagesadapter.h
+++ b/src/app/messagesadapter.h
@@ -24,6 +24,32 @@
#include <QObject>
#include <QString>
+#include <QTimer>
+
+#include <QSortFilterProxyModel>
+
+class FilteredMsgListModel final : public QSortFilterProxyModel
+{
+ Q_OBJECT
+public:
+ explicit FilteredMsgListModel(QObject* parent = nullptr)
+ : QSortFilterProxyModel(parent)
+ {
+ sort(0, Qt::AscendingOrder);
+ }
+ bool filterAcceptsRow(int sourceRow, const QModelIndex& sourceParent) const override
+ {
+ auto index = sourceModel()->index(sourceRow, 0, sourceParent);
+ auto type = static_cast<interaction::Type>(
+ sourceModel()->data(index, MessageList::Role::Type).toInt());
+ return type != interaction::Type::MERGE && type != interaction::Type::EDITED
+ && type != interaction::Type::REACTION;
+ };
+ bool lessThan(const QModelIndex& left, const QModelIndex& right) const override
+ {
+ return left.row() > right.row();
+ };
+};
class AppSettingsManager;
@@ -50,6 +76,7 @@ Q_SIGNALS:
void newTextPasted();
void previewInformationToQML(QString messageId, QStringList previewInformation);
void moreMessagesLoaded();
+ void timestampUpdated();
protected:
void safeInit() override;
@@ -113,6 +140,8 @@ protected:
void setMessagesFileContent(const QString& path);
void setSendMessageContent(const QString& content);
+ inline MessageListModel* getMsgListSourceModel() const;
+
private Q_SLOTS:
void onNewInteraction(const QString& convUid,
const QString& interactionId,
@@ -132,8 +161,12 @@ private:
AppSettingsManager* settingsManager_;
PreviewEngine* previewEngine_;
+ FilteredMsgListModel* filteredMsgListModel_;
static constexpr const int loadChunkSize_ {20};
std::unique_ptr<MessageListModel> mediaInteractions_;
+
+ QTimer* timestampTimer_ {nullptr};
+ static constexpr const int timestampUpdateIntervalMs_ {1000};
};
diff --git a/src/libclient/messagelistmodel.cpp b/src/libclient/messagelistmodel.cpp
index 4828e48ed..0e64cf44f 100644
--- a/src/libclient/messagelistmodel.cpp
+++ b/src/libclient/messagelistmodel.cpp
@@ -37,11 +37,8 @@ using reverseIterator = MessageListModel::reverseIterator;
MessageListModel::MessageListModel(QObject* parent)
: QAbstractListModel(parent)
- , timestampTimer_(new QTimer(this))
-{
- connect(timestampTimer_, &QTimer::timeout, this, &MessageListModel::timestampUpdated);
- timestampTimer_->start(1000);
-}
+
+{}
QPair<iterator, bool>
MessageListModel::emplace(const QString& msgId, interaction::Info message, bool beginning)
diff --git a/src/libclient/messagelistmodel.h b/src/libclient/messagelistmodel.h
index 2625a6125..452be1b8a 100644
--- a/src/libclient/messagelistmodel.h
+++ b/src/libclient/messagelistmodel.h
@@ -22,7 +22,6 @@
#include "api/interaction.h"
#include <QAbstractListModel>
-#include <QTimer>
namespace lrc {
namespace api {
@@ -134,8 +133,6 @@ public:
void emitDataChanged(const QString& msgId, VectorInt roles = {});
bool isOnlyEmoji(const QString& text) const;
- Q_SIGNAL void timestampUpdated();
-
void addEdition(const QString& msgId, const interaction::Info& info, bool end);
void addReaction(const QString& messageId, const QString& reactionId);
void editMessage(const QString& msgId, interaction::Info& info);
@@ -172,8 +169,6 @@ private:
iterator insertMessage(iterator it, item_t& message);
void removeMessage(int index, iterator it);
void moveMessages(int from, int last, int to);
-
- QTimer* timestampTimer_ {nullptr};
};
} // namespace api
} // namespace lrc
--
GitLab