Disallow the creation of invalid curve

but allow invalid initialized curves to be moved into a legal state.
Visualize invalid curve-segments by painting them in the errorColor

Fixes: QDS-2130
Change-Id: Ida44c3b4f5e3d113df7d1e8e7a2b965d26f43815
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

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

@@ -71,6 +71,7 @@ struct CurveItemStyleOption
 {
     double width = 1.0;
     QColor color = QColor(0, 200, 0);
+    QColor errorColor = QColor(200, 0, 0);
     QColor selectionColor = QColor(200, 200, 200);
     QColor easingCurveColor = QColor(200, 0, 200);
     QColor lockedColor = QColor(50, 50, 50);

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

@@ -164,6 +164,29 @@ bool CurveSegment::isValid() const
     return true;
 }
 
+bool CurveSegment::isLegal() const
+{
+    if (!isValid())
+        return false;
+
+    if (interpolation() == Keyframe::Interpolation::Step)
+        return true;
+
+    if (interpolation() == Keyframe::Interpolation::Linear)
+        return true;
+
+    std::vector<double> ex = CubicPolynomial(m_left.position().x(),
+                                             m_left.rightHandle().x(),
+                                             m_right.leftHandle().x(),
+                                             m_right.position().x())
+                                 .extrema();
+
+    ex.erase(std::remove_if(ex.begin(), ex.end(), [](double val) { return val <= 0. || val >= 1.; }),
+             ex.end());
+
+    return ex.size() == 0;
+}
+
 bool CurveSegment::containsX(double x) const
 {
     return m_left.position().x() <= x && m_right.position().x() >= x;
@@ -500,6 +523,31 @@ void CurveSegment::setRight(const Keyframe &frame)
     m_right = frame;
 }
 
+void CurveSegment::moveLeftTo(const QPointF &pos)
+{
+    QPointF delta = pos - m_left.position();
+
+    if (m_left.hasLeftHandle())
+        m_left.setLeftHandle(m_left.leftHandle() + delta);
+
+    if (m_left.hasRightHandle())
+        m_left.setRightHandle(m_left.rightHandle() + delta);
+
+    m_left.setPosition(pos);
+}
+
+void CurveSegment::moveRightTo(const QPointF &pos)
+{
+    QPointF delta = pos - m_right.position();
+    if (m_right.hasLeftHandle())
+        m_right.setLeftHandle(m_right.leftHandle() + delta);
+
+    if (m_right.hasRightHandle())
+        m_right.setRightHandle(m_right.rightHandle() + delta);
+
+    m_right.setPosition(pos);
+}
+
 void CurveSegment::setInterpolation(const Keyframe::Interpolation &interpol)
 {
     m_right.setInterpolation(interpol);

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

@@ -47,6 +47,8 @@ public:
 
     bool isValid() const;
 
+    bool isLegal() const;
+
     bool containsX(double x) const;
 
     Keyframe left() const;
@@ -83,6 +85,10 @@ public:
 
     void setRight(const Keyframe &frame);
 
+    void moveLeftTo(const QPointF &pos);
+
+    void moveRightTo(const QPointF &pos);
+
     void setInterpolation(const Keyframe::Interpolation &interpol);
 
 private:

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

@@ -119,6 +119,8 @@ void CurveItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *, QWidg
             } else {
                 if (locked())
                     pen.setColor(m_style.lockedColor);
+                else if (!segment.isLegal())
+                    pen.setColor(m_style.errorColor);
                 else if (isUnderMouse())
                     pen.setColor(m_style.hoverColor);
                 else if (hasSelectedKeyframe())
@@ -178,6 +180,36 @@ bool CurveItem::hasEditableSegment(double time) const
     return curve().segment(time).interpolation() != Keyframe::Interpolation::Easing;
 }
 
+bool CurveItem::isFirst(const KeyframeItem *key) const
+{
+    if (m_keyframes.empty())
+        return false;
+
+    return m_keyframes.first() == key;
+}
+
+bool CurveItem::isLast(const KeyframeItem *key) const
+{
+    if (m_keyframes.empty())
+        return false;
+
+    return m_keyframes.last() == key;
+}
+
+int CurveItem::indexOf(const KeyframeItem *key) const
+{
+    if (m_keyframes.empty())
+        return -1;
+
+    int out = 0;
+    for (auto &&el : m_keyframes) {
+        if (el == key)
+            return out;
+        out++;
+    }
+    return -1;
+}
+
 unsigned int CurveItem::id() const
 {
     return m_id;
@@ -283,6 +315,22 @@ QVector<HandleItem *> CurveItem::handles() const
     return out;
 }
 
+CurveSegment CurveItem::segment(const KeyframeItem *keyframe, HandleItem::Slot slot) const
+{
+    auto finder = [keyframe](KeyframeItem *item) { return item == keyframe; };
+    auto iter = std::find_if(m_keyframes.begin(), m_keyframes.end(), finder);
+    if (iter == m_keyframes.end())
+        return CurveSegment();
+
+    int index = static_cast<int>(std::distance(m_keyframes.begin(), iter));
+    if (slot == HandleItem::Slot::Left && index > 0)
+        return CurveSegment(m_keyframes[index - 1]->keyframe(), keyframe->keyframe());
+    else if (slot == HandleItem::Slot::Right && index < (m_keyframes.size() - 1))
+        return CurveSegment(keyframe->keyframe(), m_keyframes[index + 1]->keyframe());
+
+    return CurveSegment();
+}
+
 void CurveItem::restore()
 {
     if (m_keyframes.empty())

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

@@ -81,6 +81,12 @@ public:
 
     bool hasEditableSegment(double time) const;
 
+    bool isFirst(const KeyframeItem *key) const;
+
+    bool isLast(const KeyframeItem *key) const;
+
+    int indexOf(const KeyframeItem *key) const;
+
     unsigned int id() const;
 
     ValueType valueType() const;
@@ -99,6 +105,8 @@ public:
 
     QVector<HandleItem *> handles() const;
 
+    CurveSegment segment(const KeyframeItem *keyframe, HandleItem::Slot slot) const;
+
     void restore();
 
     void setDirty(bool dirty);

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

@@ -24,6 +24,7 @@
 ****************************************************************************/
 
 #include "handleitem.h"
+#include "curvesegment.h"
 #include "graphicsscene.h"
 #include "keyframeitem.h"
 #include "utils.h"
@@ -72,6 +73,14 @@ bool HandleItem::keyframeSelected() const
     return false;
 }
 
+CurveSegment HandleItem::segment() const
+{
+    if (KeyframeItem *parent = qgraphicsitem_cast<KeyframeItem *>(parentItem()))
+        return parent->segment(m_slot);
+
+    return CurveSegment();
+}
+
 KeyframeItem *HandleItem::keyframe() const
 {
     if (KeyframeItem *parent = qgraphicsitem_cast<KeyframeItem *>(parentItem()))
@@ -146,17 +155,39 @@ void HandleItem::setStyle(const CurveEditorStyle &style)
 QVariant HandleItem::itemChange(QGraphicsItem::GraphicsItemChange change, const QVariant &value)
 {
     if (change == ItemPositionChange) {
-        if (keyframe()) {
+
+        if (!scene())
+            return QGraphicsItem::itemChange(change, value);
+
+        if (KeyframeItem *keyItem = keyframe()) {
+            CurveSegment seg = segment();
+            if (!seg.isLegal())
+                return value;
+
             QPointF pos = value.toPointF();
+            QPointF relativePosition = keyItem->transform().inverted().map(pos);
+
             if (m_slot == HandleItem::Slot::Left) {
                 if (pos.x() > 0.0)
                     pos.rx() = 0.0;
 
+                Keyframe key = seg.right();
+                key.setLeftHandle(key.position() + relativePosition);
+                seg.setRight(key);
+
             } else if (m_slot == HandleItem::Slot::Right) {
                 if (pos.x() < 0.0)
                     pos.rx() = 0.0;
+
+                Keyframe key = seg.left();
+                key.setRightHandle(key.position() + relativePosition);
+                seg.setLeft(key);
             }
-            return QVariant(pos);
+
+            if (seg.isLegal())
+                m_validPos = pos;
+
+            return QVariant(m_validPos);
         }
     }
     return QGraphicsItem::itemChange(change, value);

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

@@ -31,6 +31,7 @@
 namespace DesignTools {
 
 class KeyframeItem;
+class CurveSegment;
 
 class HandleItem : public SelectableItem
 {
@@ -57,6 +58,8 @@ public:
 
     bool keyframeSelected() const;
 
+    CurveSegment segment() const;
+
     KeyframeItem *keyframe() const;
 
     Slot slot() const;
@@ -70,6 +73,8 @@ private:
     Slot m_slot;
 
     HandleItemStyleOption m_style;
+
+    QPointF m_validPos;
 };
 
 } // End namespace DesignTools.

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

@@ -35,15 +35,22 @@ namespace DesignTools {
 
 KeyframeItem::KeyframeItem(QGraphicsItem *parent)
     : SelectableItem(parent)
+    , m_transform()
+    , m_style()
     , m_frame()
+    , m_left(nullptr)
+    , m_right(nullptr)
+    , m_validPos()
 {}
 
 KeyframeItem::KeyframeItem(const Keyframe &keyframe, QGraphicsItem *parent)
     : SelectableItem(parent)
     , m_transform()
+    , m_style()
     , m_frame()
     , m_left(nullptr)
     , m_right(nullptr)
+    , m_validPos()
 {
     setKeyframe(keyframe);
 }
@@ -137,6 +144,14 @@ HandleItem *KeyframeItem::rightHandle() const
     return m_right;
 }
 
+CurveSegment KeyframeItem::segment(HandleItem::Slot slot) const
+{
+    if (auto *curveItem = qgraphicsitem_cast<CurveItem *>(parentItem()))
+        return curveItem->segment(this, slot);
+
+    return CurveSegment();
+}
+
 QTransform KeyframeItem::transform() const
 {
     return m_transform;
@@ -364,19 +379,51 @@ void KeyframeItem::updateHandle(HandleItem *handle, bool emitChanged)
 QVariant KeyframeItem::itemChange(QGraphicsItem::GraphicsItemChange change, const QVariant &value)
 {
     if (change == ItemPositionChange) {
+        if (!scene())
+            return QGraphicsItem::itemChange(change, value);
+
+        CurveItem *curveItem = qgraphicsitem_cast<CurveItem *>(parentItem());
+        if (!curveItem)
+            return QGraphicsItem::itemChange(change, value);
+
+        CurveSegment lseg = segment(HandleItem::Slot::Left);
+        CurveSegment rseg = segment(HandleItem::Slot::Right);
+
+        auto legalLeft = [this, curveItem, &lseg]() {
+            if (curveItem->isFirst(this))
+                return true;
+            else
+                return lseg.isLegal();
+        };
+
+        auto legalRight = [this, curveItem, &rseg]() {
+            if (curveItem->isLast(this))
+                return true;
+            else
+                return rseg.isLegal();
+        };
+
         bool ok;
         QPointF position = m_transform.inverted(&ok).map(value.toPointF());
         if (ok) {
             position.setX(std::round(position.x()));
 
-            if (auto *curveItem = qgraphicsitem_cast<CurveItem *>(parentItem())) {
+            if (curveItem->valueType() == ValueType::Integer)
-                if (curveItem->valueType() == ValueType::Integer)
+                position.setY(std::round(position.y()));
-                    position.setY(std::round(position.y()));
+            else if (curveItem->valueType() == ValueType::Bool)
-                else if (curveItem->valueType() == ValueType::Bool)
+                position.setY(position.y() > 0.5 ? 1.0 : 0.0);
-                    position.setY(position.y() > 0.5 ? 1.0 : 0.0);
-            }
 
-            return QVariant(m_transform.map(position));
+            if (!legalLeft() || !legalRight()) {
+                return QVariant(m_transform.map(position));
+            } else {
+                lseg.moveRightTo(position);
+                rseg.moveLeftTo(position);
+
+                if (legalLeft() && legalRight())
+                    m_validPos = position;
+
+                return QVariant(m_transform.map(m_validPos));
+            }
         }
     }
 

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

@@ -78,6 +78,8 @@ public:
 
     HandleItem *rightHandle() const;
 
+    CurveSegment segment(HandleItem::Slot slot) const;
+
     QTransform transform() const;
 
     void setHandleVisibility(bool visible);
@@ -127,6 +129,8 @@ private:
 
     HandleItem *m_right;
 
+    QPointF m_validPos;
+
     bool m_visibleOverride = true;
 };