[gtk-vnc-devel] Fix pixel shift for BGR format servers

["Daniel P. Berrange" <berrange redhat com>]

The code which currently calculates the pixel shift for converting data
from remote to local encoding is assuming the server is in RGB format
pixels. If I run a server in BGR format, then the colours are rather
messed up. I've attached a patch which tries to correct it - although its
not perfect because now, the code assume either RGB, or BGR - is it possible
for a server to use GRB, or GBR ?!?!  If not then this may be sufficient.

I'm also not entirely convinced this is correct wrt to local server
format, eg if your local desktop ran in BGR instead of RGB. I don't have
any system I can make run in BGR locally though, so can't test.

BTW, for testing I was running a server with

vncserver -geometry 1024x800 -depth 16 -pixelformat bgr565

And another with

vncserver -geometry 1024x800 -depth 16 -pixelformat rgb565

With this patch, both now work.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

diff -r 8d2ec85be8c3 src/gvnc.c
--- a/src/gvnc.c	Tue Jul 10 13:34:34 2007 -0400
+++ b/src/gvnc.c	Tue Jul 10 17:10:57 2007 -0400
@@ -127,7 +127,7 @@ enum {
 };
 
 
-#define DEBUG 0
+#define DEBUG 1
 #if DEBUG
 #define GVNC_DEBUG(fmt, ...) do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
 
@@ -543,7 +543,7 @@ static void gvnc_read_pixel_format(struc
 	fmt->big_endian_flag = gvnc_read_u8(gvnc);
 	fmt->true_color_flag = gvnc_read_u8(gvnc);
 
-	GVNC_DEBUG("Pixel format BPP: %d,  Depth: %d, Endian: %d, True color: %d\n",
+	GVNC_DEBUG("Read Pixel format BPP: %d,  Depth: %d, Endian: %d, True color: %d\n",
 		   fmt->bits_per_pixel, fmt->depth, fmt->big_endian_flag, fmt->true_color_flag);
 
 	fmt->red_max         = gvnc_read_u16(gvnc);
@@ -554,6 +554,11 @@ static void gvnc_read_pixel_format(struc
 	fmt->green_shift     = gvnc_read_u8(gvnc);
 	fmt->blue_shift      = gvnc_read_u8(gvnc);
 
+	GVNC_DEBUG("Pixel max: R: %d, G: %d, B:%d\n",
+		   fmt->red_max, fmt->green_max, fmt->blue_max);
+	GVNC_DEBUG("Pixel shift: R: %d, G: %d, B:%d\n",
+		   fmt->red_shift, fmt->green_shift, fmt->blue_shift);
+
 	gvnc_read(gvnc, pad, 3);
 }
 
@@ -571,6 +576,9 @@ gboolean gvnc_set_pixel_format(struct gv
 
 	gvnc_write_u8(gvnc, 0);
 	gvnc_write(gvnc, pad, 3);
+
+	GVNC_DEBUG("Write Pixel format BPP: %d,  Depth: %d, Endian: %d, True color: %d\n",
+		   fmt->bits_per_pixel, fmt->depth, fmt->big_endian_flag, fmt->true_color_flag);
 
 	gvnc_write_u8(gvnc, fmt->bits_per_pixel);
 	gvnc_write_u8(gvnc, fmt->depth);
@@ -584,6 +592,11 @@ gboolean gvnc_set_pixel_format(struct gv
 	gvnc_write_u8(gvnc, fmt->red_shift);
 	gvnc_write_u8(gvnc, fmt->green_shift);
 	gvnc_write_u8(gvnc, fmt->blue_shift);
+
+	GVNC_DEBUG("Pixel max: R: %d, G: %d, B:%d\n",
+		   fmt->red_max, fmt->green_max, fmt->blue_max);
+	GVNC_DEBUG("Pixel shift: R: %d, G: %d, B:%d\n",
+		   fmt->red_shift, fmt->green_shift, fmt->blue_shift);
 
 	gvnc_write(gvnc, pad, 3);
 	gvnc_flush(gvnc);
@@ -1534,16 +1547,26 @@ gboolean gvnc_set_local(struct gvnc *gvn
 	if (depth == 32)
 		depth = 24;
 
-	gvnc->rp =  (gvnc->local.depth - gvnc->local.red_shift);
-	gvnc->rp -= (depth - gvnc->fmt.red_shift);
-	gvnc->gp =  (gvnc->local.red_shift - gvnc->local.green_shift);
-	gvnc->gp -= (gvnc->fmt.red_shift - gvnc->fmt.green_shift);
-	gvnc->bp =  (gvnc->local.green_shift - gvnc->local.blue_shift);
-	gvnc->bp -= (gvnc->fmt.green_shift - gvnc->fmt.blue_shift);
-
-	gvnc->rp = gvnc->local.red_shift + gvnc->rp;
-	gvnc->gp = gvnc->local.green_shift + gvnc->gp;
-	gvnc->bp = gvnc->local.blue_shift + gvnc->bp;
+	GVNC_DEBUG("Depth local %d remote %d\n", gvnc->local.depth, depth);
+	if (gvnc->fmt.red_shift > gvnc->fmt.blue_shift) {
+		gvnc->rp =  gvnc->local.depth;
+		gvnc->rp -= (depth - gvnc->fmt.red_shift);
+
+		gvnc->gp =  gvnc->local.red_shift;
+		gvnc->gp -= (gvnc->fmt.red_shift - gvnc->fmt.green_shift);
+
+		gvnc->bp =  gvnc->local.green_shift;
+		gvnc->bp -= (gvnc->fmt.green_shift - gvnc->fmt.blue_shift);
+	} else {
+		gvnc->rp =  gvnc->local.depth;
+		gvnc->rp -= (gvnc->fmt.green_shift - gvnc->fmt.red_shift);
+
+		gvnc->gp =  gvnc->local.red_shift;
+		gvnc->gp -= (gvnc->fmt.blue_shift - gvnc->fmt.green_shift);
+
+		gvnc->bp =  gvnc->local.green_shift;
+		gvnc->bp -= (depth - gvnc->fmt.blue_shift);
+	}
 
 	gvnc->rm = gvnc->local.red_mask & gvnc->fmt.red_max;
 	gvnc->gm = gvnc->local.green_mask & gvnc->fmt.green_max;