Cycles: Refactor GGX closure implementation for a small performance improvement
Needs ReviewPublic
Actions

Authored by Lukas Stockner (lukasstockner97) on Aug 10 2018, 6:47 AM.

Details

Reviewers

Brecht Van Lommel (brecht)
Sergey Sharybin (sergey)

Summary

This patch refactors the implementation of the GGX microfacet BSDF.

On the one hand, it provides a small but measurable performance improvement due to the following changes:

Using a reformulated version of the G1 term (credit for that one goes to @Erik Englesson (ErikEnglesson)).
Not calculating tanTheta in the calculation of the isotropic distribution term D.
Skipping some terms in the anisotropic D.
Deduplicating some terms in the final BSDF refraction calculation.
Not calculating the reflection direction and fresnel term in the refraction code since they're unused anyways.

On the other hand, it moves some of the shared terms to separate functions and adds more comments to the code, therefore improving overall readability.

Diff Detail

Repository

rB Blender

Branch

ggx_refactor

Build Status

Buildable 1884
Build 1884: arc lint + arc unit

Event Timeline

Lukas Stockner (lukasstockner97) created this revision.Aug 10 2018, 6:47 AM

When you mention measurable speedup, what scene you used and what were the timings?

Adding comments is surely nice, but readability of long lists of obscurely named arguments, which goes far behind the screen is questionable to me. What is the point, say, calling argument YdotD and then on every call pass dotprocut? I could buy it if the dotproduct was reused, but here, nah. Also, obviously, you don't pass dot(Y, D), you pass dot(I, Y), go figure.

Manuel Grad (manitwo) added a subscriber: Manuel Grad (manitwo).Nov 7 2018, 4:07 PM