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

Improvements to document validation #1729

30 changes: 24 additions & 6 deletions python/Scripts/mxvalidate.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,43 @@ def main():
parser = argparse.ArgumentParser(description="Verify that the given file is a valid MaterialX document.")
parser.add_argument("--resolve", dest="resolve", action="store_true", help="Resolve inheritance and string substitutions.")
parser.add_argument("--verbose", dest="verbose", action="store_true", help="Print summary of elements found in the document.")
parser.add_argument("--stdlib", dest="stdlib", action="store_true", help="Import standard MaterialX libraries into the document.")
grp = parser.add_mutually_exclusive_group()
grp.add_argument("--stdlib", dest="stdlib", action="store_true", help="Import standard MaterialX libraries into the document.")
grp.add_argument("--path", dest="searchPath", action="store", help="")
parser.add_argument("--library", dest="libraryFolders", action="append", help="")
parser.add_argument("--errorOnDuplicates", dest="errorOnDuplicates", action="store_true", help="")
parser.add_argument(dest="inputFilename", help="Filename of the input document.")
opts = parser.parse_args()

print(opts)

doc = mx.createDocument()
try:
mx.readFromXmlFile(doc, opts.inputFilename)
except mx.ExceptionFileMissing as err:
print(err)
sys.exit(0)

if opts.stdlib:
stdlib = mx.createDocument()
if opts.stdlib or opts.searchPath:
libDoc = mx.createDocument()
try:
mx.loadLibraries(mx.getDefaultDataLibraryFolders(), mx.getDefaultDataSearchPath(), stdlib)
except Exception as err:
if opts.stdlib:
dataSearchPath = mx.getDefaultDataSearchPath()
else:
dataSearchPath = mx.FileSearchPath(opts.searchPath, ";")

if opts.libraryFolders:
dataLibaryFolders = opt.libraryFolders
else:
dataLibaryFolders = mx.getDefaultDataLibraryFolders()

print(f"Loading Libraries\nSearch Path: {dataSearchPath.asString()}\nFolders : {dataLibaryFolders}")

mx.loadLibraries(dataLibaryFolders, dataSearchPath, libDoc, errorOnDuplicates=opts.errorOnDuplicates)
except err:
print(err)
sys.exit(0)
doc.importLibrary(stdlib)
doc.importLibrary(libDoc, errorOnDuplicates=opts.errorOnDuplicates)

(valid, message) = doc.validate()
if (valid):
Expand Down
2 changes: 1 addition & 1 deletion source/JsMaterialX/JsMaterialXCore/JsDocument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ EMSCRIPTEN_BINDINGS(document)
.class_function("createDocument", &mx::Document::createDocument<mx::Document>)
.function("initialize", &mx::Document::initialize)
.function("copy", &mx::Document::copy)
.function("importLibrary", &mx::Document::importLibrary)
BIND_MEMBER_FUNC("importLibrary", mx::Document, importLibrary, 1, 2, const mx::ConstDocumentPtr&, bool)
.function("getReferencedSourceUris", ems::optional_override([](mx::Document &self) {
mx::StringSet set = self.getReferencedSourceUris();
return ems::val::array(set.begin(), set.end());
Expand Down
8 changes: 4 additions & 4 deletions source/MaterialXCore/Definition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ InterfaceElementPtr NodeDef::getImplementation(const string& target) const
return InterfaceElementPtr();
}

bool NodeDef::validate(string* message) const
bool NodeDef::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;
validateRequire(!hasType(), res, message, "Nodedef should not have a type but an explicit output");
return InterfaceElement::validate(message) && res;
return InterfaceElement::validate(message, validationOptions) && res;
}

bool NodeDef::isVersionCompatible(const string& version) const
Expand Down Expand Up @@ -146,11 +146,11 @@ NodeDefPtr Implementation::getNodeDef() const
return resolveNameReference<NodeDef>(getNodeDefString());
}

bool Implementation::validate(string* message) const
bool Implementation::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;
validateRequire(!hasVersionString(), res, message, "Implementation elements do not support version strings");
return InterfaceElement::validate(message) && res;
return InterfaceElement::validate(message, validationOptions) && res;
}

ConstInterfaceElementPtr Implementation::getDeclaration(const string&) const
Expand Down
4 changes: 2 additions & 2 deletions source/MaterialXCore/Definition.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class MX_CORE_API NodeDef : public InterfaceElement

/// Validate that the given element tree, including all descendants, is
/// consistent with the MaterialX specification.
bool validate(string* message = nullptr) const override;
bool validate(string* message = nullptr, const ValidationOptions* validationOptions = nullptr) const override;

/// @}
/// @name Utility
Expand Down Expand Up @@ -280,7 +280,7 @@ class MX_CORE_API Implementation : public InterfaceElement

/// Validate that the given element tree, including all descendants, is
/// consistent with the MaterialX specification.
bool validate(string* message = nullptr) const override;
bool validate(string* message = nullptr, const ValidationOptions* validationOptions = nullptr) const override;

/// @}
/// @name Utility
Expand Down
8 changes: 5 additions & 3 deletions source/MaterialXCore/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ NodeDefPtr Document::addNodeDefFromGraph(const NodeGraphPtr nodeGraph, const str
return nodeDef;
}

void Document::importLibrary(const ConstDocumentPtr& library)
void Document::importLibrary(const ConstDocumentPtr& library, bool errorOnDuplicates)
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
{
if (!library)
{
Expand All @@ -213,6 +213,8 @@ void Document::importLibrary(const ConstDocumentPtr& library)
ConstElementPtr previous = getChild(childName);
if (previous)
{
if (errorOnDuplicates)
throw Exception("Trying to import a child that already exists " + child->getName());
continue;
}

Expand Down Expand Up @@ -353,13 +355,13 @@ vector<InterfaceElementPtr> Document::getMatchingImplementations(const string& n
}
}

bool Document::validate(string* message) const
bool Document::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;
std::pair<int, int> expectedVersion(MATERIALX_MAJOR_VERSION, MATERIALX_MINOR_VERSION);
validateRequire(getVersionIntegers() >= expectedVersion, res, message, "Unsupported document version");
validateRequire(getVersionIntegers() <= expectedVersion, res, message, "Future document version");
return GraphElement::validate(message) && res;
return GraphElement::validate(message, validationOptions) && res;
}

void Document::invalidateCache()
Expand Down
7 changes: 5 additions & 2 deletions source/MaterialXCore/Document.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ class MX_CORE_API Document : public GraphElement
/// The contents of the library document are copied into this one, and
/// are assigned the source URI of the library.
/// @param library The library document to be imported.
void importLibrary(const ConstDocumentPtr& library);
/// @param errorOnDuplicates If true then an Exception will be raised if elements have duplicate names
void importLibrary(const ConstDocumentPtr& library, bool errorOnDuplicates=false);

/// Get a list of source URI's referenced by the document
StringSet getReferencedSourceUris() const;
Expand Down Expand Up @@ -657,8 +658,10 @@ class MX_CORE_API Document : public GraphElement
/// specification.
/// @param message An optional output string, to which a description of
/// each error will be appended.
/// @param validationOptions An optional pointer to a object that holds
/// options to configure the validation process
/// @return True if the document passes all tests, false otherwise.
bool validate(string* message = nullptr) const override;
bool validate(string* message = nullptr, const ValidationOptions* validationOptions = nullptr) const override;

/// @}
/// @name Utility
Expand Down
37 changes: 33 additions & 4 deletions source/MaterialXCore/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ void Element::clearContent()
_childOrder.clear();
}

bool Element::validate(string* message) const
bool Element::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;
validateRequire(isValidName(getName()), res, message, "Invalid element name");
Expand All @@ -404,7 +404,7 @@ bool Element::validate(string* message) const
}
for (auto child : getChildren())
{
res = child->validate(message) && res;
res = child->validate(message, validationOptions) && res;
}
validateRequire(!hasInheritanceCycle(), res, message, "Cycle in element inheritance chain");
return res;
Expand Down Expand Up @@ -476,6 +476,35 @@ TypeDefPtr TypedElement::getTypeDef() const
return resolveNameReference<TypeDef>(getType());
}

bool TypedElement::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;

if (validationOptions && validationOptions->typesDefined)
{
if (hasType())
{
// check that the type specified is declared as a type in the document
const auto& typeStr = getType();
bool isValidType = getDocument()->getTypeDef(typeStr) != nullptr;

if (!isValidType)
{
// there is one special type "multioutput" that can be used on node
// elements when they have more than one output port.
if (typeStr == "multioutput" && isA<Node>())
{
isValidType = true;
}
}

validateRequire(isValidType, res, message, "Element type is not defined '" + getType() + "'");
}
}

return Element::validate(message, validationOptions) && res;
}

//
// ValueElement methods
//
Expand Down Expand Up @@ -532,7 +561,7 @@ const string& ValueElement::getActiveUnit() const
return EMPTY_STRING;
}

bool ValueElement::validate(string* message) const
bool ValueElement::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;
if (hasType() && hasValueString())
Expand Down Expand Up @@ -583,7 +612,7 @@ bool ValueElement::validate(string* message) const
}
validateRequire(foundUnit, res, message, "Unit definition does not exist in document");
}
return TypedElement::validate(message) && res;
return TypedElement::validate(message, validationOptions) && res;
}

//
Expand Down
12 changes: 10 additions & 2 deletions source/MaterialXCore/Element.h
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,7 @@ class MX_CORE_API Element : public std::enable_shared_from_this<Element>

/// Validate that the given element tree, including all descendants, is
/// consistent with the MaterialX specification.
virtual bool validate(string* message = nullptr) const;
virtual bool validate(string* message = nullptr, const ValidationOptions* readOptions = nullptr) const;

/// @}
/// @name Utility
Expand Down Expand Up @@ -905,6 +905,14 @@ class MX_CORE_API TypedElement : public Element
TypeDefPtr getTypeDef() const;

/// @}
/// @name Validation
/// @{

/// Validate that the given element is
/// consistent with the MaterialX specification.
bool validate(string* message = nullptr, const ValidationOptions* readOptions = nullptr) const override;

/// @}

public:
static const string TYPE_ATTRIBUTE;
Expand Down Expand Up @@ -1118,7 +1126,7 @@ class MX_CORE_API ValueElement : public TypedElement

/// Validate that the given element tree, including all descendants, is
/// consistent with the MaterialX specification.
bool validate(string* message = nullptr) const override;
bool validate(string* message = nullptr, const ValidationOptions* readOptions = nullptr) const override;

/// @}

Expand Down
8 changes: 4 additions & 4 deletions source/MaterialXCore/Geom.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ CollectionPtr GeomElement::getCollection() const
return resolveNameReference<Collection>(getCollectionString());
}

bool GeomElement::validate(string* message) const
bool GeomElement::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;
if (hasCollectionString())
{
validateRequire(getCollection() != nullptr, res, message, "Invalid collection string");
}
return Element::validate(message) && res;
return Element::validate(message, validationOptions) && res;
}

//
Expand Down Expand Up @@ -171,11 +171,11 @@ bool Collection::matchesGeomString(const string& geom) const
return false;
}

bool Collection::validate(string* message) const
bool Collection::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;
validateRequire(!hasIncludeCycle(), res, message, "Cycle in collection include chain");
return Element::validate(message) && res;
return Element::validate(message, validationOptions) && res;
}

MATERIALX_NAMESPACE_END
4 changes: 2 additions & 2 deletions source/MaterialXCore/Geom.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ class MX_CORE_API GeomElement : public Element

/// Validate that the given element tree, including all descendants, is
/// consistent with the MaterialX specification.
bool validate(string* message = nullptr) const override;
bool validate(string* message = nullptr, const ValidationOptions* readOptions = nullptr) const override;

/// @}

Expand Down Expand Up @@ -554,7 +554,7 @@ class MX_CORE_API Collection : public Element

/// Validate that the given element tree, including all descendants, is
/// consistent with the MaterialX specification.
bool validate(string* message = nullptr) const override;
bool validate(string* message = nullptr, const ValidationOptions* readOptions = nullptr) const override;

/// @}

Expand Down
12 changes: 6 additions & 6 deletions source/MaterialXCore/Interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ OutputPtr PortElement::getConnectedOutput() const
return result;
}

bool PortElement::validate(string* message) const
bool PortElement::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;

Expand Down Expand Up @@ -195,7 +195,7 @@ bool PortElement::validate(string* message) const
validateRequire(getType() == connectedNode->getType(), res, message, "Mismatched types in port connection");
}
}
return ValueElement::validate(message) && res;
return ValueElement::validate(message, validationOptions) && res;
}

//
Expand Down Expand Up @@ -259,7 +259,7 @@ GeomPropDefPtr Input::getDefaultGeomProp() const
return nullptr;
}

bool Input::validate(string* message) const
bool Input::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;
ConstElementPtr parent = getParent();
Expand All @@ -279,7 +279,7 @@ bool Input::validate(string* message) const
{
validateRequire(parent->asA<NodeGraph>()->getNodeDef() == nullptr, res, message, "Input element in a functional nodegraph has no effect");
}
return PortElement::validate(message) && res;
return PortElement::validate(message, validationOptions) && res;
}

//
Expand Down Expand Up @@ -309,11 +309,11 @@ bool Output::hasUpstreamCycle() const
return false;
}

bool Output::validate(string* message) const
bool Output::validate(string* message, const ValidationOptions* validationOptions) const
{
bool res = true;
validateRequire(!hasUpstreamCycle(), res, message, "Cycle in upstream path");
return PortElement::validate(message) && res;
return PortElement::validate(message, validationOptions) && res;
}

//
Expand Down
6 changes: 3 additions & 3 deletions source/MaterialXCore/Interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ class MX_CORE_API PortElement : public ValueElement

/// Validate that the given element tree, including all descendants, is
/// consistent with the MaterialX specification.
bool validate(string* message = nullptr) const override;
bool validate(string* message = nullptr, const ValidationOptions* readOptions = nullptr) const override;

/// @}

Expand Down Expand Up @@ -221,7 +221,7 @@ class MX_CORE_API Input : public PortElement

/// Validate that the given element tree, including all descendants, is
/// consistent with the MaterialX specification.
bool validate(string* message = nullptr) const override;
bool validate(string* message = nullptr, const ValidationOptions* readOptions = nullptr) const override;

/// @}

Expand Down Expand Up @@ -264,7 +264,7 @@ class MX_CORE_API Output : public PortElement

/// Validate that the given element tree, including all descendants, is
/// consistent with the MaterialX specification.
bool validate(string* message = nullptr) const override;
bool validate(string* message = nullptr, const ValidationOptions* readOptions = nullptr) const override;

/// @}

Expand Down
Loading