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 76051dc449d0b3d82eaff692c16fd1acb4d269af..883b0410e02327b1c51c0e062d066aa33611913e 100644 (file)
--- 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 8d67f8a1549fcb995d43f25977ffe70699649adc..5b4b6332390a97c9ccd2467b17efdb1e476cbced 100644 (file)
--- 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 f9fb1254034b34e857e385a9791f09bd0b0d51e9..5a40379a37fe6b2b0153082a1898e0c956c67c27 100644 (file)
--- 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 6b5ad6bba82681d2b4ea626e51e3e9163688d9cb..c8e432df8626f9250878ffceaca571ef7cd1623d 100644 (file)
--- 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)