From f529c831e6a06c126293f58df5828f143c59ef68 Mon Sep 17 00:00:00 2001 From: rajvira10 Date: Sat, 17 Oct 2026 01:41:43 +0600 Subject: [PATCH] Synchronize graphical node moves with model positions Fixes #525. Fixes #526. --- src/NodeGraphicsObject.cpp | 4 ++ test/CMakeLists.txt | 1 + test/src/TestNodePosition.cpp | 129 ++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 test/src/TestNodePosition.cpp diff --git a/src/NodeGraphicsObject.cpp b/src/NodeGraphicsObject.cpp index c13b92086..7a52422a9 100644 --- a/src/NodeGraphicsObject.cpp +++ b/src/NodeGraphicsObject.cpp @@ -188,6 +188,10 @@ void NodeGraphicsObject::paint(QPainter *painter, QStyleOptionGraphicsItem const QVariant NodeGraphicsObject::itemChange(GraphicsItemChange change, const QVariant &value) { if (change == ItemScenePositionHasChanged && scene()) { + QPointF const position = pos(); + if (_graphModel.nodeData(_nodeId, NodeRole::Position) != position) { + _graphModel.setNodeData(_nodeId, NodeRole::Position, position); + } moveConnections(); } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e8f5bdc1f..2342ff64b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -9,6 +9,7 @@ add_executable(test_nodes src/TestAbstractGraphModel.cpp src/TestAbstractGraphModelSignals.cpp src/TestDataFlowGraphModel.cpp + src/TestNodePosition.cpp src/TestNodeDelegateModelRegistry.cpp src/TestConnectionId.cpp src/TestSerialization.cpp diff --git a/test/src/TestNodePosition.cpp b/test/src/TestNodePosition.cpp new file mode 100644 index 000000000..fc61b2d32 --- /dev/null +++ b/test/src/TestNodePosition.cpp @@ -0,0 +1,129 @@ +#include "ApplicationSetup.hpp" +#include "UITestHelper.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace { +class PositionTestNode : public QtNodes::NodeDelegateModel +{ +public: + QString name() const override { return "PositionTestNode"; } + QString caption() const override { return "Position Test"; } + unsigned int nPorts(QtNodes::PortType) const override { return 0; } + QtNodes::NodeDataType dataType(QtNodes::PortType, QtNodes::PortIndex) const override + { + return {}; + } + void setInData(std::shared_ptr, QtNodes::PortIndex const) override {} + std::shared_ptr outData(QtNodes::PortIndex const) override + { + return nullptr; + } + QWidget *embeddedWidget() override { return nullptr; } +}; +} // namespace + +TEST_CASE("Graphical node position changes reach the model", "[position]") +{ + auto app = applicationSetup(); + qRegisterMetaType("NodeId"); + auto registry = std::make_shared(); + registry->registerModel("PositionTestNode"); + QtNodes::DataFlowGraphModel model(registry); + auto const nodeId = model.addNode("PositionTestNode"); + QPointF const initial(100, 150); + QPointF const destination(160, 190); + model.setNodeData(nodeId, QtNodes::NodeRole::Position, initial); + QtNodes::BasicGraphicsScene scene(model); + auto *node = scene.nodeGraphicsObject(nodeId); + REQUIRE(node != nullptr); + QSignalSpy updates(&model, &QtNodes::AbstractGraphModel::nodePositionUpdated); + + SECTION("Graphical moves update and serialize the position") + { + node->setPos(destination); + CHECK(model.nodeData(nodeId, QtNodes::NodeRole::Position).value() == destination); + CHECK(updates.count() == 1); + QJsonObject const position = model.saveNode(nodeId)["position"].toObject(); + CHECK(position["x"].toDouble() == destination.x()); + CHECK(position["y"].toDouble() == destination.y()); + QtNodes::DataFlowGraphModel restored(registry); + restored.load(model.save()); + CHECK(restored.nodeData(nodeId, QtNodes::NodeRole::Position).value() + == destination); + } + + SECTION("Dragging updates the model and reports the completed move") + { + QtNodes::GraphicsView view(&scene); + view.resize(800, 600); + view.show(); + REQUIRE(QTest::qWaitForWindowExposed(&view)); + UITestHelper::waitForUI(); + QSignalSpy moved(&scene, &QtNodes::BasicGraphicsScene::nodeMoved); + QPointF const start = node->mapToScene(node->boundingRect().center()); + UITestHelper::simulateMouseDrag(&view, start, start + QPointF(40, 30)); + CHECK(node->pos() != initial); + CHECK(model.nodeData(nodeId, QtNodes::NodeRole::Position).value() == node->pos()); + REQUIRE(moved.count() == 1); + CHECK(moved.at(0).at(1).toPointF() == node->pos()); + } + + SECTION("Moving a group updates every member position") + { + auto const secondId = model.addNode("PositionTestNode"); + QPointF const secondInitial(300, 150); + model.setNodeData(secondId, QtNodes::NodeRole::Position, secondInitial); + auto *second = scene.nodeGraphicsObject(secondId); + REQUIRE(second != nullptr); + std::vector members{node, second}; + auto group = scene.createGroup(members, "Position group").lock(); + REQUIRE(group); + updates.clear(); + QPointF const offset(20, 30); + group->groupGraphicsObject().moveNodes(offset); + CHECK(model.nodeData(nodeId, QtNodes::NodeRole::Position).value() + == initial + offset); + CHECK(model.nodeData(secondId, QtNodes::NodeRole::Position).value() + == secondInitial + offset); + CHECK(updates.count() == 2); + } + + SECTION("Model updates do not emit duplicate position changes") + { + model.setNodeData(nodeId, QtNodes::NodeRole::Position, destination); + CHECK(node->pos() == destination); + CHECK(updates.count() == 1); + } + + SECTION("Unchanged graphical positions do not notify the model") + { + node->setPos(initial); + CHECK(updates.count() == 0); + } + + SECTION("Mouse release reports a completed graphical move") + { + QSignalSpy moved(&scene, &QtNodes::BasicGraphicsScene::nodeMoved); + node->setSelected(true); + node->setPos(destination); + QGraphicsSceneMouseEvent release(QEvent::GraphicsSceneMouseRelease); + release.setButton(Qt::LeftButton); + release.setPos(node->boundingRect().center()); + scene.sendEvent(node, &release); + REQUIRE(moved.count() == 1); + CHECK(moved.at(0).at(1).toPointF() == destination); + } +}