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

Apple: Usdmtlx updates to pass usdchecker #3243

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

ld-kerley
Copy link

Description of Change(s)

usdchecker currently fails some fairly trivial things that can be fixed when converting a MaterialX document using UsdMtlx.

  • Add layer metadata that is missing
  • Define the NodeGraphs prim as a UsdShade NodeGraph to pass the Connectable Shader test.

Also fixed an compiler warning in materialXFilter.cpp where a variable was being used before initialized.

Fixes Issue(s)

usdchecker currently reports errors with layers created from MaterialX documents.

> usdchecker pxr/usd/usdMtlx/testenv/testUsdMtlxFileFormat.testenv/baseline/GraphlessNodes.usda
Stage does not specify an upAxis. (fails 'StageMetadataChecker')
Stage does not specify its linear scale in metersPerUnit. (fails 'StageMetadataChecker')
Stage has missing or invalid defaultPrim. (fails 'StageMetadataChecker')
Connectable Shader </MaterialX/Materials/surface_material_node/NodeGraphs/redClorVal> can only have Connectable Container ancestors up to Material ancestor </MaterialX/Materials/surface_material_node>, but its parent NodeGraphs is a . (fails 'PrimEncapsulationChecker')
Connectable Shader </MaterialX/Materials/surface_material_node/NodeGraphs/blueColorVal> can only have Connectable Container ancestors up to Material ancestor </MaterialX/Materials/surface_material_node>, but its parent NodeGraphs is a . (fails 'PrimEncapsulationChecker')
Connectable Shader </MaterialX/Materials/surface_material_node/NodeGraphs/multi1> can only have Connectable Container ancestors up to Material ancestor </MaterialX/Materials/surface_material_node>, but its parent NodeGraphs is a . (fails 'PrimEncapsulationChecker')
Connectable Shader </MaterialX/Materials/surface_material_node/NodeGraphs/myDiffColor> can only have Connectable Container ancestors up to Material ancestor </MaterialX/Materials/surface_material_node>, but its parent NodeGraphs is a . (fails 'PrimEncapsulationChecker')
Connectable Shader </MaterialX/Materials/surface_material_node/NodeGraphs/mySpecColor> can only have Connectable Container ancestors up to Material ancestor </MaterialX/Materials/surface_material_node>, but its parent NodeGraphs is a . (fails 'PrimEncapsulationChecker')
Failed!
  • I have verified that all unit tests pass with the proposed changes
  • I have submitted a signed Contributor License Agreement

…n usdchecker. Fix small error in MaterialXFilter.cpp. Update and add unit tests for UsdMtlx
@jesschimein
Copy link

Filed as internal issue #USD-10009

@jesschimein
Copy link

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

(*outputs)[_Name(mtlxOutput)] = TfToken(nodeName);
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why you deleted the second clause here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly, credit to this goes to Dan Knowlton, who did the investigative work to figure this stuff out...

... So currently if there are "dangling" <output> elements at the top level of a document, then those outputs are never created, but the indirection implied by the <output> is resolved directly in the connections between the shader nodes.

In the PR, the "Nodegraphs" prim, is now created at a UsdShadeNodegraph type, which then causes UsdMtlx to create the "dangling" <output> on the "Nodegraphs" prim itself, so we no longer need this map to track the indirection of the outputs.

Thats not the easiest explanation to grok, so here's a super small contrived example....

> cat example.mtlx
<?xml version="1.0"?>
 <materialx version="1.38">
   <ramplr name="layered_layer2_gradient" type="color3" />
   <output name="o_layered_layer2_gradient" type="color3" nodename="layered_layer2_gradient" />
   <nodedef name="ND_layerShader" node="layerShader">
     <input name="weight_1" type="color3" />
     <input name="weight_2" type="color3" />
     <input name="weight_3" type="color3" />
     <output name="out" type="surfaceshader" />
   </nodedef>
   <layerShader name="layered_sr" type="surfaceshader">
     <!-- An output coming from an "output" outside of an explicit node graph. -->
     <input name="weight_2" type="color3" output="o_layered_layer2_gradient" />
   </layerShader>
   <surfacematerial name="layered" type="material">
     <input name="surfaceshader" type="surfaceshader" nodename="layered_sr" />
   </surfacematerial>
 </materialx>

In USD 24.08 if we convert this to USD we get...

> usdcat example.mtlx
#usda 1.0

def "MaterialX"
{
    def "Materials"
    {
        def Material "layered"
        {
            color3f inputs:weight_1
            color3f inputs:weight_2
            color3f inputs:weight_3
            token outputs:mtlx:surface.connect = </MaterialX/Materials/layered/ND_layerShader.outputs:surface>

            def Shader "ND_layerShader" (
                prepend references = </MaterialX/Shaders/ND_layerShader>
            )
            {
                color3f inputs:weight_1.connect = </MaterialX/Materials/layered.inputs:weight_1>
                color3f inputs:weight_2.connect = </MaterialX/Materials/layered/NodeGraphs/layered_layer2_gradient.outputs:out>
                color3f inputs:weight_3.connect = </MaterialX/Materials/layered.inputs:weight_3>
            }

            def "NodeGraphs" (
                prepend references = </MaterialX/NodeGraphs>
            )
            {
            }
        }
    }

    def "Shaders"
    {
        def Shader "ND_layerShader"
        {
            uniform token info:id = "ND_layerShader"
            token outputs:surface
        }
    }

    def "NodeGraphs"
    {
        def Shader "layered_layer2_gradient"
        {
            uniform token info:id = "ND_ramplr_color3"
            color3f outputs:out
        }
    }
}

Notice here that there is no output in the "Nodegraphs" prim, but instead the inputs:weight_2 on the ND_layerShader prim is directly connected to the layered_layer2_gradient prim inside the "Nodegraphs" prim.

With this patch applied we now get

> usdcat example.mtlx
#usda 1.0
(
    defaultPrim = "MaterialX"
    metersPerUnit = 0.01
    upAxis = "Y"
)

def "MaterialX"
{
    def "Materials"
    {
        def Material "layered"
        {
            color3f inputs:weight_1
            color3f inputs:weight_2
            color3f inputs:weight_3
            token outputs:mtlx:surface.connect = </MaterialX/Materials/layered/ND_layerShader.outputs:surface>

            def Shader "ND_layerShader" (
                prepend references = </MaterialX/Shaders/ND_layerShader>
            )
            {
                color3f inputs:weight_1.connect = </MaterialX/Materials/layered.inputs:weight_1>
                color3f inputs:weight_2.connect = </MaterialX/Materials/layered/NodeGraphs.outputs:o_layered_layer2_gradient>
                color3f inputs:weight_3.connect = </MaterialX/Materials/layered.inputs:weight_3>
            }

            def "NodeGraphs" (
                prepend references = </MaterialX/NodeGraphs>
            )
            {
            }
        }
    }

    def "Shaders"
    {
        def Shader "ND_layerShader"
        {
            uniform token info:id = "ND_layerShader"
            token outputs:surface
        }
    }

    def NodeGraph "NodeGraphs"
    {
        color3f outputs:o_layered_layer2_gradient.connect = </MaterialX/NodeGraphs/layered_layer2_gradient.outputs:out>

        def Shader "layered_layer2_gradient"
        {
            uniform token info:id = "ND_ramplr_color3"
            color3f outputs:out
        }
    }
}

Now with the change, that tags the "Nodegraphs" prim as a UsdShadeNodegraph, and thus allows connections through that prim, we add the outputs:o_layered_layer2_gradient, which itself is connected to the layered_layer2_gradient prim, and then the inputs:weight_2 connection on ND_layerShader, is just connected to the new output on the "Nodegraphs" prim.

Because we're directly creating the same output and connection structure as is represented in the MaterialX document we no longer need the additional map to maintain the indirection.

Hopefully this makes sense - sorry for being so wordy :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the great explanation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants