nodes: allow stalling graph signals until an operation is over

app/node/graph.cpp

@@ -22,6 +22,11 @@
 
 OLIVE_NAMESPACE_ENTER
 
+NodeGraph::NodeGraph() :
+  operation_stack_(0)
+{
+}
+
 void NodeGraph::Clear()
 {
   foreach (Node* node, node_children_) {
@@ -38,14 +43,94 @@ void NodeGraph::AddNode(Node *node)
 
   node->setParent(this);
 
-  connect(node, &Node::EdgeAdded, this, &NodeGraph::EdgeAdded);
-  connect(node, &Node::EdgeRemoved, this, &NodeGraph::EdgeRemoved);
+  connect(node, &Node::EdgeAdded, this, &NodeGraph::SignalEdgeAdded);
+  connect(node, &Node::EdgeRemoved, this, &NodeGraph::SignalEdgeRemoved);
 
   node_children_.append(node);
 
   emit NodeAdded(node);
 }
 
+void NodeGraph::BeginOperation()
+{
+  operation_stack_++;
+}
+
+void NodeGraph::EndOperation()
+{
+  operation_stack_--;
+
+  if (!operation_stack_) {
+    // Signal everything that we cached during the operation
+
+    // First, signal the removed edges
+    foreach (NodeEdgePtr e, cached_removed_edges_) {
+      emit EdgeRemoved(e);
+    }
+    cached_removed_edges_.clear();
+
+    // Next, signal the removed nodes
+    foreach (Node* n, cached_removed_nodes_) {
+      emit NodeRemoved(n);
+    }
+    cached_removed_nodes_.clear();
+
+    // Next, signal the added nodes
+    foreach (Node* n, cached_added_nodes_) {
+      emit NodeAdded(n);
+    }
+    cached_added_nodes_.clear();
+
+    // Finally, signal the added edges
+    foreach (NodeEdgePtr e, cached_added_edges_) {
+      emit EdgeAdded(e);
+    }
+    cached_added_edges_.clear();
+  }
+}
+
+void NodeGraph::SignalNodeAdded(Node* node)
+{
+  if (!operation_stack_) {
+    emit NodeAdded(node);
+  } else if (!cached_removed_nodes_.removeOne(node)) {
+    // If we already removed this node during the operation (appending a signal to
+    // cached_removed_nodes_), we just remove that instead of appending a new signal. However if we
+    // didn't (removeOne returning false), only then do we append an add signal
+    cached_added_nodes_.append(node);
+  }
+}
+
+void NodeGraph::SignalNodeRemoved(Node *node)
+{
+  if (!operation_stack_) {
+    emit NodeRemoved(node);
+  } else if (!cached_added_nodes_.removeOne(node)) {
+    // See SignalNodeAdded() for explanation of this
+    cached_removed_nodes_.append(node);
+  }
+}
+
+void NodeGraph::SignalEdgeAdded(NodeEdgePtr edge)
+{
+  if (!operation_stack_) {
+    emit EdgeAdded(edge);
+  } else if (!cached_removed_edges_.removeOne(edge)) {
+    // See SignalNodeAdded() for explanation of this
+    cached_added_edges_.append(edge);
+  }
+}
+
+void NodeGraph::SignalEdgeRemoved(NodeEdgePtr edge)
+{
+  if (!operation_stack_) {
+    emit EdgeRemoved(edge);
+  } else if (!cached_added_edges_.removeOne(edge)) {
+    // See SignalNodeAdded() for explanation of this
+    cached_removed_edges_.append(edge);
+  }
+}
+
 void NodeGraph::TakeNode(Node *node, QObject* new_parent)
 {
   if (!ContainsNode(node)) {

app/node/graph.h

@@ -37,7 +37,7 @@ public:
   /**
    * @brief NodeGraph Constructor
    */
-  NodeGraph() = default;
+  NodeGraph();
 
   /**
    * @brief Destructively destroys all nodes in the graph
@@ -67,6 +67,10 @@ public:
    */
   bool ContainsNode(Node* n) const;
 
+  void BeginOperation();
+
+  void EndOperation();
+
 signals:
   /**
    * @brief Signal emitted when a Node is added to the graph
@@ -90,6 +94,20 @@ signals:
 
 private:
   QList<Node*> node_children_;
+
+  int operation_stack_;
+
+  QList<Node*> cached_added_nodes_;
+  QList<Node*> cached_removed_nodes_;
+  QList<NodeEdgePtr> cached_added_edges_;
+  QList<NodeEdgePtr> cached_removed_edges_;
+
+private slots:
+  void SignalNodeAdded(Node *node);
+  void SignalNodeRemoved(Node* node);
+  void SignalEdgeAdded(NodeEdgePtr edge);
+  void SignalEdgeRemoved(NodeEdgePtr edge);
+
 };
 
 OLIVE_NAMESPACE_EXIT

app/node/node.h

@@ -528,6 +528,8 @@ T* Node::FindOutputNode()
   return static_cast<T*>(FindOutputNodeInternal<T>(this));
 }
 
+using NodePtr = std::shared_ptr<Node>;
+
 OLIVE_NAMESPACE_EXIT
 
 #endif // NODE_H

app/node/output/viewer/viewer.cpp

@@ -52,7 +52,7 @@ ViewerOutput::ViewerOutput() :
     connect(list, &TrackList::TrackListChanged, this, &ViewerOutput::UpdateTrackCache);
     connect(list, &TrackList::LengthChanged, this, &ViewerOutput::VerifyLength);
     connect(list, &TrackList::BlockAdded, this, &ViewerOutput::TrackListAddedBlock);
-    connect(list, &TrackList::BlockRemoved, this, &ViewerOutput::BlockRemoved);
+    connect(list, &TrackList::BlockRemoved, this, &ViewerOutput::SignalBlockRemoved);
     connect(list, &TrackList::TrackAdded, this, &ViewerOutput::TrackListAddedTrack);
     connect(list, &TrackList::TrackRemoved, this, &ViewerOutput::TrackRemoved);
     connect(list, &TrackList::TrackHeightChanged, this, &ViewerOutput::TrackHeightChangedSlot);
@@ -274,6 +274,27 @@ void ViewerOutput::set_media_name(const QString &name)
   emit MediaNameChanged(media_name_);
 }
 
+void ViewerOutput::SignalBlockAdded(Block *block, const TrackReference& track)
+{
+  if (!operation_stack_) {
+    emit BlockAdded(block, track);
+  } else {
+    cached_block_removed_.removeOne(block);
+    cached_block_added_.insert(block, track);
+  }
+}
+
+void ViewerOutput::SignalBlockRemoved(Block *block)
+{
+  if (!operation_stack_) {
+    emit BlockRemoved({block});
+  } else {
+    // We keep track of all blocks that are removed, even if we don't end up signalling them
+    cached_block_added_.remove(block);
+    cached_block_removed_.append(block);
+  }
+}
+
 void ViewerOutput::BeginOperation()
 {
   operation_stack_++;
@@ -285,13 +306,23 @@ void ViewerOutput::EndOperation()
 {
   operation_stack_--;
 
+  if (!operation_stack_) {
+    for (auto it=cached_block_added_.cbegin(); it!=cached_block_added_.cend(); it++) {
+      emit BlockAdded(it.key(), it.value());
+    }
+    cached_block_added_.clear();
+
+    emit BlockRemoved(cached_block_removed_);
+    cached_block_removed_.clear();
+  }
+
   Node::EndOperation();
 }
 
 void ViewerOutput::TrackListAddedBlock(Block *block, int index)
 {
   Timeline::TrackType type = static_cast<TrackList*>(sender())->type();
-  emit BlockAdded(block, TrackReference(type, index));
+  SignalBlockAdded(block, TrackReference(type, index));
 }
 
 void ViewerOutput::TrackListAddedTrack(TrackOutput *track)

app/node/output/viewer/viewer.h

@@ -139,7 +139,7 @@ signals:
   void AudioParamsChanged();
 
   void BlockAdded(Block* block, TrackReference track);
-  void BlockRemoved(Block* block);
+  void BlockRemoved(const QList<Block*>& blocks);
 
   void TrackAdded(TrackOutput* track, Timeline::TrackType type);
   void TrackRemoved(TrackOutput* track);
@@ -149,6 +149,9 @@ signals:
   void MediaNameChanged(const QString& name);
 
 private:
+  QMap<Block*, TrackReference> cached_block_added_;
+  QList<Block*> cached_block_removed_;
+
   QUuid uuid_;
 
   NodeInput* texture_input_;
@@ -186,6 +189,9 @@ private slots:
 
   void TrackHeightChangedSlot(int index, int height);
 
+  void SignalBlockAdded(Block *block, const TrackReference &track);
+  void SignalBlockRemoved(Block *block);
+
 };
 
 OLIVE_NAMESPACE_EXIT

app/timeline/trackreference.cpp

@@ -49,6 +49,11 @@ bool TrackReference::operator==(const TrackReference &ref) const
   return type_ == ref.type_ && index_ == ref.index_;
 }
 
+bool TrackReference::operator!=(const TrackReference &ref) const
+{
+  return !(*this == ref);
+}
+
 uint qHash(const TrackReference &r, uint seed)
 {
   // Not super efficient, but couldn't think of any better way to ensure a different hash each time

app/timeline/trackreference.h

@@ -40,10 +40,13 @@ public:
 
   bool operator==(const TrackReference& ref) const;
 
+  bool operator!=(const TrackReference& ref) const;
+
 private:
   Timeline::TrackType type_;
 
   int index_;
+
 };
 
 uint qHash(const TrackReference& r, uint seed);

app/widget/nodeview/nodeviewundo.h

@@ -149,6 +149,64 @@ private:
 
 };
 
+class NodeGraphBeginOperationCommand : public UndoCommand {
+public:
+  NodeGraphBeginOperationCommand(NodeGraph* graph, QUndoCommand* parent = nullptr) :
+    UndoCommand(parent),
+    graph_(graph)
+  {
+  }
+
+  virtual Project* GetRelevantProject() const override
+  {
+    return static_cast<Sequence*>(graph_)->project();
+  }
+
+protected:
+  virtual void redo_internal() override
+  {
+    graph_->BeginOperation();
+  }
+
+  virtual void undo_internal() override
+  {
+    graph_->EndOperation();
+  }
+
+private:
+  NodeGraph* graph_;
+
+};
+
+class NodeGraphEndOperationCommand : public UndoCommand {
+public:
+  NodeGraphEndOperationCommand(NodeGraph* graph, QUndoCommand* parent = nullptr) :
+    UndoCommand(parent),
+    graph_(graph)
+  {
+  }
+
+  virtual Project* GetRelevantProject() const override
+  {
+    return static_cast<Sequence*>(graph_)->project();
+  }
+
+protected:
+  virtual void redo_internal() override
+  {
+    graph_->EndOperation();
+  }
+
+  virtual void undo_internal() override
+  {
+    graph_->BeginOperation();
+  }
+
+private:
+  NodeGraph* graph_;
+
+};
+
 OLIVE_NAMESPACE_EXIT
 
 #endif // NODEVIEWUNDO_H

app/widget/timelinewidget/timelinewidget.cpp

@@ -918,40 +918,62 @@ void TimelineWidget::ViewDragDropped(TimelineViewMouseEvent *event)
 void TimelineWidget::AddBlock(Block *block, TrackReference track)
 {
   // Set up clip with view parameters (clip item will automatically size its rect accordingly)
-  TimelineViewBlockItem* item = new TimelineViewBlockItem(block);
+  TimelineViewBlockItem* item = block_items_.value(block);
 
-  item->SetYCoords(GetTrackY(track), GetTrackHeight(track));
-  item->SetScale(GetScale());
-  item->SetTrack(track);
-  item->SetTimebase(timebase());
+  if (!item) {
 
-  // Add to list of clip items that can be iterated through
-  block_items_.insert(block, item);
+    // Add to list of clip items that can be iterated through
+    item = new TimelineViewBlockItem(block);
+    block_items_.insert(block, item);
 
-  // Add item to graphics scene
-  views_.at(track.type())->view()->scene()->addItem(item);
+    // Set scale parameters
+    item->SetScale(GetScale());
+    item->SetTimebase(timebase());
+    item->SetYCoords(GetTrackY(track), GetTrackHeight(track));
+    item->SetTrack(track);
 
-  connect(block, &Block::Refreshed, this, &TimelineWidget::BlockRefreshed);
-  connect(block, &Block::LinksChanged, this, &TimelineWidget::BlockUpdated);
-  connect(block, &Block::LabelChanged, this, &TimelineWidget::BlockUpdated);
-  connect(block, &Block::EnabledChanged, this, &TimelineWidget::BlockUpdated);
+    // Add item to graphics scene
+    views_.at(track.type())->view()->scene()->addItem(item);
+
+    connect(block, &Block::Refreshed, this, &TimelineWidget::BlockRefreshed);
+    connect(block, &Block::LinksChanged, this, &TimelineWidget::BlockUpdated);
+    connect(block, &Block::LabelChanged, this, &TimelineWidget::BlockUpdated);
+    connect(block, &Block::EnabledChanged, this, &TimelineWidget::BlockUpdated);
+
+  } else if (item->Track() != track) {
+
+    item->SetYCoords(GetTrackY(track), GetTrackHeight(track));
+    item->SetTrack(track);
+
+  }
 }
 
-void TimelineWidget::RemoveBlock(Block *block)
+void TimelineWidget::RemoveBlock(const QList<Block *> &blocks)
 {
-  disconnect(block, &Block::Refreshed, this, &TimelineWidget::BlockRefreshed);
-  disconnect(block, &Block::LinksChanged, this, &TimelineWidget::BlockUpdated);
-  disconnect(block, &Block::LabelChanged, this, &TimelineWidget::BlockUpdated);
-  disconnect(block, &Block::EnabledChanged, this, &TimelineWidget::BlockUpdated);
+  QList<TimelineViewBlockItem*> delete_items;
+  delete_items.reserve(blocks.size());
 
-  TimelineViewBlockItem* item = block_items_.take(block);
+  QList<Block*> deselect_blocks;
 
-  if (item->isSelected()) {
-    // Sending a list of one item all the time is not very efficient
-    emit BlocksDeselected({block});
+  foreach (Block* b, blocks) {
+    disconnect(b, &Block::Refreshed, this, &TimelineWidget::BlockRefreshed);
+    disconnect(b, &Block::LinksChanged, this, &TimelineWidget::BlockUpdated);
+    disconnect(b, &Block::LabelChanged, this, &TimelineWidget::BlockUpdated);
+    disconnect(b, &Block::EnabledChanged, this, &TimelineWidget::BlockUpdated);
+
+    TimelineViewBlockItem* item = block_items_.take(b);
+    delete_items.append(item);
+
+    if (item->isSelected()) {
+      deselect_blocks.append(b);
+    }
   }
 
-  delete item;
+  if (!deselect_blocks.isEmpty()) {
+    emit BlocksDeselected(deselect_blocks);
+  }
+
+  qDeleteAll(delete_items);
 }
 
 void TimelineWidget::AddTrack(TrackOutput *track, Timeline::TrackType type)
@@ -969,9 +991,7 @@ void TimelineWidget::RemoveTrack(TrackOutput *track)
   disconnect(track, &TrackOutput::IndexChanged, this, &TimelineWidget::TrackIndexChanged);
   disconnect(track, &TrackOutput::PreviewChanged, this, &TimelineWidget::TrackPreviewUpdated);
 
-  foreach (Block* b, track->Blocks()) {
-    RemoveBlock(b);
-  }
+  RemoveBlock(track->Blocks());
 }
 
 void TimelineWidget::TrackIndexChanged()

app/widget/timelinewidget/timelinewidget.h

@@ -532,7 +532,7 @@ private slots:
   void ViewDragDropped(TimelineViewMouseEvent* event);
 
   void AddBlock(Block* block, TrackReference track);
-  void RemoveBlock(Block* block);
+  void RemoveBlock(const QList<Block*>& blocks);
 
   void AddTrack(TrackOutput* track, Timeline::TrackType type);
   void RemoveTrack(TrackOutput* track);

app/widget/timelinewidget/tool/pointer.cpp

@@ -555,6 +555,8 @@ void TimelineWidget::PointerTool::FinishDrag(TimelineViewMouseEvent *event)
 
   QUndoCommand* command = new QUndoCommand();
 
+  new NodeGraphBeginOperationCommand(static_cast<NodeGraph*>(parent()->GetConnectedNode()->parent()), command);
+
   foreach (const GhostBlockPair& p, blocks_trimming) {
     TimelineViewGhostItem* ghost = p.ghost;
 
@@ -676,6 +678,8 @@ void TimelineWidget::PointerTool::FinishDrag(TimelineViewMouseEvent *event)
     }
   }
 
+  new NodeGraphEndOperationCommand(static_cast<NodeGraph*>(parent()->GetConnectedNode()->parent()), command);
+
   Core::instance()->undo_stack()->pushIfHasChildren(command);
 }
 

app/widget/timelinewidget/undo/undo.cpp

@@ -295,7 +295,7 @@ void TrackRippleRemoveAreaCommand::undo_internal()
     track_->RippleRemoveBlock(trim_in_);
     trim_out_->set_length_and_media_out(trim_out_old_length_);
 
-    delete TakeNodeFromParentGraph(trim_in_);
+    TakeNodeFromParentGraph(trim_in_, &memory_manager_);
 
   } else {
 
@@ -390,7 +390,7 @@ void TrackPlaceBlockCommand::undo_internal()
 
     if (gap_ != nullptr) {
       track_->RippleRemoveBlock(gap_);
-      delete TakeNodeFromParentGraph(gap_);
+      TakeNodeFromParentGraph(gap_, &memory_manager_);
     }
   } else {
     TrackRippleRemoveAreaCommand::undo_internal();
@@ -926,7 +926,7 @@ void BlockTrimCommand::undo_internal()
     if (we_created_adjacent_) {
       // If we created a gap, just remove it straight up
       track_->RippleRemoveBlock(adjacent_);
-      delete TakeNodeFromParentGraph(adjacent_);
+      TakeNodeFromParentGraph(adjacent_, &memory_manager_);
       adjacent_ = nullptr;
       we_created_adjacent_ = false;
     } else if (adjacent_) {
@@ -1066,7 +1066,7 @@ void TrackReplaceBlockWithGapCommand::undo_internal()
 
     // We made this gap, simply swap our gap back
     track_->ReplaceBlock(our_gap_, block_);
-    delete TakeNodeFromParentGraph(our_gap_);
+    TakeNodeFromParentGraph(our_gap_, &memory_manager_);
     our_gap_ = nullptr;
 
   } else if (existing_gap_) {
@@ -1157,7 +1157,7 @@ void TrackSlideCommand::slide_internal(bool undo)
     if (we_created_in_adjacent_) {
       // This is a gap we made, we can just delete it entirely
       track_->RippleRemoveBlock(in_adjacent_);
-      delete TakeNodeFromParentGraph(in_adjacent_);
+      TakeNodeFromParentGraph(in_adjacent_, &memory_manager_);
       we_created_in_adjacent_ = false;
       in_adjacent_ = nullptr;
     } else if (in_adjacent_->parent() == &memory_manager_) {
@@ -1172,7 +1172,7 @@ void TrackSlideCommand::slide_internal(bool undo)
     if (we_created_out_adjacent_) {
       // This is a gap we made, we can just delete it entirely
       track_->RippleRemoveBlock(out_adjacent_);
-      delete TakeNodeFromParentGraph(out_adjacent_);
+      TakeNodeFromParentGraph(out_adjacent_, &memory_manager_);
       we_created_out_adjacent_ = false;
       out_adjacent_ = nullptr;
     } else if (out_adjacent_) {
@@ -1490,7 +1490,7 @@ void TrackListRippleToolCommand::undo_internal()
       GapBlock* gap = working_data_.at(i).created_gap;
 
       info.track->RippleRemoveBlock(gap);
-      delete TakeNodeFromParentGraph(gap);
+      TakeNodeFromParentGraph(gap, &memory_manager_);
     }
   }
 }
@@ -1593,7 +1593,7 @@ void TrackListInsertGaps::undo_internal()
   // Remove added gaps
   foreach (GapBlock* gap, gaps_added_) {
     TrackOutput::TrackFromBlock(gap)->RippleRemoveBlock(gap);
-    delete TakeNodeFromParentGraph(gap);
+    TakeNodeFromParentGraph(gap, &memory_manager_);
   }
   gaps_added_.clear();
 

app/widget/timelinewidget/undo/undo.h

@@ -589,6 +589,8 @@ private:
 
   BlockSplitPreservingLinksCommand* split_command_;
 
+  QObject memory_manager_;
+
 };
 
 class TransitionRemoveCommand : public UndoCommand {

app/window/mainwindow/mainwindow.cpp

@@ -492,8 +492,8 @@ TimelinePanel* MainWindow::AppendTimelinePanel()
   connect(panel, &TimelinePanel::TimeChanged, param_panel_, &ParamPanel::SetTimestamp);
   connect(panel, &TimelinePanel::TimeChanged, table_panel_, &NodeTablePanel::SetTimestamp);
   connect(panel, &TimelinePanel::TimeChanged, sequence_viewer_panel_, &SequenceViewerPanel::SetTimestamp);
-  connect(panel, &TimelinePanel::BlocksSelected, node_panel_, &NodePanel::SelectBlocks);
-  connect(panel, &TimelinePanel::BlocksDeselected, node_panel_, &NodePanel::DeselectBlocks);
+  //connect(panel, &TimelinePanel::BlocksSelected, node_panel_, &NodePanel::SelectBlocks);
+  //connect(panel, &TimelinePanel::BlocksDeselected, node_panel_, &NodePanel::DeselectBlocks);
   connect(param_panel_, &ParamPanel::TimeChanged, panel, &TimelinePanel::SetTimestamp);
   connect(curve_panel_, &ParamPanel::TimeChanged, panel, &TimelinePanel::SetTimestamp);
   connect(sequence_viewer_panel_, &SequenceViewerPanel::TimeChanged, panel, &TimelinePanel::SetTimestamp);