color: slightly tweak the RGB to HSV and HSL conversions for

improved numerical stability on i386.

src/lol/image/color.h

@@ -88,14 +88,16 @@ public:
     {
         float K = 0.f;
 
-        if (src.b - src.g > 1e-20f)
+        if (src.g < src.b)
             src = src.rbg, K = -1.f;
 
-        if (src.g - src.r > 1e-20f)
+        if (src.r < src.g)
             src = src.grb, K = -2.f / 6.f - K;
 
         float chroma = src.r - min(src.g, src.b);
-        return vec3(abs(K + (src.g - src.b) / (6.f * chroma + 1e-20f)),
+        /* XXX: we use min() here because numerical stability is not
+         * guaranteed with -ffast-math, I’ve seen it fail on i386. */
+        return vec3(min(abs(K + (src.g - src.b) / (6.f * chroma)), 1.f),
                     chroma / (src.r + 1e-20f),
                     src.r);
     }
@@ -112,20 +114,17 @@ public:
     {
         float K = 0.f;
 
-        /* FIXME: this appears to be needed for numerical stability on
-         * i386 hardware using -ffast-math. Otherwise if (src.g < src.b)
-         * would suffice. */
-        if (src.b - src.g > 1e-20f)
+        if (src.g < src.b)
             src = src.rbg, K = -1.f;
 
-        if (src.g - src.r > 1e-20f)
+        if (src.r < src.g)
             src = src.grb, K = -2.f / 6.f - K;
 
         float chroma = src.r - min(src.g, src.b);
         float luma = src.r + min(src.g, src.b);
-        return vec3(abs(K + (src.g - src.b) / (6.f * chroma + 1e-20f)),
-                    chroma / (min(luma, 2.f - luma) + 1e-20f),
-                    0.5f * luma);
+        float h = min(abs(K + (src.g - src.b) / (6.f * chroma)), 1.f);
+        float s = clamp(chroma / (min(luma, 2.f - luma)), 0.f, 1.f);
+        return vec3(h, s, 0.5f * luma);
     }
 
     static vec4 RGBToHSL(vec4 src)

test/unit/color.cpp

@@ -84,6 +84,7 @@ LOLUNIT_FIXTURE(ColorTest)
             LOLUNIT_SET_CONTEXT(n / 7);
             LOLUNIT_ASSERT_DOUBLES_EQUAL(d1, d2, 0.0001);
             LOLUNIT_ASSERT_DOUBLES_EQUAL(d2, d3, 0.0001);
+            LOLUNIT_UNSET_CONTEXT();
         }
     }
 
@@ -94,11 +95,18 @@ LOLUNIT_FIXTURE(ColorTest)
         for (int b = 0; b < 20; b++)
         {
             vec3 v1 = vec3(r / 20.f, g / 20.f, b / 20.f);
-            vec3 v2 = Color::HSVToRGB(Color::RGBToHSV(v1));
+            vec3 v2 = Color::RGBToHSV(v1);
+            vec3 v3 = Color::HSVToRGB(v2);
 
-            LOLUNIT_ASSERT_DOUBLES_EQUAL(v1.r, v2.r, 0.0001);
-            LOLUNIT_ASSERT_DOUBLES_EQUAL(v1.g, v2.g, 0.0001);
-            LOLUNIT_ASSERT_DOUBLES_EQUAL(v1.b, v2.b, 0.0001);
+            String rgb = String::Printf("[%f %f %f]", v1.r, v1.g, v1.b);
+            LOLUNIT_SET_CONTEXT(&rgb[0]);
+
+            if (r != g || g != b)
+                LOLUNIT_ASSERT_DOUBLES_EQUAL(v1.r, v3.r, 0.0001);
+            LOLUNIT_ASSERT_DOUBLES_EQUAL(v1.g, v3.g, 0.0001);
+            LOLUNIT_ASSERT_DOUBLES_EQUAL(v1.b, v3.b, 0.0001);
+
+            LOLUNIT_UNSET_CONTEXT();
         }
     }
 
@@ -112,9 +120,15 @@ LOLUNIT_FIXTURE(ColorTest)
             vec3 v2 = Color::RGBToHSL(v1);
             vec3 v3 = Color::HSVToHSL(Color::RGBToHSV(v1));
 
-            LOLUNIT_ASSERT_DOUBLES_EQUAL(v2.x, v3.x, 0.0001);
+            String rgb = String::Printf("[%f %f %f]", v1.r, v1.g, v1.b);
+            LOLUNIT_SET_CONTEXT(&rgb[0]);
+
+            if (r != g || g != b)
+                LOLUNIT_ASSERT_DOUBLES_EQUAL(v2.x, v3.x, 0.0001);
             LOLUNIT_ASSERT_DOUBLES_EQUAL(v2.y, v3.y, 0.0001);
             LOLUNIT_ASSERT_DOUBLES_EQUAL(v2.z, v3.z, 0.0001);
+
+            LOLUNIT_UNSET_CONTEXT();
         }
     }
 };