Skip to content

Commit

Permalink
libvncclient: fix tight decoding endianness issues
Browse files Browse the repository at this point in the history
When the host byte order of a client is different from that of the
RFB pixel format it is using, we must swap bytes before delivering them
to the client.  This commit addresses places where we were not properly
doing that during tight decoding, manifesting in issues in which solid
filled rectangles and gradients could be decoded to the wrong color.  As
the RFB protocol says: "Swap the pixel value according to
big-endian-flag (e.g. if big-endian-flag is zero (false) and host byte
order is big endian, then swap)."
  • Loading branch information
jknockel authored and bk138 committed Feb 6, 2024
1 parent 784cccb commit de7e92b
Showing 1 changed file with 54 additions and 16 deletions.
70 changes: 54 additions & 16 deletions src/libvncclient/tight.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,24 +54,62 @@

#ifndef RGB_TO_PIXEL

#define RGB_TO_PIXEL(bpp,r,g,b) \
(((CARD##bpp)(r) & client->format.redMax) << client->format.redShift | \
((CARD##bpp)(g) & client->format.greenMax) << client->format.greenShift | \
((CARD##bpp)(b) & client->format.blueMax) << client->format.blueShift)

#define RGB24_TO_PIXEL(bpp,r,g,b) \
((((CARD##bpp)(r) & 0xFF) * client->format.redMax + 127) / 255 \
<< client->format.redShift | \
(((CARD##bpp)(g) & 0xFF) * client->format.greenMax + 127) / 255 \
<< client->format.greenShift | \
(((CARD##bpp)(b) & 0xFF) * client->format.blueMax + 127) / 255 \
<< client->format.blueShift)

#define RGB24_TO_PIXEL32(r,g,b) \
(((uint32_t)(r) & 0xFF) << client->format.redShift | \
((uint32_t)(g) & 0xFF) << client->format.greenShift | \
/*
* Unfortunately we cannot use rfbClientSwap16IfLE() and related macros here as
* rfbClientSwap16IfLE() swaps when the host byte order of the client is
* little-endian, but we want to swap when the host byte order of the client is
* different from the byte order of the pixel format it is using. In other
* words, rfbClientSwap16IfLE() will do the right thing only when the pixel
* format is big-endian.
*/
#define Swap8(c) (c)
#define Swap16(s) ((((s) & 0xff) << 8) | (((s) >> 8) & 0xff))
#define Swap32(l) ((((l) >> 24) & 0x000000ff)| \
(((l) & 0x00ff0000) >> 8) | \
(((l) & 0x0000ff00) << 8) | \
(((l) & 0x000000ff) << 24))

#ifdef LIBVNCSERVER_WORDS_BIGENDIAN
#define NEED_SWAP (!client->format.bigEndian)
#else
#define NEED_SWAP (client->format.bigEndian)
#endif

#define RGB_TO_PIXEL_NOSWAP(card,r,g,b) \
(((card)(r) & client->format.redMax) << client->format.redShift | \
((card)(g) & client->format.greenMax) << client->format.greenShift | \
((card)(b) & client->format.blueMax) << client->format.blueShift)

#define RGB_TO_PIXEL(bpp,r,g,b) NEED_SWAP ? \
CONCAT2(Swap,bpp)(RGB_TO_PIXEL_NOSWAP(CARD##bpp,r,g,b)) : \
RGB_TO_PIXEL_NOSWAP(CARD##bpp,r,g,b)

#define RGB24_TO_PIXEL_NOSWAP(card,r,g,b) \
((((card)(r) & 0xFF) * client->format.redMax + 127) / 255 \
<< client->format.redShift | \
(((card)(g) & 0xFF) * client->format.greenMax + 127) / 255 \
<< client->format.greenShift | \
(((card)(b) & 0xFF) * client->format.blueMax + 127) / 255 \
<< client->format.blueShift)

#define RGB24_TO_PIXEL(bpp,r,g,b) NEED_SWAP ? \
CONCAT2(Swap,bpp)(RGB24_TO_PIXEL_NOSWAP(CARD##bpp,r,g,b)) : \
RGB24_TO_PIXEL_NOSWAP(CARD##bpp,r,g,b)

#define RGB24_TO_PIXEL32_NOSWAP(r,g,b) \
(((uint32_t)(r) & 0xFF) << client->format.redShift | \
((uint32_t)(g) & 0xFF) << client->format.greenShift | \
((uint32_t)(b) & 0xFF) << client->format.blueShift)

#define RGB24_TO_PIXEL32_SWAP(r,g,b) \
(((uint32_t)(r) & 0xFF) << (24 - client->format.redShift) | \
((uint32_t)(g) & 0xFF) << (24 - client->format.greenShift) | \
((uint32_t)(b) & 0xFF) << (24 - client->format.blueShift))

#define RGB24_TO_PIXEL32(r,g,b) NEED_SWAP ? \
RGB24_TO_PIXEL32_SWAP(r,g,b) : \
RGB24_TO_PIXEL32_NOSWAP(r,g,b)

#endif

/* Type declarations */
Expand Down

0 comments on commit de7e92b

Please sign in to comment.