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.

Sign up for GitHub

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

Simplify element equivalence interface #2133

Merged
merged 2 commits into from
Dec 6, 2024
Merged

Simplify element equivalence interface #2133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96bc156
Simplify element equivalence interface
jstone-lucasfilm Dec 6, 2024
7e5de0d
Update documentation
jstone-lucasfilm Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 38 additions & 30 deletions source/MaterialXCore/Element.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,6 @@ const string ValueElement::UI_ADVANCED_ATTRIBUTE = "uiadvanced";
const string ValueElement::UNIT_ATTRIBUTE = "unit";
const string ValueElement::UNITTYPE_ATTRIBUTE = "unittype";
const string ValueElement::UNIFORM_ATTRIBUTE = "uniform";
const string ElementEquivalenceResult::ATTRIBUTE = "attribute";
const string ElementEquivalenceResult::ATTRIBUTE_NAMES = "attribute names";
const string ElementEquivalenceResult::CHILD_COUNT = "child count";
const string ElementEquivalenceResult::CHILD_NAME = "child name";
const string ElementEquivalenceResult::NAME = "name";
const string ElementEquivalenceResult::CATEGORY = "category";

Element::CreatorMap Element::_creatorMap;

Expand Down Expand Up @@ -375,19 +369,22 @@ bool Element::hasInheritanceCycle() const
return false;
}

bool Element::isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& options,
ElementEquivalenceResultVec* results) const
bool Element::isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& options, string* message) const
{
if (getName() != rhs->getName())
{
if (results)
results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::NAME));
if (message)
{
*message += "Name of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n";
}
return false;
}
if (getCategory() != rhs->getCategory())
{
if (results)
results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::CATEGORY));
if (message)
{
*message += "Category of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n";
}
return false;
}

Expand All @@ -413,14 +410,16 @@ bool Element::isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions&

if (attributeNames != rhsAttributeNames)
{
if (results)
results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::ATTRIBUTE_NAMES));
if (message)
{
*message += "Attribute names of '" + getNamePath() + "' differ from '" + rhs->getNamePath() + "\n";
}
return false;
}

for (const string& attr : rhsAttributeNames)
{
if (!isAttributeEquivalent(rhs, attr, options, results))
if (!isAttributeEquivalent(rhs, attr, options, message))
{
return false;
}
Expand All @@ -447,8 +446,10 @@ bool Element::isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions&
}
if (children.size() != rhsChildren.size())
{
if (results)
results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::CHILD_COUNT));
if (message)
{
*message += "Child count of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n";
}
return false;
}
for (size_t i = 0; i < children.size(); i++)
Expand All @@ -468,26 +469,29 @@ bool Element::isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions&
rhsElement = rhs->getChild(childName);
if (!rhsElement)
{
if (results)
results->push_back(ElementEquivalenceResult(children[i]->getNamePath(), "<NONE>",
ElementEquivalenceResult::CHILD_NAME, childName));
if (message)
{
*message += "Child name `" + childName + "` of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n";
}
return false;
}
}
}
if (!children[i]->isEquivalent(rhsElement, options, results))
if (!children[i]->isEquivalent(rhsElement, options, message))
return false;
}
return true;
}

bool Element::isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName,
const ElementEquivalenceOptions& /*options*/, ElementEquivalenceResultVec* results) const
const ElementEquivalenceOptions& /*options*/, string* message) const
{
if (getAttribute(attributeName) != rhs->getAttribute(attributeName))
{
if (results)
results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::ATTRIBUTE, attributeName));
if (message)
{
*message += "Attribute name `" + attributeName + "` of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n";
}
return false;
}
return true;
Expand Down Expand Up @@ -710,7 +714,7 @@ const string& ValueElement::getActiveUnit() const
}

bool ValueElement::isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName,
const ElementEquivalenceOptions& options, ElementEquivalenceResultVec* results) const
const ElementEquivalenceOptions& options, string* message) const
{
// Perform value comparisons
bool performedValueComparison = false;
Expand All @@ -737,8 +741,10 @@ bool ValueElement::isAttributeEquivalent(ConstElementPtr rhs, const string& attr
{
if (thisValue->getValueString() != rhsValue->getValueString())
{
if (results)
results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::ATTRIBUTE, attributeName));
if (message)
{
*message += "Attribute `" + attributeName + "` of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n";
}
return false;
}
}
Expand All @@ -756,8 +762,10 @@ bool ValueElement::isAttributeEquivalent(ConstElementPtr rhs, const string& attr
{
if (uiValue->getValueString() != rhsUiValue->getValueString())
{
if (results)
results->push_back(ElementEquivalenceResult(getNamePath(), rhs->getNamePath(), ElementEquivalenceResult::ATTRIBUTE, attributeName));
if (message)
{
*message += "Attribute `" + attributeName + "` of " + getNamePath() + " differs from " + rhs->getNamePath() + "\n";
}
return false;
}
}
Expand All @@ -769,7 +777,7 @@ bool ValueElement::isAttributeEquivalent(ConstElementPtr rhs, const string& attr
// If did not peform a value comparison, perform the default comparison
if (!performedValueComparison)
{
return Element::isAttributeEquivalent(rhs, attributeName, options, results);
return Element::isAttributeEquivalent(rhs, attributeName, options, message);
}

return true;
Expand Down
45 changes: 7 additions & 38 deletions source/MaterialXCore/Element.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ using ElementMap = std::unordered_map<string, ElementPtr>;
using ElementPredicate = std::function<bool(ConstElementPtr)>;

class ElementEquivalenceOptions;
class ElementEquivalenceResult;
using ElementEquivalenceResultVec = vector<ElementEquivalenceResult>;

/// @class Element
/// The base class for MaterialX elements.
Expand Down Expand Up @@ -608,21 +606,21 @@ class MX_CORE_API Element : public std::enable_shared_from_this<Element>
/// criteria provided.
/// @param rhs Element to compare against
/// @param options Equivalence criteria
/// @param results Results of comparison if argument is specified.
/// @param message Optional text description of differences
/// @return True if the elements are equivalent. False otherwise.
bool isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& options,
ElementEquivalenceResultVec* results = nullptr) const;
bool isEquivalent(ConstElementPtr rhs, const ElementEquivalenceOptions& options,
string* message = nullptr) const;

/// Return true if the attribute on a given element is equivalent
/// based on the equivalence criteria provided.
/// @param rhs Element to compare against
/// @param attributeName Name of attribute to compare
/// @param options Equivalence criteria
/// @param results Results of comparison if argument is specified.
/// @param message Optional text description of differences
/// @return True if the attribute on the elements are equivalent. False otherwise.
virtual bool isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName,
const ElementEquivalenceOptions& options,
ElementEquivalenceResultVec* results = nullptr) const;
string* message = nullptr) const;
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved

/// @}
/// @name Traversal
Expand Down Expand Up @@ -1125,11 +1123,11 @@ class MX_CORE_API ValueElement : public TypedElement
/// @param rhs Element to compare against
/// @param attributeName Name of attribute to compare
/// @param options Equivalence criteria
/// @param results Results of comparison if argument is specified.
/// @param message Optional text description of differences
/// @return True if the attribute on the elements are equivalent. False otherwise.
bool isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName,
const ElementEquivalenceOptions& options,
ElementEquivalenceResultVec* results = nullptr) const override;
string* message = nullptr) const override;
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved

/// @}
/// @name Validation
Expand Down Expand Up @@ -1353,35 +1351,6 @@ class MX_CORE_API StringResolver
StringMap _geomNameMap;
};

/// @class ElementEquivalenceResult
/// A comparison result for the functional equivalence of two elements.
class MX_CORE_API ElementEquivalenceResult
{
public:
ElementEquivalenceResult(const string& p1, const string& p2, const string& type,
const string& attrName = EMPTY_STRING)
{
path1 = p1;
path2 = p2;
differenceType = type;
attributeName = attrName;
}
ElementEquivalenceResult() = delete;
~ElementEquivalenceResult() = default;

string path1;
string path2;
string differenceType;
string attributeName;

static const string ATTRIBUTE;
static const string ATTRIBUTE_NAMES;
static const string CHILD_COUNT;
static const string CHILD_NAME;
static const string NAME;
static const string CATEGORY;
};

/// @class ElementEquivalenceOptions
/// A set of options for comparing the functional equivalence of elements.
class MX_CORE_API ElementEquivalenceOptions
Expand Down
19 changes: 7 additions & 12 deletions source/MaterialXTest/MaterialXCore/Document.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,17 +215,16 @@ TEST_CASE("Document equivalence", "[document]")
comment->setDocString("Comment 3");

mx::ElementEquivalenceOptions options;
mx::ElementEquivalenceResultVec results;
std::string message;

// Check that this fails when not performing value comparisons
options.performValueComparisons = false;
bool equivalent = doc->isEquivalent(doc2, options, &results);
bool equivalent = doc->isEquivalent(doc2, options, &message);
REQUIRE(!equivalent);

// Check attibute values
options.performValueComparisons = true;
results.clear();
equivalent = doc->isEquivalent(doc2, options, &results);
equivalent = doc->isEquivalent(doc2, options, &message);
REQUIRE(equivalent);

unsigned int currentPrecision = mx::Value::getFloatPrecision();
Expand All @@ -236,14 +235,13 @@ TEST_CASE("Document equivalence", "[document]")
options.floatPrecision = currentPrecision;

// Check attribute filtering of inputs
results.clear();
options.attributeExclusionList = { mx::ValueElement::UI_MIN_ATTRIBUTE, mx::ValueElement::UI_MAX_ATTRIBUTE };
for (mx::InputPtr floatInput : floatInputs)
{
floatInput->setAttribute(mx::ValueElement::UI_MIN_ATTRIBUTE, "0.9");
floatInput->setAttribute(mx::ValueElement::UI_MAX_ATTRIBUTE, "100.0");
}
equivalent = doc->isEquivalent(doc2, options, &results);
equivalent = doc->isEquivalent(doc2, options, &message);
REQUIRE(equivalent);
for (mx::InputPtr floatInput : floatInputs)
{
Expand All @@ -255,12 +253,10 @@ TEST_CASE("Document equivalence", "[document]")
mx::ElementPtr mismatchElement = doc->getDescendant("mygraph/input_color4");
std::string previousName = mismatchElement->getName();
mismatchElement->setName("mismatch_color4");
results.clear();
equivalent = doc->isEquivalent(doc2, options, &results);
equivalent = doc->isEquivalent(doc2, options, &message);
REQUIRE(!equivalent);
mismatchElement->setName(previousName);
results.clear();
equivalent = doc->isEquivalent(doc2, options, &results);
equivalent = doc->isEquivalent(doc2, options, &message);
REQUIRE(equivalent);

// Check for functional nodegraphs
Expand All @@ -272,7 +268,6 @@ TEST_CASE("Document equivalence", "[document]")
REQUIRE(nodeGraph2);
doc2->addNodeDef("ND_mygraph");
nodeGraph2->setNodeDefString("ND_mygraph");
results.clear();
equivalent = doc->isEquivalent(doc2, options, &results);
equivalent = doc->isEquivalent(doc2, options, &message);
REQUIRE(!equivalent);
}
18 changes: 3 additions & 15 deletions source/PyMaterialX/PyMaterialXCore/PyElement.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ void bindPyElement(py::module& mod)
.def(py::self != py::self)
.def("isEquivalent", [](const mx::Element& elem, mx::ConstElementPtr& rhs, const mx::ElementEquivalenceOptions& options)
{
mx::ElementEquivalenceResultVec results;
bool res = elem.isEquivalent(rhs, options, &results);
return std::pair<bool, mx::ElementEquivalenceResultVec>(res, results);
std::string message;
bool res = elem.isEquivalent(rhs, options, &message);
return std::pair<bool, std::string>(res, message);
})
.def("setCategory", &mx::Element::setCategory)
.def("getCategory", &mx::Element::getCategory)
Expand Down Expand Up @@ -211,18 +211,6 @@ void bindPyElement(py::module& mod)
py::class_<mx::GenericElement, mx::GenericElementPtr, mx::Element>(mod, "GenericElement")
.def_readonly_static("CATEGORY", &mx::GenericElement::CATEGORY);

py::class_<mx::ElementEquivalenceResult>(mod, "ElementEquivalenceResult")
.def_readonly_static("ATTRIBUTE", &mx::ElementEquivalenceResult::ATTRIBUTE)
.def_readonly_static("ATTRIBUTE_NAMES", &mx::ElementEquivalenceResult::ATTRIBUTE_NAMES)
.def_readonly_static("CHILD_COUNT", &mx::ElementEquivalenceResult::CHILD_COUNT)
.def_readonly_static("CHILD_NAME", &mx::ElementEquivalenceResult::CHILD_NAME)
.def_readonly_static("NAME", &mx::ElementEquivalenceResult::NAME)
.def_readonly_static("CATEGORY", &mx::ElementEquivalenceResult::CATEGORY)
.def_readwrite("path1", &mx::ElementEquivalenceResult::path1)
.def_readwrite("path2", &mx::ElementEquivalenceResult::path2)
.def_readwrite("differenceType", &mx::ElementEquivalenceResult::differenceType)
.def_readwrite("attributeName", &mx::ElementEquivalenceResult::attributeName);

py::class_<mx::ElementEquivalenceOptions>(mod, "ElementEquivalenceOptions")
.def_readwrite("performValueComparisons", &mx::ElementEquivalenceOptions::performValueComparisons)
.def_readwrite("floatFormat", &mx::ElementEquivalenceOptions::floatFormat)
Expand Down