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

@@ -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

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:

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<size;i++) {
-    track_cache_.replace(i, nullptr);
-  }
-}
-
-const QVector<TrackOutput *> &TrackList::Tracks() const
+const QVector<TrackOutput *> &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<TrackOutput*>(connected_node);
 
-    track_cache_.replace(track_index, connected_track);
+    {
+      // Find "real" index
+      TrackOutput* next = nullptr;
+      for (int i=track_index+1; i<track_input_->GetSize(); i++) {
+        Node* that_track = track_input_->At(i)->get_connected_node();
+
+        if (that_track->IsTrack()) {
+          next = static_cast<TrackOutput*>(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<TrackOutput*>(connected_node) : nullptr;
 
-  if (track) {
-    track_cache_.replace(track_index, nullptr);
+  if (connected_node->IsTrack()) {
+    TrackOutput* track = static_cast<TrackOutput*>(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; i<track_cache_.size(); i++) {
+      track_cache_.at(i)->SetIndex(i);
+    }
 
     // Traverse through Tracks uncaching and disconnecting them
     emit TrackRemoved(track);

app/node/output/track/tracklist.h

@@ -38,17 +38,17 @@ public:
 
   const Timeline::TrackType& type() const;
 
-  const QVector<TrackOutput*>& Tracks() const;
+  const QVector<TrackOutput*>& 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
    */

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_) {

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<TrackOutput*>(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()) {

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();
 

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)

app/widget/timelinewidget/trackview/trackviewsplitter.cpp

@@ -141,7 +141,7 @@ void TrackViewSplitter::Remove(int index)
   QList<int> sz = sizes();
 
   if (alignment_ == Qt::AlignBottom) {
-    index = count() - index;
+    index = count() - 1 - index;
   }
 
   sz.removeAt(index);

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<GapBlock*> 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_) {

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