From: Pierre Ossman <ossman@cendio.se>
Date: Thu, 1 Mar 2018 13:11:39 +0000 (+0100)
Subject: Fix rounding error in pixel down conversion
X-Git-Tag: v1.8.90~36
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=816baa35ae5374f350ee4031ac54d7d1e90af183;p=tigervnc.git

Fix rounding error in pixel down conversion

Simple shifting can give noticable rounding errors if there is a large
difference in the number of bits between the formats. Do the proper
thing via a lookup table, the same way things are done for up conversion.
---

diff --git a/common/rfb/PixelFormat.cxx b/common/rfb/PixelFormat.cxx
index 76051dc4..883b0410 100644
--- a/common/rfb/PixelFormat.cxx
+++ b/common/rfb/PixelFormat.cxx
@@ -34,6 +34,7 @@
 using namespace rfb;
 
 rdr::U8 PixelFormat::upconvTable[256*8];
+rdr::U8 PixelFormat::downconvTable[256*8];
 
 class PixelFormat::Init {
 public:
@@ -47,24 +48,29 @@ PixelFormat::Init::Init()
 {
   int bits;
 
-  // Bit replication is almost perfect, but not quite. And
+  // Shifting bits is almost perfect, but not quite. And
   // a lookup table is still quicker when there is a large
   // difference between the source and destination depth.
 
   for (bits = 1;bits <= 8;bits++) {
     int i, maxVal;
-    rdr::U8 *subTable;
+    rdr::U8 *subUpTable;
+    rdr::U8 *subDownTable;
 
     maxVal = (1 << bits) - 1;
-    subTable = &upconvTable[(bits-1)*256];
+    subUpTable = &upconvTable[(bits-1)*256];
+    subDownTable = &downconvTable[(bits-1)*256];
 
     for (i = 0;i <= maxVal;i++)
-      subTable[i] = i * 255 / maxVal;
+      subUpTable[i] = i * 255 / maxVal;
 
-    // Duplicate the table so that we don't have to care about
+    // Duplicate the up table so that we don't have to care about
     // the upper bits when doing a lookup
     for (;i < 256;i += maxVal+1)
-      memcpy(&subTable[i], &subTable[0], maxVal+1);
+      memcpy(&subUpTable[i], &subUpTable[0], maxVal+1);
+
+    for (i = 0;i <= 255;i++)
+      subDownTable[i] = (i * maxVal + 128) / 255;
   }
 }
 
diff --git a/common/rfb/PixelFormat.h b/common/rfb/PixelFormat.h
index 8d67f8a1..5b4b6332 100644
--- a/common/rfb/PixelFormat.h
+++ b/common/rfb/PixelFormat.h
@@ -137,6 +137,7 @@ namespace rfb {
     bool endianMismatch;
 
     static rdr::U8 upconvTable[256*8];
+    static rdr::U8 downconvTable[256*8];
 
     class Init;
     friend class Init;
diff --git a/common/rfb/PixelFormat.inl b/common/rfb/PixelFormat.inl
index f9fb1254..5a40379a 100644
--- a/common/rfb/PixelFormat.inl
+++ b/common/rfb/PixelFormat.inl
@@ -79,10 +79,9 @@ inline Pixel PixelFormat::pixelFromRGB(rdr::U16 red, rdr::U16 green, rdr::U16 bl
 {
   Pixel p;
 
-  /* We don't need to mask since we shift out unwanted bits */
-  p = ((Pixel)red >> (16 - redBits)) << redShift;
-  p |= ((Pixel)green >> (16 - greenBits)) << greenShift;
-  p |= ((Pixel)blue >> (16 - blueBits)) << blueShift;
+  p = (Pixel)downconvTable[(redBits-1)*256 + (red >> 8)] << redShift;
+  p |= (Pixel)downconvTable[(greenBits-1)*256 + (green >> 8)] << greenShift;
+  p |= (Pixel)downconvTable[(blueBits-1)*256 + (blue >> 8)] << blueShift;
 
   return p;
 }
@@ -92,9 +91,9 @@ inline Pixel PixelFormat::pixelFromRGB(rdr::U8 red, rdr::U8 green, rdr::U8 blue)
 {
   Pixel p;
 
-  p = ((Pixel)red >> (8 - redBits)) << redShift;
-  p |= ((Pixel)green >> (8 - greenBits)) << greenShift;
-  p |= ((Pixel)blue >> (8 - blueBits)) << blueShift;
+  p = (Pixel)downconvTable[(redBits-1)*256 + red] << redShift;
+  p |= (Pixel)downconvTable[(greenBits-1)*256 + green] << greenShift;
+  p |= (Pixel)downconvTable[(blueBits-1)*256 + blue] << blueShift;
 
   return p;
 }
diff --git a/common/rfb/PixelFormatBPP.cxx b/common/rfb/PixelFormatBPP.cxx
index 6b5ad6bb..c8e432df 100644
--- a/common/rfb/PixelFormatBPP.cxx
+++ b/common/rfb/PixelFormatBPP.cxx
@@ -41,11 +41,11 @@ void PixelFormat::directBufferFromBufferFrom888(rdr::UOUT* dst,
   const rdr::U8 *r, *g, *b;
   int dstPad, srcPad;
 
-  int redTruncShift, greenTruncShift, blueTruncShift;
+  const rdr::U8 *redDownTable, *greenDownTable, *blueDownTable;
 
-  redTruncShift = 8 - redBits;
-  greenTruncShift = 8 - greenBits;
-  blueTruncShift = 8 - blueBits;
+  redDownTable = &downconvTable[(redBits-1)*256];
+  greenDownTable = &downconvTable[(greenBits-1)*256];
+  blueDownTable = &downconvTable[(blueBits-1)*256];
 
   if (srcPF.bigEndian) {
     r = src + (24 - srcPF.redShift)/8;
@@ -64,9 +64,9 @@ void PixelFormat::directBufferFromBufferFrom888(rdr::UOUT* dst,
     while (w_--) {
       rdr::UOUT d;
 
-      d = (*r >> redTruncShift) << redShift;
-      d |= (*g >> greenTruncShift) << greenShift;
-      d |= (*b >> blueTruncShift) << blueShift;
+      d = redDownTable[*r] << redShift;
+      d |= greenDownTable[*g] << greenShift;
+      d |= blueDownTable[*b] << blueShift;
 
 #if OUTBPP != 8
       if (endianMismatch)