From f61321d68bcab3d863f1202e1d33be1adff5514b Mon Sep 17 00:00:00 2001 From: itsmattkc Date: Mon, 27 Apr 2020 01:01:12 +1000 Subject: [PATCH] nodes/timeline: correctly detect when tracks are removed from the graph Fixes several segfaults that could result from this occurring. --- app/node/output/track/track.cpp | 2 + app/node/output/track/track.h | 5 ++ app/node/output/track/tracklist.cpp | 69 +++++++++++-------- app/node/output/track/tracklist.h | 13 ++-- app/node/output/viewer/viewer.cpp | 8 +-- app/widget/timelinewidget/timelinewidget.cpp | 25 +++++-- app/widget/timelinewidget/timelinewidget.h | 1 + .../timelinewidget/trackview/trackview.cpp | 4 +- .../trackview/trackviewsplitter.cpp | 2 +- app/widget/timelinewidget/undo/undo.cpp | 8 +-- .../timelinewidget/view/timelineview.cpp | 18 ++--- 11 files changed, 92 insertions(+), 63 deletions(-) diff --git a/app/node/output/track/track.cpp b/app/node/output/track/track.cpp index aeb6be27c..ab77c7690 100644 --- a/app/node/output/track/track.cpp +++ b/app/node/output/track/track.cpp @@ -128,6 +128,8 @@ const int &TrackOutput::Index() void TrackOutput::SetIndex(const int &index) { index_ = index; + + emit IndexChanged(index); } Block *TrackOutput::BlockContainingTime(const rational &time) const diff --git a/app/node/output/track/track.h b/app/node/output/track/track.h index 11eec0f7b..6bc342256 100644 --- a/app/node/output/track/track.h +++ b/app/node/output/track/track.h @@ -185,6 +185,11 @@ signals: */ void MutedChanged(bool e); + /** + * @brief Signal emitted when the index has changed + */ + void IndexChanged(int i); + protected: private: diff --git a/app/node/output/track/tracklist.cpp b/app/node/output/track/tracklist.cpp index b3246f257..646a8309a 100644 --- a/app/node/output/track/tracklist.cpp +++ b/app/node/output/track/tracklist.cpp @@ -33,7 +33,6 @@ TrackList::TrackList(ViewerOutput *parent, const Timeline::TrackType &type, Node { connect(track_input, &NodeInputArray::SubParamEdgeAdded, this, &TrackList::TrackConnected); connect(track_input, &NodeInputArray::SubParamEdgeRemoved, this, &TrackList::TrackDisconnected); - connect(track_input, &NodeInputArray::SizeChanged, this, &TrackList::TrackListSizeChanged); } const Timeline::TrackType &TrackList::type() const @@ -51,38 +50,22 @@ void TrackList::TrackRemovedBlock(Block *block) emit BlockRemoved(block); } -void TrackList::TrackListSizeChanged(int size) -{ - int old_size = track_cache_.size(); - - track_cache_.resize(size); - - // Fill new slots with nullptr - for (int i=old_size;i &TrackList::Tracks() const +const QVector &TrackList::GetTracks() const { return track_cache_; } -TrackOutput *TrackList::TrackAt(int index) const +TrackOutput *TrackList::GetTrackAt(int index) const { - if (index < 0 || index >= track_cache_.size()) { - return nullptr; - } - return track_cache_.at(index); } -const rational &TrackList::TrackLength() const +const rational &TrackList::GetTotalLength() const { return total_length_; } -int TrackList::TrackCount() const +int TrackList::GetTrackCount() const { return track_cache_.size(); } @@ -98,7 +81,7 @@ TrackOutput* TrackList::AddTrack() NodeParam::ConnectEdge(track->output(), track_input_->At(track_input_->GetSize() - 1)); - // FIXME: Test code only + // Auto-merge with previous track if (track_input_->GetSize() > 1) { TrackOutput* last_track = nullptr; @@ -142,7 +125,6 @@ TrackOutput* TrackList::AddTrack() } } } - // End test code return track; } @@ -173,14 +155,38 @@ void TrackList::TrackConnected(NodeEdgePtr edge) if (connected_node->IsTrack()) { TrackOutput* connected_track = static_cast(connected_node); - track_cache_.replace(track_index, connected_track); + { + // Find "real" index + TrackOutput* next = nullptr; + for (int i=track_index+1; iGetSize(); i++) { + Node* that_track = track_input_->At(i)->get_connected_node(); + + if (that_track->IsTrack()) { + next = static_cast(that_track); + break; + } + } + + int track_index; + + if (next) { + // Insert track before "next" + track_index = track_cache_.indexOf(next); + track_cache_.insert(track_index, connected_track); + } else { + // No "next", this track must come at the end + track_index = track_cache_.size(); + track_cache_.append(connected_track); + } + + connected_track->SetIndex(track_index); + } connect(connected_track, &TrackOutput::BlockAdded, this, &TrackList::TrackAddedBlock); connect(connected_track, &TrackOutput::BlockRemoved, this, &TrackList::TrackRemovedBlock); connect(connected_track, &TrackOutput::TrackLengthChanged, this, &TrackList::UpdateTotalLength); connect(connected_track, &TrackOutput::TrackHeightChanged, this, &TrackList::TrackHeightChangedSlot); - connected_track->SetIndex(track_index); connected_track->set_track_type(type_); emit TrackListChanged(); @@ -201,10 +207,17 @@ void TrackList::TrackDisconnected(NodeEdgePtr edge) Q_ASSERT(track_index >= 0); Node* connected_node = edge->output()->parentNode(); - TrackOutput* track = connected_node->IsTrack() ? static_cast(connected_node) : nullptr; - if (track) { - track_cache_.replace(track_index, nullptr); + if (connected_node->IsTrack()) { + TrackOutput* track = static_cast(connected_node); + + int index_of_track = track_cache_.indexOf(track); + track_cache_.removeAt(index_of_track); + + // Update indices for all subsequent tracks + for (int i=index_of_track; iSetIndex(i); + } // Traverse through Tracks uncaching and disconnecting them emit TrackRemoved(track); diff --git a/app/node/output/track/tracklist.h b/app/node/output/track/tracklist.h index 283394909..44c380cc9 100644 --- a/app/node/output/track/tracklist.h +++ b/app/node/output/track/tracklist.h @@ -38,17 +38,17 @@ public: const Timeline::TrackType& type() const; - const QVector& Tracks() const; + const QVector& GetTracks() const; - TrackOutput* TrackAt(int index) const; + TrackOutput* GetTrackAt(int index) const; TrackOutput *AddTrack(); void RemoveTrack(); - const rational& TrackLength() const; + const rational& GetTotalLength() const; - int TrackCount() const; + int GetTrackCount() const; NodeGraph* GetParentGraph() const; @@ -100,11 +100,6 @@ private slots: */ void TrackRemovedBlock(Block* block); - /** - * @brief Slot for when the count of tracks in the track input changes - */ - void TrackListSizeChanged(int size); - /** * @brief Slot for when any of the track's length changes so we can update the length of the tracklist */ diff --git a/app/node/output/viewer/viewer.cpp b/app/node/output/viewer/viewer.cpp index 665f287f6..e06c5443d 100644 --- a/app/node/output/viewer/viewer.cpp +++ b/app/node/output/viewer/viewer.cpp @@ -167,10 +167,8 @@ void ViewerOutput::UpdateTrackCache() track_cache_.clear(); foreach (TrackList* list, track_lists_) { - foreach (TrackOutput* track, list->Tracks()) { - if (track) { - track_cache_.append(track); - } + foreach (TrackOutput* track, list->GetTracks()) { + track_cache_.append(track); } } } @@ -193,7 +191,7 @@ void ViewerOutput::UpdateLength(const rational &length) rational new_length = 0; foreach (TrackList* list, track_lists_) { - new_length = qMax(new_length, list->TrackLength()); + new_length = qMax(new_length, list->GetTotalLength()); } if (new_length != timeline_length_) { diff --git a/app/widget/timelinewidget/timelinewidget.cpp b/app/widget/timelinewidget/timelinewidget.cpp index 0ce7604d8..41a1da410 100644 --- a/app/widget/timelinewidget/timelinewidget.cpp +++ b/app/widget/timelinewidget/timelinewidget.cpp @@ -241,7 +241,7 @@ void TimelineWidget::ConnectNodeInternal(ViewerOutput *n) view->ConnectTrackList(track_list); // Defer to the track to make all the block UI items necessary - foreach (TrackOutput* track, n->track_list(track_type)->Tracks()) { + foreach (TrackOutput* track, n->track_list(track_type)->GetTracks()) { AddTrack(track, track_type); } } @@ -881,7 +881,7 @@ void TimelineWidget::InsertGapsAt(const rational &earliest_point, const rational TrackOutput *TimelineWidget::GetTrackFromReference(const TrackReference &ref) { - return GetConnectedNode()->track_list(ref.type())->TrackAt(ref.index()); + return GetConnectedNode()->track_list(ref.type())->GetTrackAt(ref.index()); } int TimelineWidget::GetTrackY(const TrackReference &ref) @@ -1017,9 +1017,7 @@ void TimelineWidget::AddBlock(Block *block, TrackReference track) void TimelineWidget::RemoveBlock(Block *block) { - delete block_items_[block]; - - block_items_.remove(block); + delete block_items_.take(block); } void TimelineWidget::AddTrack(TrackOutput *track, Timeline::TrackType type) @@ -1027,15 +1025,32 @@ void TimelineWidget::AddTrack(TrackOutput *track, Timeline::TrackType type) foreach (Block* b, track->Blocks()) { AddBlock(b, TrackReference(type, track->Index())); } + + connect(track, &TrackOutput::IndexChanged, this, &TimelineWidget::TrackIndexChanged); } void TimelineWidget::RemoveTrack(TrackOutput *track) { + disconnect(track, &TrackOutput::IndexChanged, this, &TimelineWidget::TrackIndexChanged); + foreach (Block* b, track->Blocks()) { RemoveBlock(b); } } +void TimelineWidget::TrackIndexChanged() +{ + TrackOutput* track = static_cast(sender()); + TrackReference ref(track->track_type(), track->Index()); + + foreach (Block* b, track->Blocks()) { + TimelineViewBlockItem* item = block_items_.value(b); + + item->SetYCoords(GetTrackY(ref), GetTrackHeight(ref)); + item->SetTrack(ref); + } +} + void TimelineWidget::ViewSelectionChanged() { if (rubberband_.isVisible()) { diff --git a/app/widget/timelinewidget/timelinewidget.h b/app/widget/timelinewidget/timelinewidget.h index a00027602..6f7b6a41c 100644 --- a/app/widget/timelinewidget/timelinewidget.h +++ b/app/widget/timelinewidget/timelinewidget.h @@ -486,6 +486,7 @@ private slots: void AddTrack(TrackOutput* track, Timeline::TrackType type); void RemoveTrack(TrackOutput* track); + void TrackIndexChanged(); void ViewSelectionChanged(); diff --git a/app/widget/timelinewidget/trackview/trackview.cpp b/app/widget/timelinewidget/trackview/trackview.cpp index 1d0721ea1..06d34f136 100644 --- a/app/widget/timelinewidget/trackview/trackview.cpp +++ b/app/widget/timelinewidget/trackview/trackview.cpp @@ -81,7 +81,7 @@ void TrackView::ConnectTrackList(TrackList *list) list_ = list; if (list_ != nullptr) { - foreach (TrackOutput* track, list_->Tracks()) { + foreach (TrackOutput* track, list_->GetTracks()) { TrackViewItem* item = new TrackViewItem(track); items_.append(item); splitter_->Insert(track->Index(), track->GetTrackHeight(), item); @@ -120,7 +120,7 @@ void TrackView::ScrollbarRangeChanged(int, int max) void TrackView::TrackHeightChanged(int index, int height) { - list_->TrackAt(index)->SetTrackHeight(height); + list_->GetTrackAt(index)->SetTrackHeight(height); } void TrackView::InsertTrack(TrackOutput *track) diff --git a/app/widget/timelinewidget/trackview/trackviewsplitter.cpp b/app/widget/timelinewidget/trackview/trackviewsplitter.cpp index b8895876e..108d70039 100644 --- a/app/widget/timelinewidget/trackview/trackviewsplitter.cpp +++ b/app/widget/timelinewidget/trackview/trackviewsplitter.cpp @@ -141,7 +141,7 @@ void TrackViewSplitter::Remove(int index) QList sz = sizes(); if (alignment_ == Qt::AlignBottom) { - index = count() - index; + index = count() - 1 - index; } sz.removeAt(index); diff --git a/app/widget/timelinewidget/undo/undo.cpp b/app/widget/timelinewidget/undo/undo.cpp index 8ba10b15e..992a69a89 100644 --- a/app/widget/timelinewidget/undo/undo.cpp +++ b/app/widget/timelinewidget/undo/undo.cpp @@ -347,13 +347,13 @@ void TrackPlaceBlockCommand::redo_internal() added_track_count_ = 0; // Get track (or make it if necessary) - while (track_index_ >= timeline_->Tracks().size()) { + while (track_index_ >= timeline_->GetTracks().size()) { timeline_->AddTrack(); added_track_count_++; } - track_ = timeline_->TrackAt(track_index_); + track_ = timeline_->GetTrackAt(track_index_); append_ = (in_ >= track_->track_length()); @@ -610,7 +610,7 @@ void TrackCleanGapsCommand::redo_internal() GapBlock* on_gap = nullptr; QList consecutive_gaps; - TrackOutput* track = track_list_->TrackAt(track_index_); + TrackOutput* track = track_list_->GetTrackAt(track_index_); foreach (Block* b, track->Blocks()) { if (b) { @@ -657,7 +657,7 @@ void TrackCleanGapsCommand::redo_internal() void TrackCleanGapsCommand::undo_internal() { - TrackOutput* track = track_list_->TrackAt(track_index_); + TrackOutput* track = track_list_->GetTrackAt(track_index_); // Restored removed end gaps foreach (GapBlock* gap, removed_end_gaps_) { diff --git a/app/widget/timelinewidget/view/timelineview.cpp b/app/widget/timelinewidget/view/timelineview.cpp index 0970cb2bf..04dc87864 100644 --- a/app/widget/timelinewidget/view/timelineview.cpp +++ b/app/widget/timelinewidget/view/timelineview.cpp @@ -189,11 +189,7 @@ void TimelineView::drawBackground(QPainter *painter, const QRectF &rect) int line_y = 0; - foreach (TrackOutput* track, connected_track_list_->Tracks()) { - if (!track) { - continue; - } - + foreach (TrackOutput* track, connected_track_list_->GetTracks()) { line_y += track->GetTrackHeight(); // One px gap between tracks @@ -294,9 +290,9 @@ int TimelineView::GetHeightOfAllTracks() const { if (connected_track_list_) { if (alignment() & Qt::AlignTop) { - return GetTrackY(connected_track_list_->TrackCount()); + return GetTrackY(connected_track_list_->GetTrackCount()); } else { - return GetTrackY(connected_track_list_->TrackCount() - 1); + return GetTrackY(connected_track_list_->GetTrackCount() - 1); } } else { return 0; @@ -305,6 +301,10 @@ int TimelineView::GetHeightOfAllTracks() const int TimelineView::GetTrackY(int track_index) const { + if (!connected_track_list_ || !connected_track_list_->GetTrackCount()) { + return 0; + } + int y = 0; if (alignment() & Qt::AlignBottom) { @@ -327,11 +327,11 @@ int TimelineView::GetTrackY(int track_index) const int TimelineView::GetTrackHeight(int track_index) const { - if (!connected_track_list_ || track_index >= connected_track_list_->TrackCount()) { + if (!connected_track_list_ || track_index >= connected_track_list_->GetTrackCount()) { return TrackOutput::GetDefaultTrackHeight(); } - return connected_track_list_->TrackAt(track_index)->GetTrackHeight(); + return connected_track_list_->GetTrackAt(track_index)->GetTrackHeight(); } QPoint TimelineView::GetScrollCoordinates() const