From de74adeb5e2501e3c3a802358f201a9ae0deeeb5 Mon Sep 17 00:00:00 2001 From: itsmattkc Date: Thu, 16 Jul 2020 00:32:00 +1000 Subject: [PATCH] nodes: improved changed signal processing Improves stability and cache reliability. Earlier iterations were prone to skipping necessary signals (usually leading to some sort of assert fail), particularly when track optimizations were used. Those optimizations have been moved to the viewer node so there's a higher degree of control over which signals get optimized and in which ways. --- app/node/block/block.cpp | 10 ---- app/node/block/block.h | 2 - app/node/node.cpp | 27 ++++++++-- app/node/node.h | 13 +++++ app/node/output/track/track.cpp | 70 ++++++++++--------------- app/node/output/track/track.h | 11 ---- app/node/output/viewer/viewer.cpp | 41 ++++++++++----- app/node/output/viewer/viewer.h | 6 +++ app/widget/timelinewidget/undo/undo.cpp | 66 +++++++++++------------ 9 files changed, 128 insertions(+), 118 deletions(-) diff --git a/app/node/block/block.cpp b/app/node/block/block.cpp index f46179786..222bc042d 100644 --- a/app/node/block/block.cpp +++ b/app/node/block/block.cpp @@ -355,16 +355,6 @@ NodeInput *Block::speed_input() const return speed_input_; } -void Block::InvalidateCache(const TimeRange &range, NodeInput *from, NodeInput *source) -{ - if (range.out() <= in() || range.in() >= out()) { - // Ignore this range - return; - } - - Node::InvalidateCache(TimeRange(qMax(range.in(), in()), qMin(range.out(), out())), from, source); -} - void Block::Hash(QCryptographicHash &, const rational &) const { // A block does nothing by default, so we hash nothing diff --git a/app/node/block/block.h b/app/node/block/block.h index 5694d4391..09182e7ea 100644 --- a/app/node/block/block.h +++ b/app/node/block/block.h @@ -88,8 +88,6 @@ public: NodeInput* media_in_input() const; NodeInput* speed_input() const; - virtual void InvalidateCache(const TimeRange& range, NodeInput* from, NodeInput* source) override; - virtual void Hash(QCryptographicHash &hash, const rational &time) const override; public slots: diff --git a/app/node/node.cpp b/app/node/node.cpp index 46d615b60..051575c05 100644 --- a/app/node/node.cpp +++ b/app/node/node.cpp @@ -177,6 +177,28 @@ void Node::InvalidateCache(const TimeRange &range, NodeInput *from, NodeInput *s SendInvalidateCache(range, source); } +void Node::BeginOperation() +{ + foreach (NodeParam* param, params_) { + if (param->type() == NodeParam::kOutput) { + foreach (NodeEdgePtr edge, param->edges()) { + edge->input()->parentNode()->BeginOperation(); + } + } + } +} + +void Node::EndOperation() +{ + foreach (NodeParam* param, params_) { + if (param->type() == NodeParam::kOutput) { + foreach (NodeEdgePtr edge, param->edges()) { + edge->input()->parentNode()->EndOperation(); + } + } + } +} + TimeRange Node::InputTimeAdjustment(NodeInput *, const TimeRange &input_time) const { // Default behavior is no time adjustment at all @@ -195,10 +217,7 @@ void Node::SendInvalidateCache(const TimeRange &range, NodeInput *source) foreach (NodeParam* param, params_) { // If the Node is an output, relay the signal to any Nodes that are connected to it if (param->type() == NodeParam::kOutput) { - - QVector edges = param->edges(); - - foreach (NodeEdgePtr edge, edges) { + foreach (NodeEdgePtr edge, param->edges()) { NodeInput* connected_input = edge->input(); Node* connected_node = connected_input->parentNode(); diff --git a/app/node/node.h b/app/node/node.h index eb3b272a2..9260ed44f 100644 --- a/app/node/node.h +++ b/app/node/node.h @@ -304,6 +304,19 @@ public: */ virtual void InvalidateCache(const TimeRange& range, NodeInput* from, NodeInput* source); + /** + * @brief Limits cache invalidation temporarily + * + * If you intend to do a number of operations in quick succession, you can optimize it by running + * this function with EndOperation(). + */ + virtual void BeginOperation(); + + /** + * @brief Stops limiting cache invalidation and flushes changes + */ + virtual void EndOperation(); + /** * @brief Adjusts time that should be sent to nodes connected to certain inputs. * diff --git a/app/node/output/track/track.cpp b/app/node/output/track/track.cpp index 96c2f6145..daa15748a 100644 --- a/app/node/output/track/track.cpp +++ b/app/node/output/track/track.cpp @@ -31,10 +31,8 @@ OLIVE_NAMESPACE_ENTER TrackOutput::TrackOutput() : track_type_(Timeline::kTrackTypeNone), - block_invalidate_cache_stack_(0), index_(-1), - locked_(false), - queued_length_change_(false) + locked_(false) { block_input_ = new NodeInputArray("block_in", NodeParam::kAny); block_input_->set_is_keyframable(false); @@ -252,11 +250,24 @@ const QList &TrackOutput::Blocks() const void TrackOutput::InvalidateCache(const TimeRange &range, NodeInput *from, NodeInput *source) { - if (block_invalidate_cache_stack_ == 0) { - PushLengthChangeSignal(true); + TimeRange limited; - Node::InvalidateCache(TimeRange(qMax(range.in(), rational(0)), qMin(range.out(), track_length())), from, source); + if (block_input_->sub_params().contains(from) + && from->get_connected_node() + && from->get_connected_node()->IsBlock()) { + // Limit the range signal to the corresponding block + Block* b = static_cast(from->get_connected_node()); + + if (range.out() <= b->in() || range.in() >= b->out()) { + return; + } + + limited = TimeRange(qMax(range.in(), b->in()), qMin(range.out(), b->out())); + } else { + limited = TimeRange(qMax(range.in(), rational(0)), qMin(range.out(), track_length())); } + + Node::InvalidateCache(limited, from, source); } void TrackOutput::InsertBlockBefore(Block* block, Block* after) @@ -279,12 +290,12 @@ void TrackOutput::InsertBlockAfter(Block *block, Block *before) void TrackOutput::PrependBlock(Block *block) { - BlockInvalidateCache(); + BeginOperation(); block_input_->Prepend(); NodeParam::ConnectEdge(block->output(), block_input_->First()); - UnblockInvalidateCache(); + EndOperation(); // Everything has shifted at this point InvalidateCache(TimeRange(0, track_length()), block_input_, block_input_); @@ -292,57 +303,47 @@ void TrackOutput::PrependBlock(Block *block) void TrackOutput::InsertBlockAtIndex(Block *block, int index) { - BlockInvalidateCache(); + BeginOperation(); int insert_index = GetInputIndexFromCacheIndex(index); block_input_->InsertAt(insert_index); NodeParam::ConnectEdge(block->output(), block_input_->At(insert_index)); - UnblockInvalidateCache(); + EndOperation(); InvalidateCache(TimeRange(block->in(), track_length()), block_input_, block_input_); } void TrackOutput::AppendBlock(Block *block) { - BlockInvalidateCache(); + BeginOperation(); block_input_->Append(); NodeParam::ConnectEdge(block->output(), block_input_->Last()); - UnblockInvalidateCache(); + EndOperation(); // Invalidate area that block was added to InvalidateCache(TimeRange(block->in(), track_length()), block_input_, block_input_); } -void TrackOutput::BlockInvalidateCache() -{ - block_invalidate_cache_stack_++; -} - -void TrackOutput::UnblockInvalidateCache() -{ - block_invalidate_cache_stack_--; -} - void TrackOutput::RippleRemoveBlock(Block *block) { - BlockInvalidateCache(); + BeginOperation(); rational remove_in = block->in(); block_input_->RemoveAt(GetInputIndexFromCacheIndex(block)); - UnblockInvalidateCache(); + EndOperation(); InvalidateCache(TimeRange(remove_in, track_length()), block_input_, block_input_); } void TrackOutput::ReplaceBlock(Block *old, Block *replace) { - BlockInvalidateCache(); + BeginOperation(); int index_of_old_block = GetInputIndexFromCacheIndex(old); @@ -352,7 +353,7 @@ void TrackOutput::ReplaceBlock(Block *old, Block *replace) NodeParam::ConnectEdge(replace->output(), block_input_->At(index_of_old_block)); - UnblockInvalidateCache(); + EndOperation(); if (old->length() == replace->length()) { InvalidateCache(TimeRange(replace->in(), replace->out()), block_input_, block_input_); @@ -443,14 +444,6 @@ void TrackOutput::Hash(QCryptographicHash &hash, const rational &time) const } } -void TrackOutput::PushLengthChangeSignal(bool invalidate) -{ - if (queued_length_change_) { - queued_length_change_ = false; - SetLengthInternal(queued_length_, invalidate); - } -} - void TrackOutput::SetTrackName(const QString &name) { track_name_ = name; @@ -507,15 +500,6 @@ int TrackOutput::GetInputIndexFromCacheIndex(Block *block) void TrackOutput::SetLengthInternal(const rational &r, bool invalidate) { - if (block_invalidate_cache_stack_ > 0) { - queued_length_change_ = true; - } - - if (queued_length_change_) { - queued_length_ = r; - return; - } - if (r != track_length_) { TimeRange invalidate_range(track_length_, r); diff --git a/app/node/output/track/track.h b/app/node/output/track/track.h index 35c444a0e..ad658bcd2 100644 --- a/app/node/output/track/track.h +++ b/app/node/output/track/track.h @@ -166,10 +166,6 @@ public: */ void ReplaceBlock(Block* old, Block* replace); - void BlockInvalidateCache(); - - void UnblockInvalidateCache(); - static TrackOutput* TrackFromBlock(const Block *block); const rational& track_length() const; @@ -192,8 +188,6 @@ public: virtual void Hash(QCryptographicHash& hash, const rational &time) const override; - void PushLengthChangeSignal(bool invalidate = false); - AudioVisualWaveform& waveform() { return waveform_; @@ -274,15 +268,10 @@ private: QString track_name_; - int block_invalidate_cache_stack_; - int index_; bool locked_; - bool queued_length_change_; - rational queued_length_; - AudioVisualWaveform waveform_; QMutex waveform_lock_; diff --git a/app/node/output/viewer/viewer.cpp b/app/node/output/viewer/viewer.cpp index b5b0f495b..457ce99de 100644 --- a/app/node/output/viewer/viewer.cpp +++ b/app/node/output/viewer/viewer.cpp @@ -26,7 +26,8 @@ OLIVE_NAMESPACE_ENTER ViewerOutput::ViewerOutput() : video_frame_cache_(this), - audio_playback_cache_(this) + audio_playback_cache_(this), + operation_stack_(0) { texture_input_ = new NodeInput("tex_in", NodeInput::kTexture); AddInput(texture_input_); @@ -49,7 +50,7 @@ ViewerOutput::ViewerOutput() : TrackList* list = new TrackList(this, static_cast(i), track_input); track_lists_.replace(i, list); connect(list, &TrackList::TrackListChanged, this, &ViewerOutput::UpdateTrackCache); - connect(list, &TrackList::LengthChanged, this, &ViewerOutput::VerifyLength); + //connect(list, &TrackList::LengthChanged, this, &ViewerOutput::VerifyLength); connect(list, &TrackList::BlockAdded, this, &ViewerOutput::TrackListAddedBlock); connect(list, &TrackList::BlockRemoved, this, &ViewerOutput::BlockRemoved); connect(list, &TrackList::TrackAdded, this, &ViewerOutput::TrackListAddedTrack); @@ -111,20 +112,22 @@ void ViewerOutput::InvalidateCache(const TimeRange &range, NodeInput *from, Node { emit GraphChangedFrom(source); - if (from == texture_input_ || from == samples_input_) { - TimeRange invalidated_range(qMax(rational(), range.in()), - qMin(GetLength(), range.out())); + if (operation_stack_ == 0) { + if (from == texture_input_ || from == samples_input_) { + TimeRange invalidated_range(qMax(rational(), range.in()), + qMin(GetLength(), range.out())); - if (invalidated_range.in() != invalidated_range.out()) { - if (from == texture_input_) { - video_frame_cache_.Invalidate(invalidated_range); - } else { - audio_playback_cache_.Invalidate(invalidated_range); + if (invalidated_range.in() != invalidated_range.out()) { + if (from == texture_input_) { + video_frame_cache_.Invalidate(invalidated_range); + } else { + audio_playback_cache_.Invalidate(invalidated_range); + } } } - } - VerifyLength(); + VerifyLength(); + } Node::InvalidateCache(range, from, source); } @@ -257,6 +260,20 @@ void ViewerOutput::set_media_name(const QString &name) emit MediaNameChanged(media_name_); } +void ViewerOutput::BeginOperation() +{ + operation_stack_++; + + Node::BeginOperation(); +} + +void ViewerOutput::EndOperation() +{ + operation_stack_--; + + Node::EndOperation(); +} + void ViewerOutput::TrackListAddedBlock(Block *block, int index) { Timeline::TrackType type = static_cast(sender())->type(); diff --git a/app/node/output/viewer/viewer.h b/app/node/output/viewer/viewer.h index b2ebb3b34..e57406c46 100644 --- a/app/node/output/viewer/viewer.h +++ b/app/node/output/viewer/viewer.h @@ -118,6 +118,10 @@ public: return &audio_playback_cache_; } + virtual void BeginOperation() override; + + virtual void EndOperation() override; + signals: void TimebaseChanged(const rational&); @@ -164,6 +168,8 @@ private: AudioPlaybackCache audio_playback_cache_; + int operation_stack_; + private slots: void UpdateTrackCache(); diff --git a/app/widget/timelinewidget/undo/undo.cpp b/app/widget/timelinewidget/undo/undo.cpp index 9b03b5bec..ce66be641 100644 --- a/app/widget/timelinewidget/undo/undo.cpp +++ b/app/widget/timelinewidget/undo/undo.cpp @@ -202,7 +202,7 @@ void TrackRippleRemoveAreaCommand::redo_internal() } } - track_->BlockInvalidateCache(); + track_->BeginOperation(); // If we picked up a block to splice if (splice_) { @@ -273,7 +273,7 @@ void TrackRippleRemoveAreaCommand::redo_internal() } } - track_->UnblockInvalidateCache(); + track_->EndOperation(); track_->InvalidateCache(TimeRange(in_, insert_ ? out_ : RATIONAL_MAX), track_->block_input(), @@ -282,7 +282,7 @@ void TrackRippleRemoveAreaCommand::redo_internal() void TrackRippleRemoveAreaCommand::undo_internal() { - track_->BlockInvalidateCache(); + track_->BeginOperation(); // If we were given a block to insert, insert it here if (insert_ != nullptr) { @@ -330,7 +330,7 @@ void TrackRippleRemoveAreaCommand::undo_internal() } - track_->UnblockInvalidateCache(); + track_->EndOperation(); track_->InvalidateCache(TimeRange(in_, insert_ ? out_ : RATIONAL_MAX), track_->block_input(), track_->block_input()); } @@ -432,7 +432,7 @@ Project *BlockSplitCommand::GetRelevantProject() const void BlockSplitCommand::redo_internal() { - track_->BlockInvalidateCache(); + track_->BeginOperation(); static_cast(block_->parent())->AddNode(new_block_); Node::CopyInputs(block_, new_block_); @@ -450,12 +450,12 @@ void BlockSplitCommand::redo_internal() NodeParam::ConnectEdge(new_block_->output(), transition); } - track_->UnblockInvalidateCache(); + track_->EndOperation(); } void BlockSplitCommand::undo_internal() { - track_->BlockInvalidateCache(); + track_->BeginOperation(); block_->set_length_and_media_out(old_length_); track_->RippleRemoveBlock(new_block_); @@ -467,7 +467,7 @@ void BlockSplitCommand::undo_internal() NodeParam::ConnectEdge(block_->output(), transition); } - track_->UnblockInvalidateCache(); + track_->EndOperation(); } Block *BlockSplitCommand::new_block() @@ -856,7 +856,7 @@ Project *BlockTrimCommand::GetRelevantProject() const void BlockTrimCommand::redo_internal() { - track_->BlockInvalidateCache(); + track_->BeginOperation(); // Will be POSITIVE if trimming shorter and NEGATIVE if trimming longer rational trim_diff = old_length_ - new_length_; @@ -923,14 +923,14 @@ void BlockTrimCommand::redo_internal() } } - track_->UnblockInvalidateCache(); + track_->EndOperation(); track_->InvalidateCache(invalidate_range, track_->block_input(), track_->block_input()); } void BlockTrimCommand::undo_internal() { - track_->BlockInvalidateCache(); + track_->BeginOperation(); // Will be POSITIVE if trimming shorter and NEGATIVE if trimming longer rational trim_diff = old_length_ - new_length_; @@ -979,7 +979,7 @@ void BlockTrimCommand::undo_internal() invalidate_range = TimeRange(block_->out(), block_->out() - trim_diff); } - track_->UnblockInvalidateCache(); + track_->EndOperation(); track_->InvalidateCache(invalidate_range, track_->block_input(), track_->block_input()); } @@ -1003,7 +1003,7 @@ void TrackReplaceBlockWithGapCommand::redo_internal() { TimeRange invalidate_range; - track_->BlockInvalidateCache(); + track_->BeginOperation(); // If the block has no next, it's at the end of the track and there's no need to create a gap if (block_->next()) { @@ -1062,7 +1062,7 @@ void TrackReplaceBlockWithGapCommand::redo_internal() invalidate_range = TimeRange(earliest_change, RATIONAL_MAX); } - track_->UnblockInvalidateCache(); + track_->EndOperation(); track_->InvalidateCache(invalidate_range, track_->block_input(), track_->block_input()); } @@ -1071,7 +1071,7 @@ void TrackReplaceBlockWithGapCommand::undo_internal() { TimeRange invalidate_range; - track_->BlockInvalidateCache(); + track_->BeginOperation(); if (gap_) { @@ -1116,7 +1116,7 @@ void TrackReplaceBlockWithGapCommand::undo_internal() merged_gap_ = nullptr; - track_->UnblockInvalidateCache(); + track_->EndOperation(); track_->InvalidateCache(invalidate_range, track_->block_input(), track_->block_input()); } @@ -1156,7 +1156,7 @@ void TrackSlideCommand::slide_internal(bool undo) // Perform trims foreach (const BlockSlideInfo& info, blocks_) { - info.track->BlockInvalidateCache(); + info.track->BeginOperation(); if (info.mode == Timeline::kTrimIn || info.mode == Timeline::kTrimOut) { rational new_len = undo ? info.old_time : info.new_time; @@ -1176,7 +1176,7 @@ void TrackSlideCommand::slide_internal(bool undo) added_gaps_.append(gap); } - info.track->UnblockInvalidateCache(); + info.track->EndOperation(); } if (undo) { @@ -1184,12 +1184,12 @@ void TrackSlideCommand::slide_internal(bool undo) foreach (GapBlock* gap, added_gaps_) { TrackOutput* track = TrackOutput::TrackFromBlock(gap); - track->BlockInvalidateCache(); + track->BeginOperation(); track->RippleRemoveBlock(gap); delete TakeNodeFromParentGraph(gap); - track->UnblockInvalidateCache(); + track->EndOperation(); } added_gaps_.clear(); @@ -1253,7 +1253,7 @@ void TrackListRippleRemoveAreaCommand::redo_internal() } foreach (TrackOutput* track, working_tracks_) { - track->BlockInvalidateCache(); + track->BeginOperation(); } } @@ -1263,8 +1263,7 @@ void TrackListRippleRemoveAreaCommand::redo_internal() if (all_tracks_unlocked_) { foreach (TrackOutput* track, working_tracks_) { - track->UnblockInvalidateCache(); - track->PushLengthChangeSignal(); + track->EndOperation(); } } } @@ -1281,7 +1280,7 @@ void TrackListRippleRemoveAreaCommand::undo_internal() } foreach (TrackOutput* track, working_tracks_) { - track->BlockInvalidateCache(); + track->BeginOperation(); } } @@ -1291,8 +1290,7 @@ void TrackListRippleRemoveAreaCommand::undo_internal() if (all_tracks_unlocked_) { foreach (TrackOutput* track, working_tracks_) { - track->UnblockInvalidateCache(); - track->PushLengthChangeSignal(); + track->EndOperation(); } } } @@ -1338,7 +1336,7 @@ void TrackListRippleToolCommand::redo_internal() if (all_tracks_unlocked_) { // We can do some optimization here foreach (const RippleInfo& info, info_) { - info.track->BlockInvalidateCache(); + info.track->BeginOperation(); } old_latest_pt = RATIONAL_MIN; @@ -1415,15 +1413,13 @@ void TrackListRippleToolCommand::redo_internal() } foreach (const RippleInfo& info, info_) { - info.track->UnblockInvalidateCache(); + info.track->EndOperation(); // FIXME: Untested, is this desirable behavior? if (earliest_pt < new_latest_pt) { info.track->InvalidateCache(TimeRange(earliest_pt, new_latest_pt), info.track->block_input(), info.track->block_input()); - } else { - info.track->PushLengthChangeSignal(); } } } @@ -1505,7 +1501,7 @@ void TrackListInsertGaps::redo_internal() } foreach (TrackOutput* track, working_tracks_) { - track->BlockInvalidateCache(); + track->BeginOperation(); } } @@ -1545,8 +1541,7 @@ void TrackListInsertGaps::redo_internal() if (all_tracks_unlocked_) { foreach (TrackOutput* track, working_tracks_) { - track->UnblockInvalidateCache(); - track->PushLengthChangeSignal(false); + track->EndOperation(); } } } @@ -1562,7 +1557,7 @@ void TrackListInsertGaps::undo_internal() } foreach (TrackOutput* track, working_tracks_) { - track->BlockInvalidateCache(); + track->BeginOperation(); } } @@ -1588,8 +1583,7 @@ void TrackListInsertGaps::undo_internal() if (all_tracks_unlocked_) { foreach (TrackOutput* track, working_tracks_) { - track->UnblockInvalidateCache(); - track->PushLengthChangeSignal(false); + track->EndOperation(); } } }