Skip to content
GitLab
Commit 2dad6ae4 authored by Jean-Baptiste Mardelle's avatar Jean-Baptiste Mardelle
Browse files

Fix some spacer inconsistencies when used with groups, add some tests

parent 4147467f
Pipeline #255246 canceled with stage
in 34 seconds
Hide whitespace changes
Inline Side-by-side
src/timeline2/model/timelinefunctions.cpp
View file @ 2dad6ae4
......@@ -320,11 +320,15 @@ bool TimelineFunctions::requestClipCutAll(std::shared_ptr<TimelineItemModel> tim
return count > 0;
}
int TimelineFunctions::requestSpacerStartOperation(const std::shared_ptr<TimelineItemModel> &timeline, int trackId, int position, bool ignoreMultiTrackGroups,
bool allowGroupBreaking)
std::pair<int, int> TimelineFunctions::requestSpacerStartOperation(const std::shared_ptr<TimelineItemModel> &timeline, int trackId, int position,
bool ignoreMultiTrackGroups, bool allowGroupBreaking)
{
std::unordered_set<int> clips = timeline->getItemsInRange(trackId, position, -1);
timeline->requestClearSelection();
// Find the first clip on each track to calculate the minimum space operation
QMap<int, int> firstClipOnTrack;
// Find the maximum space allowed by grouped clips placed before the operation start {trackid,blank_duration}
QMap<int, int> relatedMaxSpace;
spacerMinPosition = -1;
if (!clips.empty()) {
// Remove grouped items that are before the click position
......@@ -335,8 +339,14 @@ int TimelineFunctions::requestSpacerStartOperation(const std::shared_ptr<Timelin
std::unordered_set<int> groupsToRemove;
int firstCid = -1;
int firstPosition = -1;
QMap<int, int> firstPositions;
int spaceDuration = -1;
std::unordered_set<int> toSelect;
// List all clips involved in the spacer operation
std::unordered_set<int> allClips;
for (int r : roots) {
std::unordered_set<int> children = timeline->m_groups->getLeaves(r);
allClips.insert(children.begin(), children.end());
}
for (int r : roots) {
if (timeline->isGroup(r)) {
std::unordered_set<int> leaves = timeline->m_groups->getLeaves(r);
......@@ -344,7 +354,7 @@ int TimelineFunctions::requestSpacerStartOperation(const std::shared_ptr<Timelin
std::unordered_set<int> leavesToKeep;
for (int l : leaves) {
int pos = timeline->getItemPosition(l);
bool outOfRange = pos + timeline->getItemPlaytime(l) < position;
bool outOfRange = timeline->getItemEnd(l) < position;
int tid = timeline->getItemTrackId(l);
bool unaffectedTrack = ignoreMultiTrackGroups && trackId > -1 && tid != trackId;
if (allowGroupBreaking) {
......@@ -353,20 +363,37 @@ int TimelineFunctions::requestSpacerStartOperation(const std::shared_ptr<Timelin
} else {
leavesToKeep.insert(l);
}
}
if (!outOfRange && !unaffectedTrack) {
// Check space in all tracks
if (!firstPositions.contains(tid)) {
firstPositions.insert(tid, pos);
} else {
if (pos < firstPositions.value(tid)) {
firstPositions.insert(tid, pos);
} else if (outOfRange) {
// This is a grouped clip positionned before the spacer operation position, check maximum space before
std::unordered_set<int> beforeOnTrack = timeline->getItemsInRange(tid, 0, pos - 1);
for (auto &c : allClips) {
beforeOnTrack.erase(c);
}
int lastPos = 0;
for (int c : beforeOnTrack) {
int p = timeline->getClipEnd(c);
if (p >= pos - 1) {
lastPos = pos;
break;
}
if (p > lastPos) {
lastPos = p;
}
}
if (relatedMaxSpace.contains(trackId)) {
if (relatedMaxSpace.value(trackId) > (pos - lastPos)) {
relatedMaxSpace.insert(trackId, pos - lastPos);
}
} else {
relatedMaxSpace.insert(trackId, pos - lastPos);
}
}
if (!outOfRange && !unaffectedTrack) {
// Find first item
if (firstPosition == -1 || pos < firstPosition) {
firstCid = l;
firstPosition = pos;
if (!firstClipOnTrack.contains(tid)) {
firstClipOnTrack.insert(tid, l);
} else if (timeline->getItemPosition(firstClipOnTrack.value(tid)) > pos) {
firstClipOnTrack.insert(tid, l);
}
}
}
......@@ -382,16 +409,8 @@ int TimelineFunctions::requestSpacerStartOperation(const std::shared_ptr<Timelin
groupsToRemove.insert(r);
}
} else {
// Find first clip on track
int pos = timeline->getItemPosition(r);
int tid = timeline->getItemTrackId(r);
// Check space in all tracks
if (!firstPositions.contains(tid)) {
firstPositions.insert(tid, pos);
} else {
if (pos < firstPositions.value(tid)) {
firstPositions.insert(tid, pos);
}
}
if (firstPosition == -1 || pos < firstPosition) {
firstCid = r;
firstPosition = pos;
......@@ -412,38 +431,40 @@ int TimelineFunctions::requestSpacerStartOperation(const std::shared_ptr<Timelin
}
timeline->requestSetSelection(toSelect);
if (!firstPositions.isEmpty()) {
// Find minimum position, parse all tracks
QMapIterator<int, int> it(firstClipOnTrack);
int firstPos = -1;
while (it.hasNext()) {
it.next();
int clipPos = timeline->getItemPosition(it.value());
if (trackId > -1) {
// Easy, check blank size
int spaceDuration = timeline->getTrackById_const(trackId)->getBlankSizeAtPos(firstPosition - 1);
if (spaceDuration > 0) {
spacerMinPosition = firstPosition - spaceDuration;
if (it.key() == trackId) {
firstCid = it.value();
}
} else {
// Check space in all tracks
auto it = timeline->m_allTracks.cbegin();
int space = -1;
while (it != timeline->m_allTracks.cend()) {
int tid = (*it)->getId();
if (!firstPositions.contains(tid)) {
++it;
continue;
}
int spaceDuration = (*it)->getBlankSizeAtPos(firstPositions.value(tid) - 1);
if (space == -1 || spaceDuration < space) {
space = spaceDuration;
}
++it;
if (firstPos == -1) {
firstCid = it.value();
firstPos = clipPos;
} else if (firstPos < clipPos) {
firstCid = it.value();
}
if (space > -1) {
spacerMinPosition = firstPosition - space;
}
if (timeline->getTrackById_const(it.key())->isBlankAt(clipPos - 1)) {
if (spaceDuration == -1) {
spaceDuration = timeline->getTrackById_const(it.key())->getBlankSizeAtPos(clipPos - 1);
} else {
int blank = timeline->getTrackById_const(it.key())->getBlankSizeAtPos(clipPos - 1);
spaceDuration = qMin(blank, spaceDuration);
}
}
if (relatedMaxSpace.contains(it.key())) {
spaceDuration = qMin(spaceDuration, relatedMaxSpace.value(it.key()));
}
}
return (firstCid);
spacerMinPosition = timeline->getItemPosition(firstCid) - spaceDuration;
return {firstCid, spaceDuration};
}
return -1;
return {-1, -1};
}
bool TimelineFunctions::requestSpacerEndOperation(const std::shared_ptr<TimelineItemModel> &timeline, int itemId, int startPosition, int endPosition,
......@@ -2151,50 +2172,102 @@ bool TimelineFunctions::pasteTimelineClips(const std::shared_ptr<TimelineItemMod
bool TimelineFunctions::requestDeleteBlankAt(const std::shared_ptr<TimelineItemModel> &timeline, int trackId, int position, bool affectAllTracks)
{
// find blank duration
int spaceStart = 0;
// Check we have blank at position
int startPos = -1;
int endPos = -1;
if (affectAllTracks) {
int lastFrame = 0;
for (const auto &track : timeline->m_allTracks) {
if (!track->isLocked()) {
if (!track->isBlankAt(position)) {
return false;
}
lastFrame = track->getBlankStart(position);
if (lastFrame > spaceStart) {
spaceStart = lastFrame;
startPos = track->getBlankStart(position) - 1;
endPos = track->getBlankEnd(position) + 2;
if (startPos > -1) {
std::unordered_set<int> clips = timeline->getItemsInRange(trackId, startPos, endPos);
if (clips.size() == 2) {
auto it = clips.begin();
int firstCid = *it;
++it;
int lastCid = *it;
if (timeline->m_groups->isInGroup(firstCid)) {
int groupId = timeline->m_groups->getRootId(firstCid);
std::unordered_set<int> all_children = timeline->m_groups->getLeaves(groupId);
if (all_children.find(lastCid) != all_children.end()) {
return false;
}
}
}
}
}
}
// check subtitle track
if (timeline->getSubtitleModel() && !timeline->getSubtitleModel()->isLocked()) {
lastFrame = timeline->getSubtitleModel()->getBlankStart(position);
if (lastFrame > spaceStart) {
spaceStart = lastFrame;
if (!timeline->getSubtitleModel()->isBlankAt(position)) {
return false;
}
startPos = timeline->getSubtitleModel()->getBlankStart(position) - 1;
endPos = timeline->getSubtitleModel()->getBlankEnd(position) + 1;
if (startPos > -1) {
std::unordered_set<int> clips = timeline->getItemsInRange(trackId, startPos, endPos);
if (clips.size() == 2) {
auto it = clips.begin();
int firstCid = *it;
++it;
int lastCid = *it;
if (timeline->m_groups->isInGroup(firstCid)) {
int groupId = timeline->m_groups->getRootId(firstCid);
std::unordered_set<int> all_children = timeline->m_groups->getLeaves(groupId);
if (all_children.find(lastCid) != all_children.end()) {
return false;
}
}
}
}
}
} else {
// Check we have a blank and that it is in not between 2 grouped clips
if (timeline->isSubtitleTrack(trackId)) {
// Subtitle track
if (!timeline->getSubtitleModel()->isBlankAt(position)) {
return false;
}
spaceStart = timeline->getSubtitleModel()->getBlankStart(position);
startPos = timeline->getSubtitleModel()->getBlankStart(position) - 1;
endPos = timeline->getSubtitleModel()->getBlankEnd(position) + 1;
} else {
if (!timeline->getTrackById_const(trackId)->isBlankAt(position)) {
return false;
}
spaceStart = timeline->getTrackById_const(trackId)->getBlankStart(position);
startPos = timeline->getTrackById_const(trackId)->getBlankStart(position) - 1;
endPos = timeline->getTrackById_const(trackId)->getBlankEnd(position) + 2;
}
if (startPos > -1) {
std::unordered_set<int> clips = timeline->getItemsInRange(trackId, startPos, endPos);
if (clips.size() == 2) {
auto it = clips.begin();
int firstCid = *it;
++it;
int lastCid = *it;
if (timeline->m_groups->isInGroup(firstCid)) {
int groupId = timeline->m_groups->getRootId(firstCid);
std::unordered_set<int> all_children = timeline->m_groups->getLeaves(groupId);
if (all_children.find(lastCid) != all_children.end()) {
return false;
}
}
}
}
}
if (spaceStart > position) {
std::pair<int, int> spacerOp = requestSpacerStartOperation(timeline, affectAllTracks ? -1 : trackId, position);
int cid = spacerOp.first;
if (cid == -1 || spacerOp.second == -1) {
return false;
}
int cid = requestSpacerStartOperation(timeline, affectAllTracks ? -1 : trackId, position);
if (cid == -1) {
int start = timeline->getItemPosition(cid);
int spaceStart = start - spacerOp.second;
if (spaceStart >= start) {
return false;
}
int start = timeline->getItemPosition(cid);
// Start undoable command
std::function<bool(void)> undo = []() { return true; };
std::function<bool(void)> redo = []() { return true; };
......@@ -2221,7 +2294,8 @@ bool TimelineFunctions::requestDeleteAllBlanksFrom(const std::shared_ptr<Timelin
return false;
}
while (blankStart != -1) {
int cid = requestSpacerStartOperation(timeline, trackId, blankStart, true);
std::pair<int, int> spacerOp = requestSpacerStartOperation(timeline, trackId, blankStart, true);
int cid = spacerOp.first;
if (cid == -1) {
break;
}
......@@ -2266,7 +2340,8 @@ bool TimelineFunctions::requestDeleteAllBlanksFrom(const std::shared_ptr<Timelin
return false;
}
while (blankStart != -1) {
int cid = requestSpacerStartOperation(timeline, trackId, blankStart, true);
std::pair<int, int> spacerOp = requestSpacerStartOperation(timeline, trackId, blankStart, true);
int cid = spacerOp.first;
if (cid == -1) {
break;
}
......
src/timeline2/model/timelinefunctions.hpp
View file @ 2dad6ae4
......@@ -86,7 +86,7 @@ struct TimelineFunctions
static bool requestDeleteAllClipsFrom(const std::shared_ptr<TimelineItemModel> &timeline, int trackId, int position);
/** @brief Starts a spacer operation. Should be used together with requestSpacerEndOperation
@returns clipId of the position-wise first clip in the temporary group
@returns {clipId, max blank duration} of the position-wise first clip in the temporary group
@param timeline TimelineItemModel where the operation should be performed on
@param trackId
@param position
......@@ -94,8 +94,8 @@ struct TimelineFunctions
@param allowGroupBreaking Whether independant move of grouped items is allowed
@see requestSpacerEndOperation
*/
static int requestSpacerStartOperation(const std::shared_ptr<TimelineItemModel> &timeline, int trackId, int position, bool ignoreMultiTrackGroups = false,
bool allowGroupBreaking = false);
static std::pair<int, int> requestSpacerStartOperation(const std::shared_ptr<TimelineItemModel> &timeline, int trackId, int position,
bool ignoreMultiTrackGroups = false, bool allowGroupBreaking = false);
/**
@see requestSpacerStartOperation
*/
......
src/timeline2/model/timelinemodel.cpp
View file @ 2dad6ae4
......@@ -284,6 +284,15 @@ int TimelineModel::getClipPosition(int clipId) const
return pos;
}
int TimelineModel::getClipEnd(int clipId) const
{
READ_LOCK();
Q_ASSERT(m_allClips.count(clipId) > 0);
const auto clip = m_allClips.at(clipId);
int pos = clip->getPosition() + clip->getPlaytime();
return pos;
}
double TimelineModel::getClipSpeed(int clipId) const
{
READ_LOCK();
......@@ -2105,7 +2114,7 @@ std::unordered_set<int> TimelineModel::getItemsInRange(int trackId, int start, i
Q_UNUSED(listCompositions)
std::unordered_set<int> allClips;
if (trackId < 0) {
if (isSubtitleTrack(trackId)) {
// Subtitles
if (m_subtitleModel) {
std::unordered_set<int> subs = m_subtitleModel->getItemsInRange(start, end);
......@@ -3794,7 +3803,8 @@ bool TimelineModel::requestItemRippleResize(const std::shared_ptr<TimelineItemMo
if (right && getTrackById_const(trackId)->isLastClip(getItemPosition(itemId))) {
return true;
}
int cid = TimelineFunctions::requestSpacerStartOperation(timeline, affectAllTracks ? -1 : trackId, position + 1, true, true);
std::pair<int, int> spacerOp = TimelineFunctions::requestSpacerStartOperation(timeline, affectAllTracks ? -1 : trackId, position + 1, true, true);
int cid = spacerOp.first;
if (cid == -1) {
return false;
}
......@@ -4888,6 +4898,13 @@ int TimelineModel::getCompositionPosition(int compoId) const
return trans->getPosition();
}
int TimelineModel::getCompositionEnd(int compoId) const
{
Q_ASSERT(m_allCompositions.count(compoId) > 0);
const auto trans = m_allCompositions.at(compoId);
return trans->getPosition() + trans->getPlaytime();
}
int TimelineModel::getCompositionPlaytime(int compoId) const
{
READ_LOCK();
......@@ -4923,6 +4940,20 @@ const QString TimelineModel::getClipName(int cid) const
return m_allClips.at(cid)->clipName();
}
int TimelineModel::getItemEnd(int itemId) const
{
if (isClip(itemId)) {
return getClipEnd(itemId);
}
if (isComposition(itemId)) {
return getCompositionEnd(itemId);
}
if (isSubTitle(itemId)) {
return m_subtitleModel->getSubtitleEnd(itemId);
}
return -1;
}
int TimelineModel::getItemPlaytime(int itemId) const
{
if (isClip(itemId)) {
......
src/timeline2/model/timelinemodel.hpp
View file @ 2dad6ae4
......@@ -211,13 +211,16 @@ public:
Q_INVOKABLE int getCompositionPosition(int compoId) const;
int getSubtitlePosition(int subId) const;
int getCompositionPlaytime(int compoId) const;
int getCompositionEnd(int compoId) const;
std::pair<int, int> getMixInOut(int cid) const;
int getMixDuration(int cid) const;
/** @brief Returns an item position, item can be clip or composition */
/** @brief Returns an item position, item can be clip, subtitle or composition */
Q_INVOKABLE int getItemPosition(int itemId) const;
/** @brief Returns an item duration, item can be clip or composition */
/** @brief Returns an item duration, item can be clip, subtitle or composition */
int getItemPlaytime(int itemId) const;
/** @brief Returns an item's out point on its track, item can be clip, subtitle or composition */
int getItemEnd(int itemId) const;
/** @brief Returns the subplaylist index of a clip in a track */
int getClipSubPlaylistIndex(int cid) const;
......@@ -308,6 +311,11 @@ public:
*/
int getClipPlaytime(int clipId) const;
/** @brief Returns the out point of a clip in its track
@param clipId Id of the clip to test
*/
int getClipEnd(int clipId) const;
/** @brief Returns the size of the clip's frame (widthxheight)
@param clipId Id of the clip to test
*/
......
src/timeline2/view/timelinecontroller.cpp
View file @ 2dad6ae4
......@@ -1761,7 +1761,8 @@ void TimelineController::cutAllClipsUnderCursor(int position)
int TimelineController::requestSpacerStartOperation(int trackId, int position)
{
QMutexLocker lk(&m_metaMutex);
int itemId = TimelineFunctions::requestSpacerStartOperation(m_model, trackId, position);
std::pair<int, int> spacerOp = TimelineFunctions::requestSpacerStartOperation(m_model, trackId, position);
int itemId = spacerOp.first;
return itemId;
}
......
tests/spacertest.cpp
View file @ 2dad6ae4
......@@ -148,6 +148,48 @@ TEST_CASE("Remove all spaces", "[Spacer]")
state1();
undoStack->undo();
}
SECTION("Ensure remove all clips on track behaves correctly")
{
// We have clips at 10, 80, 101 on track 1 (length 20 frames each)
// One clip at 20 on track 2
// Grouping clip at 10 on tid1 and at 20 on tid2, so the clip on tid2 will be moved
std::unordered_set<int> ids = {cid1, cid4};
int gid = timeline->requestClipsGroup(ids);
REQUIRE(gid > -1);
REQUIRE(TimelineFunctions::requestDeleteAllClipsFrom(timeline, tid1, 80));
REQUIRE(timeline->getTrackClipsCount(tid1) == 1);
REQUIRE(timeline->getTrackClipsCount(tid2) == 1);
undoStack->undo();
state1();
REQUIRE(TimelineFunctions::requestDeleteAllClipsFrom(timeline, tid1, 20));
REQUIRE(timeline->getTrackClipsCount(tid1) == 0);
REQUIRE(timeline->getTrackClipsCount(tid2) == 0);
undoStack->undo();
state1();
undoStack->undo();
}
SECTION("Ensure delete blank works correctly with grouped clips")
{
// We have clips at 10, 80, 101 on track 1 (length 20 frames each)
// One clip at 20 on track 2
// Grouping clip at 80 and 101 on tid1
std::unordered_set<int> ids = {cid2, cid3};
int gid = timeline->requestClipsGroup(ids);
REQUIRE(gid > -1);
REQUIRE(TimelineFunctions::requestDeleteBlankAt(timeline, tid1, 80, false) == false);
REQUIRE(TimelineFunctions::requestDeleteBlankAt(timeline, tid1, 70, false));
REQUIRE(timeline->getTrackClipsCount(tid1) == 3);
REQUIRE(timeline->getTrackClipsCount(tid2) == 1);
REQUIRE(timeline->getClipPosition(cid1) == 10);
REQUIRE(timeline->getClipPosition(cid2) == 30);
REQUIRE(timeline->getClipPosition(cid3) == 51);
REQUIRE(timeline->getClipPosition(cid4) == 20);
undoStack->undo();
state1();
// Delete space between 2 grouped clips, should fail
REQUIRE(TimelineFunctions::requestDeleteBlankAt(timeline, tid1, 100, false) == false);
undoStack->undo();
}
binModel->clean();
pCore->m_projectManager = nullptr;
}
tests/trimmingtest.cpp
View file @ 2dad6ae4
......@@ -609,7 +609,7 @@ TEST_CASE("Spacer operations", "[Spacer]")
};
state();
int spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, l + 5);
int spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, l + 5).first;
REQUIRE(spacerIid > -1);
std::function<bool(void)> undo = []() { return true; };
std::function<bool(void)> redo = []() { return true; };
......@@ -634,7 +634,7 @@ TEST_CASE("Spacer operations", "[Spacer]")
state1();
int startPos = timeline->getClipPosition(cid3) + l / 2;
spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, startPos);
spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, startPos).first;
REQUIRE(spacerIid > -1);
undo = []() { return true; };
redo = []() { return true; };
......@@ -700,7 +700,7 @@ TEST_CASE("Spacer operations", "[Spacer]")
};
state();
int spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, l + 5);
int spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, l + 5).first;
REQUIRE(spacerIid > -1);
std::function<bool(void)> undo = []() { return true; };
std::function<bool(void)> redo = []() { return true; };
......@@ -726,7 +726,7 @@ TEST_CASE("Spacer operations", "[Spacer]")
state1();
int startPos = timeline->getClipPosition(cid3) + l / 2;
spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, startPos);
spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, startPos).first;
REQUIRE(spacerIid > -1);
undo = []() { return true; };
redo = []() { return true; };
......@@ -788,7 +788,7 @@ TEST_CASE("Spacer operations", "[Spacer]")
};
state();
int spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, l + 5);
int spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, l + 5).first;
REQUIRE(spacerIid > -1);
std::function<bool(void)> undo = []() { return true; };
std::function<bool(void)> redo = []() { return true; };
......@@ -814,7 +814,7 @@ TEST_CASE("Spacer operations", "[Spacer]")
state1();
int startPos = timeline->getClipPosition(cid3) + l / 2;
spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, startPos);
spacerIid = TimelineFunctions::requestSpacerStartOperation(timeline, tid1, startPos).first;
REQUIRE(spacerIid > -1);
undo = []() { return true; };
redo = []() { return true; };
......
Supports Markdown
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