Skip to content

Support the implname attribute during shader generation - #3044

Open
ld-kerley wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
ld-kerley:genshader/implname-support
Open

Support the implname attribute during shader generation#3044
ld-kerley wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
ld-kerley:genshader/implname-support

Conversation

@ld-kerley

Copy link
Copy Markdown
Contributor

(This work is extracted and refined from #2739)

Summary

The implname attribute lets an <implementation> declare that a port defined in the <nodedef> is called something else in the implementation's code. The specification describes it as a way to avoid clashes with reserved words in the target shading language, and the data libraries already used it — every <image> implementation declares that the nodedef's default input is called default_value — but shader generation ignored it entirely.

Ports whose names clash with a reserved word were instead disambiguated by appending a counter, so the mix input of <mix> became mix1 and the normal input of a BSDF became normal1: names that carry no meaning, and that shift if the reserved word list changes, and is also dependent on network topology in some cases.

This PR makes shader generation honor implname, declares it for the genosl inputs that clash, and applies it in the one place where a port name crosses a real interface boundary — the OSL network target.

Shader generation

Port name remappings are read from the implementation element and applied when naming variables in generated code.

  • ShaderNodeImpl stores the remappings declared by its implementation element and exposes them through a new getPortName().
  • ShaderNode::getPortName() forwards to its implementation, and ShaderGraph overrides it with its own set, for generators that build a graph directly from a nodedef rather than from a node. ShaderNode::getPortVariableName() is the single source of truth for the variable naming formula and is used at every site that names a port.
  • ShaderGenerator::getImplementation() populates the remappings once per implementation element before it is cached. It reads both the resolved element and the unresolved declaration: an <implementation> that references a nodegraph is resolved to that nodegraph before shader generation sees it, so an implname on the <implementation> itself was being discarded. Reading both, with the declaration taking precedence, allows a nodegraph shared by every target to be given target-specific port names.
  • CompoundNode forwards its remappings to ShaderGraph::create(), which applies them before finalizing the graph, since finalizing is what assigns the variable names that become a function's parameter names.
  • A shader generated for a single node is the implementation of that node's nodedef, so its interface is published under that implementation's names as well.
  • Nodes inserted by inlineNodeBeforeOutput() are created after finalize() has run and named their own port variables, bypassing the remapping. They now use the same formula.

Variable names remain unique, as they still pass through Syntax::getVariableName().

Data libraries

I audited every genosl <implementation>, checking each input name of its <nodedef> against the reserved words registered by OslSyntax — the OSL keywords and types, the OSL standard library function names, and the type name that registering a type syntax reserves.

Only the 9 default inputs of the image and hextiled nodes were declared. 163 more are added here, each declaring an implname of the input name with a _value suffix, following the convention already established by defaultdefault_value. We can evolve these to other names as we move forward if desired

For nodedefs whose implementation is a <nodegraph>, that nodegraph's interface is shared by every target, so the declaration cannot go on the nodegraph itself. Each is declared instead on a new target-specific <implementation>
that references the existing nodegraph, leaving the nodegraph untouched and every other target unaffected.

Only genosl is covered, as currently this is the only language with a node-based output where is it important these names are consistent. The other source code targets are untouched, and the same treatment could follow for them if that's wanted.

OSL network target

The OSL network generator emits param and connect statements that bind by name against
the compiled oso for each node, making it the one place where a port name crosses a real
interface boundary. Both sides of a binding are now resolved through getPortName().

LibsToOso generates the <implementation> elements for the genoslnetwork target, and these
carried none of the port remappings declared by the genosl implementation they are derived
from.

Tests

GenShader: Implementation Names checks that a remapped input is named by its implname
while an input without one keeps the nodedef name, for every source code target.
GenShader: OSL Implicit Surfacematerial Port Names covers the node that the OSL generator
inserts after finalization.

The implementation coverage check now tests an <implementation> that references a nodegraph
through that nodegraph, since it is the nodegraph that gets recorded as used, and logs each
one together with the nodegraph so that the indirection is visible in the report rather than
silently trusted.

Test suite coverage is added for nodes that no material exercised

  • <overlay>
  • <ramp>
  • <randomfloat>
  • <bump>
  • <refract>
  • <latlongimage>

The `implname` attribute lets an <implementation> declare that a port
defined in the <nodedef> is called something else in the implementation's
code, which the specification describes as a way to avoid clashes with
reserved words in the target shading language. The standard libraries
already used it - every <image> implementation declares that the nodedef's
`default` input is called `default_value` - but shader generation ignored
it entirely. Those ports were instead named by appending a counter, so the
`mix` input of <mix> became `mix1` and the `normal` input of a BSDF became
`normal1`: names that carry no meaning, and that shift if the reserved word
list changes.

Shader generation
-----------------

Port name remappings are now read from the implementation element and
applied when naming variables in generated code.

* `ShaderNodeImpl` stores the remappings declared by its implementation
  element and exposes them through a new `getPortName()`.
* `ShaderNode::getPortName()` forwards to its implementation, and
  `ShaderGraph` overrides it with its own set, for generators that build a
  graph directly from a nodedef rather than from a node.
  `ShaderNode::getPortVariableName()` is the single source of truth for the
  variable naming formula and is used at every site that names a port.
* `ShaderGenerator::getImplementation()` populates the remappings via the
  new `addPortImplementationNames()` helper, once per implementation element
  before it is cached. It reads both the resolved element and the unresolved
  declaration: an <implementation> that references a nodegraph is resolved
  to that nodegraph before shader generation sees it, so an `implname` on
  the <implementation> itself was being discarded. Reading both, with the
  declaration taking precedence, allows a nodegraph shared by every target
  to be given target-specific port names.
* `CompoundNode` forwards its remappings to `ShaderGraph::create()`, which
  applies them before finalizing the graph, since finalizing is what assigns
  the variable names that become a function's parameter names.
* A shader generated for a single node is the implementation of that node's
  nodedef, so its interface is published under that implementation's names
  as well.
* Nodes inserted by `inlineNodeBeforeOutput()` are created after
  `finalize()` has run and named their own port variables, bypassing the
  remapping. They now use the same formula.

Variable names remain unique, as they still pass through
`Syntax::getVariableName()`.

Data libraries
--------------

Audited every genosl <implementation>, checking each input name of its
<nodedef> against the reserved words registered by OslSyntax - the OSL
keywords and types, the OSL standard library function names, and the type
name that registering a type syntax reserves.

Only the 9 `default` inputs of the <image> and hextiled nodes were
declared. 163 more are added here, each declaring an `implname` of the
input name with a `_value` suffix, following the convention already
established by `default` -> `default_value`.

78 of them are on implementations backed by OSL source or by an inline
expression. The remaining 85 belong to nodedefs whose OSL implementation is
a <nodegraph> - standard_surface, open_pbr_surface, gltf_pbr,
UsdPreviewSurface, disney_principled, the lama nodes and others. That
nodegraph's interface is shared by every target, so the declaration cannot
go on the nodegraph itself; each is declared instead on a new
target-specific <implementation> that references the existing nodegraph,
leaving the nodegraph untouched and every other target unaffected. For the
bxdf library these live in a new `bxdf/genosl` folder, alongside the
equivalent folders of the other data libraries.

The clashing inputs that remain undeclared all belong to nodedefs with no
genosl implementation at all, mostly the light and volume material nodes.

Generated output
----------------

The parameters of a shader generated from a single nodedef are renamed:
<overlay> now takes `mix_value` rather than `mix1`, <image> takes
`default_value` and `layer_value`, and <standard_surface> takes
`subsurface_value`, `sheen_value`, `emission_value` and `normal_value`.
Calls between nodes are emitted positionally, so nothing else in the
generated source changes.

This renames the parameters of the reference shaders and of the oso
libraries built from them. Anything binding to one of those osos by the
previous names has to be updated.

OSL network target
------------------

The OSL network generator emits `param` and `connect` statements that bind
by name against the compiled oso for each node, making it the one place
where a port name crosses a real interface boundary. Both sides of a
binding are now resolved through `getPortName()`.

`LibsToOso` generates the <implementation> elements for the genoslnetwork
target, and these carried none of the port remappings declared by the
genosl implementation they are derived from. They now carry them, so that a
consumer of an oso can bind against the names its parameters actually have.
Without this the network generator emitted a connection to `surfaceshader1`
on a shader whose parameter is `surfaceshader_value`, which testrender
rejects.

Tests
-----

`GenShader: Implementation Names` checks that a remapped input is named by
its `implname` while an input without one keeps the nodedef name, for every
source code target. `GenShader: OSL Implicit Surfacematerial Port Names`
covers the node that the OSL generator inserts after finalization.

The implementation coverage check now tests an <implementation> that
references a nodegraph through that nodegraph, since it is the nodegraph
that gets recorded as used, and logs each one together with the nodegraph
so that the indirection is visible in the report rather than silently
trusted.

Test suite coverage is added for nodes that no material exercised, and that
were therefore never shader generated by the generator tests: <overlay> in
each of its three variants, <ramp> (the multi interval ramp, as distinct
from ramplr / ramptb / ramp4), <randomfloat> for a float and an integer
input, <bump>, <refract>, <latlongimage>, the glTF PBR image nodes not
reached by the glTF PBR examples, and the three shader translation graphs.
All are implemented by nodegraphs, which is why the gap went unnoticed: the
coverage check only counts implementation elements, and a nodedef
implemented by a nodegraph has none. Like the other unit test files for
individual nodes these declare nodegraph outputs rather than materials, so
they are exercised by the shader generation tests of every target.
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.

1 participant