How does it not work so well? You can't just commit potentially disruptive patches after the code has been stabilized without first opening them up for discussion.
I expect a more detailed answer than your commit log below. On 1/30/12 7:58 AM, ossm...@users.sourceforge.net wrote: > Revision: 4841 > http://tigervnc.svn.sourceforge.net/tigervnc/?rev=4841&view=rev > Author: ossman_ > Date: 2012-01-30 13:58:44 +0000 (Mon, 30 Jan 2012) > Log Message: > ----------- > 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. > > Modified Paths: > -------------- > trunk/common/rfb/TightEncoder.cxx > trunk/common/rfb/TightEncoder.h > trunk/common/rfb/TransImageGetter.cxx > trunk/common/rfb/TransImageGetter.h > trunk/common/rfb/tightEncode.h > > Modified: trunk/common/rfb/TightEncoder.cxx > =================================================================== > --- trunk/common/rfb/TightEncoder.cxx 2012-01-30 13:53:11 UTC (rev 4840) > +++ trunk/common/rfb/TightEncoder.cxx 2012-01-30 13:58:44 UTC (rev 4841) > @@ -418,3 +418,19 @@ > 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()); > +} > > Modified: trunk/common/rfb/TightEncoder.h > =================================================================== > --- trunk/common/rfb/TightEncoder.h 2012-01-30 13:53:11 UTC (rev 4840) > +++ trunk/common/rfb/TightEncoder.h 2012-01-30 13:58:44 UTC (rev 4841) > @@ -100,9 +100,9 @@ > 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 @@ > 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; > > Modified: trunk/common/rfb/TransImageGetter.cxx > =================================================================== > --- trunk/common/rfb/TransImageGetter.cxx 2012-01-30 13:53:11 UTC (rev > 4840) > +++ trunk/common/rfb/TransImageGetter.cxx 2012-01-30 13:58:44 UTC (rev > 4841) > @@ -56,12 +56,12 @@ > 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) > > Modified: trunk/common/rfb/TransImageGetter.h > =================================================================== > --- trunk/common/rfb/TransImageGetter.h 2012-01-30 13:53:11 UTC (rev > 4840) > +++ trunk/common/rfb/TransImageGetter.h 2012-01-30 13:58:44 UTC (rev > 4841) > @@ -72,11 +72,11 @@ > // 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 > > Modified: trunk/common/rfb/tightEncode.h > =================================================================== > --- trunk/common/rfb/tightEncode.h 2012-01-30 13:53:11 UTC (rev 4840) > +++ trunk/common/rfb/tightEncode.h 2012-01-30 13:58:44 UTC (rev 4841) > @@ -189,10 +189,10 @@ > > 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 @@ > 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 @@ > // Truecolor image > #if (BPP != 8) > if (jpegQuality != -1) { > - ENCODE_JPEG_RECT(pixels, stride, r, os); > + encodeJpegRect(r, os); > break; > } > #endif > @@ -396,23 +404,6 @@ > #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 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 @@ > 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 @@ > > 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 @@ > > 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; > > This was sent by the SourceForge.net collaborative development platform, the > world's largest Open Source development site. > > > ------------------------------------------------------------------------------ > Try before you buy = See our experts in action! > The most comprehensive online learning library for Microsoft developers > is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, > Metro Style Apps, more. Free future releases when you subscribe now! > http://p.sf.net/sfu/learndevnow-dev2 > _______________________________________________ > Tigervnc-commits mailing list > tigervnc-comm...@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/tigervnc-commits ------------------------------------------------------------------------------ Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2 _______________________________________________ Tigervnc-devel mailing list Tigervnc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tigervnc-devel
