Support the implname attribute during shader generation - #3044
Open
ld-kerley wants to merge 1 commit into
Open
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(This work is extracted and refined from #2739)
Summary
The
implnameattribute 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'sdefaultinput is calleddefault_value— but shader generation ignored it entirely.Ports whose names clash with a reserved word were instead disambiguated by appending a counter, so the
mixinput of<mix>becamemix1and thenormalinput of a BSDF becamenormal1: 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.
ShaderNodeImplstores the remappings declared by its implementation element and exposes them through a newgetPortName().ShaderNode::getPortName()forwards to its implementation, andShaderGraphoverrides 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 animplnameon 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.CompoundNodeforwards its remappings toShaderGraph::create(), which applies them before finalizing the graph, since finalizing is what assigns the variable names that become a function's parameter names.inlineNodeBeforeOutput()are created afterfinalize()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 byOslSyntax— the OSL keywords and types, the OSL standard library function names, and the type name that registering a type syntax reserves.Only the 9
defaultinputs of the image and hextiled nodes were declared. 163 more are added here, each declaring animplnameof the input name with a_valuesuffix, following the convention already established bydefault→default_value. We can evolve these to other names as we move forward if desiredFor 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
paramandconnectstatements that bind by name againstthe 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().LibsToOsogenerates the<implementation>elements for the genoslnetwork target, and thesecarried none of the port remappings declared by the genosl implementation they are derived
from.
Tests
GenShader: Implementation Nameschecks that a remapped input is named by itsimplnamewhile an input without one keeps the nodedef name, for every source code target.
GenShader: OSL Implicit Surfacematerial Port Namescovers the node that the OSL generatorinserts after finalization.
The implementation coverage check now tests an
<implementation>that references a nodegraphthrough 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>