On 6/8/07, Andreas Färber <[EMAIL PROTECTED]> wrote:
> Blue Swirl wrote: >> On 6/6/07, Andreas Färber <[EMAIL PROTECTED]> wrote: >>> I also made some local changes to tcx.c today to avoid having a >>> blue- >>> footed penguin on Intel. There's a TODO in that file saying the RGB >>> functions should be merged with vga.c (where they are being patched >>> by Q) - should I provide a qemu patch doing this and fixing the >>> issue >>> for both? Or is this for some reason restricted to Q? >> >> I made a patch to fix both vga and tcx while also unifying the pixel >> operations, does it fix the blue foot disease on PPC? > > Hm, _with_ this patch I get (on PPC/Linux, emulating LE MIPS): > - a blue-footed penguin when the cirrus FB is started > - blue instead of red colour in console output > - A general blue-to-red inversion for X11 in 16bit mode. > > Note that the colours for the last one were already not quite right > (e.g. grey became a pale blue), but with your patch it looks even > weirder. I haven't been able to test the new patch yet. Q's patch and my adaptation for tcx just reversed the order of the colors, just like BlueSwirl's patch except that they used #if __LITTLE_ENDIAN__ in place of #ifdef WORDS_BIGENDIAN, and this worked for both i386 and sparc32 guests on i386 host (but is not applied for ppc host). http://www.kju-app.org/proj/browser/trunk/patches/q_vga.c_02.diff
I think this is not correct either, instead the DisplayState bgr attribute should be used. This version should work like before, and on BGR displays the colours should be correct. VGA needs a similar change for 15 and 16 bit depths if I'm correct.
Index: qemu/hw/pixel_ops.h =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ qemu/hw/pixel_ops.h 2007-06-08 16:08:58.000000000 +0000 @@ -0,0 +1,41 @@ +static inline unsigned int rgb_to_pixel8(unsigned int r, unsigned int g, + unsigned int b) +{ + return ((r >> 5) << 5) | ((g >> 5) << 2) | (b >> 6); +} + +static inline unsigned int rgb_to_pixel15(unsigned int r, unsigned int g, + unsigned int b) +{ + return ((r >> 3) << 10) | ((g >> 3) << 5) | (b >> 3); +} + +static inline unsigned int rgb_to_pixel15bgr(unsigned int r, unsigned int g, + unsigned int b) +{ + return ((b >> 3) << 10) | ((g >> 3) << 5) | (r >> 3); +} + +static inline unsigned int rgb_to_pixel16(unsigned int r, unsigned int g, + unsigned int b) +{ + return ((r >> 3) << 11) | ((g >> 2) << 5) | (b >> 3); +} + +static inline unsigned int rgb_to_pixel16bgr(unsigned int r, unsigned int g, + unsigned int b) +{ + return ((b >> 3) << 11) | ((g >> 2) << 5) | (r >> 3); +} + +static inline unsigned int rgb_to_pixel32(unsigned int r, unsigned int g, + unsigned int b) +{ + return (r << 16) | (g << 8) | b; +} + +static inline unsigned int rgb_to_pixel32bgr(unsigned int r, unsigned int g, + unsigned int b) +{ + return (b << 16) | (g << 8) | r; +} Index: qemu/hw/tcx.c =================================================================== --- qemu.orig/hw/tcx.c 2007-06-07 19:35:27.000000000 +0000 +++ qemu/hw/tcx.c 2007-06-08 16:09:03.000000000 +0000 @@ -22,6 +22,7 @@ * THE SOFTWARE. */ #include "vl.h" +#include "pixel_ops.h" #define MAXX 1024 #define MAXY 768 @@ -47,27 +48,6 @@ static void tcx_invalidate_display(void *opaque); static void tcx24_invalidate_display(void *opaque); -/* XXX: unify with vga draw line functions */ -static inline unsigned int rgb_to_pixel8(unsigned int r, unsigned int g, unsigned b) -{ - return ((r >> 5) << 5) | ((g >> 5) << 2) | (b >> 6); -} - -static inline unsigned int rgb_to_pixel15(unsigned int r, unsigned int g, unsigned b) -{ - return ((r >> 3) << 10) | ((g >> 3) << 5) | (b >> 3); -} - -static inline unsigned int rgb_to_pixel16(unsigned int r, unsigned int g, unsigned b) -{ - return ((r >> 3) << 11) | ((g >> 2) << 5) | (b >> 3); -} - -static inline unsigned int rgb_to_pixel32(unsigned int r, unsigned int g, unsigned b) -{ - return (r << 16) | (g << 8) | b; -} - static void update_palette_entries(TCXState *s, int start, int end) { int i; @@ -78,13 +58,22 @@ s->palette[i] = rgb_to_pixel8(s->r[i], s->g[i], s->b[i]); break; case 15: - s->palette[i] = rgb_to_pixel15(s->r[i], s->g[i], s->b[i]); + if (s->ds->bgr) + s->palette[i] = rgb_to_pixel15bgr(s->r[i], s->g[i], s->b[i]); + else + s->palette[i] = rgb_to_pixel15(s->r[i], s->g[i], s->b[i]); break; case 16: - s->palette[i] = rgb_to_pixel16(s->r[i], s->g[i], s->b[i]); + if (s->ds->bgr) + s->palette[i] = rgb_to_pixel16bgr(s->r[i], s->g[i], s->b[i]); + else + s->palette[i] = rgb_to_pixel16(s->r[i], s->g[i], s->b[i]); break; case 32: - s->palette[i] = rgb_to_pixel32(s->r[i], s->g[i], s->b[i]); + if (s->ds->bgr) + s->palette[i] = rgb_to_pixel32bgr(s->r[i], s->g[i], s->b[i]); + else + s->palette[i] = rgb_to_pixel32(s->r[i], s->g[i], s->b[i]); break; } } Index: qemu/hw/vga.c =================================================================== --- qemu.orig/hw/vga.c 2007-06-07 19:36:38.000000000 +0000 +++ qemu/hw/vga.c 2007-06-08 16:17:02.000000000 +0000 @@ -23,6 +23,7 @@ */ #include "vl.h" #include "vga_int.h" +#include "pixel_ops.h" //#define DEBUG_VGA //#define DEBUG_VGA_MEM @@ -812,31 +813,6 @@ typedef void vga_draw_line_func(VGAState *s1, uint8_t *d, const uint8_t *s, int width); -static inline unsigned int rgb_to_pixel8(unsigned int r, unsigned int g, unsigned b) -{ - return ((r >> 5) << 5) | ((g >> 5) << 2) | (b >> 6); -} - -static inline unsigned int rgb_to_pixel15(unsigned int r, unsigned int g, unsigned b) -{ - return ((r >> 3) << 10) | ((g >> 3) << 5) | (b >> 3); -} - -static inline unsigned int rgb_to_pixel16(unsigned int r, unsigned int g, unsigned b) -{ - return ((r >> 3) << 11) | ((g >> 2) << 5) | (b >> 3); -} - -static inline unsigned int rgb_to_pixel32(unsigned int r, unsigned int g, unsigned b) -{ - return (r << 16) | (g << 8) | b; -} - -static inline unsigned int rgb_to_pixel32bgr(unsigned int r, unsigned int g, unsigned b) -{ - return (b << 16) | (g << 8) | r; -} - #define DEPTH 8 #include "vga_template.h" Index: qemu/Makefile.target =================================================================== --- qemu.orig/Makefile.target 2007-06-07 19:47:46.000000000 +0000 +++ qemu/Makefile.target 2007-06-07 19:50:35.000000000 +0000 @@ -592,6 +592,10 @@ signal.o: signal.c $(CC) $(HELPER_CFLAGS) $(CPPFLAGS) $(BASE_CFLAGS) -c -o $@ $< +vga.o: pixel_ops.h + +tcx.o: pixel_ops.h + ifeq ($(TARGET_BASE_ARCH), i386) op.o: op.c opreg_template.h ops_template.h ops_template_mem.h ops_mem.h ops_sse.h endif