TableView: don't clear existing selection

If TableView.selectionMode is ExtendedSelection, and the
user starts a second selection (by pressing ControlModifier
while dragging), the second selection can end up clearing
the first if they temporarily overlap.

This change will make sure that we save the current selection
when a new extended selection starts, to ensure that a cell will
stay selected if either the first or the second selection selects
it.

Fixes: QTBUG-121132
Change-Id: I7bd6eeb4b0cc2372aa683c3c71d8e1b25c5ef17e
Reviewed-by: Santhosh Kumar <santhosh.kumar.selvaraj@qt.io>
(cherry picked from commit 6eb26fe88648265ff4f7f1508280a24bb2cb900d)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit ac4ff59e7be0d80ae5f278491207c5776a080a50)

4 files changed, 56 insertions, 49 deletions

diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index 1e913ac10e..325ca66edd 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -1636,6 +1636,8 @@ bool QQuickTableViewPrivate::startSelection(const QPointF &pos)
     if (selectionMode == QQuickTableView::SingleSelection
             || selectionMode == QQuickTableView::ContiguousSelection)
         clearSelection();
+    else if (selectionModel)
+        existingSelection = selectionModel->selection();
 
     selectionStartCell = QPoint(-1, -1);
     selectionEndCell = QPoint(-1, -1);
@@ -1774,39 +1776,49 @@ void QQuickTableViewPrivate::updateSelection(const QRect &oldSelection, const QR
     const QRect oldRect = oldSelection.normalized();
     const QRect newRect = newSelection.normalized();
 
+    QItemSelection select;
+    QItemSelection deselect;
+
     // Select cells inside the new selection rect
     {
         const QModelIndex startIndex = qaim->index(newRect.y(), newRect.x());
         const QModelIndex endIndex = qaim->index(newRect.y() + newRect.height(), newRect.x() + newRect.width());
-        selectionModel->select(QItemSelection(startIndex, endIndex), QItemSelectionModel::Select);
+        select = QItemSelection(startIndex, endIndex);
     }
 
     // Unselect cells in the new minus old rects
     if (oldRect.x() < newRect.x()) {
         const QModelIndex startIndex = qaim->index(oldRect.y(), oldRect.x());
         const QModelIndex endIndex = qaim->index(oldRect.y() + oldRect.height(), newRect.x() - 1);
-        selectionModel->select(QItemSelection(startIndex, endIndex), QItemSelectionModel::Deselect);
+        deselect.merge(QItemSelection(startIndex, endIndex), QItemSelectionModel::Select);
     } else if (oldRect.x() + oldRect.width() > newRect.x() + newRect.width()) {
         const QModelIndex startIndex = qaim->index(oldRect.y(), newRect.x() + newRect.width() + 1);
         const QModelIndex endIndex = qaim->index(oldRect.y() + oldRect.height(), oldRect.x() + oldRect.width());
-        selectionModel->select(QItemSelection(startIndex, endIndex), QItemSelectionModel::Deselect);
+        deselect.merge(QItemSelection(startIndex, endIndex), QItemSelectionModel::Select);
     }
 
     if (oldRect.y() < newRect.y()) {
         const QModelIndex startIndex = qaim->index(oldRect.y(), oldRect.x());
         const QModelIndex endIndex = qaim->index(newRect.y() - 1, oldRect.x() + oldRect.width());
-        selectionModel->select(QItemSelection(startIndex, endIndex), QItemSelectionModel::Deselect);
+        deselect.merge(QItemSelection(startIndex, endIndex), QItemSelectionModel::Select);
     } else if (oldRect.y() + oldRect.height() > newRect.y() + newRect.height()) {
         const QModelIndex startIndex = qaim->index(newRect.y() + newRect.height() + 1, oldRect.x());
         const QModelIndex endIndex = qaim->index(oldRect.y() + oldRect.height(), oldRect.x() + oldRect.width());
-        selectionModel->select(QItemSelection(startIndex, endIndex), QItemSelectionModel::Deselect);
+        deselect.merge(QItemSelection(startIndex, endIndex), QItemSelectionModel::Select);
     }
+
+    // Don't clear the selection that existed before the user started a new selection block
+    deselect.merge(existingSelection, QItemSelectionModel::Deselect);
+
+    selectionModel->select(deselect, QItemSelectionModel::Deselect);
+    selectionModel->select(select, QItemSelectionModel::Select);
 }
 
 void QQuickTableViewPrivate::clearSelection()
 {
     selectionStartCell = QPoint(-1, -1);
     selectionEndCell = QPoint(-1, -1);
+    existingSelection.clear();
 
     if (selectionModel)
         selectionModel->clearSelection();
diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h
index 60ccb26c9e..a3768fda71 100644
--- a/src/quick/items/qquicktableview_p_p.h
+++ b/src/quick/items/qquicktableview_p_p.h
@@ -377,6 +377,7 @@ public:
 
     QPoint selectionStartCell = {-1, -1};
     QPoint selectionEndCell = {-1, -1};
+    QItemSelection existingSelection;
 
     QMargins edgesBeforeRebuild;
     QSize tableSizeBeforeRebuild;
diff --git a/src/quick/items/qquicktreeview.cpp b/src/quick/items/qquicktreeview.cpp
index 7fe3777c39..f2613b4f48 100644
--- a/src/quick/items/qquicktreeview.cpp
+++ b/src/quick/items/qquicktreeview.cpp
@@ -344,56 +344,33 @@ void QQuickTreeViewPrivate::updateSelection(const QRect &oldSelection, const QRe
 {
     Q_Q(QQuickTreeView);
 
-    const QRect oldRect = oldSelection.normalized();
-    const QRect newRect = newSelection.normalized();
-
     if (oldSelection == newSelection)
         return;
 
-    // Select the rows inside newRect that doesn't overlap with oldRect
-    for (int row = newRect.y(); row <= newRect.y() + newRect.height(); ++row) {
-        if (oldRect.y() != -1 && oldRect.y() <= row && row <= oldRect.y() + oldRect.height())
-            continue;
-        const QModelIndex startIndex = q->index(row, newRect.x());
-        const QModelIndex endIndex = q->index(row, newRect.x() + newRect.width());
-        selectionModel->select(QItemSelection(startIndex, endIndex), QItemSelectionModel::Select);
+    QItemSelection select;
+    QItemSelection deselect;
+
+    // Because each row can have a different parent, we need to create separate QItemSelections
+    // per row. But all the cells in a given row have the same parent, so they can be combined.
+    // As a result, the final QItemSelection can end up more fragmented compared to a selection
+    // in QQuickTableView, where all cells have the same parent. In the end, if TreeView has
+    // a lot of columns and the selection mode is "SelectCells", using the mouse to adjust
+    // a selection containing a _large_ number of columns can be slow.
+    const QRect cells = newSelection.normalized();
+    for (int row = cells.y(); row <= cells.y() + cells.height(); ++row) {
+        const QModelIndex startIndex = q->index(row, cells.x());
+        const QModelIndex endIndex = q->index(row, cells.x() + cells.width());
+        select.merge(QItemSelection(startIndex, endIndex), QItemSelectionModel::Select);
     }
 
-    if (oldRect.x() != -1) {
-        // Since oldRect is valid, this update is a continuation of an already existing selection!
-
-        // Select the columns inside newRect that don't overlap with oldRect
-        for (int column = newRect.x(); column <= newRect.x() + newRect.width(); ++column) {
-            if (oldRect.x() <= column && column <= oldRect.x() + oldRect.width())
-                continue;
-            for (int row = newRect.y(); row <= newRect.y() + newRect.height(); ++row)
-                selectionModel->select(q->index(row, column), QItemSelectionModel::Select);
-        }
-
-        // Unselect the rows inside oldRect that don't overlap with newRect
-        for (int row = oldRect.y(); row <= oldRect.y() + oldRect.height(); ++row) {
-            if (newRect.y() <= row && row <= newRect.y() + newRect.height())
-                continue;
-            const QModelIndex startIndex = q->index(row, oldRect.x());
-            const QModelIndex endIndex = q->index(row, oldRect.x() + oldRect.width());
-            selectionModel->select(QItemSelection(startIndex, endIndex), QItemSelectionModel::Deselect);
-        }
-
-        // Unselect the columns inside oldRect that don't overlap with newRect
-        for (int column = oldRect.x(); column <= oldRect.x() + oldRect.width(); ++column) {
-            if (newRect.x() <= column && column <= newRect.x() + newRect.width())
-                continue;
-            // Since we're not allowed to call select/unselect on the selectionModel with
-            // indices from different parents, and since indicies from different parents are
-            // expected when working with trees, we need to unselect the indices in the column
-            // one by one, rather than the whole column in one go. This, however, can cause a
-            // lot of selection fragments in the selectionModel, which eventually can hurt
-            // performance. But large selections containing a lot of columns is not normally
-            // the case for a treeview, so accept this potential corner case for now.
-            for (int row = newRect.y(); row <= newRect.y() + newRect.height(); ++row)
-                selectionModel->select(q->index(row, column), QItemSelectionModel::Deselect);
-        }
+    const QModelIndexList indexes = selectionModel->selection().indexes();
+    for (const QModelIndex &index : indexes) {
+        if (!select.contains(index) && !existingSelection.contains(index))
+            deselect.merge(QItemSelection(index, index), QItemSelectionModel::Select);
     }
+
+    selectionModel->select(deselect, QItemSelectionModel::Deselect);
+    selectionModel->select(select, QItemSelectionModel::Select);
 }
 
 QQuickTreeView::QQuickTreeView(QQuickItem *parent)
diff --git a/tests/auto/quickcontrols/controls/data/tst_selectionrectangle.qml b/tests/auto/quickcontrols/controls/data/tst_selectionrectangle.qml
index 932ca0b1c7..5f6f60eba8 100644
--- a/tests/auto/quickcontrols/controls/data/tst_selectionrectangle.qml
+++ b/tests/auto/quickcontrols/controls/data/tst_selectionrectangle.qml
@@ -415,6 +415,23 @@ TestCase {
         for (let r = 0; r < 2; ++r)
             for (let c = 0; c < 3; ++c)
                 verify(tableView.selectionModel.isSelected(tableView.model.index(r, c)))
+
+        // Shift click the second selection so that it overlaps with the first
+        mouseClick(tableView, 1, 1, Qt.LeftButton, Qt.ShiftModifier)
+        compare(tableView.selectionModel.selectedIndexes.length, 4)
+        verify(tableView.selectionModel.isSelected(tableView.model.index(0, 0)))
+        verify(tableView.selectionModel.isSelected(tableView.model.index(0, 1)))
+        verify(tableView.selectionModel.isSelected(tableView.model.index(0, 2)))
+        verify(tableView.selectionModel.isSelected(tableView.model.index(1, 0)))
+
+        // Shift click the selection back again. The first selection on
+        // row 0 should still be present, even if the second selection
+        // no longer overlaps it.
+        mouseClick(tableView, (cellWidth * 3) - 2, (cellHeight * 2) - 1, Qt.LeftButton, Qt.ShiftModifier)
+        compare(tableView.selectionModel.selectedIndexes.length, 6)
+        for (let r = 0; r < 2; ++r)
+            for (let c = 0; c < 3; ++c)
+                verify(tableView.selectionModel.isSelected(tableView.model.index(r, c)))
     }
 
     function test_handle_position() {