Skip to content

Improve ShaderGenerator API#2652

Merged
jstone-lucasfilm merged 2 commits intoAcademySoftwareFoundation:mainfrom
ld-kerley:shadergen/simplify-createImplementation-interface
Oct 31, 2025
Merged

Improve ShaderGenerator API#2652
jstone-lucasfilm merged 2 commits intoAcademySoftwareFoundation:mainfrom
ld-kerley:shadergen/simplify-createImplementation-interface

Conversation

@ld-kerley
Copy link
Copy Markdown
Contributor

In #2604 two new API calls for the ShaderGenerator class were added to allow customization of the creation of ShaderNodeImplementation objects for NodeGraph and Implementation elements respectively.

I think the API is cleaner, and more robust if we pass the actual implementation objects, and not the associated NodeDef. In some cases these function have to re-acquire the implementation object from the NodeDef, but at the call sight we already have it.

#2604 has not been included in a MaterialX release yet - so I don't believe we have any problems changing the API.

Comment thread source/MaterialXGenHw/HwShaderGenerator.h Outdated
Copy link
Copy Markdown
Contributor

@niklasharrysson niklasharrysson 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 great! I agree it's a lot cleaner, and more efficient too. Just need to update the doc strings as well for these methods.

@ld-kerley ld-kerley force-pushed the shadergen/simplify-createImplementation-interface branch from 711f218 to fac1609 Compare October 31, 2025 19:23
Copy link
Copy Markdown
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.

Ship it!

@jstone-lucasfilm jstone-lucasfilm merged commit e83abdb into AcademySoftwareFoundation:main Oct 31, 2025
32 checks passed
@ld-kerley ld-kerley deleted the shadergen/simplify-createImplementation-interface branch November 21, 2025 16:57
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.

3 participants