Specification proposal for template elements

[jstone-lucasfilm]

This changelist proposes the addition of template elements to the MaterialX specification, along with an extension of token elements in the context of a typedef.

Accompanying the new specification language is a proposed implementation, outlining a concrete approach that automatically expands templates within existing MaterialX API calls, without impacting downstream clients such as OpenUSD.

[ld-kerley]

As I'm reading this - OpenUSD would be required to call Document::expandTemplates(). So it's not quite true to say that downstream clients require no changes?

[jstone-lucasfilm]

I'm imagining that Document::expandTemplates would be handled exactly like the existing Document::upgradeVersion, where it's automatically invoked during the load process, and never needs to be called directly by the client.

[ld-kerley]

That's not how I read what you've written above - maybe it's worth making that idea more explicit in the proposed text.

[jstone-lucasfilm]

Good idea, and I'll add a sentence to state this clearly in the proposal, since it's an important detail.

[ld-kerley]

The type specification here feels unnecessary, given the scope of the element.

[jstone-lucasfilm]

That's a completely fair point, and I mainly added this to be consistent with the concept of "typed values" in MaterialX, where the element itself has enough information for an accessor to interpret its combined type and value attributes as a typed value in the engine.

It's certainly not a requirement here, but it seems consistent with how typed values are handled elsewhere in the MaterialX specification and API.

[ld-kerley]

Are any of those other typed value use-cases encapsulated in a container element that concretely defines the type? if they are, then I think maybe there is precedent enough to retain this, otherwise I feel we should remove it for both brevity and lack of potential error/confusion.

[jstone-lucasfilm]

Although I see the perspective that you're outlining, it's worth noting that there are no typed MaterialX elements that state a value attribute without also stating the type attribute that clarifies its interpretation. This is what allows the API calls in TypedElement to be shared across a wide set of Element subclasses, since they all build upon the same expression of a typed value.

From that perspective, it seems more robust to state both value and type within the token element itself, rather than making an exception when the parent of the element has a type-like name.

[ld-kerley]

I'll note that in my PR contrast could be represented in an even more compact form.

  <template name="TP_ND_contrastFA" varnames="nodeDefExt,floatTypeName,typeList" options="(@typeName@,@typeName@FA), (@typeName@,float), ('float, color3, color4, vector2, vector3, vector4', 'color3, color4, vector2, vector3, vector4')">
    <template name="TP_ND_contrast_@nodeDefExt@" varnames="typeName" options="(@typeList@)">
      <nodedef name="ND_contrast_@nodeDefExt@" node="contrast" nodegroup="adjustment">
        <input name="in" type="@typeName@" value="Constant:zero" spec_desc="The input color stream to be adjusted" />
        <input name="amount" type="@floatTypeName@" value="Constant:one" spec_desc="Slope multiplier for contrast adjustment. Values greater than 1.0 increase contrast, values between 0.0 and 1.0 reduce contrast." spec_acceptedvalues="[__zero__, __+inf__)" />
        <input name="pivot" type="@floatTypeName@" value="Constant:half" spec_desc="Center pivot value of contrast adjustment; this is the value that will not change as contrast is adjusted. " />
        <output name="out" type="@typeName@" defaultinput="in" spec_desc="the adjusted color value" />
      </nodedef>
    </template>
  </template>

I haven't templatized the nodegraph elements yet - but I believe they could be similarly simplified.

[jstone-lucasfilm]

Although this would be a clever way to further compress templates, to my eye it seems less visually clear than the proposed key and values syntax, and reads more as a "macro language" convention than a core syntax for MaterialX elements.

[ld-kerley]

Thanks for putting this together - I hope the community can use this as a way to refine the two approaches in to something we can move forward with soon.

[ld-kerley]

The great news is that I think our proposals look pretty similar, so that should make things easy!

For clarity perhaps I can enumerate what I see as the differences and we can discuss from there....

1) ( and ) vs @ to wrap token

This is something that would be straight forward to adopt in my PR if thats desired. I have no objections here, the choice of @ was always somewhat arbitrary - I agree that the visual appearance of the key being surrounded by ( and ) appears visually more appealing.

2) Simple variable replacement

You are choosing to restrict the template replacement mechanism to just a single key. In my investigations while templatizing the entire standard data library I found this was insufficient. You can look at my PR for the concrete examples where I have more than one name specified in typenames. I am more than happy to conform on naming from typenames to keys in thats preferred - that seems like a nice improvement.

3) Defining the constant values

We're pretty similar here, happy to debating naming of attributes and elements. Other than naming the difference is that you do include a type attribute in the element, which to me feels unnecessary given the scope of the element, the parent element is already defining the type. Including a type attribute here feels like it doesn't add anything, and would only be a source of errors of frustration if they differ with the parent type. Much like in the function nodegraph proposal is removing the node definition attribute from its description because its not needed due to element scope, I feel we should do the same here.

And here's the place where maybe we're furthest apart...

4) Reusing the syntax for constant value and template replacement

You're re-using the (key) syntax for the constant value replacement. I think they can and should be separate, because they really are different elements of functionality.

Personally I think that <input name="in" type="vector3" value="(zero)" /> is less clear in its intentions than <input name="in" type="vector3" value="Value:zero" /> or <input name="in" type="vector3" value="Constant:zero" />. I also worry that given we're re-using the template replacement syntax, it leads the users to think they are allowed to write something like. <input name="in" type="vector3" value="(zero) (zero) (zero)" /> because it is completely valid to re-use the (key) string multiple times in <template> replacement context. Leveraging a separate syntax construct for this cleanly differentiates the two ideas.

The other significant disadvantage to overloading the syntax is that we remove the ability for us to be able to process one feature and not the other. For instance it can be beneficial to expand the templates and leave the named constants in place. I think that is a lot cleaner and clearer to implement if they are separate concerns. We want to be able to raise warnings/errors if keys are mis-typed in a <template> element. If we overload the syntax and want to only expand the templates and not the constants then we won't be able to easily identify typos.

[jstone-lucasfilm]

@ld-kerley What would you think of the idea of merging this syntax PR as a starting point, and then you would follow up with a PR of the deltas that you'd still like to make to the syntax, since it sounds like that's a relatively small list?

[dgovil]

@jstone-lucasfilm Just my 2c, but I think it would be better to incorporate these changes into @ld-kerley 's existing PR since it has precedence and there's a longer tail of discussion associated with it. It probably would be best to wait to merge either based on community feedback.

Your PR is very helpful for the community to clearly understand your perspective on the problem however, so I don't want to trivialize that. I'd just rather have the pre-existing PR be the baseline if both directions require delta's to be merged.

Per our previous discussion, we assumed that this PR would be helping to guide/facilitate discussion for the other PR, rather than taking precedence over it.

In my opinion, we should use this PR to find the places where we can align. For instance Lee and I are very open to adopting some your alternative choices of attributes names or tokenization characters, and then we should let the community weigh in as to the relative merits of where they differ in both concepts and potential implementation.

[jstone-lucasfilm]

That's completely fair, @dgovil, and my suggestion was intended as a potential way to get us closer to the goal of a consensus specification in steps, assuming you and @ld-kerley feel that my proposal is in the right general direction.

[jstone-lucasfilm]

I like @ld-kerley's recommendation to use different syntactic conventions for key-value pairs and TypeDef token substitutions, and I've written up a refinement of the current proposal that makes this change.

In the latest proposal, TypeDef token substitutions use the standard [ and ] bracketing characters for token substitutions defined within the document itself. To handle precedence in an unambiguous way, I've included the proposed rule that TypeDef token substitutions take precedence over other forms of token substitution within a typed value attribute, which seems intuitive.