qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] Updated BGR vs. RGB vga patch...
@ 2006-04-10 16:25 Leonardo E. Reiter
  2006-04-10 16:35 ` Paul Brook
  2006-04-10 23:12 ` Thiemo Seufer
  0 siblings, 2 replies; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-10 16:25 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 965 bytes --]

Hello,

attached is an updated version (against today's CVS) of a patch to 
enable B/G/R color encoding rather than R/G/B with the command-line 
option -bgr.  I found the original here (post by Martin Bochnig):

http://lists.nongnu.org/archive/html/qemu-devel/2005-09/msg00059.html

It's main intention is to deal with some 24-bit Sun/SPARC X servers, 
including SunRays.  But you can also mimic this behavior with a VNC 
server if you want.

Anyway, it may not be too interesting to most, but it is to me at the 
moment, so I figured I'd share.  To use it, you can pass in the -bgr 
command line option to qemu.  You can get some psychadelic colors if 
you're really bored and want to try it on an R/G/B 24-bit X server, like 
XFree86 or X.org :)

Regards,

Leo Reiter


-- 
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

[-- Attachment #2: qemu-vga-bgr.patch --]
[-- Type: text/x-patch, Size: 3466 bytes --]

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 14:07:26 -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 14:07:26 -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 14:07:26 -0000
@@ -790,23 +790,43 @@
 
 static inline unsigned int rgb_to_pixel8(unsigned int r, unsigned int g, unsigned b)
 {
+if (bgr_display_enabled) {
+    return ((b >> 5) << 5) | ((g >> 5) << 2) | (r >> 6);
+}
+else {
     return ((r >> 5) << 5) | ((g >> 5) << 2) | (b >> 6);
 }
+}
 
 static inline unsigned int rgb_to_pixel15(unsigned int r, unsigned int g, unsigned b)
 {
+if (bgr_display_enabled) {
+    return ((b >> 3) << 10) | ((g >> 3) << 5) | (r >> 3);
+}
+else {
     return ((r >> 3) << 10) | ((g >> 3) << 5) | (b >> 3);
 }
+}
 
 static inline unsigned int rgb_to_pixel16(unsigned int r, unsigned int g, unsigned b)
 {
+if (bgr_display_enabled) {
+    return ((b >> 3) << 11) | ((g >> 2) << 5) | (r >> 3);
+}
+else {
     return ((r >> 3) << 11) | ((g >> 2) << 5) | (b >> 3);
 }
+}
 
 static inline unsigned int rgb_to_pixel32(unsigned int r, unsigned int g, unsigned b)
 {
+if (bgr_display_enabled) {
+    return (b << 16) | (g << 8) | r;
+}
+else {
     return (r << 16) | (g << 8) | b;
 }
+}
 
 #define DEPTH 8
 #include "vga_template.h"

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:25 [Qemu-devel] Updated BGR vs. RGB vga patch Leonardo E. Reiter
@ 2006-04-10 16:35 ` Paul Brook
  2006-04-10 16:41   ` Leonardo E. Reiter
  2006-04-10 16:44   ` Leonardo E. Reiter
  2006-04-10 23:12 ` Thiemo Seufer
  1 sibling, 2 replies; 17+ messages in thread
From: Paul Brook @ 2006-04-10 16:35 UTC (permalink / raw)
  To: qemu-devel

On Monday 10 April 2006 17:25, Leonardo E. Reiter wrote:
> Hello,
>
> attached is an updated version (against today's CVS) of a patch to
> enable B/G/R color encoding rather than R/G/B with the command-line
> option -bgr.  I found the original here (post by Martin Bochnig):

Shouldn't we be able to figure this out by asking SDL and/or the X server?
Also, adding an if to the low-level conversion routines seems a bad idea for 
performance.

Paul

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:35 ` Paul Brook
@ 2006-04-10 16:41   ` Leonardo E. Reiter
  2006-04-10 16:46     ` Paul Brook
  2006-04-10 16:44   ` Leonardo E. Reiter
  1 sibling, 1 reply; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-10 16:41 UTC (permalink / raw)
  To: Paul Brook; +Cc: qemu-devel

You can definitely figure it out by asking the X server or SDL.  I don't 
know enough SDL to have hacked it in myself.  X is pretty simple on the 
other hand.

As for where to add it, I agree that the low level conversions are not a 
great place to add this.  I didn't originate the patch, I just updated 
it.  Unfortunately none of the VGA is optimized - everything happens 
pixel by pixel already.  Adding what probably amounts to 2 instructions 
(probably a couple of clock cycles) per pixel doesn't seem to do 
anything for performance really.  The VGA is already slow.

Unfortunately X11 will not allow you to force a certain color order in 
24-bit mode.  You can tell it to force byte-order but this only affects 
16-bit blits - it's ignored for 24-bit since the individual components 
of the 24-bit blits, even if packed into 32-bits, are still 8-bits long. 
  That's what the color masks are for.  There is no efficient way to do 
this at the server level that I can see.  If you find a better way 
(within the current scope of the VGA architecture), I'd be glad to try 
to implement it.  I was just sharing something that was useful to me.

Thanks,

Leo Reiter

Paul Brook wrote:
> On Monday 10 April 2006 17:25, Leonardo E. Reiter wrote:
> 
>>Hello,
>>
>>attached is an updated version (against today's CVS) of a patch to
>>enable B/G/R color encoding rather than R/G/B with the command-line
>>option -bgr.  I found the original here (post by Martin Bochnig):
> 
> 
> Shouldn't we be able to figure this out by asking SDL and/or the X server?
> Also, adding an if to the low-level conversion routines seems a bad idea for 
> performance.
> 
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:35 ` Paul Brook
  2006-04-10 16:41   ` Leonardo E. Reiter
@ 2006-04-10 16:44   ` Leonardo E. Reiter
  2006-04-10 16:49     ` Paul Brook
  1 sibling, 1 reply; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-10 16:44 UTC (permalink / raw)
  Cc: qemu-devel

Actually it should probably be made conditionally compiled for now, 
until the VGA architecture changes to something more efficient.  Would 
you agree with that?  That way, people who know they will need the hack 
can enable it at configure/compile time, and the rest will not be affected.

How does that sound?  I'd be happy to post an updated patch that does that.

- Leo

Paul Brook wrote:
> On Monday 10 April 2006 17:25, Leonardo E. Reiter wrote:
> 
>>Hello,
>>
>>attached is an updated version (against today's CVS) of a patch to
>>enable B/G/R color encoding rather than R/G/B with the command-line
>>option -bgr.  I found the original here (post by Martin Bochnig):
> 
> 
> Shouldn't we be able to figure this out by asking SDL and/or the X server?
> Also, adding an if to the low-level conversion routines seems a bad idea for 
> performance.
> 
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:41   ` Leonardo E. Reiter
@ 2006-04-10 16:46     ` Paul Brook
  2006-04-10 16:48       ` Leonardo E. Reiter
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Brook @ 2006-04-10 16:46 UTC (permalink / raw)
  To: qemu-devel

> Unfortunately X11 will not allow you to force a certain color order in
> 24-bit mode.  You can tell it to force byte-order but this only affects
> 16-bit blits - it's ignored for 24-bit since the individual components
> of the 24-bit blits, even if packed into 32-bits, are still 8-bits long.
>   That's what the color masks are for.  There is no efficient way to do
> this at the server level that I can see.  If you find a better way
> (within the current scope of the VGA architecture), I'd be glad to try
> to implement it.  I was just sharing something that was useful to me.

It should be possible to select between different 24-bit packings the same way 
different bit depths are handled.

Paul

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:46     ` Paul Brook
@ 2006-04-10 16:48       ` Leonardo E. Reiter
  0 siblings, 0 replies; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-10 16:48 UTC (permalink / raw)
  To: qemu-devel

Okay, that makes perfect sense.  Let me see if I can figure out how to 
get SDL to report the ordering, and I'll try to roll it out that way.

thanks for clarifying,

- Leo Reiter

Paul Brook wrote:
>>Unfortunately X11 will not allow you to force a certain color order in
>>24-bit mode.  You can tell it to force byte-order but this only affects
>>16-bit blits - it's ignored for 24-bit since the individual components
>>of the 24-bit blits, even if packed into 32-bits, are still 8-bits long.
>>  That's what the color masks are for.  There is no efficient way to do
>>this at the server level that I can see.  If you find a better way
>>(within the current scope of the VGA architecture), I'd be glad to try
>>to implement it.  I was just sharing something that was useful to me.
> 
> 
> It should be possible to select between different 24-bit packings the same way 
> different bit depths are handled.
> 
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:44   ` Leonardo E. Reiter
@ 2006-04-10 16:49     ` Paul Brook
  2006-04-10 16:51       ` Leonardo E. Reiter
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Brook @ 2006-04-10 16:49 UTC (permalink / raw)
  To: qemu-devel

On Monday 10 April 2006 17:44, Leonardo E. Reiter wrote:
> Actually it should probably be made conditionally compiled for now,
> until the VGA architecture changes to something more efficient.  Would
> you agree with that?  That way, people who know they will need the hack
> can enable it at configure/compile time, and the rest will not be affected.
>
> How does that sound?  I'd be happy to post an updated patch that does that.

Making it a configure option sounds a stunningly bad idea. If you're going 
down that route you may as well make all the output format selection a 
compile-time option.

Paul

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:49     ` Paul Brook
@ 2006-04-10 16:51       ` Leonardo E. Reiter
  2006-04-10 16:59         ` Paul Brook
  0 siblings, 1 reply; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-10 16:51 UTC (permalink / raw)
  To: Paul Brook; +Cc: qemu-devel

I was talking about enabling the use of the -bgr flag at compile time, 
to save the check at runtime if the user doesn't even care about the 
flag, not forcing the determination of the blit order altogether at 
compile-time.  In any case, this is moot, your clarification on how to 
better implement it makes it much more clear to me and is a much better 
idea.  Mails to the list are a bit delayed, that's all ;)

Thanks,

Leo

Paul Brook wrote:
> On Monday 10 April 2006 17:44, Leonardo E. Reiter wrote:
> 
>>Actually it should probably be made conditionally compiled for now,
>>until the VGA architecture changes to something more efficient.  Would
>>you agree with that?  That way, people who know they will need the hack
>>can enable it at configure/compile time, and the rest will not be affected.
>>
>>How does that sound?  I'd be happy to post an updated patch that does that.
> 
> 
> Making it a configure option sounds a stunningly bad idea. If you're going 
> down that route you may as well make all the output format selection a 
> compile-time option.
> 
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:51       ` Leonardo E. Reiter
@ 2006-04-10 16:59         ` Paul Brook
  2006-04-10 18:24           ` Leonardo E. Reiter
  2006-04-10 18:25           ` Leonardo E. Reiter
  0 siblings, 2 replies; 17+ messages in thread
From: Paul Brook @ 2006-04-10 16:59 UTC (permalink / raw)
  To: qemu-devel

On Monday 10 April 2006 17:51, Leonardo E. Reiter wrote:
> I was talking about enabling the use of the -bgr flag at compile time,
> to save the check at runtime if the user doesn't even care about the
> flag, not forcing the determination of the blit order altogether at
> compile-time.  In any case, this is moot, your clarification on how to
> better implement it makes it much more clear to me and is a much better
> idea.  Mails to the list are a bit delayed, that's all ;)

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:59         ` Paul Brook
@ 2006-04-10 18:24           ` Leonardo E. Reiter
  2006-04-10 19:08             ` malc
  2006-04-10 18:25           ` Leonardo E. Reiter
  1 sibling, 1 reply; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-10 18:24 UTC (permalink / raw)
  To: qemu-devel

Here's a better patch... 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:59         ` Paul Brook
  2006-04-10 18:24           ` Leonardo E. Reiter
@ 2006-04-10 18:25           ` Leonardo E. Reiter
  1 sibling, 0 replies; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-10 18:25 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]

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


[-- Attachment #2: qemu-vga-bgr-fast.patch --]
[-- Type: text/x-patch, Size: 11488 bytes --]

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 18:24           ` Leonardo E. Reiter
@ 2006-04-10 19:08             ` malc
  2006-04-10 19:11               ` Leonardo E. Reiter
                                 ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: malc @ 2006-04-10 19:08 UTC (permalink / raw)
  To: qemu-devel

On Mon, 10 Apr 2006, Leonardo E. Reiter wrote:

> 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.

I maybe going out on a limb here, but isn't rgb/bgr trivially deducible
from: sdl.c:screen->format->[RGBA]shift.

-- 
mailto:malc@pulsesoft.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 19:08             ` malc
@ 2006-04-10 19:11               ` Leonardo E. Reiter
  2006-04-11 17:06               ` Leonardo E. Reiter
  2006-04-11 17:11               ` Leonardo E. Reiter
  2 siblings, 0 replies; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-10 19:11 UTC (permalink / raw)
  To: qemu-devel

Probably... I just don't know anything about SDL and I needed this patch 
to work right away with at least the command-line option.  It's 
definitely deducible from X11, and that's very easy.

- Leo Reiter

malc wrote:
> On Mon, 10 Apr 2006, Leonardo E. Reiter wrote:
> 
>> 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.
> 
> 
> I maybe going out on a limb here, but isn't rgb/bgr trivially deducible
> from: sdl.c:screen->format->[RGBA]shift.
> 

-- 
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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 16:25 [Qemu-devel] Updated BGR vs. RGB vga patch Leonardo E. Reiter
  2006-04-10 16:35 ` Paul Brook
@ 2006-04-10 23:12 ` Thiemo Seufer
  2006-04-10 23:22   ` Leonardo E. Reiter
  1 sibling, 1 reply; 17+ messages in thread
From: Thiemo Seufer @ 2006-04-10 23:12 UTC (permalink / raw)
  To: qemu-devel

Leonardo E. Reiter wrote:
> Hello,
> 
> attached is an updated version (against today's CVS) of a patch to 
> enable B/G/R color encoding rather than R/G/B with the command-line 
> option -bgr.  I found the original here (post by Martin Bochnig):
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2005-09/msg00059.html
> 
> It's main intention is to deal with some 24-bit Sun/SPARC X servers, 
> including SunRays.  But you can also mimic this behavior with a VNC 
> server if you want.
> 
> Anyway, it may not be too interesting to most, but it is to me at the 
> moment, so I figured I'd share.  To use it, you can pass in the -bgr 
> command line option to qemu.  You can get some psychadelic colors if 
> you're really bored and want to try it on an R/G/B 24-bit X server, like 
> XFree86 or X.org :)

FWIW, the SGI O2 has the same problem, I heard the Xorg server got
fixed recently to take this into account. I don't know if the fix is
O2 specific.

Btw, wouldn't it be the right thing to add a Sun framebuffer emulation
instead of fiddling with VGA? Or is the hardware in question actually
VGA compatible?


Thiemo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 23:12 ` Thiemo Seufer
@ 2006-04-10 23:22   ` Leonardo E. Reiter
  0 siblings, 0 replies; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-10 23:22 UTC (permalink / raw)
  To: qemu-devel

Hi Thiemo,

the goal of my patch (and the original which I derived from) was to give 
the VGA device the ability to display correctly on BGR X servers.  Most 
likely this is for target-i386 emulation, such as for running Windows 
guests.  That's my need for the patch anyway.  In my case, I'm trying to 
get the colors to look right on a Sun Ray thin client served up from an 
x86_64 Linux box running the SRSS software from Sun (SunRay server 
basically.)  The emulator sessions run Windows 2000/XP, hence the need 
for VGA fixes.

If the O2's 24-bit display is similar to the Sun's (i.e. B/G/R, 24-bit 
depth, 32-bit pixel width), then the patch should fix the problem on the 
O2 as well when using PC hardware emulation (i.e. i386-softmmu)

- Leo Reiter

Thiemo Seufer wrote:
> 
> FWIW, the SGI O2 has the same problem, I heard the Xorg server got
> fixed recently to take this into account. I don't know if the fix is
> O2 specific.
> 
> Btw, wouldn't it be the right thing to add a Sun framebuffer emulation
> instead of fiddling with VGA? Or is the hardware in question actually
> VGA compatible?
> 
> 
> Thiemo
> 
> 
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel

-- 
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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 19:08             ` malc
  2006-04-10 19:11               ` Leonardo E. Reiter
@ 2006-04-11 17:06               ` Leonardo E. Reiter
  2006-04-11 17:11               ` Leonardo E. Reiter
  2 siblings, 0 replies; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-11 17:06 UTC (permalink / raw)
  To: qemu-devel

How's this:

Index: sdl.c
===================================================================
RCS file: /cvsroot/qemu/qemu/sdl.c,v
retrieving revision 1.25
diff -a -u -r1.25 sdl.c
--- sdl.c       9 Apr 2006 01:06:34 -0000       1.25
+++ sdl.c       11 Apr 2006 17:03:51 -0000
@@ -510,4 +510,9 @@
          gui_fullscreen_initial_grab = 1;
          sdl_grab_start();
      }
+
+    /* guess to use BGR mode by seeing if the blue color mask is 
greater than
+     * the red color mask, indicating that blue comes before red */
+    if (screen->format->Bmask > screen->format->Rmask)
+        bgr_display_enabled = 1;
  }

combined with my prior bgr patch of course (bgr_display_enabled doesn't 
exist otherwise.)  If you think it's right, let me know, or post a 
better one.  Either way I can post an updated BGR patch with this hack. 
  You can still manually use -bgr if you want, or perhaps the meaning of 
-bgr should be to enable this test.  It may not be correct in all 
circumstances.  Maybe Paul Brook can weigh in too since he originally 
suggested that this patch should determine this automatically.

Thanks,

Leo Reiter

malc wrote:
> On Mon, 10 Apr 2006, Leonardo E. Reiter wrote:
> 
>> 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.
> 
> 
> I maybe going out on a limb here, but isn't rgb/bgr trivially deducible
> from: sdl.c:screen->format->[RGBA]shift.
> 

-- 
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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Updated BGR vs. RGB vga patch...
  2006-04-10 19:08             ` malc
  2006-04-10 19:11               ` Leonardo E. Reiter
  2006-04-11 17:06               ` Leonardo E. Reiter
@ 2006-04-11 17:11               ` Leonardo E. Reiter
  2 siblings, 0 replies; 17+ messages in thread
From: Leonardo E. Reiter @ 2006-04-11 17:11 UTC (permalink / raw)
  To: qemu-devel

By the way, I did verify that the patch works correctly in B/G/R 5/6/5 
mode - I wasn't sure about 16-bit BGR before.  However, in 16-bit mode, 
only -std-vga works correctly if the X server is in BGR mode.  In 24-bit 
BGR mode, cirrus has no problem.  I think there is some 16-bit trickery 
going on in the cirrus implementation, but I haven't had much time to 
figure it out.  Also the cirrus implementation is really hard to follow 
for me, so if anyone has any idea, I'd appreciate it.

Thanks,

Leo Reiter

malc wrote:
> On Mon, 10 Apr 2006, Leonardo E. Reiter wrote:
> 
>> 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.
> 
> 
> I maybe going out on a limb here, but isn't rgb/bgr trivially deducible
> from: sdl.c:screen->format->[RGBA]shift.
> 

-- 
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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2006-04-11 17:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-10 16:25 [Qemu-devel] Updated BGR vs. RGB vga patch Leonardo E. Reiter
2006-04-10 16:35 ` Paul Brook
2006-04-10 16:41   ` Leonardo E. Reiter
2006-04-10 16:46     ` Paul Brook
2006-04-10 16:48       ` Leonardo E. Reiter
2006-04-10 16:44   ` Leonardo E. Reiter
2006-04-10 16:49     ` Paul Brook
2006-04-10 16:51       ` Leonardo E. Reiter
2006-04-10 16:59         ` Paul Brook
2006-04-10 18:24           ` Leonardo E. Reiter
2006-04-10 19:08             ` malc
2006-04-10 19:11               ` Leonardo E. Reiter
2006-04-11 17:06               ` Leonardo E. Reiter
2006-04-11 17:11               ` Leonardo E. Reiter
2006-04-10 18:25           ` Leonardo E. Reiter
2006-04-10 23:12 ` Thiemo Seufer
2006-04-10 23:22   ` Leonardo E. Reiter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).