Refactor the Outliner drag&drop and selection to fix UX and technical issues. (#7559)

* Prototype - refactor the selection system for the Outliner (WIP)

Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>

* WIP - unify custom drag, still trying to find a way to make it so that the selection isn't affected by drag/drop of unselected entity

Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>

* Wrap up changes

Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>

* Minor refactor - move from enum to bool

Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>

* Prevent clicks on the spacing column from selecting the entity; better refresh the hover state.

Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>

* Fix weird flicker introduced by latest commit

Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>

* Change color for the drag select rectangle.

Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>

* Add more thorough comments, and also make some helper variables const.

Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>

* Fixed incorrect variable types (Qt uses ints for on-screen dimensions)

Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>

Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.cpp

@@ -2091,8 +2091,14 @@ namespace AzToolsFramework
             customOption.state ^= QStyle::State_HasFocus;
         }
 
+        // Don't allow to paint on the spacing column
+        if (index.column() == EntityOutlinerListModel::ColumnSpacing)
+        {
+            return;
+        }
+
         // Retrieve the Entity UI Handler
-        auto firstColumnIndex = index.siblingAtColumn(0);
+        auto firstColumnIndex = index.siblingAtColumn(EntityOutlinerListModel::ColumnName);
         AZ::EntityId entityId(firstColumnIndex.data(EntityOutlinerListModel::EntityIdRole).value<AZ::u64>());
         auto entityUiHandler = m_editorEntityFrameworkInterface->GetHandler(entityId);
 

Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.hxx

@@ -69,6 +69,7 @@ namespace AzToolsFramework
             ColumnName,                 //!< Entity name
             ColumnVisibilityToggle,     //!< Visibility Icons
             ColumnLockToggle,           //!< Lock Icons
+            ColumnSpacing,              //!< Spacing to allow for drag select
             ColumnSortIndex,            //!< Index of sort order
             ColumnCount                 //!< Total number of columns
         };

Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerTreeView.cpp

@@ -30,7 +30,6 @@ namespace AzToolsFramework
     EntityOutlinerTreeView::EntityOutlinerTreeView(QWidget* pParent)
         : AzQtComponents::StyledTreeView(pParent)
         , m_queuedMouseEvent(nullptr)
-        , m_draggingUnselectedItem(false)
     {
         setUniformRowHeights(true);
         setHeaderHidden(true);
@@ -65,22 +64,6 @@ namespace AzToolsFramework
         m_expandOnlyDelay = delay;
     }
 
-    void EntityOutlinerTreeView::ClearQueuedMouseEvent()
-    {
-        if (m_queuedMouseEvent)
-        {
-            delete m_queuedMouseEvent;
-            m_queuedMouseEvent = nullptr;
-        }
-    }
-
-    void EntityOutlinerTreeView::leaveEvent([[maybe_unused]] QEvent* event)
-    {
-        m_mousePosition = QPoint(-1, -1);
-        m_currentHoveredIndex = QModelIndex();
-        update();
-    }
-
     void EntityOutlinerTreeView::dataChanged(const QModelIndex& topLeft, const QModelIndex& bottomRight, const QVector<int>& roles)
     {
         AzQtComponents::StyledTreeView::dataChanged(topLeft, bottomRight, roles);
@@ -93,7 +76,7 @@ namespace AzToolsFramework
                 auto modelRow = topLeft.sibling(i, EntityOutlinerListModel::ColumnName);
                 if (modelRow.isValid())
                 {
-                    checkExpandedState(modelRow);
+                    CheckExpandedState(modelRow);
                 }
             }
         }
@@ -108,15 +91,15 @@ namespace AzToolsFramework
                 auto modelRow = model()->index(i, EntityOutlinerListModel::ColumnName, parent);
                 if (modelRow.isValid())
                 {
-                    checkExpandedState(modelRow);
+                    CheckExpandedState(modelRow);
-                    recursiveCheckExpandedStates(modelRow);
+                    RecursiveCheckExpandedStates(modelRow);
                 }
             }
         }
         AzQtComponents::StyledTreeView::rowsInserted(parent, start, end);
     }
 
-    void EntityOutlinerTreeView::recursiveCheckExpandedStates(const QModelIndex& current)
+    void EntityOutlinerTreeView::RecursiveCheckExpandedStates(const QModelIndex& current)
     {
         const int rowCount = model()->rowCount(current);
         for (int i = 0; i < rowCount; i++)
@@ -124,13 +107,13 @@ namespace AzToolsFramework
             auto modelRow = model()->index(i, EntityOutlinerListModel::ColumnName, current);
             if (modelRow.isValid())
             {
-                checkExpandedState(modelRow);
+                CheckExpandedState(modelRow);
-                recursiveCheckExpandedStates(modelRow);
+                RecursiveCheckExpandedStates(modelRow);
             }
         }
     }
 
-    void EntityOutlinerTreeView::checkExpandedState(const QModelIndex& current)
+    void EntityOutlinerTreeView::CheckExpandedState(const QModelIndex& current)
     {
         const bool expandState = current.data(EntityOutlinerListModel::ExpandedRole).template value<bool>();
         setExpanded(current, expandState);
@@ -138,87 +121,125 @@ namespace AzToolsFramework
 
     void EntityOutlinerTreeView::mousePressEvent(QMouseEvent* event)
     {
-        //postponing normal mouse pressed logic until mouse is released or dragged
+        // Postponing normal mouse press logic until mouse is released or dragged.
-        //this means selection occurs on mouse released now
+        // This allows drag/drop of non-selected items.
-        //this is to support drag/drop of non-selected items
         ClearQueuedMouseEvent();
         m_queuedMouseEvent = new QMouseEvent(*event);
     }
 
-    void EntityOutlinerTreeView::mouseReleaseEvent(QMouseEvent* event)
+    void EntityOutlinerTreeView::mouseMoveEvent(QMouseEvent* event)
-    {
-        if (m_queuedMouseEvent && !m_draggingUnselectedItem)
     {
-            // mouseMoveEvent will set the state to be DraggingState, which will make Qt ignore 
+        // Prevent multiple updates throughout the function for changing UIs.
-            // mousePressEvent in QTreeViewPrivate::expandOrCollapseItemAtPos. So we manually 
+        bool forceUpdate = false;
-            // and temporarily set it to EditingState.
-            QAbstractItemView::State stateBefore = QAbstractItemView::state();
-            QAbstractItemView::setState(QAbstractItemView::State::EditingState);
 
-            //treat this as a mouse pressed event to process selection etc
+        QModelIndex previousHoveredIndex = m_currentHoveredIndex;
-            processQueuedMousePressedEvent(m_queuedMouseEvent);
+        m_mousePosition = event->pos();
 
-            QAbstractItemView::setState(stateBefore);
+        if (QModelIndex hoveredIndex = indexAt(m_mousePosition);
+            m_currentHoveredIndex != hoveredIndex)
+        {
+            m_currentHoveredIndex = hoveredIndex;
         }
         
-        ClearQueuedMouseEvent();
+        if (m_queuedMouseEvent)
-        m_draggingUnselectedItem = false;
+        {
+            if (!m_isDragSelectActive)
+            {
+                // Determine whether the mouse move should trigger a rect selection or an entity drag.
+                QModelIndex clickedIndex = indexAt(m_queuedMouseEvent->pos());
+                // Even though the drag started on an index, we want to trigger a drag select from the last column.
+                // This is to allow drag selection to be triggered from anywhere in the hierarchy.
+                if (clickedIndex.isValid() && clickedIndex.column() != EntityOutlinerListModel::ColumnSpacing)
+                {
+                    HandleDrag();
+                }
+                else
+                {
+                    m_isDragSelectActive = true;
+                    forceUpdate = true;
+                }
+            }
+            else
+            {
+                SelectAllEntitiesInSelectionRect();
+                forceUpdate = true;
+            }
+        }
 
-        QTreeView::mouseReleaseEvent(event);
+        if (previousHoveredIndex != m_currentHoveredIndex)
+        {
+            forceUpdate = true;
         }
 
-    void EntityOutlinerTreeView::mouseDoubleClickEvent(QMouseEvent* event)
+        if (forceUpdate)
         {
-        //cancel pending mouse press
+            update();
-        ClearQueuedMouseEvent();
+        }
-        QTreeView::mouseDoubleClickEvent(event);
     }
 
-    void EntityOutlinerTreeView::mouseMoveEvent(QMouseEvent* event)
+    void EntityOutlinerTreeView::mouseReleaseEvent(QMouseEvent* event)
     {
-        if (m_queuedMouseEvent)
+        if (m_isDragSelectActive)
+        {
+            SelectAllEntitiesInSelectionRect();
+            update();
+        }
+        else if (m_queuedMouseEvent)
         {
-            //disable selection for the pending click if the mouse moved so selection is maintained for dragging
+            ProcessQueuedMousePressedEvent(m_queuedMouseEvent);
-            QAbstractItemView::SelectionMode selectionModeBefore = selectionMode();
+        }
-            setSelectionMode(QAbstractItemView::NoSelection);
 
-            //treat this as a mouse pressed event to process everything but selection, but use the position data from the mousePress message
+        ClearQueuedMouseEvent();
-            processQueuedMousePressedEvent(m_queuedMouseEvent);
+        m_isDragSelectActive = false;
 
-            //restore selection state
+        QTreeView::mouseReleaseEvent(event);
-            setSelectionMode(selectionModeBefore);
     }
 
-        m_mousePosition = event->pos();
+    void EntityOutlinerTreeView::mouseDoubleClickEvent(QMouseEvent* event)
-        if (QModelIndex hoveredIndex = indexAt(m_mousePosition); m_currentHoveredIndex != indexAt(m_mousePosition))
     {
-            m_currentHoveredIndex = hoveredIndex;
+        // Cancel pending mouse press.
-            update();
+        ClearQueuedMouseEvent();
-        }
+        QTreeView::mouseDoubleClickEvent(event);
-
-        //process mouse movement as normal, potentially triggering drag and drop
-        QTreeView::mouseMoveEvent(event);
     }
 
     void EntityOutlinerTreeView::focusInEvent(QFocusEvent* event)
     {
-        //cancel pending mouse press
+        // Cancel pending mouse press.
         ClearQueuedMouseEvent();
         QTreeView::focusInEvent(event);
     }
 
     void EntityOutlinerTreeView::focusOutEvent(QFocusEvent* event)
     {
-        //cancel pending mouse press
+        // Cancel pending mouse press.
         ClearQueuedMouseEvent();
         QTreeView::focusOutEvent(event);
     }
 
-    void EntityOutlinerTreeView::startDrag(Qt::DropActions supportedActions)
+    void EntityOutlinerTreeView::dragMoveEvent([[maybe_unused]] QDragMoveEvent* event)
     {
-        QModelIndex index = indexAt(m_queuedMouseEvent->pos());
+        if (m_expandOnlyDelay >= 0)
-        AZ::EntityId entityId(index.data(EntityOutlinerListModel::EntityIdRole).value<AZ::u64>());
+        {
+            m_expandTimer.start(m_expandOnlyDelay, this);
+        }
+
+        QTreeView::dragMoveEvent(event);
+    }
+
+    void EntityOutlinerTreeView::dropEvent([[maybe_unused]] QDropEvent* event)
+    {
+        emit ItemDropped();
+        QTreeView::dropEvent(event);
+
+        ClearQueuedMouseEvent();
+    }
+
+    void EntityOutlinerTreeView::HandleDrag()
+    {
+        // Retrieve the index at the click position.
+        QModelIndex indexAtClick = indexAt(m_queuedMouseEvent->pos()).siblingAtColumn(EntityOutlinerListModel::ColumnName);
 
+        AZ::EntityId entityId(indexAtClick.data(EntityOutlinerListModel::EntityIdRole).value<AZ::u64>());
         AZ::EntityId parentEntityId;
         EditorEntityInfoRequestBus::EventResult(parentEntityId, entityId, &EditorEntityInfoRequestBus::Events::GetParent);
 
@@ -228,41 +249,116 @@ namespace AzToolsFramework
             return;
         }
 
-        //if we are attempting to drag an unselected item then we must special case drag and drop logic
+        // If the index is selected, we should move the whole selection.
-        //QAbstractItemView::startDrag only supports selected items
+        if (selectionModel()->isSelected(indexAtClick))
-        if (m_queuedMouseEvent)
+        {
+            StartCustomDrag(selectionModel()->selectedIndexes(), defaultDropAction());
+        }
+        else
+        {
+            StartCustomDrag(QModelIndexList{ indexAtClick }, defaultDropAction());
+        }
+    }
+
+    void EntityOutlinerTreeView::SelectAllEntitiesInSelectionRect()
     {
-            if (!index.isValid() || index.column() != 0)
+        if (!m_queuedMouseEvent)
         {
             return;
         }
 
-            if (!selectionModel()->isSelected(index))
+        // Retrieve the two opposing corners of the rect.
+        const QPoint point1 = (m_queuedMouseEvent->pos());  // The position the drag operation started at.
+        const QPoint point2 = (m_mousePosition);            // The current mouse position.
+
+        // Determine which point's y is the top and which is the bottom.
+        const int top(AZStd::min(point1.y(), point2.y()));
+        const int bottom(AZStd::max(point1.y(), point2.y()));
+        // We don't really need the x values for the rect, just use the center of the viewport.
+        const int middle(viewport()->rect().center().x());
+
+        // Find the extremes of the range of indices that are in the selection rect.
+        QModelIndex topIndex = indexAt(QPoint(middle, top));
+        const QModelIndex bottomIndex = indexAt(QPoint(middle, bottom));
+
+        // If we have no top index, the mouse may have been dragged above the top item. Let's try to course correct.
+        const int topDistanceForFirstItem = 10; // A reasonable distance from the top we're sure to encounter the first item.
+        const QModelIndex firstIndex = indexAt(QPoint(middle, topDistanceForFirstItem));
+
+        if (!topIndex.isValid() && top < topDistanceForFirstItem)
+        {
+            topIndex = firstIndex;
+        }
+
+        // We can assume that if topIndex is still invalid, it was below the last item in the hierarchy, hence no selection is made.
+        if (!topIndex.isValid())
         {
-                StartCustomDrag({ index }, supportedActions);
             return;
         }
+
+        QItemSelection selection;
+
+        // Starting from the top index, traverse all visible elements of the list and select them until the bottom index is hit.
+        // If the bottom index is undefined, just keep going to the end.
+        QModelIndex iter = topIndex;
+        selection.select(iter, iter);
+
+        while (iter.isValid() && iter != bottomIndex)
+        {
+            iter = indexBelow(iter);
+            selection.select(iter, iter);
         }
 
-        StyledTreeView::startDrag(supportedActions);
+        selectionModel()->select(selection, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
     }
 
-    void EntityOutlinerTreeView::dragMoveEvent(QDragMoveEvent* event)
+    void EntityOutlinerTreeView::ClearQueuedMouseEvent()
     {
-        if (m_expandOnlyDelay >= 0)
+        if (m_queuedMouseEvent)
         {
-            m_expandTimer.start(m_expandOnlyDelay, this);
+            delete m_queuedMouseEvent;
+            m_queuedMouseEvent = nullptr;
+        }
     }
 
-        QTreeView::dragMoveEvent(event);
+    void EntityOutlinerTreeView::leaveEvent([[maybe_unused]] QEvent* event)
+    {
+        ClearQueuedMouseEvent();
+
+        // Only clear the mouse position if the last mouse position registered is inside.
+        // This allows drag to select to work correctly in all situations.
+        if(this->viewport()->rect().contains(m_mousePosition))
+        {
+            m_mousePosition = QPoint(-1, -1);
+        }
+        m_currentHoveredIndex = QModelIndex();
+        update();
     }
 
-    void EntityOutlinerTreeView::dropEvent(QDropEvent* event)
+    void EntityOutlinerTreeView::paintEvent(QPaintEvent* event)
     {
-        emit ItemDropped();
+        AzQtComponents::StyledTreeView::paintEvent(event);
-        QTreeView::dropEvent(event);
 
-        m_draggingUnselectedItem = false;
+        // Draw the drag selection rect.
+        if (m_isDragSelectActive && m_queuedMouseEvent)
+        {
+            // Create a painter to draw on the viewport.
+            QPainter painter(viewport());
+
+            // Retrieve the two corners of the rect.
+            const QPoint point1 = (m_queuedMouseEvent->pos());  // The position the drag operation started at.
+            const QPoint point2 = (m_mousePosition);            // The current mouse position.
+
+            // We need the top left and bottom right corners, which may not be the two corners we got above.
+            // So we composite the corners based on the coordinates of the points.
+            const QPoint topLeft(AZStd::min(point1.x(), point2.x()), AZStd::min(point1.y(), point2.y()));
+            const QPoint bottomRight(AZStd::max(point1.x(), point2.x()), AZStd::max(point1.y(), point2.y()));
+
+            // Paint the rect.
+            painter.setBrush(m_dragSelectRectColor);
+            painter.setPen(m_dragSelectBorderColor);
+            painter.drawRect(QRect(topLeft, bottomRight));
+        }
     }
 
     void EntityOutlinerTreeView::drawBranches(QPainter* painter, const QRect& rect, const QModelIndex& index) const
@@ -270,7 +366,7 @@ namespace AzToolsFramework
         const bool isEnabled = (this->model()->flags(index) & Qt::ItemIsEnabled);
 
         const bool isSelected = selectionModel()->isSelected(index);
-        const bool isHovered = (index == indexAt(m_mousePosition).siblingAtColumn(0)) && isEnabled;
+        const bool isHovered = (index == m_currentHoveredIndex.siblingAtColumn(0)) && isEnabled;
 
         // Paint the branch Selection/Hover Rect
         PaintBranchSelectionHoverRect(painter, rect, isSelected, isHovered);
@@ -352,7 +448,10 @@ namespace AzToolsFramework
         QTreeView::timerEvent(event);
     }
 
-    void EntityOutlinerTreeView::processQueuedMousePressedEvent(QMouseEvent* event)
+    void EntityOutlinerTreeView::ProcessQueuedMousePressedEvent(QMouseEvent* event)
+    {
+        QModelIndex clickedIndex = indexAt(m_queuedMouseEvent->pos());
+        if (!clickedIndex.isValid() || clickedIndex.column() != EntityOutlinerListModel::ColumnSpacing)
         {
             //interpret the mouse event as a button press
             QMouseEvent mousePressedEvent(
@@ -366,11 +465,10 @@ namespace AzToolsFramework
                 event->source());
             QTreeView::mousePressEvent(&mousePressedEvent);
         }
+    }
 
     void EntityOutlinerTreeView::StartCustomDrag(const QModelIndexList& indexList, Qt::DropActions supportedActions)
     {
-        m_draggingUnselectedItem = true;
-
         //sort by container entity depth and order in hierarchy for proper drag image and drop order
         QModelIndexList indexListSorted = indexList;
         AZStd::unordered_map<AZ::EntityId, AZStd::list<AZ::u64>> locations;

Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerTreeView.hxx

@@ -63,7 +63,6 @@ namespace AzToolsFramework
         void mouseMoveEvent(QMouseEvent* event) override;
         void focusInEvent(QFocusEvent* event) override;
         void focusOutEvent(QFocusEvent* event) override;
-        void startDrag(Qt::DropActions supportedActions) override;
         void dragMoveEvent(QDragMoveEvent* event) override;
         void dropEvent(QDropEvent* event) override;
         void leaveEvent(QEvent* event) override;
@@ -71,33 +70,39 @@ namespace AzToolsFramework
         // FocusModeNotificationBus overrides ...
         void OnEditorFocusChanged(AZ::EntityId previousFocusEntityId, AZ::EntityId newFocusEntityId) override;
 
+        void paintEvent(QPaintEvent* event) override;
+
         //! Renders the left side of the item: appropriate background, branch lines, icons.
         void drawBranches(QPainter* painter, const QRect& rect, const QModelIndex& index) const override;
 
         void timerEvent(QTimerEvent* event) override;
     private:
         void ClearQueuedMouseEvent();
+        void ProcessQueuedMousePressedEvent(QMouseEvent* event);
 
-        void processQueuedMousePressedEvent(QMouseEvent* event);
+        void SelectAllEntitiesInSelectionRect();
-        void recursiveCheckExpandedStates(const QModelIndex& parent);
-        void checkExpandedState(const QModelIndex& current);
 
+        void HandleDrag();
         void StartCustomDrag(const QModelIndexList& indexList, Qt::DropActions supportedActions) override;
 
+        void RecursiveCheckExpandedStates(const QModelIndex& parent);
+        void CheckExpandedState(const QModelIndex& current);
+
         void PaintBranchBackground(QPainter* painter, const QRect& rect, const QModelIndex& index) const;
         void PaintBranchSelectionHoverRect(QPainter* painter, const QRect& rect, bool isSelected, bool isHovered) const;
         
         QMouseEvent* m_queuedMouseEvent;
+        QModelIndex m_currentHoveredIndex;
         QPoint m_mousePosition;
-        bool m_draggingUnselectedItem; // This is set when an item is dragged outside its bounding box.
 
+        bool m_isDragSelectActive = false;
         int m_expandOnlyDelay = -1;
         QBasicTimer m_expandTimer;
 
         const QColor m_selectedColor = QColor(255, 255, 255, 45);
         const QColor m_hoverColor = QColor(255, 255, 255, 30);
-
+        const QColor m_dragSelectRectColor = QColor(255, 255, 255, 20);
-        QModelIndex m_currentHoveredIndex;
+        const QColor m_dragSelectBorderColor = QColor(255, 255, 255);
 
         EditorEntityUiInterface* m_editorEntityFrameworkInterface = nullptr;
         ReadOnlyEntityPublicInterface* m_readOnlyEntityPublicInterface = nullptr;

Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerWidget.cpp

@@ -193,7 +193,6 @@ namespace AzToolsFramework
         m_listModel->SetSortMode(m_sortMode);
 
         const int autoExpandDelayMilliseconds = 2500;
-        m_gui->m_objectTree->setSelectionMode(QAbstractItemView::ExtendedSelection);
         SetDefaultTreeViewEditTriggers();
         m_gui->m_objectTree->setAutoExpandDelay(autoExpandDelayMilliseconds);
         m_gui->m_objectTree->setDragEnabled(true);
@@ -208,6 +207,7 @@ namespace AzToolsFramework
         m_gui->m_objectTree->setAutoScrollMargin(20);
         m_gui->m_objectTree->setIndentation(24);
         m_gui->m_objectTree->setRootIsDecorated(false);
+        m_gui->m_objectTree->setSelectionMode(QAbstractItemView::ExtendedSelection);
         connect(m_gui->m_objectTree, &QTreeView::customContextMenuRequested, this, &EntityOutlinerWidget::OnOpenTreeContextMenu);
 
         // custom item delegate
@@ -260,6 +260,8 @@ namespace AzToolsFramework
         m_gui->m_objectTree->header()->resizeSection(EntityOutlinerListModel::ColumnVisibilityToggle, 20);
         m_gui->m_objectTree->header()->setSectionResizeMode(EntityOutlinerListModel::ColumnLockToggle, QHeaderView::Fixed);
         m_gui->m_objectTree->header()->resizeSection(EntityOutlinerListModel::ColumnLockToggle, 24);
+        m_gui->m_objectTree->header()->setSectionResizeMode(EntityOutlinerListModel::ColumnSpacing, QHeaderView::Fixed);
+        m_gui->m_objectTree->header()->resizeSection(EntityOutlinerListModel::ColumnSpacing, 16);
 
         connect(m_gui->m_objectTree->selectionModel(),
             &QItemSelectionModel::selectionChanged,