mmp / pbrt-v4

Source code to pbrt, the ray tracer described in the forthcoming 4th edition of the "Physically Based Rendering: From Theory to Implementation" book.
https://pbrt.org
Apache License 2.0
2.8k stars 429 forks source link

Reintroducing HaltonSampler's `samplepixelcenter` that existed in v3? #340

Closed kenshi84 closed 1 year ago

kenshi84 commented 1 year ago

I found that pbrt-v3 had this option samplepixelcenter in HaltonSampler which could be useful for some debugging purposes (https://github.com/BachiLi/redner/issues/98), but seems like it was removed in pbrt-v4 (https://github.com/shadeops/pbrt_v3-to-v4_migration#halton-sampler). I wonder why, and how to bring it back.

I tried to port the same patch to pbrt-v4 as follows, but it didn't seem to have any effect.

diff --git a/src/pbrt/samplers.cpp b/src/pbrt/samplers.cpp
index 5953f17..2e6ea1e 100644
--- a/src/pbrt/samplers.cpp
+++ b/src/pbrt/samplers.cpp
@@ -30,8 +30,8 @@ std::string Sampler::ToString() const {

 // HaltonSampler Method Definitions
 HaltonSampler::HaltonSampler(int samplesPerPixel, Point2i fullRes,
-                             RandomizeStrategy randomize, int seed, Allocator alloc)
-    : samplesPerPixel(samplesPerPixel), randomize(randomize) {
+                             RandomizeStrategy randomize, int seed, Allocator alloc, bool sampleAtPixelCenter)
+    : samplesPerPixel(samplesPerPixel), randomize(randomize), sampleAtPixelCenter(sampleAtPixelCenter) {
     if (randomize == RandomizeStrategy::PermuteDigits)
         digitPermutations = ComputeRadicalInversePermutations(seed, alloc);
     // Find radical inverse base scales and exponents that cover sampling area
@@ -73,6 +73,7 @@ HaltonSampler *HaltonSampler::Create(const ParameterDictionary &parameters,
     int seed = parameters.GetOneInt("seed", Options->seed);
     if (Options->quickRender)
         nsamp = 1;
+    bool sampleAtCenter = parameters.GetOneBool("samplepixelcenter", false);

     RandomizeStrategy randomizer;
     std::string s = parameters.GetOneString("randomization", "permutedigits");
@@ -88,7 +89,7 @@ HaltonSampler *HaltonSampler::Create(const ParameterDictionary &parameters,
         ErrorExit(loc, "%s: unknown randomization strategy given to HaltonSampler", s);

     return alloc.new_object<HaltonSampler>(nsamp, fullResolution, randomizer, seed,
-                                           alloc);
+                                           alloc, sampleAtCenter);
 }

 Sampler SobolSampler::Clone(Allocator alloc) {
diff --git a/src/pbrt/samplers.h b/src/pbrt/samplers.h
index 2d13f94..76615d5 100644
--- a/src/pbrt/samplers.h
+++ b/src/pbrt/samplers.h
@@ -35,7 +35,7 @@ class HaltonSampler {
     // HaltonSampler Public Methods
     HaltonSampler(int samplesPerPixel, Point2i fullResolution,
                   RandomizeStrategy randomize = RandomizeStrategy::PermuteDigits,
-                  int seed = 0, Allocator alloc = {});
+                  int seed = 0, Allocator alloc = {}, bool sampleAtPixelCenter = false);

     PBRT_CPU_GPU
     static constexpr const char *Name() { return "HaltonSampler"; }
@@ -117,6 +117,7 @@ class HaltonSampler {

     PBRT_CPU_GPU
     Float SampleDimension(int dimension) const {
+        if (sampleAtPixelCenter && (dimension == 0 || dimension == 1)) return 0.5f;
         if (randomize == RandomizeStrategy::None)
             return RadicalInverse(dimension, haltonIndex);
         else if (randomize == RandomizeStrategy::PermuteDigits)
@@ -138,6 +139,7 @@ class HaltonSampler {
     int multInverse[2];
     int64_t haltonIndex = 0;
     int dimension = 0;
+    bool sampleAtPixelCenter = false;
 };

 // PaddedSobolSampler Definition
mmp commented 1 year ago

pbrt-v4 has a --disable-pixel-jitter command line option, or you can add:

Option "disablepixeljitter" true

to a pbrt scene file to get that (definitely useful!) behavior.

kenshi84 commented 1 year ago

Confirmed it does work! Many thanks for your quick reply :)