Merge pull request #544 from k-badz/kbadz/black_image_fix

Fix recent-MSVC device miscompile causing NaNs and black images

src/pbrt/film.cpp

@@ -502,7 +502,7 @@ PBRT_CPU_GPU void RGBFilm::AddSplat(Point2f p, SampledSpectrum L, const SampledW
     RGB rgb = sensor->ToSensorRGB(L, lambda);
 
     // Optionally clamp sensor RGB value
-    Float m = std::max({rgb.r, rgb.g, rgb.b});
+    Float m = std::max(rgb.r, std::max(rgb.g, rgb.b));
     if (m > maxComponentValue)
         rgb *= maxComponentValue / m;
 
@@ -540,7 +540,7 @@ Image RGBFilm::GetImage(ImageMetadata *metadata, Float splatScale) {
     ParallelFor2D(pixelBounds, [&](Point2i p) {
         RGB rgb = GetPixelRGB(p, splatScale);
 
-        if (writeFP16 && std::max({rgb.r, rgb.g, rgb.b}) > 65504) {
+        if (writeFP16 && std::max(rgb.r, std::max(rgb.g, rgb.b)) > 65504) {
             if (rgb.r > 65504)
                 rgb.r = 65504;
             if (rgb.g > 65504)
@@ -589,7 +589,7 @@ PBRT_CPU_GPU void GBufferFilm::AddSample(Point2i pFilm, SampledSpectrum L,
                             const SampledWavelengths &lambda,
                             const VisibleSurface *visibleSurface, Float weight) {
     RGB rgb = sensor->ToSensorRGB(L, lambda);
-    Float m = std::max({rgb.r, rgb.g, rgb.b});
+    Float m = std::max(rgb.r, std::max(rgb.g, rgb.b));
     if (m > maxComponentValue)
         rgb *= maxComponentValue / m;
 
@@ -661,7 +661,7 @@ PBRT_CPU_GPU void GBufferFilm::AddSplat(Point2f p, SampledSpectrum v,
     // NOTE: same code as RGBFilm::AddSplat()...
     CHECK(!v.HasNaNs());
     RGB rgb = sensor->ToSensorRGB(v, lambda);
-    Float m = std::max({rgb.r, rgb.g, rgb.b});
+    Float m = std::max(rgb.r, std::max(rgb.g, rgb.b));
     if (m > maxComponentValue)
         rgb *= maxComponentValue / m;
 
@@ -758,7 +758,7 @@ Image GBufferFilm::GetImage(ImageMetadata *metadata, Float splatScale) {
 
         rgb = outputRGBFromSensorRGB * rgb;
 
-        if (writeFP16 && std::max({rgb.r, rgb.g, rgb.b}) > 65504) {
+        if (writeFP16 && std::max(rgb.r, std::max(rgb.g, rgb.b)) > 65504) {
             if (rgb.r > 65504)
                 rgb.r = 65504;
             if (rgb.g > 65504)
@@ -917,7 +917,7 @@ PBRT_CPU_GPU void SpectralFilm::AddSplat(Point2f p, SampledSpectrum L,
     RGB rgb = sensor->ToSensorRGB(L, lambda);
 
     // Optionally clamp sensor RGB value
-    Float m = std::max({rgb.r, rgb.g, rgb.b});
+    Float m = std::max(rgb.r, std::max(rgb.g, rgb.b));
     if (m > maxComponentValue)
         rgb *= maxComponentValue / m;
 

src/pbrt/film.h

@@ -242,7 +242,7 @@ class RGBFilm : public FilmBase {
         RGB rgb = sensor->ToSensorRGB(L, lambda);
 
         // Optionally clamp sensor RGB value
-        Float m = std::max({rgb.r, rgb.g, rgb.b});
+        Float m = std::max(rgb.r, std::max(rgb.g, rgb.b));
         if (m > maxComponentValue)
             rgb *= maxComponentValue / m;
 
@@ -419,7 +419,7 @@ class SpectralFilm : public FilmBase {
         RGB rgb = sensor->ToSensorRGB(L, lambda);
 
         // Optionally clamp sensor RGB value
-        Float m = std::max({rgb.r, rgb.g, rgb.b});
+        Float m = std::max(rgb.r, std::max(rgb.g, rgb.b));
         if (m > maxComponentValue)
             rgb *= maxComponentValue / m;
 

src/pbrt/lights.cpp

@@ -385,8 +385,9 @@ pstd::optional<LightBounds> ProjectionLight::Bounds() const {
     Float sum = 0;
     for (int v = 0; v < image.Resolution().y; ++v)
         for (int u = 0; u < image.Resolution().x; ++u)
-            sum += std::max({image.GetChannel({u, v}, 0), image.GetChannel({u, v}, 1),
+            sum += std::max(image.GetChannel({u, v}, 0),
-                             image.GetChannel({u, v}, 2)});
+                            std::max(image.GetChannel({u, v}, 1),
+                                     image.GetChannel({u, v}, 2)));
     Float phi = scale * sum / (image.Resolution().x * image.Resolution().y);
 
     Point3f pCorner(screenBounds.pMax.x, screenBounds.pMax.y, 0);

src/pbrt/shapes.cpp

@@ -1121,7 +1121,7 @@ PBRT_CPU_GPU DirectionCone BilinearPatch::NormalBounds() const {
 
     // Compute average normal and return normal bounds for patch
     Vector3f n = Normalize(n00 + n10 + n01 + n11);
-    Float cosTheta = std::min({Dot(n, n00), Dot(n, n01), Dot(n, n10), Dot(n, n11)});
+    Float cosTheta = std::min(std::min(Dot(n, n00), Dot(n, n01)), std::min(Dot(n, n10), Dot(n, n11)));
     return DirectionCone(n, Clamp(cosTheta, -1, 1));
 }
 

src/pbrt/util/color.h

@@ -364,13 +364,18 @@ class RGBSigmoidPolynomial {
     Float c0, c1, c2;
 };
 
+// Namespace scope, not a class member: recent MSVC build tools reject a member
+// used as an array extent in a member type-alias (CoefficientArray below).
+inline constexpr int RGBToSpectrumTableRes = 64;
+
 // RGBToSpectrumTable Definition
 class RGBToSpectrumTable {
   public:
     // RGBToSpectrumTable Public Constants
-    static constexpr int res = 64;
+    static constexpr int res = RGBToSpectrumTableRes;
 
-    using CoefficientArray = float[3][res][res][res][3];
+    using CoefficientArray = float[3][RGBToSpectrumTableRes][RGBToSpectrumTableRes]
+                                  [RGBToSpectrumTableRes][3];
 
     // RGBToSpectrumTable Public Methods
     RGBToSpectrumTable(const float *zNodes, const CoefficientArray *coeffs)

src/pbrt/util/math.h

@@ -885,8 +885,11 @@ class Interval {
                        MulRoundDown(low, i.high), MulRoundDown(high, i.high)};
         Float hp[4] = {MulRoundUp(low, i.low), MulRoundUp(high, i.low),
                        MulRoundUp(low, i.high), MulRoundUp(high, i.high)};
-        return {std::min({lp[0], lp[1], lp[2], lp[3]}),
+        // std::min/max's initializer_list overload (std::min({...})) miscompiles
-                std::max({hp[0], hp[1], hp[2], hp[3]})};
+        // in device code on recent MSVC build tools causing black GPU
+        // image; Use binary min/max. See mmp/pbrt-v4#495.
+        return {std::min(std::min(lp[0], lp[1]), std::min(lp[2], lp[3])),
+                std::max(std::max(hp[0], hp[1]), std::max(hp[2], hp[3]))};
     }
 
     PBRT_CPU_GPU
@@ -966,8 +969,8 @@ PBRT_CPU_GPU inline Interval Interval::operator/(Interval i) const {
                         DivRoundDown(low, i.high), DivRoundDown(high, i.high)};
     Float highQuot[4] = {DivRoundUp(low, i.low), DivRoundUp(high, i.low),
                          DivRoundUp(low, i.high), DivRoundUp(high, i.high)};
-    return {std::min({lowQuot[0], lowQuot[1], lowQuot[2], lowQuot[3]}),
+    return {std::min(std::min(lowQuot[0], lowQuot[1]), std::min(lowQuot[2], lowQuot[3])),
-            std::max({highQuot[0], highQuot[1], highQuot[2], highQuot[3]})};
+            std::max(std::max(highQuot[0], highQuot[1]), std::max(highQuot[2], highQuot[3]))};
 }
 
 PBRT_CPU_GPU inline Interval Sqr(Interval i) {
@@ -1075,14 +1078,16 @@ PBRT_CPU_GPU inline Interval sqrt(Interval i) {
 }
 
 PBRT_CPU_GPU inline Interval FMA(Interval a, Interval b, Interval c) {
-    Float low = std::min({FMARoundDown(a.LowerBound(), b.LowerBound(), c.LowerBound()),
+    Float low = std::min(
-                          FMARoundDown(a.UpperBound(), b.LowerBound(), c.LowerBound()),
+        std::min(FMARoundDown(a.LowerBound(), b.LowerBound(), c.LowerBound()),
-                          FMARoundDown(a.LowerBound(), b.UpperBound(), c.LowerBound()),
+                 FMARoundDown(a.UpperBound(), b.LowerBound(), c.LowerBound())),
-                          FMARoundDown(a.UpperBound(), b.UpperBound(), c.LowerBound())});
+        std::min(FMARoundDown(a.LowerBound(), b.UpperBound(), c.LowerBound()),
-    Float high = std::max({FMARoundUp(a.LowerBound(), b.LowerBound(), c.UpperBound()),
+                 FMARoundDown(a.UpperBound(), b.UpperBound(), c.LowerBound())));
-                           FMARoundUp(a.UpperBound(), b.LowerBound(), c.UpperBound()),
+    Float high = std::max(
-                           FMARoundUp(a.LowerBound(), b.UpperBound(), c.UpperBound()),
+        std::max(FMARoundUp(a.LowerBound(), b.LowerBound(), c.UpperBound()),
-                           FMARoundUp(a.UpperBound(), b.UpperBound(), c.UpperBound())});
+                 FMARoundUp(a.UpperBound(), b.LowerBound(), c.UpperBound())),
+        std::max(FMARoundUp(a.LowerBound(), b.UpperBound(), c.UpperBound()),
+                 FMARoundUp(a.UpperBound(), b.UpperBound(), c.UpperBound())));
     return Interval(low, high);
 }
 
@@ -1090,16 +1095,16 @@ PBRT_CPU_GPU inline Interval DifferenceOfProducts(Interval a, Interval b, Interv
                                                   Interval d) {
     Float ab[4] = {a.LowerBound() * b.LowerBound(), a.UpperBound() * b.LowerBound(),
                    a.LowerBound() * b.UpperBound(), a.UpperBound() * b.UpperBound()};
-    Float abLow = std::min({ab[0], ab[1], ab[2], ab[3]});
+    Float abLow = std::min(std::min(ab[0], ab[1]), std::min(ab[2], ab[3]));
-    Float abHigh = std::max({ab[0], ab[1], ab[2], ab[3]});
+    Float abHigh = std::max(std::max(ab[0], ab[1]), std::max(ab[2], ab[3]));
     int abLowIndex = abLow == ab[0] ? 0 : (abLow == ab[1] ? 1 : (abLow == ab[2] ? 2 : 3));
     int abHighIndex =
         abHigh == ab[0] ? 0 : (abHigh == ab[1] ? 1 : (abHigh == ab[2] ? 2 : 3));
 
     Float cd[4] = {c.LowerBound() * d.LowerBound(), c.UpperBound() * d.LowerBound(),
                    c.LowerBound() * d.UpperBound(), c.UpperBound() * d.UpperBound()};
-    Float cdLow = std::min({cd[0], cd[1], cd[2], cd[3]});
+    Float cdLow = std::min(std::min(cd[0], cd[1]), std::min(cd[2], cd[3]));
-    Float cdHigh = std::max({cd[0], cd[1], cd[2], cd[3]});
+    Float cdHigh = std::max(std::max(cd[0], cd[1]), std::max(cd[2], cd[3]));
     int cdLowIndex = cdLow == cd[0] ? 0 : (cdLow == cd[1] ? 1 : (cdLow == cd[2] ? 2 : 3));
     int cdHighIndex =
         cdHigh == cd[0] ? 0 : (cdHigh == cd[1] ? 1 : (cdHigh == cd[2] ? 2 : 3));

src/pbrt/util/mipmap.cpp

@@ -229,8 +229,8 @@ template <typename T>
 T MIPMap::Filter(Point2f st, Vector2f dst0, Vector2f dst1) const {
     if (options.filter != FilterFunction::EWA) {
         // Handle non-EWA MIP Map filter
-        Float width = 2 * std::max({std::abs(dst0[0]), std::abs(dst0[1]),
+        Float width = 2 * std::max(std::max(std::abs(dst0[0]), std::abs(dst0[1])),
-                                    std::abs(dst1[0]), std::abs(dst1[1])});
+                                    std::max(std::abs(dst1[0]), std::abs(dst1[1])));
         // Compute MIP Map level for _width_ and handle very wide filter
         int nLevels = Levels();
         Float level = nLevels - 1 + Log2(std::max<Float>(width, 1e-8));

src/pbrt/util/spectrum.cpp

@@ -228,20 +228,20 @@ PBRT_CPU_GPU RGB SampledSpectrum::ToRGB(const SampledWavelengths &lambda,
 }
 
 PBRT_CPU_GPU RGBAlbedoSpectrum::RGBAlbedoSpectrum(const RGBColorSpace &cs, RGB rgb) {
-    DCHECK_LE(std::max({rgb.r, rgb.g, rgb.b}), 1);
+    DCHECK_LE(std::max(rgb.r, std::max(rgb.g, rgb.b)), 1);
-    DCHECK_GE(std::min({rgb.r, rgb.g, rgb.b}), 0);
+    DCHECK_GE(std::min(rgb.r, std::min(rgb.g, rgb.b)), 0);
     rsp = cs.ToRGBCoeffs(rgb);
 }
 
 PBRT_CPU_GPU RGBUnboundedSpectrum::RGBUnboundedSpectrum(const RGBColorSpace &cs, RGB rgb) {
-    Float m = std::max({rgb.r, rgb.g, rgb.b});
+    Float m = std::max(rgb.r, std::max(rgb.g, rgb.b));
     scale = 2 * m;
     rsp = cs.ToRGBCoeffs(scale ? rgb / scale : RGB(0, 0, 0));
 }
 
 PBRT_CPU_GPU RGBIlluminantSpectrum::RGBIlluminantSpectrum(const RGBColorSpace &cs, RGB rgb)
     : illuminant(&cs.illuminant) {
-    Float m = std::max({rgb.r, rgb.g, rgb.b});
+    Float m = std::max(rgb.r, std::max(rgb.g, rgb.b));
     scale = 2 * m;
     rsp = cs.ToRGBCoeffs(scale ? rgb / scale : RGB(0, 0, 0));
 }

src/pbrt/util/vecmath.h

@@ -223,7 +223,7 @@ PBRT_CPU_GPU inline C<T> Min(Tuple2<C, T> t0, Tuple2<C, T> t1) {
 template <template <class> class C, typename T>
 PBRT_CPU_GPU inline T MinComponentValue(Tuple2<C, T> t) {
     using std::min;
-    return min({t.x, t.y});
+    return min(t.x, t.y);
 }
 
 template <template <class> class C, typename T>
@@ -240,7 +240,7 @@ PBRT_CPU_GPU inline C<T> Max(Tuple2<C, T> t0, Tuple2<C, T> t1) {
 template <template <class> class C, typename T>
 PBRT_CPU_GPU inline T MaxComponentValue(Tuple2<C, T> t) {
     using std::max;
-    return max({t.x, t.y});
+    return max(t.x, t.y);
 }
 
 template <template <class> class C, typename T>
@@ -430,7 +430,7 @@ PBRT_CPU_GPU inline C<T> Min(Tuple3<C, T> t1, Tuple3<C, T> t2) {
 template <template <class> class C, typename T>
 PBRT_CPU_GPU inline T MinComponentValue(Tuple3<C, T> t) {
     using std::min;
-    return min({t.x, t.y, t.z});
+    return min(t.x, min(t.y, t.z));
 }
 
 template <template <class> class C, typename T>
@@ -447,7 +447,7 @@ PBRT_CPU_GPU inline C<T> Max(Tuple3<C, T> t1, Tuple3<C, T> t2) {
 template <template <class> class C, typename T>
 PBRT_CPU_GPU inline T MaxComponentValue(Tuple3<C, T> t) {
     using std::max;
-    return max({t.x, t.y, t.z});
+    return max(t.x, max(t.y, t.z));
 }
 
 template <template <class> class C, typename T>
@@ -1770,7 +1770,7 @@ class OctahedralVector {
   private:
     // OctahedralVector Private Methods
     PBRT_CPU_GPU
-    static Float Sign(Float v) { return std::copysign(1.f, v); }
+    static Float Sign(Float v) { return pstd::copysign(Float(1), v); }
 
     PBRT_CPU_GPU
     static uint16_t Encode(Float f) {