mmp / pbrt-v3

Source code for pbrt, the renderer described in the third edition of "Physically Based Rendering: From Theory To Implementation", by Matt Pharr, Wenzel Jakob, and Greg Humphreys.
http://pbrt.org
BSD 2-Clause "Simplified" License
4.87k stars 1.18k forks source link

Why the S vector in triangle mesh are inversed when computing SurfaceInteraction #188

Open dogod621 opened 6 years ago

dogod621 commented 6 years ago

Hi,

I found the S vector in triangle mesh are inversed when computing SurfaceInteraction. And I found the code in "triangle.cpp" line 361:

       // Compute shading tangent _ss_ for triangle
        Vector3f ss;
        if (mesh->s) {
            ss = (b0 * mesh->s[v[0]] + b1 * mesh->s[v[1]] + b2 * mesh->s[v[2]]);
            if (ss.LengthSquared() > 0)
                ss = Normalize(ss);
            else
                ss = Normalize(isect->dpdu);
        } else
            ss = Normalize(isect->dpdu);

        // Compute shading bitangent _ts_ for triangle and adjust _ss_
        Vector3f ts = Cross(ss, ns);
        if (ts.LengthSquared() > 0.f) {
            ts = Normalize(ts);
            ss = Cross(ts, ns);
        } else
            CoordinateSystem((Vector3f)ns, &ss, &ts);

it will corss ( s, n ) to get t,and corss ( t, n ) to update s. This behavior will inverse S vector. Am I right? tt

I am so carefully check the local shading frame, because I define my BRDF's NDF data ( has it's own orientation ) under local shading frame. If this inversion of S vector is truth and I am not wrong. I need to fix my data to handle the inversion.

mmp commented 5 years ago

You are correct; pbrt currently incorrectly flips S in that case. :-(

Unfortunately, fixing this makes all bump maps go in the opposite direction (down rather than up and vice versa). Therefore, we'll save fixing this for pbrt-v4. I'll leave this bug report open so that the issue can be known.

tototuo commented 4 years ago

Is it right if I changed to below code?

         // Compute shading bitangent _ts_ for triangle and adjust _ss_
-        Vector3f ts = Cross(ss, ns);
+        Vector3f ts = Cross(ns, ss);

======================================

and another question, when compute triangle Intersection, the SetShadingGeometry function get ss/ts and assgined to shading.dpdu/dpdv. But the ss and ts is not the original dpdu/dpdv, it's normalized and maybe rotated, is this right?