pbrt-v3 source code now available

Practical and theoretical implementation discussion.
Dietger
Posts: 50
Joined: Tue Nov 29, 2011 10:33 am

Re: pbrt-v3 source code now available

Post by Dietger » Wed Jul 08, 2015 2:30 pm

ypoissant wrote:On page 214, I don't understand the sentence "When working with these error intervals, it’s important to remember that because
(1 ± em) terms represent intervals, canceling them incorrect:"
Seems fine to me. Although the numerator and denominator contain the same 'term', they don't cancel because the terms are actually intervals: (1 ± em) is the interval [(1 - em),(1 + em)], so (1 ± em)/(1 ± em) = [(1 - em)/(1 + em), (1 + em)/(1 - em)] (with 0 < em < 1).

ypoissant
Posts: 97
Joined: Wed Nov 30, 2011 12:44 pm

Re: pbrt-v3 source code now available

Post by ypoissant » Wed Jul 08, 2015 11:35 pm

Dietger wrote:
ypoissant wrote:On page 214, I don't understand the sentence "When working with these error intervals, it’s important to remember that because
(1 ± em) terms represent intervals, canceling them incorrect:"
Seems fine to me. Although the numerator and denominator contain the same 'term', they don't cancel because the terms are actually intervals: (1 ± em) is the interval [(1 - em),(1 + em)], so (1 ± em)/(1 ± em) = [(1 - em)/(1 + em), (1 + em)/(1 - em)] (with 0 < em < 1).
I understand the mathematical implication. I thought the expression "canceling them incorrect" felt strange. But maybe that is because I'm not a native English speaker. Anyway, I underlined the expression in my previous post to make it clearer.

mattpharr
Posts: 9
Joined: Sat Jun 20, 2015 8:59 pm

Re: pbrt-v3 source code now available

Post by mattpharr » Thu Jul 16, 2015 12:09 am

dr_eck wrote:I took a quick run through the comments in the BDPT integrator but couldn't determine if you have implemented Georgiev's VCM or Hachisuka's near-equivalent. I'm under the impression that these are really important contributions. Are they included? Will they be?

How about Markov Chain Monte Carlo?

Will you discuss Kelemen-style MLT versus Veach-style? (Or did I miss that because I'm still at v1.0 of your book?)

How about Adaptive Progressive Photon Mapping? (I only see SPPM in the code comments.)

PBRT is a great book. I'd love to have a copy that include all of the best available algorithms.
The second edition had Kelemen-style MLT; the third will have Hachisuka's improved variant (now running on top of BDPT with MIS.)

Unfortunately we don't have VCM stuff or APPM in there. Basically, we can't add any more pages to the book, plus those algorithms are particularly tricky to implement. So the return on pages consumed didn't make sense--we'd have to have cut too much other content to include them.

There's always the 4th edition. :-)

Thanks,
Matt

mattpharr
Posts: 9
Joined: Sat Jun 20, 2015 8:59 pm

Re: pbrt-v3 source code now available

Post by mattpharr » Thu Jul 16, 2015 12:12 am

papaboo wrote:I'm with Koiava on this. Generally I find it much easier to return both sample and pdf as a single entity. It just makes it clear that you need to take the pdf into account.
The only exception is when we can sample something uniformly and the pdf is the same for all samples. In some of those cases we can just average by the number of samples taken and pdf be damned. However, even in those cases I would still implement both sample methods, just to make it clear that there is still a pdf involved.
It's an interesting question. And, for Lights and BSDFs, we always do return the value of the pdf with the sample, so it's not fully consistent. (Though with those, there'd be a bunch of unnecessary redundant computation when Pdf() was called right after the sampling method, which isn't true with shapes, where the Pdfs are simpler.)

Anyway, good point--we'll think about it!

Thanks,
Matt

mattpharr
Posts: 9
Joined: Sat Jun 20, 2015 8:59 pm

Re: pbrt-v3 source code now available

Post by mattpharr » Thu Jul 16, 2015 12:13 am

ypoissant wrote:On page 214, I don't understand the sentence "When working with these error intervals, it’s important to remember that because
(1 ± em) terms represent intervals, canceling them incorrect:"

On page 219 "then it’s impossible to tell whether it is exactly zero or whether a small positive negative value has rounded to zero."

Page 229 "guaranteed to be on the right side of the surface so that they ray doesn’t incorrectly intersect the surface it’s leaving." and "a second source of rounding error most also be addressed"
Nice catches--fixed (in the book source, if not yet the posted PDF)

Thanks!

Matt

MohamedSakr
Posts: 83
Joined: Thu Apr 24, 2014 2:27 am

Re: pbrt-v3 source code now available

Post by MohamedSakr » Mon Jul 20, 2015 8:25 pm

I think at these lines:
https://github.com/mmp/pbrt-v3/blob/6b6 ... #L130-L152
for loop should be using <=

so if we are at pixel_x = 11, filter radius = 0.5
xmin = 11 - 0.5 - 0.5 = 10
xmax = 11 - 0.5 + 0.5 + 1 = 12
loop from 10 to 12 (int i = xmin; i <= xmax; ++i)

mrluzeiro
Posts: 28
Joined: Tue May 26, 2015 2:27 pm

Re: pbrt-v3 source code now available

Post by mrluzeiro » Tue Jul 21, 2015 7:39 am

Hi Matt,

[1] I am not a C++ expert, but isn't this line here redundant?
inline uint32_t LeftShift3(uint32_t x);
https://github.com/mmp/pbrt-v3/blob/mas ... vh.cpp#L81

EDIT:
[2] The depth in flattenBVHTree(BVHBuildNode *node, int *offset, int depth)
https://github.com/mmp/pbrt-v3/blob/mas ... h.cpp#L614
looks like there is no propose / use of that variable.

[3] At https://github.com/mmp/pbrt-v3/blob/mas ... h.cpp#L273
I was confused in that switch / fall break, then I notice that in PBRT source code from book 2, you have a comment that helps understand the fall and make the things more clear.

EDIT2: I added this issues into the github system.
Last edited by mrluzeiro on Mon Jul 27, 2015 9:44 am, edited 1 time in total.

dr_eck
Posts: 46
Joined: Mon Dec 05, 2011 7:35 pm

Re: pbrt-v3 source code now available

Post by dr_eck » Wed Jul 22, 2015 3:48 pm

mattpharr wrote:
dr_eck wrote:I took a quick run through the comments in the BDPT integrator but couldn't determine if you have implemented Georgiev's VCM or Hachisuka's near-equivalent. I'm under the impression that these are really important contributions. Are they included? Will they be?

How about Markov Chain Monte Carlo?

Will you discuss Kelemen-style MLT versus Veach-style? (Or did I miss that because I'm still at v1.0 of your book?)

How about Adaptive Progressive Photon Mapping? (I only see SPPM in the code comments.)

PBRT is a great book. I'd love to have a copy that include all of the best available algorithms.
The second edition had Kelemen-style MLT; the third will have Hachisuka's improved variant (now running on top of BDPT with MIS.)

Unfortunately we don't have VCM stuff or APPM in there. Basically, we can't add any more pages to the book, plus those algorithms are particularly tricky to implement. So the return on pages consumed didn't make sense--we'd have to have cut too much other content to include them.

There's always the 4th edition. :-)

Thanks,
Matt
I'm not sure how the 4th edition would solve the page count problem. Perhaps a second volume that addresses "Advanced PBRT Techniques" is called for. It could be the basis for a second semester course.

papaboo
Posts: 45
Joined: Fri Jun 21, 2013 10:02 am
Contact:

Re: pbrt-v3 source code now available

Post by papaboo » Thu Jul 23, 2015 5:52 am

You could always implement them as very well documented integrators and add them to the source code, then they wouldn't even have to be ready for the release. For VCM/UPS though there is already three papers and SmallVCM. If you've read and understood PBRT, then that should be enough to get anyone started on their own VCM implementation.

andersll
Posts: 22
Joined: Thu Mar 21, 2013 4:34 pm

Re: pbrt-v3 source code now available

Post by andersll » Mon Jul 27, 2015 5:56 pm

In the BeckmannSample11 function in microfacet.cpp you have special logic to check for whether we're at normal incidence and sample the slope directly, but then the rest of the function continues as normal and *slope_x and *slope_y will be overwritten. Shouldn't the function return after line 48?

https://github.com/mmp/pbrt-v3/blob/6b6 ... et.cpp#L43

Post Reply