Optimise the RGB to pixel conversion to simple shifts

Might as well make it inline at this point as well as it is used
heavily in other routines that convert entire blocks of pixels.

3 files changed, 65 insertions, 51 deletions

diff --git a/common/rfb/PixelFormat.cxx b/common/rfb/PixelFormat.cxx
index 769afc6a..8a4e71c0 100644
--- a/common/rfb/PixelFormat.cxx
+++ b/common/rfb/PixelFormat.cxx
@@ -132,55 +132,6 @@ bool PixelFormat::isLittleEndian(void) const
 }
 
 
-Pixel PixelFormat::pixelFromRGB(rdr::U16 red, rdr::U16 green, rdr::U16 blue,
-                                ColourMap* cm) const
-{
-  if (trueColour) {
-    rdr::U32 r = ((rdr::U32)red   * redMax   + 32767) / 65535;
-    rdr::U32 g = ((rdr::U32)green * greenMax + 32767) / 65535;
-    rdr::U32 b = ((rdr::U32)blue  * blueMax  + 32767) / 65535;
-
-    return (r << redShift) | (g << greenShift) | (b << blueShift);
-  } else if (cm) {
-    // Try to find the closest pixel by Cartesian distance
-    int colours = 1 << depth;
-    int diff = 256 * 256 * 4;
-    int col = 0;
-    for (int i=0; i<colours; i++) {
-      int r, g, b;
-      cm->lookup(i, &r, &g, &b);
-      int rd = (r-red) >> 8;
-      int gd = (g-green) >> 8;
-      int bd = (b-blue) >> 8;
-      int d = rd*rd + gd*gd + bd*bd;
-      if (d < diff) {
-        col = i;
-        diff = d;
-      }
-    }
-    return col;
-  }
-  // XXX just return 0 for colour map?
-  return 0;
-}
-
-
-Pixel PixelFormat::pixelFromRGB(rdr::U8 red, rdr::U8 green, rdr::U8 blue,
-                                ColourMap* cm) const
-{
-  if (trueColour) {
-    rdr::U32 r = ((rdr::U32)red   * redMax   + 127) / 255;
-    rdr::U32 g = ((rdr::U32)green * greenMax + 127) / 255;
-    rdr::U32 b = ((rdr::U32)blue  * blueMax  + 127) / 255;
-
-    return (r << redShift) | (g << greenShift) | (b << blueShift);
-  }
-
-  return pixelFromRGB((rdr::U16)(red << 8), (rdr::U16)(green << 8),
-      (rdr::U16)(blue << 8), cm);
-}
-
-
 void PixelFormat::bufferFromRGB(rdr::U8 *dst, const rdr::U8* src,
                                 int pixels, ColourMap* cm) const
 {
@@ -594,6 +545,17 @@ bool PixelFormat::isSane(void)
     if ((blueMax & (blueMax + 1)) != 0)
       return false;
 
+    /*
+     * We don't allow individual channels > 8 bits in order to keep our
+     * conversions simple.
+     */
+    if (redMax >= (1 << 8))
+      return false;
+    if (greenMax >= (1 << 8))
+      return false;
+    if (blueMax >= (1 << 8))
+      return false;
+
     totalBits = bits(redMax) + bits(greenMax) + bits(blueMax);
     if (totalBits > bpp)
       return false;
diff --git a/common/rfb/PixelFormat.h b/common/rfb/PixelFormat.h
index e05e6f9f..fdb829c9 100644
--- a/common/rfb/PixelFormat.h
+++ b/common/rfb/PixelFormat.h
@@ -59,8 +59,8 @@ namespace rfb {
     inline Pixel pixelFromBuffer(const rdr::U8* buffer) const;
     inline void bufferFromPixel(rdr::U8* buffer, Pixel pixel) const;
 
-    Pixel pixelFromRGB(rdr::U16 red, rdr::U16 green, rdr::U16 blue, ColourMap* cm=0) const;
-    Pixel pixelFromRGB(rdr::U8 red, rdr::U8 green, rdr::U8 blue, ColourMap* cm=0) const;
+    inline Pixel pixelFromRGB(rdr::U16 red, rdr::U16 green, rdr::U16 blue, ColourMap* cm=0) const;
+    inline Pixel pixelFromRGB(rdr::U8 red, rdr::U8 green, rdr::U8 blue, ColourMap* cm=0) const;
 
     void bufferFromRGB(rdr::U8 *dst, const rdr::U8* src, int pixels, ColourMap* cm=0) const;
     void bufferFromRGB(rdr::U8 *dst, const rdr::U8* src, int w, int pitch,
diff --git a/common/rfb/PixelFormat.inl b/common/rfb/PixelFormat.inl
index 5743e495..547fae5d 100644
--- a/common/rfb/PixelFormat.inl
+++ b/common/rfb/PixelFormat.inl
@@ -75,6 +75,58 @@ inline void PixelFormat::bufferFromPixel(rdr::U8* buffer, Pixel p) const
 }
 
 
+inline Pixel PixelFormat::pixelFromRGB(rdr::U16 red, rdr::U16 green, rdr::U16 blue, ColourMap* cm) const
+{
+  if (trueColour) {
+    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;
+  } else if (cm) {
+    // Try to find the closest pixel by Cartesian distance
+    int colours = 1 << depth;
+    int diff = 256 * 256 * 4;
+    int col = 0;
+    for (int i=0; i<colours; i++) {
+      int r, g, b;
+      cm->lookup(i, &r, &g, &b);
+      int rd = (r-red) >> 8;
+      int gd = (g-green) >> 8;
+      int bd = (b-blue) >> 8;
+      int d = rd*rd + gd*gd + bd*bd;
+      if (d < diff) {
+        col = i;
+        diff = d;
+      }
+    }
+    return col;
+  } else {
+    // XXX just return 0 for colour map?
+    return 0;
+  }
+}
+
+
+inline Pixel PixelFormat::pixelFromRGB(rdr::U8 red, rdr::U8 green, rdr::U8 blue, ColourMap* cm) const
+{
+  if (trueColour) {
+    Pixel p;
+
+    p = ((Pixel)red >> (8 - redBits)) << redShift;
+    p |= ((Pixel)green >> (8 - greenBits)) << greenShift;
+    p |= ((Pixel)blue >> (8 - blueBits)) << blueShift;
+
+    return p;
+  } else {
+    return pixelFromRGB((rdr::U16)(red << 8 | red),
+                        (rdr::U16)(green << 8 | green),
+                        (rdr::U16)(blue << 8 | blue), cm);
+  }
+}
+
+
 inline void PixelFormat::rgbFromPixel(Pixel p, ColourMap* cm, rdr::U16 *r, rdr::U16 *g, rdr::U16 *b) const
 {
   if (trueColour) {