The Tight encoder uses the pixel buffer as a scratch pad, which doesn't

work so well with the new optimisation to feed it the raw frame buffer.
Reorganise and clean up the code to handle this, and make the raw frame
buffer pointer const so that we might avoid such bugs in the future.


git-svn-id: svn://svn.code.sf.net/p/tigervnc/code/trunk@4841 3789f03b-4d11-0410-bbf8-ca57d06f2519

5 files changed, 64 insertions, 56 deletions

diff --git a/common/rfb/TightEncoder.cxx b/common/rfb/TightEncoder.cxx
index 9be4581e..fcd9d9b0 100644
--- a/common/rfb/TightEncoder.cxx
+++ b/common/rfb/TightEncoder.cxx
@@ -418,3 +418,19 @@ void TightEncoder::writeSubrect(const Rect& r, bool forceSolid)
   os->writeBytes(mos.data(), mos.length());
   writer->endRect();
 }
+
+void TightEncoder::encodeJpegRect(const Rect& r, rdr::OutStream *os)
+{
+  const rdr::U8 *buf;
+  int stride;
+
+  buf = ig->getRawPixelsR(r, &stride);
+
+  jc.clear();
+  jc.compress(buf, stride * serverpf.bpp / 8, r, serverpf,
+              jpegQuality, jpegSubsampling);
+
+  os->writeU8(0x09 << 4);
+  os->writeCompactLength(jc.length());
+  os->writeBytes(jc.data(), jc.length());
+}
diff --git a/common/rfb/TightEncoder.h b/common/rfb/TightEncoder.h
index 95c2b984..10ca7616 100644
--- a/common/rfb/TightEncoder.h
+++ b/common/rfb/TightEncoder.h
@@ -100,9 +100,9 @@ namespace rfb {
     int paletteInsert(rdr::U32 rgb, int numPixels, int bpp);
     void paletteReset(void);
 
-    void fastFillPalette8(rdr::U8 *data, int stride, const Rect &r);
-    void fastFillPalette16(rdr::U16 *data, int stride, const Rect &r);
-    void fastFillPalette32(rdr::U32 *data, int stride, const Rect &r);
+    void fastFillPalette8(const rdr::U8 *buffer, int stride, const Rect &r);
+    void fastFillPalette16(const rdr::U8 *buffer, int stride, const Rect &r);
+    void fastFillPalette32(const rdr::U8 *buffer, int stride, const Rect &r);
 
     void fillPalette8(rdr::U8 *data, int count);
     void fillPalette16(rdr::U16 *data, int count);
@@ -135,10 +135,7 @@ namespace rfb {
     void encodeIndexedRect16(rdr::U16 *buf, const Rect& r, rdr::OutStream *os);
     void encodeIndexedRect32(rdr::U32 *buf, const Rect& r, rdr::OutStream *os);
 
-    void encodeJpegRect16(rdr::U16 *buf, int stride, const Rect& r,
-                          rdr::OutStream *os);
-    void encodeJpegRect32(rdr::U32 *buf, int stride, const Rect& r,
-                          rdr::OutStream *os);
+    void encodeJpegRect(const Rect& r, rdr::OutStream *os);
 
     SMsgWriter* writer;
     rdr::MemOutStream mos;
diff --git a/common/rfb/TransImageGetter.cxx b/common/rfb/TransImageGetter.cxx
index bf26b44d..c1add1ce 100644
--- a/common/rfb/TransImageGetter.cxx
+++ b/common/rfb/TransImageGetter.cxx
@@ -56,12 +56,12 @@ void TransImageGetter::setColourMapEntries(int firstCol, int nCols)
   PixelTransformer::setColourMapEntries(firstCol, nCols);
 }
 
-rdr::U8 *TransImageGetter::getRawPixelsRW(const Rect &r, int *stride)
+const rdr::U8 *TransImageGetter::getRawPixelsR(const Rect &r, int *stride)
 {
   if (!offset.equals(Point(0, 0)))
-    return pb->getPixelsRW(r.translate(offset.negate()), stride);
+    return pb->getPixelsR(r.translate(offset.negate()), stride);
   else
-    return pb->getPixelsRW(r, stride);
+    return pb->getPixelsR(r, stride);
 }
 
 void TransImageGetter::getImage(void* outPtr, const Rect& r, int outStride)
diff --git a/common/rfb/TransImageGetter.h b/common/rfb/TransImageGetter.h
index 1ad49b7c..f2b35b45 100644
--- a/common/rfb/TransImageGetter.h
+++ b/common/rfb/TransImageGetter.h
@@ -72,11 +72,11 @@ namespace rfb {
     // padding will be outStride-r.width() pixels).
     void getImage(void* outPtr, const Rect& r, int outStride=0);
 
-    // getRawPixelsRW() gets the given rectangle of data directly from the
+    // getRawPixelsR() gets the given rectangle of data directly from the
     // underlying PixelBuffer, bypassing the translation logic. Only use
     // this when doing something that's independent of the client's pixel
     // format.
-    rdr::U8 *getRawPixelsRW(const Rect &r, int *stride);
+    const rdr::U8 *getRawPixelsR(const Rect &r, int *stride);
 
     // setPixelBuffer() changes the pixel buffer to be used.  The new pixel
     // buffer MUST have the same pixel format as the old one - if not you
diff --git a/common/rfb/tightEncode.h b/common/rfb/tightEncode.h
index a1d5dbe9..446d45e7 100644
--- a/common/rfb/tightEncode.h
+++ b/common/rfb/tightEncode.h
@@ -189,10 +189,10 @@ unsigned int PACK_PIXELS (PIXEL_T *buf, unsigned int count)
 
 void TIGHT_ENCODE (const Rect& r, rdr::OutStream *os, bool forceSolid)
 {
-  int stride = r.width();
+  int stride;
   rdr::U32 solidColor;
-  PIXEL_T *pixels = (PIXEL_T *)ig->getRawPixelsRW(r, &stride);
-  bool grayScaleJPEG = (jpegSubsampling == SUBSAMP_GRAY && jpegQuality != -1);
+  const rdr::U8 *rawPixels;
+  PIXEL_T *pixels;
 
 #if (BPP == 32)
   // Check if it's necessary to pack 24-bit pixels, and
@@ -200,37 +200,45 @@ void TIGHT_ENCODE (const Rect& r, rdr::OutStream *os, bool forceSolid)
   pack24 = clientpf.is888();
 #endif
 
+  rawPixels = ig->getRawPixelsR(r, &stride);
+  pixels = NULL;
+
   if (forceSolid) {
+    // Forced solid block
     palNumColors = 1;
-    if (ig->willTransform()) {
-      ig->translatePixels(pixels, &solidColor, 1);
-      pixels = (PIXEL_T *)&solidColor;
-    }
-  }
-  else {
+    ig->translatePixels(rawPixels, &solidColor, 1);
+    pixels = (PIXEL_T *)&solidColor;
+  } else if (jpegSubsampling == SUBSAMP_GRAY && jpegQuality != -1) {
+    // Forced gray scale (JPEG)
+    palNumColors = 0;
+  } else {
+    // Normal analysis
     palMaxColors = r.area() / pconf->idxMaxColorsDivisor;
-    if (jpegQuality != -1) palMaxColors = pconf->palMaxColorsWithJPEG;
-    if (palMaxColors < 2 && r.area() >= pconf->monoMinRectSize) {
+
+    if (jpegQuality != -1)
+      palMaxColors = pconf->palMaxColorsWithJPEG;
+
+    if (palMaxColors < 2 && r.area() >= pconf->monoMinRectSize)
       palMaxColors = 2;
-    }
 
     if (clientpf.equal(serverpf) && clientpf.bpp >= 16) {
-      // This is so we can avoid translating the pixels when compressing
-      // with JPEG, since it is unnecessary
-      if (grayScaleJPEG) palNumColors = 0;
-      else FAST_FILL_PALETTE(pixels, stride, r);
+      // No conversion, so just analyse the raw buffer
+      FAST_FILL_PALETTE(rawPixels, stride, r);
+
+      // JPEG reads from the raw buffer and has its own output buffer,
+      // but the other encodings need some help.
       if(palNumColors != 0 || jpegQuality == -1) {
         pixels = (PIXEL_T *)writer->getImageBuf(r.area());
         stride = r.width();
         ig->getImage(pixels, r);
       }
-    }
-    else {
+    } else {
+      // Need to do the conversion first so we have something to analyse
       pixels = (PIXEL_T *)writer->getImageBuf(r.area());
       stride = r.width();
       ig->getImage(pixels, r);
-      if (grayScaleJPEG) palNumColors = 0;
-      else FILL_PALETTE(pixels, r.area());
+
+      FILL_PALETTE(pixels, r.area());
     }
   }
 
@@ -239,7 +247,7 @@ void TIGHT_ENCODE (const Rect& r, rdr::OutStream *os, bool forceSolid)
     // Truecolor image
 #if (BPP != 8)
     if (jpegQuality != -1) {
-      ENCODE_JPEG_RECT(pixels, stride, r, os);
+      encodeJpegRect(r, os);
       break;
     }
 #endif
@@ -396,23 +404,6 @@ void ENCODE_INDEXED_RECT (PIXEL_T *buf, const Rect& r, rdr::OutStream *os)
 #endif  // #if (BPP != 8)
 
 //
-// JPEG compression.
-//
-
-#if (BPP != 8)
-void ENCODE_JPEG_RECT (PIXEL_T *buf, int stride, const Rect& r,
-                       rdr::OutStream *os)
-{
-  jc.clear();
-  jc.compress((rdr::U8 *)buf, stride * clientpf.bpp / 8, r, clientpf,
-    jpegQuality, jpegSubsampling);
-  os->writeU8(0x09 << 4);
-  os->writeCompactLength(jc.length());
-  os->writeBytes(jc.data(), jc.length());
-}
-#endif  // #if (BPP != 8)
-
-//
 // Determine the number of colors in the rectangle, and fill in the palette.
 //
 
@@ -458,7 +449,7 @@ void FILL_PALETTE (PIXEL_T *data, int count)
   }
 }
 
-void FAST_FILL_PALETTE (PIXEL_T *data, int stride, const Rect& r)
+void FAST_FILL_PALETTE (const rdr::U8 *buffer, int stride, const Rect& r)
 {
 }
 
@@ -523,14 +514,17 @@ void FILL_PALETTE (PIXEL_T *data, int count)
   paletteInsert (ci, (rdr::U32)ni, BPP);
 }
 
-void FAST_FILL_PALETTE (PIXEL_T *data, int stride, const Rect& r)
+void FAST_FILL_PALETTE (const rdr::U8 *buffer, int stride, const Rect& r)
 {
   PIXEL_T c0, c1, ci = 0, mask, c0t, c1t, cit;
   int n0, n1, ni;
   int w = r.width(), h = r.height();
-  PIXEL_T *rowptr, *colptr, *rowptr2, *colptr2, *dataend = &data[stride * h];
+  const PIXEL_T *data, *rowptr, *colptr, *rowptr2, *colptr2, *dataend;
   bool willTransform = ig->willTransform();
 
+  data = (const PIXEL_T*)buffer;
+  dataend = &data[stride * h];
+
   if (willTransform) {
     mask = serverpf.redMax << serverpf.redShift;
     mask |= serverpf.greenMax << serverpf.greenShift;
@@ -636,11 +630,12 @@ void FAST_FILL_PALETTE (PIXEL_T *data, int stride, const Rect& r)
 
 bool CHECK_SOLID_TILE(Rect& r, rdr::U32 *colorPtr, bool needSameColor)
 {
-  PIXEL_T *buf, colorValue;
+  const PIXEL_T *buf;
+  PIXEL_T colorValue;
   int w = r.width(), h = r.height();
 
   int stride = w;
-  buf = (PIXEL_T *)ig->getRawPixelsRW(r, &stride);
+  buf = (const PIXEL_T *)ig->getRawPixelsR(r, &stride);
 
   colorValue = *buf;
   if (needSameColor && (rdr::U32)colorValue != *colorPtr)
@@ -648,7 +643,7 @@ bool CHECK_SOLID_TILE(Rect& r, rdr::U32 *colorPtr, bool needSameColor)
 
   int bufPad = stride - w;
   while (h > 0) {
-    PIXEL_T *bufEndOfRow = buf + w;
+    const PIXEL_T *bufEndOfRow = buf + w;
     while (buf < bufEndOfRow) {
       if (colorValue != *(buf++))
         return false;