Re: [AD] Small bugfix in hsv_to_rgb() and smaller fix in rgb_to_hsv()

[Eric Botcazou <ebotcazou@xxxxxxxxxx>]

> Here is a patch for colour.c that fixes a small bug I discovered in
> hsv_to_rgb(). This bug existed before I submitied the changes to
> hsv_to_rgb() but propagated it's way into my optimised code I submitted.
> When rounding values of V to the nearest 1/255, the patch makes the V
> value round to the nearest whole fraction instead of truncating to the
> lower fraction like it used to. The patched code will return the same
> value for HSV values that have come straight from rgb_to_hsv(), but when V
> is a computed value, it will better make use of the numerical range 0-255
> (before, when a colour-value was set to be equal to V*255.0f, the value
> 255 would only result if V was 1.0. Now, 255 results if V>=509/510).

I guess this is ok.

> Also, this patch replaces all occurences in hsv_to_rgb() and rgb_to_hsv()
> of "== 0.0f" with "<FLT_EPSILON" (this requires float.h to be #included).

@@ -350,57 +351,58 @@ void hsv_to_rgb(float h, float s, float 
 
    v *= 255.0f;
 
-   if (s == 0.0f) {
-      *r = *g = *b = (int)v;
+   if (s < FLT_EPSILON) {
+      *r = *g = *b = (int)(v+0.5f);

Are you sure it is worthwhile to do the replacement? Loading a non-zero 
floating-point constant requires a memory access while the FPU has 'fldz' 
for 0.0 (well, at least on x86, because on SPARC you need to load 0.0 too!!).

@@ -444,9 +446,9 @@ void rgb_to_hsv(int r, int g, int b, flo
       else {
 	 /* g>=b and g>=r */
 	 delta = g - MIN(r, b);
-	 if (delta == 0) {
+	 if (delta < FLT_EPSILON) {
 	    *h = 0.0f;
-	    if (g == 0)
+	    if (g < FLT_EPSILON)
 	       *s = *v = 0.0f;
 	    else {
 	       *s = (float)delta / (float)g;

These are bogus, 'delta' and 'g' are integers.

> BTW, Was the improved exrgbhsv.c I submitted a month or so ago approved? I
> still see the old version in the latest snapshot.

I apologize for that. I remember downloading the zip file but it appears 
that, for some reason, I deleted it afterwards. Could you re-upload it, 
because it seems the link is currently broken?

-- 
Eric Botcazou