Fix index corruption related to entity removal

The main problem was the selection that was done when the selected
entity was removed. Now we change the selection asynchronously
in that case, which ensures the selection model on the qml side
won't try to access invalid indexes.

Change-Id: Ieaf984eed5c74a5d3b595aeb04d23e26f66a0db9
Reviewed-by: Titta Heikkala <titta.heikkala@qt.io>
Reviewed-by: Tomi Korpipää <tomi.korpipaa@qt.io>

6 files changed, 107 insertions, 57 deletions

diff --git a/editorlib/qml/EntityTree.qml b/editorlib/qml/EntityTree.qml
index 8a8549e..b03e4e7 100644
--- a/editorlib/qml/EntityTree.qml
+++ b/editorlib/qml/EntityTree.qml
@@ -197,10 +197,6 @@ Item {
                 for (var i = 0; i < items.length; i++)
                     entityTreeView.expand(items[i])
             }
-            onSelectIndex: {
-                entityTreeView.selection.setCurrentIndex(selectIndex,
-                                                         ItemSelectionModel.SelectCurrent)
-            }
             onModelAboutToBeReset: {
                 entityTreeView.preResetContentY = entityTreeView.flickableItem.contentY
             }
diff --git a/editorlib/src/editorscene.cpp b/editorlib/src/editorscene.cpp
index f7c1a1f..612a1d6 100644
--- a/editorlib/src/editorscene.cpp
+++ b/editorlib/src/editorscene.cpp
@@ -240,8 +240,8 @@ void EditorScene::removeEntity(Qt3DCore::QEntity *entity)
 
     m_sceneItems.remove(entity->id());
 
-    if (!multiSelection() && m_sceneEntity && m_selectedEntity == entity)
-        setSelection(m_sceneEntity);
+    if (m_selectedEntity == entity || multiSelection())
+        ensureSelection();
 
     delete item;
     delete entity;
@@ -249,7 +249,8 @@ void EditorScene::removeEntity(Qt3DCore::QEntity *entity)
 
 void EditorScene::resetScene()
 {
-    m_selectedEntity = nullptr;
+    clearSingleSelection();
+
     // Clear the existing scene
     setFrameGraphCamera(nullptr);
     m_undoHandler->clear();
@@ -304,7 +305,7 @@ bool EditorScene::loadScene(const QUrl &fileUrl)
     Qt3DCore::QEntity *newSceneEntity = m_sceneParser->importQmlScene(fileUrl, camera);
 
     if (newSceneEntity) {
-        m_selectedEntity = nullptr;
+        clearSingleSelection();
         if (!m_freeView)
             setFrameGraphCamera(nullptr);
         m_undoHandler->clear();
@@ -809,6 +810,79 @@ void EditorScene::showDebugHandle(bool show, int handleIndex, const QVector3D &w
                               show, handleIndex, 0);
 }
 
+void EditorScene::ensureSelection()
+{
+    // Ensure that something is selected after the current pending events have executed
+    if (m_sceneEntity && m_ensureSelectionEntityName.isEmpty()) {
+        if (m_selectedEntity) {
+            m_ensureSelectionEntityName = m_selectedEntity->objectName();
+            clearSingleSelection();
+        } else {
+            m_ensureSelectionEntityName = m_sceneEntity->objectName();
+        }
+        QMetaObject::invokeMethod(this, "doEnsureSelection", Qt::QueuedConnection);
+    }
+}
+
+void EditorScene::doEnsureSelection()
+{
+    if (!m_selectedEntity) {
+        // If we have multiselection active, update the multiselection list
+        const int count = m_selectedEntityNameList.size();
+        if (count > 0) {
+            QStringList newList;
+            newList.reserve(count);
+            for (int i = 0; i < count; ++i) {
+                QString entityName = m_selectedEntityNameList.at(i);
+                if (itemByName(entityName))
+                    newList.append(entityName);
+            }
+            if (newList.size() == 1) {
+                m_ensureSelectionEntityName = newList.at(0);
+                newList.clear();
+            }
+
+            if (count != newList.size()) {
+                m_selectedEntityNameList = newList;
+                if (newList.size() == 0)
+                    emit multiSelectionChanged(false);
+            }
+            checkMultiSelectionHighlights();
+            emit multiSelectionListChanged();
+        }
+        if (!m_selectedEntityNameList.size()) {
+            EditorSceneItem *item = itemByName(m_ensureSelectionEntityName);
+            if (item)
+                setSelection(item->entity());
+            else
+                setSelection(m_sceneEntity);
+        }
+    }
+    m_ensureSelectionEntityName.clear();
+}
+
+EditorSceneItem *EditorScene::itemByName(const QString &name)
+{
+    const QList<EditorSceneItem *> items = m_sceneItems.values();
+    for (int i = 0; i < items.size(); ++i) {
+        if (items.at(i)->entity()->objectName() == name)
+            return items.at(i);
+    }
+    return nullptr;
+}
+
+void EditorScene::clearSingleSelection()
+{
+    if (m_selectedEntity) {
+        EditorSceneItem *oldItem = m_sceneItems.value(m_selectedEntity->id(), nullptr);
+        if (oldItem) {
+            connectDragHandles(oldItem, false);
+            oldItem->setShowSelectionBox(false);
+        }
+        m_selectedEntity = nullptr;
+    }
+}
+
 int EditorScene::cameraIndexForEntity(Qt3DCore::QEntity *entity)
 {
     int index = -1;
@@ -1792,13 +1866,7 @@ void EditorScene::setSelection(Qt3DCore::QEntity *entity)
     EditorSceneItem *item = m_sceneItems.value(entity->id(), nullptr);
     if (item) {
         if (entity != m_selectedEntity) {
-            if (m_selectedEntity) {
-                EditorSceneItem *oldItem = m_sceneItems.value(m_selectedEntity->id(), nullptr);
-                if (oldItem) {
-                    connectDragHandles(oldItem, false);
-                    oldItem->setShowSelectionBox(false);
-                }
-            }
+            clearSingleSelection();
 
             m_selectedEntity = entity;
 
@@ -1880,9 +1948,8 @@ void EditorScene::toggleEntityMultiSelection(const QString &name)
 void EditorScene::clearMultiSelection()
 {
     if (m_selectedEntityNameList.size() > 0) {
-        QStringList oldList = m_selectedEntityNameList;
         m_selectedEntityNameList.clear();
-        checkMultiSelectionHighlights(oldList, m_selectedEntityNameList);
+        checkMultiSelectionHighlights();
         emit multiSelectionChanged(false);
         emit multiSelectionListChanged();
         setSelection(m_sceneEntity);
@@ -1893,7 +1960,7 @@ QVector3D EditorScene::getMultiSelectionCenter()
 {
     QVector3D pos;
     for (int i = 0; i < m_selectedEntityNameList.size(); i++) {
-        EditorSceneItem *item = m_sceneModel->getItemByName(m_selectedEntityNameList.at(i));
+        EditorSceneItem *item = itemByName(m_selectedEntityNameList.at(i));
         if (item) {
             item->doUpdateSelectionBoxTransform();
             pos += item->selectionBoxCenter();
@@ -1904,9 +1971,8 @@ QVector3D EditorScene::getMultiSelectionCenter()
 
 void EditorScene::addEntityToMultiSelection(const QString &name)
 {
-    QStringList oldList = m_selectedEntityNameList;
-
-    if (oldList.size() == 0) {
+    const int oldSize = m_selectedEntityNameList.size();
+    if (oldSize == 0) {
         // Do not add if multiselecting the currently selected entity as the first entity
         if (m_selectedEntity->objectName() == name)
             return;
@@ -1915,7 +1981,7 @@ void EditorScene::addEntityToMultiSelection(const QString &name)
             m_selectedEntityNameList.append(m_selectedEntity->objectName());
             m_dragHandlesTransform->setEnabled(false);
             handleSelectionTransformChange();
-            m_selectedEntity = nullptr;
+            clearSingleSelection();
         } else {
             // Just single-select the new entity and return if the other entity was scene entity
             EditorSceneItem *item = m_sceneModel->getItemByName(name);
@@ -1927,20 +1993,18 @@ void EditorScene::addEntityToMultiSelection(const QString &name)
     }
     m_selectedEntityNameList.append(name);
 
-    checkMultiSelectionHighlights(oldList, m_selectedEntityNameList);
+    checkMultiSelectionHighlights();
 
-    if (oldList.size() == 0)
+    if (oldSize == 0)
         emit multiSelectionChanged(true);
     emit multiSelectionListChanged();
 }
 
 void EditorScene::removeEntityFromMultiSelection(const QString &name)
 {
-    QStringList oldList = m_selectedEntityNameList;
     bool removed = m_selectedEntityNameList.removeOne(name);
 
     if (removed) {
-        checkMultiSelectionHighlights(oldList, m_selectedEntityNameList);
         bool lastRemoved = m_selectedEntityNameList.size() == 1;
         EditorSceneItem *lastItem = nullptr;
         if (lastRemoved) {
@@ -1948,6 +2012,7 @@ void EditorScene::removeEntityFromMultiSelection(const QString &name)
             m_selectedEntityNameList.clear();
             emit multiSelectionChanged(false);
         }
+        checkMultiSelectionHighlights();
         emit multiSelectionListChanged();
         if (lastRemoved) {
             if (lastItem)
@@ -2615,24 +2680,15 @@ bool EditorScene::eventFilter(QObject *obj, QEvent *event)
     return false;
 }
 
-void EditorScene::checkMultiSelectionHighlights(const QStringList &oldlist,
-                                                const QStringList &newlist)
+void EditorScene::checkMultiSelectionHighlights()
 {
-    // Find deselected entities
-    for (int i = 0; i < oldlist.length(); ++i) {
-        if (!newlist.contains(oldlist.at(i))) {
-            // Remove highlight
-            m_sceneModel->editorSceneItemFromIndex(m_sceneModel->getModelIndexByName(
-                                                       oldlist.at(i)))->setShowSelectionBox(false);
-        }
-    }
-    // Find selected entities
-    for (int i = 0; i < newlist.length(); ++i) {
-        if (!oldlist.contains(newlist.at(i))) {
-            // Add highlight
-            m_sceneModel->editorSceneItemFromIndex(m_sceneModel->getModelIndexByName(
-                                                       newlist.at(i)))->setShowSelectionBox(true);
-        }
+    const QList<EditorSceneItem *> items = m_sceneItems.values();
+    for (int i = 0; i < items.size(); ++i) {
+        EditorSceneItem *item = items.at(i);
+        if (m_selectedEntityNameList.contains(item->entity()->objectName()))
+            item->setShowSelectionBox(true);
+        else
+            item->setShowSelectionBox(false);
     }
 }
 
diff --git a/editorlib/src/editorscene.h b/editorlib/src/editorscene.h
index 8aaa8f8..88ea589 100644
--- a/editorlib/src/editorscene.h
+++ b/editorlib/src/editorscene.h
@@ -310,6 +310,7 @@ public:
     Qt3DRender::QObjectPicker *createObjectPickerForEntity(Qt3DCore::QEntity *entity);
 
     void showDebugHandle(bool show, int handleIndex = 0, const QVector3D &worldPosition = QVector3D());
+    void ensureSelection();
 
 public slots:
     void clearSelectionBoxes(Qt3DCore::QEntity *skipEntity = nullptr);
@@ -383,8 +384,12 @@ private:
     void setSceneEntity(Qt3DCore::QEntity *newSceneEntity = nullptr);
     void createSceneLoaderChildPickers(Qt3DCore::QEntity *entity,
                                        QList<Qt3DRender::QObjectPicker *> *pickers);
-    void checkMultiSelectionHighlights(const QStringList &oldlist, const QStringList &newlist);
+    void checkMultiSelectionHighlights();
     QVector3D snapPosition(const QVector3D &worldPos, bool x, bool y, bool z);
+    Q_INVOKABLE void doEnsureSelection();
+    EditorSceneItem *itemByName(const QString &name);
+    void clearSingleSelection();
+
 private:
     Qt3DCore::QEntity *m_rootEntity;
     Qt3DCore::QEntity *m_componentCache;
@@ -400,6 +405,7 @@ private:
     EditorSceneItem *m_sceneEntityItem;
     Qt3DCore::QEntity *m_selectedEntity;
     Qt3DCore::QTransform *m_selectedEntityTransform;
+    QString m_ensureSelectionEntityName;
     bool m_cameraViewCenterSelected;
 
     QString m_errorString;
diff --git a/editorlib/src/editorsceneitemmodel.cpp b/editorlib/src/editorsceneitemmodel.cpp
index ae5a2da..d3912b3 100644
--- a/editorlib/src/editorsceneitemmodel.cpp
+++ b/editorlib/src/editorsceneitemmodel.cpp
@@ -195,7 +195,7 @@ void EditorSceneItemModel::handleImportEntityLoaderStatusChanged()
                     fixEntityNames(duplicate);
                 }
                 resetModel();
-                emit selectIndex(getModelIndexByName(m_sceneLoaderEntity->objectName()));
+                m_scene->setSelection(m_sceneLoaderEntity);
             } else if (sceneLoader->status() == Qt3DRender::QSceneLoader::Error) {
                 m_scene->setError(tr("Failed to import an Entity"));
             }
@@ -260,6 +260,8 @@ QHash<int, QByteArray> EditorSceneItemModel::roleNames() const
 
 void EditorSceneItemModel::resetModel()
 {
+    m_scene->ensureSelection();
+
     QAbstractItemModel::beginResetModel();
 
     // Reconnect all entities
@@ -274,10 +276,6 @@ void EditorSceneItemModel::resetModel()
         expandedIndexList.append(getModelIndexByName(entityName));
     emit expandItems(expandedIndexList);
 
-    // Select scene root after reset, unless multiselecting
-    if (!m_scene->multiSelection())
-        emit selectIndex(sceneEntityIndex());
-
     emit resetComplete();
 }
 
@@ -580,9 +578,6 @@ void EditorSceneItemModel::reparentEntity(const QModelIndex &newParentIndex,
         entityTransform->setMatrix(newMatrix);
 
     resetModel();
-
-    // Keep the moved item selected
-    emit selectIndex(getModelIndexByName(duplicate->objectName()));
 }
 
 void EditorSceneItemModel::addExpandedItem(const QModelIndex &index)
diff --git a/editorlib/src/editorsceneitemmodel.h b/editorlib/src/editorsceneitemmodel.h
index aef98e9..58bf43a 100644
--- a/editorlib/src/editorsceneitemmodel.h
+++ b/editorlib/src/editorsceneitemmodel.h
@@ -103,7 +103,6 @@ public:
 signals:
     void freeViewChanged(bool enabled);
     void expandItems(const QModelIndexList &items);
-    void selectIndex(const QModelIndex &selectIndex);
     void importEntityInProgressChanged(bool importInProgress);
     void resetComplete();
 
diff --git a/editorlib/src/undohandler/resetentitycommand.cpp b/editorlib/src/undohandler/resetentitycommand.cpp
index 536f15e..a7fb755 100644
--- a/editorlib/src/undohandler/resetentitycommand.cpp
+++ b/editorlib/src/undohandler/resetentitycommand.cpp
@@ -60,9 +60,8 @@ void ResetEntityCommand::undo()
         // Insert the old entity back
         QModelIndex parentIndex = m_sceneModel->getModelIndexByName(m_parentEntityName);
         m_sceneModel->insertExistingEntity(m_removedEntity, m_row, parentIndex);
+        m_sceneModel->scene()->setSelection(m_removedEntity);
         m_removedEntity = nullptr;
-
-        emit m_sceneModel->selectIndex(m_sceneModel->getModelIndexByName(m_entityName));
     }
 }
 
@@ -95,6 +94,5 @@ void ResetEntityCommand::redo()
             break;
         }
     }
-
-    emit m_sceneModel->selectIndex(m_sceneModel->getModelIndexByName(m_entityName));
+    m_sceneModel->scene()->setSelection(entity);
 }