finally added limits to bezier so curves can't loop over themselves

app/node/input.cpp

@@ -577,13 +577,13 @@ QVariant NodeInput::GetValueAtTimeForTrack(const rational &time, int track, int
 
           double t = Bezier::CubicXtoT(time.toDouble(),
                                        before->time().toDouble(),
-                                       before->time().toDouble() + before->bezier_control_out().x(),
-                                       after->time().toDouble() + after->bezier_control_in().x(),
+                                       before->time().toDouble() + before->valid_bezier_control_out().x(),
+                                       after->time().toDouble() + after->valid_bezier_control_in().x(),
                                        after->time().toDouble());
 
           double y = Bezier::CubicTtoY(before->value().toDouble(),
-                                       before->value().toDouble() + before->bezier_control_out().y(),
-                                       after->value().toDouble() + after->bezier_control_in().y(),
+                                       before->value().toDouble() + before->valid_bezier_control_out().y(),
+                                       after->value().toDouble() + after->valid_bezier_control_in().y(),
                                        after->value().toDouble(),
                                        t);
 
@@ -597,11 +597,11 @@ QVariant NodeInput::GetValueAtTimeForTrack(const rational &time, int track, int
           double control_point_value;
 
           if (before->type() == NodeKeyframe::kBezier) {
-            control_point = before->bezier_control_out();
+            control_point = before->valid_bezier_control_out();
             control_point_time = before->time().toDouble() + control_point.x();
             control_point_value = before->value().toDouble() + control_point.y();
           } else {
-            control_point = after->bezier_control_in();
+            control_point = after->valid_bezier_control_in();
             control_point_time = after->time().toDouble() + control_point.x();
             control_point_value = after->value().toDouble() + control_point.y();
           }

app/node/inputimmediate.cpp

@@ -220,6 +220,8 @@ void NodeInputImmediate::insert_keyframe(NodeKeyframe* key)
 {
   NodeKeyframeTrack& key_track = keyframe_tracks_[key->track()];
 
+  int insert_index = key_track.size();
+
   for (int i=0;i<key_track.size();i++) {
     NodeKeyframe* compare = key_track.at(i);
 
@@ -227,16 +229,41 @@ void NodeInputImmediate::insert_keyframe(NodeKeyframe* key)
     Q_ASSERT(compare->time() != key->time());
 
     if (compare->time() > key->time()) {
-      key_track.insert(i, key);
-      return;
+      insert_index = i;
+      break;
     }
   }
 
-  key_track.append(key);
+  key_track.insert(insert_index, key);
+
+  NodeKeyframe* previous = insert_index > 0 ? key_track.at(insert_index-1) : nullptr;
+  NodeKeyframe* next = insert_index < key_track.size()-1 ? key_track.at(insert_index+1) : nullptr;
+
+  key->set_previous(previous);
+  key->set_next(next);
+
+  if (previous) {
+    previous->set_next(key);
+  }
+
+  if (next) {
+    next->set_previous(key);
+  }
 }
 
 void NodeInputImmediate::remove_keyframe(NodeKeyframe *key)
 {
+  if (key->previous()) {
+    key->previous()->set_next(key->next());
+  }
+
+  if (key->next()) {
+    key->next()->set_previous(key->previous());
+  }
+
+  key->set_previous(nullptr);
+  key->set_next(nullptr);
+
   keyframe_tracks_[key->track()].removeOne(key);
 }
 

app/node/keyframe.cpp

@@ -30,10 +30,12 @@ NodeKeyframe::NodeKeyframe(const rational &time, const QVariant &value, const No
   time_(time),
   value_(value),
   type_(type),
-  bezier_control_in_(QPointF(-1.0, 0.0)),
-  bezier_control_out_(QPointF(1.0, 0.0)),
+  bezier_control_in_(QPointF(0.0, 0.0)),
+  bezier_control_out_(QPointF(0.0, 0.0)),
   track_(track),
-  element_(element)
+  element_(element),
+  previous_(nullptr),
+  next_(nullptr)
 {
   setParent(parent);
 }
@@ -85,8 +87,31 @@ const NodeKeyframe::Type &NodeKeyframe::type() const
 
 void NodeKeyframe::set_type(const NodeKeyframe::Type &type)
 {
-  type_ = type;
-  emit TypeChanged(type_);
+  if (type_ != type) {
+    type_ = type;
+
+    if (type_ == kBezier) {
+      // Set some sane defaults if this keyframe existed in the track and was just changed
+      if (bezier_control_in_.isNull()) {
+        if (previous_) {
+          // Set the in point to be half way between
+          set_bezier_control_in(QPointF((previous_->time().toDouble() - this->time().toDouble()) * 0.5, 0.0));
+        } else {
+          set_bezier_control_in(QPointF(-1.0, 0.0));
+        }
+      }
+
+      if (bezier_control_out_.isNull()) {
+        if (next_) {
+          set_bezier_control_out(QPointF((next_->time().toDouble() - this->time().toDouble()) * 0.5, 0.0));
+        } else {
+          set_bezier_control_out(QPointF(1.0, 0.0));
+        }
+      }
+    }
+
+    emit TypeChanged(type_);
+  }
 }
 
 const QPointF &NodeKeyframe::bezier_control_in() const
@@ -111,6 +136,32 @@ void NodeKeyframe::set_bezier_control_out(const QPointF &control)
   emit BezierControlOutChanged(bezier_control_out_);
 }
 
+QPointF NodeKeyframe::valid_bezier_control_in() const
+{
+  double t = time().toDouble();
+  qreal adjusted_x = t + bezier_control_in_.x();
+
+  if (previous_) {
+    // Limit to the point of that keyframe
+    adjusted_x = qMax(adjusted_x, previous_->time().toDouble());
+  }
+
+  return QPointF(adjusted_x - t, bezier_control_in_.y());
+}
+
+QPointF NodeKeyframe::valid_bezier_control_out() const
+{
+  double t = time().toDouble();
+  qreal adjusted_x = t + bezier_control_out_.x();
+
+  if (next_) {
+    // Limit to the point of that keyframe
+    adjusted_x = qMin(adjusted_x, next_->time().toDouble());
+  }
+
+  return QPointF(adjusted_x - t, bezier_control_out_.y());
+}
+
 const QPointF &NodeKeyframe::bezier_control(NodeKeyframe::BezierType type) const
 {
   if (type == kInHandle) {

app/node/keyframe.h

@@ -99,6 +99,16 @@ public:
   const QPointF& bezier_control_out() const;
   void set_bezier_control_out(const QPointF& control);
 
+  /**
+  * @brief Returns a known good bezier that should be used in actual animation
+  *
+  * While users can move the bezier controls wherever they want, we have to limit their usage
+  * internally to prevent a situation where the animation overlaps (i.e. there can only be one Y
+  * value for any given X in the bezier line). This returns a value that is known good.
+  */
+  QPointF valid_bezier_control_in() const;
+  QPointF valid_bezier_control_out() const;
+
   /**
    * @brief Convenience functions for retrieving/setting bezier handle information with a BezierType
    */
@@ -126,6 +136,26 @@ public:
    */
   static BezierType get_opposing_bezier_type(BezierType type);
 
+  NodeKeyframe* previous() const
+  {
+    return previous_;
+  }
+
+  void set_previous(NodeKeyframe* keyframe)
+  {
+    previous_ = keyframe;
+  }
+
+  NodeKeyframe* next() const
+  {
+    return next_;
+  }
+
+  void set_next(NodeKeyframe* keyframe)
+  {
+    next_ = keyframe;
+  }
+
 signals:
   /**
    * @brief Signal emitted when this keyframe's time is changed
@@ -167,6 +197,10 @@ private:
 
   int element_;
 
+  NodeKeyframe* previous_;
+
+  NodeKeyframe* next_;
+
 };
 
 using NodeKeyframeTrack = QVector<NodeKeyframe*>;

app/widget/curvewidget/curveview.cpp

@@ -165,11 +165,13 @@ void CurveView::drawBackground(QPainter *painter, const QRectF &rect)
         painter->setPen(QPen(keyframe_colors_.value(ref),
                              qMax(1, fontMetrics().height() / 4)));
 
-        QVector<QLineF> keyframe_lines;
+        // Create a path
+        QPainterPath path;
 
         // Draw straight line leading to first keyframe
         QPointF first_key_pos = item_map().value(track.first())->pos();
-        keyframe_lines.append(QLineF(QPointF(scene_bottom_left.x(), first_key_pos.y()), first_key_pos));
+        path.moveTo(QPointF(scene_bottom_left.x(), first_key_pos.y()));
+        path.lineTo(first_key_pos);
 
         // Draw lines between each keyframe
         for (int i=1;i<track.size();i++) {
@@ -180,26 +182,20 @@ void CurveView::drawBackground(QPainter *painter, const QRectF &rect)
           KeyframeViewItem* after_item = item_map().value(after);
 
           if (before->type() == NodeKeyframe::kHold) {
+
             // Draw a hold keyframe (basically a right angle)
-            keyframe_lines.append(QLineF(before_item->pos().x(),
-                                         before_item->pos().y(),
-                                         after_item->pos().x(),
-                                         before_item->pos().y()));
-            keyframe_lines.append(QLineF(after_item->pos().x(),
-                                         before_item->pos().y(),
-                                         after_item->pos().x(),
-                                         after_item->pos().y()));
+            path.lineTo(after_item->pos().x(), before_item->pos().y());
+            path.lineTo(after_item->pos().x(), after_item->pos().y());
+
           } else if (before->type() == NodeKeyframe::kBezier && after->type() == NodeKeyframe::kBezier) {
+
             // Draw a cubic bezier
 
             // Cubic beziers have two control points, so we can just use both
-            QPointF before_control_point = before_item->pos() + ScalePoint(before->bezier_control_out());
-            QPointF after_control_point = after_item->pos() + ScalePoint(after->bezier_control_in());
+            QPointF before_control_point = before_item->pos() + ScalePoint(before->valid_bezier_control_out());
+            QPointF after_control_point = after_item->pos() + ScalePoint(after->valid_bezier_control_in());
 
-            QPainterPath path;
-            path.moveTo(before_item->pos());
             path.cubicTo(before_control_point, after_control_point, after_item->pos());
-            painter->drawPath(path);
 
           } else if (before->type() == NodeKeyframe::kBezier || after->type() == NodeKeyframe::kBezier) {
             // Draw a quadratic bezier
@@ -210,32 +206,31 @@ void CurveView::drawBackground(QPainter *painter, const QRectF &rect)
 
             if (before->type() == NodeKeyframe::kBezier) {
               key_anchor = before_item->pos();
-              control_point = before->bezier_control_out();
+              control_point = before->valid_bezier_control_out();
             } else {
               key_anchor = after_item->pos();
-              control_point = after->bezier_control_in();
+              control_point = after->valid_bezier_control_in();
             }
 
             // Scale control point
             control_point = key_anchor + ScalePoint(control_point);
 
             // Create the path from both keyframes
-            QPainterPath path;
-            path.moveTo(before_item->pos());
             path.quadTo(control_point, after_item->pos());
-            painter->drawPath(path);
 
           } else {
+
             // Linear to linear
-            keyframe_lines.append(QLineF(before_item->pos(), after_item->pos()));
+            path.lineTo(after_item->pos());
+
           }
         }
 
         // Draw straight line leading from end keyframe
         QPointF last_key_pos = item_map().value(track.last())->pos();
-        keyframe_lines.append(QLineF(last_key_pos, QPointF(scene_top_right.x(), last_key_pos.y())));
+        path.lineTo(QPointF(scene_top_right.x(), last_key_pos.y()));
 
-        painter->drawLines(keyframe_lines);
+        painter->drawPath(path);
       }
     }
   }