mmp / pbrt-v2

Source code for the version of pbrt described in the second edition of "Physically Based Rendering"
http://pbrt.org
990 stars 343 forks source link

Wrong direction of shading tangents in Triangle::GetShadingGeometry causing wrong bump mapping #61

Closed z-boson closed 1 year ago

z-boson commented 1 year ago

In Triangle::GetShadingGeometry the shading tangents ss and ts are calculated, but it seems they have the wrong direction, they point in the opposite direction of dpdu and dpdv.

ss = Normalize(dg.dpdu);
ts = Cross(ss, ns);
ss = Cross(ts, ns);

After the above cross products ss points in the opposite direction of dpdu and ts points in the opposite direction of dpdv. But ss is used as dpdu and ts as dpdv in the shading DifferentialGeometry.

p1 = p0 + u * dpdu + v * dpdv;
// using ss and ts in this formula results in p1 lying in the opposite direction

I have checked the code in pbrt-v4 and there the first formula was changed to ts = Cross(ns, ss); which fixes the problem (pbrt-v3 has the same problem). Is this correct?

mmp commented 1 year ago

Correct!

We left that unfixed in pbrt-v2 since fixing it would have changed rendered images and since many scenes were set up assuming the buggy behavior.

All the more reason to upgrade to pbrt-v3 (or v4!) :-)