Skip to content

Commit

Permalink
KnobSerialization: fix deserializing multi-dim Knobs
Browse files Browse the repository at this point in the history
bug was introduced in #594
fixes #631
  • Loading branch information
devernay committed Jun 12, 2021
1 parent 83d8f07 commit 08d5e07
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 13 deletions.
5 changes: 2 additions & 3 deletions Engine/Knob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,8 @@ KnobI::restoreLinks(const NodesList & allNodes,
const std::map<std::string, std::string>& oldNewScriptNamesMapping,
bool throwOnFailure)
{
int i = 0;
KnobIPtr thisKnob = shared_from_this();
for (std::vector<Link>::const_iterator l = _links.begin(); l != _links.end(); ++l) {

KnobIPtr linkedKnob = findMaster(thisKnob, allNodes, l->nodeNameFull, l->nodeName, l->trackName, l->knobName, oldNewScriptNamesMapping);
if (!linkedKnob) {
if (throwOnFailure) {
Expand All @@ -287,7 +285,8 @@ KnobI::restoreLinks(const NodesList & allNodes,
} else if (l->dimension == -1) {
setKnobAsAliasOfThis(linkedKnob, true);
} else {
slaveTo(i, linkedKnob, l->dimension);
assert(l->slaveDimension >= 0);
slaveTo(l->slaveDimension, linkedKnob, l->dimension);
}
}
_links.clear();
Expand Down
12 changes: 8 additions & 4 deletions Engine/Knob.h
Original file line number Diff line number Diff line change
Expand Up @@ -1189,13 +1189,14 @@ class KnobI
bool slaveTo(int dimension, const KnobIPtr & other, int otherDimension, bool ignoreMasterPersistence = false);


void storeLink(const std::string& nodeNameFull,
void storeLink(int slaveDimension,
const std::string& nodeNameFull,
const std::string& nodeName,
const std::string& trackName,
const std::string& knobName,
int dimension)
{
_links.push_back(Link(nodeNameFull, nodeName, trackName, knobName, dimension));
_links.push_back(Link(slaveDimension, nodeNameFull, nodeName, trackName, knobName, dimension));
}

void restoreLinks(const NodesList & allNodes,
Expand All @@ -1204,18 +1205,21 @@ class KnobI

private:
struct Link {
int slaveDimension;
std::string nodeNameFull;
std::string nodeName;
std::string trackName;
std::string knobName;
int dimension; // -1 means alias

Link(const std::string& nodeNameFull_,
Link(int slaveDimension_,
const std::string& nodeNameFull_,
const std::string& nodeName_,
const std::string& trackName_,
const std::string& knobName_,
int dimension_)
: nodeNameFull(nodeNameFull_)
: slaveDimension(slaveDimension_)
, nodeNameFull(nodeNameFull_)
, nodeName(nodeName_)
, trackName(trackName_)
, knobName(knobName_)
Expand Down
11 changes: 5 additions & 6 deletions Engine/KnobSerialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,6 @@ KnobSerialization::createKnob(const std::string & typeName,
void
KnobSerialization::storeKnobLinks(const KnobIPtr & knob)
{
int i = 0;

if (_masterIsAlias) {
/*
* _masters can be empty for example if we expand a group: the slaved knobs are no longer slaves
Expand All @@ -210,14 +208,15 @@ KnobSerialization::storeKnobLinks(const KnobIPtr & knob)
const std::string& aliasNodeName = _masters.front().masterNodeName;
const std::string& masterTrackName = _masters.front().masterTrackName;
const std::string& aliasKnobName = _masters.front().masterKnobName;
knob->storeLink(aliasNodeNameFull, aliasNodeName, masterTrackName, aliasKnobName, -1);
knob->storeLink(-1, aliasNodeNameFull, aliasNodeName, masterTrackName, aliasKnobName, -1);
}
} else {
for (std::list<MasterSerialization>::iterator it = _masters.begin(); it != _masters.end(); ++it) {
assert(_masters.size() == _dimension);
int slaveDim = 0;
for (std::list<MasterSerialization>::iterator it = _masters.begin(); it != _masters.end(); ++it, ++slaveDim) {
if (it->masterDimension != -1) {
knob->storeLink(it->masterNodeNameFull, it->masterNodeName, it->masterTrackName, it->masterKnobName, it->masterDimension);
knob->storeLink(slaveDim, it->masterNodeNameFull, it->masterNodeName, it->masterTrackName, it->masterKnobName, it->masterDimension);
}
++i;
}
}
}
Expand Down

0 comments on commit 08d5e07

Please sign in to comment.