search

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.88k stars 1.19k forks source link

src/cameras/realistic.cpp, translation of lenses #124

Closed chaosink closed 6 years ago

chaosink commented 7 years ago

src/core/transform.cpp, LookAt(), left -> right

Because pbrt uses a left-handed coordiante system and the camera direction is +z, the direction of the cross product of up and dir should be +x which is right.

chaosink commented 7 years ago

src/shapes/loopsubdiv.cpp, SDEdge, f[2] -> f, f0edgeNum -> edgeNum

It seems that only f[0] of SDEdge is used.

So I changed SDFace *f[2]; into SDFace *f; and int f0edgeNum; into int edgeNum;.

Maybe it's better to use SDFace *f0; and int f0edgeNum; to indicate only f0 and its corresponding edge need to be recorded.

chaosink commented 7 years ago

src/cameras/realistic.cpp, translation of lenses

Float c = (pz[1] - z - pz[0]) * (pz[1] - z * 4 * f - pz[0]); should be Float c = (pz[1] - z - pz[0]) * (pz[1] - z - 4 * f - pz[0]);

@syoyo typed a wrong charactor. :smiley: And... I rendered a lot of weird pictures before discovering this. :sweat_smile:

syoyo commented 7 years ago

Oops..😱 Sorry for my mistake..

syoyo commented 7 years ago

...and I think it would be better PBRT has regression test suites.

syoyo commented 7 years ago

@mmp Please accept this PR as soon as possible, otherwise PBRTv3 users get wrong result.

mmp commented 7 years ago

The camera bug was fixed in 8fa49b1491bfc9b0772dd8b510ca41eaab19ec99 (sorry that took so long :-p).

syoyo commented 7 years ago

Thanks!

mmp commented 6 years ago

Thanks for noticing those redundant bits in the subdivision data structures. I've made a note to fix those as you suggest in the 4th edition, but will leave the pbrt-v3 code as is, since I'd prefer to keep it in sync with the book text as much as possible.

Thanks!