From b04536d5dd3905578ded14c2f624da3a37e805b2 Mon Sep 17 00:00:00 2001 From: barrulus Date: Fri, 18 Sep 2026 20:17:24 +0100 Subject: [PATCH 1/3] refactor(gui): extract GraphEditor for topology edits --- CMakeLists.txt | 5 + Hesiod/CMakeLists.txt | 33 +- .../include/hesiod/app/hesiod_application.hpp | 8 +- Hesiod/include/hesiod/gui/graph_editor.hpp | 94 ++ .../hesiod/gui/widgets/graph_node_widget.hpp | 25 +- .../include/hesiod/model/graph/graph_node.hpp | 2 + Hesiod/src/app/hesiod_application.cpp | 11 +- Hesiod/src/gui/graph_editor.cpp | 424 +++++++++ Hesiod/src/gui/widgets/graph_node_widget.cpp | 837 +++++------------- Hesiod/src/gui/widgets/graph_tabs_widget.cpp | 4 + Hesiod/src/model/graph/graph_node.cpp | 1 - docs/graph-editor-refactor.md | 110 +++ external/GNodeGUI | 2 +- tests/gui/test_graph_editor.cpp | 614 +++++++++++++ 14 files changed, 1515 insertions(+), 655 deletions(-) create mode 100644 Hesiod/include/hesiod/gui/graph_editor.hpp create mode 100644 Hesiod/src/gui/graph_editor.cpp create mode 100644 docs/graph-editor-refactor.md create mode 100644 tests/gui/test_graph_editor.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 2abfc978b..b6c07ef41 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26,6 +26,11 @@ option(HESIOD_ENABLE_UI_TESTS option(HESIOD_MINIMAL_NODE_SET "Use a minimal set of nodes (ADVANCED DEV ONLY!!!)" OFF) +option(HESIOD_ENABLE_TESTS "Build Hesiod integration tests" OFF) +if(HESIOD_ENABLE_TESTS) + enable_testing() +endif() + option(HESIOD_ENABLE_LTO "Enable LTO and unused-function detection" OFF) option(HESIOD_UNUSED_FUNCTIONS "Detect unused static functions" OFF) option(HESIOD_PROFILE_BUILD "Enable build time profiling (-ftime-trace)" OFF) diff --git a/Hesiod/CMakeLists.txt b/Hesiod/CMakeLists.txt index ddd40b71c..d23742439 100644 --- a/Hesiod/CMakeLists.txt +++ b/Hesiod/CMakeLists.txt @@ -10,8 +10,7 @@ set(CMAKE_AUTOMOC_VERBOSE ON) # Source files # ------------------------------ file(GLOB_RECURSE HESIOD_GUI_INCLUDES ${CMAKE_CURRENT_SOURCE_DIR}/include/*.hpp) -file(GLOB_RECURSE HESIOD_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/app/main.cpp) +file(GLOB_RECURSE HESIOD_SOURCES CONFIGURE_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp) if(HESIOD_MINIMAL_NODE_SET) # option for a minimal set of nodes for quick compile time when tempering with @@ -49,28 +48,30 @@ endif() # ------------------------------ # Executable # ------------------------------ -add_executable(${PROJECT_NAME}) -target_sources(${PROJECT_NAME} PRIVATE ${HESIOD_SOURCES} ${HESIOD_GUI_INCLUDES}) +# Share the application implementation with integration tests; compile it once. +add_library(hesiod_core OBJECT ${HESIOD_SOURCES} ${HESIOD_GUI_INCLUDES}) +add_executable(${PROJECT_NAME} app/main.cpp) +target_link_libraries(${PROJECT_NAME} PRIVATE hesiod_core) # ------------------------------ # Include directories # ------------------------------ target_include_directories( - ${PROJECT_NAME} - PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include ${CMAKE_BINARY_DIR}/include + hesiod_core + PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include ${CMAKE_BINARY_DIR}/include ${OPENGL_INCLUDE_DIRS} ${GLEW_INCLUDE_DIRS} ${GLUT_INCLUDE_DIRS}) # ------------------------------ # Compiler features # ------------------------------ -target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_20) +target_compile_features(hesiod_core PUBLIC cxx_std_20) # ------------------------------ # Link libraries # ------------------------------ target_link_libraries( - ${PROJECT_NAME} - PRIVATE hesiod_options + hesiod_core + PUBLIC hesiod_options hesiod_platform hesiod_qt_logging args @@ -94,9 +95,9 @@ target_link_libraries( # Precompiled Headers (PCH) # ------------------------------ if(HESIOD_ENABLE_PCH) - message(STATUS "Precompiled Headers (PCH) enabled for target ${PROJECT_NAME}") + message(STATUS "Precompiled Headers (PCH) enabled for target hesiod_core") target_precompile_headers( - ${PROJECT_NAME} + hesiod_core PRIVATE @@ -130,3 +131,13 @@ file(COPY ${CMAKE_SOURCE_DIR}/Hesiod/data # Build information messages # ------------------------------ message(STATUS "Build type: ${CMAKE_BUILD_TYPE}") + +if(HESIOD_ENABLE_TESTS) + find_package(Qt6 REQUIRED COMPONENTS Test) + add_executable(test_graph_editor ${CMAKE_SOURCE_DIR}/tests/gui/test_graph_editor.cpp) + target_link_libraries(test_graph_editor PRIVATE hesiod_core Qt6::Test) + add_test(NAME graph_editor COMMAND test_graph_editor) + set_tests_properties(graph_editor PROPERTIES + WORKING_DIRECTORY ${EXECUTABLE_OUTPUT_PATH} + ENVIRONMENT "QT_QPA_PLATFORM=offscreen") +endif() diff --git a/Hesiod/include/hesiod/app/hesiod_application.hpp b/Hesiod/include/hesiod/app/hesiod_application.hpp index e44e51cfc..7a4729b87 100644 --- a/Hesiod/include/hesiod/app/hesiod_application.hpp +++ b/Hesiod/include/hesiod/app/hesiod_application.hpp @@ -36,7 +36,13 @@ class HesiodApplication : public QApplication { Q_OBJECT public: - HesiodApplication(int &argc, char **argv); + enum class StartupMode + { + Normal, + ContextOnly // CPU/Qt integration tests: no engine, services or main window + }; + + HesiodApplication(int &argc, char **argv, StartupMode mode = StartupMode::Normal); ~HesiodApplication(); bool is_headless() const; diff --git a/Hesiod/include/hesiod/gui/graph_editor.hpp b/Hesiod/include/hesiod/gui/graph_editor.hpp new file mode 100644 index 000000000..8fd41ae89 --- /dev/null +++ b/Hesiod/include/hesiod/gui/graph_editor.hpp @@ -0,0 +1,94 @@ +/* Copyright (c) 2026 Otto Link. Distributed under the terms of the GNU General + * Public License. The full license is in the file LICENSE, distributed with + * this software. */ +#pragma once +#include +#include +#include +#include +#include + +#include "gnodegui/graph_viewer.hpp" + +namespace hesiod +{ +class BaseNode; +class GraphNode; + +// Coordinates topology edits and synchronization with the existing GraphViewer. +class GraphEditor +{ +public: + using Link = gngui::LinkEndpoints; + struct NodePresentation + { + std::function create; + std::function created; + std::function deleted; + std::function changed; + }; + + GraphEditor(std::weak_ptr graph, + gngui::GraphViewer &view, + NodePresentation presentation); + + // The outermost commit computes and publishes deferred notifications. + // Destruction restores scheduling state without computing; callers must roll + // back their own topology changes before abandoning a batch. + class Batch + { + public: + explicit Batch(GraphEditor &editor); + ~Batch(); + Batch(const Batch &) = delete; + Batch &operator=(const Batch &) = delete; + void commit(); + + private: + GraphEditor &editor; + std::set previous_dirty; + bool previous_full_update; + bool previous_changed; + size_t previous_notifications; + bool active = true; + }; + + std::string add_node(const std::string &type, + QPointF position, + const std::function &initialize = {}); + bool connect(const Link &link); + bool disconnect(const Link &link); + void erase(const std::vector &node_ids, + const std::vector &links = {}); + void clear(); + std::string replace_node(const std::string &id, const std::string &type); + std::string insert_node(const std::string &id, + const std::string &type, + QPointF position); + nlohmann::json import_nodes(const nlohmann::json &json, QPointF origin); + void request_update(const std::vector &ids = {}); + +private: + struct Notification + { + std::string id; + bool created; + }; + + std::shared_ptr graph() const; + std::vector links_for(const std::string &id) const; + void validate_connection(const Link &link) const; + bool compatible(const Link &link) const; + void finish_batch(); + void restore_links(const std::vector &links); + + std::weak_ptr model; + gngui::GraphViewer &view; + NodePresentation presentation; + size_t batch_depth = 0; + std::set dirty; + bool full_update = false; + bool changed = false; + std::vector notifications; +}; +} // namespace hesiod diff --git a/Hesiod/include/hesiod/gui/widgets/graph_node_widget.hpp b/Hesiod/include/hesiod/gui/widgets/graph_node_widget.hpp index b99574436..5ee7135fe 100644 --- a/Hesiod/include/hesiod/gui/widgets/graph_node_widget.hpp +++ b/Hesiod/include/hesiod/gui/widgets/graph_node_widget.hpp @@ -14,6 +14,7 @@ namespace hesiod { class GraphNode; // forward +class GraphEditor; // ===================================== // GraphNodeWidget @@ -50,11 +51,6 @@ class GraphNodeWidget : public gngui::GraphViewer void apply_new_config(int new_resolution); void apply_new_config(const GraphConfig &new_config); - // NB - only block updates coming from GraphNodeWidget, other classes may trigger an - // update of the model - bool is_graph_model_updates_blocked() const; - void set_block_graph_model_updates(bool new_state); - void update_graph_model(const std::vector &node_ids = {}); void update_graph_model(const std::string &node_id); @@ -62,6 +58,7 @@ class GraphNodeWidget : public gngui::GraphViewer // TODO REMOVE GRAPH_ID // --- User Actions Signals --- + void graph_edited(); void copy_buffer_has_changed(const nlohmann::json &new_json); void has_been_cleared(const std::string &graph_id); void new_node_created(const std::string &graph_id, const std::string &id); @@ -79,19 +76,9 @@ public slots: void closeEvent(QCloseEvent *event) override; // --- User Actions --- - void on_connection_deleted(const std::string &id_out, - const std::string &port_id_out, - const std::string &id_in, - const std::string &port_id_in, - bool prevent_graph_update); void on_connection_dropped(const std::string &node_id, const std::string &port_id, QPointF scene_pos); - void on_connection_finished(const std::string &id_out, - const std::string &port_id_out, - const std::string &id_in, - const std::string &port_id_in); - void on_graph_clear_request(); void on_graph_import_request(); void on_graph_new_request(); @@ -101,7 +88,6 @@ public slots: std::string on_new_node_request(const std::string &node_type, QPointF scene_pos); std::string on_new_node_request_chain(const std::string &node_type); std::string on_new_node_request_replace(const std::string &node_type); - void on_node_deleted_request(const std::string &node_id); void on_node_reload_request(const std::string &node_id); void on_node_right_clicked(const std::string &node_id, QPointF scene_pos); @@ -118,6 +104,11 @@ public slots: // --- Others... --- void on_new_graphics_node_request(const std::string &node_id, QPointF scene_pos); +protected: + void request_connection(const gngui::LinkEndpoints &link) override; + void request_deletion(const std::vector &ids, + const std::vector &links) override; + private: QScrollArea *create_attributes_scroll(QWidget *parent, QWidget *attr_widget); @@ -127,7 +118,7 @@ public slots: // --- Members --- std::weak_ptr p_graph_node; // own by GraphManager std::vector> data_viewers; - bool block_graph_model_updates = false; + std::unique_ptr editor; nlohmann::json json_copy_buffer; std::string last_node_created_id = ""; bool is_selecting_with_rubber_band = false; diff --git a/Hesiod/include/hesiod/model/graph/graph_node.hpp b/Hesiod/include/hesiod/model/graph/graph_node.hpp index 7efd4d174..33e49729b 100644 --- a/Hesiod/include/hesiod/model/graph/graph_node.hpp +++ b/Hesiod/include/hesiod/model/graph/graph_node.hpp @@ -38,6 +38,8 @@ class GraphNode : public gnode::Graph, void change_config_values(const GraphConfig &new_config); // --- Node Factory (create nodes from their type) --- + // Construction does not compute: callers update after setting parameters and + // connections, so compound edits never evaluate a partially constructed graph. std::string add_node(const std::string &node_type); // --- GNode::Graph override --- diff --git a/Hesiod/src/app/hesiod_application.cpp b/Hesiod/src/app/hesiod_application.cpp index 002363dfc..2d39fcc55 100644 --- a/Hesiod/src/app/hesiod_application.cpp +++ b/Hesiod/src/app/hesiod_application.cpp @@ -51,7 +51,8 @@ namespace fs = std::filesystem; namespace hesiod { -HesiodApplication::HesiodApplication(int &argc, char **argv) : QApplication(argc, argv) +HesiodApplication::HesiodApplication(int &argc, char **argv, StartupMode mode) + : QApplication(argc, argv) { Logger::log()->trace("HesiodApplication::HesiodApplication"); @@ -60,6 +61,14 @@ HesiodApplication::HesiodApplication(int &argc, char **argv) : QApplication(argc // context this->context.initialize(); + if (mode == StartupMode::ContextOnly) + { + meta::qt::stock::register_design(); + this->headless = true; + this->context.headless = true; + return; + } + // force icons visibility in the menu bar this->setAttribute(Qt::AA_DontShowIconsInMenus, false); QStyle *style = QApplication::style(); diff --git a/Hesiod/src/gui/graph_editor.cpp b/Hesiod/src/gui/graph_editor.cpp new file mode 100644 index 000000000..b98eb0a72 --- /dev/null +++ b/Hesiod/src/gui/graph_editor.cpp @@ -0,0 +1,424 @@ +/* Copyright (c) 2026 Otto Link. Distributed under the terms of the GNU General + * Public License. The full license is in the file LICENSE, distributed with + * this software. */ +#include "hesiod/gui/graph_editor.hpp" + +#include +#include + +#include "hesiod/model/graph/graph_node.hpp" +#include "hesiod/model/nodes/base_node.hpp" +#include "hesiod/model/nodes/legacy/legacy_converter.hpp" +#include "hesiod/model/utils.hpp" + +namespace hesiod +{ +GraphEditor::GraphEditor(std::weak_ptr graph, + gngui::GraphViewer &view, + NodePresentation presentation) + : model(std::move(graph)), view(view), presentation(std::move(presentation)) +{ +} + +GraphEditor::Batch::Batch(GraphEditor &editor) + : editor(editor), previous_dirty(editor.dirty), + previous_full_update(editor.full_update), previous_changed(editor.changed), + previous_notifications(editor.notifications.size()) +{ + ++editor.batch_depth; +} + +GraphEditor::Batch::~Batch() +{ + if (active) + { + editor.dirty = std::move(previous_dirty); + editor.full_update = previous_full_update; + editor.changed = previous_changed; + editor.notifications.resize(previous_notifications); + --editor.batch_depth; + } +} + +void GraphEditor::Batch::commit() +{ + if (!active) + return; + active = false; + if (--editor.batch_depth == 0) + editor.finish_batch(); +} + +std::shared_ptr GraphEditor::graph() const +{ + auto graph = model.lock(); + if (!graph) + throw std::runtime_error("The graph is no longer available."); + return graph; +} + +void GraphEditor::finish_batch() +{ + auto graph = this->graph(); + auto pending = std::exchange(notifications, {}); + auto pending_dirty = std::exchange(dirty, {}); + const bool update_all = std::exchange(full_update, false); + const bool edited = std::exchange(changed, false); + + // Publish only once model and scene agree. Clear scheduling state first so a + // callback or a failed computation cannot leave future edits blocked. + for (const auto ¬ice : pending) + { + const auto &callback = notice.created ? presentation.created : presentation.deleted; + if (callback) + callback(notice.id); + } + + if (edited && presentation.changed) + presentation.changed(); + + if (update_all) + graph->update(); + else + { + std::vector ids; + for (const auto &id : pending_dirty) + if (graph->get_node(id)) + ids.push_back(id); + if (!ids.empty()) + graph->update(ids); + } +} + +void GraphEditor::request_update(const std::vector &ids) +{ + Batch batch(*this); + if (ids.empty()) + full_update = true; + else + dirty.insert(ids.begin(), ids.end()); + batch.commit(); +} + +std::vector GraphEditor::links_for(const std::string &id) const +{ + std::vector result; + for (const auto &link : graph()->get_link_views(id)) + result.push_back({link.from, link.port_label_from, link.to, link.port_label_to}); + return result; +} + +bool GraphEditor::compatible(const Link &link) const +{ + auto graph = this->graph(); + auto *from = graph->get_node(link.node_out); + auto *to = graph->get_node(link.node_in); + if (!from || !to || from == to) + return false; + const int output = from->get_port_index(link.port_out); + const int input = to->get_port_index(link.port_in); + return output >= 0 && input >= 0 && + from->get_port_type(link.port_out) == gnode::PortType::OUT && + to->get_port_type(link.port_in) == gnode::PortType::IN && + from->get_data_type(output) == to->get_data_type(input); +} + +void GraphEditor::validate_connection(const Link &link) const +{ + if (!compatible(link)) + throw std::invalid_argument("Cannot connect the selected nodes: incompatible ports."); + if (graph()->is_reachable(link.node_in, link.node_out)) + throw std::invalid_argument( + "Cannot connect the selected nodes: this would create a cycle."); + auto *from = view.get_graphics_node_by_id(link.node_out); + auto *to = view.get_graphics_node_by_id(link.node_in); + if (!from || !to || from->get_port_index(link.port_out) < 0 || + to->get_port_index(link.port_in) < 0) + throw std::runtime_error("Cannot connect nodes missing from the scene."); +} + +std::string GraphEditor::add_node(const std::string &type, + QPointF position, + const std::function &initialize) +{ + if (type.empty()) + return {}; + auto graph = this->graph(); + Batch batch(*this); + const std::string id = graph->add_node(type); + try + { + auto *node = graph->get_node_ref_by_id(id); + if (initialize) + initialize(*node); + node->set_id(id); // imported settings must not restore the original ID + presentation.create(id, position); + if (!view.get_graphics_node_by_id(id)) + throw std::runtime_error("Could not create the graphics node."); + } + catch (...) + { + // Proxies still refer to live model nodes while their graphics are erased. + graph->get_node_ref_by_id(id)->set_id(id); + view.erase_node(id); + graph->remove_node(id); + throw; + } + notifications.push_back({id, true}); + changed = true; + dirty.insert(id); + batch.commit(); + return id; +} + +bool GraphEditor::connect(const Link &link) +{ + auto graph = this->graph(); + validate_connection(link); + const auto adjacent = links_for(link.node_in); + if (hesiod::contains(adjacent, link)) + return false; + + std::vector previous; + for (const auto &old : adjacent) + if (old.node_in == link.node_in && old.port_in == link.port_in) + previous.push_back(old); + + Batch batch(*this); + // Validate before disturbing the old input, and keep its graphics until the + // model accepts the replacement. Restore both sides if synchronization fails. + try + { + for (const auto &old : previous) + graph->remove_link(old.node_out, old.port_out, old.node_in, old.port_in); + if (!graph->new_link(link.node_out, link.port_out, link.node_in, link.port_in)) + throw std::runtime_error("The graph did not accept the connection."); + for (const auto &old : previous) + view.erase_link(old); + view.add_link(link.node_out, link.port_out, link.node_in, link.port_in); + } + catch (...) + { + graph->remove_link(link.node_out, link.port_out, link.node_in, link.port_in); + view.erase_link(link); + for (const auto &old : previous) + { + graph->new_link(old.node_out, old.port_out, old.node_in, old.port_in); + view.erase_link(old); + view.add_link(old.node_out, old.port_out, old.node_in, old.port_in); + } + throw; + } + dirty.insert(link.node_in); + changed = true; + batch.commit(); + return true; +} + +bool GraphEditor::disconnect(const Link &link) +{ + auto graph = this->graph(); + Batch batch(*this); + const bool removed = graph->remove_link(link.node_out, + link.port_out, + link.node_in, + link.port_in); + view.erase_link(link); + if (removed) + { + dirty.insert(link.node_in); + changed = true; + } + batch.commit(); + return removed; +} + +void GraphEditor::erase(const std::vector &node_ids, + const std::vector &links) +{ + auto graph = this->graph(); + Batch batch(*this); + for (const auto &link : links) + disconnect(link); + const std::set unique_ids(node_ids.begin(), node_ids.end()); + for (const auto &id : unique_ids) + { + if (!graph->get_node(id)) + continue; + for (const auto &link : links_for(id)) + if (link.node_out == id) + dirty.insert(link.node_in); + view.erase_node(id); + // GraphNode owns Broadcast/Receive cleanup; never bypass its remove_node. + graph->remove_node(id); + notifications.push_back({id, false}); + changed = true; + dirty.erase(id); + } + batch.commit(); +} + +void GraphEditor::clear() +{ + std::vector ids; + for (const auto &[id, node] : graph()->get_nodes()) + ids.push_back(id); + erase(ids); + view.clear(); // comments and groups also belong to the cleared scene +} + +void GraphEditor::restore_links(const std::vector &links) +{ + for (const auto &link : links) + connect(link); +} + +std::string GraphEditor::replace_node(const std::string &id, const std::string &type) +{ + auto graph = this->graph(); + auto *graphics = view.get_graphics_node_by_id(id); + if (!graph->get_node(id) || !graphics) + throw std::invalid_argument("Select an existing node to replace."); + const auto previous = links_for(id); + Batch batch(*this); + const std::string replacement = add_node(type, graphics->pos()); + if (replacement.empty()) + return {}; + try + { + for (auto link : previous) + { + if (link.node_out == id) + link.node_out = replacement; + if (link.node_in == id) + link.node_in = replacement; + if (compatible(link)) + connect(link); + } + } + catch (...) + { + erase({replacement}); + restore_links(previous); + throw; + } + // Keep the original node until replacement creation and reconnection succeed. + erase({id}); + batch.commit(); + return replacement; +} + +std::string GraphEditor::insert_node(const std::string &id, + const std::string &type, + QPointF position) +{ + auto graph = this->graph(); + if (!graph->get_node(id) || !view.get_graphics_node_by_id(id)) + throw std::invalid_argument("Select an existing node to insert after."); + const auto previous = links_for(id); + Batch batch(*this); + const std::string inserted = add_node(type, position); + if (inserted.empty()) + return {}; + try + { + bool connected = false; + for (const auto &old : previous) + { + if (old.node_out != id) + continue; + const Link upstream{id, old.port_out, inserted, old.port_in}; + const Link downstream{inserted, old.port_out, old.node_in, old.port_in}; + // Preserve branches unless both halves can reconnect. + if (compatible(upstream) && compatible(downstream)) + { + connect(upstream); + connect(downstream); + connected = true; + } + } + if (!connected) + { + auto *from = graph->get_node(id); + auto *to = graph->get_node(inserted); + for (int out = 0; out < from->get_nports() && !connected; ++out) + for (int in = 0; in < to->get_nports() && !connected; ++in) + { + Link link{id, from->get_port_label(out), inserted, to->get_port_label(in)}; + if (compatible(link)) + { + connect(link); + connected = true; + } + } + } + } + catch (...) + { + erase({inserted}); + restore_links(previous); + throw; + } + batch.commit(); + return inserted; +} + +nlohmann::json GraphEditor::import_nodes(const nlohmann::json &json, QPointF origin) +{ + nlohmann::json result = convert_legacy_graph_widget_json(json); + if (!result.contains("nodes") || result["nodes"].is_null()) + return result; + if (!result["nodes"].is_array()) + throw std::invalid_argument("Imported nodes must be an array."); + Batch batch(*this); + std::map ids; + std::vector created; + try + { + for (auto &node : result["nodes"]) + { + const auto old_id = node.at("id").get(); + if (ids.contains(old_id)) + throw std::invalid_argument("Duplicate node ID in the imported graph."); + const QPointF offset(node.at("scene_position.x").get(), + node.at("scene_position.y").get()); + const auto id = add_node(node.at("caption").get(), + origin + offset, + [&](BaseNode &model) + { + auto settings = node.at("settings"); + settings["id"] = model.get_id(); + model.json_from(settings); + }); + if (id.empty()) + throw std::invalid_argument("Missing node type in the imported graph."); + created.push_back(id); + ids.emplace(old_id, id); + node["id"] = id; + node["settings"]["id"] = id; + } + if (result.contains("links") && !result["links"].is_null()) + { + if (!result["links"].is_array()) + throw std::invalid_argument("Imported links must be an array."); + for (auto &link : result["links"]) + { + const auto from = ids.at(link.at("node_out_id").get()); + const auto to = ids.at(link.at("node_in_id").get()); + const auto out = link.at("port_out_id").get(); + const auto in = link.at("port_in_id").get(); + connect({from, out, to, in}); + link["node_out_id"] = from; + link["node_in_id"] = to; + } + } + } + catch (...) + { + erase(created); + throw; + } + batch.commit(); + return result; +} +} // namespace hesiod diff --git a/Hesiod/src/gui/widgets/graph_node_widget.cpp b/Hesiod/src/gui/widgets/graph_node_widget.cpp index 8a473eb79..1ce522a61 100644 --- a/Hesiod/src/gui/widgets/graph_node_widget.cpp +++ b/Hesiod/src/gui/widgets/graph_node_widget.cpp @@ -3,6 +3,7 @@ * this software. */ #include #include +#include #include #include @@ -16,6 +17,7 @@ #include #include "hesiod/app/hesiod_application.hpp" +#include "hesiod/gui/graph_editor.hpp" #include "hesiod/gui/widgets/custom_qmenu.hpp" #include "hesiod/gui/widgets/graph_config_widgets/graph_config_dialog.hpp" #include "hesiod/gui/widgets/graph_node_widget.hpp" @@ -35,12 +37,45 @@ namespace hesiod { +namespace +{ +// Qt event handlers report an edit failure without unwinding through the event loop. +template auto perform_graph_edit(F &&edit) -> std::invoke_result_t +{ + try + { + return edit(); + } + catch (const std::exception &error) + { + Logger::log()->error("Graph edit failed: {}", error.what()); + if (!HSD_CTX.headless) + HSD_APP->notify(error.what()); + if constexpr (!std::is_void_v>) + return {}; + } +} +} // namespace GraphNodeWidget::GraphNodeWidget(std::weak_ptr p_graph_node, QWidget *parent) : GraphViewer("", parent), p_graph_node(p_graph_node) { Logger::log()->trace("GraphNodeWidget::GraphNodeWidget: id: {}", this->get_id()); + this->editor = std::make_unique( + p_graph_node, + *this, + GraphEditor::NodePresentation{[this](const std::string &id, QPointF pos) + { this->on_new_graphics_node_request(id, pos); }, + [this](const std::string &id) + { + this->last_node_created_id = id; + Q_EMIT this->new_node_created(this->get_id(), id); + }, + [this](const std::string &id) + { Q_EMIT this->node_deleted(this->get_id(), id); }, + [this]() { Q_EMIT this->graph_edited(); }}); + auto gno = this->p_graph_node.lock(); if (!gno) return; @@ -63,83 +98,56 @@ GraphNodeWidget::~GraphNodeWidget() void GraphNodeWidget::add_import_heightmap_node(const QImage &img) { - Logger::log()->trace("GraphNodeWidget::add_import_heightmap_node"); - - auto gno = this->p_graph_node.lock(); - if (!gno) - return; - - // --- Crop and save heightmap file - - const std::filesystem::path path = HSD_CTX.project_model->get_path(); - const glm::ivec2 model_shape = gno->get_config_ref()->shape; - - float aspect_ratio = model_shape.x / model_shape.y; - - // --- Create new import node - - Logger::log()->trace("GraphNodeWidget::add_import_heightmap_node: creating new node"); - - // create both model and graphic nodes - std::string node_id = this->on_new_node_request("ImportHeightmap", this->get_center()); - - // setup attributes - BaseNode *p_node = gno->get_node_ref_by_id(node_id); - - if (p_node) - { - // create filename and save image - std::filesystem::path fpath = path / ("heightmapper_import_" + node_id + ".png"); - save_heightmap(img, fpath, aspect_ratio); - - // adjust node parameter accordingly - p_node->set_value("fname", fpath.string()); - p_node->set_value("dequantize", true); - - p_node->compute(); - } - else - { - Logger::log()->error( - "GraphNodeWidget::add_import_heightmap_node: dangling ptr for node_id {}", - node_id); - } + perform_graph_edit( + [&]() + { + auto graph = this->p_graph_node.lock(); + if (!graph) + return; + const auto path = HSD_CTX.project_model->get_path(); + const auto shape = graph->get_config_ref()->shape; + this->editor->add_node( + "ImportHeightmap", + this->get_center(), + [&](BaseNode &node) + { + const auto file = path / ("heightmapper_import_" + node.get_id() + ".png"); + save_heightmap(img, file, static_cast(shape.x) / shape.y); + node.set_value("fname", file); + node.set_value("dequantize", true); + }); + }); } -void GraphNodeWidget::add_import_texture_nodes( - const std::vector &texture_paths) +void GraphNodeWidget::add_import_texture_nodes(const std::vector &paths) { - auto gno = this->p_graph_node.lock(); - if (!gno) - return; - - float dx = HSD_CTX.app_settings.node_editor.position_delta_when_duplicating_node; - float dy = 0.f; - - for (auto &fname : texture_paths) - { - Logger::log()->trace("GraphNodeWidget::add_import_texture_nodes: {}", fname); - - // create both model and graphic nodes - QPointF pos = this->get_center() + QPointF(dx, dy); - std::string node_id = this->on_new_node_request("ImportTexture", pos); - - // setup attributes - BaseNode *p_node = gno->get_node_ref_by_id(node_id); - if (p_node) - { - p_node->set_value("fname", fname); - p_node->compute(); - } - else - { - Logger::log()->error( - "GraphNodeWidget::add_import_texture_nodes: dangling ptr for node_id {}", - node_id); - } - - dy += dx; - } + perform_graph_edit( + [&]() + { + GraphEditor::Batch batch(*this->editor); + const float delta = HSD_CTX.app_settings.node_editor + .position_delta_when_duplicating_node; + float y = 0.f; + std::vector created; + try + { + for (const auto &path : paths) + { + created.push_back(this->editor->add_node( + "ImportTexture", + this->get_center() + QPointF(delta, y), + [&](BaseNode &node) + { node.set_value("fname", path); })); + y += delta; + } + } + catch (...) + { + this->editor->erase(created); + throw; + } + batch.commit(); + }); } void GraphNodeWidget::apply_new_config(int new_resolution) @@ -218,7 +226,7 @@ void GraphNodeWidget::automatic_node_layout() p_gfx_node->setPos(scene_pos); } - QTimer::singleShot(0, [this]() { this->zoom_to_content(); }); + QTimer::singleShot(0, this, [this]() { this->zoom_to_content(); }); } void GraphNodeWidget::backup_selected_ids() @@ -228,9 +236,13 @@ void GraphNodeWidget::backup_selected_ids() void GraphNodeWidget::clear_all() { - this->clear_graphic_scene(); - - Q_EMIT this->has_been_cleared(this->get_id()); + perform_graph_edit( + [&]() + { + this->clear_data_viewers(); + this->editor->clear(); + Q_EMIT this->has_been_cleared(this->get_id()); + }); } void GraphNodeWidget::clear_data_viewers() @@ -300,28 +312,15 @@ GraphNode *GraphNodeWidget::get_p_graph_node() return gno.get(); } -bool GraphNodeWidget::is_graph_model_updates_blocked() const -{ - return this->block_graph_model_updates; -} - void GraphNodeWidget::json_from(nlohmann::json const &json) { Logger::log()->trace("GraphNodeWidget::json_from"); this->clear_graphic_scene(); - // legacy projects may reference output ports by their pre-consolidation - // label "out"; the model applies the same rename in GraphNode::json_from, - // and the graphics links need it too or their port lookup fails nlohmann::json gui_json = convert_legacy_graph_widget_json(json); - // the graph model loading and updating is taken care of by - // GraphNode (model) and does not need to be updated again when the - // graphics object are recreated (and are going to trigger signals - // requesting some model updates...) - this->set_block_graph_model_updates(true); + // Loading graphics is presentation-only; edit requests are not emitted. GraphViewer::json_from(gui_json); - this->set_block_graph_model_updates(false); // viewers (skipped in headless CLI modes, e.g. --snapshot: no 3D viewer is // created there, so there is nothing to restore state into) @@ -359,6 +358,7 @@ void GraphNodeWidget::json_from(nlohmann::json const &json) // defer QTimer::singleShot(0, + this, [this]() { this->update(); @@ -368,65 +368,8 @@ void GraphNodeWidget::json_from(nlohmann::json const &json) nlohmann::json GraphNodeWidget::json_import(nlohmann::json const &json, QPointF scene_pos) { - // import used when copy/pasting only - Logger::log()->trace("GraphNodeWidget::json_import"); - - auto gno = this->p_graph_node.lock(); - if (!gno) - return nlohmann::json(); - - // work on a copy of the json to modify the node IDs and return it - nlohmann::json json_copy = json; - - // nodes - if (!json_copy["nodes"].is_null()) - { - // storage of id correspondance storage between original node and it copied version - std::map copy_id_map = {}; - - for (auto &json_node : json_copy["nodes"]) - { - QPointF pos = scene_pos; - QPointF delta = QPointF(json_node["scene_position.x"], - json_node["scene_position.y"]); - - // create both model and graphic nodes - std::string node_id = this->on_new_node_request(json_node["caption"], pos + delta); - - // use this new node id and backup it for links - copy_id_map[json_node["id"].get()] = node_id; - json_node["id"] = node_id; - - // setup attributes - BaseNode *p_node = gno->get_node_ref_by_id(node_id); - p_node->json_from(json_node["settings"]); - p_node->set_id(node_id); - } - - // links - if (!json_copy["links"].is_null()) - for (auto &json_link : json_copy["links"]) - { - std::string node_id_from = json_link["node_out_id"].get(); - std::string node_id_to = json_link["node_in_id"].get(); - - node_id_from = copy_id_map.at(node_id_from); - node_id_to = copy_id_map.at(node_id_to); - - std::string port_out_id = json_link["port_out_id"]; - std::string port_in_id = json_link["port_in_id"]; - - // add graphics link - this->add_link(node_id_from, port_out_id, node_id_to, port_in_id); - - // add model link - gno->new_link(node_id_from, port_out_id, node_id_to, port_in_id); - } - - this->update_graph_model(); - } - - return json_copy; + return perform_graph_edit([&]() + { return this->editor->import_nodes(json, scene_pos); }); } nlohmann::json GraphNodeWidget::json_to() const @@ -445,168 +388,111 @@ nlohmann::json GraphNodeWidget::json_to() const return json; } -void GraphNodeWidget::on_connection_deleted(const std::string &id_out, - const std::string &port_id_out, - const std::string &id_in, - const std::string &port_id_in, - bool prevent_graph_update) +void GraphNodeWidget::request_connection(const gngui::LinkEndpoints &link) { - Logger::log()->trace("GraphNodeWidget::on_connection_deleted, {}/{} -> {}/{}", - id_out, - port_id_out, - id_in, - port_id_in); - - auto gno = this->p_graph_node.lock(); - if (!gno) - return; - - Logger::log()->debug("BLOCKED? {}", - this->is_graph_model_updates_blocked() ? "TRUE" : "FALSE"); - - this->set_enabled(false); - - // see GraphNodeWidget::on_node_deleted - QCoreApplication::processEvents(); - - gno->remove_link(id_out, port_id_out, id_in, port_id_in); - - // see GraphNodeWidget::on_node_deleted - QCoreApplication::processEvents(); - - if (!prevent_graph_update) - this->update_graph_model(id_in); + perform_graph_edit([&]() { this->editor->connect(link); }); +} - this->set_enabled(true); +void GraphNodeWidget::request_deletion(const std::vector &ids, + const std::vector &links) +{ + perform_graph_edit([&]() { this->editor->erase(ids, links); }); } void GraphNodeWidget::on_connection_dropped(const std::string &node_id, const std::string &port_id, QPointF /*scene_pos*/) { - Logger::log()->trace("GraphNodeWidget::on_connection_dropped: {}/{}", node_id, port_id); - - auto gno = this->p_graph_node.lock(); - if (!gno) - return; - - BaseNode *p_node_from = gno->get_node_ref_by_id(node_id); - if (!p_node_from) - return; - - // --- what was dragged, and what would we need on the other end? - - const int from_index = p_node_from->get_port_index(port_id); + perform_graph_edit( + [&]() + { + Logger::log()->trace("GraphNodeWidget::on_connection_dropped: {}/{}", + node_id, + port_id); - // NOTE: get_data_type() returns a MANGLED typeid name (e.g. - // "N4hmap12VirtualArrayE"). The catalog and select_port both speak the - // friendly name the documentation uses ("VirtualArray") — the documentation - // is literally built with map_type_name(get_data_type(k)) (base_node.cpp). - // Convert once, here at the boundary. - const std::string dragged_type = map_type_name(p_node_from->get_data_type(from_index)); + auto gno = this->p_graph_node.lock(); + if (!gno) + return; - const gngui::PortType dragged_dir = p_node_from->get_port_type(from_index); - const gngui::PortType wanted_dir = (dragged_dir == gngui::PortType::OUT) - ? gngui::PortType::IN - : gngui::PortType::OUT; + BaseNode *p_node_from = gno->get_node_ref_by_id(node_id); + if (!p_node_from) + return; - // --- offer only node types that can actually connect - // - // The menu is built by GraphViewer from its node inventory, so filtering is - // done by swapping the inventory around the (blocking) menu call and putting - // the full one back afterwards. + const int from_index = p_node_from->get_port_index(port_id); + if (from_index < 0) + return; - const std::map full_inventory = get_node_inventory(); - const PortCatalog catalog = PortCatalog::from_documentation(); + // The catalog and select_port use documentation names, not typeid names. + const std::string dragged_type = map_type_name( + p_node_from->get_data_type(from_index)); - std::map filtered; - for (const auto &[node_type, category] : full_inventory) - if (catalog.is_offerable(node_type, dragged_type, wanted_dir)) - filtered[node_type] = category; + const gngui::PortType dragged_dir = p_node_from->get_port_type(from_index); + const gngui::PortType wanted_dir = (dragged_dir == gngui::PortType::OUT) + ? gngui::PortType::IN + : gngui::PortType::OUT; - // if nothing accepts this type, fall back to the full list rather than - // opening an empty menu - const bool use_filtered = !filtered.empty(); + // Filter GraphViewer's inventory for the duration of its blocking menu. + const std::map full_inventory = get_node_inventory(); + const PortCatalog catalog = PortCatalog::from_documentation(); - if (use_filtered) - this->set_node_inventory(filtered); + std::map filtered; + for (const auto &[node_type, category] : full_inventory) + if (catalog.is_offerable(node_type, dragged_type, wanted_dir)) + filtered[node_type] = category; - const bool created = this->execute_new_node_context_menu(); + // Fall back to the full inventory if no compatible types are documented. + const bool use_filtered = !filtered.empty(); - if (use_filtered) - this->set_node_inventory(full_inventory); + if (use_filtered) + this->set_node_inventory(filtered); - if (!created) - return; + GraphEditor::Batch batch(*this->editor); + this->last_node_created_id.clear(); + const bool created = this->execute_new_node_context_menu(); - // --- connect the node that was just created + if (use_filtered) + this->set_node_inventory(full_inventory); - const std::string node_to = this->last_node_created_id; - BaseNode *p_node_to = gno->get_node_ref_by_id(node_to); - - if (!p_node_to) - { - Logger::log()->trace("GraphNodeWidget::on_connection_dropped: p_node_to is nullptr"); - return; - } + if (!created) + return; - const std::optional port_to = select_port(*p_node_to, - dragged_type, - wanted_dir); + const std::string node_to = this->last_node_created_id; + BaseNode *p_node_to = gno->get_node_ref_by_id(node_to); - if (!port_to) - { - Logger::log()->trace( - "GraphNodeWidget::on_connection_dropped: node '{}' has no {} port of type {}, " - "leaving it unconnected", - node_to, - wanted_dir == gngui::PortType::IN ? "input" : "output", - dragged_type); - return; - } - - // order the operands so that 'from' is always the OUTPUT side - const bool dragged_is_output = (dragged_dir == gngui::PortType::OUT); - - const std::string id_out = dragged_is_output ? node_id : node_to; - const std::string port_out = dragged_is_output ? port_id : *port_to; - const std::string id_in = dragged_is_output ? node_to : node_id; - const std::string port_in = dragged_is_output ? *port_to : port_id; - - // model first: only draw the GUI link if the model accepted it - try - { - gno->new_link(id_out, port_out, id_in, port_in); - } - catch (const std::exception &e) - { - Logger::log()->error("GraphNodeWidget::on_connection_dropped: link refused: {}", - e.what()); - return; - } + if (!p_node_to) + { + Logger::log()->trace( + "GraphNodeWidget::on_connection_dropped: p_node_to is nullptr"); + batch.commit(); + return; + } - this->add_link(id_out, port_out, id_in, port_in); - gno->update(node_to); -} + const std::optional port_to = select_port(*p_node_to, + dragged_type, + wanted_dir); -void GraphNodeWidget::on_connection_finished(const std::string &id_out, - const std::string &port_id_out, - const std::string &id_in, - const std::string &port_id_in) -{ - Logger::log()->trace("GraphNodeWidget::on_connection_finished, {}/{} -> {}/{}", - id_out, - port_id_out, - id_in, - port_id_in); + if (!port_to) + { + Logger::log()->trace("GraphNodeWidget::on_connection_dropped: node '{}' has no " + "{} port of type {}, " + "leaving it unconnected", + node_to, + wanted_dir == gngui::PortType::IN ? "input" : "output", + dragged_type); + batch.commit(); + return; + } - auto gno = this->p_graph_node.lock(); - if (!gno) - return; + const bool dragged_is_output = (dragged_dir == gngui::PortType::OUT); - gno->new_link(id_out, port_id_out, id_in, port_id_in); + const std::string id_out = dragged_is_output ? node_id : node_to; + const std::string port_out = dragged_is_output ? port_id : *port_to; + const std::string id_in = dragged_is_output ? node_to : node_id; + const std::string port_in = dragged_is_output ? *port_to : port_id; - this->update_graph_model(id_in); + this->request_connection({id_out, port_out, id_in, port_in}); + batch.commit(); + }); } void GraphNodeWidget::on_graph_clear_request() @@ -707,15 +593,13 @@ void GraphNodeWidget::on_graph_import_request() for (auto &json_node : json_mod["nodes"]) { - const std::string node_id = json_node["id"].get(); - gngui::GraphicsNode *p_gfx_node = this->get_graphics_node_by_id(node_id); - - // Qt mystery, this needs to be delayed to be effective + const std::string node_id = json_node["id"].get(); QTimer::singleShot(0, - [p_gfx_node]() + this, + [this, node_id]() { - if (p_gfx_node) - p_gfx_node->setSelected(true); + if (auto *node = this->get_graphics_node_by_id(node_id)) + node->setSelected(true); }); } @@ -757,18 +641,7 @@ void GraphNodeWidget::on_graph_settings_request() void GraphNodeWidget::on_new_graphics_node_request(const std::string &node_id, QPointF scene_pos) { - // GraphicsNodes cannot generated by the GraphViewer instance by - // itself, it is outsourced to the outer nodes manager (this - // class). This slot respond to a request for the creation of a - // GraphicsNodes (only). This is different from - // GraphNodeWidget::on_new_node_request which generates both the - // model and the GUI nodes... - - // This one is actually used for serialization, when the graph - // viewer requests the creation of a graphics node while the base - // node has aldready been created when the GraphNode has been - // deserialized - + // Also used when loading a scene for nodes already present in the model. Logger::log()->trace("GraphNodeWidget::on_new_graphics_node_request: {} {},{}", node_id, scene_pos.x(), @@ -779,7 +652,10 @@ void GraphNodeWidget::on_new_graphics_node_request(const std::string &node_id, return; BaseNode *p_node = gno->get_node_ref_by_id(node_id); - auto *p_proxy = new gngui::TypedNodeProxy(p_node->get_shared()); + if (!p_node) + throw std::runtime_error("Cannot display a missing model node."); + auto *p_proxy = new gngui::TypedNodeProxy(p_node->get_shared()); + p_proxy->setParent(this); auto *widget = node_widget_factory(p_node->get_caption(), p_node->get_shared(), this); this->add_node(p_proxy, scene_pos, node_id); @@ -789,297 +665,54 @@ void GraphNodeWidget::on_new_graphics_node_request(const std::string &node_id, std::string GraphNodeWidget::on_new_node_request(const std::string &node_type, QPointF scene_pos) { - Logger::log()->trace("GraphNodeWidget::on_new_node_request: node_type {}", node_type); - - auto gno = this->p_graph_node.lock(); - if (!gno) - return ""; - - if (node_type == "") - return ""; - - // add control node (compute) - std::string node_id = gno->add_node(node_type); - - // add corresponding graphics node (GUI) - this->on_new_graphics_node_request(node_id, scene_pos); - - Q_EMIT this->new_node_created(this->get_id(), node_id); - - this->last_node_created_id = node_id; - - return node_id; + const auto id = perform_graph_edit( + [&]() { return this->editor->add_node(node_type, scene_pos); }); + // A drag-to-create gesture needs the ID before its outer batch commits. + this->last_node_created_id = id; + return id; } std::string GraphNodeWidget::on_new_node_request_chain(const std::string &node_type) { - Logger::log()->trace("GraphNodeWidget::on_new_node_request_chain: node_type {}", - node_type); - - // --- Safeguards - - auto gno = this->p_graph_node.lock(); - if (!gno) - return ""; - - if (node_type == "") - return ""; - - // get current node selection - std::vector selected_ids = this->get_selected_node_ids(); - - // empty selection => skip - if (selected_ids.empty()) + const auto ids = this->get_selected_node_ids(); + if (ids.empty()) { HSD_APP->notify("Select a node before inserting a new node."); - return ""; + return {}; } - - // --- Backup selected node connections - - const std::string selected_id = selected_ids.back(); - - // position - gngui::GraphicsNode *p_gx_node = this->get_graphics_node_by_id(selected_id); - if (!p_gx_node) + auto *graphics = this->get_graphics_node_by_id(ids.back()); + if (!graphics) + return {}; + const auto position = graphics->pos() + + QPointF(HSD_CTX.app_settings.node_editor + .position_delta_when_duplicating_node, + 0.f); + const auto id = perform_graph_edit( + [&]() { return this->editor->insert_node(ids.back(), node_type, position); }); + if (!id.empty()) { - Logger::log()->error( - "GraphNodeWidget::on_new_node_request_replace: p_gx_node is nullptr"); - return ""; - } - QPointF node_pos = p_gx_node->pos(); - - float dx = HSD_CTX.app_settings.node_editor.position_delta_when_duplicating_node; - node_pos = node_pos + QPointF(dx, 0.f); - - // backup links - std::vector link_views = gno->get_link_views(selected_id); - - // --- BLOCK model updates - - this->set_block_graph_model_updates(true); - - // --- Delete downstream links of selected node - - for (const auto &data : link_views) - { - if (data.from == selected_id) - { - // graphics object first and then the model link - this->remove_link(data.from, data.port_from, data.to, data.port_to); - // gno->remove_link(data.from, data.port_from, data.to, data.port_to); - } - } - - // --- Create new node - - this->deselect_all(); - std::string new_id = this->on_new_node_request(node_type, node_pos); - this->set_node_as_selected(new_id); - - // --- Recreate the links if possible - - int link_creation_count = 0; - - for (const auto &data : link_views) - { - // change only the output links of the selected node - if (data.from != selected_id) - continue; - - gnode::Node *p_new_node = gno->get_node_ref_by_id(new_id); - - // reconnect selected_id (output) => new_id (input) - { - std::string from = data.from; - std::string to = new_id; - - // check the port exists - int port_id = p_new_node->get_port_index(data.port_label_to); - if (port_id < 0) - continue; - - this->add_link(from, data.port_label_from, to, data.port_label_to); - gno->new_link(from, data.port_label_from, to, data.port_label_to); - link_creation_count++; - } - - // connect new_id (output) => some downstream node (input) - { - std::string from = new_id; - std::string to = data.to; - - // check the port exists - int port_id = p_new_node->get_port_index(data.port_label_from); - if (port_id < 0) - continue; - - this->add_link(from, data.port_label_from, to, data.port_label_to); - gno->new_link(from, data.port_label_from, to, data.port_label_to); - link_creation_count++; - } - } - - // if no link has been created, try to connect the first output of - // 'selected_id' to the first input of 'new_id' - if (link_creation_count == 0) - { - BaseNode *p_bnode_from = gno->get_node_ref_by_id(selected_id); - BaseNode *p_bnode_to = gno->get_node_ref_by_id(new_id); - - if (p_bnode_from && p_bnode_to) - { - // 1st outlet - int kfrom = -1; - - for (int k = 0; k < p_bnode_from->get_nports(); ++k) - if (p_bnode_from->get_port_type(k) == gngui::PortType::OUT) - { - kfrom = k; - break; - } - - // 1st inlet - if (kfrom != -1) - { - std::string data_type = p_bnode_from->get_data_type(kfrom); - int kto = -1; - for (int k = 0; k < p_bnode_to->get_nports(); ++k) - if (p_bnode_to->get_port_type(k) == gngui::PortType::IN && - p_bnode_to->get_data_type(k) == data_type) - { - kto = k; - break; - } - - if (kto != -1) - { - std::string port_label_from = p_bnode_from->get_port_label(kfrom); - std::string port_label_to = p_bnode_to->get_port_label(kto); - - this->add_link(selected_id, port_label_from, new_id, port_label_to); - gno->new_link(selected_id, port_label_from, new_id, port_label_to); - } - } - } + this->deselect_all(); + this->set_node_as_selected(id); } - - // --- UNBLOCK model updates - - this->set_block_graph_model_updates(false); - - // --- Update and exit - - this->update_graph_model(selected_id); - return new_id; + return id; } std::string GraphNodeWidget::on_new_node_request_replace(const std::string &node_type) { - Logger::log()->trace("GraphNodeWidget::on_new_node_request_replace: node_type {}", - node_type); - - // --- Safeguards - - auto gno = this->p_graph_node.lock(); - if (!gno) - return ""; - - if (node_type == "") - return ""; - - // get current node selection - std::vector selected_ids = this->get_selected_node_ids(); - - // empty selection => skip - if (selected_ids.empty()) + const auto ids = this->get_selected_node_ids(); + if (ids.empty()) { HSD_APP->notify("Select a node before replacing it."); - return ""; - } - - // --- Backup selected node connections - - const std::string selected_id = selected_ids.back(); - - // position - gngui::GraphicsNode *p_gx_node = this->get_graphics_node_by_id(selected_id); - if (!p_gx_node) - { - Logger::log()->error( - "GraphNodeWidget::on_new_node_request_replace: p_gx_node is nullptr"); - return ""; + return {}; } - const QPointF node_pos = p_gx_node->pos(); - - // backup links - std::vector link_views = gno->get_link_views(selected_id); - - // --- BLOCK model updates - - this->set_block_graph_model_updates(true); - - // --- Remove selected node - - this->remove_node(selected_id); // graphics object first - this->on_node_deleted_request(selected_id); // then propagate - - // --- Create new node - - this->deselect_all(); - std::string new_id = this->on_new_node_request(node_type, node_pos); - this->set_node_as_selected(new_id); - - // --- Recreate the links if possible - - for (const auto &data : link_views) + const auto id = perform_graph_edit( + [&]() { return this->editor->replace_node(ids.back(), node_type); }); + if (!id.empty()) { - // replace former ID by new one - std::string from = (data.from == selected_id) ? new_id : data.from; - std::string to = (data.to == selected_id) ? new_id : data.to; - - // check if the port to be connected actually exists on the new - // node before continuing - std::string port_label = (from == new_id) ? data.port_label_from : data.port_label_to; - gnode::Node *p_new_node = gno->get_node_ref_by_id(new_id); - int port_id = p_new_node->get_port_index(port_label); - - if (port_id < 0) - continue; - - // add graphics link and then model link - this->add_link(from, data.port_label_from, to, data.port_label_to); - gno->new_link(from, data.port_label_from, to, data.port_label_to); + this->deselect_all(); + this->set_node_as_selected(id); } - - // --- UNBLOCK model updates - - this->set_block_graph_model_updates(false); - - // --- Update and exit - - this->update_graph_model(new_id); - return new_id; -} - -void GraphNodeWidget::on_node_deleted_request(const std::string &node_id) -{ - Logger::log()->trace("GraphNodeWidget::on_node_deleted_request, node {}", node_id); - - auto gno = this->p_graph_node.lock(); - if (!gno) - return; - - // block connection-related updates - this->set_block_graph_model_updates(true); - - this->set_enabled(false); - gno->remove_node(node_id); - this->set_enabled(true); - - this->set_block_graph_model_updates(false); - - Q_EMIT this->node_deleted(this->get_id(), node_id); + return id; } void GraphNodeWidget::on_node_info(const std::string &node_id) @@ -1325,6 +958,7 @@ void GraphNodeWidget::reselect_backup_ids() { QTimer::singleShot( 0, + this, [this]() { for (size_t k = 0; k < this->selected_ids.size(); ++k) @@ -1337,14 +971,6 @@ void GraphNodeWidget::reselect_backup_ids() }); } -void GraphNodeWidget::set_block_graph_model_updates(bool new_state) -{ - Logger::log()->trace("GraphNodeWidget::set_block_graph_model_updates: state is now {}", - new_state ? "BLOCKED" : "UNBLOCKED"); - - this->block_graph_model_updates = new_state; -} - void GraphNodeWidget::set_json_copy_buffer(nlohmann::json const &new_json_copy_buffer) { this->json_copy_buffer = new_json_copy_buffer; @@ -1396,21 +1022,11 @@ void GraphNodeWidget::setup_connections() [this]() { this->is_selecting_with_rubber_band = false; }); // GraphViewer -> GraphNodeWidget - this->connect(this, - &gngui::GraphViewer::connection_deleted, - this, - &GraphNodeWidget::on_connection_deleted); - this->connect(this, &gngui::GraphViewer::connection_dropped, this, &GraphNodeWidget::on_connection_dropped); - this->connect(this, - &gngui::GraphViewer::connection_finished, - this, - &GraphNodeWidget::on_connection_finished); - this->connect(this, &gngui::GraphViewer::new_graphics_node_request, this, @@ -1421,11 +1037,6 @@ void GraphNodeWidget::setup_connections() this, &GraphNodeWidget::on_new_node_request); - this->connect(this, - &gngui::GraphViewer::node_deleted, - this, - &GraphNodeWidget::on_node_deleted_request); - this->connect(this, &gngui::GraphViewer::node_reload_request, this, @@ -1513,27 +1124,7 @@ void GraphNodeWidget::setup_connections() void GraphNodeWidget::update_graph_model(const std::vector &node_ids) { - Logger::log()->trace("GraphNodeWidget::update_graph_model"); - - if (this->is_graph_model_updates_blocked()) - { - Logger::log()->trace("GraphNodeWidget::update_graph_model: graph model updates are " - "blocked, no update"); - return; - } - - auto gno = this->p_graph_node.lock(); - if (!gno) - { - Logger::log()->error( - "GraphNodeWidget::update_graph_model: graph node model ptr is nullptr"); - return; - } - - if (node_ids.empty()) - gno->update(); - else - gno->update(node_ids); + perform_graph_edit([&]() { this->editor->request_update(node_ids); }); } void GraphNodeWidget::update_graph_model(const std::string &node_id) diff --git a/Hesiod/src/gui/widgets/graph_tabs_widget.cpp b/Hesiod/src/gui/widgets/graph_tabs_widget.cpp index cd5f3958f..3c7511dcd 100644 --- a/Hesiod/src/gui/widgets/graph_tabs_widget.cpp +++ b/Hesiod/src/gui/widgets/graph_tabs_widget.cpp @@ -338,6 +338,10 @@ void GraphTabsWidget::update_tab_widget() // Connect signals auto *gnw = editor_widget->get_graph_node_widget(); + this->connect(gnw, + &GraphNodeWidget::graph_edited, + this, + &GraphTabsWidget::has_changed); this->connect(gnw, &GraphNodeWidget::has_been_cleared, this, diff --git a/Hesiod/src/model/graph/graph_node.cpp b/Hesiod/src/model/graph/graph_node.cpp index afc85a969..5b8eac1dc 100644 --- a/Hesiod/src/model/graph/graph_node.cpp +++ b/Hesiod/src/model/graph/graph_node.cpp @@ -51,7 +51,6 @@ std::string GraphNode::add_node(const std::string &node_type) Logger::log()->trace("GraphNode::add_node: node_type = {}", node_type); std::shared_ptr node = node_factory(node_type, this->config); - node->compute(); std::string node_id = this->add_node(node); diff --git a/docs/graph-editor-refactor.md b/docs/graph-editor-refactor.md new file mode 100644 index 000000000..14e4bdd6e --- /dev/null +++ b/docs/graph-editor-refactor.md @@ -0,0 +1,110 @@ +# Graph editor extraction — issue #751 + +The target is one owner for interactive graph edits: `GraphEditor`. `GraphNode` +keeps graph state and domain operations; `GraphNodeWidget` keeps Qt interaction, +the `GraphViewer` scene, selection, dialogs and viewers. `GraphEditorWidget` +remains the existing panel container. + +The implementation is split so each increment has a small behavioral contract +and can be reviewed and merged independently. GNodeGUI changes should land in +its repository first, followed by the corresponding Hesiod dependency update. + +## 1. GNodeGUI edit requests (merged) + +Connection drags, Delete and Ctrl + right-click enter overridable request methods +before modifying established graph items. The requests contain stable node/port +identifiers, not graphics pointers. A selection deletion arrives as one request. +An editor can reject a replacement without losing the existing input link. + +The new `erase_node` and `erase_link` methods only synchronize the scene, without +emitting the legacy model-edit notifications. Selection notifications remain +active. Existing users retain the default request implementations and legacy +signals. No global signal suppression or update-blocking flag is introduced. + +Offscreen Qt regression tests exercise accepted/rejected connections, replacement, +reverse drags, invalid gestures, batch deletion, Ctrl + right-click, repeated +erasures and legacy notification counts. They use real graphics nodes and the +gesture callbacks installed by GraphViewer, plus keyboard/mouse events for +deletion. They do not yet test Hesiod model computation. + +GNodeGUI PR #12 is merged. It supplies the dependency for the Hesiod extraction +below; by itself it retains the legacy behavior for existing callers. + +## 2. Extract GraphEditor and move topology edits (this change) + +GraphNodeWidget now owns one GraphEditor and delegates node creation/deletion, +connection/disconnection, replacement, chain insertion, paste/duplicate, import +and clear. The GNodeGUI request overrides call the editor; the old after-edit +mutation handlers and the widget's update-blocking flag are removed. The widget +supplies node presentation and completion callbacks, so it still owns dialogs, +selection, graphics widgets and viewers. + +Explicitly committed, nesting-safe batches collect affected nodes and publish +completion notifications after model and scene agree. They compute once at the +outermost successful commit; their destructors restore scheduling state and never +compute during exception unwinding. GraphNode's node factory now constructs nodes +without computing them, letting initialization and connections precede the first +update. Computation failure leaves the accepted topology intact and scheduling +available for a retry. + +Connections validate node/port existence, direction, type and cycles before +changing an occupied input. Duplicate links are no-ops. Failed presentation, +replacement reconnection or paste rolls back the affected topology. Replacement +keeps the original node until the new one can be displayed and reconnected; +incompatible ports are omitted and their former downstream nodes are recomputed. +Chain insertion preserves branches it cannot reconnect. Deletion removes graphics +while proxies still reference live nodes, then uses GraphNode's deletion API to +preserve Broadcast/Receive cleanup. Clear removes the model nodes as well as the +scene. Link changes now mark the project dirty through a graph-edit notification. + +These batches are not general-purpose transactions: callers of the low-level batch +API must undo their own mutations when abandoning a batch. Cross-graph broadcasts +and existing direct settings/configuration update paths remain outside the editor's +batch boundary until step 4. + +The Qt integration suite uses the real GraphNode, node factory and graphics scene. +It checks graph/scene node and link sets, notification and computation counts, +rejected edits, nested failures, replacement rollback, fanout insertion, pasted ID +remapping, malformed paste rollback, Broadcast cleanup, presentation-only loading, +widget gestures/duplication and drag-to-create in both directions. A context-only +application startup avoids OpenCL, service and main-window initialization in CPU +integration tests. The application and tests share an object library to avoid +compiling the application implementation twice. + +Run from the repository root, with the full node set and Qt Test available: + +```sh +cmake -S . -B build -DHESIOD_ENABLE_TESTS=ON +cmake --build build --target hesiod test_graph_editor +ctest --test-dir build -R '^graph_editor$' --output-on-failure +``` + +## 3. Move the Hesiod node proxy into the GUI layer + +Introduce an explicit Hesiod adapter implementing GNodeGUI's NodeProxy interface. +Move GUI port conversion and presentation responsibilities out of BaseNode. Use +GNode's port direction type in model APIs and convert it at the adapter boundary. +Give the proxy a clear owner and make identifier/lifetime behavior explicit. + +BaseNode currently includes `gnodegui/node_proxy.hpp`, which includes Qt. Thus +GraphNode's header has no direct Qt include, but the model implementation is not +yet Qt-independent. Verify removal with a model-header compilation check without +Qt include paths, as well as the normal application build. Avoid changing saved +node identifiers, port IDs or captions as an incidental effect of this move. + +## 4. Loading, settings and remaining update paths + +Separate loading an already-built model into the view from interactive import +and paste. Rebuild graphics connections from the accepted model and preserve +layout/viewer state from the project file. Keep headless loading independent of +widgets and retain legacy port migration behavior. + +Route GUI settings, configuration and reload requests through the editor's update +policy. Inventory other direct calls, including NodeAttributesWidget and special +node widgets, to complete the editor update policy. The widget blocker was removed +in step 2. Domain-driven broadcasting and headless execution remain valid model callers; the editor is the single entry +point for interactive edits, not a compulsory Qt dependency for all computation. + +Finish with project round-trip, paste/import, legacy project, selection/viewer +lifetime and Broadcast/Receive regression checks. The existing editor widget API +can remain as forwarding methods during migration, then shrink once callers move. diff --git a/external/GNodeGUI b/external/GNodeGUI index 46538e095..ba56329fd 160000 --- a/external/GNodeGUI +++ b/external/GNodeGUI @@ -1 +1 @@ -Subproject commit 46538e0955909e732dd7fcc8c11a4d0e07d75db3 +Subproject commit ba56329fd5c611762f8bb629b74d4cd8d69d8586 diff --git a/tests/gui/test_graph_editor.cpp b/tests/gui/test_graph_editor.cpp new file mode 100644 index 000000000..78f01d37d --- /dev/null +++ b/tests/gui/test_graph_editor.cpp @@ -0,0 +1,614 @@ +/* Copyright (c) 2026 Otto Link. Distributed under the terms of the GNU General + * Public License. The full license is in the file LICENSE, distributed with + * this software. */ +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "gnodegui/style.hpp" +#include "hesiod/app/hesiod_application.hpp" +#include "hesiod/gui/graph_editor.hpp" +#include "hesiod/gui/widgets/graph_node_widget.hpp" +#include "hesiod/model/graph/graph_manager.hpp" +#include "hesiod/model/graph/graph_node.hpp" +#include "hesiod/model/nodes/base_node.hpp" + +using namespace hesiod; + +namespace +{ +std::shared_ptr small_config() +{ + auto config = std::make_shared(); + config->set_shape({8, 8}); + config->set_tiling({1, 1}); + config->set_overlap(0.f); + config->storage_mode = hmap::StorageMode::VA_RAM; + return config; +} + +bool consistent(GraphNode &graph, gngui::GraphViewer &view) +{ + std::set model_ids, view_ids; + using Edge = std::tuple; + std::set model_links, view_links; + for (const auto &[id, node] : graph.get_nodes()) + model_ids.insert(id); + const auto json = view.json_to(); + for (const auto &node : json.at("nodes")) + view_ids.insert(node.at("id").get()); + for (const auto &link : graph.get_links()) + model_links.emplace(link.from, + graph.get_node(link.from)->get_port_label(link.port_from), + link.to, + graph.get_node(link.to)->get_port_label(link.port_to)); + for (const auto &link : json.at("links")) + view_links.emplace(link.at("node_out_id").get(), + link.at("port_out_id").get(), + link.at("node_in_id").get(), + link.at("port_in_id").get()); + return model_ids == view_ids && model_links == view_links && + view_ids.size() == json.at("nodes").size() && + view_links.size() == json.at("links").size(); +} + +class UnavailableOutputProxy : public gngui::TypedNodeProxy +{ +public: + using gngui::TypedNodeProxy::TypedNodeProxy; + std::string get_port_id(int index) const override + { + return index == 1 ? "unavailable" : TypedNodeProxy::get_port_id(index); + } +}; + +struct Fixture +{ + std::shared_ptr config = small_config(); + std::shared_ptr graph = std::make_shared("test", config); + gngui::GraphViewer view; + int updates = 0; + int edits = 0; + std::map computed; + std::vector created, deleted; + bool notifications_consistent = true; + bool fail_presentation = false; + bool fail_reconnection = false; + GraphEditor editor; + + Fixture() : editor(graph, view, callbacks()) + { + view.scene()->setParent(&view); + graph->update_started = [this]() { ++updates; }; + graph->compute_finished = [this](const std::string &id) { ++computed[id]; }; + } + + GraphEditor::NodePresentation callbacks() + { + GraphEditor::NodePresentation result; + result.create = [this](const std::string &id, QPointF position) + { + auto node = graph->get_node_ref_by_id(id)->get_shared(); + gngui::NodeProxy *proxy = fail_reconnection + ? static_cast( + new UnavailableOutputProxy(node)) + : new gngui::TypedNodeProxy(node); + proxy->setParent(&view); + view.add_node(proxy, position, id); + if (fail_presentation) + throw std::runtime_error("Injected presentation failure"); + }; + result.created = [this](const std::string &id) + { + created.push_back(id); + notifications_consistent &= consistent(*graph, view); + }; + result.deleted = [this](const std::string &id) + { + deleted.push_back(id); + notifications_consistent &= consistent(*graph, view); + }; + result.changed = [this]() { ++edits; }; + return result; + } + + ~Fixture() + { + // Destroy links before their node proxies/model references become invalid. + for (const auto &[id, node] : graph->get_nodes()) + view.erase_node(id); + } + + void reset_counts() + { + updates = 0; + edits = 0; + computed.clear(); + created.clear(); + deleted.clear(); + } + + std::string add(const std::string &type = "Thru") + { + return editor.add_node(type, QPointF(100 * graph->get_nodes().size(), 50)); + } + + nlohmann::json copy() const + { + auto json = view.json_to(); + for (auto &node : json["nodes"]) + node["settings"] = graph->get_node_ref_by_id(node["id"])->json_to(); + return json; + } +}; +} // namespace + +class GraphEditorTest : public QObject +{ + Q_OBJECT +private Q_SLOTS: + void nested_batches_compute_only_after_commit() + { + Fixture f; + GraphEditor::Batch outer(f.editor); + const auto source = f.add("Constant"); + const auto sink = f.add(); + f.editor.connect({source, "output", sink, "input"}); + QCOMPARE(f.updates, 0); + QVERIFY(f.computed.empty()); + QVERIFY(f.created.empty()); + outer.commit(); + QCOMPARE(f.updates, 1); + QCOMPARE(f.computed[source], 1); + QCOMPARE(f.computed[sink], 1); + QCOMPARE(f.created.size(), size_t(2)); + QCOMPARE(f.edits, 1); + QVERIFY(f.notifications_consistent); + QVERIFY(consistent(*f.graph, f.view)); + } + + void initialized_parameters_are_used_by_the_first_compute() + { + Fixture f; + const auto id = f.editor.add_node("Constant", + {}, + [](BaseNode &node) + { node.set_value("value", 0.75f); }); + auto *value = f.graph->get_node(id)->get_value_ref("output"); + const auto array = value->to_array(f.config->cm_cpu); + QCOMPARE(array(0, 0), 0.75f); + QCOMPARE(f.computed[id], 1); + } + + void rejected_connection_preserves_input_data_and_scene_data() + { + QTest::addColumn("reason"); + QTest::newRow("missing-port") << 0; + QTest::newRow("wrong-direction") << 1; + QTest::newRow("wrong-type") << 2; + QTest::newRow("cycle") << 3; + QTest::newRow("missing-node") << 4; + } + + void rejected_connection_preserves_input_data_and_scene() + { + QFETCH(int, reason); + Fixture f; + const auto a = f.add(); + const auto b = f.add(); + const auto c = f.add(); + f.editor.connect({a, "output", b, "input"}); + f.editor.connect({b, "output", c, "input"}); + // Add a deliberately different data type before constructing its graphics. + const auto wrong = f.editor.add_node( + "Thru", + {}, + [](BaseNode &node) { node.add_port(gnode::PortType::OUT, "float"); }); + auto *input = f.graph->get_node(b)->get_value_ref("input"); + const auto before = f.view.json_to(); + f.reset_counts(); + GraphEditor::Link link{c, "output", b, "input"}; + if (reason == 0) + link.port_out = "missing"; + if (reason == 1) + link.port_out = "input"; + if (reason == 2) + { + link.node_out = wrong; + link.port_out = "float"; + } + if (reason == 4) + link.node_out = "missing"; + QVERIFY_EXCEPTION_THROWN(f.editor.connect(link), std::invalid_argument); + QVERIFY(f.view.json_to() == before); + QCOMPARE(f.edits, 0); + QCOMPARE(f.graph->get_node(b)->get_value_ref("input"), input); + QCOMPARE(f.updates, 0); + QVERIFY(consistent(*f.graph, f.view)); + } + + void connection_replacement_and_duplicate() + { + Fixture f; + const auto a = f.add(), b = f.add(), c = f.add(); + f.editor.connect({a, "output", b, "input"}); + f.reset_counts(); + QVERIFY(f.editor.connect({c, "output", b, "input"})); + QCOMPARE(f.updates, 1); + QCOMPARE(f.graph->get_links().size(), size_t(1)); + QCOMPARE(f.graph->get_node(b)->get_value_ref("input"), + f.graph->get_node(c)->get_value_ref("output")); + QVERIFY(!f.editor.connect({c, "output", b, "input"})); + QCOMPARE(f.updates, 1); + QCOMPARE(f.edits, 1); + QVERIFY(consistent(*f.graph, f.view)); + } + + void deletion_batches_shared_links_and_recomputes_survivors() + { + Fixture f; + const auto a = f.add(), b = f.add(), c = f.add(); + f.editor.connect({a, "output", b, "input"}); + f.editor.connect({b, "output", c, "input"}); + f.reset_counts(); + QSignalSpy legacy_nodes(&f.view, &gngui::GraphViewer::node_deleted); + QSignalSpy legacy_links(&f.view, &gngui::GraphViewer::connection_deleted); + f.editor.erase({a, b, a}, {{a, "output", b, "input"}}); + QCOMPARE(f.updates, 1); + QCOMPARE(f.deleted.size(), size_t(2)); + QCOMPARE(f.computed.size(), size_t(1)); + QCOMPARE(f.computed[c], 1); + QVERIFY(!f.graph->get_node(c)->get_value_ref("input")); + QCOMPARE(legacy_nodes.count(), 0); + QCOMPARE(legacy_links.count(), 0); + QVERIFY(f.notifications_consistent); + QVERIFY(consistent(*f.graph, f.view)); + } + + void replacement_reconnects_once_and_preserves_position() + { + Fixture f; + const auto a = f.add(), b = f.add(), c = f.add(); + f.editor.connect({a, "output", b, "input"}); + f.editor.connect({b, "output", c, "input"}); + const auto position = f.view.get_graphics_node_by_id(b)->pos(); + f.reset_counts(); + const auto replacement = f.editor.replace_node(b, "Thru"); + QVERIFY(!f.graph->get_node(b)); + QCOMPARE(f.created, std::vector{replacement}); + QCOMPARE(f.deleted, std::vector{b}); + QCOMPARE(f.updates, 1); + QCOMPARE(f.computed[replacement], 1); + QCOMPARE(f.computed[c], 1); + QCOMPARE(f.view.get_graphics_node_by_id(replacement)->pos(), position); + QCOMPARE(f.graph->get_links().size(), size_t(2)); + QVERIFY(f.notifications_consistent); + QVERIFY(consistent(*f.graph, f.view)); + } + + void replacement_disconnects_incompatible_ports_and_updates_old_downstream() + { + Fixture f; + const auto a = f.add(), b = f.add(); + f.editor.connect({a, "output", b, "input"}); + f.reset_counts(); + const auto replacement = f.editor.replace_node(a, "Debug"); + QCOMPARE(f.updates, 1); + QCOMPARE(f.computed[b], 1); + QVERIFY(!f.graph->get_node(b)->get_value_ref("input")); + QVERIFY(f.graph->get_node(replacement)); + QVERIFY(consistent(*f.graph, f.view)); + } + + void failed_presentation_preserves_original_and_does_not_poison_outer_batch() + { + Fixture f; + const auto a = f.add(), b = f.add(); + f.editor.connect({a, "output", b, "input"}); + const auto before = f.view.json_to(); + f.reset_counts(); + GraphEditor::Batch outer(f.editor); + f.editor.request_update({a}); + f.fail_presentation = true; + QVERIFY_EXCEPTION_THROWN(f.editor.replace_node(a, "Thru"), std::runtime_error); + f.fail_presentation = false; + QVERIFY(f.view.json_to() == before); + QCOMPARE(f.updates, 0); + QVERIFY(f.created.empty()); + QVERIFY(f.deleted.empty()); + outer.commit(); + QCOMPARE(f.updates, 1); + QCOMPARE(f.computed[a], 1); + QCOMPARE(f.computed[b], 1); + QVERIFY(consistent(*f.graph, f.view)); + } + + void failed_reconnection_rolls_back_the_replacement() + { + Fixture f; + const auto a = f.add(), b = f.add(), c = f.add(); + f.editor.connect({a, "output", b, "input"}); + f.editor.connect({b, "output", c, "input"}); + const auto before = f.view.json_to(); + f.reset_counts(); + f.fail_reconnection = true; + QVERIFY_EXCEPTION_THROWN(f.editor.replace_node(b, "Thru"), std::runtime_error); + f.fail_reconnection = false; + QVERIFY(f.view.json_to() == before); + QVERIFY(consistent(*f.graph, f.view)); + QCOMPARE(f.updates, 0); + QVERIFY(f.created.empty() && f.deleted.empty()); + QCOMPARE(f.edits, 0); + QVERIFY(!f.editor.replace_node(b, "Thru").empty()); + QCOMPARE(f.updates, 1); + } + + void insertion_preserves_fanout() + { + Fixture f; + const auto a = f.add(), b = f.add(), c = f.add(); + f.editor.connect({a, "output", b, "input"}); + f.editor.connect({a, "output", c, "input"}); + f.reset_counts(); + const auto inserted = f.editor.insert_node(a, "Thru", {12, 34}); + QCOMPARE(f.graph->get_links().size(), size_t(3)); + QCOMPARE(f.updates, 1); + QCOMPARE(f.computed[inserted], 1); + QCOMPARE(f.computed[b], 1); + QCOMPARE(f.computed[c], 1); + QCOMPARE(f.graph->get_node(b)->get_value_ref("input"), + f.graph->get_node(inserted)->get_value_ref("output")); + QVERIFY(consistent(*f.graph, f.view)); + } + + void insertion_keeps_branches_it_cannot_reroute() + { + Fixture f; + const auto a = f.add(), b = f.add(); + f.editor.connect({a, "output", b, "input"}); + f.reset_counts(); + const auto inserted = f.editor.insert_node(a, "Debug", {}); + QCOMPARE(f.graph->get_links().size(), size_t(2)); + QCOMPARE(f.graph->get_node(b)->get_value_ref("input"), + f.graph->get_node(a)->get_value_ref("output")); + QVERIFY(f.graph->get_node(inserted)->get_value_ref("input")); + QCOMPARE(f.updates, 1); + QVERIFY(consistent(*f.graph, f.view)); + } + + void paste_remaps_ids_and_computes_copies_only() + { + Fixture f; + const auto a = f.add(), b = f.add(); + f.editor.connect({a, "output", b, "input"}); + auto input = f.copy(); + input["links"][0]["port_out_id"] = "out"; + input["links"][0]["port_in_id"] = "in"; + f.reset_counts(); + const auto pasted = f.editor.import_nodes(input, {20, 40}); + QCOMPARE(f.graph->get_nodes().size(), size_t(4)); + QCOMPARE(f.graph->get_links().size(), size_t(2)); + QCOMPARE(f.updates, 1); + QCOMPARE(f.computed.size(), size_t(2)); + QVERIFY(!f.computed.contains(a)); + QVERIFY(!f.computed.contains(b)); + const auto from = pasted["links"][0]["node_out_id"].get(); + const auto to = pasted["links"][0]["node_in_id"].get(); + QVERIFY(from != a && to != b); + QVERIFY(f.computed.contains(from) && f.computed.contains(to)); + QCOMPARE(pasted["links"][0]["port_out_id"].get(), std::string("output")); + QCOMPARE(pasted["links"][0]["port_in_id"].get(), std::string("input")); + QVERIFY(f.notifications_consistent); + QVERIFY(consistent(*f.graph, f.view)); + } + + void invalid_paste_rolls_back_created_nodes() + { + Fixture f; + const auto a = f.add(), b = f.add(); + f.editor.connect({a, "output", b, "input"}); + auto input = f.copy(); + input["links"][0]["port_in_id"] = "nonexistent"; + const auto before = f.view.json_to(); + f.reset_counts(); + QVERIFY_EXCEPTION_THROWN(f.editor.import_nodes(input, {}), std::invalid_argument); + QCOMPARE(f.graph->get_nodes().size(), size_t(2)); + QVERIFY(f.view.json_to() == before); + QCOMPARE(f.updates, 0); + QVERIFY(f.created.empty() && f.deleted.empty()); + QCOMPARE(f.edits, 0); + f.editor.request_update({a}); + QCOMPARE(f.updates, 1); + QVERIFY(consistent(*f.graph, f.view)); + } + + void clear_removes_model_nodes_and_broadcast_registration() + { + auto manager = std::make_shared("graphs"); + Fixture f; + manager->add_graph_node(f.graph, "test"); + f.add("Broadcast"); + f.add("Receive"); + QCOMPARE(manager->get_broadcast_params().size(), size_t(1)); + f.reset_counts(); + f.editor.clear(); + QVERIFY(f.graph->get_nodes().empty()); + QVERIFY(manager->get_broadcast_params().empty()); + QCOMPARE(f.deleted.size(), size_t(2)); + QCOMPARE(f.updates, 0); + QVERIFY(f.notifications_consistent); + QVERIFY(consistent(*f.graph, f.view)); + } + + void computation_failure_does_not_leave_edits_blocked() + { + Fixture f; + const auto id = f.add(); + f.graph->update_started = []() + { throw std::runtime_error("Injected update failure"); }; + QVERIFY_EXCEPTION_THROWN(f.editor.request_update({id}), std::runtime_error); + f.graph->update_started = [&]() { ++f.updates; }; + f.reset_counts(); + f.editor.request_update({id}); + QCOMPARE(f.updates, 1); + QCOMPARE(f.computed[id], 1); + } + + void loading_an_existing_model_is_presentation_only_data() + { + QTest::addColumn("output_port"); + QTest::newRow("current-port") << QString("output"); + QTest::newRow("legacy-port") << QString("out"); + } + + void loading_an_existing_model_is_presentation_only() + { + QFETCH(QString, output_port); + Fixture f; + const auto a = f.add(), b = f.add(); + f.editor.connect({a, "output", b, "input"}); + auto saved = f.view.json_to(); + saved["links"][0]["port_out_id"] = output_port.toStdString(); + auto model = f.graph->json_to(); + model["links"][0]["port_id_from"] = output_port.toStdString(); + auto loaded = std::make_shared("loaded", f.config); + loaded->json_from(model); + GraphNodeWidget widget(loaded); + widget.scene()->setParent(&widget); + QSignalSpy edits(&widget, &GraphNodeWidget::graph_edited); + QSignalSpy updates(&widget, &GraphNodeWidget::update_started); + widget.json_from(saved); + QCOMPARE(edits.count(), 0); + QCOMPARE(updates.count(), 0); + QCOMPARE(loaded->get_links().size(), size_t(1)); + QVERIFY(consistent(*loaded, widget)); + widget.erase_node(a); + widget.erase_node(b); + } + + void drag_to_create_is_one_edit_data() + { + QTest::addColumn("port"); + QTest::newRow("from-output") << QString("output"); + QTest::newRow("from-input") << QString("input"); + } + + void drag_to_create_is_one_edit() + { + QFETCH(QString, port); + auto graph = std::make_shared("drag", small_config()); + GraphNodeWidget widget(graph); + widget.scene()->setParent(&widget); + const auto original = widget.on_new_node_request("Thru", {}); + QSignalSpy updates(&widget, &GraphNodeWidget::update_started); + QSignalSpy edits(&widget, &GraphNodeWidget::graph_edited); + QTimer timeout; + timeout.setSingleShot(true); + QObject::connect( + &timeout, + &QTimer::timeout, + [&]() + { + if (auto *menu = qobject_cast(QApplication::activePopupWidget())) + menu->close(); + }); + timeout.start(2000); + QTimer::singleShot(0, + &widget, + [&]() + { + auto *menu = qobject_cast( + QApplication::activePopupWidget()); + QVERIFY(menu); + auto *filter = menu->findChild(); + QVERIFY(filter); + QTest::keyClicks(filter, "Thru"); + for (auto *action : menu->actions()) + if (action->text() == "Thru") + { + menu->setActiveAction(action); + QTest::keyClick(menu, Qt::Key_Return); + return; + } + QFAIL("The node menu did not offer Thru"); + }); + widget.on_connection_dropped(original, port.toStdString(), {}); + timeout.stop(); + QCOMPARE(graph->get_nodes().size(), size_t(2)); + QCOMPARE(graph->get_links().size(), size_t(1)); + QCOMPARE(updates.count(), 1); + QCOMPARE(edits.count(), 1); + QVERIFY(consistent(*graph, widget)); + widget.clear_all(); + QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); + } + + void widget_gestures_and_duplicate_use_the_editor() + { + auto graph = std::make_shared("widget", small_config()); + GraphNodeWidget widget(graph); + widget.scene()->setParent(&widget); + int updates = 0; + QObject::connect(&widget, &GraphNodeWidget::update_started, [&]() { ++updates; }); + QSignalSpy deleted(&widget, &GraphNodeWidget::node_deleted); + QSignalSpy edits(&widget, &GraphNodeWidget::graph_edited); + const auto a = widget.on_new_node_request("Thru", {0, 0}); + const auto b = widget.on_new_node_request("Thru", {400, 0}); + auto *from = widget.get_graphics_node_by_id(a); + auto *to = widget.get_graphics_node_by_id(b); + updates = 0; + edits.clear(); + from->connection_started(from, 1); + from->connection_finished(from, 1, to, 0); + QCOMPARE(updates, 1); + QCOMPARE(edits.count(), 1); + // The widget catches a rejected cyclic drag without changing either side. + to->connection_started(to, 1); + to->connection_finished(to, 1, from, 0); + QCOMPARE(updates, 1); + QCOMPARE(edits.count(), 1); + QVERIFY(consistent(*graph, widget)); + widget.deselect_all(); + widget.set_node_as_selected(a); + updates = 0; + const auto replacement = widget.on_new_node_request_replace("Thru"); + QVERIFY(!replacement.empty()); + QCOMPARE(deleted.count(), 1); + QCOMPARE(updates, 1); + QVERIFY(consistent(*graph, widget)); + updates = 0; + widget.on_nodes_duplicate_request({replacement, b}, + {widget.get_graphics_node_by_id(replacement)->pos(), + widget.get_graphics_node_by_id(b)->pos()}); + QCOMPARE(updates, 1); + QCOMPARE(graph->get_nodes().size(), size_t(4)); + QVERIFY(consistent(*graph, widget)); + widget.clear_all(); + QVERIFY(graph->get_nodes().empty()); + QVERIFY(consistent(*graph, widget)); + QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); + } +}; + +int main(int argc, char **argv) +{ + // Use the real application context and node factory, but skip main-window and + // OpenGL viewer creation. Isolate per-user configuration for the test process. + QTemporaryDir config_dir; + qputenv("XDG_CONFIG_HOME", config_dir.path().toUtf8()); + qputenv("XDG_CACHE_HOME", config_dir.path().toUtf8()); + qputenv("QT_LOGGING_RULES", HESIOD_QPUTENV_QT_LOGGING_RULES); + HesiodApplication app(argc, argv, HesiodApplication::StartupMode::ContextOnly); + app.get_context().app_settings.interface.enable_node_settings_in_node_body = false; + GN_STYLE->viewer.add_toolbar = false; + GraphEditorTest tests; + return QTest::qExec(&tests, argc, argv); +} + +#include "test_graph_editor.moc" From 3a649cbb3f5db72774205a9e81a714dcd539ab1a Mon Sep 17 00:00:00 2001 From: barrulus Date: Fri, 18 Sep 2026 21:06:50 +0100 Subject: [PATCH 2/3] refactor(gui): move the Hesiod node proxy out of the model --- Hesiod/CMakeLists.txt | 7 ++ .../include/hesiod/gui/hesiod_node_proxy.hpp | 32 +++++++ .../include/hesiod/model/nodes/base_node.hpp | 13 +-- .../hesiod/model/nodes/port_catalog.hpp | 8 +- Hesiod/src/app/hesiod_application.cpp | 2 +- Hesiod/src/cli/check_port_links.cpp | 38 ++++---- Hesiod/src/gui/hesiod_node_proxy.cpp | 94 +++++++++++++++++++ Hesiod/src/gui/widgets/data_preview.cpp | 10 +- Hesiod/src/gui/widgets/graph_node_widget.cpp | 18 ++-- Hesiod/src/gui/widgets/node_info_dialog.cpp | 8 +- .../src/gui/widgets/node_settings_widget.cpp | 2 +- Hesiod/src/gui/widgets/viewers/viewer.cpp | 6 +- .../widgets/viewers/wild_guess_view_param.cpp | 4 +- Hesiod/src/model/nodes/base_node.cpp | 21 +++-- Hesiod/src/model/nodes/base_node_proxy.cpp | 47 ---------- Hesiod/src/model/nodes/port_catalog.cpp | 12 +-- docs/graph-editor-refactor.md | 28 +++--- tests/gui/test_graph_editor.cpp | 80 ++++++++++++++-- tests/model/model_headers.cpp | 20 ++++ 19 files changed, 313 insertions(+), 137 deletions(-) create mode 100644 Hesiod/include/hesiod/gui/hesiod_node_proxy.hpp create mode 100644 Hesiod/src/gui/hesiod_node_proxy.cpp delete mode 100644 Hesiod/src/model/nodes/base_node_proxy.cpp create mode 100644 tests/model/model_headers.cpp diff --git a/Hesiod/CMakeLists.txt b/Hesiod/CMakeLists.txt index d23742439..ad34ef935 100644 --- a/Hesiod/CMakeLists.txt +++ b/Hesiod/CMakeLists.txt @@ -134,7 +134,14 @@ message(STATUS "Build type: ${CMAKE_BUILD_TYPE}") if(HESIOD_ENABLE_TESTS) find_package(Qt6 REQUIRED COMPONENTS Test) + add_library(test_model_headers OBJECT ${CMAKE_SOURCE_DIR}/tests/model/model_headers.cpp) + set_target_properties(test_model_headers PROPERTIES AUTOMOC OFF) + target_include_directories(test_model_headers PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include) + target_link_libraries(test_model_headers PRIVATE gnode highmap meta) + target_compile_features(test_model_headers PRIVATE cxx_std_20) + add_executable(test_graph_editor ${CMAKE_SOURCE_DIR}/tests/gui/test_graph_editor.cpp) + add_dependencies(test_graph_editor test_model_headers) target_link_libraries(test_graph_editor PRIVATE hesiod_core Qt6::Test) add_test(NAME graph_editor COMMAND test_graph_editor) set_tests_properties(graph_editor PROPERTIES diff --git a/Hesiod/include/hesiod/gui/hesiod_node_proxy.hpp b/Hesiod/include/hesiod/gui/hesiod_node_proxy.hpp new file mode 100644 index 000000000..63a733ebc --- /dev/null +++ b/Hesiod/include/hesiod/gui/hesiod_node_proxy.hpp @@ -0,0 +1,32 @@ +/* Copyright (c) 2026 Otto Link. Distributed under the terms of the GNU General + * Public License. The full license is in the file LICENSE, distributed with + * this software. */ +#pragma once +#include "gnodegui/node_proxy.hpp" + +namespace hesiod +{ +class BaseNode; + +// The view owns the proxy; the graph owns the model. +class HesiodNodeProxy : public gngui::NodeProxy +{ +public: + HesiodNodeProxy(std::weak_ptr model, QObject *owner); + + std::string get_id() const override; + void set_id(const std::string &id) override; + std::string get_caption() const override; + std::string get_category() const override; + std::string get_comment() const override; + std::string get_tool_tip_text() const override; + int get_nports() const override; + std::string get_port_caption(int index) const override; + gngui::PortType get_port_type(int index) const override; + std::string get_data_type(int index) const override; + void *get_data_ref(int index) const override; + +private: + std::weak_ptr model; +}; +} // namespace hesiod diff --git a/Hesiod/include/hesiod/model/nodes/base_node.hpp b/Hesiod/include/hesiod/model/nodes/base_node.hpp index 73b4ace4e..b6bdc7a4a 100644 --- a/Hesiod/include/hesiod/model/nodes/base_node.hpp +++ b/Hesiod/include/hesiod/model/nodes/base_node.hpp @@ -11,7 +11,6 @@ #include #include "gnode/node.hpp" -#include "gnodegui/node_proxy.hpp" #include "meta/core/container_group.hpp" @@ -57,6 +56,7 @@ class BaseNode : public gnode::Node, public std::enable_shared_from_this decltype(auto) val(const std::string &key) const diff --git a/Hesiod/include/hesiod/model/nodes/port_catalog.hpp b/Hesiod/include/hesiod/model/nodes/port_catalog.hpp index a51b70790..506168753 100644 --- a/Hesiod/include/hesiod/model/nodes/port_catalog.hpp +++ b/Hesiod/include/hesiod/model/nodes/port_catalog.hpp @@ -7,7 +7,7 @@ #include #include -#include "gnodegui/node_proxy.hpp" // gngui::PortType +#include "gnode/port.hpp" namespace hesiod { @@ -19,7 +19,7 @@ struct PortInfo { std::string name; std::string data_type; - gngui::PortType direction; + gnode::PortType direction; }; /** @@ -45,7 +45,7 @@ class PortCatalog */ bool is_offerable(const std::string &node_type, const std::string &data_type, - gngui::PortType wanted_direction) const; + gnode::PortType wanted_direction) const; /// Ports of a node type, or nullptr when the type is unknown. const std::vector *find(const std::string &node_type) const; @@ -65,6 +65,6 @@ class PortCatalog */ std::optional select_port(const BaseNode &node, const std::string &data_type, - gngui::PortType wanted_direction); + gnode::PortType wanted_direction); } // namespace hesiod diff --git a/Hesiod/src/app/hesiod_application.cpp b/Hesiod/src/app/hesiod_application.cpp index 2d39fcc55..461e291ae 100644 --- a/Hesiod/src/app/hesiod_application.cpp +++ b/Hesiod/src/app/hesiod_application.cpp @@ -565,7 +565,7 @@ void HesiodApplication::on_export_batch() BaseNode *p_base = p_graph->get_node_ref_by_id(nid); NodeExportStatus st; st.node_id = nid; - st.node_label = p_base ? p_base->get_caption() : nid; + st.node_label = p_base ? p_base->get_label() : nid; st.node_type = p_base ? p_base->get_node_type() : ""; st.state = NodeComputeState::Pending; scheduled_nodes.push_back(st); diff --git a/Hesiod/src/cli/check_port_links.cpp b/Hesiod/src/cli/check_port_links.cpp index 31280ec61..b9fce4027 100644 --- a/Hesiod/src/cli/check_port_links.cpp +++ b/Hesiod/src/cli/check_port_links.cpp @@ -23,7 +23,7 @@ int conventional_rule_changed_outcome = 0; void expect_offerable(const PortCatalog &catalog, const std::string &node_type, const std::string &data_type, - gngui::PortType wanted, + gnode::PortType wanted, bool expected) { const bool got = catalog.is_offerable(node_type, data_type, wanted); @@ -32,7 +32,7 @@ void expect_offerable(const PortCatalog &catalog, Logger::log()->error("check-port-links: {} [{}, want {}]: offerable={} expected={}", node_type, data_type, - wanted == gngui::PortType::IN ? "IN" : "OUT", + wanted == gnode::PortType::IN ? "IN" : "OUT", got, expected); failures++; @@ -41,7 +41,7 @@ void expect_offerable(const PortCatalog &catalog, void expect_selected(const std::string &node_type, const std::string &data_type, - gngui::PortType wanted, + gnode::PortType wanted, const std::string &expected_port) { auto config = std::make_shared(); @@ -64,7 +64,7 @@ void expect_selected(const std::string &node_type, "check-port-links: {} [{}, want {}]: selected '{}' expected '{}'", node_type, data_type, - wanted == gngui::PortType::IN ? "IN" : "OUT", + wanted == gnode::PortType::IN ? "IN" : "OUT", got_str, expected_port); failures++; @@ -110,7 +110,7 @@ void sweep_all_node_types(const PortCatalog &catalog) } for (const std::string &data_type : data_types) - for (gngui::PortType wanted : {gngui::PortType::IN, gngui::PortType::OUT}) + for (gnode::PortType wanted : {gnode::PortType::IN, gnode::PortType::OUT}) { const bool offered = catalog.is_offerable(node_type, data_type, wanted); const std::optional selected = hesiod::select_port(*p_base, @@ -125,7 +125,7 @@ void sweep_all_node_types(const PortCatalog &catalog) "node selected={} (documentation drift?)", node_type, data_type, - wanted == gngui::PortType::IN ? "IN" : "OUT", + wanted == gnode::PortType::IN ? "IN" : "OUT", offered, selected.has_value()); failures++; @@ -164,7 +164,7 @@ void sweep_all_node_types(const PortCatalog &catalog) std::string lower; for (char c : label) lower += static_cast(std::tolower(static_cast(c))); - const bool conventional = (wanted == gngui::PortType::IN) + const bool conventional = (wanted == gnode::PortType::IN) ? (lower == "input" || lower == "in") : (lower == "output" || lower == "out"); if (conventional) @@ -182,7 +182,7 @@ void sweep_all_node_types(const PortCatalog &catalog) "check-port-links: {} [{}, want {}]: oracle expected='{}' got='{}'", node_type, data_type, - wanted == gngui::PortType::IN ? "IN" : "OUT", + wanted == gnode::PortType::IN ? "IN" : "OUT", expected ? *expected : std::string(""), selected ? *selected : std::string("")); failures++; @@ -216,40 +216,40 @@ int run_check_port_links() // IslandChain's only VirtualArray port is its OUTPUT, so dragging a // VirtualArray from an output (wanting an input) must NOT offer it. // This is the case that aborted the application. - expect_offerable(catalog, "IslandChain", "VirtualArray", gngui::PortType::IN, false); + expect_offerable(catalog, "IslandChain", "VirtualArray", gnode::PortType::IN, false); // Dragging backwards from an input (wanting an output) must offer it. - expect_offerable(catalog, "IslandChain", "VirtualArray", gngui::PortType::OUT, true); + expect_offerable(catalog, "IslandChain", "VirtualArray", gnode::PortType::OUT, true); // Its Path input is offerable when a Path is dragged from an output. - expect_offerable(catalog, "IslandChain", "Path", gngui::PortType::IN, true); + expect_offerable(catalog, "IslandChain", "Path", gnode::PortType::IN, true); // Ordinary filters accept a VirtualArray input. - expect_offerable(catalog, "Laplace", "VirtualArray", gngui::PortType::IN, true); - expect_offerable(catalog, "Bump", "VirtualArray", gngui::PortType::IN, true); + expect_offerable(catalog, "Laplace", "VirtualArray", gnode::PortType::IN, true); + expect_offerable(catalog, "Bump", "VirtualArray", gnode::PortType::IN, true); // Incompatible type is never offered. - expect_offerable(catalog, "Laplace", "VirtualTexture", gngui::PortType::IN, false); + expect_offerable(catalog, "Laplace", "VirtualTexture", gnode::PortType::IN, false); // Unknown node type fails OPEN (never hide a real node if docs drift). - expect_offerable(catalog, "NoSuchNodeType", "VirtualArray", gngui::PortType::IN, true); + expect_offerable(catalog, "NoSuchNodeType", "VirtualArray", gnode::PortType::IN, true); // --- pinned port-selection cases (live node, true declaration order) // Conventional name wins: Laplace declares an "input" port. - expect_selected("Laplace", "VirtualArray", gngui::PortType::IN, "input"); + expect_selected("Laplace", "VirtualArray", gnode::PortType::IN, "input"); // No conventional name: Bump declares dx, dy, control, envelope -> first // declared wins. NOTE this is "dx" only because selection reads the LIVE // node; the documentation's alphabetical key order would have given // "control", which is why the catalog must never be used for selection. - expect_selected("Bump", "VirtualArray", gngui::PortType::IN, "dx"); + expect_selected("Bump", "VirtualArray", gnode::PortType::IN, "dx"); // Backwards drag: wanting an OUTPUT of type VirtualArray. - expect_selected("IslandChain", "VirtualArray", gngui::PortType::OUT, "output"); + expect_selected("IslandChain", "VirtualArray", gnode::PortType::OUT, "output"); // Forwards drag onto IslandChain has no VirtualArray input at all. - expect_selected("IslandChain", "VirtualArray", gngui::PortType::IN, ""); + expect_selected("IslandChain", "VirtualArray", gnode::PortType::IN, ""); sweep_all_node_types(catalog); diff --git a/Hesiod/src/gui/hesiod_node_proxy.cpp b/Hesiod/src/gui/hesiod_node_proxy.cpp new file mode 100644 index 000000000..56bfb3857 --- /dev/null +++ b/Hesiod/src/gui/hesiod_node_proxy.cpp @@ -0,0 +1,94 @@ +/* Copyright (c) 2026 Otto Link. Distributed under the terms of the GNU General + * Public License. The full license is in the file LICENSE, distributed with + * this software. */ +#include "hesiod/gui/hesiod_node_proxy.hpp" + +#include + +#include "hesiod/model/nodes/base_node.hpp" + +namespace hesiod +{ +HesiodNodeProxy::HesiodNodeProxy(std::weak_ptr model, QObject *owner) + : model(std::move(model)) +{ + this->setParent(owner); +} + +std::string HesiodNodeProxy::get_id() const +{ + if (auto node = model.lock()) + return node->get_id(); + return {}; +} + +void HesiodNodeProxy::set_id(const std::string &id) +{ + if (auto node = model.lock()) + node->set_id(id); +} + +std::string HesiodNodeProxy::get_caption() const +{ + if (auto node = model.lock()) + return node->get_label(); + return {}; +} + +std::string HesiodNodeProxy::get_category() const +{ + if (auto node = model.lock()) + return node->get_category(); + return {}; +} + +std::string HesiodNodeProxy::get_comment() const +{ + if (auto node = model.lock()) + return node->get_comment(); + return {}; +} + +std::string HesiodNodeProxy::get_tool_tip_text() const +{ + if (auto node = model.lock()) + return node->get_documentation_short_html(); + return {}; +} + +int HesiodNodeProxy::get_nports() const +{ + if (auto node = model.lock()) + return node->get_nports(); + return 0; +} + +std::string HesiodNodeProxy::get_port_caption(int index) const +{ + if (auto node = model.lock()) + return node->get_port_label(index); + return {}; +} + +gngui::PortType HesiodNodeProxy::get_port_type(int index) const +{ + if (auto node = model.lock()) + return node->get_port_type(index) == gnode::PortType::OUT ? gngui::PortType::OUT + : gngui::PortType::IN; + return gngui::PortType::IN; +} + +std::string HesiodNodeProxy::get_data_type(int index) const +{ + if (auto node = model.lock()) + return node->get_data_type(index); + return {}; +} + +void *HesiodNodeProxy::get_data_ref(int index) const +{ + if (auto node = model.lock()) + return node->get_value_ref_void(index); + return nullptr; +} +} // namespace hesiod diff --git a/Hesiod/src/gui/widgets/data_preview.cpp b/Hesiod/src/gui/widgets/data_preview.cpp index 65e4c8ba7..ba0cc85e3 100644 --- a/Hesiod/src/gui/widgets/data_preview.cpp +++ b/Hesiod/src/gui/widgets/data_preview.cpp @@ -28,7 +28,7 @@ DataPreview::DataPreview(std::weak_ptr model, QWidget *parent) throw std::invalid_argument("DataPreview::DataPreview: p_model_node is nullptr"); Logger::log()->trace("DataPreview::DataPreview, node {}({})", - p_model->get_caption(), + p_model->get_label(), p_model->get_id()); AppContext &ctx = HSD_CTX; @@ -43,7 +43,7 @@ DataPreview::DataPreview(std::weak_ptr model, QWidget *parent) // Select first output, or fallback to first port this->preview_port_index = 0; for (int k = 0; k < p_model->get_nports(); ++k) - if (p_model->get_port_type(k) == gngui::PortType::OUT) + if (p_model->get_port_type(k) == gnode::PortType::OUT) { this->preview_port_index = k; break; @@ -89,7 +89,7 @@ void DataPreview::contextMenuEvent(QContextMenuEvent *event) context_menu.addSection("Data"); for (int k = 0; k < p_model->get_nports(); ++k) { - const std::string caption = p_model->get_port_caption(k); + const std::string caption = p_model->get_port_label(k); QAction *action = context_menu.addAction(QString::fromStdString(caption)); action->setCheckable(true); if (k == preview_port_index) @@ -110,7 +110,7 @@ void DataPreview::contextMenuEvent(QContextMenuEvent *event) // Port selection for (int k = 0; k < p_model->get_nports(); ++k) - if (p_model->get_port_caption(k) == label) + if (p_model->get_port_label(k) == label) { preview_port_index = k; update_preview(); @@ -130,7 +130,7 @@ void DataPreview::update_preview() return; } - void *blind_ptr = p_model->get_data_ref(preview_port_index); + void *blind_ptr = p_model->get_value_ref_void(preview_port_index); const std::string data_type = p_model->get_data_type(preview_port_index); AppContext &ctx = HSD_CTX; diff --git a/Hesiod/src/gui/widgets/graph_node_widget.cpp b/Hesiod/src/gui/widgets/graph_node_widget.cpp index 1ce522a61..9c44e4fa7 100644 --- a/Hesiod/src/gui/widgets/graph_node_widget.cpp +++ b/Hesiod/src/gui/widgets/graph_node_widget.cpp @@ -18,6 +18,7 @@ #include "hesiod/app/hesiod_application.hpp" #include "hesiod/gui/graph_editor.hpp" +#include "hesiod/gui/hesiod_node_proxy.hpp" #include "hesiod/gui/widgets/custom_qmenu.hpp" #include "hesiod/gui/widgets/graph_config_widgets/graph_config_dialog.hpp" #include "hesiod/gui/widgets/graph_node_widget.hpp" @@ -426,10 +427,10 @@ void GraphNodeWidget::on_connection_dropped(const std::string &node_id, const std::string dragged_type = map_type_name( p_node_from->get_data_type(from_index)); - const gngui::PortType dragged_dir = p_node_from->get_port_type(from_index); - const gngui::PortType wanted_dir = (dragged_dir == gngui::PortType::OUT) - ? gngui::PortType::IN - : gngui::PortType::OUT; + const gnode::PortType dragged_dir = p_node_from->get_port_type(from_index); + const gnode::PortType wanted_dir = (dragged_dir == gnode::PortType::OUT) + ? gnode::PortType::IN + : gnode::PortType::OUT; // Filter GraphViewer's inventory for the duration of its blocking menu. const std::map full_inventory = get_node_inventory(); @@ -477,13 +478,13 @@ void GraphNodeWidget::on_connection_dropped(const std::string &node_id, "{} port of type {}, " "leaving it unconnected", node_to, - wanted_dir == gngui::PortType::IN ? "input" : "output", + wanted_dir == gnode::PortType::IN ? "input" : "output", dragged_type); batch.commit(); return; } - const bool dragged_is_output = (dragged_dir == gngui::PortType::OUT); + const bool dragged_is_output = (dragged_dir == gnode::PortType::OUT); const std::string id_out = dragged_is_output ? node_id : node_to; const std::string port_out = dragged_is_output ? port_id : *port_to; @@ -654,9 +655,8 @@ void GraphNodeWidget::on_new_graphics_node_request(const std::string &node_id, BaseNode *p_node = gno->get_node_ref_by_id(node_id); if (!p_node) throw std::runtime_error("Cannot display a missing model node."); - auto *p_proxy = new gngui::TypedNodeProxy(p_node->get_shared()); - p_proxy->setParent(this); - auto *widget = node_widget_factory(p_node->get_caption(), p_node->get_shared(), this); + auto *p_proxy = new HesiodNodeProxy(p_node->get_shared(), this); + auto *widget = node_widget_factory(p_node->get_label(), p_node->get_shared(), this); this->add_node(p_proxy, scene_pos, node_id); this->get_graphics_node_by_id(node_id)->set_widget(widget); diff --git a/Hesiod/src/gui/widgets/node_info_dialog.cpp b/Hesiod/src/gui/widgets/node_info_dialog.cpp index 96cff2901..42b9b1003 100644 --- a/Hesiod/src/gui/widgets/node_info_dialog.cpp +++ b/Hesiod/src/gui/widgets/node_info_dialog.cpp @@ -160,7 +160,7 @@ void NodeInfoDialog::setup_layout() // --- main label { - std::string str = ptrs.node->get_caption() + "/" + ptrs.node->get_id(); + std::string str = ptrs.node->get_label() + "/" + ptrs.node->get_id(); QLabel *label = new QLabel(str.c_str()); this->layout->addWidget(label); } @@ -251,7 +251,7 @@ void NodeInfoDialog::update_info_content() auto cfg = ptrs.node->get_config_ref(); std::vector rows = { - {"Type", ptrs.node->get_caption()}, + {"Type", ptrs.node->get_label()}, {"Category", ptrs.node->get_category()}, {"ID", ptrs.node->get_id()}, {"Created", timestamp(info.time_creation)}, @@ -310,14 +310,14 @@ void NodeInfoDialog::update_ports_content() for (int k = 0; k < ptrs.node->get_nports(); k++) { Row new_row; - new_row.caption = ptrs.node->get_port_caption(k); + new_row.caption = ptrs.node->get_port_label(k); new_row.is_connected = ptrs.node->is_port_connected(k); std::string str_ct = new_row.is_connected ? "✓" : " "; std::string str_in = std::format("→[{}] ", str_ct); std::string str_out = std::format(" [{}]→", str_ct); - new_row.type = (ptrs.node->get_port_type(k) == gngui::PortType::IN) ? str_in + new_row.type = (ptrs.node->get_port_type(k) == gnode::PortType::IN) ? str_in : str_out; new_row.data_type = map_type_name(ptrs.node->get_data_type(k)); diff --git a/Hesiod/src/gui/widgets/node_settings_widget.cpp b/Hesiod/src/gui/widgets/node_settings_widget.cpp index 2cc3d4f91..fca81ef8b 100644 --- a/Hesiod/src/gui/widgets/node_settings_widget.cpp +++ b/Hesiod/src/gui/widgets/node_settings_widget.cpp @@ -165,7 +165,7 @@ void NodeSettingsWidget::update_content() continue; } - const QString node_caption = QString::fromStdString(p_node->get_caption()); + const QString node_caption = QString::fromStdString(p_node->get_label()); const bool add_toolbar = HSD_CTX.app_settings.node_editor .show_node_toolbar_in_settings_pan; diff --git a/Hesiod/src/gui/widgets/viewers/viewer.cpp b/Hesiod/src/gui/widgets/viewers/viewer.cpp index ef9f41843..48d828a8b 100644 --- a/Hesiod/src/gui/widgets/viewers/viewer.cpp +++ b/Hesiod/src/gui/widgets/viewers/viewer.cpp @@ -452,10 +452,10 @@ void Viewer::update_widgets() } else if (BaseNode *p_node = this->safe_get_node()) { - std::string new_title = this->label + " - " + p_node->get_caption() + "(" + + std::string new_title = this->label + " - " + p_node->get_label() + "(" + p_node->get_id() + ")"; this->setWindowTitle(new_title.c_str()); - this->button_pin_current_node->set_label(p_node->get_caption().c_str()); + this->button_pin_current_node->set_label(p_node->get_label().c_str()); } // --- update combo content @@ -469,7 +469,7 @@ void Viewer::update_widgets() { combo_options.reserve(p_node->get_nports()); for (int k = 0; k < p_node->get_nports(); ++k) - combo_options.push_back(p_node->get_port_caption(k)); + combo_options.push_back(p_node->get_port_label(k)); } } diff --git a/Hesiod/src/gui/widgets/viewers/wild_guess_view_param.cpp b/Hesiod/src/gui/widgets/viewers/wild_guess_view_param.cpp index 85705c887..f198e0dfe 100644 --- a/Hesiod/src/gui/widgets/viewers/wild_guess_view_param.cpp +++ b/Hesiod/src/gui/widgets/viewers/wild_guess_view_param.cpp @@ -33,12 +33,12 @@ std::string helper_get_preferred_port_inout(const BaseNode &node { const std::string port_label = node.get_port_label(k); - if (node.get_port_type(k) == gngui::PortType::OUT && !is_excluded(port_label)) + if (node.get_port_type(k) == gnode::PortType::OUT && !is_excluded(port_label)) { value = port_label; break; // OUT has priority } - else if (in_candidate == -1 && node.get_port_type(k) == gngui::PortType::IN && + else if (in_candidate == -1 && node.get_port_type(k) == gnode::PortType::IN && !is_excluded(port_label)) { in_candidate = k; diff --git a/Hesiod/src/model/nodes/base_node.cpp b/Hesiod/src/model/nodes/base_node.cpp index 1354401cc..d1cb2ef76 100644 --- a/Hesiod/src/model/nodes/base_node.cpp +++ b/Hesiod/src/model/nodes/base_node.cpp @@ -76,7 +76,7 @@ const GraphConfig &BaseNode::cfg() const if (!ptr) { Logger::log()->critical("BaseNode::get_category: Config ptr is nullptr, node: {}/{}", - this->get_caption(), + this->get_label(), this->get_id()); throw std::runtime_error("Config ptr is nullptr."); } @@ -197,7 +197,7 @@ std::shared_ptr BaseNode::get_config_ref() const if (!ptr) { Logger::log()->critical("BaseNode::get_category: Config ptr is nullptr, node: {}/{}", - this->get_caption(), + this->get_label(), this->get_id()); throw std::runtime_error("Config ptr is nullptr."); } @@ -391,6 +391,13 @@ std::string BaseNode::get_documentation_short_html() const return html; } +std::string BaseNode::get_comment() const { return this->comment; } + +gnode::PortType BaseNode::get_port_type(int port_index) const +{ + return gnode::Node::get_port_type(this->get_port_label(port_index)); +} + std::string BaseNode::get_id() const { return gnode::Node::get_id(); } float BaseNode::get_memory_usage() const @@ -404,7 +411,7 @@ float BaseNode::get_memory_usage() const for (int k = 0; k < this->get_nports(); k++) { // only outputs carry data - if (this->get_port_type(k) == gngui::PortType::IN) + if (this->get_port_type(k) == gnode::PortType::IN) continue; if (this->get_data_type(k) == typeid(hmap::VirtualArray).name()) @@ -571,9 +578,9 @@ nlohmann::json BaseNode::node_parameters_to_json() const for (int k = 0; k < this->get_nports(); k++) { nlohmann::json port_info; - const std::string caption = this->get_port_caption(k); + const std::string caption = this->get_port_label(k); - port_info["type"] = (this->get_port_type(k) == gngui::PortType::IN) ? "input" + port_info["type"] = (this->get_port_type(k) == gnode::PortType::IN) ? "input" : "output"; port_info["caption"] = caption; port_info["data_type"] = map_type_name(this->get_data_type(k)); @@ -650,14 +657,14 @@ nlohmann::json BaseNode::node_parameters_to_json() const void BaseNode::propagate_config_change() { Logger::log()->trace("BaseNode::propagate_config_change: node {}/{}", - this->get_caption(), + this->get_label(), this->get_id()); const GraphConfig &cfg = *this->get_config_ref(); // go through the data and modify is needed (only outputs hold data) for (int k = 0; k < this->get_nports(); k++) - if (this->get_port_type(k) == gngui::PortType::OUT) + if (this->get_port_type(k) == gnode::PortType::OUT) { const std::string type = this->get_data_type(k); diff --git a/Hesiod/src/model/nodes/base_node_proxy.cpp b/Hesiod/src/model/nodes/base_node_proxy.cpp deleted file mode 100644 index 9dc7269cb..000000000 --- a/Hesiod/src/model/nodes/base_node_proxy.cpp +++ /dev/null @@ -1,47 +0,0 @@ -/* Copyright (c) 2023 Otto Link. Distributed under the terms of the GNU General - * Public License. The full license is in the file LICENSE, distributed with - * this software. */ -#include - -#include "gnode/graph.hpp" -#include "hesiod/app/hesiod_application.hpp" -#include "hesiod/logger.hpp" -#include "hesiod/model/nodes/base_node.hpp" - -namespace hesiod -{ - -std::string BaseNode::get_caption() const { return this->get_label(); } - -std::string BaseNode::get_comment() const { return this->comment; } - -void *BaseNode::get_data_ref(int port_index) -{ - return this->get_value_ref_void(port_index); -} - -std::string BaseNode::get_data_type(int port_index) const -{ - return gnode::Node::get_data_type(port_index); -} - -int BaseNode::get_nports() const { return gnode::Node::get_nports(); } - -std::string BaseNode::get_port_caption(int port_index) const -{ - return gnode::Node::get_port_label(port_index); -}; - -gngui::PortType BaseNode::get_port_type(int port_index) const -{ - gnode::PortType ptype = gnode::Node::get_port_type(this->get_port_label(port_index)); - - if (ptype == gnode::PortType::IN) - return gngui::PortType::IN; - else - return gngui::PortType::OUT; -} - -std::string BaseNode::get_tool_tip_text() { return this->get_documentation_short_html(); } - -} // namespace hesiod diff --git a/Hesiod/src/model/nodes/port_catalog.cpp b/Hesiod/src/model/nodes/port_catalog.cpp index cf0ede4a5..17b76ad1b 100644 --- a/Hesiod/src/model/nodes/port_catalog.cpp +++ b/Hesiod/src/model/nodes/port_catalog.cpp @@ -36,8 +36,8 @@ PortCatalog PortCatalog::from_documentation() info.name = port_name; info.data_type = port["data_type"].get(); info.direction = (port["type"].get() == "input") - ? gngui::PortType::IN - : gngui::PortType::OUT; + ? gnode::PortType::IN + : gnode::PortType::OUT; infos.push_back(std::move(info)); } @@ -55,7 +55,7 @@ const std::vector *PortCatalog::find(const std::string &node_type) con bool PortCatalog::is_offerable(const std::string &node_type, const std::string &data_type, - gngui::PortType wanted_direction) const + gnode::PortType wanted_direction) const { const std::vector *infos = this->find(node_type); @@ -73,14 +73,14 @@ bool PortCatalog::is_offerable(const std::string &node_type, namespace { -bool is_conventional_name(const std::string &name, gngui::PortType direction) +bool is_conventional_name(const std::string &name, gnode::PortType direction) { std::string lower; lower.reserve(name.size()); for (char c : name) lower += static_cast(std::tolower(static_cast(c))); - if (direction == gngui::PortType::IN) + if (direction == gnode::PortType::IN) return lower == "input" || lower == "in"; return lower == "output" || lower == "out"; @@ -90,7 +90,7 @@ bool is_conventional_name(const std::string &name, gngui::PortType direction) std::optional select_port(const BaseNode &node, const std::string &data_type, - gngui::PortType wanted_direction) + gnode::PortType wanted_direction) { std::optional first_match; diff --git a/docs/graph-editor-refactor.md b/docs/graph-editor-refactor.md index 14e4bdd6e..d018e99a7 100644 --- a/docs/graph-editor-refactor.md +++ b/docs/graph-editor-refactor.md @@ -30,7 +30,7 @@ deletion. They do not yet test Hesiod model computation. GNodeGUI PR #12 is merged. It supplies the dependency for the Hesiod extraction below; by itself it retains the legacy behavior for existing callers. -## 2. Extract GraphEditor and move topology edits (this change) +## 2. Extract GraphEditor and move topology edits (PR #771) GraphNodeWidget now owns one GraphEditor and delegates node creation/deletion, connection/disconnection, replacement, chain insertion, paste/duplicate, import @@ -79,18 +79,19 @@ cmake --build build --target hesiod test_graph_editor ctest --test-dir build -R '^graph_editor$' --output-on-failure ``` -## 3. Move the Hesiod node proxy into the GUI layer +## 3. Move the Hesiod node proxy into the GUI layer (this change) -Introduce an explicit Hesiod adapter implementing GNodeGUI's NodeProxy interface. -Move GUI port conversion and presentation responsibilities out of BaseNode. Use -GNode's port direction type in model APIs and convert it at the adapter boundary. -Give the proxy a clear owner and make identifier/lifetime behavior explicit. +`HesiodNodeProxy` adapts GNode's accessors and port directions to GNodeGUI. +BaseNode no longer includes GNodeGUI or implements its proxy interface. +PortCatalog and select_port also use GNode's direction type. -BaseNode currently includes `gnodegui/node_proxy.hpp`, which includes Qt. Thus -GraphNode's header has no direct Qt include, but the model implementation is not -yet Qt-independent. Verify removal with a model-header compilation check without -Qt include paths, as well as the normal application build. Avoid changing saved -node identifiers, port IDs or captions as an incidental effect of this move. +The widget owns each proxy through QObject parenting; the model reference is weak. +Identifiers, captions and serialized fields retain their previous values. Expired +models return empty proxy values. + +Tests cover the adapter, ownership and model expiry. Building `test_graph_editor` +also compiles BaseNode, GraphNode and PortCatalog headers without Qt or GNodeGUI +include paths. Model implementations still use Qt and application services. ## 4. Loading, settings and remaining update paths @@ -102,8 +103,9 @@ widgets and retain legacy port migration behavior. Route GUI settings, configuration and reload requests through the editor's update policy. Inventory other direct calls, including NodeAttributesWidget and special node widgets, to complete the editor update policy. The widget blocker was removed -in step 2. Domain-driven broadcasting and headless execution remain valid model callers; the editor is the single entry -point for interactive edits, not a compulsory Qt dependency for all computation. +in step 2. Domain-driven broadcasting and headless execution remain valid model +callers; the editor is the single entry point for interactive edits, not a +compulsory Qt dependency for all computation. Finish with project round-trip, paste/import, legacy project, selection/viewer lifetime and Broadcast/Receive regression checks. The existing editor widget API diff --git a/tests/gui/test_graph_editor.cpp b/tests/gui/test_graph_editor.cpp index 78f01d37d..ecb58b83e 100644 --- a/tests/gui/test_graph_editor.cpp +++ b/tests/gui/test_graph_editor.cpp @@ -14,6 +14,7 @@ #include "gnodegui/style.hpp" #include "hesiod/app/hesiod_application.hpp" #include "hesiod/gui/graph_editor.hpp" +#include "hesiod/gui/hesiod_node_proxy.hpp" #include "hesiod/gui/widgets/graph_node_widget.hpp" #include "hesiod/model/graph/graph_manager.hpp" #include "hesiod/model/graph/graph_node.hpp" @@ -58,13 +59,13 @@ bool consistent(GraphNode &graph, gngui::GraphViewer &view) view_links.size() == json.at("links").size(); } -class UnavailableOutputProxy : public gngui::TypedNodeProxy +class UnavailableOutputProxy : public HesiodNodeProxy { public: - using gngui::TypedNodeProxy::TypedNodeProxy; + using HesiodNodeProxy::HesiodNodeProxy; std::string get_port_id(int index) const override { - return index == 1 ? "unavailable" : TypedNodeProxy::get_port_id(index); + return index == 1 ? "unavailable" : HesiodNodeProxy::get_port_id(index); } }; @@ -97,9 +98,8 @@ struct Fixture auto node = graph->get_node_ref_by_id(id)->get_shared(); gngui::NodeProxy *proxy = fail_reconnection ? static_cast( - new UnavailableOutputProxy(node)) - : new gngui::TypedNodeProxy(node); - proxy->setParent(&view); + new UnavailableOutputProxy(node, &view)) + : new HesiodNodeProxy(node, &view); view.add_node(proxy, position, id); if (fail_presentation) throw std::runtime_error("Injected presentation failure"); @@ -153,6 +153,74 @@ class GraphEditorTest : public QObject { Q_OBJECT private Q_SLOTS: + void proxy_preserves_node_identity_and_port_values() + { + Fixture f; + const auto id = f.add(); + auto *node = f.graph->get_node_ref_by_id(id); + const auto *proxy = f.view.get_graphics_node_by_id(id)->get_proxy_ref(); + QVERIFY(dynamic_cast(proxy)); + QCOMPARE(proxy->get_id(), id); + QCOMPARE(proxy->get_caption(), std::string("Thru")); + QCOMPARE(proxy->get_category(), node->get_category()); + QCOMPARE(proxy->get_tool_tip_text(), node->get_documentation_short_html()); + QCOMPARE(proxy->get_nports(), 2); + QCOMPARE(proxy->get_port_id(0), std::string("input")); + QCOMPARE(proxy->get_port_id(1), std::string("output")); + QCOMPARE(proxy->get_port_caption(0), std::string("input")); + QCOMPARE(proxy->get_port_caption(1), std::string("output")); + QCOMPARE(proxy->get_port_type(0), gngui::PortType::IN); + QCOMPARE(proxy->get_port_type(1), gngui::PortType::OUT); + QCOMPARE(proxy->get_data_type(1), std::string(typeid(hmap::VirtualArray).name())); + QCOMPARE(proxy->get_data_ref(1), node->get_value_ref_void(1)); + node->set_comment("Updated comment"); + QCOMPARE(proxy->get_comment(), std::string("Updated comment")); + QCOMPARE(f.view.json_to()["nodes"][0]["id"].get(), id); + QCOMPARE(node->json_to()["id"].get(), id); + } + + void proxy_does_not_extend_model_lifetime() + { + QObject owner; + auto node = std::make_shared(); + node->set_id("before"); + auto *proxy = new HesiodNodeProxy(node, &owner); + proxy->set_id("after"); + QCOMPARE(node->get_id(), std::string("after")); + std::weak_ptr weak = node; + node.reset(); + QVERIFY(weak.expired()); + proxy->set_id("expired"); + QVERIFY(proxy->get_id().empty()); + QVERIFY(proxy->get_caption().empty()); + QVERIFY(proxy->get_category().empty()); + QVERIFY(proxy->get_comment().empty()); + QVERIFY(proxy->get_tool_tip_text().empty()); + QCOMPARE(proxy->get_nports(), 0); + QVERIFY(proxy->get_port_id(0).empty()); + QVERIFY(proxy->get_port_caption(0).empty()); + QVERIFY(proxy->get_data_type(0).empty()); + QCOMPARE(proxy->get_port_type(0), gngui::PortType::IN); + QVERIFY(proxy->get_data_ref(0) == nullptr); + } + + void widget_owns_its_node_proxies() + { + auto graph = std::make_shared("owner", small_config()); + QPointer proxy; + { + GraphNodeWidget widget(graph); + widget.scene()->setParent(&widget); + const auto id = widget.on_new_node_request("Thru", {}); + proxy = widget.get_graphics_node_by_id(id)->get_proxy_ref(); + QCOMPARE(proxy->parent(), &widget); + widget.erase_node(id); + QVERIFY(proxy); + } + QVERIFY(proxy.isNull()); + QCOMPARE(graph->get_nodes().size(), size_t(1)); + } + void nested_batches_compute_only_after_commit() { Fixture f; diff --git a/tests/model/model_headers.cpp b/tests/model/model_headers.cpp new file mode 100644 index 000000000..d94d51b53 --- /dev/null +++ b/tests/model/model_headers.cpp @@ -0,0 +1,20 @@ +/* Copyright (c) 2026 Otto Link. Distributed under the terms of the GNU General + * Public License. The full license is in the file LICENSE, distributed with + * this software. */ +#include +#include + +#if __has_include() || __has_include("gnodegui/node_proxy.hpp") +#error "Compile model headers without Qt or GNodeGUI include paths" +#endif + +#include "hesiod/model/graph/graph_node.hpp" +#include "hesiod/model/nodes/base_node.hpp" +#include "hesiod/model/nodes/port_catalog.hpp" + +static_assert( + std::is_same_v().get_port_type(0)), + gnode::PortType>); +static_assert(std::is_same_v() + .get_port_type(std::string{})), + gnode::PortType>); From dafe668e7dc95109f687a235cc9ceff87879ac5a Mon Sep 17 00:00:00 2001 From: Otto Link Date: Sat, 19 Sep 2026 09:19:03 +0200 Subject: [PATCH 3/3] chore: format code --- Hesiod/CMakeLists.txt | 56 +++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/Hesiod/CMakeLists.txt b/Hesiod/CMakeLists.txt index ad34ef935..91251a489 100644 --- a/Hesiod/CMakeLists.txt +++ b/Hesiod/CMakeLists.txt @@ -10,7 +10,8 @@ set(CMAKE_AUTOMOC_VERBOSE ON) # Source files # ------------------------------ file(GLOB_RECURSE HESIOD_GUI_INCLUDES ${CMAKE_CURRENT_SOURCE_DIR}/include/*.hpp) -file(GLOB_RECURSE HESIOD_SOURCES CONFIGURE_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp) +file(GLOB_RECURSE HESIOD_SOURCES CONFIGURE_DEPENDS + ${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp) if(HESIOD_MINIMAL_NODE_SET) # option for a minimal set of nodes for quick compile time when tempering with @@ -59,7 +60,7 @@ target_link_libraries(${PROJECT_NAME} PRIVATE hesiod_core) target_include_directories( hesiod_core PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include ${CMAKE_BINARY_DIR}/include - ${OPENGL_INCLUDE_DIRS} ${GLEW_INCLUDE_DIRS} ${GLUT_INCLUDE_DIRS}) + ${OPENGL_INCLUDE_DIRS} ${GLEW_INCLUDE_DIRS} ${GLUT_INCLUDE_DIRS}) # ------------------------------ # Compiler features @@ -72,24 +73,24 @@ target_compile_features(hesiod_core PUBLIC cxx_std_20) target_link_libraries( hesiod_core PUBLIC hesiod_options - hesiod_platform - hesiod_qt_logging - args - spdlog::spdlog - nlohmann_json::nlohmann_json - highmap - gnode - gnodegui - meta - meta_qt - Qt6::Core - Qt6::OpenGL - Qt6::Widgets - Qt6::OpenGLWidgets - Qt6::WebEngineWidgets - qterrain-renderer - qtexture_downloader - ZLIB::ZLIB) + hesiod_platform + hesiod_qt_logging + args + spdlog::spdlog + nlohmann_json::nlohmann_json + highmap + gnode + gnodegui + meta + meta_qt + Qt6::Core + Qt6::OpenGL + Qt6::Widgets + Qt6::OpenGLWidgets + Qt6::WebEngineWidgets + qterrain-renderer + qtexture_downloader + ZLIB::ZLIB) # ------------------------------ # Precompiled Headers (PCH) @@ -134,17 +135,20 @@ message(STATUS "Build type: ${CMAKE_BUILD_TYPE}") if(HESIOD_ENABLE_TESTS) find_package(Qt6 REQUIRED COMPONENTS Test) - add_library(test_model_headers OBJECT ${CMAKE_SOURCE_DIR}/tests/model/model_headers.cpp) + add_library(test_model_headers OBJECT + ${CMAKE_SOURCE_DIR}/tests/model/model_headers.cpp) set_target_properties(test_model_headers PROPERTIES AUTOMOC OFF) - target_include_directories(test_model_headers PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include) + target_include_directories(test_model_headers + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include) target_link_libraries(test_model_headers PRIVATE gnode highmap meta) target_compile_features(test_model_headers PRIVATE cxx_std_20) - add_executable(test_graph_editor ${CMAKE_SOURCE_DIR}/tests/gui/test_graph_editor.cpp) + add_executable(test_graph_editor + ${CMAKE_SOURCE_DIR}/tests/gui/test_graph_editor.cpp) add_dependencies(test_graph_editor test_model_headers) target_link_libraries(test_graph_editor PRIVATE hesiod_core Qt6::Test) add_test(NAME graph_editor COMMAND test_graph_editor) - set_tests_properties(graph_editor PROPERTIES - WORKING_DIRECTORY ${EXECUTABLE_OUTPUT_PATH} - ENVIRONMENT "QT_QPA_PLATFORM=offscreen") + set_tests_properties( + graph_editor PROPERTIES WORKING_DIRECTORY ${EXECUTABLE_OUTPUT_PATH} + ENVIRONMENT "QT_QPA_PLATFORM=offscreen") endif()