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

Why impletment SpatialLightDistribution using a hash table rather than a simple array? #204

Closed ethanleau closed 4 years ago

ethanleau commented 5 years ago

In "src/core/lightdistrib.h(.cpp)", SpatialLightDistribution use a hash table to record light distribution in each voxel. I wonder if a simple array is enough.

For example, each element in this simple array is (simliar to HashEntry):

struct Entry {
    std::atomic<bool> empty;  // for detecting whether another thread is computing the distribution
    std::atomic<Distribution1D*> distribution;
}

And to index a point(Point3f p), we just

Vector3f offset = scene.WorldBound().Offset(p);
    Point3i pi;
    for (int i = 0; i < 3; ++i)
        pi[i] = Clamp(int(offset[i] * nVoxels[i]), 0, nVoxels[i] - 1);
    size_t index = size_t(pi[0]) * size_t(nVoxels[1]) * size_t(nVoxels[2]) +
                   size_t(pi[1]) * size_t(nVoxels[2]) +
                   size_t(pi[2]);

Then we can get the ditribution using atomic operations

Entry &entry = tables[index];
while (true)
{
    bool isEmpty = entry.empty.load(std::memory_order_acquire);
    if (!isEmpty)
    {
        Distribution1D *dist= regionInfo.distribution.load(std::memory_order_acquire);
        if (dist == nullptr)
        {
            while ((dist = entry.distribution.load(std::memory_order_acquire)) == nullptr)
                ;
        }
        return dist;
    }
    else
    {
        bool em = false;
        if (entry.empty.compare_exchange_weak(em, true))
        {
            Distribution1D *dist = ComputeDistribution(pi);
            region.distribution.store(dist, std::memory_order_release);
            return dist;
        }
    }
}
mmp commented 5 years ago

You are correct--nice catch!

I think that's an artifact from an early version of that code, where I wanted to try high resolution voxelizations. In that case, it'd be a lot of wasted memory for all of the empty voxels that didn't have geometry in them, so the idea was to use a hash table. It's now very silly to allocate 4x more voxels than will ever be necessary for the hash table.

FWIW this turns out to be a not great way to sample lights; the Conty and Kulla paper from EGSR 2018 on many light source sampling works much better (and that, or something like it, will replace this code in the next version of pbrt.)

mmp commented 4 years ago

Closing this, since that code is effectively obsolete for the future.