Improvements to document validation

[ld-kerley]

This PR adds two new pieces to the correctness checking within MaterialX, these were discovered while doing work on MaterialX library files. I found them helpful, so I thought it might help to upstream the changes for others to benefit from too.

When loading libraries, there is now an optional boolean errorOnDuplicates, that will raise an error if a library contains an element is already present, previously this case would silently continue. I did wrestle with if this should just be called strict, incase there are other cases we want to add in the future. Open to input there.

Secondly I have added validation to TypedElement to validate if the type used has actually been registered in the document as a type. This very helpfully tracked down a couple of typos in work-in-progress code.

Finally I added the ability for the mxvalidate.py script to directly accept a path to alternate library locations.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ld-kerley (ce648d1, e6c6221, 30de947, 062d337)
• ✅ login: jstone-lucasfilm / name: Jonathan Stone (533a053, 1214d9c, 7b217bb, e96d237)

[kwokcb]

Hi @ld-kerley
A couple of background points:

1. This error condition did exist before and it sounds reasonable to add it back in but will defer to Jonathan.

2. There is a case to be made to allow the same libraries to be loaded in more than once and ignore duplicates. I think this came / comes up more when folks import libraries into 2 or more main documents and then try and merge them together. Also xincludes can do this -- though there is no "firewall" check to avoid duplicate xincludes.

3. There are some nodes without types. I left a note.

4. Type checking is useful to have but will fail validation if the std lib file is not loaded in which may be the case sometimes. I'm not sure what is the "best" way to handle this as types are not declared based on the stdlib file itself (data-driven). It's in C++ and also happens to be in the file. The same issue can occur for the unit typedef.

[ld-kerley]

Thanks for the feedback @kwokcb. This was certainly not posted as final work, and I wanted to get input on what I had thrown together internally to get me unstuck.

[ld-kerley]

After a brief chat with @jstone-lucasfilm I added a ValidationOptions class to pass around configuration for the validation stage, this way we can add additional validations, without changing existing behavior.

[jstone-lucasfilm]

Just on this specific build error, it looks like we'd need to add the new argument for Document::importLibrary to its JavaScript binding, so that existing calls continue to work as before:

https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/source/JsMaterialX/JsMaterialXCore/JsDocument.cpp#L26

[ld-kerley]

I took a stab at resolving the JS bindings - but I don't have a local build setup to build them, and I also don't have a lot of experience with them either - perhaps someone with more JS chops could give me some pointers (or patch this PR)

[jstone-lucasfilm]

Thanks for taking a look, @ld-kerley, and either @kwokcb or I can help to resolve that last JavaScript issue.

[jstone-lucasfilm]

@ld-kerley We've now merged development work on MaterialX 1.39 back to the main branch of MaterialX, in preparation for the final weeks of development on the 1.39.0 release. When you have a chance, could you retarget this pull request back to the main branch as well?

[jstone-lucasfilm]

@ld-kerley I'll close out this pull request for now, since it's framed in terms of a branch that we're planning to delete, but feel free to propose a similar pull request for the main branch in the future!