Asset Browser Search View fixes (#4814)

* Fixed RowCount method.

Signed-off-by: igarri <igarri@amazon.com>

* Fixed Signals

Signed-off-by: igarri <igarri@amazon.com>

* Fixed Delegate case

Signed-off-by: igarri <igarri@amazon.com>

* Fixed issue when displaying branch icons

Signed-off-by: igarri <igarri@amazon.com>

* Fixed AssetBrowser Delegate

Signed-off-by: igarri <igarri@amazon.com>

* Removed optimize flags

Signed-off-by: igarri <igarri@amazon.com>

* AssetBrowsertableModel cleanup

Signed-off-by: igarri <igarri@amazon.com>

* Fixed Typos

Signed-off-by: igarri <igarri@amazon.com>

* Fixed Comment

Signed-off-by: igarri <igarri@amazon.com>

* Added check for rowCount == 0

Signed-off-by: igarri <igarri@amazon.com>

Code/Editor/AzAssetBrowser/AzAssetBrowserWindow.cpp

@@ -96,10 +96,6 @@ AzAssetBrowserWindow::AzAssetBrowserWindow(QWidget* parent)
         connect(
             m_filterModel.data(), &AzAssetBrowser::AssetBrowserFilterModel::filterChanged, this,
             &AzAssetBrowserWindow::SetTableViewVisibleAfterFilter);
-
-        connect(
-            m_filterModel.data(), &AzAssetBrowser::AssetBrowserFilterModel::filterChanged, this,
-            &AzAssetBrowserWindow::UpdateTableModelAfterFilter);
         connect(
             m_ui->m_assetBrowserTableViewWidget, &AzAssetBrowser::AssetBrowserTableView::selectionChangedSignal, this,
             &AzAssetBrowserWindow::SelectionChangedSlot);
@@ -251,24 +247,6 @@ void AzAssetBrowserWindow::SetExpandedAssetBrowserMode()
 
     m_assetBrowserDisplayState = AzAssetBrowser::AssetBrowserDisplayState::ExpandedMode;
 
-    disconnect(
-        m_filterModel.data(), &AzAssetBrowser::AssetBrowserFilterModel::filterChanged, this,
-        &AzAssetBrowserWindow::UpdateTableModelAfterFilter);
-    disconnect(
-        m_filterModel.data(), &AzAssetBrowser::AssetBrowserFilterModel::filterChanged, this,
-        &AzAssetBrowserWindow::SetTableViewVisibleAfterFilter);
-
-    disconnect(
-        m_ui->m_assetBrowserTableViewWidget, &AzAssetBrowser::AssetBrowserTableView::selectionChangedSignal, this,
-        &AzAssetBrowserWindow::SelectionChangedSlot);
-    disconnect(m_ui->m_assetBrowserTableViewWidget, &QAbstractItemView::doubleClicked, this, &AzAssetBrowserWindow::DoubleClickedItem);
-    disconnect(
-        m_ui->m_assetBrowserTableViewWidget, &AzAssetBrowser::AssetBrowserTableView::ClearStringFilter, m_ui->m_searchWidget,
-        &AzAssetBrowser::SearchWidget::ClearStringFilter);
-    disconnect(
-        m_ui->m_assetBrowserTableViewWidget, &AzAssetBrowser::AssetBrowserTableView::ClearTypeFilter, m_ui->m_searchWidget,
-        &AzAssetBrowser::SearchWidget::ClearTypeFilter);
-
     if (m_ui->m_assetBrowserTableViewWidget->isVisible())
     {
         m_ui->m_assetBrowserTableViewWidget->setVisible(false);
@@ -281,37 +259,9 @@ void AzAssetBrowserWindow::SetDefaultAssetBrowserMode()
     namespace AzAssetBrowser = AzToolsFramework::AssetBrowser;
 
     m_assetBrowserDisplayState = AzAssetBrowser::AssetBrowserDisplayState::DefaultMode;
-
-    connect(
-        m_filterModel.data(), &AzAssetBrowser::AssetBrowserFilterModel::filterChanged, this,
-        &AzAssetBrowserWindow::SetTableViewVisibleAfterFilter);
-
-    connect(
-        m_filterModel.data(), &AzAssetBrowser::AssetBrowserFilterModel::filterChanged, this,
-        &AzAssetBrowserWindow::UpdateTableModelAfterFilter);
-    connect(
-        m_ui->m_assetBrowserTableViewWidget, &AzAssetBrowser::AssetBrowserTableView::selectionChangedSignal, this,
-        &AzAssetBrowserWindow::SelectionChangedSlot);
-    connect(m_ui->m_assetBrowserTableViewWidget, &QAbstractItemView::doubleClicked, this, &AzAssetBrowserWindow::DoubleClickedItem);
-    connect(
-        m_ui->m_assetBrowserTableViewWidget, &AzAssetBrowser::AssetBrowserTableView::ClearStringFilter, m_ui->m_searchWidget,
-        &AzAssetBrowser::SearchWidget::ClearStringFilter);
-    connect(
-        m_ui->m_assetBrowserTableViewWidget, &AzAssetBrowser::AssetBrowserTableView::ClearTypeFilter, m_ui->m_searchWidget,
-        &AzAssetBrowser::SearchWidget::ClearTypeFilter);
-
-    //If the filter is not empty we want to switch views and Update the model
-    UpdateTableModelAfterFilter();
     SetTableViewVisibleAfterFilter();
 }
 
-void AzAssetBrowserWindow::UpdateTableModelAfterFilter()
-{
-    if (!m_ui->m_searchWidget->GetFilterString().isEmpty())
-    {
-        m_tableModel->UpdateTableModelMaps();
-    }
-}
 
 void AzAssetBrowserWindow::SetTableViewVisibleAfterFilter()
 {
@@ -389,8 +339,8 @@ void AzAssetBrowserWindow::SelectionChangedSlot(const QItemSelection& /*selected
     UpdatePreview();
 }
 
-// while its tempting to use Activated here, we dont actually want it to count as activation
+// while its tempting to use Activated here, we don't actually want it to count as activation
-// just becuase on some OS clicking once is activation.
+// just because on some OS clicking once is activation.
 void AzAssetBrowserWindow::DoubleClickedItem([[maybe_unused]] const QModelIndex& element)
 {
     namespace AzAssetBrowser = AzToolsFramework::AssetBrowser;

Code/Editor/AzAssetBrowser/AzAssetBrowserWindow.h

@@ -68,7 +68,6 @@ protected slots:
     void CreateSwitchViewMenu();
     void SetExpandedAssetBrowserMode();
     void SetDefaultAssetBrowserMode();
-    void UpdateTableModelAfterFilter();
     void SetTableViewVisibleAfterFilter();
 
 private:

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserTableModel.cpp

@@ -24,24 +24,13 @@ namespace AzToolsFramework
             AZ_Assert(
                 m_filterModel,
                 "Error in AssetBrowserTableModel initialization, class expects source model to be an AssetBrowserFilterModel.");
-            connect(sourceModel, &QAbstractItemModel::rowsInserted, this, &AssetBrowserTableModel::UpdateTableModelMaps);
-            connect(sourceModel, &QAbstractItemModel::rowsRemoved, this, &AssetBrowserTableModel::UpdateTableModelMaps);
-            connect(sourceModel, &QAbstractItemModel::modelAboutToBeReset, this, &AssetBrowserTableModel::beginResetModel);
-            connect(
-                sourceModel, &QAbstractItemModel::modelReset, this,
-                [this]()
-                {
-                    {
-                        QSignalBlocker sb(this);
-                        UpdateTableModelMaps();
-                    }
-                    endResetModel();
-                });
-            connect(sourceModel, &QAbstractItemModel::layoutChanged, this, &AssetBrowserTableModel::UpdateTableModelMaps);
-            connect(sourceModel, &QAbstractItemModel::dataChanged, this, &AssetBrowserTableModel::SourceDataChanged);
-
-
             QSortFilterProxyModel::setSourceModel(sourceModel);
+
+            connect(m_filterModel, &QAbstractItemModel::rowsInserted, this, &AssetBrowserTableModel::UpdateTableModelMaps);
+            connect(m_filterModel, &QAbstractItemModel::rowsRemoved, this, &AssetBrowserTableModel::UpdateTableModelMaps);
+            connect(m_filterModel, &QAbstractItemModel::layoutChanged, this, &AssetBrowserTableModel::UpdateTableModelMaps);
+            connect(m_filterModel, &AssetBrowserFilterModel::filterChanged, this, &AssetBrowserTableModel::beginResetModel);
+            connect(m_filterModel, &QAbstractItemModel::dataChanged, this, &AssetBrowserTableModel::SourceDataChanged);
         }
 
         QModelIndex AssetBrowserTableModel::mapToSource(const QModelIndex& proxyIndex) const
@@ -112,7 +101,7 @@ namespace AzToolsFramework
 
         int AssetBrowserTableModel::rowCount(const QModelIndex& parent) const
         {
-            return !parent.isValid() ? m_indexMap.size() : sourceModel()->rowCount(parent);
+            return !parent.isValid() ? m_indexMap.size() : 0;
         }
 
         int AssetBrowserTableModel::BuildTableModelMap(
@@ -162,28 +151,29 @@ namespace AzToolsFramework
 
         AssetBrowserEntry* AssetBrowserTableModel::GetAssetEntry(QModelIndex index) const
         {
-            if (index.isValid())
+            if (!index.isValid())
-            {
-                return static_cast<AssetBrowserEntry*>(index.internalPointer());
-            }
-            else
             {
                 AZ_Error("AssetBrowser", false, "Invalid Source Index provided to GetAssetEntry.");
                 return nullptr;
             }
+            return static_cast<AssetBrowserEntry*>(index.internalPointer());
         }
 
         void AssetBrowserTableModel::UpdateTableModelMaps()
         {
+            beginResetModel();
             emit layoutAboutToBeChanged();
-            m_indexMap.clear();
-            m_rowMap.clear();
 
+            if (!m_indexMap.isEmpty() || !m_rowMap.isEmpty())
+            {
+                m_indexMap.clear();
+                m_rowMap.clear();
+            }
             AzToolsFramework::EditorSettingsAPIBus::BroadcastResult(
                 m_numberOfItemsDisplayed, &AzToolsFramework::EditorSettingsAPIBus::Handler::GetMaxNumberOfItemsShownInSearchView);
-
             BuildTableModelMap(sourceModel());
             emit layoutChanged();
+            endResetModel();
         }
     } // namespace AssetBrowser
 } // namespace AzToolsFramework

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserTableModel.h

@@ -21,7 +21,9 @@ namespace AzToolsFramework
         class AssetBrowserFilterModel;
         class AssetBrowserEntry;
 
-        class AssetBrowserTableModel : public QSortFilterProxyModel
+        class AssetBrowserTableModel
+            : public QSortFilterProxyModel
+            , public AssetBrowserComponentNotificationBus::Handler
         {
             Q_OBJECT
 
@@ -43,7 +45,7 @@ namespace AzToolsFramework
             int rowCount(const QModelIndex& parent = QModelIndex()) const override;
             QVariant headerData(int section, Qt::Orientation orientation, int role /* = Qt::DisplayRole */) const override;
             ////////////////////////////////////////////////////////////////////
-        private:
+
             AssetBrowserEntry* GetAssetEntry(QModelIndex index) const;
             int BuildTableModelMap(const QAbstractItemModel* model, const QModelIndex& parent = QModelIndex(), int row = 0);
 

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Views/EntryDelegate.cpp

@@ -225,29 +225,36 @@ namespace AzToolsFramework
                         const QModelIndex indexBelow = viewModel->index(index.row() + 1, index.column());
                         const QModelIndex indexAbove = viewModel->index(index.row() - 1, index.column());
 
-                        auto aboveEntry = qvariant_cast<const AssetBrowserEntry*>(indexBelow.data(AssetBrowserModel::Roles::EntryRole));
+                        auto belowEntry = qvariant_cast<const AssetBrowserEntry*>(indexBelow.data(AssetBrowserModel::Roles::EntryRole));
-                        auto belowEntry = qvariant_cast<const AssetBrowserEntry*>(indexAbove.data(AssetBrowserModel::Roles::EntryRole));
+                        auto aboveEntry = qvariant_cast<const AssetBrowserEntry*>(indexAbove.data(AssetBrowserModel::Roles::EntryRole));
 
-                        auto aboveSourceEntry = azrtti_cast<const SourceAssetBrowserEntry*>(aboveEntry);
                         auto belowSourceEntry = azrtti_cast<const SourceAssetBrowserEntry*>(belowEntry);
+                        auto aboveSourceEntry = azrtti_cast<const SourceAssetBrowserEntry*>(aboveEntry);
 
-                        // if current index is the last entry in the view
+                        // Last item and the above entry is a source entry
-                        // or the index above it is a Source Entry and
+                        // or indexBelow is a source entry and the index above is not
-                        // the index below is invalid or is valid but it is also a source entry
+                        if (viewModel->rowCount() > 0 && index.row() == viewModel->rowCount() - 1)
-                        // then the current index is the only child.
+                        {
-                        if (index.row() == viewModel->rowCount() - 1 ||
+                            if (aboveSourceEntry)
-                            (indexBelow.isValid() && aboveSourceEntry &&
+                            {
-                             (!indexAbove.isValid() || (indexAbove.isValid() && belowSourceEntry))))
+                                DrawBranchPixMap(EntryBranchType::OneChild, painter, branchIconTopLeft, iconSize);
+                            }
+                            else
+                            {
+                                DrawBranchPixMap(EntryBranchType::Last, painter, branchIconTopLeft, iconSize);
+                            }
+                        }
+                        else if (belowSourceEntry && aboveSourceEntry)
                         {
                             DrawBranchPixMap(EntryBranchType::OneChild, painter, branchIconTopLeft, iconSize); // Draw One Child Icon
                         }
-                        else if (indexBelow.isValid() && aboveSourceEntry) // The index above is a source entry
+                        else if (belowSourceEntry && !aboveSourceEntry)
                         {
-                            DrawBranchPixMap(EntryBranchType::Last, painter, branchIconTopLeft, iconSize); // Draw First child Icon
+                            DrawBranchPixMap(EntryBranchType::Last, painter, branchIconTopLeft, iconSize);
                         }
-                        else if (indexAbove.isValid() && belowSourceEntry) // The index below is a source entry
+                        else if (aboveSourceEntry) // The index above is a source entry
                         {
-                            DrawBranchPixMap(EntryBranchType::First, painter, branchIconTopLeft, iconSize); // Draw Last Child Icon
+                            DrawBranchPixMap(EntryBranchType::First, painter, branchIconTopLeft, iconSize); // Draw First Child Icon
                         }
                         else //the index above and below are also child entries
                         {
@@ -286,7 +293,6 @@ namespace AzToolsFramework
                     absoluteIconPath = AZ::IO::FixedMaxPath(AZ::Utils::GetEnginePath()) / TreeIconPathLast;
                     break;
                 case AzToolsFramework::AssetBrowser::EntryBranchType::OneChild:
-                default:
                     absoluteIconPath = AZ::IO::FixedMaxPath(AZ::Utils::GetEnginePath()) / TreeIconPathOneChild;
                     break;
                 }
@@ -311,5 +317,4 @@ namespace AzToolsFramework
 
     } // namespace AssetBrowser
 } // namespace AzToolsFramework
-
 #include "AssetBrowser/Views/moc_EntryDelegate.cpp"