On linux, I got a segmentation fault on some serene. This happens on gcc-4.8.2. But it seems work fine on gcc-4.7.

segmentation fault  pbrt killeroo-gold.pbrt

Omitting octaves essentially replaces them by the constant 0. This is only correct if the average of an octave is zero, as is the case for the Noise() function used in FBm.

But the average of the fabsf(Noise()) function used in Turbulence is non-zero. It is in fact the standard deviation of the Noise() function (roughly 0.2). Omitted octaves should be replaced by that non-zero constant or the result will be too small. In fact, if the maximum differential dpdx or dpdy > 2 (e.g. at grazing angles), the number of octaves n < 0, and the result will be black instead of gray.

(From mantis bug tracker, reported by Bachi)

Triangle::GetShadingGeometry compute shading coordinate system by interpolating given per-vertex normal/tangents, and use the axes of the shading frame as the dpdu/dpdv values in the returned DifferentialGeometry.

However, the magnitude of dpdu/dpdv is lost due to normalization, and this will affect bump mapping calculation since it depends on the magnitude of dpdu/dpdv.

One way to fix this is to separate dpdu/dpdv and shading coordinate system in the DifferentialGeometry structure.

Upon implementing a quick hack for leaf-rendering in picogen, I found the BRDF-to-BTDF adapter on top of a diffuse model a dirty, but good candidate. However, I recognized that there's a "discontinuity" at:

reflection.cpp:

float BRDFToBTDF::Pdf(const Vector &wo,
    const Vector &wi) const {
    return brdf->Pdf(wo, -wi);
}

where I would have expected

float BRDFToBTDF::Pdf(const Vector &wo,
    const Vector &wi) const {
    return brdf->Pdf(wo, otherHemisphere(wi));
}

When I think about it, it doesn't seem right. The adapter would convert "near-specular" reflection into near-specular transmission:

      /     <-- out
 - - - - - -
   /        <-- in

The negation in Pdf(.) though would make "in" near-identical to "out", and yield a pdf of [near] zero.

Which would be the reason for negation instead of re-hemisphering in or out, if any?

(sidenote: this ticket would also apply to pbrt-v1)

I add a new shape called heightfield2 into pbrt, and compile it. Then i run the pbrt on my own scene files.
But i got different results on Windows and Linux. As running on Windows, the result images just looks fine. As running on Linux, it can still compile and didn't get any error message. But the result image is broken. I'm really confused.

The environment of Windows is Win10, AMD A8-3850 APU with Radeon(tm) HD Graphics 2.90GHz, 64bits. And i use Microsoft Visual Studio Professional 2015 Version 14.0.25431.01 to compile it.
The environment of Linux is Debian 64bit, Intel Xeon E5345.

(From mantis bug tracker)

fpsunflower: I've noticed that the EWA filter exhibits strange artifacts under magnification. This is apparent in the two following images (closeup of the lines.exr texture)

Film "image"
    "integer xresolution" [200] "integer yresolution" [100]
    "integer xresolution" [800] "integer yresolution" [400]
#   "integer xresolution" [750] "integer yresolution" [375]
#   "float cropwindow" [.2 .4 .07 .22]

Sampler "lowdiscrepancy" "integer pixelsamples" [1]
PixelFilter "box"

LookAt 0 0.7 8  0 0.7 0   0 1 0
Camera "perspective" "float fov" [1]

SurfaceIntegrator "directlighting"

WorldBegin

AttributeBegin
#  LightSource "distant" "color L" [3 3 3] "point from" [0 10 3] "point to" [0 0 0 ]
AttributeEnd

AttributeBegin
  CoordSysTransform "camera"
  AreaLightSource "area" "color L" [6 6 6] "integer nsamples" [32]
  Translate 0 3 -10
  Material "matte" "color Kd" [ 0 0 0 ]
  Shape "disk" "float radius" [3] 
AttributeEnd

AttributeBegin
  AreaLightSource "area" "color L" [3.2 3.2 3.2] "integer nsamples" [32]
  Translate 0 15 0
  Rotate 90 1 0 0 
  Material "matte" "color Kd" [ 0 0 0 ]
  Shape "disk" "float radius" [20] 
AttributeEnd

AttributeBegin
  Material "matte" "color Kd" [.1 .1 .1]
  Shape "trianglemesh" "integer indices" [ 0 1 2 2 0 3 ]
    "point P" [-10 0 -10   10 0 -10   10 0 10   -10 0 10 ]
  Shape "trianglemesh" "integer indices" [ 0 1 2 2 0 3 ]
    "point P" [-10 0 -10   10 0 -10   10 9 -10   -10 9 -10 ]
  Shape "trianglemesh" "integer indices" [ 0 1 2 2 0 3 ]
    "point P" [-10 0 -10   -10 0 10   -10 9 10   -10 9 -10 ]
AttributeEnd

Texture "g-tri" "color" "imagemap" "string filename" "textures/lines.exr"
    "bool trilinear" ["true"]
    "float uscale" [1] "float vscale" [1]
Texture "g-ewa" "color" "imagemap" "string filename" "textures/lines.exr"
    "float uscale" [1] "float vscale" [1]


AttributeBegin

## switch texture filtering here ##
Material "uber" "texture Kd" "g-ewa" "color Ks" [.1 .1 .1]
#Material "uber" "texture Kd" "g-tri" "color Ks" [.1 .1 .1]


Translate 0 1 6
Rotate 10 0 0 1
Rotate -35 1 0 0
Rotate -20 1 1 1
Shape "sphere"
AttributeEnd

WorldEnd

(From mantis bug tracker)

In BxDF::Sample_f(), function CosineSampleHemisphere() may generate a wi with the z component being 0, which means the probability of choosing the wi is 0. But how could we get this wi since the probability distribution function says it's not possible?

Hello, I am new to pbrt, just start to try it, I wanna render an animation sequences, maybe 150 frames, how do I set the related setting in the scene file? Hope someone can give me a guide?
Thanks!

Shouldn't the AmbientOcclusionIntegrator::Li method use uniform cosine weighted sphere/hemisphere sampling or at least take the cosine factor separately into account in case of uniform sphere/hemisphere sampling?

Possible fix:

// AmbientOcclusionIntegrator Method Definitions
Spectrum AmbientOcclusionIntegrator::Li(const Scene *scene, const Renderer *renderer,
        const RayDifferential &ray, const Intersection &isect,
        const Sample *sample, RNG &rng, MemoryArena &arena) const {

    const BSDF *bsdf = isect.GetBSDF(ray, arena);
    const Point &p = bsdf->dgShading.p;
    const Normal n = Faceforward(isect.dg.nn, -ray.d);

    const uint32_t scramble[2] = { rng.RandomUInt(), rng.RandomUInt() };
    float u[2];
    int nClear = 0;
    for (int i = 0; i < nSamples; ++i) {
        Sample02(i, scramble, u);

        const Vector axis_w = (Vector) n;
        const Vector axis_u = Cross((abs(axis_w.x) > 0.1f ? Vector(0.0f, 1.0f, 0.0f) : Vector(1.0f, 0.0f, 0.0f)), axis_w);
        const Vector axis_v = Cross(axis_w, axis_u);
        const Vector sample_d = CosineSampleHemisphere(u[0], u[1]);
        const Vector d = (sample_d.x * axis_u + sample_d.y * axis_v + sample_d.z * axis_w);

        const Ray r(p, d, .01f, maxDist);
        if (!scene->IntersectP(r)) ++nClear;
        ray.scount += r.count;
        ray.tcount += r.rcount;
    }
    return Spectrum(float(nClear) / float(nSamples));
}

Given a LookAt statement like

LookAt 0 2 0 0 0 0 0 1 0

we get either an assertion hit in debug builds, or an illegal hw instruction trap with release builds. This should be more user friendly....

Where's the 4D noise?

In bounding volume hierarchy building function, I believe there is a condition in one if-statement which prevents us to use the maximum number of primitives in node properly. The code is as follows:

            // Either create leaf or split primitives at selected SAH bucket
            if (nPrimitives > maxPrimsInNode || minCost < nPrimitives) split the node
            else                                                                                       create leaf node

The minCost < nPrimitives is usually fulfilled and a leaf node with maximum primitives in that leaf node is created in very rare cases but as we all knows, when the number of primitives per node is less than maxPrimsInNode, it is better to not split in most cases. Have a look on the attached image. The SAH cost function returns 2.625 for all possible split and it is less than 4 (number of triangles), so the triangles are split. But, it is not give us any performance improvement (it even can adds some overheads).

Upon integrating some of of the spectral code in picogen (http://picogen.org , http://gitorious.org/picogen ), I found that due to a greater-equal in

// Spectrum Method Definitions
bool SpectrumSamplesSorted(const float *lambda, const float *vals, int n) {
    for (int i = 0; i < n-1; ++i)
        if (lambda[i] >= lambda[i+1]) return false;
    return true;
}

(http://github.com/mmp/pbrt-v2/blob/master/core/spectrum.cpp#L28)

there will be an infinite recursion starting in http://github.com/mmp/pbrt-v2/blob/master/core/spectrum.h#L250 in case that duplicate wavelengths are present.

Fix is to replace the greater-equal with a simple greater.

Greets

Sebastian Mach / phresnel(.org)

Just creating a single hyperboloid shape with no parameters causes pbrt to loop forever in the Hyperboloid constructor. The isinf() || isnan() do/while loop never finishes.

The problem is that PBRT is blurring textures at the focal plane in scenes with DOF, a region where everything should be in focus. The problem seems to be in the rays differentials for texture antialiasing.

I prepared a scene showing the problem and comparing the results with other rendering systems:
https://dl.dropboxusercontent.com/u/8033997/test_scene.tar.gz

hi guys,

When I use path tracing to generate a very simple scene image, I got some ringing patterns. The scene is just a Cornell box with back, floor and left wall and a point light source. The material of all the walls are diffuse material. I used 200k samples/pixel to generate the scene. I set the maxDepth=0, which means the generated image only shows the direct light. When I use less samples number, the ringing pattern becomes less obvious. The attached image showed the R channel of the generated image. Any ideas?

Hi guys,

First of all i find the book extreamly instructive and interesting.
I noticed some inconsistency in Distribution1D::SampleContinuous: values of pdf can be greater than 1.0.
In my opinion funcInt should be sum of all input values as stated in
Monte Carlo Rendering with Natural Illumination paper(http://www.cs.virginia.edu/~gfx/courses/2007/ImageSynthesis/assignments/envsample.pdf).

// Compute PDF for sampled offset
if (pdf) *pdf = func[offset] / funcInt;

Thanks,
Looking forward to 3rd edition of your book.

Regards,
Mario Vidov

Since the pixel spacing is computed from dpdx and dpdy of differential geometry, if the camera ray hits many mirror before reaching a diffuse surface, the spacing may become very large. Because the minimum contribution range is
" minSamplePixelSpacing * pixelSpacing "
the cache point will cover a very large area. And if it's position is close to the camera, the artifact appears.

I've been tracking down this bug for the better part of the day. It seems that PBRT cannot handle single-channel EXR images as textures. In my case, it was mapped to a projection light. If you load the single-channel image into Nuke or Photoshop, it displays correctly. If you write a tri-channel image (RGB) with the same information in each channel, PBRT will render this correctly. You can find two example images in the following links. It's not a big issue if this is supported, however I think there should be a warning if only a single channel is read.

https://www.dropbox.com/s/dxao3g8j917b9xz/binary_singleChannel.exr
https://www.dropbox.com/s/u0b103z0juc9v6p/binary_triChannel.exr

Hello, when I used PBRT-V3 to render a scene witch is called "villa-daylight" in the PBRT-V2.It is said that "Warning: Parameter 'gamma' not used".How to change the scene, then we can use the PBRT-V3 to render it.Thank you so much.

root@PBRT:~/pbrt-v2/src# make
/bin/mkdir -p bin objs
Building object objs/core_imageio.o
core/imageio.cpp:137:26: fatal error: ImfInputFile.h: No such file or directory
#include <ImfInputFile.h>
^
compilation terminated.
Makefile:115: recipe for target 'objs/core_imageio.o' failed
make: *** [objs/core_imageio.o] Error 1

I'm in the process of porting the solution files to VS2012. Simply importing the VS2010 project files into VS2012 doesn't work because some of the include and lib paths for the windows API have been changed.

To be specific, the variable $(WindowsSdkDir)include needs to be changed for $(WindowsSDK_IncludePath), and $(WindowsSdkDir)lib\x64 need to be changed to $(WindowsSDK_LibraryPath_x64). (The same thing applies for x86 target, just replace x64 with x86).

I simply replaced all the paths in the relevant property pages (a tedious process). This fixed build issues on my machine, and hopefully this should work on a machine with a fresh VS2012 install.
I have created the relevant pbrt.vs2012 subdirectory to hold all the converted project files.

I'll be pushing that fix to my fork soon and will be glad to issue a pull request if you accept these.

Problem with the zlib visual studio solution file because one of the upgrade files from vc 7.1 is missing.

I can not find file src/windows/stdafx.cpp and folder "windows".

https://github.com/mmp/pbrt-v2/commits/885626bc802446f0da8e5489336608ba8cd71b69/src/windows/stdafx.cpp

(From mantis bug tracker)

There's a small function named SearchPath in dynload.cpp

static string SearchPath(const string &searchpath,
const string &filename)
{
const char _start = searchpath.c_str();
const char *end = start;
while (_start) {
while (*end && *end != PBRT_PATH_SEP[0])
++end;
string component(start, end);

string fn = component + "/" + filename;

FILE _f = fopen(fn.c_str(), "r");
if (f) {
fclose(f);
return fn;
}
if (_end == PBRT_PATH_SEP[0]) ++end;
start = end;
}
return "";

}

the problem is caused by
... ...
string fn = component + "/" + filename;
... ...

The filename variable contains the name of the plugin, and the component variable contains one component of search paths, which are specified in PBRT_SEARCHPATH. When you have a path in PBRT_SEARCHPATH like

c:\pbrt-1.03

Suppose the content of filename variable is mitchell.dll, then the fn becomes

c:\pbrt-1.03/mitchell.dll

which can be correctly found.

But in windows system, when you have a space in the path name, you have to use " " to enclose the path, e.g.

"c:\Program Files\pbrt-1.03"

if you add a path like this to PBRT_SEARCHPATH, then the fn becomes

"c:\Program Files\pbrt-1.03"/mitchell.dll

which can NOT be found by the Windows system, though you have given the correct path and file name.

Wondering if you can consider to put a tag like '2.0' to the repo to make a record of what 2.0 is. Then you can keep work on 'master' branch for other development. Furthermore, that also helps to put pbrt into mxcl/homebrew.

For package manager like homebrew, it is best to grab the source from a publish and static way then a dynamic 'master' branch. I like to help provide the necessary homebrew formula for pbrt as soon as you can provide a tag :)

In Triangle::GetShadingGeometry the shading tangents ss and ts are calculated, but it seems they have the wrong direction, they point in the opposite direction of dpdu and dpdv.

ss = Normalize(dg.dpdu);
ts = Cross(ss, ns);
ss = Cross(ts, ns);

After the above cross products ss points in the opposite direction of dpdu and ts points in the opposite direction of dpdv. But ss is used as dpdu and ts as dpdv in the shading DifferentialGeometry.

p1 = p0 + u * dpdu + v * dpdv;
// using ss and ts in this formula results in p1 lying in the opposite direction

I have checked the code in pbrt-v4 and there the first formula was changed to ts = Cross(ns, ss); which fixes the problem (pbrt-v3 has the same problem).
Is this correct?

Please consider to use tinyexr, embeddable OpenEXR image loader into PBRTv2(or maybe v3?).

https://github.com/syoyo/tinyexr

I saw many PBRT beginner struggle to build and link ILM's OpenEXR library into PBRT, before they actually start to use/hack PBRT.

tinyexr will solve such a problem and users can concentrate on doing light transport using PBRT. It seems tinyexr already has enough features to replace ILM's OpenEXR lib(writing EXR image is not yet, but you can use miniexr https://github.com/aras-p/miniexr )

Using an AmbientOcclusionIntegrator (or another SurfaceIntegrator which Li method does not return physical radiance values) in open scenes with light sources results in some dual behavior:

Spectrum SamplerRenderer::Li(...) const {
    ...
    if (scene->Intersect(ray, isect))
        Li = surfaceIntegrator->Li(...);       // Could be any spectrum
    else {
        // Handle ray that doesn't intersect any geometry
        for (uint32_t i = 0; i < scene->lights.size(); ++i)
            Li += scene->lights[i]->Le(ray); // Always physical radiance spectrum
    }
    ...
}

This dual behavior is illustrated in the image below:

The easiest way to fix this is to let the user take care of it by not adding light sources in the .pbrt file. Or refactor the else part to a new abstract method in the SurfaceIntegrator hierarchy.

Hy,

i have a problem compiling pbrt-v2 correct if stdint.h is include. I've use Visual Studio 2010.
The problem is when pbrt.h and stdint.h are both included (I use OpenCascade 6.7.0 to generate the meshes for pbrt). The problem is the pbrt-typedef of int8_t (i pbrt.h). VC++ 2010 does define int8_t with "signed char" => so typdef does see an error because the 2 definitions of int8_t are not the same.

pbrt.h (at the moment)
...
typedef __int8 int8_t;
typedef unsigned __int8 uint8_t;
typedef __int16 int16_t;
typedef unsigned __int16 uint16_t;
typedef __int32 int32_t;
typedef unsigned __int32 uint32_t;
typedef __int64 int64_t;
typedef unsigned __int64 uint64_t;
...

The problem is the "signed" modifier. typedef does work perfect if the int8_t is define as "signed __int8". The hole pbrt code works perfect with this change - so there should be no problem with that (with VC++).
I have found a little project which tried to build a StdInt.h for Visual Studio C++ compiler (http://msinttypes.googlecode.com/svn/trunk/stdint.h).
Their solution:

// Visual Studio 6 and Embedded Visual C++ 4 doesn't
// realize that, e.g. char has the same size as __int8
// so we give up on __intX for them.

if (_MSC_VER < 1300)

typedef signed char int8_t;
typedef signed short int16_t;
typedef signed int int32_t;
typedef unsigned char uint8_t;
typedef unsigned short uint16_t;
typedef unsigned int uint32_t;

else

typedef signed __int8 int8_t;
typedef signed __int16 int16_t;
typedef signed __int32 int32_t;
typedef unsigned __int8 uint8_t;
typedef unsigned __int16 uint16_t;
typedef unsigned __int32 uint32_t;

endif

typedef signed __int64 int64_t;
typedef unsigned __int64 uint64_t;

If I replace the old definition with the new one, pbrt-v2 will work absolutely perfect regardless of whether stdint.h is included or not. This should work with all VC++ compilers.

How to reproduce the problem:
Add "#include <stdint.h>" before "typedef __int8 int8_t;" in pbrt.h and try to compile with Visual Studio 2010.

Should I make a pull-request?

Greetings,
Tonka

Per Christensen noted the following. (Text slightly edited.)

I came across an issue with the PermutedRadicalInverse() function from the 2nd edition. To explain the issue as simply as possible, assume we’re computing in base 2 and the permutation array p is {1,0} so ones get converted to zeroes and vice versa. (I know that for the particular case of base 2 there are much more efficient ways of doing digit scrambling using e.g. bit-wise xor, but let’s ignore that for now.)

Here’s my version of your function — pretty much verbatim straight out of the book:

float 
PermutedRadicalInverse(unsigned n, const unsigned base, unsigned *perm)
{
    float val = 0.0f;
    float invBase = 1.0f / base, invBi = invBase;
    while (n > 0) {
        unsigned d_i = perm[n%base];
        val += d_i * invBi;
        n *= invBase;
        invBi *= invBase;
    }
    return val;
}

The results I get for n=0..15 are not what I’d expect:

0 0.000000
1 0.000000
2 0.500000
3 0.000000
4 0.750000
5 0.250000
6 0.500000
7 0.000000
8 0.875000
9 0.375000
10 0.625000
11 0.125000
12 0.750000
13 0.250000
14 0.500000
15 0.000000

There are several repeats of 0 and 0.5. It seems to me that the issue is that the loop exits too early — just because n is 0 doesn’t mean we can skip the rest of the digits. Here is a slightly modified version that loops over all digits to give them a chance to be permuted into non-zero values:

float 
PermutedRadicalInverseBetter(unsigned n, const unsigned base, unsigned *perm)
{
    float val = 0.0f;
    float invBase = 1.0f / base, invBi = invBase;
    for (unsigned d = 0; d < 32; d++) { // loop over all digits
        unsigned d_i = perm[n%base];
        val += d_i * invBi;
        n *= invBase;
        invBi *= invBase;
    }
    return val;
}

With this change I get the following values (which seem more reasonable):

0 1.000000
1 0.500000
2 0.750000
3 0.250000
4 0.875000
5 0.375000
6 0.625000
7 0.125000
8 0.937500
9 0.437500
10 0.687500
11 0.187500
12 0.812500
13 0.312500
14 0.562500
15 0.062500

(From mantis bug tracker)

In prebuild.makefile, the "ren" command causes problem.

ren $(core_dir)\pbrtparse.hpp $(core_dir)\pbrtparse.hh

The format there is "ren path\name1 path\name2". Unfortunately, win7 shell says it's invalid because ren only support format like "ren [path]name1 name2".

change

ren $(core_dir)\pbrtparse.hpp $(core_dir)\pbrtparse.hh

to
ren $(core_dir)\pbrtparse.hpp pbrtparse.hh

can fix the problem. Another thing to note is that if the pbrtparse.hh already exist when executing ren, the execution will return 0x1, which will cause nmake to fail.

Any plan to implement L2 norm in exrdiff? Or any known tools out there to do that? The RMS in exrdiff appears to be squaring the R, G, and B distances for each pixel, averaging them across the whole image, and taking the square root, whereas the average L2-norm squares the R, G, and B distances for each pixel, takes the square root of their sum, then averages that across the whole image.

I can whip up a working implementation that could be added if you're interested.

In function PhotonShootingTask::Run, the photon power is initiated by function Light::Sample_L which gives radiance instead of flux. In this case, the result of function EPhoton will have a wrong unit. Any explanation?