Make moving rows asynchronous

This is the simplest way to ensure row indices stay in sync with
asset graph indices in cases where multiple children are added
to indices beyond the current child count (e.g. undoing a
multiselection delete/move).

This change also makes multiselection moves quite a bit more
efficient, though in release builds the effect is barely noticeable.

Also optimized a frequently used function getBindingForHandle to
first look for the binding using the m_handlesMap as pretty much
every time, except when called from onAssetCreated, the handle
can be found from m_handlesMap.

Task-number: QT3DS-1758
Change-Id: Ie95aef5f5f54c794db6a5d9c7515314b3182b65e
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>
Reviewed-by: Tomi Korpipää <tomi.korpipaa@qt.io>

4 files changed, 64 insertions, 73 deletions

diff --git a/src/Authoring/Studio/Palettes/Timeline/Bindings/Qt3DSDMTimelineItemBinding.cpp b/src/Authoring/Studio/Palettes/Timeline/Bindings/Qt3DSDMTimelineItemBinding.cpp
index 7b70458f..999062da 100644
--- a/src/Authoring/Studio/Palettes/Timeline/Bindings/Qt3DSDMTimelineItemBinding.cpp
+++ b/src/Authoring/Studio/Palettes/Timeline/Bindings/Qt3DSDMTimelineItemBinding.cpp
@@ -755,44 +755,22 @@ int Qt3DSDMTimelineItemBinding::getAnimatedPropertyIndex(int propertyHandle) con
     return -1;
 }
 
-// Converts global asset graph index of a child into local time context index or vice versa
-int Qt3DSDMTimelineItemBinding::convertIndex(int index, bool isAssetGraphIndex) const
-{
-    // If we are not a master slide object or scene root, global and local indexes always match
-    int retval = 0;
-    if (!IsMaster() && GetObjectType() != OBJTYPE_SCENE) {
-        retval = index;
-    } else {
-        qt3dsdm::Qt3DSDMInstanceHandle instance = GetInstance();
-        if (instance.Valid()) {
-            Q3DStudio::CGraphIterator currentChildren;
-            Qt3DSDMSlideHandle activeSlide = m_TransMgr->GetDoc()->GetActiveSlide();
-            GetAssetChildrenInTimeParent(instance, m_TransMgr->GetDoc(), AmITimeParent(),
-                                         currentChildren, activeSlide);
-
-            Q3DStudio::CGraphIterator allChildren;
-            GetAssetChildren(m_TransMgr->GetDoc(), instance, allChildren);
-
-            // Compare children to adjust the index
-            size_t skip = 0;
-            size_t maxCurrentIndex = currentChildren.GetCount() - 1;
-            size_t count = qMin(allChildren.GetCount(), size_t(index));
-            for (size_t current = 0; current < count; ++current) {
-                size_t skipIndex = current - skip;
-                if (skipIndex > maxCurrentIndex || allChildren.GetResult(current)
-                        != currentChildren.GetResult(skipIndex)) {
-                    ++skip;
-                }
-            }
-            retval = index;
-            if (isAssetGraphIndex)
-                retval -= int(skip);
-            else
-                retval += int(skip);
+void Qt3DSDMTimelineItemBinding::getTimeContextIndices(const QSet<int> &children,
+                                                       QMap<int, int> &indexMap)
+{
+    qt3dsdm::Qt3DSDMInstanceHandle instance = GetInstance();
+    if (instance.Valid()) {
+        Q3DStudio::CGraphIterator graphChildren;
+        Qt3DSDMSlideHandle activeSlide = m_TransMgr->GetDoc()->GetActiveSlide();
+        GetAssetChildrenInTimeParent(instance, m_TransMgr->GetDoc(), AmITimeParent(),
+                                     graphChildren, activeSlide);
+        const size_t count = graphChildren.GetCount();
+        for (size_t current = 0; current < count; ++current) {
+            auto handle = graphChildren.GetResult(current);
+            if (children.contains(handle))
+                indexMap.insert(current, int(handle));
         }
     }
-
-    return retval;
 }
 
 void Qt3DSDMTimelineItemBinding::InsertKeyframe()
diff --git a/src/Authoring/Studio/Palettes/Timeline/Bindings/Qt3DSDMTimelineItemBinding.h b/src/Authoring/Studio/Palettes/Timeline/Bindings/Qt3DSDMTimelineItemBinding.h
index 7852719f..9d14cc84 100644
--- a/src/Authoring/Studio/Palettes/Timeline/Bindings/Qt3DSDMTimelineItemBinding.h
+++ b/src/Authoring/Studio/Palettes/Timeline/Bindings/Qt3DSDMTimelineItemBinding.h
@@ -210,7 +210,7 @@ public:
     virtual qt3dsdm::Qt3DSDMInstanceHandle GetInstance() const;
 
     int getAnimatedPropertyIndex(int propertyHandle) const;
-    int convertIndex(int index, bool isAssetGraphIndex) const;
+    void getTimeContextIndices(const QSet<int> &children, QMap<int ,int> &indexMap);
 
     ITimelineItemProperty *GetOrCreatePropertyBinding(qt3dsdm::Qt3DSDMPropertyHandle inPropertyHandle);
     ITimelineItemProperty *GetPropertyBinding(qt3dsdm::Qt3DSDMPropertyHandle inPropertyHandle);
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
index c204b280..eb7f3553 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
@@ -683,6 +683,41 @@ void TimelineWidget::onAsyncUpdate()
         m_fullReconstruct = false;
         onSelectionChange(doc->GetSelectedValue());
     } else {
+        if (!m_moveMap.isEmpty()) {
+            // Flip the hash around so that we collect moves by parent.
+            // We can't do this with m_moveMap originally, as things break if
+            // same row receives consecutive moves to different parents.
+            QMultiHash<int, int> flippedMap;
+            QHashIterator<int, int> it(m_moveMap);
+            while (it.hasNext()) {
+                it.next();
+                flippedMap.insert(it.value(), it.key());
+            }
+            const auto parentHandles = flippedMap.keys();
+            for (const auto parentHandle : parentHandles) {
+                QSet<int> movedInstances(flippedMap.values(parentHandle).toSet());
+                RowTree *rowParent = m_handlesMap.value(parentHandle);
+                if (rowParent) {
+                    Qt3DSDMTimelineItemBinding *bindingParent
+                            = static_cast<Qt3DSDMTimelineItemBinding *>(rowParent->getBinding());
+                    if (bindingParent) {
+                        // Resolve indexes for handles. QMap used for its automatic sorting by keys.
+                        QMap<int, int> indexMap;
+                        bindingParent->getTimeContextIndices(movedInstances, indexMap);
+                        QMapIterator<int, int> indexIt(indexMap);
+                        while (indexIt.hasNext()) {
+                            indexIt.next();
+                            RowTree *row = m_handlesMap.value(indexIt.value());
+                            if (row)
+                                rowParent->addChildAt(row, indexIt.key());
+                        }
+                    }
+                }
+            }
+            // Make sure selections on UI matches bindings
+            CDoc *doc = g_StudioApp.GetCore()->GetDoc();
+            onSelectionChange(doc->GetSelectedValue());
+        }
         // Update properties
         if (!m_dirtyProperties.isEmpty()) {
             const SDataModelSceneAsset &asset = m_bridge->GetSceneAsset();
@@ -718,6 +753,7 @@ void TimelineWidget::onAsyncUpdate()
         }
     }
     m_dirtyProperties.clear();
+    m_moveMap.clear();
 }
 
 void TimelineWidget::onActionEvent(qt3dsdm::Qt3DSDMActionHandle inAction,
@@ -776,33 +812,15 @@ void TimelineWidget::onPropertyUnlinked(qt3dsdm::Qt3DSDMInstanceHandle inInstanc
 
 void TimelineWidget::onChildAdded(int inParent, int inChild, long inIndex)
 {
+    Q_UNUSED(inIndex)
+
     if (m_fullReconstruct)
         return;
 
-    Qt3DSDMTimelineItemBinding *binding = getBindingForHandle(inChild, m_binding);
-    Qt3DSDMTimelineItemBinding *bindingParent = getBindingForHandle(inParent, m_binding);
-
-    if (binding && bindingParent) {
-        int convertedIndex = bindingParent->convertIndex(inIndex, true);
-        RowTree *row = binding->getRowTree();
-        RowTree *rowParent = bindingParent->getRowTree();
-        if (row && rowParent) {
-            // Row already exists in bindings, just add it into UI
-            // (This happens e.g. when changing material row type)
-            rowParent->addChildAt(row, convertedIndex);
-        } else if (rowParent && !row) {
-            // Parent exists but row not, so create new row.
-            // (This happens e.g. when deleting a row -> undo)
-            if (m_bridge->IsSceneGraphInstance(inChild)) {
-                m_graphicsScene->rowManager()
-                        ->createRowFromBinding(binding, rowParent, convertedIndex);
-                insertToHandlesMap(binding);
-            }
-        }
-        // Make sure selections on UI matches bindings
-        CDoc *doc = g_StudioApp.GetCore()->GetDoc();
-        onSelectionChange(doc->GetSelectedValue());
-    }
+    // Handle row moves asynch, as we won't be able to get the final order correct otherwise
+    m_moveMap.insert(inChild, inParent);
+    if (!m_asyncUpdateTimer.isActive())
+        m_asyncUpdateTimer.start();
 }
 
 void TimelineWidget::onChildRemoved(int inParent, int inChild, long inIndex)
@@ -816,20 +834,10 @@ void TimelineWidget::onChildRemoved(int inParent, int inChild, long inIndex)
 void TimelineWidget::onChildMoved(int inParent, int inChild, long inOldIndex,
                                   long inNewIndex)
 {
-    if (m_fullReconstruct)
-        return;
-
     Q_UNUSED(inOldIndex)
 
-    Qt3DSDMTimelineItemBinding *binding = getBindingForHandle(inChild, m_binding);
-    Qt3DSDMTimelineItemBinding *bindingParent = getBindingForHandle(inParent, m_binding);
-
-    if (binding && bindingParent) {
-        RowTree *row = binding->getRowTree();
-        RowTree *rowParent = bindingParent->getRowTree();
-        if (row && rowParent)
-            rowParent->addChildAt(row, bindingParent->convertIndex(inNewIndex, true));
-    }
+    // Move and add are essentially the same operation
+    onChildAdded(inParent, inChild, inNewIndex);
 }
 
 void TimelineWidget::OnDraw(CRenderer *inRenderer, CRct &inDirtyRect, bool inIgnoreValidation)
@@ -918,6 +926,10 @@ void TimelineWidget::enableDnD(bool b)
 Qt3DSDMTimelineItemBinding *TimelineWidget::getBindingForHandle(int handle,
                                                 Qt3DSDMTimelineItemBinding *binding) const
 {
+    const RowTree *row = m_handlesMap.value(handle);
+    if (row && row->getBinding())
+        return static_cast<Qt3DSDMTimelineItemBinding *>(row->getBinding());
+
     if (binding) {
         if (binding->GetInstance().GetHandleValue() == handle)
             return binding;
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.h b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.h
index e5bfb1ef..d34d0ec7 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.h
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.h
@@ -153,6 +153,7 @@ private:
     bool m_splitterPressed = false;
     QSize m_preferredSize;
     QMultiHash<qt3dsdm::Qt3DSDMInstanceHandle, qt3dsdm::Qt3DSDMPropertyHandle> m_dirtyProperties;
+    QHash<int, int> m_moveMap;
     QTimer m_asyncUpdateTimer;
     bool m_fullReconstruct = false;
     CClientDataModelBridge *m_bridge = nullptr;