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 57417d35 authored by Nicolas Carion's avatar Nicolas Carion

[Timeline2][Model] Undoable track insertion and deletion + tests

parent 9f047f78
......@@ -66,9 +66,6 @@ std::shared_ptr<TimelineItemModel> TimelineItemModel::construct(std::weak_ptr<Do
TimelineItemModel::~TimelineItemModel()
{
for(auto tracks : m_iteratorTable) {
deleteTrackById(tracks.first);
}
}
QModelIndex TimelineItemModel::index(int row, int column, const QModelIndex &parent) const
......
......@@ -55,8 +55,12 @@ TimelineModel::TimelineModel(std::weak_ptr<DocUndoStack> undo_stack) :
TimelineModel::~TimelineModel()
{
std::vector<int> all_ids;
for(auto tracks : m_iteratorTable) {
deleteTrackById(tracks.first);
all_ids.push_back(tracks.first);
}
for(auto tracks : all_ids) {
deregisterTrack_lambda(tracks)();
}
}
......@@ -74,14 +78,6 @@ int TimelineModel::getClipsCount() const
}
void TimelineModel::deleteTrackById(int id)
{
Q_ASSERT(m_iteratorTable.count(id) > 0);
auto it = m_iteratorTable[id];
(*it)->destruct();
}
int TimelineModel::getClipTrackId(int cid) const
{
Q_ASSERT(m_allClips.count(cid) > 0);
......@@ -449,7 +445,64 @@ bool TimelineModel::requestUngroupClip(int id)
return result;
}
void TimelineModel::registerTrack(std::unique_ptr<TrackModel>&& track, int pos)
bool TimelineModel::requestTrackInsertion(int position, int &id)
{
if (position == -1) {
position = (int)(m_allTracks.size());
}
if (position < 0 || position > (int)m_allTracks.size()) {
return false;
}
int trackId = TimelineModel::getNextId();
id = trackId;
Fun undo = deregisterTrack_lambda(trackId);
TrackModel::construct(shared_from_this(), trackId, position);
auto track = getTrackById(trackId);
Fun redo = [track, position, this](){
// We capture a shared_ptr to the track, which means that as long as this undo object lives, the track object is not deleted. To insert it back it is sufficient to register it.
registerTrack(track, position);
return true;
};
PUSH_UNDO(undo, redo, i18n("Insert Track"));
return true;
}
bool TimelineModel::requestTrackDeletion(int tid)
{
Q_ASSERT(isTrack(tid));
std::vector<int> clips_to_delete;
for(const auto& it : getTrackById(tid)->m_allClips) {
clips_to_delete.push_back(it.first);
}
Fun undo = [](){return true;};
Fun redo = [](){return true;};
for(int clip : clips_to_delete) {
bool res = requestClipDeletion(clip, undo, redo);
if (!res) {
bool u = undo();
Q_ASSERT(u);
return false;
}
}
int old_position = getTrackPosition(tid);
auto operation = deregisterTrack_lambda(tid);
std::shared_ptr<TrackModel> track = getTrackById(tid);
auto reverse = [this, track, old_position]() {
// We capture a shared_ptr to the track, which means that as long as this undo object lives, the track object is not deleted. To insert it back it is sufficient to register it.
registerTrack(track, old_position);
return true;
};
if (operation()) {
UPDATE_UNDO_REDO(operation, reverse, undo, redo);
PUSH_UNDO(undo, redo, i18n("Delete Track"));
return true;
}
undo();
return false;
}
void TimelineModel::registerTrack(std::shared_ptr<TrackModel> track, int pos)
{
int id = track->getId();
if (pos == -1) {
......@@ -485,13 +538,16 @@ void TimelineModel::registerGroup(int groupId)
m_allGroups.insert(groupId);
}
void TimelineModel::deregisterTrack(int id)
Fun TimelineModel::deregisterTrack_lambda(int id)
{
auto it = m_iteratorTable[id]; //iterator to the element
auto index = getTrackPosition(id); //compute index in list
m_tractor->remove_track(static_cast<int>(index)); //melt operation
m_allTracks.erase(it); //actual deletion of object
m_iteratorTable.erase(id); //clean table
return [this, id]() {
auto it = m_iteratorTable[id]; //iterator to the element
auto index = getTrackPosition(id); //compute index in list
m_tractor->remove_track(static_cast<int>(index)); //melt operation
m_allTracks.erase(it); //actual deletion of object
m_iteratorTable.erase(id); //clean table
return true;
};
}
Fun TimelineModel::deregisterClip_lambda(int cid)
......@@ -512,13 +568,13 @@ void TimelineModel::deregisterGroup(int id)
m_allGroups.erase(id);
}
std::unique_ptr<TrackModel>& TimelineModel::getTrackById(int tid)
std::shared_ptr<TrackModel> TimelineModel::getTrackById(int tid)
{
Q_ASSERT(m_iteratorTable.count(tid) > 0);
return *m_iteratorTable[tid];
}
const std::unique_ptr<TrackModel>& TimelineModel::getTrackById_const(int tid) const
const std::shared_ptr<TrackModel> TimelineModel::getTrackById_const(int tid) const
{
Q_ASSERT(m_iteratorTable.count(tid) > 0);
return *m_iteratorTable.at(tid);
......
......@@ -81,9 +81,6 @@ public:
/* @brief returns the number of clips */
int getClipsCount() const;
/* @brief Delete track based on its id */
void deleteTrackById(int id);
/* @brief Returns the id of the track containing clip (-1 if it is not inserted)
@param cid Id of the clip to test
*/
......@@ -110,6 +107,7 @@ public:
int getTrackPosition(int tid) const;
/* @brief Move a clip to a specific position
This action is undoable
Returns true on success. If it fails, nothing is modified.
If the clip is not in inserted in a track yet, it gets inserted for the first time.
If the clip is in a group, the call is deferred to requestGroupMove
......@@ -143,6 +141,7 @@ public:
bool requestClipInsertion(std::shared_ptr<Mlt::Producer> prod, int trackId, int position, int &id);
/* @brief Deletes the given clip from the timeline
This action is undoable
Returns true on success. If it fails, nothing is modified.
If the clip is in a group, the call is deferred to requestGroupDeletion
@param cid is the ID of the clip
......@@ -152,6 +151,7 @@ public:
bool requestClipDeletion(int cid, Fun &undo, Fun &redo);
/* @brief Move a group to a specific position
This action is undoable
Returns true on success. If it fails, nothing is modified.
If the clips in the group are not in inserted in a track yet, they get inserted for the first time.
@param cid is the id of the clip that triggers the group move
......@@ -164,6 +164,7 @@ public:
bool requestGroupMove(int cid, int gid, int delta_track, int delta_pos, bool updateView = true, bool logUndo = true);
/* @brief Deletes all clips inside the group that contains the given clip.
This action is undoable
Note that if their is a hierarchy of groups, all of them will be deleted.
Returns true on success. If it fails, nothing is modified.
@param cid is the id of the clip that triggers the group deletion
......@@ -171,6 +172,7 @@ public:
bool requestGroupDeletion(int cid);
/* @brief Change the duration of a clip
This action is undoable
Returns true on success. If it fails, nothing is modified.
@param cid is the ID of the clip
@param size is the new size of the clip
......@@ -180,6 +182,7 @@ public:
bool requestClipResize(int cid, int size, bool right, bool logUndo = true);
/* @brief Similar to requestClipResize but takes a delta instead of absolute size
This action is undoable
Returns true on success. If it fails, nothing is modified.
@param cid is the ID of the clip
@param delta is the delta to be applied to the length of the clip
......@@ -190,6 +193,7 @@ public:
bool requestClipTrim(int cid, int delta, bool right, bool ripple = false, bool test_only = false);
/* @brief Group together a set of ids
This action is undoable
Returns true on success. If it fails, nothing is modified.
Typically, ids would be ids of clips, but for convenience, some of them can be ids of groups as well.
@param ids Set of ids to group
......@@ -197,11 +201,28 @@ public:
bool requestGroupClips(const std::unordered_set<int>& ids);
/* @brief Destruct the topmost group containing clip
This action is undoable
Returns true on success. If it fails, nothing is modified.
@param id of the clip to degroup (all clips belonging to the same group will be ungrouped as well)
*/
bool requestUngroupClip(int id);
/* @brief Create a track at given position
This action is undoable
Returns true on success. If it fails, nothing is modified.
@param Requested position (order). If set to -1, the track is inserted last.
@param id is a return parameter that holds the id of the resulting track (-1 on failure)
*/
bool requestTrackInsertion(int pos, int& id);
/* @brief Delete track with given id
This also deletes all the clips contained in the track.
This action is undoable
Returns true on success. If it fails, nothing is modified.
@param tid id of the track to delete
*/
bool requestTrackDeletion(int tid);
/* @brief Get project duration
Returns the duration in frames
*/
......@@ -211,7 +232,7 @@ protected:
/* @brief Register a new track. This is a call-back meant to be called from TrackModel
@param pos indicates the number of the track we are adding. If this is -1, then we add at the end.
*/
void registerTrack(std::unique_ptr<TrackModel>&& track, int pos = -1);
void registerTrack(std::shared_ptr<TrackModel> track, int pos = -1);
/* @brief Register a new track. This is a call-back meant to be called from ClipModel
*/
......@@ -223,7 +244,7 @@ protected:
/* @brief Deregister and destruct the track with given id.
*/
void deregisterTrack(int id);
Fun deregisterTrack_lambda(int id);
/* @brief Return a lambda that deregisters and destructs the clip with given id.
Note that the clip must already be deleted from its track and groups.
......@@ -236,8 +257,8 @@ protected:
/* @brief Helper function to get a pointer to the track, given its id
*/
std::unique_ptr<TrackModel>& getTrackById(int tid);
const std::unique_ptr<TrackModel>& getTrackById_const(int tid) const;
std::shared_ptr<TrackModel> getTrackById(int tid);
const std::shared_ptr<TrackModel> getTrackById_const(int tid) const;
/*@brief Helper function to get a pointer to a clip, given its id*/
std::shared_ptr<ClipModel> getClipPtr(int cid) const;
......@@ -260,9 +281,9 @@ protected:
protected:
std::unique_ptr<Mlt::Tractor> m_tractor;
std::list<std::unique_ptr<TrackModel>> m_allTracks;
std::list<std::shared_ptr<TrackModel>> m_allTracks;
std::unordered_map<int, std::list<std::unique_ptr<TrackModel>>::iterator> m_iteratorTable; //this logs the iterator associated which each track id. This allows easy access of a track based on its id.
std::unordered_map<int, std::list<std::shared_ptr<TrackModel>>::iterator> m_iteratorTable; //this logs the iterator associated which each track id. This allows easy access of a track based on its id.
std::unordered_map<int, std::shared_ptr<ClipModel>> m_allClips; //the keys are the clip id, and the values are the corresponding pointers
......
......@@ -27,16 +27,16 @@
TrackModel::TrackModel(std::weak_ptr<TimelineModel> parent) :
TrackModel::TrackModel(std::weak_ptr<TimelineModel> parent, int id) :
m_parent(parent)
, m_id(TimelineModel::getNextId())
, m_id(id == -1 ? TimelineModel::getNextId() : id)
{
}
int TrackModel::construct(std::weak_ptr<TimelineModel> parent, int pos)
int TrackModel::construct(std::weak_ptr<TimelineModel> parent, int id, int pos)
{
std::unique_ptr<TrackModel> track(new TrackModel(parent));
int id = track->m_id;
std::shared_ptr<TrackModel> track(new TrackModel(parent, id));
id = track->m_id;
if (auto ptr = parent.lock()) {
ptr->registerTrack(std::move(track), pos);
} else {
......@@ -46,13 +46,6 @@ int TrackModel::construct(std::weak_ptr<TimelineModel> parent, int pos)
return id;
}
void TrackModel::destruct()
{
if (auto ptr = m_parent.lock()) {
ptr->deregisterTrack(m_id);
}
}
int TrackModel::getClipsCount()
{
int count = 0;
......
......@@ -47,17 +47,14 @@ public:
friend class ClipModel;
private:
/* This constructor is private, call the static construct instead */
TrackModel(std::weak_ptr<TimelineModel> parent);
TrackModel(std::weak_ptr<TimelineModel> parent, int id = -1);
public:
/* @brief Creates a track, which references itself to the parent
Returns the (unique) id of the created track
@param id Requested id of the track. Automatic if id = -1
@param pos is the optional position of the track. If left to -1, it will be added at the end
*/
static int construct(std::weak_ptr<TimelineModel> parent, int pos = -1);
/* @brief The destructor. It asks the parent to be deleted
*/
void destruct();
static int construct(std::weak_ptr<TimelineModel> parent, int id = -1, int pos = -1);
/* @brief returns the number of clips */
int getClipsCount();
......
......@@ -26,22 +26,37 @@ TEST_CASE("Basic creation/deletion of a track", "[TrackModel]")
int id1 = TrackModel::construct(timeline);
REQUIRE(timeline->getTracksCount() == 1);
REQUIRE(timeline->getTrackPosition(id1) == 0);
int id2 = TrackModel::construct(timeline);
REQUIRE(timeline->getTracksCount() == 2);
REQUIRE(timeline->getTrackPosition(id2) == 1);
int id3 = TrackModel::construct(timeline);
REQUIRE(timeline->getTracksCount() == 3);
REQUIRE(timeline->getTrackPosition(id3) == 2);
int id4;
REQUIRE(timeline->requestTrackInsertion(1, id4));
REQUIRE(timeline->getTracksCount() == 4);
REQUIRE(timeline->getTrackPosition(id1) == 0);
REQUIRE(timeline->getTrackPosition(id4) == 1);
REQUIRE(timeline->getTrackPosition(id2) == 2);
REQUIRE(timeline->getTrackPosition(id3) == 3);
// Test deletion
timeline->deleteTrackById(id1);
REQUIRE(timeline->requestTrackDeletion(id3));
REQUIRE(timeline->getTracksCount() == 3);
REQUIRE(timeline->requestTrackDeletion(id1));
REQUIRE(timeline->getTracksCount() == 2);
timeline->deleteTrackById(id2);
REQUIRE(timeline->requestTrackDeletion(id4));
REQUIRE(timeline->getTracksCount() == 1);
timeline->deleteTrackById(id3);
REQUIRE(timeline->requestTrackDeletion(id2));
REQUIRE(timeline->getTracksCount() == 0);
}
......@@ -951,6 +966,67 @@ TEST_CASE("Undo and Redo", "[ClipModel]")
undoStack->redo();
state2();
}
SECTION("Track insertion undo") {
int nb_tracks = timeline->getTracksCount();
std::map<int, int> orig_trackPositions, final_trackPositions;
for (const auto& it : timeline->m_iteratorTable) {
int track = it.first;
int pos = timeline->getTrackPosition(track);
orig_trackPositions[track] = pos;
if (pos >= 1) pos++;
final_trackPositions[track] = pos;
}
auto checkPositions = [&](const std::map<int,int>& pos) {
for (const auto& p : pos ) {
REQUIRE(timeline->getTrackPosition(p.first) == p.second);
}
};
checkPositions(orig_trackPositions);
int new_tid;
REQUIRE(timeline->requestTrackInsertion(1, new_tid));
checkPositions(final_trackPositions);
undoStack->undo();
checkPositions(orig_trackPositions);
undoStack->redo();
checkPositions(final_trackPositions);
undoStack->undo();
checkPositions(orig_trackPositions);
}
SECTION("Track deletion undo") {
int nb_clips = timeline->getClipsCount();
int nb_tracks = timeline->getTracksCount();
REQUIRE(timeline->requestClipMove(cid1, tid1, 5));
auto state1 = [&]() {
REQUIRE(timeline->getTrackById(tid1)->checkConsistency());
REQUIRE(timeline->getTrackClipsCount(tid1) == 1);
REQUIRE(timeline->getClipTrackId(cid1) == tid1);
REQUIRE(timeline->getClipPosition(cid1) == 5);
REQUIRE(undoStack->index() == init_index + 1);
REQUIRE(timeline->getClipsCount() == nb_clips);
REQUIRE(timeline->getTracksCount() == nb_tracks);
};
state1();
REQUIRE(timeline->requestTrackDeletion(tid1));
REQUIRE(timeline->getClipsCount() == nb_clips - 1);
REQUIRE(timeline->getTracksCount() == nb_tracks - 1);
undoStack->undo();
state1();
undoStack->redo();
REQUIRE(timeline->getClipsCount() == nb_clips - 1);
REQUIRE(timeline->getTracksCount() == nb_tracks - 1);
undoStack->undo();
state1();
}
}
TEST_CASE("Snapping", "[Snapping]") {
......
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