Fix moving keyframe snap accuracy

Instead of using delta x for moving keyframes which loses accuracy
during snapping, use target x directly.

Task-number: QT3DS-2937
Change-Id: I5324bde9406d594cc926f777649968858d57af02
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>
Reviewed-by: Tomi Korpipää <tomi.korpipaa@qt.io>

5 files changed, 46 insertions, 22 deletions

diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/KeyframeManager.cpp b/src/Authoring/Studio/Palettes/TimelineGraphicsView/KeyframeManager.cpp
index dbd21279..c495b099 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/KeyframeManager.cpp
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/KeyframeManager.cpp
@@ -52,7 +52,6 @@
 #include "StudioPreferences.h"
 #include "Qt3DSDMAnimation.h"
 #include "Dialogs.h"
-#include "TimeEditDlg.h"
 #include "TimeEnums.h"
 #include "Bindings/PasteKeyframesCommandHelper.h"
 
@@ -371,6 +370,31 @@ void KeyframeManager::pasteKeyframes()
     }
 }
 
+void KeyframeManager::moveSelectedKeyframesTo(Keyframe *pressedKeyframe, double newX)
+{
+    long t = m_scene->ruler()->distanceToTime(newX);
+
+    // make sure the min-time keyframe doesn't go below zero
+    long minTime = LONG_MAX;
+    for (auto keyframe : qAsConst(m_selectedKeyframes)) {
+        if (keyframe->time < minTime)
+            minTime = keyframe->time;
+    }
+
+    if (pressedKeyframe->time - minTime > t)
+        t = pressedKeyframe->time - minTime;
+
+    for (auto keyframe : qAsConst(m_selectedKeyframes)) {
+        if (keyframe != pressedKeyframe)
+            keyframe->time = t - (pressedKeyframe->time - keyframe->time);
+    }
+    pressedKeyframe->time = t;
+
+    for (auto row : qAsConst(m_selectedKeyframesMasterRows))
+        row->updateKeyframes();
+}
+
+// TODO: update TimeEditDlg.cpp to use the method above and remove this one
 void KeyframeManager::moveSelectedKeyframes(double dx)
 {
     long dt = m_scene->ruler()->distanceToTime(dx);
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/KeyframeManager.h b/src/Authoring/Studio/Palettes/TimelineGraphicsView/KeyframeManager.h
index 0e3f8aaa..4b6d8f80 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/KeyframeManager.h
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/KeyframeManager.h
@@ -64,6 +64,7 @@ public:
     void copySelectedKeyframes();
     void pasteKeyframes();
     void moveSelectedKeyframes(double dx);
+    void moveSelectedKeyframesTo(Keyframe *pressedKeyframe, double newX);
     void commitMoveSelectedKeyframes();
     bool deleteSelectedKeyframes();
     bool oneMasterRowSelected() const;
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.cpp b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.cpp
index c05056f7..1f09f899 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.cpp
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.cpp
@@ -50,7 +50,6 @@
 #include "RowTreeContextMenu.h"
 #include "RowTimelineContextMenu.h"
 #include "StudioPreferences.h"
-#include "TimeEditDlg.h"
 #include "TimeEnums.h"
 #include "StudioClipboard.h"
 #include "Dialogs.h"
@@ -548,7 +547,7 @@ void TimelineGraphicsScene::mousePressEvent(QGraphicsSceneMouseEvent *event)
                         m_pressPosInKeyframe = (m_pressPos.x() - m_ruler->x())
                                 - (TimelineConstants::RULER_EDGE_OFFSET
                                    + m_ruler->timeToDistance(keyframe->time));
-                        m_keyframePressed = true;
+                        m_pressedKeyframe = keyframe;
                     }
                 } else {
                     m_keyframeManager->deselectAllKeyframes();
@@ -632,16 +631,16 @@ void TimelineGraphicsScene::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
                 m_rowMover->updateTargetRow(event->scenePos());
                 updateAutoScrolling(event->scenePos().y());
             }
-        } else if (m_keyframePressed) { // moving selected keyframes
-            double newX = event->scenePos().x() - m_ruler->x() - m_pressPosInKeyframe;
+        } else if (m_pressedKeyframe) { // moving selected keyframes
+            double newX = event->scenePos().x() - m_ruler->x()
+                          - TimelineConstants::RULER_EDGE_OFFSET - m_pressPosInKeyframe;
 
-            if (newX < TimelineConstants::RULER_EDGE_OFFSET)
-                newX = TimelineConstants::RULER_EDGE_OFFSET;
+            if (newX < 0)
+                newX = 0;
             if (shift)
                 snap(newX);
-            newX += m_ruler->x() + m_pressPosInKeyframe;
-            double dx = newX - m_pressPos.x();
-            m_keyframeManager->moveSelectedKeyframes(dx);
+
+            m_keyframeManager->moveSelectedKeyframesTo(m_pressedKeyframe, newX);
 
             m_pressPos.setX(newX);
         }
@@ -696,18 +695,20 @@ void TimelineGraphicsScene::updateSnapSteps()
         RowTree *rowTree = static_cast<RowTree *>
                 (m_layoutTree->itemAt(i)->graphicsItem());
         if (rowTree->hasDurationBar() && rowTree->isVisible()) {
-            if (!m_snapSteps.contains(rowTree->rowTimeline()->getStartX()))
-                m_snapSteps.push_back(rowTree->rowTimeline()->getStartX());
+            auto startX = rowTree->rowTimeline()->getStartX()
+                          - TimelineConstants::RULER_EDGE_OFFSET;
+            if (!m_snapSteps.contains(startX))
+                m_snapSteps.push_back(startX);
 
-            if (!m_snapSteps.contains(rowTree->rowTimeline()->getEndX()))
-                m_snapSteps.push_back(rowTree->rowTimeline()->getEndX());
+            auto endX = rowTree->rowTimeline()->getEndX() - TimelineConstants::RULER_EDGE_OFFSET;
+            if (!m_snapSteps.contains(endX))
+                m_snapSteps.push_back(endX);
 
             // add keyframes times
             if (rowTree->hasPropertyChildren()) {
                 const QList<Keyframe *> keyframes = rowTree->rowTimeline()->keyframes();
                 for (Keyframe *k : keyframes) {
-                    double kX = m_ruler->timeToDistance(k->time)
-                            + TimelineConstants::RULER_EDGE_OFFSET;
+                    double kX = m_ruler->timeToDistance(k->time);
                     if (!m_snapSteps.contains(kX))
                         m_snapSteps.push_back(kX);
                 }
@@ -731,7 +732,7 @@ void TimelineGraphicsScene::resetMousePressParams()
     m_timelinePanning = false;
     m_startRowMoverOnNextDrag = false;
     m_rulerPressed = false;
-    m_keyframePressed = false;
+    m_pressedKeyframe = nullptr;
     m_clickedTimelineControlType = TimelineControlType::None;
     m_editedTimelineRow.clear();
     m_releaseSelectRow.clear();
@@ -776,8 +777,7 @@ void TimelineGraphicsScene::snap(double &value, bool snapToPlayHead)
         else if (CStudioPreferences::GetTimelineSnappingGridResolution() == SNAPGRID_TICKMARKS)
             snapStep *= .1;
 
-        double snapValue = TimelineConstants::RULER_EDGE_OFFSET
-            + round((value - TimelineConstants::RULER_EDGE_OFFSET) / snapStep) * snapStep;
+        double snapValue = round(value / snapStep) * snapStep;
         if (abs(value - snapValue) < CStudioPreferences::GetSnapRange())
             value = snapValue;
     }
@@ -789,7 +789,7 @@ void TimelineGraphicsScene::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
         if (m_dragging) {
             if (m_rowMover->isActive()) { // moving rows (reorder/reparent)
                 commitMoveRows();
-            } else if (m_keyframePressed) {
+            } else if (m_pressedKeyframe) {
                 // update keyframe movement (time) to binding
                 m_keyframeManager->commitMoveSelectedKeyframes();
             } else if (m_clickedTimelineControlType == TimelineControlType::StartHandle) {
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.h b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.h
index e8eda8f3..66192b3e 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.h
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineGraphicsScene.h
@@ -146,7 +146,7 @@ private:
                                                         // rowtree
 
     bool m_rulerPressed = false;
-    bool m_keyframePressed = false;
+    Keyframe *m_pressedKeyframe = nullptr;
     bool m_dragging = false;
     bool m_startRowMoverOnNextDrag = false;
     bool m_timelineZooming = false;
diff --git a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
index 81ff4d36..c745ff6c 100644
--- a/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
+++ b/src/Authoring/Studio/Palettes/TimelineGraphicsView/TimelineWidget.cpp
@@ -52,7 +52,6 @@
 #include "Bindings/Qt3DSDMTimelineItemBinding.h"
 #include "Bindings/Qt3DSDMTimelineItemProperty.h"
 #include "Bindings/TimelineBreadCrumbProvider.h"
-#include "TimeEditDlg.h"
 #include "IDocumentEditor.h"
 #include "Control.h"
 #include "TimelineDropTarget.h"