Background color gets broken in 24 bit color depth.

[tyan0]

Overview:
If the vncserver is running in 24 bit color depth, background color gets broken in UltraVNC viewer.
This does not happen with RealVNC viewer and TightVNC viewer. Is this the known problem?

Steps to reproduce the issue:
1. Install tigervnc-standalone-server in debian (buster).
2. Start vncserver in debian with

Code: Select all

vncserver -geometry 1024x768 -depth 24

3. Connect to the vncserver using UltraVNC viewer (any version) from Windows10 client machine.
4. Set background color to #000070 (dark blue).

The resulting screenshot:


The expected screenshot:



I looked into this problem with UltraVNC source code in GitHub, and found the following patch fixes the issue, however, I am not sure that this is the right thing.

The patch which fixes the issue:

Code: Select all

--- ClientConnection.cpp.orig	2021-05-17 04:34:21.000000000 +0900
+++ ClientConnection.cpp	2021-08-10 12:24:17.811262900 +0900
@@ -3935,6 +3935,8 @@
 				// set colour masks and shifts
 			}
 		}
+
+		if (m_myFormat.depth == 24 && m_myFormat.bitsPerPixel == 32) m_myFormat.depth = 32;
 	}
 
 	// The endian will be set before sending

Could developers please have a look?

[Rudi De Vos]

Wat version of the viewer are you using.
A time ago we spend a lot of time with TigerVNC to make the server/viewer interconnect proper with there linux versions.

Looking at git "development branch" many fixes were added this year.

Please test with a viewer from viewtopic.php?t=37272 you need a 1.3.3 dev viewer.

We only worked together with TigerVNC...Other flavors could behave different.

[tyan0]

I tried the following UltraVNC versions in Windows 10 (21H1):

1.3.3 dev 10
1.3.2
1.2.4.0
1.0.9.6.2

The resunts are the same.

tigervnc versions I tried are as follows:
1.9.0+dfsg-3+deb10u3 (debian buster)
1.11.0+dfsg-2 (debian bullseye)

Thanks.

[tyan0]

The screenshots in the first post do not seem to be visible....

The resulting screenshot with the issue:


The expected screenshot:

[tyan0]

I looked into the code again and has been noticed that the following patch, instead of my previous patch, fixes the issue as well.

Alternative patch which fixes the issue:

Code: Select all

--- ClientConnectionTight.cpp.orig	2021-05-17 04:34:21.000000000 +0900
+++ ClientConnectionTight.cpp	2021-08-12 04:58:37.077589900 +0900
@@ -59,7 +59,10 @@
         m_myFormat.greenMax == 0xFF && m_myFormat.blueMax == 0xFF) {
       CARD8 fillColourBuf[3];
       ReadExact((char *)&fillColourBuf, 3);
-	  memcpy(colorpointer,&fillColourBuf, 3);
+      colorpointer[0] = fillColourBuf[2];
+      colorpointer[1] = fillColourBuf[1];
+      colorpointer[2] = fillColourBuf[0];
+      colorpointer[3] = 0;
       fillColour = COLOR_FROM_PIXEL24_ADDRESS(fillColourBuf);
     } else {
       CARD32 fillColourBuf;

I am not sure which is the better or more essential...

[tyan0]

Yet another patch which can fix the issue:

Code: Select all

--- ClientConnection.cpp.orig	2021-05-17 04:34:21.000000000 +0900
+++ ClientConnection.cpp	2021-08-12 05:55:09.239555600 +0900
@@ -9410,6 +9410,13 @@
     for (y=0; y<height; y++) {
 		for (x=0; x< width;x++)
 			{
+				if (m_myFormat.depth == 24 && bytes_per_pixel == 4) {
+					BYTE *p = destpos+x*bytes_per_pixel;
+					p[0] = sourcepos[2];
+					p[1] = sourcepos[1];
+					p[2] = sourcepos[0];
+					p[3] = 0;
+				} else
 				memcpy(destpos+x*bytes_per_pixel, sourcepos, bytes_per_pixel);
 			}
         destpos = (BYTE*)destpos + bytesPerOutputRow;

Edited: !!!This patch has a problem as the following post.!!!

[tyan0]

Sorry, the third one:

Code: Select all

--- ClientConnection.cpp.orig	2021-05-17 04:34:21.000000000 +0900
+++ ClientConnection.cpp	2021-08-12 05:55:09.239555600 +0900
@@ -9410,6 +9410,13 @@
     for (y=0; y<height; y++) {
 		for (x=0; x< width;x++)
 			{
+				if (m_myFormat.depth == 24 && bytes_per_pixel == 4) {
+					BYTE *p = destpos+x*bytes_per_pixel;
+					p[0] = sourcepos[2];
+					p[1] = sourcepos[1];
+					p[2] = sourcepos[0];
+					p[3] = 0;
+				} else
 				memcpy(destpos+x*bytes_per_pixel, sourcepos, bytes_per_pixel);
 			}
         destpos = (BYTE*)destpos + bytesPerOutputRow;

does not work if the encoding is Hextile.

[tyan0]

The second patch revised. The codes with no effect have been removed.

Code: Select all

--- ClientConnectionTight.cpp.orig	2021-05-17 04:34:21.000000000 +0900
+++ ClientConnectionTight.cpp	2021-08-12 13:38:48.749044300 +0900
@@ -54,29 +54,18 @@
   /* Handle solid rectangles. */
   BYTE colorpointer[4];
   if (comp_ctl == rfbTightFill) {
-    COLORREF fillColour;
     if (m_myFormat.depth == 24 && m_myFormat.redMax == 0xFF &&
         m_myFormat.greenMax == 0xFF && m_myFormat.blueMax == 0xFF) {
       CARD8 fillColourBuf[3];
       ReadExact((char *)&fillColourBuf, 3);
-	  memcpy(colorpointer,&fillColourBuf, 3);
-      fillColour = COLOR_FROM_PIXEL24_ADDRESS(fillColourBuf);
+      colorpointer[0] = fillColourBuf[2];
+      colorpointer[1] = fillColourBuf[1];
+      colorpointer[2] = fillColourBuf[0];
+      colorpointer[3] = 0;
     } else {
       CARD32 fillColourBuf;
       ReadExact((char *)&fillColourBuf, m_myFormat.bitsPerPixel / 8);
 	  memcpy(colorpointer,&fillColourBuf, m_myFormat.bitsPerPixel / 8);
-      SETUP_COLOR_SHORTCUTS;
-
-      switch (m_myFormat.bitsPerPixel) {
-      case 8:
-        fillColour = COLOR_FROM_PIXEL8_ADDRESS(&fillColourBuf);
-        break;
-      case 16:
-        fillColour = COLOR_FROM_PIXEL16_ADDRESS(&fillColourBuf);
-        break;
-      default:
-        fillColour = COLOR_FROM_PIXEL32_ADDRESS(&fillColourBuf);
-      }
     }
 
     omni_mutex_lock l(m_bitmapdcMutex);

[tyan0]

I think there are similar meaningless codes also in ClientConnectionCoRRE.cpp, ClientConnectionRRE.cpp, xz.cpp and zrle.cpp regarding COLOR_FROM_PIXEL*_ADDRESS(). The return-values of COLOR_FROM_PIXEL*_ADDRESS() are never used.

The patch to remove all these meaningless codes is as follows.

Code: Select all

diff --git a/vncviewer/ClientConnection.h b/vncviewer/ClientConnection.h
index 15e983b..11efb5e 100755
--- a/vncviewer/ClientConnection.h
+++ b/vncviewer/ClientConnection.h
@@ -921,22 +921,6 @@ public:
                 (int) ((( *(CARD32 *)(p) >> gs) & gm) * 255 / gm), \
                 (int) ((( *(CARD32 *)(p) >> bs) & bm) * 255 / bm) ))
 
-// The following may be faster if you already have a pixel value of the appropriate size
-#define COLOR_FROM_PIXEL8(p) (PALETTERGB( \
-                (int) (((p >> rs) & rm) * 255 / rm), \
-                (int) (((p >> gs) & gm) * 255 / gm), \
-                (int) (((p >> bs) & bm) * 255 / bm) ))
-
-#define COLOR_FROM_PIXEL16(p) (PALETTERGB( \
-                (int) ((( p >> rs) & rm) * 255 / rm), \
-                (int) ((( p >> gs) & gm) * 255 / gm), \
-                (int) ((( p >> bs) & bm) * 255 / bm) ))
-
-#define COLOR_FROM_PIXEL32(p) (PALETTERGB( \
-                (int) (((p >> rs) & rm) * 255 / rm), \
-                (int) (((p >> gs) & gm) * 255 / gm), \
-                (int) (((p >> bs) & bm) * 255 / bm) ))
-
 #define SETPIXEL(b,x,y,c) SetPixelV((b),(x),(y),(c))
 
 #define SETPIXELS(buffer, bpp, x, y, w, h)										\
diff --git a/vncviewer/ClientConnectionCoRRE.cpp b/vncviewer/ClientConnectionCoRRE.cpp
index e05ad57..1de255b 100755
--- a/vncviewer/ClientConnectionCoRRE.cpp
+++ b/vncviewer/ClientConnectionCoRRE.cpp
@@ -45,19 +45,6 @@ void ClientConnection::ReadCoRRERect(rfbFramebufferUpdateRectHeader *pfburh)
 
 	prreh->nSubrects = Swap32IfLE(prreh->nSubrects);
 
-	SETUP_COLOR_SHORTCUTS;
-
-    COLORREF color;
-    switch (m_myFormat.bitsPerPixel) {
-        case 8:
-            color = COLOR_FROM_PIXEL8_ADDRESS(pcolor); break;
-        case 16:
-			color = COLOR_FROM_PIXEL16_ADDRESS(pcolor); break;
-        case 24:
-        case 32:
-            color = COLOR_FROM_PIXEL32_ADDRESS(pcolor); break;
-    }
-
     // Draw the background of the rectangle
 	FillSolidRect_ultra(pfburh->r.x, pfburh->r.y, pfburh->r.w, pfburh->r.h, m_myFormat.bitsPerPixel,pcolor);
 
@@ -84,16 +71,6 @@ void ClientConnection::ReadCoRRERect(rfbFramebufferUpdateRectHeader *pfburh)
 			
 			pRect = (rfbCoRRERectangle *) (p + m_minPixelBytes);
 			
-			switch (m_myFormat.bitsPerPixel) {
-			case 8:
-				color = COLOR_FROM_PIXEL8_ADDRESS(p); break;
-			case 16:
-				color = COLOR_FROM_PIXEL16_ADDRESS(p); break;
-			case 32:
-				color = COLOR_FROM_PIXEL32_ADDRESS(p); break;
-			};
-			
-			// color = COLOR_FROM_PIXEL8_ADDRESS(netbuf);
 			rect.x = pRect->x + pfburh->r.x;
 			rect.y = pRect->y + pfburh->r.y;
 			rect.w = pRect->w;
diff --git a/vncviewer/ClientConnectionCursor.cpp b/vncviewer/ClientConnectionCursor.cpp
index 86f3bc8..3d3b1a6 100755
--- a/vncviewer/ClientConnectionCursor.cpp
+++ b/vncviewer/ClientConnectionCursor.cpp
@@ -330,8 +330,6 @@ void ClientConnection::SoftCursorDraw() {
 
 	omni_mutex_lock l(m_bitmapdcMutex);
 
-	SETUP_COLOR_SHORTCUTS;
-
 	for (y = 0; y < rcHeight; y++) {
 		y0 = rcCursorY - rcHotY + y;
 		if (y0 >= 0 && y0 < m_si.framebufferHeight) {
diff --git a/vncviewer/ClientConnectionHextile.cpp b/vncviewer/ClientConnectionHextile.cpp
index 9d43264..aea6612 100755
--- a/vncviewer/ClientConnectionHextile.cpp
+++ b/vncviewer/ClientConnectionHextile.cpp
@@ -46,7 +46,6 @@ void ClientConnection::ReadHextileRect(rfbFramebufferUpdateRectHeader *pfburh)
 void ClientConnection::HandleHextileEncoding##bpp(int rx, int ry, int rw, int rh)                    \
 {                                                                             \
     CARD##bpp bg, fg;                                                         \
-	COLORREF bgcolor, fgcolor;												  \
     int i;                                                                    \
     CARD8 *ptr;                                                               \
     int x, y, w, h;                                                           \
@@ -55,7 +54,6 @@ void ClientConnection::HandleHextileEncoding##bpp(int rx, int ry, int rw, int rh
     CARD8 nSubrects;                                                          \
 																			  \
     CheckBufferSize( 16 * 16 * bpp );										  \
-	SETUP_COLOR_SHORTCUTS;                                                    \
                                                                               \
     for (y = ry; y < ry+rh; y += 16) {                                        \
 		omni_mutex_lock l(m_bitmapdcMutex);									  \
@@ -80,13 +78,11 @@ void ClientConnection::HandleHextileEncoding##bpp(int rx, int ry, int rw, int rh
                                                                               \
 		    if (subencoding & rfbHextileBackgroundSpecified) {                \
                 ReadExact((char *)&bg, (bpp/8));                              \
-				bgcolor = COLOR_FROM_PIXEL##bpp##_ADDRESS(&bg);  			  \
 			}																  \
             FillSolidRect_ultra(x,y,w,h, m_myFormat.bitsPerPixel,(BYTE*)&bg);\
                                                                               \
             if (subencoding & rfbHextileForegroundSpecified)  {               \
                 ReadExact((char *)&fg, (bpp/8));                              \
-				fgcolor = COLOR_FROM_PIXEL##bpp##_ADDRESS(&fg);				  \
 			}                                                                 \
                                                                               \
             if (!(subencoding & rfbHextileAnySubrects)) {                     \
@@ -106,7 +102,6 @@ void ClientConnection::HandleHextileEncoding##bpp(int rx, int ry, int rw, int rh
                 ReadExact( m_netbuf, nSubrects * (2 + (bpp / 8)));            \
                                                                               \
                 for (i = 0; i < nSubrects; i++) {                             \
-                    fgcolor = COLOR_FROM_PIXEL##bpp##_ADDRESS(ptr);           \
 					memcpy(&fg,ptr,bpp/8);									  \
 					ptr += (bpp/8);                                           \
                     sx = *ptr >> 4;                                           \
diff --git a/vncviewer/ClientConnectionRRE.cpp b/vncviewer/ClientConnectionRRE.cpp
index 43ba69e..ce23b35 100755
--- a/vncviewer/ClientConnectionRRE.cpp
+++ b/vncviewer/ClientConnectionRRE.cpp
@@ -42,18 +42,6 @@ void ClientConnection::ReadRRERect(rfbFramebufferUpdateRectHeader *pfburh)
 
 	prreh->nSubrects = Swap32IfLE(prreh->nSubrects);
 	
-	SETUP_COLOR_SHORTCUTS;
-    COLORREF color;
-    switch (m_myFormat.bitsPerPixel) {
-        case 8:
-            color = COLOR_FROM_PIXEL8_ADDRESS(pcolor); break;
-        case 16:
-			color = COLOR_FROM_PIXEL16_ADDRESS(pcolor); break;
-        case 24:
-        case 32:
-            color = COLOR_FROM_PIXEL32_ADDRESS(pcolor); break;
-    }
-
 	// No other threads can use bitmap DC
 	omni_mutex_lock l(m_bitmapdcMutex);
 		
@@ -76,15 +64,6 @@ void ClientConnection::ReadRRERect(rfbFramebufferUpdateRectHeader *pfburh)
 	for (CARD32 i = 0; i < prreh->nSubrects; i++) {
 		pRect = (rfbRectangle *) (p + m_minPixelBytes);
 		
-		switch (m_myFormat.bitsPerPixel) {
-		case 8:
-			color = COLOR_FROM_PIXEL8_ADDRESS(p); break;
-		case 16:
-			color = COLOR_FROM_PIXEL16_ADDRESS(p); break;
-		case 32:
-			color = COLOR_FROM_PIXEL32_ADDRESS(p); break;
-		};
-		
 		rect.x = (CARD16) (Swap16IfLE(pRect->x) + pfburh->r.x);
 		rect.y = (CARD16) (Swap16IfLE(pRect->y) + pfburh->r.y);
 		rect.w = Swap16IfLE(pRect->w);
diff --git a/vncviewer/ClientConnectionZlib.cpp b/vncviewer/ClientConnectionZlib.cpp
index eda4c3f..21f240e 100755
--- a/vncviewer/ClientConnectionZlib.cpp
+++ b/vncviewer/ClientConnectionZlib.cpp
@@ -62,8 +62,6 @@ void ClientConnection::ReadZlibRect(rfbFramebufferUpdateRectHeader *pfburh, bool
 	if (ultraVncZlib->decompress(numCompBytes, numRawBytes, (unsigned char *)m_netbuf, m_zlibbuf, zstd) != Z_OK)
 		return;
 
-	SETUP_COLOR_SHORTCUTS;
-
 	// No other threads can use bitmap DC
 	omni_mutex_lock l(m_bitmapdcMutex);	
 
@@ -156,7 +154,6 @@ void ClientConnection::ReadQueueZip(rfbFramebufferUpdateRectHeader *pfburh,HRGN
 		SaveArea(rect);
 		if ( surh.encoding==rfbEncodingRaw) {
 			UINT numpixels = surh.r.w * surh.r.h;
-			SETUP_COLOR_SHORTCUTS;
 			omni_mutex_lock l(m_bitmapdcMutex);						  
 
 			// This big switch is untidy but fast
diff --git a/vncviewer/ClientConnectionZlibHex.cpp b/vncviewer/ClientConnectionZlibHex.cpp
index 4148bb4..10878fa 100755
--- a/vncviewer/ClientConnectionZlibHex.cpp
+++ b/vncviewer/ClientConnectionZlibHex.cpp
@@ -60,8 +60,6 @@ void ClientConnection::ReadZlibHexRect(rfbFramebufferUpdateRectHeader *pfburh, b
 	}
 }
 
-COLORREF bgcolor = 0;
-COLORREF fgcolor = 0;
 BYTE pfgcolor[4];
 BYTE pbgcolor[4];
 
@@ -74,7 +72,6 @@ void ClientConnection::HandleZlibHexEncoding##bpp(int rx, int ry, int rw, int rh
 																				\
     CheckBufferSize((( 16 * 16 + 2 ) * bpp / 8 ) + 20 );											\
     CheckZlibBufferSize((( 16 * 16 + 2 ) * bpp / 8 ) + 20 );										\
-	SETUP_COLOR_SHORTCUTS;														\
 																				\
     for (y = ry; y < ry+rh; y += 16) {											\
 		omni_mutex_lock l(m_bitmapdcMutex);										\
@@ -135,18 +132,14 @@ void ClientConnection::HandleZlibHexSubencodingStream##bpp(int x, int y, int w,
     int sx, sy, sw, sh;															\
     CARD8 nSubrects;															\
 																				\
-	SETUP_COLOR_SHORTCUTS;														\
-																				\
 	if (subencoding & rfbHextileBackgroundSpecified) {							\
 		ReadExact((char *)&bg, (bpp/8));										\
-		bgcolor = COLOR_FROM_PIXEL##bpp##_ADDRESS(&bg);							\
 		memcpy(pbgcolor,&bg,bpp/8);												\
 	}																			\
 	FillSolidRect_ultra(x,y,w,h, m_myFormat.bitsPerPixel,(BYTE*)pbgcolor);\
 																				\
 	if (subencoding & rfbHextileForegroundSpecified)  {							\
 		ReadExact((char *)&fg, (bpp/8));										\
-		fgcolor = COLOR_FROM_PIXEL##bpp##_ADDRESS(&fg);							\
 		memcpy(pfgcolor,&fg,bpp/8);												\
 	}																			\
 																				\
@@ -163,7 +156,6 @@ void ClientConnection::HandleZlibHexSubencodingStream##bpp(int x, int y, int w,
 		ReadExact( m_netbuf, nSubrects * (2 + (bpp / 8)));						\
 																				\
 		for (i = 0; i < nSubrects; i++) {										\
-			fgcolor = COLOR_FROM_PIXEL##bpp##_ADDRESS(ptr);						\
 			memcpy(pfgcolor,ptr,bpp/8);											\
 			ptr += (bpp/8);														\
 			sx = *ptr >> 4;														\
@@ -196,13 +188,10 @@ void ClientConnection::HandleZlibHexSubencodingBuf##bpp(int x, int y, int w, int
 	CARD8 nSubrects;															\
 	int bufIndex = 0;															\
 																				\
-	SETUP_COLOR_SHORTCUTS;														\
-																				\
 	if (subencoding & rfbHextileBackgroundSpecified) {							\
 		bg = *((CARD##bpp *)(buffer + bufIndex));								\
 		bufIndex += (bpp/8);													\
 		/* ReadExact((char *)&bg, (bpp/8)); */									\
-		bgcolor = COLOR_FROM_PIXEL##bpp##_ADDRESS(&bg);							\
 		memcpy(pbgcolor,&bg,bpp/8);												\
 	}																			\
 	FillSolidRect_ultra(x,y,w,h, m_myFormat.bitsPerPixel,(BYTE*)pbgcolor);\
@@ -211,7 +200,6 @@ void ClientConnection::HandleZlibHexSubencodingBuf##bpp(int x, int y, int w, int
 		fg = *((CARD##bpp *)(buffer + bufIndex));								\
 		bufIndex += (bpp/8);													\
 		/* ReadExact((char *)&fg, (bpp/8)); */									\
-		fgcolor = COLOR_FROM_PIXEL##bpp##_ADDRESS(&fg);							\
 		memcpy(pfgcolor,&fg,bpp/8);												\
 	}																			\
 																				\
@@ -230,7 +218,6 @@ void ClientConnection::HandleZlibHexSubencodingBuf##bpp(int x, int y, int w, int
 		/* ReadExact( m_netbuf, nSubrects * (2 + (bpp / 8))); */				\
 																				\
 		for (i = 0; i < nSubrects; i++) {										\
-			fgcolor = COLOR_FROM_PIXEL##bpp##_ADDRESS(ptr);						\
 			memcpy(pfgcolor,ptr,bpp/8);											\
 			ptr += (bpp/8);														\
 			sx = *ptr >> 4;														\
diff --git a/vncviewer/xz.cpp b/vncviewer/xz.cpp
index b8bdd2b..887f9ee 100755
--- a/vncviewer/xz.cpp
+++ b/vncviewer/xz.cpp
@@ -21,11 +21,8 @@
 #define BPP 8
 #define XZYW_ENDIAN ENDIAN_NO
 #define IMAGE_RECT(x,y,w,h,data)                \
-    SETUP_COLOR_SHORTCUTS;                      \
     SETPIXELS(m_netbuf,8,x,y,w,h)
 #define FILL_RECT(x,y,w,h,pix)                          \
-    SETUP_COLOR_SHORTCUTS;                              \
-    COLORREF color = COLOR_FROM_PIXEL8_ADDRESS(&pix);   \
     FillSolidRect_ultra(x,y,w,h, m_myFormat.bitsPerPixel,(BYTE*)&pix)
 
 #include <rfb/xzDecode.h>
@@ -37,11 +34,8 @@
 #define BPP 16
 #define XZYW_ENDIAN ENDIAN_LITTLE
 #define IMAGE_RECT(x,y,w,h,data)                \
-    SETUP_COLOR_SHORTCUTS;                      \
     SETPIXELS(m_netbuf,16,x,y,w,h)
 #define FILL_RECT(x,y,w,h,pix)                          \
-    SETUP_COLOR_SHORTCUTS;                              \
-    COLORREF color = COLOR_FROM_PIXEL16_ADDRESS(&pix);  \
     FillSolidRect_ultra(x,y,w,h, m_myFormat.bitsPerPixel,(BYTE*)&pix)
 
 #include <rfb/xzDecode.h>
@@ -56,11 +50,8 @@
 #undef FILL_RECT
 
 #define IMAGE_RECT(x,y,w,h,data)                \
-    SETUP_COLOR_SHORTCUTS;                      \
     SETPIXELS(m_netbuf,32,x,y,w,h)
 #define FILL_RECT(x,y,w,h,pix)                          \
-    SETUP_COLOR_SHORTCUTS;                              \
-    COLORREF color = COLOR_FROM_PIXEL32_ADDRESS(&pix);  \
     FillSolidRect_ultra(x,y,w,h, m_myFormat.bitsPerPixel,(BYTE*)&pix)
 
 
diff --git a/vncviewer/zrle.cpp b/vncviewer/zrle.cpp
index 988b823..eaa569b 100755
--- a/vncviewer/zrle.cpp
+++ b/vncviewer/zrle.cpp
@@ -21,11 +21,8 @@
 #define BPP 8
 #define ZYWRLE_ENDIAN ENDIAN_NO
 #define IMAGE_RECT(x,y,w,h,data)                \
-    SETUP_COLOR_SHORTCUTS;                      \
     SETPIXELS(m_netbuf,8,x,y,w,h)
 #define FILL_RECT(x,y,w,h,pix)                          \
-    SETUP_COLOR_SHORTCUTS;                              \
-    COLORREF color = COLOR_FROM_PIXEL8_ADDRESS(&pix);   \
     FillSolidRect_ultra(x,y,w,h, m_myFormat.bitsPerPixel,(BYTE*)&pix)
 
 #include <rfb/zrleDecode.h>
@@ -37,11 +34,8 @@
 #define BPP 16
 #define ZYWRLE_ENDIAN ENDIAN_LITTLE
 #define IMAGE_RECT(x,y,w,h,data)                \
-    SETUP_COLOR_SHORTCUTS;                      \
     SETPIXELS(m_netbuf,16,x,y,w,h)
 #define FILL_RECT(x,y,w,h,pix)                          \
-    SETUP_COLOR_SHORTCUTS;                              \
-    COLORREF color = COLOR_FROM_PIXEL16_ADDRESS(&pix);  \
     FillSolidRect_ultra(x,y,w,h, m_myFormat.bitsPerPixel,(BYTE*)&pix)
 
 #include <rfb/zrleDecode.h>
@@ -56,11 +50,8 @@
 #undef FILL_RECT
 
 #define IMAGE_RECT(x,y,w,h,data)                \
-    SETUP_COLOR_SHORTCUTS;                      \
     SETPIXELS(m_netbuf,32,x,y,w,h)
 #define FILL_RECT(x,y,w,h,pix)                          \
-    SETUP_COLOR_SHORTCUTS;                              \
-    COLORREF color = COLOR_FROM_PIXEL32_ADDRESS(&pix);  \
     FillSolidRect_ultra(x,y,w,h, m_myFormat.bitsPerPixel,(BYTE*)&pix)
 
 

I'm sorry for the content that has nothing to do with the main subject.

[Rudi De Vos]

I gonne check this, the depth part need extra attention.
Fixing it for one VNC flavor often breaks it for another.

The original rfb code used
bpp32 and depth32 for RGBA

TigerVNC and other use
bpp 32 (RGBA) with depth 24
bpp 32 with depth 32 indicate a video card/monitor with more colors.

Current only one encoder use the depth, tightEncoder, they strip the A of RGBA when depth is 24.

[tyan0]

Thanks for the reply. I also checked the interoperability with several VNC servers.

I checked with the following VNC servers using all encodings selectable from the option menu with and without the patch.

The server tested: TigerVNC (Linux), TightVNC (Linux), UltraVNC (Windows) and TightVNC (Windows)
The viewer tested: UltraVNC viewer 1.3.3 dev 10 and UltraVNC viewer with the patch viewtopic.php?p=109870#p109864.

Test results are as follows. NG means having the color issue I reported, and OK means working without the issue.

Server Type: Test result for 1.3.3 dev 10 viewer (without the patch)/with the patch
TigerVNC server (Linux) depth=16: OK/OK
TigerVNC server (Linux) depth=24: NG/OK
TigerVNC server (Linux) depth=32: OK/OK
TightVNC server (Linux) depth=16: OK/OK
TightVNC server (Linux) depth=24: NG/OK
UltraVNC server (Windows): OK/OK
TightVNC server (Windows): OK/OK

[tyan0]

I have done additional test with vnc4server (in debian wheezy).

Server Type: Test result for 1.3.3 dev 10 viewer (without the patch)/with the patch
vnc4server (Linux) depth=16: OK/OK
vnc4server (Linux) depth=24: OK/OK

[Rudi De Vos]

Can you attach the files.
The diff is giving me errors

[tyan0]

Please download from:
https://tyan0.yr32.net/files/UltraVNC-BGcolor-patch.zip

[tyan0]

Sorry, the patch was against 1.3.3 dev 10. Here is the patch against git head (553dc8815d65711472fcd45fa01bd922d5fac686).
https://tyan0.yr32.net/files/broken-bac ... -patch.zip

[tyan0]

I found yet another color issue. This also happens with 1.3.3 dev 10, 1.3.2, 1.2.4.0, etc.

Versions:
UltraVNC viewer: 1.3.3 dev 11 (current git head: daa64f029ca776cd70d5d67aefa85bccfc437c40)
vncserver: tightvncserver 1:1.3.10-3 (debian bullseye)
Server OS: Debian GNU Linux (bullseye)
Client OS: Windows 10 x64 (21H1)

Overview:
If UltraVNC viewer connects to tightvncserver (in Linux) using Tight encoding with pixel format of bgr888 (not rgb888) or rgb101010 (depth 30), screen color will be broken.

Steps to reproduce the issue (bgr888 case):
1. Install tightvncserver in debian (bullseye).
2. Start vncserver in debian with

Code: Select all

tightvncserver -geometry 1024x768 -pixelformat bgr888

3. Connect to the vncserver using UltraVNC viewer (1.3.3 dev 11) from Windows10 client machine.

Resulting screenshot with the issue (bgr888 case):
Red and blue components are swapped.


Steps to reproduce the issue (rgb101010 case):
1. Install tightvncserver in debian (bullseye).
2. Start vncserver in debian with

Code: Select all

tightvncserver -geometry 1024x768 -depth 30

3. Connect to the vncserver using UltraVNC viewer (1.3.3 dev 11) from Windows10 client machine.

Resulting screenshot with the issue (rgb101010 case):


Expected screenshot without the issue:


I looked into this problem, and found the following patch fixes the issue.

The patch which fixes the issue:
https://tyan0.yr32.net/files/another-ti ... -issue.zip

P.S.:
I tried to check bgr888 case with tigervncserver (not tightvncserver), however, it seems that tigervncserver has a bug with pixelformat bgr888, the screen color is broken even with any viewer of UltraVNC, RealVNC, TightVNC and TigerVNC (Linux). Therefore, I used TightVNC server in this report instead.

[tyan0]

The patch which fixes the issue:
https://tyan0.yr32.net/files/another-ti ... -issue.zip

The patch has been revised a bit.
https://tyan0.yr32.net/files/another-ti ... sue-v2.zip

[Rudi De Vos]

just added reference to confirm that patches where included.