From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1FT14l-0002Cs-9T for qemu-devel@nongnu.org; Mon, 10 Apr 2006 14:25:23 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1FT14j-0002BL-HK for qemu-devel@nongnu.org; Mon, 10 Apr 2006 14:25:22 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1FT14j-0002B2-A6 for qemu-devel@nongnu.org; Mon, 10 Apr 2006 14:25:21 -0400 Received: from [216.148.227.154] (helo=rwcrmhc14.comcast.net) by monty-python.gnu.org with esmtp (Exim 4.52) id 1FT19P-00024F-U1 for qemu-devel@nongnu.org; Mon, 10 Apr 2006 14:30:12 -0400 Message-ID: <443AA30D.80302@win4lin.com> Date: Mon, 10 Apr 2006 14:25:17 -0400 From: "Leonardo E. Reiter" MIME-Version: 1.0 Subject: Re: [Qemu-devel] Updated BGR vs. RGB vga patch... References: <443A86FC.9010302@win4lin.com> <200604101749.00725.paul@codesourcery.com> <443A8D0C.4030807@win4lin.com> <200604101759.34399.paul@codesourcery.com> In-Reply-To: <200604101759.34399.paul@codesourcery.com> Content-Type: multipart/mixed; boundary="------------010806060700070704070801" Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------010806060700070704070801 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 --------------010806060700070704070801 Content-Type: text/x-patch; name="qemu-vga-bgr-fast.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="qemu-vga-bgr-fast.patch" 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 --------------010806060700070704070801--