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

Proposal : Named Values #2149

Open
ld-kerley opened this issue Dec 16, 2024 · 4 comments
Open

Proposal : Named Values #2149

ld-kerley opened this issue Dec 16, 2024 · 4 comments

Comments

@ld-kerley
Copy link
Contributor

(Related to Generic Type System proposal.)

Problem

Current XML based system for describing node definitions and nodegraphs requires each specialization of the node to be spelled out explicitly (e.g. ND_add_float, ND_add_color3, ND_add_vector2, ...). This leads to a verbose data library, and encourages copy/paste authoring which can be prone to errors.

As well as the variation of types, each different type has different value strings that need to be authored.

Solutions

The possible solutions suggested in the Generic Type System proposal also needs a way to define values that correspond to the type. Most of the default values in the MaterialX Data Library are really just zero or one. So in this proposal we suggest additional syntax to describe the concrete named values indirectly at their usage site.

The <typedef> element should be extended with additional attributes to hold the concrete values for the named values.

The value attribute then uses a special token to identify the new named value syntax, including a label identifying which of the available named values to use.

The exact selection for these tokens and the list of provided named values is a subject for further debate, but for the sake of example, we're proposing the value attribute use the prefix TypeValue: to identify this new named value behavior. This proposal also recommends as a minimum zero and one always be provided as concrete named values.

Example

Note here we intentionally don't take any opinion on any generic type mechanism, and instead make the named value proposal in isolation. This could be implemented even if none of the generic type suggestions above are actioned.

<!-- type definitions are extended with their own concrete values for each of the named values -->
<typedef name="boolean" zero="False" one="True"/>
<typedef name="float" zero="0.0" one="1.0"/>
<typedef name="color3" zero="0.0, 0.0, 0.0" one="1.0, 1.0, 1.0"/>
<typedef name="vector3" zero="0.0, 0.0, 0.0" one="1.0, 1.0, 1.0"/>

<!-- original node definition -->
<nodedef name="ND_multiply_float" node="multiply" nodegroup="math">
  <input name="in1" type="float" value="0.0" />
  <input name="in2" type="float" value="1.0" />
  <output name="out" type="float" defaultinput="in1" />
</nodedef>

<!-- new node definition -->
<nodedef name="ND_multiply_float" node="multiply" nodegroup="math">
  <input name="in1" type="float" value="TypeValue:zero" />
  <input name="in2" type="float" value="TypeValue:one" />
  <output name="out" type="float" defaultinput="in1" />
</nodedef>

We use the prefix TypeValue: for a value to indicate that a named value is being used, the suffix (here one and zero) is then used along with the corresponding defined type for the value.

Evaluation

As with the Generic Type System proposal, there are a number of different ways we could decide to evaluate this.

Build time only

We could process the data library files at build time, and replace the TypeValue:<name> tokens with the concrete values. This would have the advantage of not requiring any downstream integrations change their interactions with the data library files.

Runtime evaluation only

We could install the files as-is and extend methods like ValueElement::getValueString() to look up the concrete values at runtime. This has the advantage of potentially allowing for smaller MaterialX installations

Build and/or runtime

Finally we could provide a build time configuration that would allow either build time or runtime evaluation of the named values.

ld-kerley added a commit to ld-kerley/MaterialX that referenced this issue Dec 17, 2024
…#2149).

Add 'zero' and 'one' named values to some basic types, and use those named values in some of the basic arithmetic nodes as an example.  Add build time tool to process and write out resolved values, and add support for run-time evaluation - switchable by setting MATERIALX_BUILD_BAKE_NAMED_VALUES cmake flag
@kwokcb
Copy link
Contributor

kwokcb commented Dec 19, 2024

Hi @ld-kerley

I like the idea of extending typedef to have additional information.
Two things come to mind initially:

  1. Separate out specific enumerations or sematics for typedef as is done with unitdef as child elements. I think this makes it more flexible long term. So you'd have this instead where we introduce a "constant definition" constantdef:
<typedef name="vector3">
  <constantdef name="zero" value="0.0, 0.0, 0.0"/>
  <constantdef name="one" value="0.0, 0.0, 0.0"/>
</typedef>
 One thing that just happened to pop up is I wanted a constant for PI. So I could do this by adding:
<typedef name="float>
  <constantdef name="zero" value="0.0"/>
  <constantdef name="one" value="0.0"/>
  <constantdef name="PI" value="3.14....etc"/>  
</typedef>
  1. Do not overload the semantics on the input ``value attribute but introduce new metadata so it can be added
    incrementally. For example, say the new meta-data is called constantvalue. My thinking is this is similar to geompropvalue (*)
<nodedef name="ND_multiply_float" node="multiply" nodegroup="math">
 <input name="in1" type="float" constantvalue="zero" />
 <input name="in2" type="float" constantvalue="one" />
 <output name="out" type="float" defaultinput="in1" />
</nodedef>

If you go with the convention (*) here then you end up with something like this:

<constantdef name="one" type="float" value="1.0" />
<constantdef name="zero" type="float" value="0.0" />
<constantdef name="PI" type="float" value="3.1415" />

I'm less in favour of this since it separates out the enumeration of constants from the type definition but is more flexible to for any user to add in constants as needed.

@ld-kerley
Copy link
Contributor Author

  1. I really like the <constantdef> idea here - and see the strong value add to being able to define other constants with names. It's a bit more of an invasive syntax change, but it does feel like a good direction to me.

  2. It's less clear to me if the constantvalue attribute is a win or not here - I imagine it would lead to more code to handle the cases where value and constantvalue are set? which one should win here? Is it confusing to read something like

 <input name="in2" type="float" constantvalue="one" value="2.3"/>

To my mind TypeValue:zero really is just a synonym for the real value string for the given type. But it really has the same meaning and use.

Maybe you can elaborate a little on what you see as the win to using constantvalue here - maybe I'm missing something?

@kwokcb
Copy link
Contributor

kwokcb commented Dec 19, 2024

For the second, I think if you go with explicit constant elements then I think the biggest thing from a user perspective is overloading the meaning of the value attribute from something that currently a resolved value to also possibly be a reference to a value.

I think internally this could slow things down a little to more string parsing / generation logic for all instances of string<->runtime type conversion.

Agreed it's already a bit of a pain to avoid multiple conflicting attributes but maybe additively add something like a valuref (so that value thus remains what it currently is) would allow for a more incremental adoption without "breaking" parsing of what's currently there?

@ld-kerley
Copy link
Contributor Author

Yeah I see what you're saying - though I think my hope with this proposal is that the incremental adoption is offered by way of the build time resolution of the named values. So it would be possible to generate a build that didn't have any named values in.

I guess the other thing I didn't lay out concretely - but maybe it's worth saying. The goal of this is to simplify the standard data library, so I wasn't really imagining exposing options to allow users to author these named values in their documents. I think we could get to that point, but perhaps only once all the MaterialX stakeholders have adopted the runtime approach for this - which will take some time.

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

2 participants