From e14a8d68a237dc694dbb75db8496d74a7c0b1af3 Mon Sep 17 00:00:00 2001 From: Sam Hocevar Date: Thu, 31 Jan 2013 23:30:44 +0000 Subject: [PATCH] color: slightly tweak the RGB to HSV and HSL conversions for improved numerical stability on i386. --- src/lol/image/color.h | 21 ++++++++++----------- test/unit/color.cpp | 24 +++++++++++++++++++----- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/lol/image/color.h b/src/lol/image/color.h index 248ee282..8c10627c 100644 --- a/src/lol/image/color.h +++ b/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) diff --git a/test/unit/color.cpp b/test/unit/color.cpp index 76abaabb..ed780930 100644 --- a/test/unit/color.cpp +++ b/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(); } } };