[gtk-vnc-devel] Fix endianness handling and BGR pixel formats

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

As previously mentioned a few weeks back the BGR pixel format handling is
wrong because we're getting the shifts wrong. In fact we're adjusting for
the difference between local & remote shifts twice and getting it wrong
for the mis-matched case. The adjustment in gvnc_set_local  was just not
needed at all. Since the blk functions are already doing shifts I simply
moved the application of the local shift into the blt code & it 'just works'
for both BGR and RGB pixels.

Next up, we're not checking for endianness differences between client and
server. In the case of a difference, we need to reverse the shifts. eg so
a  shift of  R:16, G:8, B:0, becomes R:0, G:8, B:16. This is done by simply
subtracting the server specified shift from the bits_per_pixel, with the
minor complication of having to also take account of possible padding (eg
the BPP=32, depth=24 case).

With the attached patch we work correctly with RGB, BGR, and mixed endian.
It is considerably nicer than my previous patch for BGR.

Regards,
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 453f9ca2a737 src/blt.h
--- a/src/blt.h	Wed Aug 22 10:40:33 2007 -0400
+++ b/src/blt.h	Wed Aug 22 11:04:17 2007 -0400
@@ -40,9 +40,9 @@ static void FILL(struct gvnc *gvnc, src_
 		int j;
 
 		for (j = 0; j < width; j++) {
-			*dp = ((*sp >> f->red_shift) & gvnc->rm) << gvnc->rp
-			    | ((*sp >> f->green_shift) & gvnc->gm) << gvnc->gp
-			    | ((*sp >> f->blue_shift) & gvnc->bm) << gvnc->bp;
+			*dp = ((*sp >> f->red_shift) & gvnc->rm) << gvnc->local.red_shift
+			    | ((*sp >> f->green_shift) & gvnc->gm) << gvnc->local.green_shift
+			    | ((*sp >> f->blue_shift) & gvnc->bm) << gvnc->local.blue_shift;
 			dp++;
 		}
 		dst += gvnc->local.linesize;
@@ -65,9 +65,9 @@ static void BLIT(struct gvnc *gvnc, uint
 		int j;
 
 		for (j = 0; j < w; j++) {
-			*dp = ((*sp >> f->red_shift) & gvnc->rm) << gvnc->rp
-			    | ((*sp >> f->green_shift) & gvnc->gm) << gvnc->gp
-			    | ((*sp >> f->blue_shift) & gvnc->bm) << gvnc->bp;
+			*dp = ((*sp >> f->red_shift) & gvnc->rm) << gvnc->local.red_shift
+			    | ((*sp >> f->green_shift) & gvnc->gm) << gvnc->local.green_shift
+			    | ((*sp >> f->blue_shift) & gvnc->bm) << gvnc->local.blue_shift;
 			dp++;
 			sp++;
 		}
diff -r 453f9ca2a737 src/gvnc.c
--- a/src/gvnc.c	Wed Aug 22 10:40:33 2007 -0400
+++ b/src/gvnc.c	Wed Aug 22 12:04:49 2007 -0400
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <endian.h>
 
 #include "coroutine.h"
 #include "d3des.h"
@@ -102,7 +103,6 @@ struct gvnc
 	gboolean perfect_match;
 	struct gvnc_framebuffer local;
 
-	int rp, gp, bp;
 	int rm, gm, bm;
 
 	gvnc_blt_func *blt;
@@ -124,7 +124,7 @@ struct gvnc
 };
 
 
-#define DEBUG 0
+#define DEBUG 1
 #if DEBUG
 #define GVNC_DEBUG(fmt, ...) do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
 
@@ -676,9 +676,6 @@ 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",
-		   fmt->bits_per_pixel, fmt->depth, fmt->big_endian_flag, fmt->true_color_flag);
-
 	fmt->red_max         = gvnc_read_u16(gvnc);
 	fmt->green_max       = gvnc_read_u16(gvnc);
 	fmt->blue_max        = gvnc_read_u16(gvnc);
@@ -688,6 +685,24 @@ static void gvnc_read_pixel_format(struc
 	fmt->blue_shift      = gvnc_read_u8(gvnc);
 
 	gvnc_read(gvnc, pad, 3);
+
+	GVNC_DEBUG("Pixel format BPP: %d,  Depth: %d, Endian: %d, True color: %d\n"
+		   "             Mask  red: %3d, green: %3d, blue: %3d\n"
+		   "             Shift red: %3d, green: %3d, blue: %3d\n",
+		   fmt->bits_per_pixel, fmt->depth, fmt->big_endian_flag, fmt->true_color_flag,
+		   fmt->red_max, fmt->green_max, fmt->blue_max,
+		   fmt->red_shift, fmt->green_shift, fmt->blue_shift);
+
+
+	if (((__BYTE_ORDER == __BIG_ENDIAN) && !fmt->big_endian_flag) ||
+	    ((__BYTE_ORDER == __LITTLE_ENDIAN) && fmt->big_endian_flag)) {
+		fmt->red_shift = fmt->bits_per_pixel - fmt->red_shift - (fmt->bits_per_pixel - fmt->depth);
+		fmt->green_shift = fmt->bits_per_pixel - fmt->green_shift - (fmt->bits_per_pixel - fmt->depth);
+		fmt->blue_shift = fmt->bits_per_pixel - fmt->blue_shift - (fmt->bits_per_pixel - fmt->depth);
+
+		GVNC_DEBUG("Flipped shifts Shift red: %3d, green: %3d, blue: %3d\n",
+			   fmt->red_shift, fmt->green_shift, fmt->blue_shift);
+	}
 }
 
 /* initialize function */
@@ -1981,7 +1996,9 @@ gboolean gvnc_set_local(struct gvnc *gvn
 	    fb->blue_mask == gvnc->fmt.blue_max &&
 	    fb->red_shift == gvnc->fmt.red_shift &&
 	    fb->green_shift == gvnc->fmt.green_shift &&
-	    fb->blue_shift == gvnc->fmt.blue_shift)
+	    fb->blue_shift == gvnc->fmt.blue_shift &&
+	    ((gvnc->fmt.big_endian_flag && (__BYTE_ORDER == __BIG_ENDIAN)) ||
+	     (!gvnc->fmt.big_endian_flag && (__BYTE_ORDER == __LITTLE_ENDIAN))))
 		gvnc->perfect_match = TRUE;
 	else
 		gvnc->perfect_match = FALSE;
@@ -1989,17 +2006,6 @@ gboolean gvnc_set_local(struct gvnc *gvn
 	depth = gvnc->fmt.depth;
 	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->rm = gvnc->local.red_mask & gvnc->fmt.red_max;
 	gvnc->gm = gvnc->local.green_mask & gvnc->fmt.green_max;