Re: [Qemu-devel] gtk UI doesn't correctly byte swap 32-bit framebuffer on qemu-system-ppc little-endian host

2013-06-04 Thread Gerd Hoffmann
On 06/04/13 01:19, Anthony Liguori wrote:
 Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk writes:
 
 Hi all,

 I've just been testing some more OpenBIOS images with the new gtk UI and 
 found that if you specify a 32-bit depth framebuffer on qemu-system-ppc 
 running on a little-endian host then the RGB - BGR byteswap doesn't 
 take place.

 Good:
 ./qemu-system-ppc -g 1024x768x32 -vnc :1
 ./qemu-system-ppc -g 1024x768x32 -sdl

 Bad:
 ./qemu-system-ppc -g 1024x768x32
 
 cairo has a pretty limited number of modes that it supports.
 
 I guess we could use pixman to do the conversion.  Gerd, is that the
 right approach?

Hmm, looks like we can't pass pixman format values to cairo, even though
cairo uses pixman under the hood.  So, yes, we must convert and using
pixman is the easiest way to get the job done.

 Is there a way I can force the display API to do the
 conversion for me?

No, but asking pixman to do it is easy, see attached patch.

cheers,
  Gerd


From 488a5fa74c6112364885c4ab4c369cb1ed096c64 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann kra...@redhat.com
Date: Tue, 4 Jun 2013 09:36:52 +0200
Subject: [PATCH] gtk: add support for surface conversion

Also use CAIRO_FORMAT_RGB24 unconditionally.  DisplaySurfaces will never
ever see 8bpp surfaces.  And using CAIRO_FORMAT_RGB16_565 for the 16bpp
case doesn't seem to be a good idea too.

quote src=/usr/include/cairo/cairo.h
 * @CAIRO_FORMAT_RGB16_565: This format value is deprecated. It has
 *   never been properly implemented in cairo and should not be used
 *   by applications. (since 1.2)
/quote

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 ui/gtk.c |   63 +++---
 1 file changed, 40 insertions(+), 23 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 52c3f95..40adb05 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -147,6 +147,7 @@ typedef struct GtkDisplayState
 GtkWidget *notebook;
 GtkWidget *drawing_area;
 cairo_surface_t *surface;
+pixman_image_t *convert;
 DisplayChangeListener dcl;
 DisplaySurface *ds;
 int button_mask;
@@ -303,6 +304,11 @@ static void gd_update(DisplayChangeListener *dcl,
 
 DPRINTF(update(x=%d, y=%d, w=%d, h=%d)\n, x, y, w, h);
 
+if (s-convert) {
+pixman_image_composite(PIXMAN_OP_SRC, s-ds-image, NULL, s-convert,
+   x, y, 0, 0, x, y, w, h);
+}
+
 x1 = floor(x * s-scale_x);
 y1 = floor(y * s-scale_y);
 
@@ -384,9 +390,7 @@ static void gd_switch(DisplayChangeListener *dcl,
   DisplaySurface *surface)
 {
 GtkDisplayState *s = container_of(dcl, GtkDisplayState, dcl);
-cairo_format_t kind;
 bool resized = true;
-int stride;
 
 DPRINTF(resize(width=%d, height=%d)\n,
 surface_width(surface), surface_height(surface));
@@ -401,29 +405,42 @@ static void gd_switch(DisplayChangeListener *dcl,
 resized = false;
 }
 s-ds = surface;
-switch (surface_bits_per_pixel(surface)) {
-case 8:
-kind = CAIRO_FORMAT_A8;
-break;
-case 16:
-kind = CAIRO_FORMAT_RGB16_565;
-break;
-case 32:
-kind = CAIRO_FORMAT_RGB24;
-break;
-default:
-g_assert_not_reached();
-break;
-}
 
-stride = cairo_format_stride_for_width(kind, surface_width(surface));
-g_assert(surface_stride(surface) == stride);
+if (s-convert) {
+pixman_image_unref(s-convert);
+s-convert = NULL;
+}
 
-s-surface = cairo_image_surface_create_for_data(surface_data(surface),
- kind,
- surface_width(surface),
- surface_height(surface),
- surface_stride(surface));
+if (surface-format == PIXMAN_x8r8g8b8) {
+/*
+ * PIXMAN_x8r8g8b8 == CAIRO_FORMAT_RGB24
+ *
+ * No need to convert, use surface directly.  Should be the
+ * common case as this is qemu_default_pixelformat(32) too.
+ */
+s-surface = cairo_image_surface_create_for_data
+(surface_data(surface),
+ CAIRO_FORMAT_RGB24,
+ surface_width(surface),
+ surface_height(surface),
+ surface_stride(surface));
+} else {
+/* Must convert surface, use pixman to do it. */
+s-convert = pixman_image_create_bits(PIXMAN_x8r8g8b8,
+  surface_width(surface),
+  surface_height(surface),
+  NULL, 0);
+s-surface = cairo_image_surface_create_for_data
+((void *)pixman_image_get_data(s-convert),
+ CAIRO_FORMAT_RGB24,
+ pixman_image_get_width(s-convert),
+ pixman_image_get_height(s-convert),
+ 

Re: [Qemu-devel] gtk UI doesn't correctly byte swap 32-bit framebuffer on qemu-system-ppc little-endian host

2013-06-04 Thread Mark Cave-Ayland

On 04/06/13 08:50, Gerd Hoffmann wrote:


On 06/04/13 01:19, Anthony Liguori wrote:

Mark Cave-Aylandmark.cave-ayl...@ilande.co.uk  writes:


Hi all,

I've just been testing some more OpenBIOS images with the new gtk UI and
found that if you specify a 32-bit depth framebuffer on qemu-system-ppc
running on a little-endian host then the RGB -  BGR byteswap doesn't
take place.

Good:
./qemu-system-ppc -g 1024x768x32 -vnc :1
./qemu-system-ppc -g 1024x768x32 -sdl

Bad:
./qemu-system-ppc -g 1024x768x32


cairo has a pretty limited number of modes that it supports.

I guess we could use pixman to do the conversion.  Gerd, is that the
right approach?


Hmm, looks like we can't pass pixman format values to cairo, even though
cairo uses pixman under the hood.  So, yes, we must convert and using
pixman is the easiest way to get the job done.


Is there a way I can force the display API to do the
conversion for me?


No, but asking pixman to do it is easy, see attached patch.


Hi Gerd,

A quick test of the patch shows that it solves the problem here - thanks 
a lot!



ATB,

Mark.



[Qemu-devel] gtk UI doesn't correctly byte swap 32-bit framebuffer on qemu-system-ppc little-endian host

2013-06-03 Thread Mark Cave-Ayland

Hi all,

I've just been testing some more OpenBIOS images with the new gtk UI and 
found that if you specify a 32-bit depth framebuffer on qemu-system-ppc 
running on a little-endian host then the RGB - BGR byteswap doesn't 
take place.


Good:
./qemu-system-ppc -g 1024x768x32 -vnc :1
./qemu-system-ppc -g 1024x768x32 -sdl

Bad:
./qemu-system-ppc -g 1024x768x32

I've quickly confirmed this with git bisect which simply returns:

commit 15546425c5527ebb08ede399373b705866f1ff84
Author: Anthony Liguori aligu...@us.ibm.com
Date:   Wed Feb 20 07:43:25 2013 -0600

gtk: make default UI (v5)


Many thanks,

Mark.



Re: [Qemu-devel] gtk UI doesn't correctly byte swap 32-bit framebuffer on qemu-system-ppc little-endian host

2013-06-03 Thread Anthony Liguori
Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk writes:

 Hi all,

 I've just been testing some more OpenBIOS images with the new gtk UI and 
 found that if you specify a 32-bit depth framebuffer on qemu-system-ppc 
 running on a little-endian host then the RGB - BGR byteswap doesn't 
 take place.

 Good:
 ./qemu-system-ppc -g 1024x768x32 -vnc :1
 ./qemu-system-ppc -g 1024x768x32 -sdl

 Bad:
 ./qemu-system-ppc -g 1024x768x32

cairo has a pretty limited number of modes that it supports.

I guess we could use pixman to do the conversion.  Gerd, is that the
right approach?  Is there a way I can force the display API to do the
conversion for me?

Regards,

Anthony Liguori


 I've quickly confirmed this with git bisect which simply returns:

 commit 15546425c5527ebb08ede399373b705866f1ff84
 Author: Anthony Liguori aligu...@us.ibm.com
 Date:   Wed Feb 20 07:43:25 2013 -0600

  gtk: make default UI (v5)


 Many thanks,

 Mark.