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

gltf image node is created with incorrect type (color3) when type is vector3. #48

Closed
kwokcb opened this issue Dec 4, 2023 · 7 comments · Fixed by #68
Closed

gltf image node is created with incorrect type (color3) when type is vector3. #48

kwokcb opened this issue Dec 4, 2023 · 7 comments · Fixed by #68
Labels
bug Something isn't working

Comments

@kwokcb
Copy link
Contributor

kwokcb commented Dec 4, 2023

Issue

A gltf image node which is of type vector3 ends up being a color3 after load for some reason.

This is an example file:

<?xml version="1.0"?>
<materialx version="1.38">
  <gltf_pbr name="SR_Motley_Patchwork_Rug_baked" type="surfaceshader" nodedef="ND_gltf_pbr_surfaceshader">
    <input name="metallic" type="float" nodename="extract_orm3" />
    <input name="roughness" type="float" nodename="extract_orm2" />
    <input name="occlusion" type="float" nodename="extract_orm" />
  </gltf_pbr>
  <surfacematerial name="MAT_SR_Motley_Patchwork_Rug_baked" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="SR_Motley_Patchwork_Rug_baked" />
  </surfacematerial>
  <gltf_image name="image_orm" type="vector3">
    <input name="file" type="filename" value="Motley_Patchwork_Rug_gltf_pbr_roughness_combined.png" />
  </gltf_image>
  <extract name="extract_orm" type="float">
    <input name="in" type="vector3" nodename="image_orm" />
    <input name="index" type="integer" value="0" />
  </extract>
  <extract name="extract_orm2" type="float">
    <input name="in" type="vector3" nodename="image_orm" />
    <input name="index" type="integer" value="1" />
  </extract>
  <extract name="extract_orm3" type="float">
    <input name="in" type="vector3" nodename="image_orm" />
    <input name="index" type="integer" value="2" />
  </extract>
</materialx>

The errors you get are:

Node interface error: Input 'in' doesn't match declaration: <extract name="extract_orm3" type="float" nodedef="ND_extract_vector3">
Node interface error: Input 'in' doesn't match declaration: <extract name="extract_orm" type="float" nodedef="ND_extract_vector3">
Node interface error: Input 'in' doesn't match declaration: <extract name="extract_orm2" type="float" nodedef="ND_extract_vector3">

This be because the gltf_image type is considered to be color3.

If you save it out you get:

<?xml version="1.0"?>
<materialx version="1.38">
  <nodegraph name="NG_main">
    <gltf_image name="image_orm" type="color3" xpos="-8.1225" ypos="0.0">
      <input name="file" type="filename" />
      <input name="factor" type="color3" value="1, 1, 1" />
      <input name="default" type="color3" value="0, 0, 0" />
      <input name="texcoord" type="vector2" value="0, 0" />
      <input name="pivot" type="vector2" value="0, 1" />
      <input name="scale" type="vector2" value="1, 1" />
      <input name="rotate" type="float" value="0" />
      <input name="offset" type="vector2" value="0, 0" />
      <input name="operationorder" type="integer" value="0" />
      <input name="uaddressmode" type="string" value="periodic" />
      <input name="vaddressmode" type="string" value="periodic" />
      <input name="filtertype" type="string" value="linear" />
    </gltf_image>
    <extract name="extract_orm" type="float" xpos="-4.3375" ypos="0.0">
      <input name="in" type="vector3" value="0, 0, 0" nodename="image_orm" />
      <input name="index" type="integer" value="0" />
    </extract>
    <extract name="extract_orm2" type="float" xpos="-4.3375" ypos="3.8000000000000003">
      <input name="in" type="vector3" value="0, 0, 0" nodename="image_orm" />
      <input name="index" type="integer" value="1" />
    </extract>
    <extract name="extract_orm3" type="float" xpos="-4.3375" ypos="1.9000000000000001">
      <input name="in" type="vector3" value="0, 0, 0" nodename="image_orm" />
      <input name="index" type="integer" value="2" />
    </extract>
    <output name="output_extract_orm_out" type="float" nodename="extract_orm" />
    <output name="output_extract_orm2_out" type="float" nodename="extract_orm2" />
    <output name="output_extract_orm3_out" type="float" nodename="extract_orm3" />
  </nodegraph>
  <gltf_pbr name="SR_Motley_Patchwork_Rug_baked" type="surfaceshader" xpos="1.1425" ypos="0.0">
    <input name="base_color" type="color3" value="1, 1, 1" />
    <input name="metallic" type="float" value="1" nodegraph="NG_main" output="output_extract_orm3_out" />
    <input name="roughness" type="float" value="1" nodegraph="NG_main" output="output_extract_orm2_out" />
    <input name="normal" type="vector3" value="0, 0, 0" />
    <input name="tangent" type="vector3" value="0, 0, 0" />
    <input name="occlusion" type="float" value="1" nodegraph="NG_main" output="output_extract_orm_out" />
    <input name="transmission" type="float" value="0" />
    <input name="specular" type="float" value="1" />
    <input name="specular_color" type="color3" value="1, 1, 1" />
    <input name="ior" type="float" value="1.5" />
    <input name="alpha" type="float" value="1" />
    <input name="alpha_mode" type="integer" value="0" />
    <input name="alpha_cutoff" type="float" value="0.5" />
    <input name="iridescence" type="float" value="0" />
    <input name="iridescence_ior" type="float" value="1.3" />
    <input name="iridescence_thickness" type="float" value="100" />
    <input name="sheen_color" type="color3" value="0, 0, 0" />
    <input name="sheen_roughness" type="float" value="0" />
    <input name="clearcoat" type="float" value="0" />
    <input name="clearcoat_roughness" type="float" value="0" />
    <input name="clearcoat_normal" type="vector3" value="0, 0, 0" />
    <input name="emissive" type="color3" value="0, 0, 0" />
    <input name="emissive_strength" type="float" value="1" />
    <input name="thickness" type="float" value="0" />
    <input name="attenuation_distance" type="float" value="0" />
    <input name="attenuation_color" type="color3" value="1, 1, 1" />
  </gltf_pbr>
  <surfacematerial name="MAT_SR_Motley_Patchwork_Rug_baked" type="material" xpos="8.1225" ypos="0.0">
    <input name="surfaceshader" type="surfaceshader" nodename="SR_Motley_Patchwork_Rug_baked" />
  </surfacematerial>
</materialx>
@manuelkoester manuelkoester added the bug Something isn't working label Dec 27, 2023
@RichardFrangenberg
Copy link
Member

@kwokcb The reason for that is that QuiltiX didn't detect the node type names from the gltf image node definitions correctly.
QuiltiX is looking at the nodef name, which is in this case: ND_gltf_image_vector3_vector3_1_0, which is quite different from other node definitions.
This pull request split the nodedef name now by underscores to find the correct type:
#68

It works for all nodes that I'm aware of, but it still feels like a dirty workaround.
Do you know a better way to detect the type from a node definition?

@kwokcb
Copy link
Contributor Author

kwokcb commented Jun 7, 2024

Hi @RichardFrangenberg ,
The safest way to do this would be to get the outputs for the nodedef. If there is more than 1 then it's type is multioutput. Otherwise get the type from the single output. Parsing the name is unreliable especially for nodedefs coming from various DCCs which can use different naming logic.

@RichardFrangenberg
Copy link
Member

Using the output type doesn't seem like a safe way too. For example there are the ND_extract_color3 and ND_extract_color4 node definitions, which outputs are of type float.

@kwokcb
Copy link
Contributor Author

kwokcb commented Jun 14, 2024

Hi @RichardFrangenberg ,

Yes, you are correct.

The more general logic is found in Node::getNodeDef()

Here it will match by:

  1. target
  2. version
  3. output type
  4. then iterate and compare inputs (either "rough" or "exact". You'd want exact matching -- which is not the default).

For 4. there still exists a problem if you don't specify enough unique inputs on the node instance. There is a logged issue for this I believe. However it is the most complete logic check that I know of in use, except for 5.

  1. The only way to guarantee to get the exact match stored is to store the nodedef name on the node instance's nodedef attribute at creation time. ( I'm unsure if this is the reason why, but as you know USD stores the nodedef name in the SDR registry and then specifies this on USDShade node instances. )

@RichardFrangenberg
Copy link
Member

With this getNodeDef() function I can get the correct definition from a mtlx node, but there is still the problem how QuiltiX should populate the type dropdown for nodes like the Extract node.
I would like to avoid having every single node definition in the tab menu like LookdevX is doing it.
Instead I would prefer to keep it as it is and have only Extract in the tab menu and a dropdown in the node settings with the node types, like Houdini is doing it.
In that case there is no mtlx node, which needs to be matched to a definition. I only have a list of definitions and I need to extract a gui friendly string for the dropdown. ND_extract_color3 or ND_gltf_image_vector3_vector3_1_0 are not really gui friendly.

@kwokcb
Copy link
Contributor Author

kwokcb commented Jun 14, 2024

For this context, yes the nodedef names in the std library weren't really intended for user facing parsing.

I also do not like exploding all variants and am interested in the solution to this.

All I can think of is to allow output type as the primary selection key and where ambiguous input type, version, target, namespace as subsequent keys. If you handle input type that should at least handle extract.

You can use getMatchingNodeDefs() as done getNodeDef() to find all the variants.


On the flip-side would you be interested in looking into a "standardized" naming scheme. I do this for new node definitions myself and reflect this in the output file name. Then there is a sequence of sub-strings that can be parsed. e.g.

NG_[<output_type>...]_[<input_type>]_<version>_<target>

(std disclaimer, I had been looking at this for LookdevX before moving on).

Yet even with this anyone can create a nodedef name which does not parse well. Thus the suggestion to parse the actual attributes on the nodedef.

@RichardFrangenberg
Copy link
Member

I changed the approach how the nodedefinitions are chosen when a mtlx file gets loaded.
It's using the getNodeDef function now, so all that logic is now handled by MaterialX itself instead of in the QuiltiX code.

The only not so clean code, which is left, is how the displayname is generated from the nodedefname. It would be great if MaterialX would provide displaynames in the future, but for the moment the current implementation in QuiltiX does the job.

I would be fine with enforcing a naming convention for nodedefs created in QuiltiX. But as you said, there might be nodedefs coming from other applications, which don't follow this convention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants