Improve update behavior

- Suppress reflections
- Keep selection when updating the model

Change-Id: I0e165f0019c8c24802193f3a59902876d4cb5060
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

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

@@ -61,12 +61,12 @@ AnimationCurve::AnimationCurve(const QEasingCurve &easing, const QPointF &start,
     };
 
     QVector<QPointF> points = easing.toCubicSpline();
-    int numSegments = points.count() / 3;
+    int numSegments = points.size() / 3;
 
     Keyframe current;
     Keyframe tmp(start);
 
-    current.setInterpolation(Keyframe::Interpolation::Bezier);
+    current.setInterpolation(Keyframe::Interpolation::Linear);
     tmp.setInterpolation(Keyframe::Interpolation::Bezier);
 
     for (int i = 0; i < numSegments; i++) {
@@ -80,6 +80,8 @@ AnimationCurve::AnimationCurve(const QEasingCurve &easing, const QPointF &start,
 
         m_frames.push_back(current);
 
+        current.setInterpolation(tmp.interpolation());
+
         tmp.setLeftHandle(p2);
         tmp.setPosition(p3);
     }
@@ -189,6 +191,14 @@ QPainterPath AnimationCurve::intersectionPath() const
     return path;
 }
 
+Keyframe AnimationCurve::keyframeAt(size_t id) const
+{
+    if (id >= m_frames.size())
+        return Keyframe();
+
+    return m_frames.at(id);
+}
+
 std::vector<Keyframe> AnimationCurve::keyframes() const
 {
     return m_frames;

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

@@ -66,6 +66,8 @@ public:
 
     QPainterPath intersectionPath() const;
 
+    Keyframe keyframeAt(size_t id) const;
+
     std::vector<Keyframe> keyframes() const;
 
     std::vector<QPointF> extrema() const;

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

@@ -35,7 +35,6 @@ CurveEditorModel::CurveEditorModel(QObject *parent)
 
 CurveEditorModel::~CurveEditorModel() {}
 
-
 void CurveEditorModel::setCurrentFrame(int frame)
 {
     if (graphicsView())
@@ -54,6 +53,8 @@ void CurveEditorModel::setCurve(unsigned int id, const AnimationCurve &curve)
 
 void CurveEditorModel::reset(const std::vector<TreeItem *> &items)
 {
+    std::vector<TreeItem::Path> sel = selection();
+
     beginResetModel();
 
     initialize();
@@ -65,6 +66,8 @@ void CurveEditorModel::reset(const std::vector<TreeItem *> &items)
     }
 
     endResetModel();
+
+    select(sel);
 }
 
 } // End namespace DesignTools.

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

@@ -64,11 +64,7 @@ CurveItem::CurveItem(unsigned int id, const AnimationCurve &curve, QGraphicsItem
 
     setFlag(QGraphicsItem::ItemIsMovable, false);
 
-    for (auto frame : curve.keyframes()) {
+    setCurve(curve);
-        auto *item = new KeyframeItem(frame, this);
-        QObject::connect(item, &KeyframeItem::redrawCurve, this, &CurveItem::emitCurveChanged);
-        m_keyframes.push_back(item);
-    }
 }
 
 CurveItem::~CurveItem() {}
@@ -334,7 +330,7 @@ void CurveItem::setInterpolation(Keyframe::Interpolation interpolation)
             prevItem->setKeyframe(segment.left());
             currItem->setKeyframe(segment.right());
 
-            m_itemDirty = true;
+            setDirty(true);
         }
 
         prevItem = currItem;
@@ -380,7 +376,7 @@ void CurveItem::deleteSelectedKeyframes()
 
 void CurveItem::emitCurveChanged()
 {
-    m_itemDirty = true;
+    setDirty(true);
     update();
 }
 

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

@@ -67,6 +67,8 @@ double GraphicsScene::maximumValue() const
 void GraphicsScene::addCurveItem(CurveItem *item)
 {
     m_dirty = true;
+    item->setDirty(false);
+
     addItem(item);
     item->connect(this);
 }
@@ -144,13 +146,10 @@ void GraphicsScene::mouseReleaseEvent(QGraphicsSceneMouseEvent *mouseEvent)
             // CurveItems might become invalid after a keyframe-drag operation.
             curveItem->restore();
 
-            if (curveItem->contains(mouseEvent->scenePos()))
-                curveItem->setSelected(true);
-
             if (curveItem->isDirty()) {
-                emit curveChanged(curveItem->id(), curveItem->curve());
-                curveItem->setDirty(false);
                 m_dirty = true;
+                curveItem->setDirty(false);
+                emit curveChanged(curveItem->id(), curveItem->curve());
             }
         }
     }

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

@@ -203,7 +203,7 @@ void KeyframeItem::updatePosition(bool update)
     if (m_right)
         updateHandle(m_right, false);
 
-    if (update) {
+    if (update && position != oldPosition) {
         emit redrawCurve();
         emit keyframeMoved(this, position - oldPosition);
     }
@@ -307,7 +307,6 @@ QVariant KeyframeItem::itemChange(QGraphicsItem::GraphicsItemChange change, cons
 void KeyframeItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
 {
     SelectableItem::mousePressEvent(event);
-
     if (auto *curveItem = qgraphicsitem_cast<CurveItem *>(parentItem()))
         curveItem->setHandleVisibility(false);
 }

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

@@ -26,6 +26,7 @@
 #include "treemodel.h"
 #include "detail/graphicsview.h"
 #include "treeitem.h"
+#include "treeview.h"
 
 #include <QIcon>
 
@@ -125,6 +126,11 @@ int TreeModel::columnCount(const QModelIndex &parent) const
     return m_root->columnCount();
 }
 
+void TreeModel::setTreeView(TreeView *view)
+{
+    m_tree = view;
+}
+
 void TreeModel::setGraphicsView(GraphicsView *view)
 {
     m_view = view;
@@ -135,6 +141,52 @@ GraphicsView *TreeModel::graphicsView() const
     return m_view;
 }
 
+std::vector<TreeItem::Path> TreeModel::selection() const
+{
+    std::vector<TreeItem::Path> out;
+    for (auto &&index : m_tree->selectionModel()->selectedIndexes()) {
+        if (index.column() == 0) {
+            TreeItem *item = static_cast<TreeItem *>(index.internalPointer());
+            out.push_back(item->path());
+        }
+    }
+    return out;
+}
+
+QModelIndex TreeModel::findIdx(const QString &name, const QModelIndex &parent) const
+{
+    for (int i = 0; i < rowCount(parent); ++i) {
+        QModelIndex idx = index(i, 0, parent);
+        if (idx.isValid()) {
+            TreeItem *item = static_cast<TreeItem *>(idx.internalPointer());
+            if (item->name() == name)
+                return idx;
+        }
+    }
+    return QModelIndex();
+}
+
+QModelIndex TreeModel::indexOf(const TreeItem::Path &path) const
+{
+    QModelIndex parent;
+    for (size_t i = 0; i < path.size(); ++i) {
+        QModelIndex idx = findIdx(path[i], parent);
+        if (idx.isValid())
+            parent = idx;
+    }
+
+    return parent;
+}
+
+void TreeModel::select(const std::vector<TreeItem::Path> &selection)
+{
+    for (auto &&sel : selection) {
+        QModelIndex idx = indexOf(sel);
+        if (idx.isValid())
+            m_tree->selectionModel()->select(idx, QItemSelectionModel::Select);
+    }
+}
+
 void TreeModel::initialize()
 {
     if (m_root)

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

@@ -25,6 +25,8 @@
 
 #pragma once
 
+#include "treeitem.h"
+
 #include <QAbstractItemModel>
 
 #include <vector>
@@ -32,7 +34,7 @@
 namespace DesignTools {
 
 class GraphicsView;
-class TreeItem;
+class TreeView;
 
 class TreeModel : public QAbstractItemModel
 {
@@ -45,7 +47,9 @@ public:
 
     QVariant data(const QModelIndex &index, int role) const override;
 
-    QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override;
+    QVariant headerData(int section,
+                        Qt::Orientation orientation,
+                        int role = Qt::DisplayRole) const override;
 
     QModelIndex index(int row, int column, const QModelIndex &parent = QModelIndex()) const override;
 
@@ -55,20 +59,32 @@ public:
 
     int columnCount(const QModelIndex &parent = QModelIndex()) const override;
 
+    void setTreeView(TreeView *view);
+
     void setGraphicsView(GraphicsView *view);
 
 protected:
     GraphicsView *graphicsView() const;
 
+    std::vector<TreeItem::Path> selection() const;
+
+    void select(const std::vector<TreeItem::Path> &selection);
+
     void initialize();
 
     TreeItem *root();
 
     TreeItem *find(unsigned int id);
 
+    QModelIndex findIdx(const QString &name, const QModelIndex &parent) const;
+
+    QModelIndex indexOf(const TreeItem::Path &path) const;
+
 private:
     GraphicsView *m_view;
 
+    TreeView *m_tree;
+
     TreeItem *m_root;
 };
 

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

@@ -36,6 +36,8 @@ namespace DesignTools {
 TreeView::TreeView(CurveEditorModel *model, QWidget *parent)
     : QTreeView(parent)
 {
+    model->setTreeView(this);
+
     setSizePolicy(QSizePolicy::MinimumExpanding, QSizePolicy::MinimumExpanding);
     setUniformRowHeights(true);
     setRootIsDecorated(false);

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

@@ -72,6 +72,25 @@ QString TreeItem::name() const
     return m_name;
 }
 
+TreeItem::Path TreeItem::path() const
+{
+    Path fullName;
+    fullName.push_back(name());
+
+    TreeItem *parent = this->parent();
+    while (parent) {
+        if (parent->name() == "Root")
+            break;
+
+        fullName.push_back(parent->name());
+        parent = parent->parent();
+    }
+
+    std::reverse(fullName.begin(), fullName.end());
+
+    return fullName;
+}
+
 bool TreeItem::hasChildren() const
 {
     return !m_children.empty();
@@ -87,6 +106,20 @@ bool TreeItem::pinned() const
     return m_pinned;
 }
 
+bool TreeItem::compare(const std::vector<QString> &path) const
+{
+    auto thisPath = this->path();
+    if (thisPath.size() != path.size())
+        return false;
+
+    for (size_t i = 0; i < thisPath.size(); ++i) {
+        if (thisPath[i] != path[i])
+            return false;
+    }
+
+    return true;
+}
+
 int TreeItem::row() const
 {
     if (m_parent) {

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

@@ -45,6 +45,9 @@ class PropertyTreeItem;
 
 class TreeItem
 {
+public:
+    using Path = std::vector<QString>;
+
 public:
     TreeItem(const QString &name);
 
@@ -60,12 +63,16 @@ public:
 
     QString name() const;
 
+    Path path() const;
+
     bool hasChildren() const;
 
     bool locked() const;
 
     bool pinned() const;
 
+    bool compare(const std::vector<QString> &path) const;
+
     int row() const;
 
     int column() const;

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

@@ -122,9 +122,7 @@ DesignTools::ValueType typeFrom(const QmlTimelineKeyframeGroup &group)
     if (group.valueType() == TypeName("integer") || group.valueType() == TypeName("int"))
         return DesignTools::ValueType::Integer;
 
-    // Ignoring types:
+    // Ignoring: QColor / HAlignment / VAlignment
-    // QColor / HAlignment / VAlignment
-
     return DesignTools::ValueType::Undefined;
 }
 
@@ -170,48 +168,61 @@ DesignTools::AnimationCurve AnimationCurveEditorModel::createAnimationCurve(
     }
 }
 
+std::vector<DesignTools::Keyframe> createKeyframes(QList<ModelNode> nodes)
+{
+    auto byTime = [](const auto &a, const auto &b) {
+        return a.variantProperty("frame").value().toDouble()
+               < b.variantProperty("frame").value().toDouble();
+    };
+    std::sort(nodes.begin(), nodes.end(), byTime);
+
+    std::vector<DesignTools::Keyframe> frames;
+    for (auto &&node : nodes) {
+        QVariant timeVariant = node.variantProperty("frame").value();
+        QVariant valueVariant = node.variantProperty("value").value();
+        if (!timeVariant.isValid() || !valueVariant.isValid())
+            continue;
+
+        QPointF position(timeVariant.toDouble(), valueVariant.toDouble());
+
+        auto keyframe = DesignTools::Keyframe(position);
+
+        if (node.hasBindingProperty("easing.bezierCurve")) {
+            EasingCurve ecurve;
+            ecurve.fromString(node.bindingProperty("easing.bezierCurve").expression());
+            keyframe.setData(static_cast<QEasingCurve>(ecurve));
+        }
+        frames.push_back(keyframe);
+    }
+    return frames;
+}
+
+std::vector<DesignTools::Keyframe> resolveSmallCurves(
+    const std::vector<DesignTools::Keyframe> &frames)
+{
+    std::vector<DesignTools::Keyframe> out;
+    for (auto &&frame : frames) {
+        if (frame.hasData() && !out.empty()) {
+            QEasingCurve curve = frame.data().toEasingCurve();
+            if (curve.toCubicSpline().count() == 3) {
+                DesignTools::Keyframe &previous = out.back();
+                DesignTools::AnimationCurve acurve(curve, previous.position(), frame.position());
+                previous = acurve.keyframeAt(0);
+                out.push_back(acurve.keyframeAt(1));
+                continue;
+            }
+        }
+        out.push_back(frame);
+    }
+    return out;
+}
+
 DesignTools::AnimationCurve AnimationCurveEditorModel::createDoubleCurve(
     const QmlTimelineKeyframeGroup &group)
 {
-    std::vector<DesignTools::Keyframe> keyframes;
+    std::vector<DesignTools::Keyframe> keyframes = createKeyframes(group.keyframePositions());
-    for (auto &&frame : group.keyframePositions()) {
+    keyframes = resolveSmallCurves(keyframes);
-        QVariant timeVariant = frame.variantProperty("frame").value();
-        QVariant valueVariant = frame.variantProperty("value").value();
-
-        if (timeVariant.isValid() && valueVariant.isValid()) {
-            QPointF position(timeVariant.toDouble(), valueFromVariant(valueVariant));
-            auto keyframe = DesignTools::Keyframe(position);
-
-            if (frame.hasBindingProperty("easing.bezierCurve")) {
-                EasingCurve ecurve;
-                ecurve.fromString(frame.bindingProperty("easing.bezierCurve").expression());
-                keyframe.setData(static_cast<QEasingCurve>(ecurve));
-            }
-
-            keyframes.push_back(keyframe);
-        }
-    }
     return DesignTools::AnimationCurve(keyframes);
 }
 
-double AnimationCurveEditorModel::valueFromVariant(const QVariant &variant)
-{
-    return variant.toDouble();
-}
-
-void AnimationCurveEditorModel::reset(const std::vector<DesignTools::TreeItem *> &items)
-{
-    beginResetModel();
-
-    initialize();
-
-    unsigned int counter = 0;
-    for (auto *item : items) {
-        item->setId(++counter);
-        root()->addChild(item);
-    }
-
-    endResetModel();
-}
-
 } // namespace QmlDesigner

src/plugins/qmldesigner/components/timelineeditor/animationcurveeditormodel.h

@@ -60,10 +60,6 @@ private:
 
     DesignTools::AnimationCurve createDoubleCurve(const QmlTimelineKeyframeGroup &group);
 
-    double valueFromVariant(const QVariant &variant);
-
-    void reset(const std::vector<DesignTools::TreeItem *> &items);
-
     double m_minTime;
 
     double m_maxTime;

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

@@ -154,8 +154,16 @@ void TimelineToolBar::setCurrentState(const QString &name)
         m_stateLabel->setText(name);
 }
 
+void TimelineToolBar::setBlockReflection(bool block)
+{
+    m_blockReflection = block;
+}
+
 void TimelineToolBar::setCurrentTimeline(const QmlTimeline &timeline)
 {
+    if (m_blockReflection)
+        return;
+
     if (timeline.isValid()) {
         setStartFrame(timeline.startKeyframe());
         setEndFrame(timeline.endKeyframe());

src/plugins/qmldesigner/components/timelineeditor/timelinetoolbar.h

@@ -79,6 +79,7 @@ public:
     QString currentTimelineId() const;
 
     void setCurrentState(const QString &name);
+    void setBlockReflection(bool block);
     void setCurrentTimeline(const QmlTimeline &timeline);
     void setStartFrame(qreal frame);
     void setCurrentFrame(qreal frame);
@@ -116,6 +117,7 @@ private:
     QLineEdit *m_lastFrame = nullptr;
 
     QAction *m_recording = nullptr;
+    bool m_blockReflection = false;
 };
 
 } // namespace QmlDesigner

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

@@ -142,7 +142,7 @@ void TimelineView::nodeRemoved(const ModelNode & /*removedNode*/,
 void TimelineView::nodeReparented(const ModelNode &node,
                                   const NodeAbstractProperty &newPropertyParent,
                                   const NodeAbstractProperty & /*oldPropertyParent*/,
-                                  AbstractView::PropertyChangeFlags /*propertyChange*/)
+                                  AbstractView::PropertyChangeFlags propertyChange)
 {
     if (newPropertyParent.isValid()
         && QmlTimelineKeyframeGroup::isValidQmlTimelineKeyframeGroup(
@@ -151,7 +151,7 @@ void TimelineView::nodeReparented(const ModelNode &node,
         m_timelineWidget->graphicsScene()->invalidateSectionForTarget(frames.target());
 
         QmlTimeline currentTimeline = m_timelineWidget->graphicsScene()->currentTimeline();
-        if (currentTimeline.isValid())
+        if (currentTimeline.isValid() && propertyChange == AbstractView::NoAdditionalChanges)
             m_timelineWidget->toolBar()->setCurrentTimeline(currentTimeline);
 
     } else if (QmlTimelineKeyframeGroup::checkKeyframesType(
@@ -196,12 +196,26 @@ void TimelineView::variantPropertiesChanged(const QList<VariantProperty> &proper
                 m_timelineWidget->graphicsScene()->invalidateKeyframesForTarget(frames.target());
 
                 QmlTimeline currentTimeline = m_timelineWidget->graphicsScene()->currentTimeline();
+                if (currentTimeline.isValid())
                     m_timelineWidget->toolBar()->setCurrentTimeline(currentTimeline);
             }
         }
     }
 }
 
+void TimelineView::bindingPropertiesChanged(const QList<BindingProperty> &propertyList,
+                                            AbstractView::PropertyChangeFlags propertyChange)
+{
+    Q_UNUSED(propertyChange)
+    for (const auto &property : propertyList) {
+        if (property.name() == "easing.bezierCurve") {
+            QmlTimeline currentTimeline = m_timelineWidget->graphicsScene()->currentTimeline();
+            if (currentTimeline.isValid())
+                m_timelineWidget->toolBar()->setCurrentTimeline(currentTimeline);
+        }
+    }
+}
+
 void TimelineView::selectedNodesChanged(const QList<ModelNode> & /*selectedNodeList*/,
                                         const QList<ModelNode> & /*lastSelectedNodeList*/)
 {

src/plugins/qmldesigner/components/timelineeditor/timelineview.h

@@ -60,6 +60,8 @@ public:
     void instancePropertyChanged(const QList<QPair<ModelNode, PropertyName>> &propertyList) override;
     void variantPropertiesChanged(const QList<VariantProperty> &propertyList,
                                   PropertyChangeFlags propertyChange) override;
+    void bindingPropertiesChanged(const QList<BindingProperty> &propertyList,
+                                  PropertyChangeFlags propertyChange) override;
     void selectedNodesChanged(const QList<ModelNode> &selectedNodeList,
                               const QList<ModelNode> &lastSelectedNodeList) override;
 

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

@@ -328,7 +328,8 @@ void TimelineWidget::updateAnimationCurve(DesignTools::PropertyTreeItem *item)
     QmlTimelineKeyframeGroup group = timelineKeyframeGroup(currentTimeline, item);
 
     if (group.isValid()) {
-        auto replaceKeyframes = [&group, currentTimeline, item]() {
+        auto replaceKeyframes = [&group, item, this]() {
+            m_toolbar->setBlockReflection(true);
             for (auto frame : group.keyframes())
                 frame.destroy();
 
@@ -353,6 +354,7 @@ void TimelineWidget::updateAnimationCurve(DesignTools::PropertyTreeItem *item)
 
                 previous = frame;
             }
+            m_toolbar->setBlockReflection(false);
         };
 
         timelineView()->executeInTransaction("TimelineWidget::handleKeyframeReplacement",