PrismPipeline / QuiltiX

QuiltiX is a graphical node editor to edit, and author MaterialX based materials of 3D assets
https://pypi.org/project/QuiltiX
Apache License 2.0
268 stars 29 forks source link

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

Closed kwokcb closed 4 months ago

kwokcb commented 11 months ago

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>
RichardFrangenberg commented 5 months ago

@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: https://github.com/PrismPipeline/QuiltiX/pull/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 commented 5 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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.