Skip to content

Commit 32bdd68

Browse files
Fix some obscure swizzle upgrade bugs (AcademySoftwareFoundation#2637)
Was working on upgrading a MaterialX material when it is written using UsdShade data model, and that required reviewing the current upgrade code to convert it to Python/USD. Also was working on a unit test, which forced me to put on my QA hat and start devising devious ways to fail the code. This resulted in the following two fixes for extremely rare corner cases.
1 parent 278dc95 commit 32bdd68

1 file changed

Lines changed: 45 additions & 21 deletions

File tree

source/MaterialXCore/Version.cpp

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,36 +1166,60 @@ void Document::upgradeVersion()
11661166
else if (nodeCategory == "swizzle")
11671167
{
11681168
InputPtr inInput = node->getInput("in");
1169-
InputPtr channelsInput = node->getInput("channels");
1170-
if (inInput &&
1171-
CHANNEL_COUNT_MAP.count(inInput->getType()) &&
1169+
const string sourceType = inInput ? inInput->getType() : "float";
1170+
if (CHANNEL_COUNT_MAP.count(sourceType) &&
11721171
CHANNEL_COUNT_MAP.count(node->getType()))
11731172
{
1173+
InputPtr channelsInput = node->getInput("channels");
11741174
string channelString = channelsInput ? channelsInput->getValueString() : EMPTY_STRING;
1175-
string sourceType = inInput->getType();
11761175
string destType = node->getType();
11771176
size_t sourceChannelCount = CHANNEL_COUNT_MAP.at(sourceType);
11781177
size_t destChannelCount = CHANNEL_COUNT_MAP.at(destType);
11791178

11801179
// Resolve the invalid case of having both a connection and a value
11811180
// by removing the value attribute.
1182-
if (inInput->hasValue())
1181+
if (inInput && inInput->hasValue())
11831182
{
11841183
if (inInput->hasNodeName() || inInput->hasNodeGraphString() || inInput->hasInterfaceName())
11851184
{
11861185
inInput->removeAttribute(ValueElement::VALUE_ATTRIBUTE);
11871186
}
11881187
}
11891188

1190-
if (inInput->hasValue())
1189+
// We convert to a constant node if "in" input is a constant value or does not exist:
1190+
bool convertToConstantNode = !inInput || inInput->hasValue();
1191+
// We also convert to a constant node if every destination
1192+
// channel is constant:
1193+
// eg: "ND_swizzle_color3_color3" node with
1194+
// "010" in the "channels" input.
1195+
if (!convertToConstantNode)
1196+
{
1197+
convertToConstantNode = true;
1198+
for (size_t i = 0; i < destChannelCount; i++)
1199+
{
1200+
if (i < channelString.size())
1201+
{
1202+
if (CHANNEL_CONSTANT_MAP.count(channelString[i]))
1203+
{
1204+
// Still in constant territory:
1205+
continue;
1206+
}
1207+
}
1208+
// Every other scenario: not constant
1209+
convertToConstantNode = false;
1210+
break;
1211+
}
1212+
}
1213+
1214+
if (convertToConstantNode)
11911215
{
11921216
// Replace swizzle with constant.
11931217
node->setCategory("constant");
11941218
if (node->hasNodeDefString())
11951219
{
11961220
node->setNodeDefString("ND_constant_" + node->getType());
11971221
}
1198-
string valueString = inInput->getValueString();
1222+
string valueString = inInput ? inInput->getValueString() : "0";
11991223
StringVec origValueTokens = splitString(valueString, ARRAY_VALID_SEPARATORS);
12001224
StringVec newValueTokens;
12011225
for (size_t i = 0; i < destChannelCount; i++)
@@ -1208,25 +1232,25 @@ void Document::upgradeVersion()
12081232
if (index < origValueTokens.size())
12091233
{
12101234
newValueTokens.push_back(origValueTokens[index]);
1235+
continue;
12111236
}
12121237
}
12131238
else if (CHANNEL_CONSTANT_MAP.count(channelString[i]))
12141239
{
12151240
newValueTokens.push_back(std::to_string(CHANNEL_CONSTANT_MAP.at(channelString[i])));
1241+
continue;
12161242
}
1217-
else
1218-
{
1219-
newValueTokens.push_back(origValueTokens[0]);
1220-
}
1221-
}
1222-
else
1223-
{
1224-
newValueTokens.push_back(origValueTokens[0]);
12251243
}
1244+
// Invalid channel name, or missing channel name:
1245+
newValueTokens.push_back(origValueTokens[0]);
12261246
}
12271247
InputPtr valueInput = node->addInput("value", node->getType());
12281248
valueInput->setValueString(joinStrings(newValueTokens, ", "));
1229-
node->removeInput(inInput->getName());
1249+
// This is the last place we need to check for nullptr for inInput.
1250+
if (inInput)
1251+
{
1252+
node->removeInput(inInput->getName());
1253+
}
12301254
}
12311255
else if (destChannelCount == 1)
12321256
{
@@ -1298,17 +1322,17 @@ void Document::upgradeVersion()
12981322
{
12991323
combineInInput->setConnectedNode(separateNode);
13001324
combineInInput->setOutputString(std::string("out") + channelString[i]);
1325+
continue;
13011326
}
13021327
else if (CHANNEL_CONSTANT_MAP.count(channelString[i]))
13031328
{
13041329
combineInInput->setValue(CHANNEL_CONSTANT_MAP.at(channelString[i]));
1330+
continue;
13051331
}
13061332
}
1307-
else
1308-
{
1309-
combineInInput->setConnectedNode(separateNode);
1310-
combineInInput->setOutputString(combineInInput->isColorType() ? "outr" : "outx");
1311-
}
1333+
// Invalid channel name, or missing channel name:
1334+
combineInInput->setConnectedNode(separateNode);
1335+
combineInInput->setOutputString(inInput->isColorType() ? "outr" : "outx");
13121336
}
13131337
copyInputWithBindings(node, inInput->getName(), separateNode, "in");
13141338
node->removeInput(inInput->getName());

0 commit comments

Comments
 (0)