Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix swizzle edge cases in version upgrade #1957

Merged
Merged
23 changes: 18 additions & 5 deletions source/MaterialXCore/Version.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -992,10 +992,21 @@ void Document::upgradeVersion()
{ "color3", 3 }, { "color4", 4 },
{ "vector2", 2 }, { "vector3", 3 }, { "vector4", 4 }
};
const StringSet CHANNEL_CONVERT_PATTERNS =

// Pairs of channels string & source channel count.
// Source channel count is needed to filter out multi-channel inputs when only the first channel is being used.
const vector<std::pair<std::string, size_t>> CHANNEL_CONVERT_PATTERNS =
{
{ { "rgb", 3 }, { "rgb", 4 }, { "rgba", 4 }, { "xyz", 3 }, { "xyz", 4 }, { "xyzw", 4 }, { "rrr", 1 }, { "xxx", 1 }, { "rr", 1 }, { "xx", 1 } }
};
auto matchesConvertPattern = [&](const string& channels, size_t sourceChannelCount)
{
"rgb", "rgba", "xyz", "xyzw", "rrr", "xxx"
for (auto& pattern : CHANNEL_CONVERT_PATTERNS)
if (pattern == std::make_pair(channels, sourceChannelCount))
return true;
return false;
};

const vector<std::pair<StringSet, string>> CHANNEL_ATTRIBUTE_PATTERNS =
{
{ { "xx", "xxx", "xxxx" }, "float" },
Expand Down Expand Up @@ -1169,8 +1180,10 @@ void Document::upgradeVersion()
CHANNEL_COUNT_MAP.count(node->getType()))
{
string channelString = channelsInput ? channelsInput->getValueString() : EMPTY_STRING;
size_t sourceChannelCount = CHANNEL_COUNT_MAP.at(inInput->getType());
size_t destChannelCount = CHANNEL_COUNT_MAP.at(node->getType());
string sourceType = inInput->getType();
string destType = node->getType();
size_t sourceChannelCount = CHANNEL_COUNT_MAP.at(sourceType);
size_t destChannelCount = CHANNEL_COUNT_MAP.at(destType);

// Resolve the invalid case of having both a connection and a value
// by removing the value attribute.
Expand Down Expand Up @@ -1228,7 +1241,7 @@ void Document::upgradeVersion()
node->setInputValue("index", (int) CHANNEL_INDEX_MAP.at(channelString[0]));
}
}
else if (CHANNEL_CONVERT_PATTERNS.count(channelString))
else if (sourceType != destType && matchesConvertPattern(channelString, sourceChannelCount))
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
{
// Replace swizzle with convert.
node->setCategory("convert");
Expand Down
88 changes: 88 additions & 0 deletions source/MaterialXTest/MaterialXCore/Version.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
//
// Copyright Contributors to the MaterialX Project
// SPDX-License-Identifier: Apache-2.0
//

#include <MaterialXTest/External/Catch/catch.hpp>

#include <MaterialXCore/Util.h>
#include <MaterialXCore/Document.h>
#include <MaterialXFormat/XmlIo.h>
#include <MaterialXFormat/Util.h>

// TODO: move to format/ ?
nadult marked this conversation as resolved.
Show resolved Hide resolved

namespace mx = MaterialX;

const char* swizzleDoc1 = R"(
nadult marked this conversation as resolved.
Show resolved Hide resolved
<?xml version="1.0"?>
<materialx version="1.38">
<constant name="f" type="float">
<input name="value" type="float" value="0" />
</constant>
<constant name="v2" type="vector2">
<input name="value" type="vector2" value="0, 0" />
</constant>
<constant name="v3" type="vector3">
<input name="value" type="vector3" value="0, 0, 0" />
</constant>
<constant name="v4" type="vector4">
<input name="value" type="vector4" value="0, 0, 0, 1" />
</constant>

<swizzle name="xyz_to_xyz" type="vector3">
<input name="in" type="vector3" nodename="v3" />
<input name="channels" type="string" value="xyz" />
</swizzle>

<swizzle name="xyz_to_rgb" type="color3">
<input name="in" type="vector3" nodename="v3" />
<input name="channels" type="string" value="xyz" />
</swizzle>

<swizzle name="xy_to_xxx" type="vector3">
<input name="in" type="vector2" nodename="v2" />
<input name="channels" type="string" value="xxx" />
</swizzle>

<swizzle name="x_to_xxx" type="vector3">
<input name="in" type="float" nodename="f" />
<input name="channels" type="string" value="xxx" />
</swizzle>
</materialx>
)";

TEST_CASE("Handling 1.38 swizzle nodes", "[version]")
{
mx::DocumentPtr libs = mx::createDocument();
mx::loadLibraries({ "libraries" }, mx::getDefaultDataSearchPath(), libs);

auto doc = mx::createDocument();
doc->importLibrary(libs);
mx::readFromXmlBuffer(doc, swizzleDoc1);
doc->validate();

struct ExpectedNodes
{
std::string name;
std::string category;
std::string nodeDefName;
};

ExpectedNodes expectedNodes[] = {
{ "xyz_to_xyz", "combine3", "ND_combine3_vector3" },
{ "xyz_to_rgb", "convert", "ND_convert_vector3_color3" },
{ "xy_to_xxx", "combine3", "ND_combine3_vector3" },
{ "x_to_xxx", "convert", "ND_convert_float_vector3" },
};

for (auto& expected : expectedNodes)
{
auto node = doc->getNode(expected.name);
REQUIRE(node);
REQUIRE(node->getCategory() == expected.category);
auto nodeDef = node->getNodeDef();
REQUIRE(nodeDef);
REQUIRE(nodeDef->getName() == expected.nodeDefName);
}
}