Skip to content

Propose an initial graph refactoring framework for shader generation#2832

Open
jstone-lucasfilm wants to merge 1 commit intoAcademySoftwareFoundation:mainfrom
jstone-lucasfilm:dev_graph_refactor
Open

Propose an initial graph refactoring framework for shader generation#2832
jstone-lucasfilm wants to merge 1 commit intoAcademySoftwareFoundation:mainfrom
jstone-lucasfilm:dev_graph_refactor

Conversation

@jstone-lucasfilm
Copy link
Member

This changelist proposes an initial graph refactoring framework for MaterialX shader generation, replacing hardcoded graph optimizations with a composable set of named refactoring passes. The following specific changes are included:

  • Add a ShaderGraphRefactor base class, with three concrete subclasses for node elision, premultiplied BSDF add (valuable in hardware languages), and layer-over-mix distribution (valuable in MDL).
  • Add a virtual setDefaultOptions method to ShaderGenerator, enabling each subclass to set target-appropriate defaults at GenContext construction time.
  • Add a MIX classification bit to ShaderNode for efficient mix node identification.
  • Remove hand-authored GLSL and MDL graph overrides for open_pbr_surface and standard_surface, as these refactoring steps are now generated automatically.

This changelist proposes an initial graph refactoring framework for MaterialX shader generation, replacing hardcoded graph optimizations with a composable set of named refactoring passes.  The following specific changes are included:

- Add a `ShaderGraphRefactor` base class, with three concrete subclasses for node elision, premultiplied BSDF add (valuable in hardware languages), and layer-over-mix distribution (valuable in MDL).
- Add a virtual `setDefaultOptions` method to `ShaderGenerator`, enabling each subclass to set target-appropriate defaults at `GenContext` construction time.
- Add a `MIX` classification bit to ShaderNode for efficient mix node identification.
- Remove hand-authored GLSL and MDL graph overrides for `open_pbr_surface` and `standard_surface`, as these refactoring steps are now generated automatically.
@jstone-lucasfilm
Copy link
Member Author

This proposal will require significant discussion from the MaterialX community, and I'm initially CC'ing @niklasharrysson, @ld-kerley, @ppenenko, and @jreichel-nvidia for their expertise and review.

This changelist directly addresses issue #2480, but it goes significantly further than this, providing a composable system of graph refactoring passes that handles constant/dot node elision, BSDF mix pre-multiplication for hardware shading languages, layer-over-mix distribution for MDL, and potentially far more in the future.

Let me know your thoughts, and I'd prefer that we take our time with this proposal, rather than rushing it into a release of MaterialX.

@jstone-lucasfilm
Copy link
Member Author

Here I'll share renders and performance numbers for common OpenPBR Surface materials, and the general pattern is that the current refactoring PR actually provides better real-time shading performance than the original manually-optimized graphs, which was not the case for earlier automated optimization proposals:

OpenPBR Surface Carpaint, Manual Optimization: 8 ms/frame
OpenPbrSurface_Carpaint_Manual

OpenPBR Surface Carpaint, Current PR: 7ms/frame
OpenPbrSurface_Carpaint_Automated

OpenPBR Surface Glass, Manual Optimization: 12ms/frame
OpenPbrSurface_Glass_Manual

OpenPBR Surface Glass, Current PR: 11ms/frame
OpenPbrSurface_Glass_Automted

OpenPBR Surface Pearl, Manual Optimization: 13ms/frame
OpenPbrSurface_Pearl_Manual

OpenPBR Surface Pearl, Current PR: 12ms/frame
OpenPbrSurface_Pearl_Automated

@ppenenko
Copy link
Contributor

Looks great @jstone-lucasfilm! I think it's important to clarify that these changes would not conflict with my current WIP on #2680 - even though I initially experimented at the shader graph optimization/rewriting stage as well, my code ended up moving to the shader graph construction stage. So, the two optimizations should be complimentary in the end.

@ppenenko
Copy link
Contributor

I would also suggest the infrastructure we discussed recently as an automated way to measure the performance impact of these changes with more precision and test against correctness regressions, namely:

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.

2 participants