Skip to content

Commit

Permalink
Review updates:
Browse files Browse the repository at this point in the history 
- Separate out results into option results class argument.
- Add isAttributeEquivalent() to avoid duplication
  • Loading branch information
@kwokcb
kwokcb committed Sep 5, 2024
1 parent 59bacd8 commit a5a3627
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 106 deletions.
152 changes: 65 additions & 87 deletions source/MaterialXCore/Element.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,14 @@ bool Element::operator!=(const Element& rhs) const
return !(*this == rhs);
}

bool Element::isEquivalent(ConstElementPtr rhs, ElementEquivalenceOptions& options) const
bool Element::isEquivalent(ConstElementPtr rhs, ElementEquivalenceOptions& options,
ElementEquivalenceResult* result) const
{
if (getCategory() != rhs->getCategory() ||
getName() != rhs->getName())
{
options.status = "Name or category does not match for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'";
if (result)
result->addMessage("Name or category does not match for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'");
return false;
}

Expand Down Expand Up @@ -115,15 +117,15 @@ bool Element::isEquivalent(ConstElementPtr rhs, ElementEquivalenceOptions& optio
}
if (attributeNames != rhsAttributeNames)
{
options.status = "Attribute names differ for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'";
if (result)
result->addMessage("Attribute names differ for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'");
return false;
}

for (const string& attr : rhsAttributeNames)
{
if (getAttribute(attr) != rhs->getAttribute(attr))
if (!isAttributeEquivalent(rhs, attr, options, result))
{
options.status = "Attribute " + attr + " differs for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'";
return false;
}
}
Expand All @@ -133,17 +135,30 @@ bool Element::isEquivalent(ConstElementPtr rhs, ElementEquivalenceOptions& optio
const vector<ElementPtr>& c2 = rhs->getChildren();
if (c1.size() != c2.size())
{
options.status = "Child count differs for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'";
if (result)
result->addMessage("Child count differs for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'");
return false;
}
for (size_t i = 0; i < c1.size(); i++)
{
if (!c1[i]->isEquivalent(c2[i], options))
if (!c1[i]->isEquivalent(c2[i], options, result))
return false;
}
return true;
}

bool Element::isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName,
ElementEquivalenceOptions& /*options*/, ElementEquivalenceResult* result) const
{
if (getAttribute(attributeName) != rhs->getAttribute(attributeName))
{
if (result)
result->addMessage("Attribute " + attributeName + " differs for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'");
return false;
}
return true;
}

void Element::setName(const string& name)
{
ElementPtr parent = getParent();
Expand Down Expand Up @@ -545,102 +560,65 @@ TypeDefPtr TypedElement::getTypeDef() const
// ValueElement methods
//

bool ValueElement::isEquivalent(ConstElementPtr rhs, ElementEquivalenceOptions& options) const
{
if (getCategory() != rhs->getCategory() ||
getName() != rhs->getName())
{
options.status = "Name or category does not match for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'";
return false;
}

// Compare attribute names.
StringVec attributeNames = getAttributeNames();
StringVec rhsAttributeNames = rhs->getAttributeNames();

// Filter out any attributes specified in the options.
const StringSet& skipAttributes = options.skipAttributes;
if (!skipAttributes.empty())
bool ValueElement::isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName,
ElementEquivalenceOptions& options, ElementEquivalenceResult* result) const
{
bool perforDefaultCompare = true;

if (!options.skipValueComparisons)
{
attributeNames.erase(std::remove_if(attributeNames.begin(), attributeNames.end(),
[&skipAttributes](const string& attr) { return skipAttributes.find(attr) != skipAttributes.end(); }),
attributeNames.end());
rhsAttributeNames.erase(std::remove_if(rhsAttributeNames.begin(), rhsAttributeNames.end(),
[&skipAttributes](const string& attr) { return skipAttributes.find(attr) != skipAttributes.end(); }),
rhsAttributeNames.end());
}

// Ignore ordering if option specified.
if (options.ignoreAttributeOrder)
{
// Sort the string arrays
std::sort(attributeNames.begin(), attributeNames.end());
std::sort(rhsAttributeNames.begin(), rhsAttributeNames.end());
}
if (attributeNames != rhsAttributeNames)
{
options.status = "Attribute names differ for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'";
return false;
}
const StringSet uiAttributes =
{
ValueElement::UI_MIN_ATTRIBUTE, ValueElement::UI_MAX_ATTRIBUTE,
ValueElement::UI_SOFT_MIN_ATTRIBUTE, ValueElement::UI_SOFT_MAX_ATTRIBUTE,
ValueElement::UI_STEP_ATTRIBUTE
};

const StringSet uiAttributes = {
ValueElement::UI_MIN_ATTRIBUTE, ValueElement::UI_MAX_ATTRIBUTE,
ValueElement::UI_SOFT_MIN_ATTRIBUTE, ValueElement::UI_SOFT_MAX_ATTRIBUTE,
ValueElement::UI_STEP_ATTRIBUTE
};
// Get precision and format options
ScopedFloatFormatting fmt(options.format, options.precision);

// Get precision and format options
ScopedFloatFormatting fmt(options.format, options.precision);
ConstValueElementPtr rhsValueElement = rhs->asA<ValueElement>();

ConstValueElementPtr rhsValueElement = rhs->asA<ValueElement>();
for (const string& attr : rhsAttributeNames)
{
bool performStringCompare = true;
if (!options.skipValueComparisons)
// Check value equality
if (attributeName == ValueElement::VALUE_ATTRIBUTE)
{
// Check value equality
if (attr == ValueElement::VALUE_ATTRIBUTE)
ValuePtr thisValue = getValue();
ValuePtr rhsValue = rhsValueElement->getValue();
if (thisValue && rhsValue)
{
ValuePtr thisValue = getValue();
ValuePtr rhsValue = rhsValueElement->getValue();
if (thisValue && rhsValue)
if (thisValue->getValueString() != rhsValue->getValueString())
{
if (thisValue->getValueString() != rhsValue->getValueString())
{
options.status = "Attribute 'value' differs for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'";
return false;
}
if (result)
result->addMessage("Attribute 'value' differs for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'");
return false;
}
performStringCompare = false;
}
perforDefaultCompare = false;
}

// Check ui attribute value equality
else if (uiAttributes.find(attr) != uiAttributes.end())
// Check ui attribute value equality
else if (uiAttributes.find(attributeName) != uiAttributes.end())
{
const string& uiAttribute = getAttribute(attributeName);
const string& rhsUiAttribute = getAttribute(attributeName);
ValuePtr uiValue = !rhsUiAttribute.empty() ? Value::createValueFromStrings(uiAttribute, getType()) : nullptr;
ValuePtr rhsUiValue = !rhsUiAttribute.empty() ? Value::createValueFromStrings(rhsUiAttribute, getType()) : nullptr;
if (uiValue && rhsUiValue)
{
const string& uiAttribute = getAttribute(attr);
const string& rhsUiAttribute = getAttribute(attr);
ValuePtr uiValue = !rhsUiAttribute.empty() ? Value::createValueFromStrings(uiAttribute, getType()) : nullptr;
ValuePtr rhsUiValue = !rhsUiAttribute.empty() ? Value::createValueFromStrings(rhsUiAttribute, getType()) : nullptr;
if (uiValue && rhsUiValue)
if (uiValue->getValueString() != rhsUiValue->getValueString())
{
if (uiValue->getValueString() != rhsUiValue->getValueString())
{
options.status = "Attribute '" + attr + "' differs for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'";
return false;
}
if (result)
result->addMessage("Attribute '" + attributeName + "' differs for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'");
return false;
}
performStringCompare = false;
}
perforDefaultCompare = false;
}
}

if (performStringCompare)
{
if (getAttribute(attr) != rhsValueElement->getAttribute(attr))
{
options.status = "Attribute '" + attr + "' differs for elements: '" + getNamePath() + "', '" + rhs->getNamePath() + "'";
return false;
}
}
if (perforDefaultCompare)
{
return Element::isAttributeEquivalent(rhs, attributeName, options, result);
}

return true;
Expand Down
68 changes: 59 additions & 9 deletions source/MaterialXCore/Element.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,37 @@ using ElementMap = std::unordered_map<string, ElementPtr>;
/// A standard function taking an ElementPtr and returning a boolean.
using ElementPredicate = std::function<bool(ConstElementPtr)>;

/// @class ElemenEquivalenceResult
/// The results of comparing for equivalence.
class MX_CORE_API ElementEquivalenceResult
{
public:
ElementEquivalenceResult() = default;
~ElementEquivalenceResult() = default;

/// Append to list of equivalence messages
void addMessage(const string& message)
{
messages.push_back(message);
}

/// Clear result information
void clear()
{
messages.clear();
}

/// Get a list of equivalence messages
const StringVec& getMessages() const
{
return messages;
}

private:
/// A list of feedback messages
StringVec messages;
};

/// @class ElemenEquivalenceOptions
/// A set of options for controlling for equivalence comparison.
class MX_CORE_API ElementEquivalenceOptions
Expand All @@ -83,7 +114,6 @@ class MX_CORE_API ElementEquivalenceOptions
skipAttributes = {};
ignoreAttributeOrder = true;
skipValueComparisons = false;
status = EMPTY_STRING;
};
~ElementEquivalenceOptions() { }

Expand Down Expand Up @@ -112,9 +142,6 @@ class MX_CORE_API ElementEquivalenceOptions
/// Default is false. The operator==() method can be used instead as it always performs
/// a strict comparison. Default is false.
bool skipValueComparisons;

/// Status string for first difference found.
string status;
};

/// @class Element
Expand Down Expand Up @@ -156,10 +183,26 @@ class MX_CORE_API Element : public std::enable_shared_from_this<Element>
/// differs from this one.
bool operator!=(const Element& rhs) const;

/// Return tue if the given element treee, including all descendents,
/// Return true if the given element treee, including all descendents,
/// is considered to be equivalent to this one based on the equivalence
/// criteria provided.
virtual bool isEquivalent(ConstElementPtr rhs, ElementEquivalenceOptions& options) const;
/// @param rhs Element to compare against
/// @param options Equivalence criteria
/// @param result Results of comparison if argument is specified.
/// @return True if the elements are equivalent. False otherwise.
bool isEquivalent(ConstElementPtr rhs, ElementEquivalenceOptions& options,
ElementEquivalenceResult* result = 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 result Results of comparison if argument is specified.
/// @return True if the attribute on the elements are equivalent. False otherwise.
virtual bool isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName,
ElementEquivalenceOptions& options,
ElementEquivalenceResult* result = nullptr) const;

/// @}
/// @name Category
Expand Down Expand Up @@ -983,9 +1026,16 @@ class MX_CORE_API ValueElement : public TypedElement
/// @name Comparison interfaces
/// @{

/// Return tue if the given value element is considered to be equivalent to this
/// one based on the equivalence criteria provided.
bool isEquivalent(ConstElementPtr rhs, ElementEquivalenceOptions& options) const override;
/// 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 result Results of comparison if argument is specified.
/// @return True if the attribute on the elements are equivalent. False otherwise.
bool isAttributeEquivalent(ConstElementPtr rhs, const string& attributeName,
ElementEquivalenceOptions& options,
ElementEquivalenceResult* result = nullptr) const override;

/// @}
/// @name Value String
Expand Down
Loading

0 comments on commit a5a3627

Please sign in to comment.