Refactor row moving

Use RearrangeRows instead of just moving a row based on index.
This way we don't need to deal with row indexes at all, reducing
risks related to calculating them incorrectly.

Change-Id: Ida11b6b48c0809f7a80c361e8f9dd811ca3f5d0f
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>
Reviewed-by: Tomi Korpipää <tomi.korpipaa@qt.io>

7 files changed, 68 insertions, 87 deletions

diff --git a/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp b/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp
index 815d90a9..a2c7255a 100644
--- a/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp
+++ b/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp
@@ -2333,18 +2333,6 @@ public:
         }
     }
 
-    void ReorderRows(qt3dsdm::Qt3DSDMInstanceHandle handleSource,
-                     qt3dsdm::Qt3DSDMInstanceHandle handleParent, int index) override
-    {
-        CString currName = m_Bridge.GetName(handleSource);
-        if (!m_Bridge.CheckNameUnique(handleParent, handleSource, currName)) {
-            CString newName = m_Bridge.GetUniqueChildName(handleParent, handleSource, currName);
-            m_Doc.getMoveRenameHandler()->displayMessageBox(currName, newName);
-            SetName(handleSource, newName);
-        }
-        m_AssetGraph.MoveTo(handleSource, handleParent, index);
-    }
-
     Qt3DSDMInstanceHandle MakeComponent(const qt3dsdm::TInstanceHandleList &inInstances) override
     {
         if (inInstances.empty())
diff --git a/src/Authoring/Client/Code/Core/Doc/IDocumentEditor.h b/src/Authoring/Client/Code/Core/Doc/IDocumentEditor.h
index 694267a7..c1bf158f 100644
--- a/src/Authoring/Client/Code/Core/Doc/IDocumentEditor.h
+++ b/src/Authoring/Client/Code/Core/Doc/IDocumentEditor.h
@@ -284,9 +284,6 @@ public:
         RearrangeObjects(theInstances, inDest, inInsertType);
     }
 
-    virtual void ReorderRows(qt3dsdm::Qt3DSDMInstanceHandle handleSource,
-                             qt3dsdm::Qt3DSDMInstanceHandle handleParent, int index) = 0;
-
     // Returns the new component.
     virtual TInstanceHandle MakeComponent(const qt3dsdm::TInstanceHandleList &inInstances) = 0;
 
diff --git a/src/Authoring/Common/Code/Graph/Graph.cpp b/src/Authoring/Common/Code/Graph/Graph.cpp
index 487f003b..75d6ee27 100644
--- a/src/Authoring/Common/Code/Graph/Graph.cpp
+++ b/src/Authoring/Common/Code/Graph/Graph.cpp
@@ -424,14 +424,11 @@ public:
     void MoveTo(const TIdentifier inNode, const TIdentifier inNewParent,
                         const COpaquePosition &inPosition) override
     {
-          // Mahmoud_TODO: will be eventually removed
-//        if (GetParent(inNode) != inNewParent
-//            || !(GetNodePosition(inNode) == GetChildCount(inNewParent) - 1)){
-//            RemoveChild(inNode, false);
-//            AddChild(inNewParent, inNode, inPosition);
-//        }
-        RemoveChild(inNode, false);
-        AddChild(inNewParent, inNode, inPosition);
+        if (GetParent(inNode) != inNewParent
+                || !(GetNodePosition(inNode) == GetChildCount(inNewParent) - 1)) {
+            RemoveChild(inNode, false);
+            AddChild(inNewParent, inNode, inPosition);
+        }
     }
 
     void InsertTo(const TIdentifier inNode, const TIdentifier inNewParent,
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/RowMover.cpp b/src/Authoring/Studio/Palettes/TimelineGraphicsView/RowMover.cpp
index 892fc721..7861023c 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/RowMover.cpp
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/RowMover.cpp
@@ -58,29 +58,31 @@ void RowMover::paint(QPainter *painter, const QStyleOptionGraphicsItem *option,
     painter->restore();
 }
 
+RowTree *RowMover::insertionTarget() const
+{
+    return m_insertionTarget;
+}
+
 RowTree *RowMover::insertionParent() const
 {
     return m_insertionParent;
 }
 
-void RowMover::resetInsertionParent(RowTree *newTarget)
+void RowMover::resetInsertionParent(RowTree *newParent)
 {
     if (m_insertionParent) {
         m_insertionParent->setMoveTarget(false);
         m_insertionParent = nullptr;
     }
 
-    if (newTarget) {
-        m_insertionParent = newTarget;
+    if (newParent) {
+        m_insertionParent = newParent;
         m_insertionParent->setMoveTarget(true);
+    } else {
+        m_insertionTarget = nullptr;
     }
 }
 
-int RowMover::targetIndex() const
-{
-    return m_targetIndex;
-}
-
 RowTree *RowMover::sourceRow() const
 {
     return m_sourceRow;
@@ -119,10 +121,8 @@ void RowMover::end(bool force)
     }
 }
 
-void RowMover::updateState(int index, int depth, double y)
+void RowMover::updateState(int depth, double y)
 {
-    m_targetIndex = index;
-
     setPos(25 + depth * TimelineConstants::ROW_DEPTH_STEP, y);
     setVisible(true);
 }
@@ -213,28 +213,28 @@ void RowMover::updateTargetRow(const QPointF &scenePos)
             valid = false; // don't insert master slide object into non-master slide object
 
         if (valid) {
-            // calc insertion index
-            int index = rowInsert1->index() + 1;
-            if (rowInsert1->isProperty() && depth == rowInsert1->depth()) {
-                index = 0;
-            } else if (rowInsert1 == insertParent) {
-                if (insertParent->expanded() || insertParent->childRows().empty())
-                    index = 0;
-                else
-                    index = insertParent->childRows().last()->index() + 1;
-            } else if (depth < rowInsert1->depth()) {
-                RowTree *row = rowInsert1;
-                for (int i = depth; i < rowInsert1->depth(); ++i)
-                    row = row->parentRow();
-                index = row->index() + 1;
-            }
-
-            if (m_sourceRow && m_sourceRow->parentRow() == insertParent
-                    && index > m_sourceRow->index()) { // moving down under the same parent
-                index--; // m_sourceRow is removed from layout, shift index up by 1
+            // calc insertion target and type
+            if (rowInsert1 == m_insertionParent) {
+                if (m_insertionParent->expanded() && !m_insertionParent->childRows().empty()) {
+                    m_insertionTarget = m_insertionParent->childRows().at(0);
+                    m_insertType = Q3DStudio::DocumentEditorInsertType::PreviousSibling;
+                } else {
+                    m_insertionTarget = m_insertionParent;
+                    m_insertType = Q3DStudio::DocumentEditorInsertType::LastChild;
+                }
+            } else if (rowInsert1->isProperty() && depth == rowInsert1->depth()) {
+                m_insertionTarget = m_insertionParent->childRows().at(0);
+                m_insertType = Q3DStudio::DocumentEditorInsertType::PreviousSibling;
+            } else {
+                m_insertionTarget = rowInsert1;
+                m_insertType = Q3DStudio::DocumentEditorInsertType::NextSibling;
+                if (depth < rowInsert1->depth()) {
+                    for (int i = depth; i < rowInsert1->depth(); ++i)
+                        m_insertionTarget = m_insertionTarget->parentRow();
+                }
             }
 
-            updateState(index, depth, rowInsert1->y() + TimelineConstants::ROW_H);
+            updateState(depth, rowInsert1->y() + TimelineConstants::ROW_H);
         }
     }
 
@@ -269,3 +269,8 @@ int RowMover::type() const
     // Enable the use of qgraphicsitem_cast with this item.
     return TypeRowMover;
 }
+
+Q3DStudio::DocumentEditorInsertType::Enum RowMover::insertionType() const
+{
+    return m_insertType;
+}
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/RowMover.h b/src/Authoring/Studio/Palettes/TimelineGraphicsView/RowMover.h
index 71d8fdf5..c8659abb 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/RowMover.h
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/RowMover.h
@@ -31,6 +31,7 @@
 
 #include "TimelineConstants.h"
 #include "TimelineItem.h"
+#include "DocumentEditorEnumerations.h"
 
 class RowTree;
 class TimelineGraphicsScene;
@@ -45,23 +46,26 @@ public:
     void start(RowTree *row = nullptr);
     void end(bool force = false);
     void updateTargetRow(const QPointF &scenePos);
-    int targetIndex() const;
     bool isActive();
     bool validLayerMove(RowTree *rowAtIndex, RowTree *nextRowAtIndex);
+    RowTree *insertionTarget() const;
     RowTree *insertionParent() const;
     RowTree *sourceRow() const;
     int type() const;
+    Q3DStudio::DocumentEditorInsertType::Enum insertionType() const;
 
 private:
-    void updateState(int index, int depth, double y);
-    void resetInsertionParent(RowTree *newTarget = nullptr);
+    void updateState(int depth, double y);
+    void resetInsertionParent(RowTree *newParent = nullptr);
     RowTree *getRowAtPos(const QPointF &scenePos);
 
     TimelineGraphicsScene *m_scene = nullptr;
+    RowTree *m_insertionTarget = nullptr; // insertion target
     RowTree *m_insertionParent = nullptr; // insertion parent
     RowTree *m_sourceRow = nullptr;       // dragged row
-    int m_targetIndex = -1;               // insertion index
     bool m_active = false;
+    Q3DStudio::DocumentEditorInsertType::Enum m_insertType =
+            Q3DStudio::DocumentEditorInsertType::Unknown;
 };
 
 #endif // ROWMOVER_H
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.cpp b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.cpp
index 1ec452ae..581a41b6 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.cpp
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.cpp
@@ -243,29 +243,22 @@ void printAsset(Q3DStudio::TIdentifier asset, QString padding = " ")
 
 void TimelineGraphicsScene::commitMoveRows()
 {
-    // same place, abort
-    if ((m_rowMover->sourceRow()->index() == m_rowMover->targetIndex())
-        && m_rowMover->sourceRow()->parentRow() == m_rowMover->insertionParent()) {
+    if (!m_rowMover->insertionTarget() || m_rowMover->sourceRow() == m_rowMover->insertionTarget())
         return;
-    }
 
     // handle for the moving row
     qt3dsdm::Qt3DSDMInstanceHandle handleSource = static_cast<Qt3DSDMTimelineItemBinding *>
             (m_rowMover->sourceRow()->getBinding())->GetInstance();
-
-    // handle for the parent of the insertion row
-    auto parentBinding = static_cast<Qt3DSDMTimelineItemBinding *>
-            (m_rowMover->insertionParent()->getBinding());
-    qt3dsdm::Qt3DSDMInstanceHandle handleParent = parentBinding->GetInstance();
-
-    // commit the row move to the binding
-    Q3DStudio::SCOPED_DOCUMENT_EDITOR(*g_StudioApp.GetCore()->GetDoc(), QObject::tr("Reorder Rows"))
-        ->ReorderRows(handleSource, handleParent,
-                      parentBinding->convertIndex(m_rowMover->targetIndex(), false));
+    qt3dsdm::Qt3DSDMInstanceHandle handleTarget = static_cast<Qt3DSDMTimelineItemBinding *>
+            (m_rowMover->insertionTarget()->getBinding())->GetInstance();
 
     if (!m_rowMover->insertionParent()->expanded())
         m_rowMover->insertionParent()->updateExpandStatus(RowTree::ExpandState::Expanded, false);
 
+    Q3DStudio::SCOPED_DOCUMENT_EDITOR(*g_StudioApp.GetCore()->GetDoc(),
+                                      QObject::tr("Move Rows"))
+            ->RearrangeObject(handleSource, handleTarget, m_rowMover->insertionType());
+
     // updating the UI happens in TimelineWidget.onChildAdded()
 }
 
@@ -542,8 +535,7 @@ void TimelineGraphicsScene::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
             }
 
         } else if (m_rowMover->isActive()) { // moving rows (reorder/reparent)
-            if (m_rowMover->insertionParent()) // valid row move, commit it
-                commitMoveRows();
+            commitMoveRows();
         } else if (m_keyframePressed) {
             // update keyframe movement (time) to binding
             m_keyframeManager->commitMoveSelectedKeyframes();
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
index 2319970e..5801ab7a 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
@@ -691,20 +691,18 @@ CDropTarget *TimelineWidget::FindDropCandidate(CPt &inMousePoint, Qt::KeyboardMo
     RowMover *mover = m_graphicsScene->rowMover();
     mover->updateTargetRow(QPointF(inMousePoint.x, mouseY));
 
-    if (mover->insertionParent()) {
-        RowTree *targetRow = mover->insertionParent()->getChildAt(mover->targetIndex()-1);
-
-        if (targetRow) {
+    if (mover->insertionTarget()) {
+        mover->insertionTarget()->getBinding()->SetDropTarget(theTarget);
+        switch (mover->insertionType()) {
+        case Q3DStudio::DocumentEditorInsertType::LastChild:
+            theTarget->SetDestination(EDROPDESTINATION_ON);
+            break;
+        case Q3DStudio::DocumentEditorInsertType::PreviousSibling:
+            theTarget->SetDestination(EDROPDESTINATION_ABOVE);
+            break;
+        default:
             theTarget->SetDestination(EDROPDESTINATION_BELOW);
-            targetRow->getBinding()->SetDropTarget(theTarget);
-        } else {
-            if (mover->insertionParent()->childRows().empty()) {
-                theTarget->SetDestination(EDROPDESTINATION_ON);
-                mover->insertionParent()->getBinding()->SetDropTarget(theTarget);
-            } else {
-                theTarget->SetDestination(EDROPDESTINATION_ABOVE);
-                mover->insertionParent()->getChildAt(0)->getBinding()->SetDropTarget(theTarget);
-            }
+            break;
         }
     }