From 52fc4f064b1c57a850f58bbb865b29891b9ca96e Mon Sep 17 00:00:00 2001 From: itsmattkc Date: Sat, 7 Mar 2020 20:41:31 +1100 Subject: [PATCH] timeline: rewrote ripple deleting into an undoable command that takes place after the gaps are made and cleaned Results in more reliable ripple delete behavior. Also fixes bug that would cause gaps that were cleaned and then restored in an undo to crash when selected. --- app/widget/timelinewidget/timelinewidget.cpp | 94 +++----------------- app/widget/timelinewidget/timelinewidget.h | 2 +- app/widget/timelinewidget/tool/pointer.cpp | 2 +- app/widget/timelinewidget/undo/undo.cpp | 65 +++++++++++++- app/widget/timelinewidget/undo/undo.h | 16 ++++ 5 files changed, 91 insertions(+), 88 deletions(-) diff --git a/app/widget/timelinewidget/timelinewidget.cpp b/app/widget/timelinewidget/timelinewidget.cpp index 2cd12b5d6..6173d9df2 100644 --- a/app/widget/timelinewidget/timelinewidget.cpp +++ b/app/widget/timelinewidget/timelinewidget.cpp @@ -338,89 +338,8 @@ void TimelineWidget::SplitAtPlayhead() void TimelineWidget::DeleteSelectedInternal(QList blocks, bool transition_aware, bool remove_from_graph, - bool ripple, QUndoCommand *command) { - if (ripple) { - for (int i=0;ilength(); - - QList blocks_at_time; - - foreach (TrackOutput* track, GetConnectedNode()->Tracks()) { - - // Ignore our track since we've already taking account of the block at this time on our track - if (track == b_track) { - continue; - } - - // Get the block from every other track that is either at or just before our block's in point - Block* block_at_time = track->NearestBlockBeforeOrAt(b->in()); - - // If we found a block, see what it is - if (block_at_time) { - - // If it's a gap, or we're deleting it (which means it will soon become a gap), it's viable for removing - // or resizing - if (block_at_time->type() == Block::kGap || blocks.contains(block_at_time)) { - - // In an effort to keep all tracks synchronized, we can only ripple a maximum of the smallest gap we find - max_ripple_length = qMin(max_ripple_length, block_at_time->length()); - - } else { - - // If there is no gap here, we cannot ripple at all and must abort this ripple - max_ripple_length = 0; - break; - - } - - blocks_at_time.append(block_at_time); - } - } - - // If we can ripple all the tracks - if (max_ripple_length > 0) { - - // Ripple everything including the main block - blocks_at_time.append(b); - - foreach (Block* resize, blocks_at_time) { - - // If we can remove this whole block, remove the whole block - if (resize->length() == max_ripple_length) { - new TrackRippleRemoveBlockCommand(TrackOutput::TrackFromBlock(resize), resize, command); - - // Also remove this block from our block list so we don't bother replacing it with a gap later - int resize_index = blocks.indexOf(resize); - - if (resize_index >= 0) { - blocks.removeOne(resize); - - // Ensure our iteration remain correct after removing blocks - if (resize_index <= i) { - i--; - } - } - } else { - - // Otherwise, we'll simply shorten the gap/clip - BlockResizeCommand* brc = new BlockResizeCommand(resize, resize->length() - max_ripple_length, command); - - // Perform the resize NOW so that if it's a clip that we're replacing with a gap later, the gap will have - // the correct length - brc->redo(); - - } - } - } - } - } - foreach (Block* b, blocks) { TrackOutput* original_track = TrackOutput::TrackFromBlock(b); @@ -490,7 +409,7 @@ void TimelineWidget::DeleteSelected(bool ripple) QUndoCommand* command = new QUndoCommand(); // Replace blocks with gaps (effectively deleting them) - DeleteSelectedInternal(blocks_to_delete, true, true, ripple, command); + DeleteSelectedInternal(blocks_to_delete, true, true, command); // Clean each track foreach (const TrackReference& track, tracks_affected) { @@ -499,6 +418,17 @@ void TimelineWidget::DeleteSelected(bool ripple) command); } + // Insert ripple command now that it's all cleaned up gaps + if (ripple) { + TimeRangeList range_list; + + foreach (Block* b, blocks_to_delete) { + range_list.InsertTimeRange(TimeRange(b->in(), b->out())); + } + + new TimelineRippleDeleteGapsAtRegions(GetConnectedNode(), range_list, command); + } + Core::instance()->undo_stack()->pushIfHasChildren(command); } diff --git a/app/widget/timelinewidget/timelinewidget.h b/app/widget/timelinewidget/timelinewidget.h index 819315456..2dd6bd758 100644 --- a/app/widget/timelinewidget/timelinewidget.h +++ b/app/widget/timelinewidget/timelinewidget.h @@ -349,7 +349,7 @@ private: bool dual_transition_; }; - void DeleteSelectedInternal(QList blocks, bool transition_aware, bool remove_from_graph, bool ripple, QUndoCommand* command); + void DeleteSelectedInternal(QList blocks, bool transition_aware, bool remove_from_graph, QUndoCommand* command); void SetBlockLinksSelected(Block *block, bool selected); diff --git a/app/widget/timelinewidget/tool/pointer.cpp b/app/widget/timelinewidget/tool/pointer.cpp index 3d21fe7a6..e88cad283 100644 --- a/app/widget/timelinewidget/tool/pointer.cpp +++ b/app/widget/timelinewidget/tool/pointer.cpp @@ -219,7 +219,7 @@ void TimelineWidget::PointerTool::MouseReleaseInternal(TimelineViewMouseEvent *e bool inserting = (event->GetModifiers() & Qt::ControlModifier); // If there are any blocks to remove, remove them - parent()->DeleteSelectedInternal(blocks_to_temp_remove, false, false, inserting, command); + parent()->DeleteSelectedInternal(blocks_to_temp_remove, false, false, command); if (inserting) { // Make room to insert clips to diff --git a/app/widget/timelinewidget/undo/undo.cpp b/app/widget/timelinewidget/undo/undo.cpp index 6cdc63613..09a9d2db8 100644 --- a/app/widget/timelinewidget/undo/undo.cpp +++ b/app/widget/timelinewidget/undo/undo.cpp @@ -547,7 +547,7 @@ void TrackCleanGapsCommand::redo_internal() rational new_gap_length = on_gap->length(); foreach (GapBlock* gap, consecutive_gaps) { track->RippleRemoveBlock(gap); - static_cast(gap->parent())->TakeNode(gap, &memory_manager_); + static_cast(track->parent())->TakeNode(gap, &memory_manager_); new_gap_length += gap->length(); } @@ -568,7 +568,7 @@ void TrackCleanGapsCommand::redo_internal() foreach (GapBlock* gap, removed_end_gaps_) { track->RippleRemoveBlock(gap); - static_cast(gap->parent())->TakeNode(gap, &memory_manager_); + static_cast(track->parent())->TakeNode(gap, &memory_manager_); } } } @@ -579,7 +579,7 @@ void TrackCleanGapsCommand::undo_internal() // Restored removed end gaps foreach (GapBlock* gap, removed_end_gaps_) { - static_cast(gap->parent())->AddNode(gap); + static_cast(track->parent())->AddNode(gap); track->AppendBlock(gap); } removed_end_gaps_.clear(); @@ -594,7 +594,7 @@ void TrackCleanGapsCommand::undo_internal() GapBlock* last_gap_added = merge_info.merged; foreach (GapBlock* gap, merge_info.removed) { - static_cast(gap->parent())->AddNode(gap); + static_cast(track->parent())->AddNode(gap); track->InsertBlockAfter(gap, last_gap_added); last_gap_added = gap; } @@ -622,3 +622,60 @@ void BlockSetSpeedCommand::undo_internal() { block_->set_speed(old_speed_); } + +TimelineRippleDeleteGapsAtRegions::TimelineRippleDeleteGapsAtRegions(ViewerOutput *vo, const TimeRangeList ®ions, QUndoCommand *parent) : + UndoCommand(parent), + timeline_(vo), + regions_(regions) +{ +} + +void TimelineRippleDeleteGapsAtRegions::redo_internal() +{ + foreach (const TimeRange& range, regions_) { + rational max_ripple_length = range.length(); + + QList blocks_around_range; + + foreach (TrackOutput* track, timeline_->Tracks()) { + // Get the block from every other track that is either at or just before our block's in point + Block* block_at_time = track->NearestBlockBeforeOrAt(range.in()); + + if (block_at_time) { + if (block_at_time->type() == Block::kGap) { + max_ripple_length = qMin(block_at_time->length(), max_ripple_length); + } else { + max_ripple_length = 0; + break; + } + + blocks_around_range.append(block_at_time); + } + } + + if (max_ripple_length > 0) { + foreach (Block* resize, blocks_around_range) { + if (resize->length() == max_ripple_length) { + // Remove block entirely + TrackRippleRemoveBlockCommand* remove_command = new TrackRippleRemoveBlockCommand(TrackOutput::TrackFromBlock(resize), resize); + remove_command->redo(); + commands_.append(remove_command); + } else { + // Resize block + BlockResizeCommand* resize_command = new BlockResizeCommand(resize, resize->length() - max_ripple_length); + resize_command->redo(); + commands_.append(resize_command); + } + } + } + } +} + +void TimelineRippleDeleteGapsAtRegions::undo_internal() +{ + for (int i=commands_.size()-1;i>=0;i--) { + commands_.at(i)->undo(); + delete commands_.at(i); + } + commands_.empty(); +} diff --git a/app/widget/timelinewidget/undo/undo.h b/app/widget/timelinewidget/undo/undo.h index a4cd2e36f..8e4d81b9c 100644 --- a/app/widget/timelinewidget/undo/undo.h +++ b/app/widget/timelinewidget/undo/undo.h @@ -280,4 +280,20 @@ private: }; +class TimelineRippleDeleteGapsAtRegions : public UndoCommand { +public: + TimelineRippleDeleteGapsAtRegions(ViewerOutput* vo, const TimeRangeList& regions, QUndoCommand* parent = nullptr); + +protected: + virtual void redo_internal() override; + virtual void undo_internal() override; + +private: + ViewerOutput* timeline_; + TimeRangeList regions_; + + QList commands_; + +}; + #endif // TIMELINEUNDOABLE_H