OpenUSD
OpenUSD copied to clipboard
Make usdMtlx pass usdchecker
Description of Change(s)
The USD MaterialX plugin would generate output files that would not pass usdchecker. This PR (thanks to Lee Kerley) makes the following changes:
- Top level NodeGraph is given a schema type. Otherwise it would fail the container check in usdchecker
- Authors default prim so that it can be referenced reliably
- Authors metersPerUnit and upAxis. Neither are necessary for materials, but I feel like it is good that usdcat outputs files that pass the checker to reduce confusion for people.
- [X] I have verified that all unit tests pass with the proposed changes
- [X] I have submitted a signed Contributor License Agreement
Thanks to Pablo for mentioning this existing PR for the default prim: https://github.com/PixarAnimationStudios/OpenUSD/pull/2661 Would be good to have both in because we encounter issues with people converting mtlx files only to have them fail checks.
My personal pet peeve is the code in _Context::AddShaderOutput that disregards the output name specified in the MaterialX NodeDef and uses instead USD terminology for the output port name.
This means the standard_surface node ends up with an "outputs:surface" port where it should instead have an "outputs:out" output port. It is incredibly minor and does not even affect MaterialX because that name is never used when referencing a node with a single output port, but it triggers the enhanced usdchecker I wrote that also checks that all the ports defined on nodes actually exist in the NodeDef/SdrShaderNode.
Still can't decide if I want to submit a patch to fix this in USD or make the checker code ignore this one. I can see some valid arguments on the USD side for keeping it as such since it makes sense to connect a port called "surface" to the surface output of the material.
Filed as internal issue #USD-9351
Dan Knowlton on our team caught that the unit test compared against an empty layer without metadata, so we've updated the test as well.
@JGamache-autodesk I agree with you on that. I wonder if it's too late to make the change to UsdMtlx for the output without invalidating existing tools that would be authoring the MaterialX graphs to UsdShade directly? Perhaps making usdchecker more flexible would be best in that way. Though if there's a way to do it to UsdMtlx without invalidating tooling, that would be best for parity I think
/AzurePipelines run
Azure Pipelines successfully started running 1 pipeline(s).