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

MaterialX: Sdr can not provide node information for ND_convert_float_color3 #1629

Closed
JGamache-autodesk opened this issue Sep 22, 2021 · 6 comments

Comments

@JGamache-autodesk
Copy link
Contributor

Description of Issue

The usdMtlx Sdr parser goes thru one level of indirection to get the mx::NodeDef by asking the mx::NodeGraph or mx::Implementation for a mx::NodeDef, and sometimes the indirect call fails.

Steps to Reproduce

  1. With a MaterialX enabled build of USD, open UsdView and do the following in the interpreter window:
>>> from pxr import Sdr
>>> r = Sdr.Registry()
>>> [i for i in r.GetNodeNames() if not r.GetNodeByName(i)]
['LightFilter', 'PluginLightFilter', 'ND_disney_brdf_2012_surface', 'ND_disney_bsdf_2015_surface', 'ND_add_displacementshader', 'ND_add_volumeshader', 'ND_arrayappend_integer_integerarray', 'ND_arrayappend_integerarray_integerarray', 'ND_arrayappend_float_floatarray', 'ND_arrayappend_floatarray_floatarray', 'ND_arrayappend_color3_color3array', 'ND_arrayappend_color3array_color3array', 'ND_arrayappend_color4_color4array', 'ND_arrayappend_color4array_color4array', 'ND_arrayappend_vector2_vector2array', 'ND_arrayappend_vector2array_vector2array', 'ND_arrayappend_vector3_vector3array', 'ND_arrayappend_vector3array_vector3array', 'ND_arrayappend_vector4_vector4array', 'ND_arrayappend_vector4array_vector4array', 'ND_arrayappend_string_stringarray', 'ND_arrayappend_stringarray_stringarray', 'ND_curveadjust_float', 'ND_curveadjust_color3', 'ND_curveadjust_color4', 'ND_curveadjust_vector2', 'ND_curveadjust_vector3', 'ND_curveadjust_vector4', 'ND_mix_displacementshader', 'ND_mix_volumeshader', 'ND_convert_float_color3', 'ND_convert_float_color4', 'ND_convert_float_vector2', 'ND_convert_float_vector3', 'ND_convert_float_vector4', 'ND_convert_vector3_color3', 'ND_convert_vector4_color4', 'ND_convert_color3_vector3', 'ND_convert_color4_vector4', 'ND_convert_color3_color4', 'ND_convert_color4_color3', '11753386459202641452']

All the buggy MaterialX nodes have in common that the NodeGraph/Implementation they use is missing a link back to the NodeDef they represent.

To fix:

Go directly from Document to NodeDef in UsdMtlxParserPlugin::Parse(const NdrNodeDiscoveryResult&) using:

    auto nodeDef = document->getNodeDef(discoveryResult.identifier.GetString());
    if (!nodeDef) {
        TF_WARN("Invalid MaterialX NodeDef; unknown node name ' %s '",
            discoveryResult.identifier.GetText());
        return GetInvalidNode(discoveryResult);
    }

    ShaderBuilder builder(discoveryResult);
    ParseElement(&builder, nodeDef);

and remove the two ParseElement functions that uses const mx::ConstNodeGraphPtr& and const mx::ConstImplementationPtr& impl in parser.cpp.

System Information (OS, Hardware)

Package Versions

Build Flags

@spiffmon
Copy link
Member

Thanks for reporting, @JGamache-autodesk - would you like to submit a PR for the fix?

@JGamache-autodesk
Copy link
Contributor Author

Hi @spiffmon, I will start the process to get a PR submitted. First one from me, so will require some time to get all the details right.

@spiffmon
Copy link
Member

No worries - we'll wait for it, and thank you!

@jilliene
Copy link

Filed as internal issue #USD-6913

@pmolodo
Copy link
Contributor

pmolodo commented Mar 18, 2022

I assume this issue can be closed now?

@spiffmon
Copy link
Member

Indeed - the fix made it into 22.03 . Sorry, our automation will close the PR but not, I think the Issue, when there are both.

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

No branches or pull requests

4 participants