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

[Neil Roberts]

When converting to a format that has fewer bits the previous code was just
shifting off the bits. This doesn't provide very accurate results. For example
when converting from 8 bits to 5 bits it is equivalent to doing this:

x * 32 / 256

This works as if it's taking a value from a range where 256 represents 1.0 and
scaling it down to a range where 32 represents 1.0. However this is not
correct because it is actually 255 and 31 that represent 1.0.

We can do better with a formula like this:

(x * 31 + 127) / 255

The +127 is to make it round correctly.

The main incentive to do this is to make the CPU conversion path pick the same
values as the hardware would if it did the conversion. This fixes failures
with the ‘texsubimage pbo’ test when using the patches from here:

http://lists.freedesktop.org/archives/mesa-dev/2015-January/074312.html
---

I've tested it with this little program here which makes sure that it
generates exactly the same values as the hardware does for all 8-bit
values when converting to 565:

https://github.com/bpeel/glthing/blob/pbo-subregion-convert/src/main.c

If we're worried about the drop in performance it might be worth
converting this to a table lookup instead for specific conversions.

We might want to do something similar for _mesa_snorm_to_snorm too.

 src/mesa/main/format_utils.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/format_utils.h b/src/mesa/main/format_utils.h
index b588695..2e6457b 100644
--- a/src/mesa/main/format_utils.h
+++ b/src/mesa/main/format_utils.h
@@ -96,10 +96,13 @@ _mesa_half_to_unorm(uint16_t x, unsigned dst_bits)
 static inline unsigned
 _mesa_unorm_to_unorm(unsigned x, unsigned src_bits, unsigned dst_bits)
 {
-   if (src_bits < dst_bits)
+   if (src_bits < dst_bits) {
       return EXTEND_NORMALIZED_INT(x, src_bits, dst_bits);
-   else
-      return x >> (src_bits - dst_bits);
+   } else {
+      unsigned src_half = (1 << (src_bits - 1)) - 1;
+      assert(src_bits + dst_bits <= sizeof(x) * 8);
+      return (x * MAX_UINT(dst_bits) + src_half) / MAX_UINT(src_bits);
+   }
 }
 
 static inline unsigned
-- 
1.9.3