From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48609) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ujmb3-0007hk-Al for qemu-devel@nongnu.org; Tue, 04 Jun 2013 04:28:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ujmay-0006R5-7j for qemu-devel@nongnu.org; Tue, 04 Jun 2013 04:28:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:62113) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ujmax-0006R0-VV for qemu-devel@nongnu.org; Tue, 04 Jun 2013 04:27:56 -0400 Message-ID: <51AD9C35.4060001@redhat.com> Date: Tue, 04 Jun 2013 09:50:13 +0200 From: Gerd Hoffmann MIME-Version: 1.0 References: <51AD1A4D.1090301@ilande.co.uk> <878v2q9405.fsf@codemonkey.ws> In-Reply-To: <878v2q9405.fsf@codemonkey.ws> Content-Type: multipart/mixed; boundary="------------030702070608080709090306" Subject: Re: [Qemu-devel] gtk UI doesn't correctly byte swap 32-bit framebuffer on qemu-system-ppc little-endian host List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Mark Cave-Ayland , qemu-devel This is a multi-part message in MIME format. --------------030702070608080709090306 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 06/04/13 01:19, Anthony Liguori wrote: > Mark Cave-Ayland 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 --------------030702070608080709090306 Content-Type: text/plain; charset=UTF-8; name="0001-gtk-add-support-for-surface-conversion.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-gtk-add-support-for-surface-conversion.patch" >>From 488a5fa74c6112364885c4ab4c369cb1ed096c64 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann 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. * @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) Signed-off-by: Gerd Hoffmann --- 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), + pixman_image_get_stride(s->convert)); + pixman_image_composite(PIXMAN_OP_SRC, s->ds->image, NULL, s->convert, + 0, 0, 0, 0, 0, 0, + pixman_image_get_width(s->convert), + pixman_image_get_height(s->convert)); + } if (resized) { gd_update_windowsize(s); -- 1.7.9.7 --------------030702070608080709090306--