From edd64d9b8fc8859d89017ff1c177346952e3d7bf Mon Sep 17 00:00:00 2001 From: Joshua Goins Date: Sat, 31 Jan 2026 13:11:25 -0500 Subject: [PATCH] Improve UserListModel performance and other preparations In a future patch I want to add support for viewing banned/invited users, and it's also been mentioned that UserListModel is quite slow too. The biggest cost is sorting the member list (power level and alphabetically) and this happened in a few different ways: * When the member list updated * The user switches rooms * Misc events such as the palette changing But this was pretty inefficient, because internally Quotient::Room keeps a list of members, and we kept re-sorting that same list. Our connections were also too broad and despite having signals for members joining and leaving we just reloaded the entire list anyway. So my new solution is to keep the list persistently sorted in NeoChatRoom, and reload that in UserListModel. This model also keeps track of *all* members - including ones that left - which will be used for the aforementioned feature. So UserFilterModel now filters out only the joined members, and that will be configurable in the future. I also added two new roles to UserListModel for membership and color respectively (which makes some dead code useful again) and fixed us overwriting the built-in Qt roles accidentally. --- src/libneochat/models/userfiltermodel.cpp | 3 ++ src/libneochat/models/userlistmodel.cpp | 51 +++++++---------------- src/libneochat/models/userlistmodel.h | 5 ++- src/libneochat/neochatroom.cpp | 39 ++++++++++++++++- src/libneochat/neochatroom.h | 10 +++++ src/roominfo/RoomInformation.qml | 2 + 6 files changed, 72 insertions(+), 38 deletions(-) diff --git a/src/libneochat/models/userfiltermodel.cpp b/src/libneochat/models/userfiltermodel.cpp index 825502635..13caa11e6 100644 --- a/src/libneochat/models/userfiltermodel.cpp +++ b/src/libneochat/models/userfiltermodel.cpp @@ -11,6 +11,9 @@ bool UserFilterModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceP if (!m_allowEmpty && m_filterText.length() < 1) { return false; } + if (sourceModel()->data(sourceModel()->index(sourceRow, 0), UserListModel::MembershipRole).value() != Quotient::Membership::Join) { + return false; + } return sourceModel()->data(sourceModel()->index(sourceRow, 0), UserListModel::DisplayNameRole).toString().contains(m_filterText, Qt::CaseInsensitive) || sourceModel()->data(sourceModel()->index(sourceRow, 0), UserListModel::UserIdRole).toString().contains(m_filterText, Qt::CaseInsensitive); } diff --git a/src/libneochat/models/userlistmodel.cpp b/src/libneochat/models/userlistmodel.cpp index 17e17051b..71226ea3c 100644 --- a/src/libneochat/models/userlistmodel.cpp +++ b/src/libneochat/models/userlistmodel.cpp @@ -40,17 +40,12 @@ void UserListModel::setRoom(NeoChatRoom *room) if (m_currentRoom) { connect(m_currentRoom, &Room::memberJoined, this, &UserListModel::memberJoined); - connect(m_currentRoom, &Room::memberLeft, this, &UserListModel::memberLeft); connect(m_currentRoom, &Room::memberNameUpdated, this, [this](RoomMember member) { refreshMember(member, {DisplayNameRole}); }); connect(m_currentRoom, &Room::memberAvatarUpdated, this, [this](RoomMember member) { refreshMember(member, {AvatarRole}); }); - connect(m_currentRoom, &Room::memberListChanged, this, [this]() { - // this is slow - UserListModel::refreshAllMembers(); - }); connect(m_currentRoom->connection(), &Connection::loggedOut, this, [this]() { setRoom(nullptr); }); @@ -79,7 +74,7 @@ QVariant UserListModel::data(const QModelIndex &index, int role) const "users.count()"; return {}; } - auto memberId = m_members.at(index.row()); + const auto &memberId = m_members.at(index.row()); if (role == DisplayNameRole) { return m_currentRoom->member(memberId).disambiguatedName(); } @@ -124,6 +119,12 @@ QVariant UserListModel::data(const QModelIndex &index, int role) const if (role == IsCreatorRole) { return m_currentRoom->isCreator(memberId); } + if (role == MembershipRole) { + return QVariant::fromValue(m_currentRoom->member(memberId).membershipState()); + } + if (role == ColorRole) { + return m_currentRoom->member(memberId).color(); + } return {}; } @@ -139,7 +140,8 @@ int UserListModel::rowCount(const QModelIndex &parent) const bool UserListModel::event(QEvent *event) { if (event->type() == QEvent::ApplicationPaletteChange) { - refreshAllMembers(); + // Quotient::RoomMember::color needs to be recalculated for the new palette + Q_EMIT dataChanged(index(0, 0), index(m_members.size(), 0), {ColorRole}); } return QObject::event(event); } @@ -156,18 +158,6 @@ void UserListModel::memberJoined(const Quotient::RoomMember &member) endInsertRows(); } -void UserListModel::memberLeft(const Quotient::RoomMember &member) -{ - auto pos = findUserPos(member); - if (pos != m_members.size()) { - beginRemoveRows(QModelIndex(), pos, pos); - m_members.removeAt(pos); - endRemoveRows(); - } else { - qWarning() << "Trying to remove a room member not in the user list"; - } -} - void UserListModel::refreshMember(const Quotient::RoomMember &member, const QList &roles) { auto pos = findUserPos(member); @@ -181,21 +171,10 @@ void UserListModel::refreshMember(const Quotient::RoomMember &member, const QLis void UserListModel::refreshAllMembers() { beginResetModel(); - if (m_currentRoom != nullptr) { - m_members = m_currentRoom->joinedMemberIds(); - MemberSorter sorter; - std::sort(m_members.begin(), m_members.end(), [&sorter, this](const auto &left, const auto &right) { - const auto leftPl = m_currentRoom->memberEffectivePowerLevel(left); - const auto rightPl = m_currentRoom->memberEffectivePowerLevel(right); - if (leftPl > rightPl) { - return true; - } else if (rightPl > leftPl) { - return false; - } - - return sorter(m_currentRoom->member(left), m_currentRoom->member(right)); - }); + m_members = m_currentRoom->sortedMemberIds(); + } else { + m_members.clear(); } endResetModel(); Q_EMIT usersRefreshed(); @@ -211,8 +190,8 @@ int UserListModel::findUserPos(const QString &userId) const if (!m_currentRoom) { return 0; } - const auto pos = std::find_if(m_members.cbegin(), m_members.cend(), [&userId](const QString &memberId) { - return userId == memberId; + const auto pos = std::find_if(m_members.cbegin(), m_members.cend(), [&userId](const QString &member) { + return userId == member; }); return pos - m_members.cbegin(); } @@ -228,6 +207,8 @@ QHash UserListModel::roleNames() const roles[PowerLevelRole] = "powerLevel"; roles[PowerLevelStringRole] = "powerLevelString"; roles[IsCreatorRole] = "isCreator"; + roles[MembershipRole] = "membership"; + roles[ColorRole] = "color"; return roles; } diff --git a/src/libneochat/models/userlistmodel.h b/src/libneochat/models/userlistmodel.h index 8f893e2fd..fb0a1b279 100644 --- a/src/libneochat/models/userlistmodel.h +++ b/src/libneochat/models/userlistmodel.h @@ -43,12 +43,14 @@ public: */ enum EventRoles { DisplayNameRole = Qt::DisplayRole, /**< The user's display name in the current room. */ - UserIdRole, /**< Matrix ID of the user. */ + UserIdRole = Qt::UserRole, /**< Matrix ID of the user. */ AvatarRole, /**< The source URL for the user's avatar in the current room. */ ObjectRole, /**< The QObject for the user. */ PowerLevelRole, /**< The user's power level in the current room. */ PowerLevelStringRole, /**< The name of the user's power level in the current room. */ IsCreatorRole, /**< Whether this user is considered a creator of the current room. */ + MembershipRole, /**< The membership state of this user. */ + ColorRole, /**< The color of this user. */ }; Q_ENUM(EventRoles) @@ -89,7 +91,6 @@ protected: private Q_SLOTS: void memberJoined(const Quotient::RoomMember &member); - void memberLeft(const Quotient::RoomMember &member); void refreshMember(const Quotient::RoomMember &member, const QList &roles = {}); void refreshAllMembers(); diff --git a/src/libneochat/neochatroom.cpp b/src/libneochat/neochatroom.cpp index 4c383fe30..fe462a815 100644 --- a/src/libneochat/neochatroom.cpp +++ b/src/libneochat/neochatroom.cpp @@ -172,6 +172,10 @@ NeoChatRoom::NeoChatRoom(Connection *connection, QString roomId, JoinState joinS Q_ASSERT(neochatconnection); connect(neochatconnection, &NeoChatConnection::globalUrlPreviewEnabledChanged, this, &NeoChatRoom::urlPreviewEnabledChanged); connect(this, &Room::fullyReadMarkerMoved, this, &NeoChatRoom::invalidateLastUnreadHighlightId); + + // Wait until the initial member list is available before sorting + connect(this, &Room::memberListChanged, this, &NeoChatRoom::refreshAllMembers, Qt::SingleShotConnection); + connect(this, &Room::memberJoined, this, &NeoChatRoom::insertMemberSorted); } bool NeoChatRoom::visible() const @@ -1910,6 +1914,34 @@ void NeoChatRoom::invalidateLastUnreadHighlightId(const QString &fromEventId, co } } +void NeoChatRoom::refreshAllMembers() +{ + m_sortedMemberIds = memberIds(); + + MemberSorter sorter; + std::ranges::sort(m_sortedMemberIds, [this, &sorter](const auto &left, const auto &right) { + const auto leftPl = memberEffectivePowerLevel(left); + const auto rightPl = memberEffectivePowerLevel(right); + if (leftPl > rightPl) { + return true; + } + if (rightPl > leftPl) { + return false; + } + + return sorter(left, right); + }); +} + +void NeoChatRoom::insertMemberSorted(const Quotient::RoomMember member) +{ + if (m_sortedMemberIds.contains(member.id())) { + return; + } + + m_sortedMemberIds.append(member.id()); +} + bool NeoChatRoom::spaceHasUnreadMessages() const { if (!isSpace()) { @@ -1924,6 +1956,11 @@ void NeoChatRoom::markAllChildrenMessagesAsRead() if (isSpace()) { SpaceHierarchyCache::instance().markAllChildrenMessagesAsRead(id()); } -}; +} + +QList NeoChatRoom::sortedMemberIds() const +{ + return m_sortedMemberIds; +} #include "moc_neochatroom.cpp" diff --git a/src/libneochat/neochatroom.h b/src/libneochat/neochatroom.h index a40371a61..d1654fe12 100644 --- a/src/libneochat/neochatroom.h +++ b/src/libneochat/neochatroom.h @@ -671,6 +671,11 @@ public: */ Q_INVOKABLE void markAllChildrenMessagesAsRead(); + /** + * @return List of members in this room, sorted by power level and then by name. + */ + QList sortedMemberIds() const; + private: bool m_visible = false; @@ -707,6 +712,7 @@ private: void loadPinnedMessage(); QString m_lastUnreadHighlightId; + QList m_sortedMemberIds; private Q_SLOTS: void updatePushNotificationState(QString type); @@ -715,6 +721,10 @@ private Q_SLOTS: void invalidateLastUnreadHighlightId(const QString &fromEventId, const QString &toEventId); + void refreshAllMembers(); + + void insertMemberSorted(Quotient::RoomMember member); + Q_SIGNALS: void cachedInputChanged(); void busyChanged(); diff --git a/src/roominfo/RoomInformation.qml b/src/roominfo/RoomInformation.qml index 97de4f652..47b93088d 100644 --- a/src/roominfo/RoomInformation.qml +++ b/src/roominfo/RoomInformation.qml @@ -283,6 +283,7 @@ QQC2.ScrollView { required property url avatar required property int powerLevel required property string powerLevelString + required property color color implicitHeight: Kirigami.Units.gridUnit * 2 @@ -304,6 +305,7 @@ QQC2.ScrollView { } source: userDelegate.avatar name: userDelegate.userId + color: userDelegate.color Layout.fillHeight: true }