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 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:"
pbrtv3 source code now available
Re: pbrtv3 source code now available
Re: pbrtv3 source code now available
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.Dietger wrote: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 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:"
Re: pbrtv3 source code now available
The second edition had Kelemenstyle MLT; the third will have Hachisuka's improved variant (now running on top of BDPT with MIS.)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 nearequivalent. 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 Kelemenstyle MLT versus Veachstyle? (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.
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 sensewe'd have to have cut too much other content to include them.
There's always the 4th edition.
Thanks,
Matt
Re: pbrtv3 source code now available
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.)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.
Anyway, good pointwe'll think about it!
Thanks,
Matt
Re: pbrtv3 source code now available
Nice catchesfixed (in the book source, if not yet the posted PDF)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"
Thanks!
Matt

 Posts: 83
 Joined: Thu Apr 24, 2014 2:27 am
Re: pbrtv3 source code now available
I think at these lines:
https://github.com/mmp/pbrtv3/blob/6b6 ... #L130L152
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)
https://github.com/mmp/pbrtv3/blob/6b6 ... #L130L152
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: pbrtv3 source code now available
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/pbrtv3/blob/mas ... vh.cpp#L81
EDIT:
[2] The depth in flattenBVHTree(BVHBuildNode *node, int *offset, int depth)
https://github.com/mmp/pbrtv3/blob/mas ... h.cpp#L614
looks like there is no propose / use of that variable.
[3] At https://github.com/mmp/pbrtv3/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.
[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/pbrtv3/blob/mas ... vh.cpp#L81
EDIT:
[2] The depth in flattenBVHTree(BVHBuildNode *node, int *offset, int depth)
https://github.com/mmp/pbrtv3/blob/mas ... h.cpp#L614
looks like there is no propose / use of that variable.
[3] At https://github.com/mmp/pbrtv3/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.
Re: pbrtv3 source code now available
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.mattpharr wrote:The second edition had Kelemenstyle MLT; the third will have Hachisuka's improved variant (now running on top of BDPT with MIS.)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 nearequivalent. 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 Kelemenstyle MLT versus Veachstyle? (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.
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 sensewe'd have to have cut too much other content to include them.
There's always the 4th edition.
Thanks,
Matt
Re: pbrtv3 source code now available
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: pbrtv3 source code now available
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/pbrtv3/blob/6b6 ... et.cpp#L43
https://github.com/mmp/pbrtv3/blob/6b6 ... et.cpp#L43