search

owl-project / NVISII

Apache License 2.0
319 stars 27 forks source link

Typo in disney brdf? #144

Open 111116 opened 2 years ago

111116 commented 2 years ago

in sample_gtr_1_h and sample_gtr_2_h of disney BRDF:

    float cos_theta_h = sqrt(cos_theta_h_sqr);
    float sin_theta_h = 1.f - cos_theta_h_sqr;
    float3 hemi_dir = normalize(spherical_dir(sin_theta_h, cos_theta_h, phi_h));

This looks mistaken to me, because apparently sin_theta_h should be square-rooted.

Also, while I'm trying to use this Disney BRDF code in my project, some materials can't be rendered correctly (even after I attempt to fix the typo above). So could you please tell whether this code have been tested? So that I can either help fix the BRDF code or figure out what can go wrong in other parts of my own project. Thanks!

natevm commented 2 years ago

The BRDF here is actively used in NVISII. You can try it out yourself by downloading the package through pip and running the interactive materials example.

I do understand that there are some correctness issues. I believe I made some changes to improve the numerical stability in extreme cases. For transmission I could never get the Disney BRDF to produce valid probabilities….

natevm commented 2 years ago

@111116 were you able to get these issues corrected in your project? Any way we might be able to get a pull request here incorporating those fixes?