Re: [Qemu-devel] Updated BGR vs. RGB vga patch...

[Leonardo E. Reiter]

Here's a better patch (attached this time!)...

I can't seem to validate BGR X servers in 15 or
16-bit mode.  This may not make sense (or be prevalent) on little-endian
machines anyway (I'm using VNC server on a little endian box to test
against.)  In any case, 24 and 32-bit works like a charm.  I use the
same basic logic as the original patch, so it's possible it was always
broken for 16-bit... especially given that most Sun X servers run in
24-bit, which is what the originators intended the patch for.

Anyway, I didn't spend time figuring out how to query SDL for the
ordering.  You still have to manually pass in -bgr.  The good news is
that all the computation is now done at compile-time, with only very few
tests done at run-time.  I agree with your assessment of compile-time
versus run-time options... I was merely suggesting a compromise to fend
off the minor performance hit of using the old patch.  But the method
you suggested is much better and that's how it's implemented now.

Thanks,

Leo Reiter

Paul Brook wrote:
> Ok. For the record I also think it's a bad idea to have features conditionally 
> compiled. Either something is worth including, or we have to ask whether 
> there's any point having it in qemu at all.
> The exception being debug code, which probably isn't useful unless you're 
> already building qemu from source.
>
> Paul

--
Leonardo E. Reiter
Vice President of Product Development, CTO

Win4Lin, Inc.
Virtual Computing from Desktop to Data Center
Main: +1 512 339 7979
Fax: +1 512 532 6501
http://www.win4lin.com

Index: vl.c
===================================================================
RCS file: /cvsroot/qemu/qemu/vl.c,v
retrieving revision 1.168
diff -a -u -r1.168 vl.c
--- vl.c        9 Apr 2006 01:32:52 -0000       1.168
+++ vl.c        10 Apr 2006 17:57:12 -0000
@@ -128,6 +128,7 @@
 int vm_running;
 int rtc_utc = 1;
 int cirrus_vga_enabled = 1;
+int bgr_display_enabled = 0;
 #ifdef TARGET_SPARC
 int graphic_width = 1024;
 int graphic_height = 768;
@@ -4131,6 +4132,7 @@
            "-m megs         set virtual RAM size to megs MB [default=%d]\n"
            "-smp n          set the number of CPUs to 'n' [default=1]\n"
            "-nographic      disable graphical output and redirect serial I/Os 
to console\n"
+           "-bgr            invert colors for HOSTS using certain SPARC frame 
buffers\n"
 #ifndef _WIN32
           "-k language     use keyboard layout (for example \"fr\" for 
French)\n"
 #endif
@@ -4283,6 +4285,7 @@
     QEMU_OPTION_cirrusvga,
     QEMU_OPTION_g,
     QEMU_OPTION_std_vga,
+    QEMU_OPTION_bgr,
     QEMU_OPTION_monitor,
     QEMU_OPTION_serial,
     QEMU_OPTION_parallel,
@@ -4360,6 +4363,7 @@
     { "full-screen", 0, QEMU_OPTION_full_screen },
     { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
     { "win2k-hack", 0, QEMU_OPTION_win2k_hack },
+    { "bgr", 0, QEMU_OPTION_bgr },
     { "usbdevice", HAS_ARG, QEMU_OPTION_usbdevice },
     { "smp", HAS_ARG, QEMU_OPTION_smp },
     
@@ -4850,6 +4854,9 @@
             case QEMU_OPTION_std_vga:
                 cirrus_vga_enabled = 0;
                 break;
+            case QEMU_OPTION_bgr:
+                bgr_display_enabled = 1;
+                break;
             case QEMU_OPTION_g:
                 {
                     const char *p;
Index: vl.h
===================================================================
RCS file: /cvsroot/qemu/qemu/vl.h,v
retrieving revision 1.107
diff -a -u -r1.107 vl.h
--- vl.h        9 Apr 2006 01:32:52 -0000       1.107
+++ vl.h        10 Apr 2006 17:57:12 -0000
@@ -130,6 +130,7 @@
 extern int bios_size;
 extern int rtc_utc;
 extern int cirrus_vga_enabled;
+extern int bgr_display_enabled;
 extern int graphic_width;
 extern int graphic_height;
 extern int graphic_depth;
Index: hw/vga.c
===================================================================
RCS file: /cvsroot/qemu/qemu/hw/vga.c,v
retrieving revision 1.42
diff -a -u -r1.42 vga.c
--- hw/vga.c    9 Apr 2006 01:06:34 -0000       1.42
+++ hw/vga.c    10 Apr 2006 17:57:12 -0000
@@ -810,15 +810,27 @@
 
 #define DEPTH 8
 #include "vga_template.h"
+#define BGR_DISPLAY_TYPE
+#define DEPTH 8
+#include "vga_template.h"
 
 #define DEPTH 15
 #include "vga_template.h"
+#define BGR_DISPLAY_TYPE
+#define DEPTH 15
+#include "vga_template.h"
 
 #define DEPTH 16
 #include "vga_template.h"
+#define BGR_DISPLAY_TYPE
+#define DEPTH 16
+#include "vga_template.h"
 
 #define DEPTH 32
 #include "vga_template.h"
+#define BGR_DISPLAY_TYPE
+#define DEPTH 32
+#include "vga_template.h"
 
 static unsigned int rgb_to_pixel8_dup(unsigned int r, unsigned int g, unsigned 
b)
 {
@@ -829,6 +841,15 @@
     return col;
 }
 
+static unsigned int bgr_to_pixel8_dup(unsigned int r, unsigned int g, unsigned 
b)
+{
+    unsigned int col;
+    col = rgb_to_pixel8(b, g, r);
+    col |= col << 8;
+    col |= col << 16;
+    return col;
+}
+
 static unsigned int rgb_to_pixel15_dup(unsigned int r, unsigned int g, 
unsigned b)
 {
     unsigned int col;
@@ -837,6 +858,14 @@
     return col;
 }
 
+static unsigned int bgr_to_pixel15_dup(unsigned int r, unsigned int g, 
unsigned b)
+{
+    unsigned int col;
+    col = rgb_to_pixel15(b, g, r);
+    col |= col << 16;
+    return col;
+}
+
 static unsigned int rgb_to_pixel16_dup(unsigned int r, unsigned int g, 
unsigned b)
 {
     unsigned int col;
@@ -845,13 +874,24 @@
     return col;
 }
 
-static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, 
unsigned b)
+static unsigned int bgr_to_pixel16_dup(unsigned int r, unsigned int g, 
unsigned b)
 {
     unsigned int col;
-    col = rgb_to_pixel32(r, g, b);
+    col = rgb_to_pixel16(b, g, r);
+    col |= col << 16;
     return col;
 }
 
+static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, 
unsigned b)
+{
+    return rgb_to_pixel32(r, g, b);
+}
+
+static unsigned int bgr_to_pixel32_dup(unsigned int r, unsigned int g, 
unsigned b)
+{
+    return rgb_to_pixel32(b, g, r);
+}
+
 /* return true if the palette was modified */
 static int update_palette16(VGAState *s)
 {
@@ -1190,9 +1230,13 @@
     VGA_DRAW_LINE8D2,
     VGA_DRAW_LINE8,
     VGA_DRAW_LINE15,
+    VGA_DRAW_LINE15BGR,
     VGA_DRAW_LINE16,
+    VGA_DRAW_LINE16BGR,
     VGA_DRAW_LINE24,
+    VGA_DRAW_LINE24BGR,
     VGA_DRAW_LINE32,
+    VGA_DRAW_LINE32BGR,
     VGA_DRAW_LINE_NB,
 };
 
@@ -1232,20 +1276,40 @@
     vga_draw_line15_16,
     vga_draw_line15_32,
 
+    vga_draw_line15bgr_8,
+    vga_draw_line15bgr_15,
+    vga_draw_line15bgr_16,
+    vga_draw_line15bgr_32,
+
     vga_draw_line16_8,
     vga_draw_line16_15,
     vga_draw_line16_16,
     vga_draw_line16_32,
 
+    vga_draw_line16bgr_8,
+    vga_draw_line16bgr_15,
+    vga_draw_line16bgr_16,
+    vga_draw_line16bgr_32,
+
     vga_draw_line24_8,
     vga_draw_line24_15,
     vga_draw_line24_16,
     vga_draw_line24_32,
 
+    vga_draw_line24bgr_8,
+    vga_draw_line24bgr_15,
+    vga_draw_line24bgr_16,
+    vga_draw_line24bgr_32,
+
     vga_draw_line32_8,
     vga_draw_line32_15,
     vga_draw_line32_16,
     vga_draw_line32_32,
+
+    vga_draw_line32bgr_8,
+    vga_draw_line32bgr_15,
+    vga_draw_line32bgr_16,
+    vga_draw_line32bgr_32,
 };
 
 static int vga_get_bpp(VGAState *s)
@@ -1349,16 +1413,16 @@
             v = VGA_DRAW_LINE8;
             break;
         case 15:
-            v = VGA_DRAW_LINE15;
+            v = bgr_display_enabled ? VGA_DRAW_LINE15BGR : VGA_DRAW_LINE15;
             break;
         case 16:
-            v = VGA_DRAW_LINE16;
+            v = bgr_display_enabled ? VGA_DRAW_LINE16BGR : VGA_DRAW_LINE16;
             break;
         case 24:
-            v = VGA_DRAW_LINE24;
+            v = bgr_display_enabled ? VGA_DRAW_LINE24BGR : VGA_DRAW_LINE24;
             break;
         case 32:
-            v = VGA_DRAW_LINE32;
+            v = bgr_display_enabled ? VGA_DRAW_LINE32BGR : VGA_DRAW_LINE32;
             break;
         }
     }
@@ -1494,17 +1558,21 @@
     } else {
         switch(s->ds->depth) {
         case 8:
-            s->rgb_to_pixel = rgb_to_pixel8_dup;
+            s->rgb_to_pixel = 
+                bgr_display_enabled ? bgr_to_pixel8_dup : rgb_to_pixel8_dup;
             break;
         case 15:
-            s->rgb_to_pixel = rgb_to_pixel15_dup;
+            s->rgb_to_pixel = 
+                bgr_display_enabled ? bgr_to_pixel15_dup : rgb_to_pixel15_dup;
             break;
         default:
         case 16:
-            s->rgb_to_pixel = rgb_to_pixel16_dup;
+            s->rgb_to_pixel = 
+                bgr_display_enabled ? bgr_to_pixel16_dup : rgb_to_pixel16_dup;
             break;
         case 32:
-            s->rgb_to_pixel = rgb_to_pixel32_dup;
+            s->rgb_to_pixel = 
+                bgr_display_enabled ? bgr_to_pixel32_dup : rgb_to_pixel32_dup;
             break;
         }
         
Index: hw/vga_template.h
===================================================================
RCS file: /cvsroot/qemu/qemu/hw/vga_template.h,v
retrieving revision 1.11
diff -a -u -r1.11 vga_template.h
--- hw/vga_template.h   10 Oct 2004 15:44:19 -0000      1.11
+++ hw/vga_template.h   10 Apr 2006 17:57:13 -0000
@@ -35,6 +35,27 @@
 #error unsupport depth
 #endif
 
+/* draw-line function base names for RGB vs. BGR */
+#ifndef BGR_DISPLAY_TYPE
+#define VGA_DRAW_LINE15_    vga_draw_line15_
+#define VGA_DRAW_LINE16_    vga_draw_line16_
+#define VGA_DRAW_LINE24_    vga_draw_line24_
+#define VGA_DRAW_LINE32_    vga_draw_line32_
+#define __R_VALUE   r
+#define __G_VALUE   g
+#define __B_VALUE   b
+#else
+#define VGA_DRAW_LINE15_    vga_draw_line15bgr_
+#define VGA_DRAW_LINE16_    vga_draw_line16bgr_
+#define VGA_DRAW_LINE24_    vga_draw_line24bgr_
+#define VGA_DRAW_LINE32_    vga_draw_line32bgr_
+#define __R_VALUE   b
+#define __G_VALUE   g
+#define __B_VALUE   r
+#endif  /* !BGR_DISPLAY_TYPE */
+
+/* avoid redifining common functions when we generate the BGR-only functions */
+#ifndef BGR_DISPLAY_TYPE
 #if DEPTH != 15
 
 static inline void glue(vga_draw_glyph_line_, DEPTH)(uint8_t *d, 
@@ -335,14 +356,14 @@
 }
 
 #endif /* DEPTH != 15 */
-
+#endif /* !BGR_DISPLAY_TYPE */
 
 /* XXX: optimize */
 
 /* 
  * 15 bit color
  */
-static void glue(vga_draw_line15_, DEPTH)(VGAState *s1, uint8_t *d, 
+static void glue(VGA_DRAW_LINE15_, DEPTH)(VGAState *s1, uint8_t *d, 
                                           const uint8_t *s, int width)
 {
 #if DEPTH == 15 && defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
@@ -354,9 +375,9 @@
     w = width;
     do {
         v = lduw_raw((void *)s);
-        r = (v >> 7) & 0xf8;
-        g = (v >> 2) & 0xf8;
-        b = (v << 3) & 0xf8;
+        __R_VALUE = (v >> 7) & 0xf8;
+        __G_VALUE = (v >> 2) & 0xf8;
+        __B_VALUE = (v << 3) & 0xf8;
         ((PIXEL_TYPE *)d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
         s += 2;
         d += BPP;
@@ -367,7 +388,7 @@
 /* 
  * 16 bit color
  */
-static void glue(vga_draw_line16_, DEPTH)(VGAState *s1, uint8_t *d, 
+static void glue(VGA_DRAW_LINE16_, DEPTH)(VGAState *s1, uint8_t *d, 
                                           const uint8_t *s, int width)
 {
 #if DEPTH == 16 && defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
@@ -379,9 +400,9 @@
     w = width;
     do {
         v = lduw_raw((void *)s);
-        r = (v >> 8) & 0xf8;
-        g = (v >> 3) & 0xfc;
-        b = (v << 3) & 0xf8;
+        __R_VALUE = (v >> 8) & 0xf8;
+        __G_VALUE = (v >> 3) & 0xfc;
+        __B_VALUE = (v << 3) & 0xf8;
         ((PIXEL_TYPE *)d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b);
         s += 2;
         d += BPP;
@@ -392,7 +413,7 @@
 /* 
  * 24 bit color
  */
-static void glue(vga_draw_line24_, DEPTH)(VGAState *s1, uint8_t *d, 
+static void glue(VGA_DRAW_LINE24_, DEPTH)(VGAState *s1, uint8_t *d, 
                                           const uint8_t *s, int width)
 {
     int w;
@@ -400,7 +421,7 @@
 
     w = width;
     do {
-#if defined(TARGET_WORDS_BIGENDIAN)
+#if defined(TARGET_WORDS_BIGENDIAN) || defined(BGR_DISPLAY_TYPE)
         r = s[0];
         g = s[1];
         b = s[2];
@@ -418,7 +439,7 @@
 /* 
  * 32 bit color
  */
-static void glue(vga_draw_line32_, DEPTH)(VGAState *s1, uint8_t *d, 
+static void glue(VGA_DRAW_LINE32_, DEPTH)(VGAState *s1, uint8_t *d, 
                                           const uint8_t *s, int width)
 {
 #if DEPTH == 32 && defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
@@ -429,7 +450,7 @@
 
     w = width;
     do {
-#if defined(TARGET_WORDS_BIGENDIAN)
+#if defined(TARGET_WORDS_BIGENDIAN) || defined(BGR_DISPLAY_TYPE)
         r = s[1];
         g = s[2];
         b = s[3];
@@ -445,6 +466,7 @@
 #endif
 }
 
+#ifndef BGR_DISPLAY_TYPE
 #if DEPTH != 15
 void glue(vga_draw_cursor_line_, DEPTH)(uint8_t *d1, 
                                         const uint8_t *src1, 
@@ -512,8 +534,19 @@
     }
 }
 #endif
+#endif  /* !BGR_DISPLAY_TYPE */
 
 #undef PUT_PIXEL2
 #undef DEPTH
 #undef BPP
 #undef PIXEL_TYPE
+#undef VGA_DRAW_LINE15_
+#undef VGA_DRAW_LINE16_
+#undef VGA_DRAW_LINE24_
+#undef VGA_DRAW_LINE32_
+#undef __R_VALUE
+#undef __G_VALUE
+#undef __B_VALUE
+#ifdef BGR_DISPLAY_TYPE
+#undef BGR_DISPLAY_TYPE
+#endif