From a1d4d77be4e97b13b719ff9340b913352d2f5b33 Mon Sep 17 00:00:00 2001 From: parker Date: Mon, 14 Jul 2025 16:36:44 +0100 Subject: [PATCH] feat: remove static members from singleton, convert to thread safe --- src/Engine/Network/NetworkManager.cpp | 21 +++++-------- src/Engine/Network/NetworkManager.h | 29 +++++++++-------- src/Engine/Operator/GeometryOpDef.cpp | 5 +-- src/Engine/Operator/GeometryOperator.cpp | 6 ++-- src/Gui/Interface.cpp | 2 +- src/Gui/Network/Network.cpp | 8 ++--- src/Gui/Network/NodeGraphic.cpp | 4 +-- tests/Benchmarks.cpp | 16 +++++----- tests/NetworkTests.cpp | 40 ++++++++++++++---------- 9 files changed, 71 insertions(+), 60 deletions(-) diff --git a/src/Engine/Network/NetworkManager.cpp b/src/Engine/Network/NetworkManager.cpp index c4f9eb6..7bfe4d3 100644 --- a/src/Engine/Network/NetworkManager.cpp +++ b/src/Engine/Network/NetworkManager.cpp @@ -21,23 +21,21 @@ enzo::nt::OpId enzo::nt::NetworkManager::addOperator(nt::opConstructor ctorFunc) } -enzo::nt::NetworkManager* enzo::nt::NetworkManager::getInstance() +enzo::nt::NetworkManager& enzo::nt::NetworkManager::getInstance() { - if(instancePtr_==nullptr) - { - instancePtr_ = new enzo::nt::NetworkManager(); - } - return instancePtr_; + static enzo::nt::NetworkManager instance; + return instance; } enzo::nt::GeometryOperator& enzo::nt::NetworkManager::getGeoOperator(nt::OpId opId) { std::cout << "gop size middle getter: " << gopStore_.size() <<"\n"; // <- size 0 - if(opId>gopStore_.size()) + auto it = gopStore_.find(opId); + if(it == gopStore_.end()) { throw std::out_of_range("OpId: " + std::to_string(opId) + " > max opId: " + std::to_string(maxOpId_) + "\n"); } - return *gopStore_.at(opId); + return *it->second; } bool enzo::nt::NetworkManager::isValidOp(nt::OpId opId) @@ -64,7 +62,7 @@ void enzo::nt::NetworkManager::setDisplayOp(OpId opId) } std::cout << "gop size middle: " << gopStore_.size() <<"\n"; // <- size: 1 enzo::nt::GeometryOperator& displayOp = getGeoOperator(opId); - getInstance()->updateDisplay(displayOp.getOutputGeo(0)); + updateDisplay(displayOp.getOutputGeo(0)); std::cout << "gop size after: " << gopStore_.size() <<"\n"; } @@ -108,8 +106,6 @@ std::optional enzo::nt::NetworkManager::getDisplayOp() void enzo::nt::NetworkManager::_reset() { std::cout << "resetting network manager\n"; - delete instancePtr_; - instancePtr_ = nullptr; gopStore_.clear(); maxOpId_=0; @@ -117,6 +113,5 @@ void enzo::nt::NetworkManager::_reset() } #endif -enzo::nt::NetworkManager* enzo::nt::NetworkManager::instancePtr_ = nullptr; -std::unordered_map> enzo::nt::NetworkManager::gopStore_; +// std::unordered_map> enzo::nt::NetworkManager::gopStore_; diff --git a/src/Engine/Network/NetworkManager.h b/src/Engine/Network/NetworkManager.h index f94fcfb..309743b 100644 --- a/src/Engine/Network/NetworkManager.h +++ b/src/Engine/Network/NetworkManager.h @@ -15,36 +15,39 @@ public: // delete copy constructor NetworkManager(const NetworkManager& obj) = delete; - static NetworkManager* getInstance(); + static NetworkManager& getInstance(); // functions - static OpId addOperator(nt::opConstructor ctorFunc); - static std::optional getDisplayOp(); - static bool isValidOp(nt::OpId opId); - static GeometryOperator& getGeoOperator(nt::OpId opId); - static void setDisplayOp(OpId opId); + OpId addOperator(nt::opConstructor ctorFunc); + std::optional getDisplayOp(); + bool isValidOp(nt::OpId opId); + GeometryOperator& getGeoOperator(nt::OpId opId); + void setDisplayOp(OpId opId); #ifdef UNIT_TEST - static void _reset(); + void _reset(); #endif private: - static NetworkManager* instancePtr_; NetworkManager() {}; // functions - static void cookOp(enzo::nt::OpId opId); - static std::vector getDependencyGraph(enzo::nt::OpId opId); + void cookOp(enzo::nt::OpId opId); + std::vector getDependencyGraph(enzo::nt::OpId opId); // variables // store for geometry operators - static std::unordered_map> gopStore_; + std::unordered_map> gopStore_; // the highest operator id currently stored - inline static enzo::nt::OpId maxOpId_=0; + enzo::nt::OpId maxOpId_=0; // operator selected for displaying in the viewport - inline static std::optional displayOp_=std::nullopt; + std::optional displayOp_=std::nullopt; signals: void updateDisplay(enzo::geo::Geometry& geometry); }; + +inline enzo::nt::NetworkManager& nm() { + return enzo::nt::NetworkManager::getInstance(); +} } diff --git a/src/Engine/Operator/GeometryOpDef.cpp b/src/Engine/Operator/GeometryOpDef.cpp index 428a5e6..090deca 100644 --- a/src/Engine/Operator/GeometryOpDef.cpp +++ b/src/Engine/Operator/GeometryOpDef.cpp @@ -22,8 +22,9 @@ const enzo::geo::Geometry& enzo::nt::GeometryOpDef::getInputGeoView(unsigned int enzo::geo::Geometry enzo::nt::GeometryOpDef::cloneInputGeo(unsigned int inputIndex) { + auto& nm = nt::nm(); // TODO: implement - enzo::nt::GeometryOperator& selfOp = nt::NetworkManager::getGeoOperator(opId_); + enzo::nt::GeometryOperator& selfOp = nm.getGeoOperator(opId_); std::vector> inputConnections = selfOp.getInputConnections(); if(inputConnections.size()==0) { @@ -31,7 +32,7 @@ enzo::geo::Geometry enzo::nt::GeometryOpDef::cloneInputGeo(unsigned int inputInd return enzo::geo::Geometry(); } std::shared_ptr inputConnection = inputConnections.at(inputIndex); - return nt::NetworkManager::getGeoOperator(inputConnection->getInputOpId()).getOutputGeo(inputConnection->getInputIndex()); + return nm.getGeoOperator(inputConnection->getInputOpId()).getOutputGeo(inputConnection->getInputIndex()); } void enzo::nt::GeometryOpDef::setOutputGeometry(unsigned int outputIndex, enzo::geo::Geometry geometry) diff --git a/src/Engine/Operator/GeometryOperator.cpp b/src/Engine/Operator/GeometryOperator.cpp index 718b267..1b31110 100644 --- a/src/Engine/Operator/GeometryOperator.cpp +++ b/src/Engine/Operator/GeometryOperator.cpp @@ -9,8 +9,10 @@ using namespace enzo; void enzo::nt::connectOperators(enzo::nt::OpId inputOpId, unsigned int inputIndex, enzo::nt::OpId outputOpId, unsigned int outputIndex) { - auto& inputOp = nt::NetworkManager::getGeoOperator(inputOpId); - auto& outputOp = nt::NetworkManager::getGeoOperator(outputOpId); + auto& nm = nt::nm(); + + auto& inputOp = nm.getGeoOperator(inputOpId); + auto& outputOp = nm.getGeoOperator(outputOpId); auto newConnection = std::make_shared(inputOpId, inputIndex, outputOpId, outputIndex); diff --git a/src/Gui/Interface.cpp b/src/Gui/Interface.cpp index d82f59c..8e3615f 100644 --- a/src/Gui/Interface.cpp +++ b/src/Gui/Interface.cpp @@ -44,6 +44,6 @@ EnzoUI::EnzoUI() mainLayout_->addWidget(viewportSplitter_); // connect signals - connect(enzo::nt::NetworkManager::getInstance(), &enzo::nt::NetworkManager::updateDisplay, viewport, &Viewport::geometryChanged); + connect(&enzo::nt::nm(), &enzo::nt::NetworkManager::updateDisplay, viewport, &Viewport::geometryChanged); } diff --git a/src/Gui/Network/Network.cpp b/src/Gui/Network/Network.cpp index 3fc772f..0167eba 100644 --- a/src/Gui/Network/Network.cpp +++ b/src/Gui/Network/Network.cpp @@ -310,7 +310,7 @@ void Network::keyPressEvent(QKeyEvent *event) NodeGraphic* Network::createNode(nt::opConstructor ctorFunc) { - if(nt::OpId id = nt::NetworkManager::addOperator(ctorFunc)) + if(nt::OpId id = enzo::nt::nm().addOperator(ctorFunc)) { NodeGraphic* newNode = new NodeGraphic(id); scene_->addItem(newNode); @@ -376,15 +376,15 @@ void Network::mouseReleaseEvent(QMouseEvent *event) QLineF(event->pos(), leftMouseStart).length()<5.0f ) { - enzo::nt::NetworkManager* nm = enzo::nt::NetworkManager::getInstance(); + enzo::nt::NetworkManager& nm = enzo::nt::nm(); NodeGraphic* clickedNode = static_cast(itemOfType(hoverItems)); enzo::nt::OpId opId = clickedNode->getOpId(); - if(auto prevDisplayOpId = nt::NetworkManager::getDisplayOp(); prevDisplayOpId) + if(auto prevDisplayOpId = nm.getDisplayOp(); prevDisplayOpId) { NodeGraphic* prevDisplayNode = nodeStore_.at(*prevDisplayOpId); prevDisplayNode->setDisplayFlag(false); } - enzo::nt::NetworkManager::setDisplayOp(opId); + nm.setDisplayOp(opId); static_cast(clickedDisplayFlag)->setEnabled(true); } if(state_==State::MOVING_NODE) diff --git a/src/Gui/Network/NodeGraphic.cpp b/src/Gui/Network/NodeGraphic.cpp index 91ca6f6..fb650ff 100644 --- a/src/Gui/Network/NodeGraphic.cpp +++ b/src/Gui/Network/NodeGraphic.cpp @@ -79,7 +79,7 @@ void NodeGraphic::initFlagButtons() void NodeGraphic::initSockets() { - enzo::nt::GeometryOperator& op = enzo::nt::NetworkManager::getGeoOperator(opId_); + enzo::nt::GeometryOperator& op = enzo::nt::nm().getGeoOperator(opId_); for(int i=0, max=op.getMaxInputs(); i prevOps; @@ -31,7 +34,7 @@ TEST_CASE_METHOD(NMReset, "Network Manager") { for(int i=0; i<4; ++i) { - nt::OpId newOp = nt::NetworkManager::addOperator(&GOP_test::ctor); + nt::OpId newOp = nm.addOperator(&GOP_test::ctor); prevOps.push_back(newOp); nt::connectOperators(newOp, i, prevOp, 0); } @@ -41,7 +44,7 @@ TEST_CASE_METHOD(NMReset, "Network Manager") for(int i=0; isetDisplayOp(startOp); + nm.setDisplayOp(startOp); }; diff --git a/tests/NetworkTests.cpp b/tests/NetworkTests.cpp index d18d503..db8e889 100644 --- a/tests/NetworkTests.cpp +++ b/tests/NetworkTests.cpp @@ -11,11 +11,11 @@ struct NMReset { NMReset() { - enzo::nt::NetworkManager::_reset(); + enzo::nt::nm()._reset(); } ~NMReset() { - enzo::nt::NetworkManager::_reset(); + enzo::nt::nm()._reset(); } }; @@ -23,32 +23,38 @@ struct NMReset TEST_CASE_METHOD(NMReset, "network fixture separation start") { using namespace enzo; - nt::OpId newOpId = nt::NetworkManager::addOperator(GOP_test::ctor); + auto& nm = nt::nm(); + + nt::OpId newOpId = nm.addOperator(GOP_test::ctor); REQUIRE(newOpId==1); - REQUIRE(nt::NetworkManager::isValidOp(1)); + REQUIRE(nm.isValidOp(1)); } TEST_CASE_METHOD(NMReset, "network fixture separation end") { using namespace enzo; - REQUIRE_FALSE(nt::NetworkManager::isValidOp(1)); + auto& nm = nt::nm(); + + REQUIRE_FALSE(nm.isValidOp(1)); } TEST_CASE_METHOD(NMReset, "network") { using namespace enzo; - nt::OpId newOpId = nt::NetworkManager::addOperator(GOP_test::ctor); - nt::OpId newOpId2 = nt::NetworkManager::addOperator(GOP_test::ctor); + auto& nm = nt::nm(); - REQUIRE(nt::NetworkManager::isValidOp(newOpId)); - if(nt::NetworkManager::isValidOp(newOpId)) + nt::OpId newOpId = nm.addOperator(GOP_test::ctor); + nt::OpId newOpId2 = nm.addOperator(GOP_test::ctor); + + REQUIRE(nm.isValidOp(newOpId)); + if(nm.isValidOp(newOpId)) { auto newConnection = std::make_shared(newOpId, 1, newOpId2, 3); - auto& inputOp = nt::NetworkManager::getGeoOperator(newOpId); - auto& outputOp = nt::NetworkManager::getGeoOperator(newOpId2); + auto& inputOp = nm.getGeoOperator(newOpId); + auto& outputOp = nm.getGeoOperator(newOpId2); // set output on the upper operator outputOp.addOutputConnection(newConnection); @@ -62,14 +68,16 @@ TEST_CASE_METHOD(NMReset, "network") TEST_CASE_METHOD(NMReset, "reset") { using namespace enzo; - nt::OpId newOpId = nt::NetworkManager::addOperator(GOP_test::ctor); + auto& nm = nt::nm(); - nt::NetworkManager::_reset(); + nt::OpId newOpId = nm.addOperator(GOP_test::ctor); - REQUIRE_FALSE(nt::NetworkManager::isValidOp(newOpId)); + nm._reset(); - nt::OpId newOpId2 = nt::NetworkManager::addOperator(GOP_test::ctor); - REQUIRE(nt::NetworkManager::isValidOp(newOpId2)); + REQUIRE_FALSE(nm.isValidOp(newOpId)); + + nt::OpId newOpId2 = nm.addOperator(GOP_test::ctor); + REQUIRE(nm.isValidOp(newOpId2)); }