Selection correction for scatter when data changes

Implements item 3) in QTRD-2645

Task-number: QTRD-264
Change-Id: Ibe758bbfb3b4a74b55589a410b402bbdf07ea64f
Reviewed-by: Tomi Korpipää <tomi.korpipaa@digia.com>

5 files changed, 153 insertions, 44 deletions

diff --git a/src/datavisualization/data/abstractitemmodelhandler.cpp b/src/datavisualization/data/abstractitemmodelhandler.cpp
index cf87c3f9..879ce086 100644
--- a/src/datavisualization/data/abstractitemmodelhandler.cpp
+++ b/src/datavisualization/data/abstractitemmodelhandler.cpp
@@ -23,7 +23,8 @@ QT_BEGIN_NAMESPACE_DATAVISUALIZATION
 
 AbstractItemModelHandler::AbstractItemModelHandler(QObject *parent)
     : QObject(parent),
-      resolvePending(0)
+      resolvePending(0),
+      m_fullReset(true)
 {
     m_resolveTimer.setSingleShot(true);
     QObject::connect(&m_resolveTimer, &QTimer::timeout,
@@ -81,9 +82,12 @@ void AbstractItemModelHandler::handleColumnsInserted(const QModelIndex &parent,
     Q_UNUSED(start)
     Q_UNUSED(end)
 
-    // Resolve new items
-    if (!m_resolveTimer.isActive())
-        m_resolveTimer.start(0); // TODO Resolving entire model is inefficient
+    // Manipulating columns changes all rows in proxies that map rows/columns directly,
+    // and its effects are not clearly defined in others -> always do full reset.
+    if (!m_resolveTimer.isActive()) {
+        m_fullReset = true;
+        m_resolveTimer.start(0);
+    }
 }
 
 void AbstractItemModelHandler::handleColumnsMoved(const QModelIndex &sourceParent,
@@ -98,9 +102,12 @@ void AbstractItemModelHandler::handleColumnsMoved(const QModelIndex &sourceParen
     Q_UNUSED(destinationParent)
     Q_UNUSED(destinationColumn)
 
-    // Resolve moved items
-    if (!m_resolveTimer.isActive())
-        m_resolveTimer.start(0); // TODO Resolving entire model is inefficient
+    // Manipulating columns changes all rows in proxies that map rows/columns directly,
+    // and its effects are not clearly defined in others -> always do full reset.
+    if (!m_resolveTimer.isActive()) {
+        m_fullReset = true;
+        m_resolveTimer.start(0);
+    }
 }
 
 void AbstractItemModelHandler::handleColumnsRemoved(const QModelIndex &parent,
@@ -110,9 +117,12 @@ void AbstractItemModelHandler::handleColumnsRemoved(const QModelIndex &parent,
     Q_UNUSED(start)
     Q_UNUSED(end)
 
-    // Remove old items
-    if (!m_resolveTimer.isActive())
-        m_resolveTimer.start(0); // TODO Resolving entire model is inefficient
+    // Manipulating columns changes all rows in proxies that map rows/columns directly,
+    // and its effects are not clearly defined in others -> always do full reset.
+    if (!m_resolveTimer.isActive()) {
+        m_fullReset = true;
+        m_resolveTimer.start(0);
+    }
 }
 
 void AbstractItemModelHandler::handleDataChanged(const QModelIndex &topLeft,
@@ -123,9 +133,13 @@ void AbstractItemModelHandler::handleDataChanged(const QModelIndex &topLeft,
     Q_UNUSED(bottomRight)
     Q_UNUSED(roles)
 
-    // Resolve changed items
-    if (!m_resolveTimer.isActive())
-        m_resolveTimer.start(0); // TODO Resolving entire model is inefficient
+    // Default handling for dataChanged is to do full reset, as it cannot be optimized
+    // in a general case, where we do not know which row/column/index the item model item
+    // actually ended up to in the proxy.
+    if (!m_resolveTimer.isActive()) {
+        m_fullReset = true;
+        m_resolveTimer.start(0);
+    }
 }
 
 void AbstractItemModelHandler::handleLayoutChanged(const QList<QPersistentModelIndex> &parents,
@@ -134,16 +148,20 @@ void AbstractItemModelHandler::handleLayoutChanged(const QList<QPersistentModelI
     Q_UNUSED(parents)
     Q_UNUSED(hint)
 
-    // Resolve moved items
-    if (!m_resolveTimer.isActive())
-        m_resolveTimer.start(0); // TODO Resolving entire model is inefficient
+    // Resolve entire model if layout changes
+    if (!m_resolveTimer.isActive()) {
+        m_fullReset = true;
+        m_resolveTimer.start(0);
+    }
 }
 
 void AbstractItemModelHandler::handleModelReset()
 {
     // Data cleared, reset array
-    if (!m_resolveTimer.isActive())
-        m_resolveTimer.start(0); // TODO Resolving entire model is inefficient
+    if (!m_resolveTimer.isActive()) {
+        m_fullReset = true;
+        m_resolveTimer.start(0);
+    }
 }
 
 void AbstractItemModelHandler::handleRowsInserted(const QModelIndex &parent, int start, int end)
@@ -152,9 +170,13 @@ void AbstractItemModelHandler::handleRowsInserted(const QModelIndex &parent, int
     Q_UNUSED(start)
     Q_UNUSED(end)
 
-    // Resolve new items
-    if (!m_resolveTimer.isActive())
-        m_resolveTimer.start(0); // TODO Resolving entire model is inefficient
+    // Default handling for rowsInserted is to do full reset, as it cannot be optimized
+    // in a general case, where we do not know which row/column/index the item model item
+    // actually ended up to in the proxy.
+    if (!m_resolveTimer.isActive()) {
+        m_fullReset = true;
+        m_resolveTimer.start(0);
+    }
 }
 
 void AbstractItemModelHandler::handleRowsMoved(const QModelIndex &sourceParent,
@@ -169,9 +191,13 @@ void AbstractItemModelHandler::handleRowsMoved(const QModelIndex &sourceParent,
     Q_UNUSED(destinationParent)
     Q_UNUSED(destinationRow)
 
-    // Resolve moved items
-    if (!m_resolveTimer.isActive())
-        m_resolveTimer.start(0); // TODO Resolving entire model is inefficient
+    // Default handling for rowsMoved is to do full reset, as it cannot be optimized
+    // in a general case, where we do not know which row/column/index the item model item
+    // actually ended up to in the proxy.
+    if (!m_resolveTimer.isActive()) {
+        m_fullReset = true;
+        m_resolveTimer.start(0);
+    }
 }
 
 void AbstractItemModelHandler::handleRowsRemoved(const QModelIndex &parent, int start, int end)
@@ -180,9 +206,13 @@ void AbstractItemModelHandler::handleRowsRemoved(const QModelIndex &parent, int
     Q_UNUSED(start)
     Q_UNUSED(end)
 
-    // Resolve removed items
-    if (!m_resolveTimer.isActive())
-        m_resolveTimer.start(0); // TODO Resolving entire model is inefficient
+    // Default handling for rowsRemoved is to do full reset, as it cannot be optimized
+    // in a general case, where we do not know which row/column/index the item model item
+    // actually ended up to in the proxy.
+    if (!m_resolveTimer.isActive()) {
+        m_fullReset = true;
+        m_resolveTimer.start(0);
+    }
 }
 
 void AbstractItemModelHandler::handleMappingChanged()
@@ -194,6 +224,7 @@ void AbstractItemModelHandler::handleMappingChanged()
 void AbstractItemModelHandler::handlePendingResolve()
 {
     resolveModel();
+    m_fullReset = false;
 }
 
 QT_END_NAMESPACE_DATAVISUALIZATION
diff --git a/src/datavisualization/data/abstractitemmodelhandler_p.h b/src/datavisualization/data/abstractitemmodelhandler_p.h
index f3b72228..daaf9906 100644
--- a/src/datavisualization/data/abstractitemmodelhandler_p.h
+++ b/src/datavisualization/data/abstractitemmodelhandler_p.h
@@ -74,6 +74,7 @@ protected:
     QPointer<const QAbstractItemModel> m_itemModel;  // Not owned
     bool resolvePending;
     QTimer m_resolveTimer;
+    bool m_fullReset;
 
 private:
     Q_DISABLE_COPY(AbstractItemModelHandler)
diff --git a/src/datavisualization/data/qitemmodelscatterdataproxy.cpp b/src/datavisualization/data/qitemmodelscatterdataproxy.cpp
index 74979309..16a7c8f7 100644
--- a/src/datavisualization/data/qitemmodelscatterdataproxy.cpp
+++ b/src/datavisualization/data/qitemmodelscatterdataproxy.cpp
@@ -32,7 +32,9 @@ QT_BEGIN_NAMESPACE_DATAVISUALIZATION
  * for Q3DScatter. It maps roles of QAbstractItemModel to the XYZ-values of Q3DScatter points.
  *
  * The data is resolved asynchronously whenever the mapping or the model changes.
- * QScatterDataProxy::arrayReset() is emitted when the data has been resolved.
+ * QScatterDataProxy::arrayReset() is emitted when the data has been resolved. However, inserts,
+ * removes, and single data item changes after the model initialization are resolved synchronously,
+ * unless the same frame also contains a change that causes the whole model to be resolved.
  *
  * Mapping ignores rows and columns of the QAbstractItemModel and treats
  * all items equally. It requires the model to provide at least three roles for the data items
diff --git a/src/datavisualization/data/scatteritemmodelhandler.cpp b/src/datavisualization/data/scatteritemmodelhandler.cpp
index 2faa02a9..81ecf8b0 100644
--- a/src/datavisualization/data/scatteritemmodelhandler.cpp
+++ b/src/datavisualization/data/scatteritemmodelhandler.cpp
@@ -21,6 +21,8 @@
 
 QT_BEGIN_NAMESPACE_DATAVISUALIZATION
 
+static const int noRoleIndex = -1;
+
 ScatterItemModelHandler::ScatterItemModelHandler(QItemModelScatterDataProxy *proxy, QObject *parent)
     : AbstractItemModelHandler(parent),
       m_proxy(proxy),
@@ -32,6 +34,79 @@ ScatterItemModelHandler::~ScatterItemModelHandler()
 {
 }
 
+void ScatterItemModelHandler::handleDataChanged(const QModelIndex &topLeft,
+                                                const QModelIndex &bottomRight,
+                                                const QVector<int> &roles)
+{
+    // Do nothing if full reset already pending
+    if (!m_fullReset) {
+        if (m_itemModel->columnCount() > 1) {
+            // If the data model is multi-column, do full asynchronous reset to simplify things
+            AbstractItemModelHandler::handleDataChanged(topLeft, bottomRight, roles);
+        } else {
+            int start = qMin(topLeft.row(), bottomRight.row());
+            int end = qMax(topLeft.row(), bottomRight.row());
+
+            QScatterDataArray array(end - start + 1);
+            int count = 0;
+            for (int i = start; i <= end; i++)
+                modelPosToScatterItem(i, 0, array[count++]);
+
+            m_proxy->setItems(start, array);
+        }
+    }
+}
+
+void ScatterItemModelHandler::handleRowsInserted(const QModelIndex &parent, int start, int end)
+{
+    // Do nothing if full reset already pending
+    if (!m_fullReset) {
+        if (!m_proxy->itemCount() || m_itemModel->columnCount() > 1) {
+            // If inserting into an empty array, do full asynchronous reset to avoid multiple
+            // separate inserts when initializing the model.
+            // If the data model is multi-column, do full asynchronous reset to simplify things
+            AbstractItemModelHandler::handleRowsInserted(parent, start, end);
+        } else {
+            QScatterDataArray array(end - start + 1);
+            int count = 0;
+            for (int i = start; i <= end; i++)
+                modelPosToScatterItem(i, 0, array[count++]);
+
+            m_proxy->insertItems(start, array);
+        }
+    }
+}
+
+void ScatterItemModelHandler::handleRowsRemoved(const QModelIndex &parent, int start, int end)
+{
+    Q_UNUSED(parent)
+
+    // Do nothing if full reset already pending
+    if (!m_fullReset) {
+        if (m_itemModel->columnCount() > 1) {
+            // If the data model is multi-column, do full asynchronous reset to simplify things
+            AbstractItemModelHandler::handleRowsRemoved(parent, start, end);
+        } else {
+            m_proxy->removeItems(start, end - start + 1);
+        }
+    }
+}
+
+void ScatterItemModelHandler::modelPosToScatterItem(int modelRow, int modelColumn, QScatterDataItem &item)
+{
+    QModelIndex index = m_itemModel->index(modelRow, modelColumn);
+    float xPos(0.0f);
+    float yPos(0.0f);
+    float zPos(0.0f);
+    if (m_xPosRole != noRoleIndex)
+        xPos = index.data(m_xPosRole).toFloat();
+    if (m_yPosRole != noRoleIndex)
+        yPos = index.data(m_yPosRole).toFloat();
+    if (m_zPosRole != noRoleIndex)
+        zPos = index.data(m_zPosRole).toFloat();
+    item.setPosition(QVector3D(xPos, yPos, zPos));
+}
+
 // Resolve entire item model into QScatterDataArray.
 void ScatterItemModelHandler::resolveModel()
 {
@@ -41,12 +116,10 @@ void ScatterItemModelHandler::resolveModel()
         return;
     }
 
-    static const int noRoleIndex = -1;
-
     QHash<int, QByteArray> roleHash = m_itemModel->roleNames();
-    const int xPosRole = roleHash.key(m_proxy->xPosRole().toLatin1(), noRoleIndex);
-    const int yPosRole = roleHash.key(m_proxy->yPosRole().toLatin1(), noRoleIndex);
-    const int zPosRole = roleHash.key(m_proxy->zPosRole().toLatin1(), noRoleIndex);
+    m_xPosRole = roleHash.key(m_proxy->xPosRole().toLatin1(), noRoleIndex);
+    m_yPosRole = roleHash.key(m_proxy->yPosRole().toLatin1(), noRoleIndex);
+    m_zPosRole = roleHash.key(m_proxy->zPosRole().toLatin1(), noRoleIndex);
     const int columnCount = m_itemModel->columnCount();
     const int rowCount = m_itemModel->rowCount();
     const int totalCount = rowCount * columnCount;
@@ -59,17 +132,7 @@ void ScatterItemModelHandler::resolveModel()
     // Parse data into newProxyArray
     for (int i = 0; i < rowCount; i++) {
         for (int j = 0; j < columnCount; j++) {
-            QModelIndex index = m_itemModel->index(i, j);
-            float xPos(0.0f);
-            float yPos(0.0f);
-            float zPos(0.0f);
-            if (xPosRole != noRoleIndex)
-                xPos = index.data(xPosRole).toFloat();
-            if (yPosRole != noRoleIndex)
-                yPos = index.data(yPosRole).toFloat();
-            if (zPosRole != noRoleIndex)
-                zPos = index.data(zPosRole).toFloat();
-            (*m_proxyArray)[runningCount].setPosition(QVector3D(xPos, yPos, zPos));
+            modelPosToScatterItem(i, j, (*m_proxyArray)[runningCount]);
             runningCount++;
         }
     }
diff --git a/src/datavisualization/data/scatteritemmodelhandler_p.h b/src/datavisualization/data/scatteritemmodelhandler_p.h
index f54ed2c5..06927509 100644
--- a/src/datavisualization/data/scatteritemmodelhandler_p.h
+++ b/src/datavisualization/data/scatteritemmodelhandler_p.h
@@ -41,11 +41,23 @@ public:
     ScatterItemModelHandler(QItemModelScatterDataProxy *proxy, QObject *parent = 0);
     virtual ~ScatterItemModelHandler();
 
+public slots:
+    virtual void handleDataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight,
+                                   const QVector<int> &roles = QVector<int> ());
+    virtual void handleRowsInserted(const QModelIndex &parent, int start, int end);
+    virtual void handleRowsRemoved(const QModelIndex &parent, int start, int end);
+
 protected:
     void virtual resolveModel();
 
+private:
+    void modelPosToScatterItem(int modelRow, int modelColumn, QScatterDataItem &item);
+
     QItemModelScatterDataProxy *m_proxy; // Not owned
     QScatterDataArray *m_proxyArray; // Not owned
+    int m_xPosRole;
+    int m_yPosRole;
+    int m_zPosRole;
 };
 
 QT_END_NAMESPACE_DATAVISUALIZATION