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.86k stars 1.18k forks source link

Wrong recursion stop case in BVH building #258

Closed attiju closed 4 years ago

attiju commented 4 years ago

I know it wont make such a difference but shouldn't you stop recursion when the number of primitives in current node is equal or bellow maxPrimsInNode ?

In the current version, end == start + 1 then the for loop is useless.

I suppose you wanted the if condition to be if (nPrimitives <= maxPrimsInNode) { ... }

https://github.com/mmp/pbrt-v3/blob/3f94503ae1777cd6d67a7788e06d67224a525ff4/src/accelerators/bvh.cpp#L248

mmp commented 4 years ago

The intent is actually to continue there: maxPrimitivesInNode is a limit that says: "always split any group of more than that many primitives into two sets during BVH construction; however, if no good split can be found and there are that many or fewer, then it's ok to create a leaf node".

Regarding the loop, indeed, it's not necessary, but it allows reusing the "Create leaf BVHBuildNode" code fragment in both branches of that if (nPrimitives == 1) test.