Commit b2ae65a7 authored by Emmet O'Neill's avatar Emmet O'Neill

Fixed Timeline's "Remove Frames/Columns and Pull" Actions.

BUG: 394842

Previously these actions failed to remove the last frame of the
selection if the next following frame was empty. Fixed now!

Revied by Dmitry. Help from Eoin. Thanks as always!
parent c2391e4f
......@@ -290,11 +290,10 @@ bool KisAnimationCurvesModel::adjustKeyframes(const QModelIndexList &indexes, in
KisImageBarrierLockerWithFeedback locker(image());
if (timeOffset != 0) {
bool ok = createOffsetFramesCommand(indexes, QPoint(timeOffset, 0), false, command.data());
bool ok = createOffsetFramesCommand(indexes, QPoint(timeOffset, 0), false, false, command.data());
if (!ok) return false;
}
using KisAnimationUtils::FrameItem;
using KisAnimationUtils::FrameItemList;
FrameItemList frameItems;
......
......@@ -174,18 +174,6 @@ namespace KisAnimationUtils {
std::sort(points->begin(), points->end(), LessOperator(offset));
}
KUndo2Command* createMoveKeyframesCommand(const FrameItemList &srcFrames,
const FrameItemList &dstFrames,
bool copy,
KUndo2Command *parentCommand) {
FrameMovePairList movedFrames;
for (int i = 0; i < srcFrames.size(); i++) {
movedFrames << std::make_pair(srcFrames[i], dstFrames[i]);
}
return createMoveKeyframesCommand(movedFrames, copy, parentCommand);
}
bool supportsContentFrames(KisNodeSP node)
{
return node->inherits("KisPaintLayer") || node->inherits("KisFilterMask") || node->inherits("KisTransparencyMask") || node->inherits("KisSelectionBasedLayer");
......@@ -202,19 +190,17 @@ namespace KisAnimationUtils {
KisKeyframeChannel *dstChannel = dstNode->getKeyframeChannel(dst.channel, true);
if (srcNode == dstNode) {
// TODO: add warning!
if (!srcChannel) return;
if (!srcChannel) return; // TODO: add warning!
srcChannel->swapFrames(srcTime, dstTime, parentCommand);
} else {
// TODO: add warning!
if (!srcChannel || !dstChannel) return;
if (!srcChannel || !dstChannel) return; // TODO: add warning!
dstChannel->swapExternalKeyframe(srcChannel, srcTime, dstTime, parentCommand);
}
}
void moveOneFrameItem(const FrameItem &src, const FrameItem &dst, bool copy, KUndo2Command *parentCommand)
void moveOneFrameItem(const FrameItem &src, const FrameItem &dst, bool copy, bool moveEmptyFrames, KUndo2Command *parentCommand)
{
const int srcTime = src.time;
KisNodeSP srcNode = src.node;
......@@ -225,24 +211,28 @@ namespace KisAnimationUtils {
KisKeyframeChannel *dstChannel = dstNode->getKeyframeChannel(dst.channel, true);
if (srcNode == dstNode) {
// TODO: add warning!
if (!srcChannel) return;
if (!srcChannel) return; // TODO: add warning!
KisKeyframeSP srcKeyframe = srcChannel->keyframeAt(srcTime);
KisKeyframeSP dstKeyFrame = srcChannel->keyframeAt(dstTime);
if (srcKeyframe) {
if (copy) {
srcChannel->copyKeyframe(srcKeyframe, dstTime, parentCommand);
} else {
srcChannel->moveKeyframe(srcKeyframe, dstTime, parentCommand);
}
} else {
if (dstKeyFrame && moveEmptyFrames && !copy) {
//Destination is effectively replaced by an empty frame.
dstChannel->deleteKeyframe(dstKeyFrame, parentCommand);
}
}
} else {
// TODO: add warning!
if (!srcChannel || !dstChannel) return;
if (!srcChannel || !dstChannel) return; // TODO: add warning!
KisKeyframeSP srcKeyframe = srcChannel->keyframeAt(srcTime);
// TODO: add warning!
if (!srcKeyframe) return;
if (!srcKeyframe) return; // TODO: add warning!
dstChannel->copyExternalKeyframe(srcChannel, srcTime, dstTime, parentCommand);
......@@ -252,39 +242,53 @@ namespace KisAnimationUtils {
}
}
KUndo2Command *createMoveKeyframesCommand(const FrameMovePairList &movePairs, bool copy, KUndo2Command *parentCommand)
KUndo2Command* createMoveKeyframesCommand(const FrameItemList &srcFrames,
const FrameItemList &dstFrames,
bool copy,
bool moveEmpty,
KUndo2Command *parentCommand)
{
FrameMovePairList srcDstPairs;
for (int i = 0; i < srcFrames.size(); i++) {
srcDstPairs << std::make_pair(srcFrames[i], dstFrames[i]);
}
return createMoveKeyframesCommand(srcDstPairs, copy, moveEmpty, parentCommand);
}
KUndo2Command* createMoveKeyframesCommand(const FrameMovePairList &srcDstPairs,
bool copy,
bool moveEmptyFrames,
KUndo2Command *parentCommand)
{
KUndo2Command *cmd = new KisCommandUtils::LambdaCommand(
!copy ?
kundo2_i18np("Move Keyframe",
"Move %1 Keyframes",
movePairs.size()) :
srcDstPairs.size()) :
kundo2_i18np("Copy Keyframe",
"Copy %1 Keyframes",
movePairs.size()),
srcDstPairs.size()),
parentCommand,
[movePairs, copy] () -> KUndo2Command* {
[srcDstPairs, copy, moveEmptyFrames] () -> KUndo2Command* {
bool result = false;
QScopedPointer<KUndo2Command> cmd(new KUndo2Command());
using MoveChain = QList<FrameItem>;
QHash<FrameItem, MoveChain> moveMap;
Q_FOREACH (const FrameMovePair &pair, movePairs) {
Q_FOREACH (const FrameMovePair &pair, srcDstPairs) {
moveMap.insert(pair.first, {pair.second});
}
auto it = moveMap.begin();
while (it != moveMap.end()) {
MoveChain &chain = it.value();
const FrameItem &lastFrame = chain.last();
const FrameItem &previousFrame = chain.last();
auto tailIt = moveMap.find(lastFrame);
auto tailIt = moveMap.find(previousFrame);
if (tailIt == it || tailIt == moveMap.end()) {
++it;
......@@ -315,7 +319,7 @@ namespace KisAnimationUtils {
FrameItem srcItem = *frameIt++;
if (!isCycle) {
moveOneFrameItem(srcItem, dstItem, copy, cmd.data());
moveOneFrameItem(srcItem, dstItem, copy, moveEmptyFrames, cmd.data());
} else {
swapOneFrameItem(srcItem, dstItem, cmd.data());
}
......
......@@ -64,21 +64,21 @@ namespace KisAnimationUtils
void sortPointsForSafeMove(QModelIndexList *points, const QPoint &offset);
KUndo2Command* createMoveKeyframesCommand(const FrameItemList &srcFrames,
const FrameItemList &dstFrames,
bool copy, KUndo2Command *parentCommand = 0);
KUndo2Command* createMoveKeyframesCommand(const FrameItemList &srcFrames, const FrameItemList &dstFrames,
bool copy, bool moveEmpty, KUndo2Command *parentCommand = 0);
/**
* @brief implements safe moves of the frames (even if there are cycling move dependencies)
* @param movePairs the jobs for the moves
* @param copy shows if the frames should be copied or not
* @param moveEmpty allows an empty frame to replace a populated one
* @param parentCommand the command that should be a parent of the created command
* @return a created undo command
*/
KRITAANIMATIONDOCKER_EXPORT
KUndo2Command* createMoveKeyframesCommand(const FrameMovePairList &movePairs,
bool copy, KUndo2Command *parentCommand = 0);
bool copy, bool moveEmptyFrames, KUndo2Command *parentCommand = 0);
bool supportsContentFrames(KisNodeSP node);
......
......@@ -281,7 +281,7 @@ bool KisTimeBasedItemModel::removeFrames(const QModelIndexList &indexes)
return true;
}
KUndo2Command* KisTimeBasedItemModel::createOffsetFramesCommand(QModelIndexList srcIndexes, const QPoint &offset, bool copyFrames, KUndo2Command *parentCommand, bool moveEmptyFrames)
KUndo2Command* KisTimeBasedItemModel::createOffsetFramesCommand(QModelIndexList srcIndexes, const QPoint &offset, bool copyFrames, bool moveEmptyFrames, KUndo2Command *parentCommand)
{
if (srcIndexes.isEmpty()) return 0;
if (offset.isNull()) return 0;
......@@ -298,10 +298,7 @@ KUndo2Command* KisTimeBasedItemModel::createOffsetFramesCommand(QModelIndexList
KisNodeSP srcNode = nodeAt(srcIndex);
KisNodeSP dstNode = nodeAt(dstIndex);
if (!srcNode || !dstNode) {
return 0;
}
if (!srcNode || !dstNode) return 0;
Q_FOREACH(KisKeyframeChannel *channel, channelsAt(srcIndex)) {
if (moveEmptyFrames || channel->keyframeAt(srcIndex.column())) {
......@@ -318,31 +315,32 @@ KUndo2Command* KisTimeBasedItemModel::createOffsetFramesCommand(QModelIndexList
KisAnimationUtils::createMoveKeyframesCommand(srcFrameItems,
dstFrameItems,
copyFrames,
moveEmptyFrames,
parentCommand);
}
bool KisTimeBasedItemModel::removeFramesAndOffset(QModelIndexList indexes)
bool KisTimeBasedItemModel::removeFramesAndOffset(QModelIndexList indicesToRemove)
{
if (indexes.isEmpty()) return true;
if (indicesToRemove.isEmpty()) return true;
std::sort(indexes.begin(), indexes.end(),
std::sort(indicesToRemove.begin(), indicesToRemove.end(),
[] (const QModelIndex &lhs, const QModelIndex &rhs) {
return lhs.column() > rhs.column();
});
const int minColumn = indexes.last().column();
const int minColumn = indicesToRemove.last().column();
KUndo2Command *parentCommand = new KUndo2Command(kundo2_i18np("Remove frame and shift", "Remove %1 frames and shift", indexes.size()));
KUndo2Command *parentCommand = new KUndo2Command(kundo2_i18np("Remove frame and shift", "Remove %1 frames and shift", indicesToRemove.size()));
{
KisImageBarrierLockerWithFeedback locker(m_d->image);
Q_FOREACH (const QModelIndex &index, indexes) {
QModelIndexList movedIndexes;
Q_FOREACH (const QModelIndex &index, indicesToRemove) {
QModelIndexList indicesToOffset;
for (int column = index.column() + 1; column < columnCount(); column++) {
movedIndexes << this->index(index.row(), column);
indicesToOffset << this->index(index.row(), column);
}
createOffsetFramesCommand(movedIndexes, QPoint(-1, 0), false, parentCommand, true);
createOffsetFramesCommand(indicesToOffset, QPoint(-1, 0), false, true, parentCommand);
}
const int oldTime = m_d->image->animationInterface()->currentUITime();
......
......@@ -53,7 +53,7 @@ public:
bool removeFrames(const QModelIndexList &indexes);
bool removeFramesAndOffset(QModelIndexList indexes);
bool removeFramesAndOffset(QModelIndexList indicesToRemove);
bool mirrorFrames(QModelIndexList indexes);
......@@ -81,7 +81,9 @@ protected:
virtual QMap<QString, KisKeyframeChannel *> channelsAt(QModelIndex index) const = 0;
KisImageWSP image() const;
KUndo2Command* createOffsetFramesCommand(QModelIndexList srcIndexes, const QPoint &offset, bool copyFrames, KUndo2Command *parentCommand = 0, bool moveEmptyFrames = true);
KUndo2Command* createOffsetFramesCommand(QModelIndexList srcIndexes, const QPoint &offset,
bool copyFrames, bool moveEmptyFrames,
KUndo2Command *parentCommand = 0);
private Q_SLOTS:
......
......@@ -125,7 +125,7 @@ void KisAnimationUtilsTest::test()
frameMoves << std::make_pair(FrameItem(layer1, "content", 10), FrameItem(layer1, "content", 20));
frameMoves << std::make_pair(FrameItem(layer1, "content", 20), FrameItem(layer1, "content", 0));
QScopedPointer<KUndo2Command> cmd(createMoveKeyframesCommand(frameMoves, false));
QScopedPointer<KUndo2Command> cmd(createMoveKeyframesCommand(frameMoves, false, false));
cmd->redo();
QVector<std::tuple<int, QRect, QRect>> referenceRects;
......@@ -150,7 +150,7 @@ void KisAnimationUtilsTest::test()
frameMoves << std::make_pair(FrameItem(layer2, "content", 10), FrameItem(layer1, "content", 10));
frameMoves << std::make_pair(FrameItem(layer1, "content", 20), FrameItem(layer2, "content", 20));
cmd.reset(createMoveKeyframesCommand(frameMoves, false));
cmd.reset(createMoveKeyframesCommand(frameMoves, false, false));
cmd->redo();
referenceRects << std::make_tuple( 0, QRect() , QRect( 0, 10, 10, 10));
......@@ -176,7 +176,7 @@ void KisAnimationUtilsTest::test()
frameMoves << std::make_pair(FrameItem(layer2, "content", 10), FrameItem(layer1, "content", 10));
frameMoves << std::make_pair(FrameItem(layer2, "content", 20), FrameItem(layer1, "content", 20));
cmd.reset(createMoveKeyframesCommand(frameMoves, false));
cmd.reset(createMoveKeyframesCommand(frameMoves, false, false));
cmd->redo();
referenceRects << std::make_tuple( 0, QRect( 0, 10, 10, 10), QRect( 0, 0, 10, 10));
......@@ -203,7 +203,7 @@ void KisAnimationUtilsTest::test()
frameMoves << std::make_pair(FrameItem(layer2, "content", 10), FrameItem(layer1, "content", 9));
frameMoves << std::make_pair(FrameItem(layer2, "content", 20), FrameItem(layer1, "content", 20));
cmd.reset(createMoveKeyframesCommand(frameMoves, false));
cmd.reset(createMoveKeyframesCommand(frameMoves, false, false));
cmd->redo();
referenceRects << std::make_tuple( 0, QRect( 0, 10, 10, 10), QRect( 0, 0, 10, 10));
......
......@@ -705,7 +705,7 @@ bool TimelineFramesModel::dropMimeDataExtended(const QMimeData *data, Qt::DropAc
if (!frameMoves.isEmpty()) {
KisImageBarrierLockerWithFeedback locker(m_d->image);
cmd = KisAnimationUtils::createMoveKeyframesCommand(frameMoves, copyFrames, 0);
cmd = KisAnimationUtils::createMoveKeyframesCommand(frameMoves, copyFrames, false, 0);
}
if (cmd) {
......@@ -814,7 +814,7 @@ bool TimelineFramesModel::insertFrames(int dstColumn, const QList<int> &dstRows,
setLastVisibleFrame(columnCount() + (count * timing) - 1);
createOffsetFramesCommand(indexes, QPoint((count * timing), 0), false, parentCommand);
createOffsetFramesCommand(indexes, QPoint((count * timing), 0), false, false, parentCommand);
Q_FOREACH (int row, dstRows) {
KisNodeDummy *dummy = m_d->converter->dummyFromRow(row);
......@@ -920,7 +920,7 @@ bool TimelineFramesModel::insertHoldFrames(QModelIndexList selectedIndexes, int
indexes << index(row, column);
}
createOffsetFramesCommand(indexes, QPoint(plannedFrameMove, 0), false, parentCommand.data());
createOffsetFramesCommand(indexes, QPoint(plannedFrameMove, 0), false, false, parentCommand.data());
}
const int oldTime = m_d->image->animationInterface()->currentUITime();
......
......@@ -1369,15 +1369,15 @@ QModelIndexList TimelineFramesView::calculateSelectionSpan(bool entireColumn, bo
return indexes;
}
void TimelineFramesView::slotRemoveSelectedFrames(bool entireColumn, bool needsOffset)
void TimelineFramesView::slotRemoveSelectedFrames(bool entireColumn, bool pull)
{
const QModelIndexList indexes = calculateSelectionSpan(entireColumn);
const QModelIndexList selectedIndices = calculateSelectionSpan(entireColumn);
if (!indexes.isEmpty()) {
if (needsOffset) {
m_d->model->removeFramesAndOffset(indexes);
if (!selectedIndices.isEmpty()) {
if (pull) {
m_d->model->removeFramesAndOffset(selectedIndices);
} else {
m_d->model->removeFrames(indexes);
m_d->model->removeFrames(selectedIndices);
}
}
}
......@@ -1439,24 +1439,25 @@ void TimelineFramesView::slotMirrorFrames(bool entireColumn)
void TimelineFramesView::cutCopyImpl(bool entireColumn, bool copy)
{
const QModelIndexList indexes = calculateSelectionSpan(entireColumn, !copy);
if (indexes.isEmpty()) return;
const QModelIndexList selectedIndices = calculateSelectionSpan(entireColumn, !copy);
if (selectedIndices.isEmpty()) return;
int minColumn = std::numeric_limits<int>::max();
int minRow = std::numeric_limits<int>::max();
Q_FOREACH (const QModelIndex &index, indexes) {
Q_FOREACH (const QModelIndex &index, selectedIndices) {
minRow = qMin(minRow, index.row());
minColumn = qMin(minColumn, index.column());
}
const QModelIndex baseIndex = m_d->model->index(minRow, minColumn);
QMimeData *data = m_d->model->mimeDataExtended(indexes,
QMimeData *data = m_d->model->mimeDataExtended(selectedIndices,
baseIndex,
copy ?
TimelineFramesModel::CopyFramesPolicy :
TimelineFramesModel::MoveFramesPolicy);
if (data) {
QClipboard *cb = QApplication::clipboard();
cb->setMimeData(data);
......
......@@ -86,7 +86,7 @@ private Q_SLOTS:
void slotInsertMultipleKeyframes() {insertMultipleKeyframes(false);}
void slotInsertMultipleKeyframeColumns() {insertMultipleKeyframes(true);}
void slotRemoveSelectedFrames(bool entireColumn = false, bool needsOffset = false);
void slotRemoveSelectedFrames(bool entireColumn = false, bool pull = false);
void slotRemoveSelectedFramesAndShift() {slotRemoveSelectedFrames(false, true);}
void slotRemoveSelectedColumns() {slotRemoveSelectedFrames(true);}
......
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