Ping, does anyone else want to review this patch?
Stéphane
On Fri, May 10, 2013 at 3:56 PM, Manfred Ernst <ern...@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 <marc...@chromium.org>
> Reviewed-by Stéphane Marchesin <marc...@chromium.org>
> ---
> src/gallium/auxiliary/util/u_math.h | 3 +--
> src/mesa/main/macros.h | 3 +--
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_math.h
> b/src/gallium/auxiliary/util/u_math.h
> index 607fbec..64d16cb 100644
> --- a/src/gallium/auxiliary/util/u_math.h
> +++ b/src/gallium/auxiliary/util/u_math.h
> @@ -540,14 +540,13 @@ ubyte_to_float(ubyte ub)
> static INLINE ubyte
> float_to_ubyte(float f)
> {
> - const int ieee_0996 = 0x3f7f0000; /* 0.996 or so */
> union fi tmp;
>
> tmp.f = f;
> if (tmp.i < 0) {
> return (ubyte) 0;
> }
> - else if (tmp.i >= ieee_0996) {
> + else if (tmp.i >= 0x3f800000 /* 1.0f */) {
> return (ubyte) 255;
> }
> else {
> diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
> index ac24672..8f5b5ae 100644
> --- a/src/mesa/main/macros.h
> +++ b/src/mesa/main/macros.h
> @@ -142,7 +142,6 @@ 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 */
> /* This function/macro is sensitive to precision. Test very carefully
> * if you change it!
> */
> @@ -152,7 +151,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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
