Members of the KDE Community are recommended to subscribe to the kde-community mailing list at https://mail.kde.org/mailman/listinfo/kde-community to allow them to participate in important discussions and receive other important announcements

Commit 3449a5f3 authored by Nicolas Carion's avatar Nicolas Carion

Rework snapping of guides (and fix memory management)

parent 9cb480fd
......@@ -28,26 +28,25 @@
#include "kdenlivesettings.h"
#include "macros.hpp"
#include "project/projectmanager.h"
#include "timeline2/model/snapmodel.hpp"
#include <QDebug>
#include <klocalizedstring.h>
std::array<QColor, 5> MarkerListModel::markerTypes = {Qt::red, Qt::blue, Qt::green, Qt::yellow, Qt::cyan};
std::array<QColor, 5> MarkerListModel::markerTypes = {{Qt::red, Qt::blue, Qt::green, Qt::yellow, Qt::cyan}};
MarkerListModel::MarkerListModel(const QString &clipId, std::weak_ptr<DocUndoStack> undo_stack, QObject *parent)
: QAbstractListModel(parent)
, m_undoStack(std::move(undo_stack))
, m_guide(false)
, m_clipId(clipId)
, m_snaps(nullptr)
{
}
MarkerListModel::MarkerListModel(std::weak_ptr<DocUndoStack> undo_stack, std::unique_ptr<SnapModel> &snapModel, QObject *parent)
MarkerListModel::MarkerListModel(std::weak_ptr<DocUndoStack> undo_stack, QObject *parent)
: QAbstractListModel(parent)
, m_undoStack(std::move(undo_stack))
, m_guide(true)
, m_snaps(snapModel.get())
{
}
......@@ -55,7 +54,7 @@ void MarkerListModel::addMarker(GenTime pos, const QString &comment, int type)
{
QWriteLocker locker(&m_lock);
if (type == -1) type = KdenliveSettings::default_marker_type();
Q_ASSERT(type >= 0 && type < markerTypes.size());
Q_ASSERT(type >= 0 && type < (int)markerTypes.size());
if (m_markerList.count(pos) > 0) {
// In this case we simply change the comment and type
QString oldComment = m_markerList[pos].first;
......@@ -160,16 +159,28 @@ QHash<int, QByteArray> MarkerListModel::roleNames() const
void MarkerListModel::addSnapPoint(GenTime pos)
{
if (m_snaps) {
m_snaps->addPoint(pos.frames(pCore->getCurrentFps()));
std::vector<std::weak_ptr<SnapModel>> validSnapModels;
for (const auto& snapModel : m_registeredSnaps) {
if (auto ptr = snapModel.lock()) {
validSnapModels.push_back(snapModel);
ptr->addPoint(pos.frames(pCore->getCurrentFps()));
}
}
// Update the list of snapModel known to be valid
std::swap(m_registeredSnaps, validSnapModels);
}
void MarkerListModel::removeSnapPoint(GenTime pos)
{
if (m_snaps) {
m_snaps->removePoint(pos.frames(pCore->getCurrentFps()));
std::vector<std::weak_ptr<SnapModel>> validSnapModels;
for (const auto& snapModel : m_registeredSnaps) {
if (auto ptr = snapModel.lock()) {
validSnapModels.push_back(snapModel);
ptr->removePoint(pos.frames(pCore->getCurrentFps()));
}
}
// Update the list of snapModel known to be valid
std::swap(m_registeredSnaps, validSnapModels);
}
QVariant MarkerListModel::data(const QModelIndex &index, int role) const
......@@ -189,7 +200,7 @@ QVariant MarkerListModel::data(const QModelIndex &index, int role) const
case FrameRole:
return it->first.frames(pCore->getCurrentFps());
case ColorRole:
return markerTypes[it->second.second];
return markerTypes[(size_t)it->second.second];
}
return QVariant();
}
......@@ -216,3 +227,9 @@ bool MarkerListModel::hasMarker(int frame) const
}
void MarkerListModel::registerSnapModel(std::weak_ptr<SnapModel> snapModel)
{
Q_ASSERT(m_guide);
m_registeredSnaps.push_back(snapModel);
}
......@@ -25,7 +25,6 @@
#include "gentime.h"
#include "definitions.h"
#include "undohelper.hpp"
#include "timeline2/model/snapmodel.hpp"
#include <QAbstractListModel>
#include <QReadWriteLock>
......@@ -34,6 +33,7 @@
#include <memory>
class DocUndoStack;
class SnapModel;
/* @brief This class is the model for a list of markers.
......@@ -53,7 +53,7 @@ public:
explicit MarkerListModel(const QString &clipId, std::weak_ptr<DocUndoStack> undo_stack, QObject *parent = nullptr);
/* @brief Construct a guide list (bound to the timeline) */
MarkerListModel(std::weak_ptr<DocUndoStack> undo_stack, std::unique_ptr<SnapModel> &snapModel, QObject *parent = nullptr);
MarkerListModel(std::weak_ptr<DocUndoStack> undo_stack, QObject *parent = nullptr);
enum { CommentRole = Qt::UserRole + 1, PosRole, FrameRole, ColorRole };
......@@ -67,24 +67,38 @@ public:
/* @brief Removes the marker at the given position. */
void removeMarker(GenTime pos);
/** This describes the available markers type and their corresponding colors */
/* @brief This describes the available markers type and their corresponding colors */
static std::array<QColor, 5> markerTypes;
/** Returns a marker data at given pos */
/* @brief Returns a marker data at given pos */
CommentedTime getMarker(const GenTime &pos) const;
/* @brief Returns true if a marker 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 hasMarker(int frame) const;
/* @brief Registers a snapModel to the marker model.
This is intended to be used only for a guide model, so that the timelines can register their snapmodel to be updated when the guide moves.
The snap logic for clips is managed from the Timeline
Note that no deregistration is necessary, the weak_ptr will be discarded as soon as it becomes invalid.
*/
void registerSnapModel(std::weak_ptr<SnapModel> snapModel);
// Mandatory overloads
QVariant data(const QModelIndex &index, int role) const override;
QHash<int, QByteArray> roleNames() const override;
int rowCount(const QModelIndex &parent = QModelIndex()) const override;
/** Returns true if a marker exists at given pos */
Q_INVOKABLE bool hasMarker(int frame) const;
/** Adds a snap point at marker position */
protected:
/* @brief Adds a snap point at marker position in the registered snap models
(those that are still valid)*/
void addSnapPoint(GenTime pos);
/* @brief Deletes a snap point at marker position in the registered snap models
(those that are still valid)*/
void removeSnapPoint(GenTime pos);
protected:
/** @brief Helper function that generate a lambda to change comment / type of given marker */
Fun changeComment_lambda(GenTime pos, const QString &comment, int type);
......@@ -106,7 +120,7 @@ private:
mutable QReadWriteLock m_lock; // This is a lock that ensures safety in case of concurrent access
std::map<GenTime, std::pair<QString, int>> m_markerList;
std::unique_ptr<SnapModel> m_snaps;
std::vector<std::weak_ptr<SnapModel>> m_registeredSnaps;
public:
// this is to enable for range loops
......
......@@ -28,7 +28,6 @@
#include "documentvalidator.h"
#include "docundostack.hpp"
#include "effectslist/initeffects.h"
#include "timeline2/model/snapmodel.hpp"
#include "kdenlivesettings.h"
#include "mainwindow.h"
#include "renderer.h"
......@@ -81,10 +80,9 @@ KdenliveDoc::KdenliveDoc(const QUrl &url, const QString &projectFolder, QUndoGro
, m_height(0)
, m_modified(false)
, m_projectFolder(projectFolder)
, m_snaps(new SnapModel())
{
m_commandStack = std::make_shared<DocUndoStack>(undoGroup);
m_guideModel.reset(new MarkerListModel(m_commandStack, m_snaps, this));
m_guideModel.reset(new MarkerListModel(m_commandStack, this));
// init m_profile struct
m_profile.frame_rate_num = 0;
......@@ -1873,7 +1871,3 @@ void KdenliveDoc::addGuides(QList<CommentedTime> &markers)
}
}
std::unique_ptr<SnapModel> &KdenliveDoc::snapModel()
{
return m_snaps;
}
......@@ -50,7 +50,6 @@ class ProjectClip;
class ClipController;
class MarkerListModel;
class Render;
class SnapModel;
class QTextEdit;
class QUndoGroup;
......@@ -173,9 +172,6 @@ public:
/** @brief Edit timeline guide */
void addGuides(QList<CommentedTime> &markers);
/** @brief Returns a pointer to snap model */
std::unique_ptr<SnapModel> &snapModel();
// TODO REFAC: delete */
Render *renderer();
......@@ -203,7 +199,6 @@ private:
QList<int> m_undoChunks;
QMap<QString, QString> m_documentProperties;
QMap<QString, QString> m_documentMetadata;
std::unique_ptr<SnapModel> m_snaps;
std::shared_ptr<MarkerListModel> m_guideModel;
QString searchFileRecursively(const QDir &dir, const QString &matchSize, const QString &matchHash) const;
......
......@@ -881,7 +881,7 @@ void ProjectManager::updateTimeline(Mlt::Tractor tractor)
pCore->bin()->slotProducerReady(info, pCore->binController()->getController(id).get());
}
pCore->binController()->setBinPlaylist(&tractor);
m_mainTimelineModel = TimelineItemModel::construct(&pCore->getCurrentProfile()->profile(), m_project->snapModel(), m_project->commandStack());
m_mainTimelineModel = TimelineItemModel::construct(&pCore->getCurrentProfile()->profile(), m_project->getGuideModel(), m_project->commandStack());
constructTimelineFromMelt(m_mainTimelineModel, tractor);
// TODO this is for testing purposes, remove.
......
......@@ -35,15 +35,16 @@
#include <mlt++/MltTransition.h>
#include <utility>
TimelineItemModel::TimelineItemModel(Mlt::Profile *profile, std::unique_ptr<SnapModel> &snapModel, std::weak_ptr<DocUndoStack> undo_stack)
: TimelineModel(profile, snapModel, undo_stack)
TimelineItemModel::TimelineItemModel(Mlt::Profile *profile, std::weak_ptr<DocUndoStack> undo_stack)
: TimelineModel(profile, undo_stack)
{
}
std::shared_ptr<TimelineItemModel> TimelineItemModel::construct(Mlt::Profile *profile, std::unique_ptr<SnapModel> &snapModel, std::weak_ptr<DocUndoStack> undo_stack)
std::shared_ptr<TimelineItemModel> TimelineItemModel::construct(Mlt::Profile *profile, std::shared_ptr<MarkerListModel> guideModel, std::weak_ptr<DocUndoStack> undo_stack)
{
std::shared_ptr<TimelineItemModel> ptr(new TimelineItemModel(profile, snapModel, std::move(undo_stack)));
std::shared_ptr<TimelineItemModel> ptr(new TimelineItemModel(profile, std::move(undo_stack)));
ptr->m_groups = std::unique_ptr<GroupsModel>(new GroupsModel(ptr));
guideModel->registerSnapModel(ptr->m_snaps);
return ptr;
}
......
......@@ -43,6 +43,8 @@
*/
class MarkerListModel;
class TimelineItemModel : public TimelineModel
{
Q_OBJECT
......@@ -50,15 +52,16 @@ class TimelineItemModel : public TimelineModel
public:
/* @brief construct a timeline object and returns a pointer to the created object
@param undo_stack is a weak pointer to the undo stack of the project
@param guideModel ptr to the guide model of the project
*/
static std::shared_ptr<TimelineItemModel> construct(Mlt::Profile *profile, std::unique_ptr<SnapModel> &snapModel, std::weak_ptr<DocUndoStack> undo_stack);
static std::shared_ptr<TimelineItemModel> construct(Mlt::Profile *profile, std::shared_ptr<MarkerListModel> guideModel, std::weak_ptr<DocUndoStack> undo_stack );
friend bool constructTimelineFromMelt(const std::shared_ptr<TimelineItemModel> &timeline, Mlt::Tractor mlt_timeline);
protected:
/* @brief this constructor should not be called. Call the static construct instead
*/
TimelineItemModel(Mlt::Profile *profile, std::unique_ptr<SnapModel> &snapModel, std::weak_ptr<DocUndoStack> undo_stack);
TimelineItemModel(Mlt::Profile *profile, std::weak_ptr<DocUndoStack> undo_stack);
public:
~TimelineItemModel();
......
......@@ -46,10 +46,10 @@
int TimelineModel::next_id = 0;
TimelineModel::TimelineModel(Mlt::Profile *profile, std::unique_ptr<SnapModel> &snapModel, std::weak_ptr<DocUndoStack> undo_stack)
TimelineModel::TimelineModel(Mlt::Profile *profile, std::weak_ptr<DocUndoStack> undo_stack)
: QAbstractItemModel()
, m_tractor(new Mlt::Tractor(*profile))
, m_snaps(snapModel.get())
, m_snaps(new SnapModel())
, m_undoStack(undo_stack)
, m_profile(profile)
, m_blackClip(new Mlt::Producer(*profile, "color:black"))
......
......@@ -86,7 +86,7 @@ class TimelineModel : public QAbstractItemModel, public std::enable_shared_from_
protected:
/* @brief this constructor should not be called. Call the static construct instead
*/
TimelineModel(Mlt::Profile *profile, std::unique_ptr<SnapModel> &snapModel, std::weak_ptr<DocUndoStack> undo_stack);
TimelineModel(Mlt::Profile *profile, std::weak_ptr<DocUndoStack> undo_stack);
public:
friend class TrackModel;
......@@ -477,9 +477,6 @@ protected:
/* @brief Debugging function that checks consistency with Mlt objects */
bool checkConsistency();
/* @brief Returns snap model */
std::unique_ptr<SnapModel> &getSnapModel();
void checkRefresh(int start, int end) const;
protected:
......@@ -498,7 +495,7 @@ protected:
static int next_id; // next valid id to assign
std::unique_ptr<GroupsModel> m_groups;
std::unique_ptr<SnapModel> m_snaps;
std::shared_ptr<SnapModel> m_snaps;
std::unordered_set<int> m_allGroups; // ids of all the groups
......
......@@ -10,11 +10,11 @@
#include <mlt++/MltRepository.h>
#define private public
#define protected public
#include "bin/model/markerlistmodel.hpp"
#include "timeline2/model/clipmodel.hpp"
#include "timeline2/model/compositionmodel.hpp"
#include "timeline2/model/timelineitemmodel.hpp"
#include "timeline2/model/timelinemodel.hpp"
#include "timeline2/model/snapmodel.hpp"
#include "timeline2/model/trackmodel.hpp"
#include "transitions/transitionsrepository.hpp"
......@@ -41,8 +41,8 @@ TEST_CASE("Basic creation/deletion of a composition", "[CompositionModel]")
REQUIRE(mlt_transition->is_valid());
std::shared_ptr<DocUndoStack> undoStack = std::make_shared<DocUndoStack>(nullptr);
std::unique_ptr<SnapModel> snap(new SnapModel());
std::shared_ptr<TimelineItemModel> timeline = TimelineItemModel::construct(new Mlt::Profile(), snap, undoStack);
std::shared_ptr<MarkerListModel> guideModel(new MarkerListModel(undoStack));
std::shared_ptr<TimelineItemModel> timeline = TimelineItemModel::construct(new Mlt::Profile(), guideModel, undoStack);
REQUIRE(timeline->getCompositionsCount() == 0);
int id1 = CompositionModel::construct(timeline, aCompo);
......@@ -66,8 +66,8 @@ TEST_CASE("Basic creation/deletion of a composition", "[CompositionModel]")
TEST_CASE("Composition manipulation", "[CompositionModel]")
{
std::shared_ptr<DocUndoStack> undoStack = std::make_shared<DocUndoStack>(nullptr);
std::unique_ptr<SnapModel> snap(new SnapModel());
std::shared_ptr<TimelineItemModel> timeline = TimelineItemModel::construct(new Mlt::Profile(), snap, undoStack);
std::shared_ptr<MarkerListModel> guideModel(new MarkerListModel(undoStack));
std::shared_ptr<TimelineItemModel> timeline = TimelineItemModel::construct(new Mlt::Profile(), guideModel, undoStack);
int tid0 = TrackModel::construct(timeline);
int tid1 = TrackModel::construct(timeline);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment