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

["AE" <ae-a@xxxxxxxxxx>]

>> 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!!).

I decided to do this because of a bug I discovered in MSVC. I discussed it
on allegro.cc in the following thread.
http://www.allegro.cc/forums/view_thread.php?_id=269409 - MSVC 6 floating
point maths bug.
To sum up: Sometimes if the MSVC compiler runs out of FPU registers, it
applys different rounding schemes to the numbers, and in equasions that rely
on comparing floats for equality, or subtracting equal numbers and doing a
test for zero to avoid a dvivde-by-zero, it produces strange results. I have
not been able to reproduce this anomalous behaviour with GCC. I found this
behaviour on MSVC 6 and I've been told it happens on MSVC7 too, but if I
manually select the "improve floating point consistency" optimisation which
isn't turned on by default in MSVC, the problem goes away.

I had another look at the code, and noticed that in the hsv_to_rgb()
function, the error does not lead to much so you could get away with
'==0.0f' for hsv_to_rgb(). However, when I was working on my own
rgb_to_hls() function, this error lead to out-of-range values when doing a
divide so I used it there and thought I might port it to hsv_to_rgb(), but
now that does seem to be a bit overkill. If you decide to get rid of the '<
FLT_EPSILON's, then also make sure you remove the #include <float.h>


>@@ -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.

Oops. Seems like I've been a bit overzealous.


AE.