[UserList] Bulk load to prevent hang on connect, fix multi-monitor positioning (#7087)

* [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 <Bruebach.Lukas@bdosecurity.de>

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);

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();

cockatrice/src/interface/widgets/server/user/user_list_widget.cpp

@@ -23,6 +23,7 @@
 #include <QPlainTextEdit>
 #include <QPushButton>
 #include <QRadioButton>
+#include <QScreen>
 #include <QSpinBox>
 #include <QWidget>
 #include <libcockatrice/card/database/card_database_manager.h>
@@ -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<UserListTWI *>(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)

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<QString, UserListTWI *> 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

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<ServerInfo_User> &_buddyList)
 {
+    buddyList->beginBulkLoad();
     for (const auto &user : _buddyList) {
         buddyList->processUserInfo(user, false);
     }
-    buddyList->sortItems();
+    buddyList->endBulkLoad();
 }
 
 void TabAccount::ignoreListReceived(const QList<ServerInfo_User> &_ignoreList)
 {
+    ignoreList->beginBulkLoad();
     for (const auto &user : _ignoreList) {
         ignoreList->processUserInfo(user, false);
     }
-    ignoreList->sortItems();
+    ignoreList->endBulkLoad();
 }
 
 void TabAccount::processAddToListEvent(const Event_AddToList &event)