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.88k stars 1.19k forks source link

Possible minor bug fix in using PBRT_FLOAT_AS_DOUBLE #151

Closed neobis001 closed 7 years ago

neobis001 commented 7 years ago

I used CMake to create a pbrt Visual Studio 2015 solution, with CMAKE_BUILD_TYPE "Release" and the solution build settings as Release - x64. From the Users Guide in the pbrt site, I uncommented "#define PBRT_FLOAT_AS_DOUBLE" in src/core/pbrt.h.

When I tried to build the solution, I got this error in the pbrt project, in the ptex.cpp file:

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

The line is

return Spectrum::FromRGB(result);

where result is a 'float' parameter in a function called "fromResult". Seems like defining PBRT_FLOAT_AS_DOUBLE makes 'Float' a double, and an implicit convesion from float to double* isn't allowed. So I made an explicit cast like below, and the build worked:

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

Would this be considered a bug and a working bug fix? I've only built pbrt to try out a render, haven't seriously studied it yet. The ptex.cpp file looks relatively new, seems related to the March 5 texture cache feature you mention on your site. Don't see that file in my first pbrt build I made back in January.

mmp commented 7 years ago

The intermediate value needs to be explicitly copied into a Float array; the ptex code always returns float values, but if Float is double, then we need to copy into a Float array to convert to doubles so that the cast is valid. Just pushed a fix to do just that.

Thanks for reporting this!