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

Improvements to publishing API #1403

Merged

Conversation

kwokcb
Copy link
Contributor

@kwokcb kwokcb commented Jul 11, 2023

API Logic Updates

This s a set of fixes to the existing "publishing" / interface API to fix logic errors and conform properly with the current specification.

<nodedef> Updates

  • The logic for creating <nodedefs> has logic errors in it so adding this PR to fix it up and to make sure it matches the current specification. Of note was that inputs were not being removed from functional graphs, inputs were not being published, and attributes which are disallowed on functional graphs or nodedefs were not being filtered out.

<nodegraph> Updates

  • Publishing interfaces is missing compound <nodegraph> (vs functional) support so add it in so input interfaces can be "published" from nodes for both compound and functional graphs.

General

  • Undesired attributes on are now filtered out properly on create. his includes handling positional information (xpos, ypos) which is generated by the MaterialXNodeEditor and sourceURI.
  • Fix so that add / remove interface transfers values from interfaces from and to interior inputs properly when adding and removing interfacename.

Test

  • Unit test has been simplified and update
  • Sample of possible nodedef creation with fixes:
<?xml version="1.0"?>
<materialx version="1.38">
  <!--
	Node: <mymarble> 
	Custom Marble
  -->
  <nodedef name="ND_mymarble_1_0_color3" node="mymarble" nodegroup="texture2d" version="1.0" isdefaultversion="true" doc="This is a custom marble definition" namespace="myspace">
    <input name="base_color_1" type="color3" value="0.8, 0.8, 0.8" uiname="Color 1" uifolder="Marble Color" />
    <input name="base_color_2" type="color3" value="0.1, 0.1, 0.3" uiname="Color 2" uifolder="Marble Color" />
    <input name="noise_scale_1" type="float" value="6.0" uisoftmin="1.0" uisoftmax="10.0" uiname="Scale 1" uifolder="Marble Noise" />
    <input name="noise_scale_2" type="float" value="4.0" uisoftmin="1.0" uisoftmax="10.0" uiname="Scale 2" uifolder="Marble Noise" />
    <input name="noise_power" type="float" value="3.0" uisoftmin="1.0" uisoftmax="10.0" uiname="Power" uifolder="Marble Noise" />
    <input name="noise_octaves" type="integer" value="3" uisoftmin="1" uisoftmax="8" uiname="Octaves" uifolder="Marble Noise" />
    <output name="out" type="color3" />
  </nodedef>
</materialx>

and

<?xml version="1.0"?>
<materialx version="1.38">
  <!--
	Node: <mymarble> 
	Custom Marble
  -->
  <nodegraph name="NG_mymarble_1_0_color3" nodedef="ND_mymarble_1_0_color3" namespace="myspace">
    <position name="obj_pos" type="vector3" />
    <dotproduct name="add_xyz" type="float">
      <input name="in1" type="vector3" nodename="obj_pos" />
      <input name="in2" type="vector3" value="1, 1, 1" />
    </dotproduct>
    <multiply name="scale_xyz" type="float">
      <input name="in1" type="float" nodename="add_xyz" />
      <input name="in2" type="float" interfacename="noise_scale_1" />
    </multiply>
    <multiply name="scale_pos" type="vector3">
      <input name="in1" type="vector3" nodename="obj_pos" />
      <input name="in2" type="float" interfacename="noise_scale_2" />
    </multiply>
    <fractal3d name="noise" type="float">
      <input name="octaves" type="integer" interfacename="noise_octaves" />
      <input name="position" type="vector3" nodename="scale_pos" />
    </fractal3d>
    <multiply name="scale_noise" type="float">
      <input name="in1" type="float" nodename="noise" />
      <input name="in2" type="float" value="3.0" />
    </multiply>
    <add name="sum" type="float">
      <input name="in1" type="float" nodename="scale_xyz" />
      <input name="in2" type="float" nodename="scale_noise" />
    </add>
    <sin name="sin" type="float">
      <input name="in" type="float" nodename="sum" />
    </sin>
    <multiply name="scale" type="float">
      <input name="in1" type="float" nodename="sin" />
      <input name="in2" type="float" value="0.5" />
    </multiply>
    <add name="bias" type="float">
      <input name="in1" type="float" nodename="scale" />
      <input name="in2" type="float" value="0.5" />
    </add>
    <power name="power" type="float">
      <input name="in1" type="float" nodename="bias" />
      <input name="in2" type="float" interfacename="noise_power" />
    </power>
    <mix name="color_mix" type="color3">
      <input name="bg" type="color3" interfacename="base_color_1" />
      <input name="fg" type="color3" interfacename="base_color_2" />
      <input name="mix" type="float" nodename="power" />
    </mix>
    <output name="out" type="color3" nodename="color_mix" />
  </nodegraph>
</materialx>

source/MaterialXCore/Document.cpp Show resolved Hide resolved
source/MaterialXCore/Node.cpp Show resolved Hide resolved
python/Scripts/createdefinition.py Outdated Show resolved Hide resolved
python/Scripts/createdefinition.py Outdated Show resolved Hide resolved
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, @kwokcb, and I had a few additional thoughts on the API that we're presenting to the user.

source/MaterialXCore/Document.h Outdated Show resolved Hide resolved
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

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

This proposal is coming along well, @kwokcb, and it looks like we just need to update JavaScript bindings to reflect the latest API signatures.

@kwokcb
Copy link
Contributor Author

kwokcb commented Sep 4, 2023

Missed this. Put in a fix.

@jstone-lucasfilm
Copy link
Member

@kwokcb Just bumping the thread above, which I believe is the last issue that needs to be resolved:

#1403 (comment)

Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

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

This looks very close to the mark, and I had just one question about the JavaScript binding below.

source/JsMaterialX/JsMaterialXCore/JsDocument.cpp Outdated Show resolved Hide resolved
@jstone-lucasfilm jstone-lucasfilm changed the title Node Definition Creation Logic Fixes Improvements to publishing API May 28, 2024
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

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

This looks ready to merge, thanks @kwokcb!

@jstone-lucasfilm jstone-lucasfilm merged commit adcc5e9 into AcademySoftwareFoundation:dev_1.39 May 28, 2024
1 check passed
@kwokcb kwokcb deleted the nodedef_creation_api branch July 11, 2024 12:53
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.

2 participants