[Mesa-dev] [PATCH] Fixed bug in unclamped float to ubyte conversion.

[Stéphane Marchesin]

On Fri, May 10, 2013 at 12:57 PM, Manfred Ernst <ernstm at chromium.org> wrote:
> Problem: The IEEE float optimized version of UNCLAMPED_FLOAT_TO_UBYTE in macros.h
> computed incorrect results for inputs in the range 0x3f7f0000 (=0.99609375) to
> 0x3f7f7f80 (=0.99803924560546875) inclusive.  0x3f7f7f80 is the IEEE float
> value that results in 254.5 when multiplied by 255.  With rounding mode
> "round to closest even integer", this is the largest float in the range 0.0-1.0
> that is converted to 254 by the generic implementation of
> UNCLAMPED_FLOAT_TO_UBYTE.  The IEEE float optimized version incorrectly defined
> the cut-off for mapping to 255 as 0x3f7f0000 (=255.0/256.0). The same bug was
> present in the function float_to_ubyte in u_math.h.
>
> Fix: The proposed fix replaces the incorrect cut-off value by 0x3f800000, which
> is the IEEE float representation of 1.0f. 0x3f7f7f81 (or any value in between)
> would also work, but 1.0f is probably cleaner.
>
> The patch does not regress piglit on llvmpipe and on i965 on sandy bridge.
> Tested-by Stéphane Marchesin <marcheu at chromium.org>
> Reviewed-by Stéphane Marchesin <marcheu at chromium.org>
> ---
>  src/gallium/auxiliary/util/u_math.h | 4 ++--
>  src/mesa/main/macros.h              | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h
> index 607fbec..7e57e61 100644
> --- a/src/gallium/auxiliary/util/u_math.h
> +++ b/src/gallium/auxiliary/util/u_math.h
> @@ -540,14 +540,14 @@ ubyte_to_float(ubyte ub)
>  static INLINE ubyte
>  float_to_ubyte(float f)
>  {
> -   const int ieee_0996 = 0x3f7f0000;   /* 0.996 or so */
> +   const int ieee_one = 0x3f800000; /* 1.0f */
>     union fi tmp;
>
>     tmp.f = f;
>     if (tmp.i < 0) {
>        return (ubyte) 0;
>     }
> -   else if (tmp.i >= ieee_0996) {
> +   else if (tmp.i >= ieee_one) {
>        return (ubyte) 255;
>     }
>     else {
> diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
> index ac24672..af10076 100644
> --- a/src/mesa/main/macros.h
> +++ b/src/mesa/main/macros.h
> @@ -142,7 +142,7 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
>   *** CLAMPED_FLOAT_TO_UBYTE: map float known to be in [0,1] to ubyte in [0,255]
>   ***/
>  #if defined(USE_IEEE) && !defined(DEBUG)
> -#define IEEE_0996 0x3f7f0000   /* 0.996 or so */
> +#define IEEE_ONE 0x3f800000 /* 1.0F */

Please respin this using IEEE_ONE from compiler.h

Stéphane

>  /* This function/macro is sensitive to precision.  Test very carefully
>   * if you change it!
>   */
> @@ -152,7 +152,7 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
>             __tmp.f = (F);                                              \
>             if (__tmp.i < 0)                                            \
>                UB = (GLubyte) 0;                                                \
> -           else if (__tmp.i >= IEEE_0996)                              \
> +           else if (__tmp.i >= IEEE_ONE)                               \
>                UB = (GLubyte) 255;                                      \
>             else {                                                      \
>                __tmp.f = __tmp.f * (255.0F/256.0F) + 32768.0F;          \
> --
> 1.8.2.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev