KhronosGroup / glTF

glTF – Runtime 3D Asset Delivery
Other
7.02k stars 1.13k forks source link

Typo in Smith joint shadowing-masking function #2385

Closed TheRealLL4 closed 1 month ago

TheRealLL4 commented 2 months ago

Section B.3.2 states that the Smith joint masking-shadowing function $G$ vanishes when $H \cdot V \le 0$ or $H \cdot L \le 0$. Firslty, these dot products are always nonnegative by the definition of $H$. Secondly, according to Heitz (2014), paragraph 6, identity 98, separable masking-shadowing function is of the form $\displaystyle \frac{\chi^+(V \cdot N)}{1 + \Lambda(V)} \frac{\chi^+(L \cdot N)}{1 + \Lambda(L)}$, which means that it vanishes when $V$ or $L$ is facing away from the microfacet normal $N$. Therefore, I believe that $\chi^+(V \cdot H) \chi^+(L \cdot H)$ in the numerator of $G$ should be replaced with $\chi^+(V \cdot N) \chi^+(L \cdot N)$. This change would also eliminate the need for $\chi^+(H \cdot N)$ in the definition of GGX distribution function $D$.

bsdorra commented 1 month ago

In Heitz (2014), the separable masking-shadow function is given as $$\dfrac{\chi^+(\omega_o\cdot \omega_m)}{1+\Lambda(\omega_o)}\dfrac{\chi^+(\omega_i\cdot \omega_m)}{1+\Lambda(\omega_i)}$$ where $\omega_m$ is the microfacet normal. You're rightfully stating that it vanishes when $L$ or $V$ are facing away from the microfacet. But, in the glTF spec Heitz's $\omega_m$ corresponds to the half-vector $H$, not the macrosurface normal $N$. So the current definition of G seems to be in line with Heitz. Consequently, $\chi^+(V\cdot N)\chi^+(V\cdot N)$ would check the macrosurface visibility.

With the current definition of $H$, these heavyside terms are indeed not particular useful. But, considering that some renderers may use sampling methods to generate $H$ this check becomes important again. Maybe we should add this info as clarification of why these terms are there.

emackey commented 1 month ago

@TheRealLL4 Does that information clarify this enough? Rather than edit the specification to add a note, perhaps we can point folks to this issue if they need the clarification.