From d4fa9cb906fd34b282d7c4c5c6eddf6812b60cf8 Mon Sep 17 00:00:00 2001 From: itsmattkc <34096995+itsmattkc@users.noreply.github.com> Date: Tue, 4 Jan 2022 11:16:56 -0800 Subject: [PATCH] nodeview: fix issues when deleting nodes while dragging --- app/widget/nodeview/nodeview.cpp | 181 ++++++++++++++---------- app/widget/nodeview/nodeview.h | 4 + app/widget/nodeview/nodeviewcontext.cpp | 6 + app/widget/nodeview/nodeviewcontext.h | 3 + 4 files changed, 121 insertions(+), 73 deletions(-) diff --git a/app/widget/nodeview/nodeview.cpp b/app/widget/nodeview/nodeview.cpp index d3a7faaa5..d01bc5e81 100644 --- a/app/widget/nodeview/nodeview.cpp +++ b/app/widget/nodeview/nodeview.cpp @@ -477,77 +477,7 @@ void NodeView::mouseReleaseEvent(QMouseEvent *event) if (HandRelease(event)) return; if (create_edge_) { - // Check if the edge was reconnected to the same place as before - MultiUndoCommand* command = new MultiUndoCommand(); - - bool reconnected_to_itself = false; - - if (create_edge_already_exists_) { - if (create_edge_output_item_ == create_edge_->from_item() && create_edge_->input() == create_edge_input_) { - reconnected_to_itself = true; - } else { - // We are moving (or removing) an existing edge - command->add_child(new NodeEdgeRemoveCommand(create_edge_->output(), create_edge_->input())); - } - } else { - // We're creating a new edge, which means this UI object is only temporary - delete create_edge_; - } - - create_edge_ = nullptr; - - // Clear highlight if we set one - if (create_edge_output_item_) { - create_edge_output_item_->SetHighlighted(false); - } - if (create_edge_input_item_) { - create_edge_input_item_->SetHighlighted(false); - } - - if (create_edge_output_item_ && create_edge_input_item_) { - NodeInput &creating_input = create_edge_input_; - if (creating_input.IsValid()) { - // Make connection - if (!reconnected_to_itself) { - Node *creating_output = create_edge_output_item_->GetNode(); - - while (NodeGroup *output_group = dynamic_cast(creating_output)) { - creating_output = output_group->GetOutputPassthrough(); - } - - while (NodeGroup *input_group = dynamic_cast(creating_input.node())) { - creating_input = input_group->GetInputPassthroughs().value(creating_input.input()); - } - - if (creating_input.IsConnected()) { - Node::OutputConnection existing_edge_to_remove = {creating_input.GetConnectedOutput(), creating_input}; - command->add_child(new NodeEdgeRemoveCommand(existing_edge_to_remove.first, existing_edge_to_remove.second)); - } - - command->add_child(new NodeEdgeAddCommand(creating_output, creating_input)); - - // If the output is not in the input's context, add it now. We check the item rather than - // the node itself, because sometimes a node may not be in the context but another node - // representing it will be (e.g. groups) - if (!scene_.context_map().value(create_edge_input_item_->GetContext())->GetItemFromMap(creating_output)) { - command->add_child(new NodeSetPositionCommand(creating_output, create_edge_input_item_->GetContext(), scene_.context_map().value(create_edge_input_item_->GetContext())->MapScenePosToNodePosInContext(create_edge_output_item_->scenePos()))); - } - } - - creating_input.Reset(); - } - } - - create_edge_output_item_ = nullptr; - create_edge_input_item_ = nullptr; - - // Collapse any items we expanded - for (auto it=create_edge_expanded_items_.crbegin(); it!=create_edge_expanded_items_.crend(); it++) { - CollapseItem(*it); - } - create_edge_expanded_items_.clear(); - - Core::instance()->undo_stack()->pushIfHasChildren(command); + EndEdgeDrag(); } MultiUndoCommand* command = new MultiUndoCommand(); @@ -1122,7 +1052,7 @@ void NodeView::PositionNewEdge(const QPoint &pos) NodeViewItem* nvi = create_edge_expanded_items_.at(i); QPointF local_pt = nvi->mapFromScene(scene_pt); - if (nvi->contains(local_pt) || (!nvi->IsOutputItem() && nvi->parentItem()->contains(nvi->parentItem()->mapFromScene(scene_pt)) && local_pt.y() > nvi->rect().bottom())) { + if (nvi->scene() == &scene_ && (nvi->contains(local_pt) || (!nvi->IsOutputItem() && nvi->parentItem()->contains(nvi->parentItem()->mapFromScene(scene_pt)) && local_pt.y() > nvi->rect().bottom()))) { break; } else { // Collapsing an item will destroy its children, so if the cursor item happens to be a child @@ -1310,6 +1240,31 @@ void NodeView::LabelSelectedNodes() Core::instance()->LabelNodes(selected_nodes_); } +void NodeView::ItemAboutToBeDeleted(NodeViewItem *item) +{ + dragging_items_.remove(item); + + if (create_edge_) { + // Item should be removed from scene, but not yet deleted, allowing a safe PositionNewEdge call + // to disconnect + PositionNewEdge(mapFromGlobal(QCursor::pos())); + + QGraphicsItem *test = item; + do { + if (test == item) { + break; + } + + test = test->parentItem(); + } while (test); + + if (test == item) { + // Cancel edge function + EndEdgeDrag(true); + } + } +} + void NodeView::PasteNodesInternal(const QVector &duplicate_nodes) { // If no graph, do nothing @@ -1381,7 +1336,10 @@ void NodeView::PasteNodesInternal(const QVector &duplicate_nodes) void NodeView::AddContext(Node *n) { - scene_.AddContext(n); + NodeViewContext *ctx = scene_.AddContext(n); + + connect(ctx, &NodeViewContext::ItemAboutToBeDeleted, this, &NodeView::ItemAboutToBeDeleted); + connect(n, &Node::RemovedFromGraph, this, &NodeView::NodeRemovedFromGraph); } @@ -1414,6 +1372,83 @@ void NodeView::CollapseItem(NodeViewItem *item) item->setZValue(0); } +void NodeView::EndEdgeDrag(bool cancel) +{ + // Check if the edge was reconnected to the same place as before + MultiUndoCommand* command = new MultiUndoCommand(); + + bool reconnected_to_itself = false; + + if (create_edge_already_exists_) { + if (!cancel) { + if (create_edge_output_item_ == create_edge_->from_item() && create_edge_->input() == create_edge_input_) { + reconnected_to_itself = true; + } else { + // We are moving (or removing) an existing edge + command->add_child(new NodeEdgeRemoveCommand(create_edge_->output(), create_edge_->input())); + } + } + } else { + // We're creating a new edge, which means this UI object is only temporary + delete create_edge_; + } + + create_edge_ = nullptr; + + // Clear highlight if we set one + if (create_edge_output_item_) { + create_edge_output_item_->SetHighlighted(false); + } + if (create_edge_input_item_) { + create_edge_input_item_->SetHighlighted(false); + } + + if (create_edge_output_item_ && create_edge_input_item_ && !cancel) { + NodeInput &creating_input = create_edge_input_; + if (creating_input.IsValid()) { + // Make connection + if (!reconnected_to_itself) { + Node *creating_output = create_edge_output_item_->GetNode(); + + while (NodeGroup *output_group = dynamic_cast(creating_output)) { + creating_output = output_group->GetOutputPassthrough(); + } + + while (NodeGroup *input_group = dynamic_cast(creating_input.node())) { + creating_input = input_group->GetInputPassthroughs().value(creating_input.input()); + } + + if (creating_input.IsConnected()) { + Node::OutputConnection existing_edge_to_remove = {creating_input.GetConnectedOutput(), creating_input}; + command->add_child(new NodeEdgeRemoveCommand(existing_edge_to_remove.first, existing_edge_to_remove.second)); + } + + command->add_child(new NodeEdgeAddCommand(creating_output, creating_input)); + + // If the output is not in the input's context, add it now. We check the item rather than + // the node itself, because sometimes a node may not be in the context but another node + // representing it will be (e.g. groups) + if (!scene_.context_map().value(create_edge_input_item_->GetContext())->GetItemFromMap(creating_output)) { + command->add_child(new NodeSetPositionCommand(creating_output, create_edge_input_item_->GetContext(), scene_.context_map().value(create_edge_input_item_->GetContext())->MapScenePosToNodePosInContext(create_edge_output_item_->scenePos()))); + } + } + + creating_input.Reset(); + } + } + + create_edge_output_item_ = nullptr; + create_edge_input_item_ = nullptr; + + // Collapse any items we expanded + for (auto it=create_edge_expanded_items_.crbegin(); it!=create_edge_expanded_items_.crend(); it++) { + CollapseItem(*it); + } + create_edge_expanded_items_.clear(); + + Core::instance()->undo_stack()->pushIfHasChildren(command); +} + void NodeView::SetAttachedItems(const QVector &items) { // Detach anything currently attached diff --git a/app/widget/nodeview/nodeview.h b/app/widget/nodeview/nodeview.h index 77a188adc..510eb54db 100644 --- a/app/widget/nodeview/nodeview.h +++ b/app/widget/nodeview/nodeview.h @@ -167,6 +167,8 @@ private: void CollapseItem(NodeViewItem *item); + void EndEdgeDrag(bool cancel = false); + NodeViewMiniMap *minimap_; struct AttachedItem { @@ -248,6 +250,8 @@ private slots: void LabelSelectedNodes(); + void ItemAboutToBeDeleted(NodeViewItem *item); + }; } diff --git a/app/widget/nodeview/nodeviewcontext.cpp b/app/widget/nodeview/nodeviewcontext.cpp index 054624a11..c7a4d27e6 100644 --- a/app/widget/nodeview/nodeviewcontext.cpp +++ b/app/widget/nodeview/nodeviewcontext.cpp @@ -85,6 +85,12 @@ void NodeViewContext::RemoveChild(Node *node) NodeViewItem *item = item_map_.take(node); + // Remove from scene before emitting signal so that any drag functions that might be happening + // now can be handled before the item is destroyed + scene()->removeItem(item); + + emit ItemAboutToBeDeleted(item); + // Delete edges first because the edge destructor will try to reference item (maybe that should // be changed...) QVector edges_to_remove = item->GetAllEdgesRecursively(); diff --git a/app/widget/nodeview/nodeviewcontext.h b/app/widget/nodeview/nodeviewcontext.h index 6d191d916..c62019672 100644 --- a/app/widget/nodeview/nodeviewcontext.h +++ b/app/widget/nodeview/nodeviewcontext.h @@ -54,6 +54,9 @@ public slots: bool ChildInputDisconnected(Node *output, const NodeInput& input); +signals: + void ItemAboutToBeDeleted(NodeViewItem *item); + protected: virtual QVariant itemChange(QGraphicsItem::GraphicsItemChange change, const QVariant &value) override;