Static analysis fixes

Fixes for static analysis warnings reported by PVS, including the following: - Remove a signed bitshift operation with undefined C++ behavior, replacing it with the intended constant result. - Restrict the capture scope of two lambdas, removing access to temporary variables. Also: - Add initial unit tests for the Half class, providing validation for future work.
AcademySoftwareFoundation · Jan 12, 2020 · 6ee4759 · 6ee4759
1 parent daaf86e
commit 6ee4759
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 24 deletions.
diff --git a/source/MaterialXRender/Types.h b/source/MaterialXRender/Types.h
@@ -101,7 +101,7 @@ class Half
     static constexpr int32_t const nanN = (infC + 1) << shift; // minimum flt16 nan as a flt32
     static constexpr int32_t const maxC = maxN >> shift;
     static constexpr int32_t const minC = minN >> shift;
-    static constexpr int32_t const signC = signN >> shiftSign; // flt16 sign bit
+    static constexpr int32_t const signC = (int32_t) 0x00008000; // flt16 sign bit
 
     static constexpr int32_t const mulN = 0x52000000; // (1 << 23) / minN
     static constexpr int32_t const mulC = 0x33800000; // minN / (1 << (23 - shift))

diff --git a/source/MaterialXTest/Render.cpp b/source/MaterialXTest/Render.cpp
@@ -4,41 +4,57 @@
 //
 
 #include <MaterialXTest/Catch/catch.hpp>
-#include <MaterialXTest/GenShaderUtil.h>
 
-#include <MaterialXCore/Document.h>
-
-#include <MaterialXFormat/XmlIo.h>
-
-#include <MaterialXGenShader/Util.h>
-#include <MaterialXGenShader/HwShaderGenerator.h>
-#include <MaterialXGenShader/DefaultColorManagementSystem.h>
+#include <MaterialXTest/RenderUtil.h>
 
 #include <MaterialXRender/ShaderRenderer.h>
-#include <MaterialXRender/Util.h>
-
-#include <MaterialXTest/RenderUtil.h>
+#include <MaterialXRender/StbImageLoader.h>
+#include <MaterialXRender/TinyObjLoader.h>
+#include <MaterialXRender/Types.h>
 
-#ifdef MATERIALX_BUILD_CONTRIB
-#include <MaterialXContrib/Handlers/TinyEXRImageLoader.h>
-#endif
 #ifdef MATERIALX_BUILD_OIIO
 #include <MaterialXRender/OiioImageLoader.h>
 #endif
-#include <MaterialXRender/StbImageLoader.h>
-
-#include <MaterialXRender/GeometryHandler.h>
-#include <MaterialXRender/TinyObjLoader.h>
+#ifdef MATERIALX_BUILD_CONTRIB
+#include <MaterialXContrib/Handlers/TinyEXRImageLoader.h>
+#endif
 
 #include <fstream>
 #include <iostream>
+#include <limits>
 #include <unordered_set>
-#include <chrono>
-#include <ctime>
 
 namespace mx = MaterialX;
 
-#define LOG_TO_FILE
+TEST_CASE("Render: Half Float", "[rendercore]")
+{
+    const std::vector<float> values =
+    {
+        0.0f, 0.25f, 0.5f, 0.75f,
+        1.0f, 8.0f, 64.0f, 512.0f,
+        std::numeric_limits<float>::infinity()
+    };
+    const std::vector<float> signs = { 1.0f, -1.0f };
+
+    for (float value : values)
+    {
+        for (float sign : signs)
+        {
+            float f(value * sign);
+            mx::Half h(f);
+            REQUIRE(h == f);
+            REQUIRE(h + mx::Half(1.0f) == f + 1.0f);
+            REQUIRE(h - mx::Half(1.0f) == f - 1.0f);
+            REQUIRE(h * mx::Half(2.0f) == f * 2.0f);
+            REQUIRE(h / mx::Half(2.0f) == f / 2.0f);
+            REQUIRE((h += mx::Half(3.0f)) == (f += 3.0f));
+            REQUIRE((h -= mx::Half(3.0f)) == (f -= 3.0f));
+            REQUIRE((h *= mx::Half(4.0f)) == (f *= 4.0f));
+            REQUIRE((h /= mx::Half(4.0f)) == (f /= 4.0f));
+            REQUIRE(-h == -f);
+        }
+    }
+}
 
 struct GeomHandlerTestOptions
 {

diff --git a/source/MaterialXView/Editor.cpp b/source/MaterialXView/Editor.cpp
@@ -51,7 +51,7 @@ class EditorColorPicker : public ng::ColorPicker
             _colorWidgets[i]->setFixedSize(ng::Vector2i(70, 20));
             _colorWidgets[i]->setFontSize(15);
             _colorWidgets[i]->setSpinnable(true);
-            _colorWidgets[i]->setCallback([&](float)
+            _colorWidgets[i]->setCallback([this](float)
             {
                 ng::Color value(_colorWidgets[0]->value(), _colorWidgets[1]->value(), _colorWidgets[2]->value(), _colorWidgets[3]->value());
                 mColorWheel->setColor(value);
@@ -62,7 +62,7 @@ class EditorColorPicker : public ng::ColorPicker
 
         // The color wheel does not handle alpha properly, so only
         // overwrite RGB in the callback.
-        mCallback = [&](const ng::Color &value) {
+        mCallback = [this](const ng::Color &value) {
             _colorWidgets[0]->setValue(value[0]);
             _colorWidgets[1]->setValue(value[1]);
             _colorWidgets[2]->setValue(value[2]);