2
0
Эх сурвалжийг харах

LYN-1767: Search View Crash Fix (#1430)

* Fixed crash in tableView when the filter goes empty and there is a selected entry.

* fixed consts

* Code Review Changes

* more minor fixes
IgnacioMartinezGarrido 4 жил өмнө
parent
commit
e4e22879b8

+ 39 - 36
Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserFilterModel.cpp

@@ -133,61 +133,64 @@ namespace AzToolsFramework
 
 
         void AssetBrowserFilterModel::FilterUpdatedSlotImmediate()
         void AssetBrowserFilterModel::FilterUpdatedSlotImmediate()
         {
         {
-            auto compFilter = qobject_cast<QSharedPointer<const CompositeFilter> >(m_filter);
+            const auto compFilter = qobject_cast<QSharedPointer<const CompositeFilter>>(m_filter);
             if (compFilter)
             if (compFilter)
             {
             {
                 const auto& subFilters = compFilter->GetSubFilters();
                 const auto& subFilters = compFilter->GetSubFilters();
+                const auto& compFilterIter = AZStd::find_if(subFilters.cbegin(), subFilters.cend(),
+                    [subFilters](FilterConstType filter) -> bool
+                    {
+                        const auto assetTypeFilter = qobject_cast<QSharedPointer<const CompositeFilter>>(filter);
+                        return !assetTypeFilter.isNull();
+                    });
 
 
-                const auto compositeFilterIterator = AZStd::find_if(subFilters.cbegin(), subFilters.cend(), [subFilters](FilterConstType filter) -> bool
-                {
-                    const auto assetTypeFilter = qobject_cast<QSharedPointer<const CompositeFilter> >(filter);
-                    return !assetTypeFilter.isNull();
-                });
-
-                if (compositeFilterIterator != subFilters.end())
+                if (compFilterIter != subFilters.end())
                 {
                 {
-                    m_assetTypeFilter = qobject_cast<QSharedPointer<const CompositeFilter> >(*compositeFilterIterator);
+                    m_assetTypeFilter = qobject_cast<QSharedPointer<const CompositeFilter>>(*compFilterIter);
                 }
                 }
 
 
-                const auto compStringFilterIter = AZStd::find_if(subFilters.cbegin(), subFilters.cend(), [](FilterConstType filter) -> bool
-                {
-                    //The real StringFilter is really a CompositeFilter with just one StringFilter in its subfilter list
-                    //To know if it is actually a StringFilter we have to get that subfilter and check if it is a Stringfilter.
-                    const auto stringCompositeFilter = qobject_cast<QSharedPointer<const CompositeFilter> >(filter);
-                    bool isStringFilter = false;
-                    if (stringCompositeFilter)
+                const auto& compositeStringFilterIter = AZStd::find_if(subFilters.cbegin(), subFilters.cend(),
+                    [subFilters](FilterConstType filter) -> bool
                     {
                     {
-                        const auto& stringSubfilters = stringCompositeFilter->GetSubFilters();
-                        auto canBeCasted = [](FilterConstType filt) -> bool
+                        // The real StringFilter is really a CompositeFilter with just one StringFilter in its subfilter list
+                        // To know if it is actually a StringFilter we have to get that subfilter and check if it is a Stringfilter.
+                        const auto& stringCompositeFilter = qobject_cast<QSharedPointer<const CompositeFilter>>(filter);
+                        if (stringCompositeFilter)
                         {
                         {
-                            auto strFilter = qobject_cast<QSharedPointer<const StringFilter>>(filt);
-                            return !strFilter.isNull();
-                        };
-                        const auto stringSubfliterConstIter = AZStd::find_if(stringSubfilters.cbegin(), stringSubfilters.cend(), canBeCasted);
-
-                        //A Composite StringFilter will only have just one subfilter and nothing more.
-                        if (stringSubfliterConstIter != stringSubfilters.end() && stringSubfilters.size() == 1)
-                        {
-                            isStringFilter = true;
+                            //Once we have the main composite filter we can now obtain its subfilters and check if
+                            //it has a StringFilter
+                            const auto& stringSubfilters = stringCompositeFilter->GetSubFilters();
+                            auto canBeCasted = [](FilterConstType filt) -> bool
+                            {
+                                const auto& strFilter = qobject_cast<QSharedPointer<const StringFilter>>(filt);
+                                return !strFilter.isNull();
+                            };
+
+                            const auto& stringSubFilterConstIt  =
+                                AZStd::find_if(stringSubfilters.cbegin(), stringSubfilters.cend(), canBeCasted);
+
+                            // A Composite StringFilter will only have just one subfilter (the StringFilter) and nothing more.
+                            return stringSubFilterConstIt != stringSubfilters.end() && stringSubfilters.size() == 1;
                         }
                         }
-                    }
+                        return false;
+                    });
 
 
-                    return isStringFilter;
-                });
-                if (compStringFilterIter != subFilters.end())
+                if (compositeStringFilterIter != subFilters.end())
                 {
                 {
-                    const auto compStringFilter = qobject_cast<QSharedPointer<const CompositeFilter>>(*compStringFilterIter);
+                    const auto& compStringFilter = qobject_cast<QSharedPointer<const CompositeFilter>>(*compositeStringFilterIter);
 
 
-                    if (!compStringFilter->GetSubFilters().isEmpty() && compStringFilter->GetSubFilters()[0])
+                    if (!compStringFilter->GetSubFilters().isEmpty())
                     {
                     {
-                        m_stringFilter = qobject_cast<QSharedPointer<const StringFilter>>(compStringFilter->GetSubFilters()[0]);
+                        const auto& stringFilter = compStringFilter->GetSubFilters()[0];
+                        AZ_Assert(
+                            stringFilter,
+                            "AssetBrowserFilterModel - String Filter is not a valid Composite Filter");
+                        m_stringFilter = qobject_cast<QSharedPointer<const StringFilter>>(stringFilter);
                     }
                     }
                 }
                 }
-
             }
             }
             invalidateFilter();
             invalidateFilter();
             Q_EMIT filterChanged();
             Q_EMIT filterChanged();
-            emit stringFilterPopulated(!m_stringFilter.isNull());
         }
         }
 
 
         void AssetBrowserFilterModel::filterUpdatedSlot()
         void AssetBrowserFilterModel::filterUpdatedSlot()

+ 0 - 1
Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserFilterModel.h

@@ -52,7 +52,6 @@ namespace AzToolsFramework
             void OnAssetBrowserComponentReady() override;
             void OnAssetBrowserComponentReady() override;
 
 
         Q_SIGNALS:
         Q_SIGNALS:
-            void stringFilterPopulated(bool);
             void filterChanged();
             void filterChanged();
             //////////////////////////////////////////////////////////////////////////
             //////////////////////////////////////////////////////////////////////////
             //QSortFilterProxyModel
             //QSortFilterProxyModel

+ 11 - 0
Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserTableModel.cpp

@@ -69,6 +69,17 @@ namespace AzToolsFramework
             return sourceIndex.data(role);
             return sourceIndex.data(role);
         }
         }
 
 
+        QModelIndex AssetBrowserTableModel::parent([[maybe_unused]] const QModelIndex& child) const
+        {
+            return QModelIndex();
+        }
+
+        QModelIndex AssetBrowserTableModel::sibling(
+            [[maybe_unused]] int row, [[maybe_unused]] int column, [[maybe_unused]] const QModelIndex& idx) const
+        {
+            return QModelIndex();
+        }
+
         QModelIndex AssetBrowserTableModel::index(int row, int column, const QModelIndex& parent) const
         QModelIndex AssetBrowserTableModel::index(int row, int column, const QModelIndex& parent) const
         {
         {
             return parent.isValid() ? QModelIndex() : createIndex(row, column, m_indexMap[row].internalPointer());
             return parent.isValid() ? QModelIndex() : createIndex(row, column, m_indexMap[row].internalPointer());

+ 5 - 3
Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserTableModel.h

@@ -32,15 +32,17 @@ namespace AzToolsFramework
         public:
         public:
             AZ_CLASS_ALLOCATOR(AssetBrowserTableModel, AZ::SystemAllocator, 0);
             AZ_CLASS_ALLOCATOR(AssetBrowserTableModel, AZ::SystemAllocator, 0);
             explicit AssetBrowserTableModel(QObject* parent = nullptr);
             explicit AssetBrowserTableModel(QObject* parent = nullptr);
+
+            void UpdateTableModelMaps();
+
             ////////////////////////////////////////////////////////////////////
             ////////////////////////////////////////////////////////////////////
             // QSortFilterProxyModel
             // QSortFilterProxyModel
             void setSourceModel(QAbstractItemModel* sourceModel) override;
             void setSourceModel(QAbstractItemModel* sourceModel) override;
             QModelIndex mapToSource(const QModelIndex& proxyIndex) const override;
             QModelIndex mapToSource(const QModelIndex& proxyIndex) const override;
             QModelIndex index(int row, int column, const QModelIndex& parent = QModelIndex()) const override;
             QModelIndex index(int row, int column, const QModelIndex& parent = QModelIndex()) const override;
             QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override;
             QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override;
-
-        public Q_SLOTS:
-            void UpdateTableModelMaps();
+            QModelIndex parent(const QModelIndex& child) const override;
+            QModelIndex sibling(int row, int column, const QModelIndex& idx) const override;
 
 
         protected:
         protected:
             int rowCount(const QModelIndex& parent = QModelIndex()) const override;
             int rowCount(const QModelIndex& parent = QModelIndex()) const override;

+ 23 - 25
Code/Sandbox/Editor/AzAssetBrowser/AzAssetBrowserWindow.cpp

@@ -90,8 +90,14 @@ AzAssetBrowserWindow::AzAssetBrowserWindow(QWidget* parent)
         m_tableModel->setSourceModel(m_filterModel.data());
         m_tableModel->setSourceModel(m_filterModel.data());
         m_ui->m_assetBrowserTableViewWidget->setModel(m_tableModel.data());
         m_ui->m_assetBrowserTableViewWidget->setModel(m_tableModel.data());
         connect(
         connect(
-            m_filterModel.data(), &AzAssetBrowser::AssetBrowserFilterModel::filterChanged, m_tableModel.data(),
-            &AzAssetBrowser::AssetBrowserTableModel::UpdateTableModelMaps);
+            m_filterModel.data(), &AzAssetBrowser::AssetBrowserFilterModel::filterChanged, this,
+            [this]()
+            {
+                if (!m_ui->m_searchWidget->GetFilterString().isEmpty())
+                {
+                    m_tableModel->UpdateTableModelMaps();
+                }
+            });
         connect(
         connect(
             m_ui->m_assetBrowserTableViewWidget, &AzAssetBrowser::AssetBrowserTableView::selectionChangedSignal, this,
             m_ui->m_assetBrowserTableViewWidget, &AzAssetBrowser::AssetBrowserTableView::selectionChangedSignal, this,
             &AzAssetBrowserWindow::SelectionChangedSlot);
             &AzAssetBrowserWindow::SelectionChangedSlot);
@@ -107,8 +113,21 @@ AzAssetBrowserWindow::AzAssetBrowserWindow(QWidget* parent)
 
 
         m_ui->m_assetBrowserTableViewWidget->SetName("AssetBrowserTableView_main");
         m_ui->m_assetBrowserTableViewWidget->SetName("AssetBrowserTableView_main");
 
 
-        connect(m_filterModel.data(), &AzAssetBrowser::AssetBrowserFilterModel::stringFilterPopulated, this, &AzAssetBrowserWindow::SwitchDisplayView);
-        connect(m_ui->m_viewSwitcherCheckBox, &QCheckBox::stateChanged, this, &AzAssetBrowserWindow::LockToDefaultView);
+        connect(
+            m_filterModel.data(), &AzAssetBrowser::AssetBrowserFilterModel::filterChanged, this,
+            [this]()
+            {
+                const bool hasFilter = !m_ui->m_searchWidget->GetFilterString().isEmpty();
+                m_ui->m_assetBrowserTableViewWidget->setVisible(hasFilter);
+                m_ui->m_assetBrowserTreeViewWidget->setVisible(!hasFilter);
+            });
+        connect(
+            m_ui->m_viewSwitcherCheckBox, &QCheckBox::stateChanged, this,
+            [this](bool visible)
+            {
+                m_ui->m_assetBrowserTableViewWidget->setVisible(visible);
+                m_ui->m_assetBrowserTreeViewWidget->setVisible(!visible);
+            });
     }
     }
 
 
     m_ui->m_assetBrowserTreeViewWidget->setModel(m_filterModel.data());
     m_ui->m_assetBrowserTreeViewWidget->setModel(m_filterModel.data());
@@ -272,25 +291,4 @@ void AzAssetBrowserWindow::DoubleClickedItem([[maybe_unused]] const QModelIndex&
     }
     }
 }
 }
 
 
-void AzAssetBrowserWindow::SwitchDisplayView(bool state)
-{
-    m_ui->m_assetBrowserTableViewWidget->setVisible(state);
-    m_ui->m_assetBrowserTreeViewWidget->setVisible(!state);
-}
-
-void AzAssetBrowserWindow::LockToDefaultView(bool state)
-{
-    using AzToolsFramework::AssetBrowser::AssetBrowserFilterModel;
-    SwitchDisplayView(!state);
-    if (state == true)
-    {
-        disconnect(
-            m_filterModel.data(), &AssetBrowserFilterModel::stringFilterPopulated, this, &AzAssetBrowserWindow::SwitchDisplayView);
-    }
-    else
-    {
-        connect(m_filterModel.data(), &AssetBrowserFilterModel::stringFilterPopulated, this, &AzAssetBrowserWindow::SwitchDisplayView);
-    }
-}
-
 #include <AzAssetBrowser/moc_AzAssetBrowserWindow.cpp>
 #include <AzAssetBrowser/moc_AzAssetBrowserWindow.cpp>

+ 0 - 2
Code/Sandbox/Editor/AzAssetBrowser/AzAssetBrowserWindow.h

@@ -63,8 +63,6 @@ private:
 private Q_SLOTS:
 private Q_SLOTS:
     void SelectionChangedSlot(const QItemSelection& selected, const QItemSelection& deselected) const;
     void SelectionChangedSlot(const QItemSelection& selected, const QItemSelection& deselected) const;
     void DoubleClickedItem(const QModelIndex& element);
     void DoubleClickedItem(const QModelIndex& element);
-    void SwitchDisplayView(bool state);
-    void LockToDefaultView(bool state);
 };
 };
 
 
 extern const char* AZ_ASSET_BROWSER_PREVIEW_NAME;
 extern const char* AZ_ASSET_BROWSER_PREVIEW_NAME;