Improve stability of the curve editor

- Clear the CurveEditor when deleting a timeline
- Prevent value-axis computation with invalid values
- Prevent invalid values when animation curves contain one keyframe only
- Set the dirty flag when deleting a keyframe in order to properly
  delete it
- Fix autoscrolling for the timeline editor

Fixes: QDS-4115
Fixes: QDS-4081
Fixes: QDS-4080
Fixes: QDS-3251
Change-Id: I3bc8406ac57f30b16bccc2e1c164a84502de7750
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

src/plugins/qmldesigner/components/curveeditor/animationcurve.cpp

@@ -93,6 +93,11 @@ AnimationCurve::AnimationCurve(const QEasingCurve &easing, const QPointF &start,
     analyze();
 }
 
+bool AnimationCurve::isEmpty() const
+{
+    return m_frames.empty();
+}
+
 bool AnimationCurve::isValid() const
 {
     return m_frames.size() >= 2;
@@ -352,45 +357,43 @@ void AnimationCurve::insert(double time)
 
 void AnimationCurve::analyze()
 {
-    if (isValid()) {
+    m_minY = std::numeric_limits<double>::max();
-        m_minY = std::numeric_limits<double>::max();
+    m_maxY = std::numeric_limits<double>::lowest();
-        m_maxY = std::numeric_limits<double>::lowest();
 
-        auto byTime = [](const auto &a, const auto &b) {
+    auto byTime = [](const auto &a, const auto &b) {
-            return a.position().x() < b.position().x();
+        return a.position().x() < b.position().x();
-        };
+    };
-        std::sort(m_frames.begin(), m_frames.end(), byTime);
+    std::sort(m_frames.begin(), m_frames.end(), byTime);
 
-        for (auto e : extrema()) {
+    for (auto e : extrema()) {
-            if (m_minY > e.y())
+        if (m_minY > e.y())
-                m_minY = e.y();
+            m_minY = e.y();
 
-            if (m_maxY < e.y())
+        if (m_maxY < e.y())
-                m_maxY = e.y();
+            m_maxY = e.y();
+    }
+
+    for (auto &frame : qAsConst(m_frames)) {
+        if (frame.position().y() < m_minY)
+            m_minY = frame.position().y();
+
+        if (frame.position().y() > m_maxY)
+            m_maxY = frame.position().y();
+
+        if (frame.hasLeftHandle()) {
+            if (frame.leftHandle().y() < m_minY)
+                m_minY = frame.leftHandle().y();
+
+            if (frame.leftHandle().y() > m_maxY)
+                m_maxY = frame.leftHandle().y();
         }
 
-        for (auto &frame : qAsConst(m_frames)) {
+        if (frame.hasRightHandle()) {
-            if (frame.position().y() < m_minY)
+            if (frame.rightHandle().y() < m_minY)
-                m_minY = frame.position().y();
+                m_minY = frame.rightHandle().y();
 
-            if (frame.position().y() > m_maxY)
+            if (frame.rightHandle().y() > m_maxY)
-                m_maxY = frame.position().y();
+                m_maxY = frame.rightHandle().y();
-
-            if (frame.hasLeftHandle()) {
-                if (frame.leftHandle().y() < m_minY)
-                    m_minY = frame.leftHandle().y();
-
-                if (frame.leftHandle().y() > m_maxY)
-                    m_maxY = frame.leftHandle().y();
-            }
-
-            if (frame.hasRightHandle()) {
-                if (frame.rightHandle().y() < m_minY)
-                    m_minY = frame.rightHandle().y();
-
-                if (frame.rightHandle().y() > m_maxY)
-                    m_maxY = frame.rightHandle().y();
-            }
         }
     }
 }

src/plugins/qmldesigner/components/curveeditor/animationcurve.h

@@ -46,6 +46,8 @@ public:
 
     AnimationCurve(const QEasingCurve &easing, const QPointF &start, const QPointF &end);
 
+    bool isEmpty() const;
+
     bool isValid() const;
 
     bool isFromData() const;

src/plugins/qmldesigner/components/curveeditor/curveeditormodel.cpp

@@ -255,7 +255,7 @@ TreeItem *CurveEditorModel::createTopLevelItem(const QmlDesigner::QmlTimeline &t
     for (auto &&grp : timeline.keyframeGroupsForTarget(node)) {
         if (grp.isValid()) {
             AnimationCurve curve = createAnimationCurve(grp);
-            if (curve.isValid()) {
+            if (!curve.isEmpty()) {
                 QString name = QString::fromUtf8(grp.propertyName());
                 auto propertyItem = new PropertyTreeItem(name, curve, typeFrom(grp));
 

src/plugins/qmldesigner/components/curveeditor/curveeditorview.cpp

@@ -101,6 +101,9 @@ void CurveEditorView::nodeRemoved(const ModelNode &removedNode,
     ModelNode parent = parentProperty.parentModelNode();
     if (dirtyfiesView(parent))
         updateKeyframes();
+
+    if (!activeTimeline().isValid())
+        m_model->reset({});
 }
 
 void CurveEditorView::nodeReparented(const ModelNode &node,

src/plugins/qmldesigner/components/curveeditor/detail/curveitem.cpp

@@ -292,7 +292,7 @@ std::vector<AnimationCurve> CurveItem::curves() const
         }
     }
 
-    if (tmp.size() >= 2)
+    if (!tmp.empty())
         out.push_back(AnimationCurve(tmp));
 
     return out;
@@ -372,6 +372,9 @@ void CurveItem::restore()
         currItem->setInterpolation(segment.interpolation());
         prevItem = currItem;
     }
+
+    // It is now the last item.
+    prevItem->setRightHandle(QPointF());
 }
 
 void CurveItem::setDirty(bool dirty)
@@ -404,12 +407,12 @@ void CurveItem::setCurve(const AnimationCurve &curve)
         item->setLocked(locked());
         item->setComponentTransform(m_transform);
         m_keyframes.push_back(item);
-        QObject::connect(item, &KeyframeItem::redrawCurve, this, &CurveItem::emitCurveChanged);
+        QObject::connect(item, &KeyframeItem::redrawCurve, this, &CurveItem::markDirty);
         QObject::connect(item, &KeyframeItem::keyframeMoved, this, &CurveItem::keyframeMoved);
         QObject::connect(item, &KeyframeItem::handleMoved, this, &CurveItem::handleMoved);
     }
 
-    emitCurveChanged();
+    markDirty();
 }
 
 QRectF CurveItem::setComponentTransform(const QTransform &transform)
@@ -499,10 +502,12 @@ void CurveItem::deleteSelectedKeyframes()
     m_keyframes.erase(iter, m_keyframes.end());
     restore();
 
-    emitCurveChanged();
+    markDirty();
+
+    emit curveChanged(id(), curve());
 }
 
-void CurveItem::emitCurveChanged()
+void CurveItem::markDirty()
 {
     setDirty(true);
     update();

src/plugins/qmldesigner/components/curveeditor/detail/curveitem.h

@@ -134,7 +134,7 @@ public:
     void deleteSelectedKeyframes();
 
 private:
-    void emitCurveChanged();
+    void markDirty();
 
     unsigned int m_id;
 

src/plugins/qmldesigner/components/curveeditor/detail/graphicsscene.cpp

@@ -216,6 +216,7 @@ void GraphicsScene::reset()
 
 void GraphicsScene::deleteSelectedKeyframes()
 {
+    m_dirty = true;
     for (auto *curve : qAsConst(m_curves))
         curve->deleteSelectedKeyframes();
 }

src/plugins/qmldesigner/components/curveeditor/detail/graphicsview.cpp

@@ -72,8 +72,8 @@ GraphicsView::GraphicsView(CurveEditorModel *model, QWidget *parent)
     connect(&m_dialog, &CurveEditorStyleDialog::styleChanged, this, &GraphicsView::setStyle);
 
     auto itemSlot = [this](unsigned int id, const AnimationCurve &curve) {
-        applyZoom(m_zoomX, m_zoomY);
         m_model->setCurve(id, curve);
+        applyZoom(m_zoomX, m_zoomY);
     };
 
     connect(m_scene, &GraphicsScene::curveChanged, itemSlot);
@@ -671,11 +671,15 @@ void GraphicsView::drawValueScale(QPainter *painter, const QRectF &rect)
         painter->drawText(textRect, Qt::AlignCenter, valueText);
     };
 
-    double density = 1. / (static_cast<double>(fm.height()) * m_style.labelDensityY);
+    double min = minimumValue();
-    Axis axis = Axis::compute(minimumValue(), maximumValue(), rect.height(), density);
+    double max = maximumValue();
-    const double eps = 1.0e-10;
+    if (std::isfinite(min) && std::isfinite(max) && rect.isValid()) {
-    for (double i = axis.lmin; i <= axis.lmax + eps; i += axis.lstep)
+        double density = 1. / (static_cast<double>(fm.height()) * m_style.labelDensityY);
-        paintLabeledTick(i);
+        Axis axis = Axis::compute(min, max, rect.height(), density);
+        const double eps = 1.0e-10;
+        for (double i = axis.lmin; i <= axis.lmax + eps; i += axis.lstep)
+            paintLabeledTick(i);
+    }
 
     painter->restore();
 }

src/plugins/qmldesigner/components/timelineeditor/timelineitem.cpp

@@ -242,13 +242,8 @@ void TimelineFrameHandle::scrollOutOfBoundsMax()
 {
     const double width = abstractScrollGraphicsScene()->width();
     if (QApplication::mouseButtons() == Qt::LeftButton) {
-        const double frameWidth = abstractScrollGraphicsScene()->rulerScaling();
+        abstractScrollGraphicsScene()->setScrollOffset(computeScrollSpeed());
-        const double upperThreshold = width - frameWidth;
+        abstractScrollGraphicsScene()->invalidateScrollbar();
-
-        if (rect().center().x() > upperThreshold) {
-            abstractScrollGraphicsScene()->setScrollOffset(computeScrollSpeed());
-            abstractScrollGraphicsScene()->invalidateScrollbar();
-        }
 
         callSetClampedXPosition(width - (rect().width() / 2) - 1);
         m_timer.start();