From bfdb9b0f1db5408f93fa85d5e9c69ed9d91dd89b Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Mon, 10 Aug 2026 22:11:14 +0200 Subject: [PATCH] [UserList] Bulk load to prevent hang on connect, fix multi-monitor positioning (#7087) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [UserList] Bulk load to prevent hang on connect, fix multi-monitor positioning Took 48 minutes * Extract slot to method Took 10 minutes --------- Co-authored-by: Lukas Brübach --- .../widgets/server/user/user_info_popup.cpp | 23 ++++-- .../widgets/server/user/user_info_popup.h | 3 + .../widgets/server/user/user_list_widget.cpp | 81 ++++++++++++++++--- .../widgets/server/user/user_list_widget.h | 8 ++ .../interface/widgets/tabs/tab_account.cpp | 9 ++- 5 files changed, 102 insertions(+), 22 deletions(-) diff --git a/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp b/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp index edb95f2df..112f107d4 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp @@ -527,6 +527,21 @@ void UserInfoPopup::onGamesContextMenu(const QPoint &pos) // ── showForUser ─────────────────────────────────────────────────────────────── +void UserInfoPopup::refreshHeader() +{ + if (m_currentUser.isEmpty()) { + return; + } + + const QPixmap avatar = m_avatarCache ? m_avatarCache->value(m_currentUser) : QPixmap{}; + const CardArtParams params = (m_cardArtParamsMap && m_cardArtParamsMap->contains(m_currentUser)) + ? m_cardArtParamsMap->value(m_currentUser) + : CardArtParams{}; + const QString artKey = m_currentUser + u'|' + params.cardName + u'|' + params.cardProviderId; + const QPixmap cardArt = (m_cardArtCache && !params.cardName.isEmpty()) ? m_cardArtCache->value(artKey) : QPixmap{}; + m_header->setUserData(m_currentUserInfo, m_currentOnline, avatar, cardArt, params); +} + void UserInfoPopup::showForUser(const QString &userName, const ServerInfo_User &userInfo, bool online, @@ -538,13 +553,7 @@ void UserInfoPopup::showForUser(const QString &userName, m_currentOnline = online; // Header - const QPixmap avatar = m_avatarCache ? m_avatarCache->value(userName) : QPixmap{}; - const CardArtParams params = (m_cardArtParamsMap && m_cardArtParamsMap->contains(userName)) - ? m_cardArtParamsMap->value(userName) - : CardArtParams{}; - const QString artKey = userName + u'|' + params.cardName + u'|' + params.cardProviderId; - const QPixmap cardArt = (m_cardArtCache && !params.cardName.isEmpty()) ? m_cardArtCache->value(artKey) : QPixmap{}; - m_header->setUserData(userInfo, online, avatar, cardArt, params); + refreshHeader(); // Actions rebuildActionButtons(userInfo, online, isBuddy, isIgnored); diff --git a/cockatrice/src/interface/widgets/server/user/user_info_popup.h b/cockatrice/src/interface/widgets/server/user/user_info_popup.h index 69517093f..c634511e1 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_popup.h +++ b/cockatrice/src/interface/widgets/server/user/user_info_popup.h @@ -116,6 +116,9 @@ public: /** Called when buddy/ignore status changes externally while popup is open. */ void updateActionButtons(const ServerInfo_User &userInfo, bool online, bool isBuddy, bool isIgnored); + /** Re-pulls the avatar/card art for the currently shown user (e.g. after it loads). */ + void refreshHeader(); + signals: void mouseEnteredPopup(); void mouseLeftPopup(); diff --git a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp index 64cbb7b7d..3ad357dd7 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -495,8 +496,6 @@ UserListWidget::UserListWidget(TabSupervisor *_tabSupervisor, m_userInfoPopup->setWindowOpacity(0.0); m_userInfoPopup->installEventFilter(this); - connectPopupSignals(); - m_showPopupTimer = new QTimer(this); m_showPopupTimer->setSingleShot(true); m_showPopupTimer->setInterval(280); @@ -515,6 +514,8 @@ UserListWidget::UserListWidget(TabSupervisor *_tabSupervisor, } }); + connectPopupSignals(); + userTree->setMouseTracking(true); userTree->viewport()->setMouseTracking(true); userTree->viewport()->installEventFilter(this); @@ -543,15 +544,14 @@ UserListWidget::UserListWidget(TabSupervisor *_tabSupervisor, connect(userTree->verticalScrollBar(), &QScrollBar::valueChanged, this, [this] { m_showPopupTimer->stop(); hidePopup(true); + requestAvatarsForVisibleItems(); }); // Forward join requests from popup upward connect(m_userInfoPopup, &UserInfoPopup::joinGameRequested, this, &UserListWidget::joinGameRequested); - connect(avatarProvider, &UserAvatarProvider::avatarUpdated, this, - [this](const QString &) { userTree->viewport()->update(); }); - connect(cardArtProvider, &UserCardArtProvider::cardArtUpdated, this, - [this](const QString &) { userTree->viewport()->update(); }); + connect(avatarProvider, &UserAvatarProvider::avatarUpdated, this, &UserListWidget::refreshVisibleUserHeader); + connect(cardArtProvider, &UserCardArtProvider::cardArtUpdated, this, &UserListWidget::refreshVisibleUserHeader); connect(&SettingsCache::instance().appearance(), &AppearanceSettings::styleUserListChanged, this, &UserListWidget::applyDisplayMode); @@ -633,6 +633,14 @@ void UserListWidget::bind(UserListManager *mgr) rebuild(); } +void UserListWidget::refreshVisibleUserHeader(const QString &name) +{ + userTree->viewport()->update(); + if (m_userInfoPopup->isVisible() && m_userInfoPopup->currentUser() == name) { + m_userInfoPopup->refreshHeader(); + } +} + void UserListWidget::refreshPopupButtons(const QString &userName) { UserListTWI *item = users.value(userName); @@ -657,6 +665,12 @@ void UserListWidget::hideEvent(QHideEvent *e) hidePopup(true); } +void UserListWidget::showEvent(QShowEvent *e) +{ + QGroupBox::showEvent(e); + requestAvatarsForVisibleItems(); +} + void UserListWidget::applyDisplayMode() { const bool styled = SettingsCache::instance().appearance().getStyleUserList(); @@ -758,6 +772,8 @@ void UserListWidget::showPopupForUser(const QString &userName) return; } + avatarProvider->requestAvatar(userName); // ensure the hovered user's avatar is fetched promptly + const ServerInfo_User &info = item->getUserInfo(); const bool online = item->data(0, UserListRoles::Online).toBool(); const bool isBuddy = userContextMenu->getUserListProxy()->isUserBuddy(userName); @@ -801,7 +817,12 @@ void UserListWidget::positionPopup(const QString &userName) const int popH = m_userInfoPopup->height(); const int margin = 12; - const QRect screen = QGuiApplication::primaryScreen()->availableGeometry(); + QScreen *activeScreen = QGuiApplication::screenAt(itemTL); + if (!activeScreen) { + activeScreen = window()->screen(); + } + const QRect screen = + activeScreen ? activeScreen->availableGeometry() : QGuiApplication::primaryScreen()->availableGeometry(); // ── X: prefer the side with more space ─────────────────────────────────── const int spaceLeft = vpTL.x() - screen.left() - margin; @@ -875,6 +896,38 @@ void UserListWidget::retranslateUi() updateCount(); } +void UserListWidget::beginBulkLoad() +{ + m_bulkLoading = true; +} + +void UserListWidget::endBulkLoad() +{ + m_bulkLoading = false; + sortItems(); + requestAvatarsForVisibleItems(); + userTree->viewport()->update(); +} + +bool UserListWidget::isItemNearViewport(const UserListTWI *item) const +{ + // Prefetch a full viewport of rows above and below so scrolling never shows + // an unloaded row. + const QRect nearView = + userTree->viewport()->rect().adjusted(0, -userTree->viewport()->height(), 0, userTree->viewport()->height()); + return userTree->visualItemRect(item).intersects(nearView); +} + +void UserListWidget::requestAvatarsForVisibleItems() +{ + for (int i = 0; i < userTree->topLevelItemCount(); ++i) { + auto *twi = static_cast(userTree->topLevelItem(i)); + if (isItemNearViewport(twi)) { + avatarProvider->requestAvatar(QString::fromStdString(twi->getUserInfo().name())); + } + } +} + void UserListWidget::rebuild() { userTree->clear(); @@ -901,11 +954,11 @@ void UserListWidget::rebuild() break; } + beginBulkLoad(); for (auto it = source->cbegin(); it != source->cend(); ++it) { processUserInfo(it.value(), manager->getOnlineUser(it.key()) != nullptr); } - - sortItems(); + endBulkLoad(); } void UserListWidget::processUserInfo(const ServerInfo_User &user, bool online) @@ -940,11 +993,15 @@ void UserListWidget::processUserInfo(const ServerInfo_User &user, bool online) ++onlineCount; } updateCount(); - avatarProvider->requestAvatar(userName); + if (!m_bulkLoading && isItemNearViewport(item)) { + avatarProvider->requestAvatar(userName); + } } item->setOnline(online); - sortItems(); - userTree->viewport()->update(); + if (!m_bulkLoading) { + sortItems(); + userTree->viewport()->update(); + } } bool UserListWidget::deleteUser(const QString &userName) diff --git a/cockatrice/src/interface/widgets/server/user/user_list_widget.h b/cockatrice/src/interface/widgets/server/user/user_list_widget.h index d70cdfbbd..c98ebebdf 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_widget.h +++ b/cockatrice/src/interface/widgets/server/user/user_list_widget.h @@ -36,6 +36,7 @@ class QPlainTextEdit; class Response; class CommandContainer; class UserContextMenu; +class QShowEvent; class BanDialog : public QDialog { @@ -158,11 +159,14 @@ private: QTimer *m_hidePopupTimer = nullptr; QString m_hoveredUser; bool m_popupPinned = false; + bool m_bulkLoading = false; void showPopupForUser(const QString &userName); void hidePopup(bool immediate = false); void positionPopup(const QString &userName); void connectPopupSignals(); + bool isItemNearViewport(const UserListTWI *item) const; + void requestAvatarsForVisibleItems(); QMap users; TabSupervisor *tabSupervisor; @@ -177,6 +181,7 @@ private: void refreshPopupButtons(const QString &userName); private slots: void userClicked(QTreeWidgetItem *item, int column); + void refreshVisibleUserHeader(const QString &name); signals: void openMessageDialog(const QString &userName, bool focus); void addBuddy(const QString &userName); @@ -192,6 +197,8 @@ public: QWidget *parent = nullptr); void bind(UserListManager *mgr); void applyDisplayMode(); + void beginBulkLoad(); + void endBulkLoad(); bool eventFilter(QObject *obj, QEvent *event) override; void retranslateUi(); void rebuild(); @@ -207,6 +214,7 @@ public: protected: void hideEvent(QHideEvent *e) override; + void showEvent(QShowEvent *e) override; }; #endif diff --git a/cockatrice/src/interface/widgets/tabs/tab_account.cpp b/cockatrice/src/interface/widgets/tabs/tab_account.cpp index 2cc8165e8..2c30178f3 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_account.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_account.cpp @@ -135,6 +135,7 @@ void TabAccount::retranslateUi() void TabAccount::processListUsersResponse(const Response &response) { const Response_ListUsers &resp = response.GetExtension(Response_ListUsers::ext); + allUsersList->beginBulkLoad(); for (int i = 0; i < resp.user_list_size(); ++i) { const ServerInfo_User &info = resp.user_list(i); const QString &userName = QString::fromStdString(info.name()); @@ -142,8 +143,8 @@ void TabAccount::processListUsersResponse(const Response &response) ignoreList->setUserOnline(userName, true); buddyList->setUserOnline(userName, true); } + allUsersList->endBulkLoad(); - allUsersList->sortItems(); ignoreList->sortItems(); buddyList->sortItems(); } @@ -188,18 +189,20 @@ void TabAccount::processUserLeftEvent(const Event_UserLeft &event) void TabAccount::buddyListReceived(const QList &_buddyList) { + buddyList->beginBulkLoad(); for (const auto &user : _buddyList) { buddyList->processUserInfo(user, false); } - buddyList->sortItems(); + buddyList->endBulkLoad(); } void TabAccount::ignoreListReceived(const QList &_ignoreList) { + ignoreList->beginBulkLoad(); for (const auto &user : _ignoreList) { ignoreList->processUserInfo(user, false); } - ignoreList->sortItems(); + ignoreList->endBulkLoad(); } void TabAccount::processAddToListEvent(const Event_AddToList &event)