From e2f300ef390c588eb9ae44d2c499466c64e19808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Devernay?= Date: Sun, 4 Apr 2021 14:59:36 -0700 Subject: [PATCH] KnobSerialization: implement a better compatible solution for #568 (#598) --- Engine/Knob.cpp | 51 ++++++++++++++++++++++++++---------- Engine/Knob.h | 16 ++++++----- Engine/KnobSerialization.cpp | 13 +++++---- Engine/KnobSerialization.h | 9 ++++++- 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/Engine/Knob.cpp b/Engine/Knob.cpp index 1083d7903f..2a45dc488a 100644 --- a/Engine/Knob.cpp +++ b/Engine/Knob.cpp @@ -154,36 +154,59 @@ KnobI::slaveTo(int dimension, static KnobIPtr findMaster(const KnobIPtr & knob, const NodesList & allNodes, - const std::string& masterKnobName, + const std::string& masterNodeNameFull, const std::string& masterNodeName, const std::string& masterTrackName, + const std::string& masterKnobName, const std::map& oldNewScriptNamesMapping) { ///we need to cycle through all the nodes of the project to find the real master NodePtr masterNode; - std::string masterNodeNameToFind = masterNodeName; - qDebug() << "Link slave/master for" << knob->getName().c_str() << "restoring linkage" << masterNodeNameToFind.c_str() << '/' << masterTrackName.c_str() << '/' << masterKnobName.c_str(); + qDebug() << "Link slave/master for" << knob->getName().c_str() << "restoring linkage" << masterNodeNameFull.c_str() << '|' << masterNodeName.c_str() << '/' << masterTrackName.c_str() << '/' << masterKnobName.c_str(); /* When copy pasting, the new node copied has a script-name different from what is inside the serialization because 2 - nodes cannot co-exist with the same script-name. We keep in the map the script-names mapping + nodes cannot co-exist with the same script-name. We keep in the map the script-names mapping. + oldNewScriptNamesMapping is set in NodeGraphPrivate::pasteNode() */ - std::map::const_iterator foundMapping = oldNewScriptNamesMapping.find(masterNodeName); - - if ( foundMapping != oldNewScriptNamesMapping.end() ) { - masterNodeNameToFind = foundMapping->second; + std::string masterNodeNameToFind = masterNodeName; + std::string masterNodeNameFullToFind = masterNodeNameFull; + { + std::map::const_iterator foundMapping = oldNewScriptNamesMapping.find(masterNodeName); + if ( foundMapping != oldNewScriptNamesMapping.end() ) { + // found it! + // first, replace the last past of the full name + if (!masterNodeNameFull.empty()) { + std::size_t found = masterNodeNameFullToFind.find_last_of('.'); + if (found != std::string::npos && masterNodeNameFullToFind.substr(found+1) == masterNodeName) { + masterNodeNameFullToFind = masterNodeNameFullToFind.substr(0, found+1) + foundMapping->second; + } + } + // then update masterNodeNameToFind + masterNodeNameToFind = foundMapping->second; + } + } + // The following is probably useless: NodeGraphPrivate::pasteNode() only uses the node name, not the fully qualified name. + // Let us still do that check. + { + std::map::const_iterator foundMapping = oldNewScriptNamesMapping.find(masterNodeNameFull.empty() ? masterNodeName : masterNodeNameFull); + if ( foundMapping != oldNewScriptNamesMapping.end() ) { + masterNodeNameFullToFind = foundMapping->second; + } + } + if (masterNodeNameFullToFind.empty()) { + masterNodeNameFullToFind = masterNodeNameToFind; } - for (NodesList::const_iterator it2 = allNodes.begin(); it2 != allNodes.end(); ++it2) { qDebug() << "Trying node" << (*it2)->getScriptName().c_str() << '/' << (*it2)->getFullyQualifiedName().c_str(); - if ( (*it2)->getFullyQualifiedName() == masterNodeNameToFind ) { + if ( (*it2)->getFullyQualifiedName() == masterNodeNameFullToFind ) { masterNode = *it2; break; } } if (!masterNode) { - qDebug() << "Link slave/master for" << knob->getName().c_str() << "could not find master node" << masterNodeNameToFind.c_str() << "trying without the group name (Natron project files before 2.3.16)..."; + qDebug() << "Link slave/master for" << knob->getName().c_str() << "could not find master node" << masterNodeNameFullToFind.c_str() << "trying without the group name (Natron project files before 2.3.16)..."; for (NodesList::const_iterator it2 = allNodes.begin(); it2 != allNodes.end(); ++it2) { qDebug() << "Trying node" << (*it2)->getScriptName().c_str() << '/' << (*it2)->getFullyQualifiedName().c_str(); @@ -195,7 +218,7 @@ findMaster(const KnobIPtr & knob, } } if (!masterNode) { - qDebug() << "Link slave/master for" << knob->getName().c_str() << "could not find master node" << masterNodeNameToFind.c_str(); + qDebug() << "Link slave/master for" << knob->getName().c_str() << "could not find master node" << masterNodeNameFullToFind.c_str() << '|' << masterNodeNameToFind.c_str(); return KnobIPtr(); } @@ -227,7 +250,7 @@ findMaster(const KnobIPtr & knob, qDebug() << "Link slave/master for" << knob->getName().c_str() << "cound not find knob" << masterNodeNameToFind.c_str() << '/' << masterTrackName.c_str() << '/' << masterKnobName.c_str(); } - qDebug() << "Link slave/master for" << knob->getName().c_str() << "failed to restore linkage" << masterNodeNameToFind.c_str() << '/' << masterTrackName.c_str() << '/' << masterKnobName.c_str(); + qDebug() << "Link slave/master for" << knob->getName().c_str() << "failed to restore linkage" << masterNodeNameFullToFind.c_str() << '|' << masterNodeNameToFind.c_str() << '/' << masterTrackName.c_str() << '/' << masterKnobName.c_str(); return KnobIPtr(); } @@ -241,7 +264,7 @@ KnobI::restoreLinks(const NodesList & allNodes, KnobIPtr thisKnob = shared_from_this(); for (std::vector::const_iterator l = _links.begin(); l != _links.end(); ++l) { - KnobIPtr linkedKnob = findMaster(thisKnob, allNodes, l->knobName, l->nodeName, l->trackName, oldNewScriptNamesMapping); + KnobIPtr linkedKnob = findMaster(thisKnob, allNodes, l->nodeNameFull, l->nodeName, l->trackName, l->knobName, oldNewScriptNamesMapping); if (!linkedKnob) { if (throwOnFailure) { throw std::runtime_error(std::string("KnobI::restoreLinks(): Could not restore link to node/knob/track ") + l->nodeName + '/' + l->knobName + '/' + l->trackName); diff --git a/Engine/Knob.h b/Engine/Knob.h index d1511ef350..db9476a7ea 100644 --- a/Engine/Knob.h +++ b/Engine/Knob.h @@ -1189,12 +1189,13 @@ class KnobI bool slaveTo(int dimension, const KnobIPtr & other, int otherDimension, bool ignoreMasterPersistence = false); - void storeLink(int dimension, - const std::string& knobName, - const std::string& nodeName, - const std::string& trackName) + void storeLink(const std::string& nodeNameFull, + const std::string& nodeName, + const std::string& trackName, + const std::string& knobName, + int dimension) { - _links.push_back(Link({dimension, knobName, nodeName, trackName})); + _links.push_back(Link({nodeNameFull, nodeName, trackName, knobName, dimension})); } void restoreLinks(const NodesList & allNodes, @@ -1203,10 +1204,11 @@ class KnobI private: struct Link { - int dimension; // -1 means alias - std::string knobName; + std::string nodeNameFull; std::string nodeName; std::string trackName; + std::string knobName; + int dimension; // -1 means alias }; std::vector _links; diff --git a/Engine/KnobSerialization.cpp b/Engine/KnobSerialization.cpp index 08ed45d4d9..4c35c188e9 100644 --- a/Engine/KnobSerialization.cpp +++ b/Engine/KnobSerialization.cpp @@ -129,10 +129,12 @@ ValueSerialization::initForSave(const KnobIPtr & knob, TrackMarker* isMarker = dynamic_cast(holder); if (isMarker) { _master.masterTrackName = isMarker->getScriptName_mt_safe(); - _master.masterNodeName = isMarker->getContext()->getNode()->getFullyQualifiedName(); + _master.masterNodeNameFull = isMarker->getContext()->getNode()->getFullyQualifiedName(); + _master.masterNodeName = isMarker->getContext()->getNode()->getScriptName_mt_safe(); } else { // coverity[dead_error_line] - _master.masterNodeName = holder ? holder->getFullyQualifiedName() : ""; + _master.masterNodeNameFull = holder ? holder->getFullyQualifiedName() : ""; + _master.masterNodeName = holder ? holder->getScriptName_mt_safe() : ""; } _master.masterKnobName = m.second->getName(); } else { @@ -204,15 +206,16 @@ KnobSerialization::storeKnobLinks(const KnobIPtr & knob) * _masters can be empty for example if we expand a group: the slaved knobs are no longer slaves */ if ( !_masters.empty() ) { - const std::string& aliasKnobName = _masters.front().masterKnobName; + const std::string& aliasNodeNameFull = _masters.front().masterNodeNameFull; const std::string& aliasNodeName = _masters.front().masterNodeName; const std::string& masterTrackName = _masters.front().masterTrackName; - knob->storeLink(-1, aliasKnobName, aliasNodeName, masterTrackName); + const std::string& aliasKnobName = _masters.front().masterKnobName; + knob->storeLink(aliasNodeNameFull, aliasNodeName, masterTrackName, aliasKnobName, -1); } } else { for (std::list::iterator it = _masters.begin(); it != _masters.end(); ++it) { if (it->masterDimension != -1) { - knob->storeLink(it->masterDimension, it->masterKnobName, it->masterNodeName, it->masterTrackName); + knob->storeLink(it->masterNodeNameFull, it->masterNodeName, it->masterTrackName, it->masterKnobName, it->masterDimension); } ++i; } diff --git a/Engine/KnobSerialization.h b/Engine/KnobSerialization.h index dc82962a86..22b58b5cee 100644 --- a/Engine/KnobSerialization.h +++ b/Engine/KnobSerialization.h @@ -86,7 +86,8 @@ GCC_DIAG_UNUSED_LOCAL_TYPEDEFS_ON #define VALUE_SERIALIZATION_VERSION VALUE_SERIALIZATION_INTRODUCES_DEFAULT_VALUES #define MASTER_SERIALIZATION_INTRODUCE_MASTER_TRACK_NAME 2 -#define MASTER_SERIALIZATION_VERSION MASTER_SERIALIZATION_INTRODUCE_MASTER_TRACK_NAME +#define MASTER_SERIALIZATION_INTRODUCE_MASTER_NODE_NAME_FULL 3 +#define MASTER_SERIALIZATION_VERSION MASTER_SERIALIZATION_INTRODUCE_MASTER_NODE_NAME_FULL NATRON_NAMESPACE_ENTER @@ -94,12 +95,14 @@ struct MasterSerialization { int masterDimension; std::string masterNodeName; + std::string masterNodeNameFull; std::string masterTrackName; std::string masterKnobName; MasterSerialization() : masterDimension(-1) , masterNodeName() + , masterNodeNameFull() , masterTrackName() , masterKnobName() { @@ -111,6 +114,7 @@ struct MasterSerialization { ar & ::boost::serialization::make_nvp("MasterDimension", masterDimension); ar & ::boost::serialization::make_nvp("MasterNodeName", masterNodeName); + ar & ::boost::serialization::make_nvp("MasterNodeNameFull", masterNodeNameFull); ar & ::boost::serialization::make_nvp("MasterKnobName", masterKnobName); ar & ::boost::serialization::make_nvp("MasterTrackName", masterTrackName); } @@ -121,6 +125,9 @@ struct MasterSerialization { ar & ::boost::serialization::make_nvp("MasterDimension", masterDimension); ar & ::boost::serialization::make_nvp("MasterNodeName", masterNodeName); + if (version >= MASTER_SERIALIZATION_INTRODUCE_MASTER_NODE_NAME_FULL) { + ar & ::boost::serialization::make_nvp("MasterNodeNameFull", masterNodeNameFull); + } ar & ::boost::serialization::make_nvp("MasterKnobName", masterKnobName); if (version >= MASTER_SERIALIZATION_INTRODUCE_MASTER_TRACK_NAME) {