Tuesday, August 29, 2017

Building pbrt-v3 in VS 2015, other notes

I thought I'd write other things I found interesting while writing in my blog about how I built pbrt-v3. Again, I hardly know anything about pbrt itself or Cmake, but I still I thought it'd be cool to share what I found about them (and the whole process in general, too).

-Two major things I put here:
  -Debug vs. Release build of pbrt
  -Possible minor bug fix with PBRT_FLOAT_AS_DOUBLE

-The pbrt authors included a Users' Guide in their site. They also provide info. about their Input File Format (probably about the .pbrt files like "teapot-metal.pbrt".

-Biggest thing to note: from the pbrt site, pbrt in "Debug mode" is understandably very slow compared to "Release mode". For "Release mode" and the faster render times, during CMake usage, the option CMAKE_BUILD_TYPE is automatically set to "Release".

But you can still build the VS solution in Debug mode, and the corresponding pbrt.exe outputs the banner *** DEBUG BUILD *** when you run it, to notify you of slow rendering times for debugging purposes. So make sure to also build the solution in Release mode if you just want to render.

Again, the differences are massive. I only found this out today, as I'm writing this! haha. I didn't really read the banner, I was just focused on getting something to work when I first did this.

Here's two images showing how different the rendering times are:

Debug build (also my first successful pbrt render back in January!): 26572.9s = 7.4 hrs



Release build: 880.5s = 14.675 min (30x faster!)


-Possible minor bug fix:

EDIT: got a confirmation from one of the pbrt authors! : ) was a bug. better solution than pointer casting is to actually create a local Float[] array where you can pass in float elements that can be implicitly converted to double elements, see highlighted edit below

(Basically, found if you #define PBRT_FLOAT_AS_DOUBLE, get an error in ptex.cpp, line 130 in VS 2015 saying can't convert a "float*" to "const pbrt::Float[]", which now is "const double[]", explicitly casting the result variable in that line to "const pbrt::Float*" worked for me)

The pbrt site says you can uncomment the //#define PBRT_FLOAT_AS_DOUBLE line in the src/core/pbrt.h file that's in the pbrt repo you can check out. You can do this even if you already have a pbrt VS solution made, since it's just a .h file you're editing.

When I built on Release - x64, however, I got these errors:

C2664 'pbrt::RGBSpectrum pbrt::RGBSpectrum::FromRGB(const pbrt::Float [],pbrt::SpectrumType)': cannot convert argument 1 from 'float *' to 'const pbrt::Float[]'

argument of type "float *" is incompatible with parameter of type "const pbrt::Float"




This is in a file called ptex.cpp, in line 130, in the pbrt project. From looking at the project properties, the pbrt project is supposed to make some "libpbrt.lib" library. With this error, the .lib file isn't made, which caused errors about not being able to open that .lib file.



The function definition containing line 130 is:

template <>
inline Spectrum fromResult<Spectrum>(int nc, float *result) {
if (nc == 1)
return Spectrum(result[0]);
else
return Spectrum::FromRGB(result);
}

Wasn't familiar with the "template <>" or "inline" keywords at the time, but saw with VS that Spectrum::FromRGB takes a "const pbrt::Float *" as its first argument. Looking back at the pbrt.h file, turning on PBRT_FLOAT_AS_DOUBLE changes the Float typedef from a float to a double (also, the pbrt.h defines this in a pbrt namespace [hence pbrt::Float]):

#define PBRT_FLOAT_AS_DOUBLE
#ifdef PBRT_FLOAT_AS_DOUBLE
typedef double Float;
#else
typedef float Float;
#endif  // PBRT_FLOAT_AS_DOUBLE

So going back to that code snippet, since "const pbrt::Float*" pretty much meants "const double *", passing "result" tries to implicitly convert a float* to a double*, and apparently that's an error in VS 2015 (not sure if that's universal).

So I put an explicit cast like below, built the solution again, and it worked.

return Spectrum::FromRGB((const pbrt::Float*)result);

I wrote about the bug fix I did in the Github issue tracker for the pbrt repo. It's minor compared to the entire scope of pbrt itself, but I thought it could help. Interestingly, I looked back at the pbrt solution I made back in January, and didn't find a ptex.cpp file there. Looks like this is relatively new. May be related to some texture cache feature they implemented back in March, according to their site news.

Edit  Got a confirmation that this line was a bug. One of the pbrt authors pushed a fix today

template <>
inline Spectrum fromResult<Spectrum>(int nc, float *result) {
if (nc == 1)
return Spectrum(result[0]);
else {
Float rgb[3] = { result[0], result[1], result[2] };
return Spectrum::FromRGB(rgb);
}

}


This is a better solution, to actually pass a pbrt::Float[] into Spectrum::FromRGB by creating a local Float array variable called "rgb". (I assume the size is 3 just because "rgb" is 3 values.) Looks like since it's assured that the array pointed to by "result" is size 3, can initialize the "rgb" by passing in float values pointed to by the "result" array. The values are then implicitly converted to double values since Float is of type double due to the PBRT_FLOAT_AS_DOUBLE define, and then the array is good to go to be passed into the "FromRGB" function.

Cleaner and makes more sense than converting a float* to a double*, but still, happy I got some sort of bug fix working. Pretty cool, this would be the first bug I ever found in source code as big as this. Minor bug, but still, big self-achievement for me! : )

-The pbrt site says it supports pfm, exr, tga, png file formats (where pfm and exr are more "accurate" file formats because they can store floats for what I think are color values). In the "teapot-metal.pbrt" file, in line 9 (doing this from Notepad++), you can change "teapot-metal.exr" to "teapot-metal.png" if you just want to see the image in a program like IrfanView immediately.



The pbrt-v3-scenes folder contains already made .exr files at pbrt-v3-scenes/images. A teapot-metal.exr is available in the "simple" folder.


-If you try to "Download ZIP" at the pbrt repo site or try to do clone the repo w/o the --recursive flag, you'll get an error in CMake GUI about the flag.


-When running "pbrt teapot-metal.pbrt" in cmd, teapot-metal.pbrt references other files. If it doesn't find them, it shows a warning, like below. The .pbrt file uses custom-made text inputs, from what I'm seeing. They include references to other files.



No comments:

Post a Comment