[Mesa-dev] [PATCH v2] format_utils: Use a more precise conversion when decreasing bits

[Jason Ekstrand]

Looks good to me.  My initial reaction was that we could simplify it but
you're right about the rounding.  This one gets my r-b too.
On Jan 15, 2015 10:52 AM, "Neil Roberts" <neil at linux.intel.com> wrote:
>
> Jason Ekstrand <jason at jlekstrand.net> writes:
>
> > This looks fine to me.  We should probably also do this for snorm
formats.
> > I don't care if that's part of this or in a separate patch.
> > --Jason
>
> The snorm formats are a bit more fiddly because the hardware doesn't
> quite seem to be doing what I'd expect. For example, when converting
> from 16 bits to 8 bits we effectively want to convert from the range
> [-32767,+32767] to [-127,+127]. I think that means we can just use a
> formula like this:
>
>  a = 127 * b / 32767
>
> This almost works except when the result is close to halfway between two
> integers the hardware seems to round in the wrong direction. For example
> the input value 16642 according to the formula should be 64.5019, which
> you'd hope it would round to 65. However it rounds to 64.
>
> We could probably bodge the formula to match what Intel hardware does
> but that seems a bit cheeky considering this is in generic Mesa code. I
> tried to check what NVidia is doing on my old GeForce 9400 with the
> binary driver and it seems to be more or less doing the formula but
> truncating the fractional part instead of rounding. That of course would
> end up with completely different results again so we wouldn't be able to
> match both ways of doing it.
>
> I made the patch below which seems to correctly implement the formula at
> least for all values of converting 16-bit to 8-bit. Maybe we could just
use
> it and accept that it doesn't match the hardware in a few cases. I don't
> think it will affect any Piglit tests which was the original incentive
> to try and fix this.
>
> diff --git a/src/mesa/main/format_utils.h b/src/mesa/main/format_utils.h
> index 2faaf76..c959b57 100644
> --- a/src/mesa/main/format_utils.h
> +++ b/src/mesa/main/format_utils.h
> @@ -150,8 +150,19 @@ _mesa_snorm_to_snorm(int x, unsigned src_bits,
unsigned dst_bits)
>        return -MAX_INT(dst_bits);
>     else if (src_bits < dst_bits)
>        return EXTEND_NORMALIZED_INT(x, src_bits - 1, dst_bits - 1);
> -   else
> -      return x >> (src_bits - dst_bits);
> +   else {
> +      /* As an example, when converting from 16 bits to 8 bits, when it
gets
> +       * here x will be in the range [-32767,+32767] and we want to map
this
> +       * to [-127,+127]. The -32768 case will have been handled in the
first
> +       * if-clause above. This just adds 32767 so that it is in the range
> +       * [0,65534], multiplies it by 254 and then divides by 65534. The
> +       * division is done with only positive numbers because dividing a
> +       * negative number has undefined rounding in C < 99. After the
division
> +       * it is subtracted by 127 to put in back in the range [-127,127].
*/
> +      return (((x + MAX_INT(src_bits)) * (MAX_INT(dst_bits) * 2) +
> +               MAX_INT(src_bits)) /
> +              (MAX_INT(src_bits) * 2) - MAX_INT(dst_bits));
> +   }
>  }
>
> - Neil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150116/70f6dd9f/attachment.html>