Page 2 of 3

Re: pbrt-v3 source code now available

Posted: Wed Jul 08, 2015 2:30 pm
by Dietger
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).

Re: pbrt-v3 source code now available

Posted: Wed Jul 08, 2015 11:35 pm
by ypoissant
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.

Re: pbrt-v3 source code now available

Posted: Thu Jul 16, 2015 12:09 am
by mattpharr
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

Re: pbrt-v3 source code now available

Posted: Thu Jul 16, 2015 12:12 am
by mattpharr
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

Re: pbrt-v3 source code now available

Posted: Thu Jul 16, 2015 12:13 am
by mattpharr
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

Re: pbrt-v3 source code now available

Posted: Mon Jul 20, 2015 8:25 pm
by MohamedSakr
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)

Re: pbrt-v3 source code now available

Posted: Tue Jul 21, 2015 7:39 am
by mrluzeiro
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.

Re: pbrt-v3 source code now available

Posted: Wed Jul 22, 2015 3:48 pm
by dr_eck
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.

Re: pbrt-v3 source code now available

Posted: Thu Jul 23, 2015 5:52 am
by papaboo
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.

Re: pbrt-v3 source code now available

Posted: Mon Jul 27, 2015 5:56 pm
by andersll
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