From ecbdd32a2d50267b4fe1efb9c6c307221ea9cb37 Mon Sep 17 00:00:00 2001 From: RickyRister <42636155+RickyRister@users.noreply.github.com> Date: Tue, 4 Mar 2025 16:56:31 -0800 Subject: [PATCH] Reduce redundant recursion in VDS (#5664) * remove recursion from flattenFolderStructure findChildren is already recursive by default * only trigger the top-level updateVisibility on filter update Every folder widget was connecting the filter update signals to their updateVisibility, but updateVisibility is already recursive. That means a bunch of redundant updateVisibility calls happen every time a filter update signal is emitted * reduce redundant recursion in updateVisibility findChildren is recursive by default, so only the top-level updateVisibility needs to loop through the found children * delete now-unused signals --- ...ual_deck_storage_folder_display_widget.cpp | 47 ++++++------------- ...isual_deck_storage_folder_display_widget.h | 2 +- .../visual_deck_storage_widget.cpp | 6 +-- .../visual_deck_storage_widget.h | 3 -- 4 files changed, 18 insertions(+), 40 deletions(-) diff --git a/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_folder_display_widget.cpp b/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_folder_display_widget.cpp index 8d68b2a97..b198d06bd 100644 --- a/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_folder_display_widget.cpp +++ b/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_folder_display_widget.cpp @@ -33,13 +33,6 @@ VisualDeckStorageFolderDisplayWidget::VisualDeckStorageFolderDisplayWidget( flowWidget = new FlowWidget(this, Qt::Horizontal, Qt::ScrollBarAlwaysOff, Qt::ScrollBarAlwaysOff); containerLayout->addWidget(flowWidget); - connect(visualDeckStorageWidget, &VisualDeckStorageWidget::tagFilterUpdated, this, - &VisualDeckStorageFolderDisplayWidget::updateVisibility); - connect(visualDeckStorageWidget, &VisualDeckStorageWidget::colorFilterUpdated, this, - &VisualDeckStorageFolderDisplayWidget::updateVisibility); - connect(visualDeckStorageWidget, &VisualDeckStorageWidget::searchFilterUpdated, this, - &VisualDeckStorageFolderDisplayWidget::updateVisibility); - createWidgetsForFiles(); createWidgetsForFolders(); @@ -104,7 +97,12 @@ void VisualDeckStorageFolderDisplayWidget::createWidgetsForFiles() } } -void VisualDeckStorageFolderDisplayWidget::updateVisibility() +/** + * Updates the visibility of this folder and all its DeckPreviewWidgets + * + * @param recursive Also update the visibility of all subfolders and their DeckPreviewWidgets. + */ +void VisualDeckStorageFolderDisplayWidget::updateVisibility(bool recursive) { bool atLeastOneWidgetVisible = checkVisibility(); if (atLeastOneWidgetVisible) { @@ -112,8 +110,10 @@ void VisualDeckStorageFolderDisplayWidget::updateVisibility() for (DeckPreviewWidget *display : flowWidget->findChildren()) { display->updateVisibility(); } - for (VisualDeckStorageFolderDisplayWidget *subFolder : findChildren()) { - subFolder->updateVisibility(); + if (recursive) { + for (auto *subFolder : findChildren()) { + subFolder->updateVisibility(false); + } } } else { setVisible(false); @@ -181,32 +181,13 @@ void VisualDeckStorageFolderDisplayWidget::updateShowFolders(bool enabled) } /** - * Recursively gets all DeckPreviewWidgets in this folder and its subfolders - */ -static QList getAllDecksRecursive(VisualDeckStorageFolderDisplayWidget *folder) -{ - QList allDecks; - - if (auto *flowWidget = folder->getFlowWidget()) { - // Iterate through all DeckPreviewWidgets in this folder - allDecks << flowWidget->findChildren(); - } - - for (auto *subFolder : folder->findChildren()) { - allDecks << getAllDecksRecursive(subFolder); - } - - return allDecks; -} - -/** - * Recursively steals all DeckPreviewWidgets from this widget's subfolders, and deletes all subfolders + * Steals all DeckPreviewWidgets from this widget's nested subfolders, and deletes those subfolders */ void VisualDeckStorageFolderDisplayWidget::flattenFolderStructure() { - for (VisualDeckStorageFolderDisplayWidget *subFolder : findChildren()) { - // steal all DeckPreviewWidgets from the subfolder and all its subfolders - for (auto *deck : getAllDecksRecursive(subFolder)) { + for (auto *subFolder : findChildren()) { + // steal all DeckPreviewWidgets from the subfolder + for (auto *deck : subFolder->getFlowWidget()->findChildren()) { flowWidget->addWidget(deck); } diff --git a/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_folder_display_widget.h b/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_folder_display_widget.h index d149e4d27..747bcf8bd 100644 --- a/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_folder_display_widget.h +++ b/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_folder_display_widget.h @@ -28,7 +28,7 @@ public: }; public slots: - void updateVisibility(); + void updateVisibility(bool recursive = true); bool checkVisibility(); void updateShowFolders(bool enabled); diff --git a/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_widget.cpp b/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_widget.cpp index 789061999..76967c784 100644 --- a/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_widget.cpp +++ b/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_widget.cpp @@ -198,16 +198,16 @@ void VisualDeckStorageWidget::updateTagFilter() { if (folderWidget) { tagFilterWidget->filterDecksBySelectedTags(folderWidget->findChildren()); + folderWidget->updateVisibility(); } - emit tagFilterUpdated(); } void VisualDeckStorageWidget::updateColorFilter() { if (folderWidget) { deckPreviewColorIdentityFilterWidget->filterWidgets(folderWidget->findChildren()); + folderWidget->updateVisibility(); } - emit colorFilterUpdated(); } void VisualDeckStorageWidget::updateSearchFilter() @@ -215,8 +215,8 @@ void VisualDeckStorageWidget::updateSearchFilter() if (folderWidget) { searchWidget->filterWidgets(folderWidget->findChildren(), searchWidget->getSearchText(), searchFolderNamesCheckBox->isChecked()); + folderWidget->updateVisibility(); } - emit searchFilterUpdated(); } void VisualDeckStorageWidget::updateTagsVisibility(const bool visible) diff --git a/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_widget.h b/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_widget.h index 6faef67d7..2a09f6736 100644 --- a/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_widget.h +++ b/cockatrice/src/client/ui/widgets/visual_deck_storage/visual_deck_storage_widget.h @@ -46,9 +46,6 @@ signals: void bannerCardsRefreshed(); void deckLoadRequested(const QString &filePath); void openDeckEditor(const DeckLoader *deck); - void tagFilterUpdated(); - void colorFilterUpdated(); - void searchFilterUpdated(); private: QVBoxLayout *layout;