Fix mapping NaN inputs for E5B9G9R9 to 0
Section 15.2.1. RGB to Shared Exponent Conversion of the Vulkan 1.1 spec
states "NaN, if supported, is handled as in IEEE 754-2008 minNum() and
maxNum(). That is the result is a NaN is mapped to zero."
This currently does not affect anything because E5B9G9R9 is not a
renderable format. The blitter merely has to support writing this format
for computing the corner texel of cube map borders, which are averaged
from other E5B9G9R9 format texels and thus no NaN can be encountered.
Bug: b/138944025
Change-Id: Ib69aca3af5afee240d9b697c47b2f1a0204fe697
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/35008
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Reviewed-by: Sean Risser <srisser@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Device/Blitter.cpp b/src/Device/Blitter.cpp
index 198f108..c8298ed 100644
--- a/src/Device/Blitter.cpp
+++ b/src/Device/Blitter.cpp
@@ -642,10 +642,10 @@
// Maximum representable value.
constexpr float sharedexp_max = ((static_cast<float>(1 << N) - 1) / static_cast<float>(1 << N)) * static_cast<float>(1 << (E_max - B));
- // Clamp components to valid range.
- Float red_c = Max(0, Min(sharedexp_max, c.x));
- Float green_c = Max(0, Min(sharedexp_max, c.y));
- Float blue_c = Max(0, Min(sharedexp_max, c.z));
+ // Clamp components to valid range. NaN becomes 0.
+ Float red_c = Min(IfThenElse(!(c.x > 0), Float(0), Float(c.x)), sharedexp_max);
+ Float green_c = Min(IfThenElse(!(c.y > 0), Float(0), Float(c.y)), sharedexp_max);
+ Float blue_c = Min(IfThenElse(!(c.z > 0), Float(0), Float(c.z)), sharedexp_max);
// We're reducing the mantissa to 9 bits, so we must round up if the next
// bit is 1. In other words add 0.5 to the new mantissa's position and
diff --git a/src/System/Half.hpp b/src/System/Half.hpp
index d908f35..e77224d 100644
--- a/src/System/Half.hpp
+++ b/src/System/Half.hpp
@@ -80,10 +80,10 @@
static_cast<float>(1 << g_sharedexp_mantissabits)) *
static_cast<float>(1 << (g_sharedexp_maxexponent - g_sharedexp_bias));
- // Clamp components to valid range.
- const float red_c = std::max<float>(0, std::min(g_sharedexp_max, r));
- const float green_c = std::max<float>(0, std::min(g_sharedexp_max, g));
- const float blue_c = std::max<float>(0, std::min(g_sharedexp_max, b));
+ // Clamp components to valid range. NaN becomes 0.
+ const float red_c = std::min(!(r > 0) ? 0 : r, g_sharedexp_max);
+ const float green_c = std::min(!(g > 0) ? 0 : g, g_sharedexp_max);
+ const float blue_c = std::min(!(b > 0) ? 0 : b, g_sharedexp_max);
// We're reducing the mantissa to 9 bits, so we must round up if the next
// bit is 1. In other words add 0.5 to the new mantissa's position and
diff --git a/tests/MathUnitTests/unittests.cpp b/tests/MathUnitTests/unittests.cpp
index c56940b..dbedd9e 100644
--- a/tests/MathUnitTests/unittests.cpp
+++ b/tests/MathUnitTests/unittests.cpp
@@ -21,6 +21,19 @@
using namespace sw;
+// Clamps to the [0, hi] range. NaN input produces 0, hi must be non-NaN.
+float clamp0hi(float x, float hi)
+{
+ // If x=NaN, x > 0 will compare false and we return 0.
+ if(!(x > 0))
+ {
+ return 0;
+ }
+
+ // x is non-NaN at this point, so std::min() is safe for non-NaN hi.
+ return std::min(x, hi);
+}
+
unsigned int RGB9E5_reference(float r, float g, float b)
{
// Vulkan 1.1.117 section 15.2.1 RGB to Shared Exponent Conversion
@@ -39,12 +52,12 @@
static_cast<float>(1 << g_sharedexp_mantissabits)) *
static_cast<float>(1 << (g_sharedexp_maxexponent - g_sharedexp_bias));
- const float red_c = std::max<float>(0, std::min(g_sharedexp_max, r));
- const float green_c = std::max<float>(0, std::min(g_sharedexp_max, g));
- const float blue_c = std::max<float>(0, std::min(g_sharedexp_max, b));
+ const float red_c = clamp0hi(r, g_sharedexp_max);
+ const float green_c = clamp0hi(g, g_sharedexp_max);
+ const float blue_c = clamp0hi(b, g_sharedexp_max);
- const float max_c = std::max<float>(std::max<float>(red_c, green_c), blue_c);
- const float exp_p = std::max<float>(-g_sharedexp_bias - 1, floor(log2(max_c))) + 1 + g_sharedexp_bias;
+ const float max_c = fmax(fmax(red_c, green_c), blue_c);
+ const float exp_p = fmax(-g_sharedexp_bias - 1, floor(log2(max_c))) + 1 + g_sharedexp_bias;
const int max_s = static_cast<int>(floor((max_c / exp2(exp_p - g_sharedexp_bias - g_sharedexp_mantissabits)) + 0.5f));
const int exp_s = static_cast<int>((max_s < exp2(g_sharedexp_mantissabits)) ? exp_p : exp_p + 1);
