Skip to content

Define Chiang BCSDF normalization - #3018

Open
tdavidovicNV wants to merge 3 commits into
AcademySoftwareFoundation:mainfrom
tdavidovicNV:spec/fix-chiang-normalization
Open

Define Chiang BCSDF normalization#3018
tdavidovicNV wants to merge 3 commits into
AcademySoftwareFoundation:mainfrom
tdavidovicNV:spec/fix-chiang-normalization

Conversation

@tdavidovicNV

Copy link
Copy Markdown
Contributor

While reviewing the Chiang hair equations added in #2964, I found an additional global 1/pi factor that is not present in the cited model. The longitudinal and azimuthal distributions already contain their normalization factors, including 1/(2*pi) for the uniform high-order azimuthal lobe.

Normalization argument

Consider a non-absorbing fiber (T = 1) with unit lobe tints. The attenuation terms in the specification become:

$$A_R = F,\qquad A_{TT} = (1-F)^2,\qquad A_{TRT} = (1-F)^2F,\qquad A_{TRRT+} = (1-F)F^2.$$

They account for all possible paths:

$$A_R + A_{TT} + A_{TRT} + A_{TRRT+} = 1.$$

Consider the simplest case, with zero cuticle tilt, in a constant white environment. A non-absorbing material should return the same unit radiance in every direction: this is the usual white-furnace test. The attenuation terms above divide the incoming energy among the four scattering paths, and their weights add up to 1. The normalized M_p and N_p distributions only determine where each path's share of the energy is scattered.

The familiar 1/pi factor is needed for a constant Lambertian BRDF because its projected hemisphere integral is pi. It is not needed for these already-normalized directional distributions. Applying it to the complete Chiang response reduces the white-furnace result from 1 to 1/pi, losing about 68% of the energy.

This is the normalization tested by the white-furnace result in Figure 5(e) of the original Chiang et al. paper.

Specification change

The specification already denotes a BCSDF by f_c. Following equations 1 and 2 of the cited Chiang paper, this PR defines it directly as:

$$f_c(\omega_i,\omega_o) = \sum_p t_p A_p M_p N_p.$$

The existing f_c notation is retained, and this change does not add a general BCSDF transport convention. Renderer APIs may account for the curve projection factor separately, but that does not introduce a global 1/pi. The PR changes the specification only; the current implementations are not changed.

Implementation notes

This follows the original Chiang et al. model, equations 1 and 2. PBRT v4 accounts for its renderer-facing measure separately and likewise has no global 1/pi.

@jstone-lucasfilm

Copy link
Copy Markdown
Member

Thanks for another excellent analysis, @tdavidovicNV! I agree that the lobe sum without a global 1/pi is the correct equation for the specification, matching the equations in the Chiang paper. Your white-furnace argument demonstrates this clearly: the attenuation factors already partition the incident energy exactly among the four scattering paths, and the normalized M_p and N_p distributions only determine where each path's share is scattered. As you note, this also resolves an internal inconsistency in the current document, since the appendix already defines N_p as a normalized distribution with N_TRRT+ = 1/(2*pi), which cannot coexist with a global 1/pi in a faithful transcription of the model.

As in #3015 and #3017, my one structural request would be to omit the Implementation notes section from the specification text, since the MaterialX PBR Specification intentionally avoids target-specific implementation details, and your per-target analysis will remain permanently accessible in the description and discussion of this PR.

One smaller editorial suggestion: since the specification is normative rather than change-relative, I'd propose rewording the clause "so there is no additional global factor of 1/pi" as a positive statement, e.g. that the lobe products require no additional normalization. Similarly, the N_TRRT+ = 1/(2*pi) value is already given in the appendix, so the main-body text can simply reference the normalization of the distributions without restating it.

With respect to our GLSL implementation, I agree that the additional M_PI_INV factor represents a divergence from the model rather than a lighting-convention compensation, and I'd suggest we address this in a follow-up PR that removes the factor and regenerates the baseline images, bringing GLSL into alignment with both the corrected specification and the MDL implementation. In the interim, this divergence should be documented in the shader code itself.

Otherwise, this looks like a solid improvement to me, and we should plan to merge it before our v1.39.6 release.

@tdavidovicNV

Copy link
Copy Markdown
Contributor Author

Thanks, @jstone-lucasfilm. I’ve reworded the normalization statement in positive terms, and removed the target specific notes (will work on the rest). I also added a TODO at the GLSL M_PI_INV application to document the current divergence.

I agree that removing the factor and updating the baseline images should be handled in a follow-up PR.

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