Various improvement to keyframe model and initial view

src/assets/view/assetparameterview.cpp

@@ -95,6 +95,7 @@ void AssetParameterView::setRange(QPair<int, int> range)
 
 void AssetParameterView::commitChanges(const QModelIndex &index, const QString &value, bool storeUndo)
 {
+    // Warning: please note that some widgets (for example keyframes) do NOT send the valueChanged signal and do modifications on their own
     AssetCommand *command = new AssetCommand(m_model, index, value);
     if (storeUndo) {
         pCore->pushUndo(command);

src/definitions.h

@@ -27,6 +27,7 @@
 
 #include <QHash>
 #include <QString>
+#include <QPersistentModelIndex>
 #include <QTreeWidgetItem>
 #include <memory>
 
@@ -240,12 +241,16 @@ private:
 
 QDebug operator<<(QDebug qd, const ItemInfo &info);
 
-// we provide hash function for qstring
+// we provide hash function for qstring and QPersistentModelIndex
 namespace std {
 template <> struct hash<QString>
 {
     std::size_t operator()(const QString &k) const { return qHash(k); }
 };
+template <> struct hash<QPersistentModelIndex>
+{
+    std::size_t operator()(const QPersistentModelIndex &k) const { return qHash(k); }
+};
 }
 
 // The following is a hack that allows to use shared_from_this in the case of a multiple inheritance.
@@ -294,7 +299,7 @@ template<class T>
 class QAbstractItemModel_shared_from_this : public QAbstractItemModel
 {
 protected:
-    QAbstractItemModel_shared_from_this() : QAbstractItemModel() {}
+QAbstractItemModel_shared_from_this() : QAbstractItemModel() {}
 
 public:
 

src/effects/CMakeLists.txt

@@ -13,5 +13,6 @@ set(kdenlive_SRCS
   effects/effectstack/view/collapsibleeffectview.cpp
   effects/effectstack/view/effectstackview.cpp
   effects/keyframes/keyframemodel.cpp
+  effects/keyframes/view/keyframeview.cpp
   PARENT_SCOPE)
 

src/effects/keyframes/keyframemodel.cpp

@@ -22,16 +22,17 @@
 #include "keyframemodel.hpp"
 #include "doc/docundostack.hpp"
 #include "core.h"
-#include "effects/effectstack/model/effectitemmodel.hpp"
+#include "assets/model/assetparametermodel.hpp"
 #include "macros.hpp"
 
 #include <QDebug>
 
 
-KeyframeModel::KeyframeModel(double init_value, std::weak_ptr<EffectItemModel> effect, std::weak_ptr<DocUndoStack> undo_stack, QObject *parent)
+KeyframeModel::KeyframeModel(double init_value, std::weak_ptr<AssetParameterModel> model, const QModelIndex &index, std::weak_ptr<DocUndoStack> undo_stack, QObject *parent)
     : QAbstractListModel(parent)
-    , m_effect(std::move(effect))
+    , m_model(std::move(model))
     , m_undoStack(std::move(undo_stack))
+    , m_index(index)
     , m_lock(QReadWriteLock::Recursive)
 {
     m_keyframeList.insert({GenTime(), {KeyframeType::Linear, init_value}});
@@ -50,6 +51,7 @@ void KeyframeModel::setup()
     connect(this, &KeyframeModel::rowsInserted, this, &KeyframeModel::modelChanged);
     connect(this, &KeyframeModel::modelReset, this, &KeyframeModel::modelChanged);
     connect(this, &KeyframeModel::dataChanged, this, &KeyframeModel::modelChanged);
+    connect(this, &KeyframeModel::modelChanged, this, &KeyframeModel::sendModification);
 }
 
 bool KeyframeModel::addKeyframe(GenTime pos, KeyframeType type, double value, Fun &undo, Fun &redo)
@@ -112,7 +114,7 @@ bool KeyframeModel::removeKeyframe(GenTime pos)
     Fun undo = []() { return true; };
     Fun redo = []() { return true; };
 
-    if (pos == GenTime()) {
+    if (m_keyframeList.count(pos) > 0 && m_keyframeList.find(pos) == m_keyframeList.begin()) {
         return false;  // initial point must stay
     }
 
@@ -123,7 +125,7 @@ bool KeyframeModel::removeKeyframe(GenTime pos)
     return res;
 }
 
-bool KeyframeModel::moveKeyframe(GenTime oldPos, GenTime pos)
+bool KeyframeModel::moveKeyframe(GenTime oldPos, GenTime pos, bool logUndo)
 {
     QWriteLocker locker(&m_lock);
     Q_ASSERT(m_keyframeList.count(oldPos) > 0);
@@ -137,7 +139,9 @@ bool KeyframeModel::moveKeyframe(GenTime oldPos, GenTime pos)
         res = addKeyframe(pos, oldType, oldValue, undo, redo);
     }
     if (res) {
-        PUSH_UNDO(undo, redo, i18n("Move keyframe"));
+        if (logUndo) {
+            PUSH_UNDO(undo, redo, i18n("Move keyframe"));
+        }
     } else {
         bool undone = undo();
         Q_ASSERT(undone);
@@ -261,10 +265,64 @@ Keyframe KeyframeModel::getKeyframe(const GenTime &pos, bool *ok) const
     return {pos, m_keyframeList.at(pos)};
 }
 
+Keyframe KeyframeModel::getNextKeyframe(const GenTime &pos, bool *ok) const
+{
+    auto it = m_keyframeList.upper_bound(pos);
+    if (it == m_keyframeList.end()) {
+        // return empty marker
+        *ok = false;
+        return {GenTime(), {KeyframeType::Linear, 0}};
+    }
+    *ok = true;
+    return {(*it).first, (*it).second};
+}
+
+Keyframe KeyframeModel::getPrevKeyframe(const GenTime &pos, bool *ok) const
+{
+    auto it = m_keyframeList.lower_bound(pos);
+    if (it == m_keyframeList.begin()) {
+        // return empty marker
+        *ok = false;
+        return {GenTime(), {KeyframeType::Linear, 0}};
+    }
+    --it;
+    *ok = true;
+    return {(*it).first, (*it).second};
+}
+
+Keyframe KeyframeModel::getClosestKeyframe(const GenTime &pos, bool *ok) const
+{
+    if (m_keyframeList.count(pos) > 0) {
+        return getKeyframe(pos, ok);
+    }
+    bool ok1, ok2;
+    auto next = getNextKeyframe(pos, &ok1);
+    auto prev = getPrevKeyframe(pos, &ok2);
+    *ok = ok1 || ok2;
+    if (ok1 && ok2) {
+        double fps = pCore->getCurrentFps();
+        if (qAbs(next.first.frames(fps) - pos.frames(fps)) < qAbs(prev.first.frames(fps) - pos.frames(fps))) {
+            return next;
+        }
+        return prev;
+    } else if (ok1) {
+        return next;
+    } else if (ok2) {
+        return prev;
+    }
+    // return empty marker
+    return {GenTime(), {KeyframeType::Linear, 0}};
+}
+
+
 bool KeyframeModel::hasKeyframe(int frame) const
+{
+    return hasKeyframe(GenTime(frame, pCore->getCurrentFps()));
+}
+bool KeyframeModel::hasKeyframe(const GenTime &pos) const
 {
     READ_LOCK();
-    return m_keyframeList.count(GenTime(frame, pCore->getCurrentFps())) > 0;
+    return m_keyframeList.count(pos) > 0;
 }
 
 bool KeyframeModel::removeAllKeyframes()
@@ -277,7 +335,12 @@ bool KeyframeModel::removeAllKeyframes()
         all_pos.push_back(m.first);
     }
     bool res = true;
+    bool first = true;
     for (const auto& p : all_pos) {
+        if (first) { // skip first point
+            first = false;
+            continue;
+        }
         res = removeKeyframe(p, local_undo, local_redo);
         if (!res) {
             bool undone = local_undo();
@@ -315,3 +378,55 @@ QString KeyframeModel::getAnimProperty() const
     }
     return prop;
 }
+
+mlt_keyframe_type convertToMltType(KeyframeType type)
+{
+    switch (type) {
+    case KeyframeType::Linear:
+        return mlt_keyframe_linear;
+    case KeyframeType::Discrete:
+        return mlt_keyframe_discrete;
+    case KeyframeType::Curve:
+        return mlt_keyframe_smooth;
+    }
+    return mlt_keyframe_linear;
+}
+
+double KeyframeModel::getInterpolatedValue(int p) const
+{
+    auto pos = GenTime(p, pCore->getCurrentFps());
+    return getInterpolatedValue(pos);
+}
+double KeyframeModel::getInterpolatedValue(const GenTime &pos) const
+{
+    int p = pos.frames(pCore->getCurrentFps());
+    if (m_keyframeList.count(pos) > 0) {
+        return m_keyframeList.at(pos).second;
+    }
+    auto next = m_keyframeList.upper_bound(pos);
+    if (next == m_keyframeList.cbegin()) {
+        return (m_keyframeList.cbegin())->second.second;
+    } else if (next == m_keyframeList.cend()) {
+        auto it = m_keyframeList.cend();
+        --it;
+        return it->second.second;
+    }
+    auto prev = next;
+    --prev;
+    // We now have surrounding keyframes, we use mlt to compute the value
+
+    Mlt::Properties prop;
+    prop.anim_set("keyframe", prev->second.second, prev->first.frames(pCore->getCurrentFps()), 0, convertToMltType(prev->second.first) );
+    prop.anim_set("keyframe", next->second.second, next->first.frames(pCore->getCurrentFps()), 0, convertToMltType(next->second.first) );
+    return prop.anim_get_double("keyframe", p);
+}
+
+void KeyframeModel::sendModification() const
+{
+    if (auto ptr = m_model.lock()) {
+        Q_ASSERT(m_index.isValid());
+        QString name =  ptr->data(m_index, AssetParameterModel::NameRole).toString();
+        ptr->setParameter(name, getAnimProperty());
+        ptr->dataChanged(m_index, m_index);
+    }
+}

src/effects/keyframes/keyframemodel.hpp

@@ -32,11 +32,12 @@
 #include <map>
 #include <memory>
 
+class AssetParameterModel;
 class DocUndoStack;
 class EffectItemModel;
 
 /* @brief This class is the model for a list of keyframes.
-   A keyframe is defined by a time, and a type;
+   A keyframe is defined by a time, a type and a value
    We store them in a sorted fashion using a std::map
  */
 
@@ -56,10 +57,13 @@ class KeyframeModel : public QAbstractListModel
 public:
     /* @brief Construct a keyframe list bound to the given effect
        @param init_value is the value taken by the param at time 0.
+       @param model is the asset this parameter belong to
+       @param index is the index of this parameter in its model
      */
-    explicit KeyframeModel(double init_value, std::weak_ptr<EffectItemModel> effect, std::weak_ptr<DocUndoStack> undo_stack, QObject *parent = nullptr);
+    explicit KeyframeModel(double init_value, std::weak_ptr<AssetParameterModel> model, const QModelIndex &index, std::weak_ptr<DocUndoStack> undo_stack, QObject *parent = nullptr);
 
     enum { TypeRole = Qt::UserRole + 1, PosRole, FrameRole, ValueRole};
+    friend class KeyframeModelList;
 
     /* @brief Adds a keyframe at the given position. If there is already one then we update it.
        @param pos defines the position of the keyframe, relative to the clip
@@ -85,8 +89,9 @@ public:
     /* @brief moves a keyframe
        @param oldPos is the old position of the keyframe
        @param pos defines the new position of the keyframe, relative to the clip
+       @param logUndo if true, then an undo object is created
     */
-    bool moveKeyframe(GenTime oldPos, GenTime pos);
+    bool moveKeyframe(GenTime oldPos, GenTime pos, bool logUndo);
 
     /* @brief updates the value of a keyframe
        @param old is the position of the keyframe
@@ -99,10 +104,28 @@ public:
      */
     Keyframe getKeyframe(const GenTime &pos, bool *ok) const;
 
+    /* @brief Returns the keyframe located after given position.
+       If there is a keyframe at given position it is ignored.
+       @param ok is a return parameter to tell if a keyframe was found.
+    */
+    Keyframe getNextKeyframe(const GenTime &pos, bool *ok) const;
+
+    /* @brief Returns the keyframe located before given position.
+       If there is a keyframe at given position it is ignored.
+       @param ok is a return parameter to tell if a keyframe was found.
+    */
+    Keyframe getPrevKeyframe(const GenTime &pos, bool *ok) const;
+
+    /* @brief Returns the closest keyframe from given position.
+       @param ok is a return parameter to tell if a keyframe was found.
+    */
+    Keyframe getClosestKeyframe(const GenTime &pos, bool *ok) const;
+
     /* @brief Returns true if a keyframe exists at given pos
        Notice that add/remove queries are done in real time (gentime), but this request is made in frame
      */
     Q_INVOKABLE bool hasKeyframe(int frame) const;
+    Q_INVOKABLE bool hasKeyframe(const GenTime &pos) const;
 
     /** @brief returns the keyframes as a Mlt Anim Property string.
         It is defined as pairs of frame and value, separated by ;
@@ -112,6 +135,10 @@ public:
     */
     QString getAnimProperty() const;
 
+    /* @brief Return the interpolated value at given pos */
+    double getInterpolatedValue(int pos) const;
+    double getInterpolatedValue(const GenTime &pos) const;
+
     // Mandatory overloads
     QVariant data(const QModelIndex &index, int role) const override;
     QHash<int, QByteArray> roleNames() const override;
@@ -130,10 +157,14 @@ protected:
 
     /* @brief Connects the signals of this object */
     void setup();
+
+    /* @brief Commit the modification to the model */
+    void sendModification() const;
 private:
 
-    std::weak_ptr<EffectItemModel> m_effect;
+    std::weak_ptr<AssetParameterModel> m_model;
     std::weak_ptr<DocUndoStack> m_undoStack;
+    QPersistentModelIndex m_index;
 
     mutable QReadWriteLock m_lock; // This is a lock that ensures safety in case of concurrent access
 

src/effects/keyframes/view/keyframeview.cpp

+/***************************************************************************
+ *   Copyright (C) 2011 by Till Theato (root@ttill.de)                     *
+ *   This file is part of Kdenlive (www.kdenlive.org).                     *
+ *                                                                         *
+ *   Kdenlive is free software: you can redistribute it and/or modify      *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation, either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ *   Kdenlive is distributed in the hope that it will be useful,           *
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
+ *   GNU General Public License for more details.                          *
+ *                                                                         *
+ *   You should have received a copy of the GNU General Public License     *
+ *   along with Kdenlive.  If not, see <http://www.gnu.org/licenses/>.     *
+ ***************************************************************************/
+
+#include "keyframeview.hpp"
+#include "core.h"
+#include "kdenlivesettings.h"
+
+#include <QMouseEvent>
+#include <QStylePainter>
+
+#include <KColorScheme>
+#include <QFontDatabase>
+
+KeyframeView::KeyframeView(std::shared_ptr<KeyframeModel> model, QWidget *parent)
+    : QWidget(parent)
+    , m_model(model)
+    , m_duration(1)
+    , m_position(0)
+    , m_currentKeyframe(-1)
+    , m_currentKeyframeOriginal(-1)
+    , m_hoverKeyframe(-1)
+    , m_scale(1)
+    , m_currentType(KeyframeType::Linear)
+{
+    setMouseTracking(true);
+    setMinimumSize(QSize(150, 20));
+    setSizePolicy(QSizePolicy(QSizePolicy::Expanding, QSizePolicy::Maximum));
+    setFont(QFontDatabase::systemFont(QFontDatabase::SmallestReadableFont));
+    QPalette p = palette();
+    KColorScheme scheme(p.currentColorGroup(), KColorScheme::Window, KSharedConfig::openConfig(KdenliveSettings::colortheme()));
+    m_colSelected = palette().highlight().color();
+    m_colKeyframe = scheme.foreground(KColorScheme::NormalText).color();
+    m_size = QFontInfo(font()).pixelSize() * 1.8;
+    m_lineHeight = m_size / 2;
+    setMinimumHeight(m_size);
+    setMaximumHeight(m_size);
+    connect(m_model.get(), &KeyframeModel::modelChanged, [&](){
+            emit atKeyframe(m_model->hasKeyframe(m_position));
+            update();
+        });
+}
+
+void KeyframeView::slotSetPosition(int pos)
+{
+    if (pos != m_position) {
+        m_position = pos;
+        emit atKeyframe(m_model->hasKeyframe(pos));
+        emit positionChanged(pos);
+        update();
+    }
+}
+
+void KeyframeView::slotAddKeyframe(int pos, double value)
+{
+    if (pos < 0) {
+        pos = m_position;
+    }
+    m_model->addKeyframe(GenTime(pos, pCore->getCurrentFps()), m_currentType, value);
+}
+
+void KeyframeView::slotAddRemove(double value)
+{
+    if (m_model->hasKeyframe(m_position)) {
+        slotRemoveKeyframe(m_position);
+    } else {
+        slotAddKeyframe(m_position, value);
+    }
+}
+
+void KeyframeView::slotRemoveKeyframe(int pos)
+{
+    if (pos < 0) {
+        pos = m_position;
+    }
+    m_model->removeKeyframe(GenTime(pos, pCore->getCurrentFps()));
+}
+
+void KeyframeView::setDuration(int dur)
+{
+    m_duration = dur;
+}
+
+void KeyframeView::slotGoToNext()
+{
+    if (m_position == m_duration) {
+        return;
+    }
+
+    bool ok;
+    auto next = m_model->getNextKeyframe(GenTime(m_position, pCore->getCurrentFps()), &ok);
+
+    if (ok) {
+        slotSetPosition(next.first.frames(pCore->getCurrentFps()));
+    } else {
+        // no keyframe after current position
+        slotSetPosition(m_duration);
+    }
+}
+
+void KeyframeView::slotGoToPrev()
+{
+    if (m_position == 0) {
+        return;
+    }
+
+    bool ok;
+    auto prev = m_model->getPrevKeyframe(GenTime(m_position, pCore->getCurrentFps()), &ok);
+
+    if (ok) {
+        slotSetPosition(prev.first.frames(pCore->getCurrentFps()));
+    } else {
+        // no keyframe after current position
+        slotSetPosition(m_duration);
+    }
+}
+
+void KeyframeView::mousePressEvent(QMouseEvent *event)
+{
+    int pos = event->x() / m_scale;
+    if (event->y() < m_lineHeight && event->button() == Qt::LeftButton) {
+        bool ok;
+        GenTime position(pos, pCore->getCurrentFps());
+        auto keyframe = m_model->getClosestKeyframe(position, &ok);
+        if (ok && qAbs(keyframe.first.frames(pCore->getCurrentFps()) - pos) < 5) {
+            m_currentKeyframeOriginal = keyframe.first.frames(pCore->getCurrentFps());
+            if (m_model->moveKeyframe(keyframe.first, position, false)) {
+                m_currentKeyframe = pos;
+                slotSetPosition(pos);
+                return;
+            }
+        }
+    }
+
+    // no keyframe next to mouse
+    m_currentKeyframe = m_currentKeyframeOriginal = -1;
+    slotSetPosition(pos);
+    update();
+}
+
+void KeyframeView::mouseMoveEvent(QMouseEvent *event)
+{
+    int pos = qBound(0, (int)(event->x() / m_scale), m_duration);
+    GenTime position(pos, pCore->getCurrentFps());
+    if ((event->buttons() & Qt::LeftButton) != 0u) {
+        if (m_currentKeyframe >= 0) {
+            if (!m_model->hasKeyframe(pos)) {
+                // snap to position cursor
+                if (KdenliveSettings::snaptopoints() && qAbs(pos - m_position) < 5 && !m_model->hasKeyframe(m_position)) {
+                    pos = m_position;
+                }
+                GenTime currentPos(m_currentKeyframe, pCore->getCurrentFps());
+                if (m_model->moveKeyframe(currentPos, position, false)) {
+                    m_currentKeyframe = pos;
+                }
+            }
+        }
+        slotSetPosition(pos);
+        return;
+    }
+    if (event->y() < m_lineHeight) {
+        bool ok;
+        auto keyframe = m_model->getClosestKeyframe(position, &ok);
+        if (ok && qAbs(keyframe.first.frames(pCore->getCurrentFps()) - pos) < 5) {
+            m_hoverKeyframe = keyframe.first.frames(pCore->getCurrentFps());
+            setCursor(Qt::PointingHandCursor);
+            update();
+            return;
+        }
+    }
+
+    if (m_hoverKeyframe != -1) {
+        m_hoverKeyframe = -1;
+        setCursor(Qt::ArrowCursor);
+        update();
+    }
+}
+
+void KeyframeView::mouseReleaseEvent(QMouseEvent *event)
+{
+    Q_UNUSED(event)
+
+    if (m_currentKeyframe >= 0) {
+        GenTime initPos(m_currentKeyframeOriginal, pCore->getCurrentFps());
+        GenTime targetPos(m_currentKeyframe, pCore->getCurrentFps());
+        m_model->moveKeyframe(targetPos, initPos, false);
+        m_model->moveKeyframe(initPos, targetPos, true);
+    }
+}
+
+void KeyframeView::mouseDoubleClickEvent(QMouseEvent *event)
+{
+    if (event->button() == Qt::LeftButton && event->y() < m_lineHeight) {
+        int pos = qBound(0, (int)(event->x() / m_scale), m_duration);
+        GenTime position(pos, pCore->getCurrentFps());
+        bool ok;
+        auto keyframe = m_model->getClosestKeyframe(position, &ok);
+        if (ok && qAbs(keyframe.first.frames(pCore->getCurrentFps()) - pos) < 5) {
+            m_model->removeKeyframe(keyframe.first);
+            if (keyframe.first.frames(pCore->getCurrentFps()) == m_currentKeyframe) {
+                m_currentKeyframe = m_currentKeyframeOriginal = -1;
+            }
+            if (keyframe.first.frames(pCore->getCurrentFps()) == m_position) {
+                emit atKeyframe(false);
+            }
+            return;
+        }
+
+        // add new keyframe
+        double value = m_model->getInterpolatedValue(pos);
+        m_model->addKeyframe(position, m_currentType, value);
+    } else {
+        QWidget::mouseDoubleClickEvent(event);
+    }
+}
+
+void KeyframeView::wheelEvent(QWheelEvent *event)
+{
+    int change = event->delta() < 0 ? -1 : 1;
+    int pos = qBound(0, m_position + change, m_duration);
+    slotSetPosition(pos);
+}
+
+void KeyframeView::paintEvent(QPaintEvent *event)
+{
+    Q_UNUSED(event)
+
+    QStylePainter p(this);
+    m_scale = width() / (double)(m_duration);
+    // p.translate(0, m_lineHeight);
+    int headOffset = m_lineHeight / 1.5;
+
+    /*
+     * keyframes
+     */
+    for (const auto &keyframe : *m_model.get()) {
+        int pos = keyframe.first.frames(pCore->getCurrentFps());
+        if (pos == m_currentKeyframe || pos == m_hoverKeyframe) {
+            p.setBrush(m_colSelected);
+        } else {
+            p.setBrush(m_colKeyframe);
+        }
+        int scaledPos = pos * m_scale;
+        p.drawLine(scaledPos, headOffset, scaledPos, m_lineHeight + (headOffset / 2));
+        p.drawEllipse(scaledPos - headOffset / 2, 0, headOffset, headOffset);
+    }
+
+    p.setPen(palette().dark().color());