Skip to content

Consolidate glTF PBR graph and remove GLSL override#2850

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

Consolidate glTF PBR graph and remove GLSL override#2850
jstone-lucasfilm wants to merge 1 commit intoAcademySoftwareFoundation:mainfrom
jstone-lucasfilm:dev_gltf_pbr

Conversation

@jstone-lucasfilm
Copy link
Copy Markdown
Member

This changelist consolidates the dielectric iridescence path in the glTF PBR shading graph, and removes the hand-authored GLSL graph override that is no longer needed. The following specific changes are included:

  • Restructure the dielectric iridescence path from mix(layer(A, C), layer(B, C), w) to layer(mix(A, B, w), C), eliminating a duplicated layer node that was a workaround for earlier codegen limitations.
  • Remove the hand-authored GLSL graph override for gltf_pbr, as the PremultipliedBsdfAddRefactor pass now reproduces these optimizations automatically from the consolidated global graph.

This changelist consolidates the dielectric iridescence path in the glTF PBR shading graph, and removes the hand-authored GLSL graph override that is no longer needed.  The following specific changes are included:

- Restructure the dielectric iridescence path from `mix(layer(A, C), layer(B, C), w)` to `layer(mix(A, B, w), C)`, eliminating a duplicated layer node that was a workaround for earlier codegen limitations.
- Remove the hand-authored GLSL graph override for `gltf_pbr`, as the `PremultipliedBsdfAddRefactor` pass now reproduces these optimizations automatically from the consolidated global graph.
@jstone-lucasfilm
Copy link
Copy Markdown
Member Author

I'm attaching render comparisons of two glTF PBR example materials, before and after the current PR, demonstrating that both visuals and performance are unchanged:

glTF PBR Carpaint (Before):

GltfPbr_Carpaint_Old

glTF PBR Carpaint (After):

GltfPbr_Carpaint_New

glTF PBR Glass (Before):

GltfPbr_Glass_Old

glTF PBR Glass (After):

GltfPbr_Glass_New

@jstone-lucasfilm
Copy link
Copy Markdown
Member Author

To confirm that these changes make sense from the glTF PBR perspective, I'm additionally CC'ing @kwokcb, @bowald, and @emackey.

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