From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LDdTb-0007G4-P6 for qemu-devel@nongnu.org; Fri, 19 Dec 2008 06:25:03 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LDdTW-0007FT-Lb for qemu-devel@nongnu.org; Fri, 19 Dec 2008 06:25:02 -0500 Received: from [199.232.76.173] (port=48385 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LDdTW-0007FQ-G1 for qemu-devel@nongnu.org; Fri, 19 Dec 2008 06:24:58 -0500 Received: from smtp.eu.citrix.com ([62.200.22.115]:32098) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LDdTV-0003Er-UM for qemu-devel@nongnu.org; Fri, 19 Dec 2008 06:24:58 -0500 Message-ID: <494B848D.9030802@eu.citrix.com> Date: Fri, 19 Dec 2008 11:25:01 +0000 From: Stefano Stabellini MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 0 of 7] DisplayState interface change References: <494AA0B2.8070702@eu.citrix.com> In-Reply-To: Content-Type: multipart/mixed; boundary="------------070307030706000509030409" Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------070307030706000509030409 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Andreas Färber wrote: >> 2) remove bgr >> the new DisplayState interface does not contain any host specific >> display detail, it is an abstraction of the backend display, hence we >> don't need to memorize the bgr flag in DisplayState. >> The frontend must be able to handle a bgr display by itself, in fact sdl >> is perfectly capable of that; > > Sorry if I've asked that before, but doesn't this (and 3) require > changes to non-SDL backends as well, especially Cocoa? > The MacBook display appeared to be bgr so that TCX had to be adapted and > bgr forcedly set to 1 in cocoa.m (r2974). > I forgot about cocoa, and yes, some changes are required. I am afraid I don't know much about it and I don't have any MacOS so I may not be the right person do it. In any case I wrote a simple patch that probably won't even compile but will give you an idea about what has to be done. I am both attaching the patch to this email and commenting it inline so that I can better explain the reason behind each change. >-static void cocoa_resize(DisplayState *ds, int w, int h) >+static void cocoa_resize(DisplayState *ds) > { > COCOA_DEBUG("qemu_cocoa: cocoa_resize\n"); > >- [cocoaView resizeContentToWidth:w height:h displayState:ds]; >+ [cocoaView resizeContentToWidth:ds_get_width(ds) height:ds_get_height(ds) displayState:ds]; > } > > static void cocoa_refresh(DisplayState *ds) > > The new resize callback interface signature is changed: now the only argument is DisplayState, because is the backend that is supposed to set the the properties in the DisplayState structure. >- screenBuffer = malloc( w * 4 * h ); > >- ds->data = screenBuffer; >- ds->linesize = (w * 4); >- ds->depth = 32; >- ds->width = w; >- ds->height = h; >-#ifdef __LITTLE_ENDIAN__ >- ds->bgr = 1; >-#else >- ds->bgr = 0; >-#endif >- >- dataProviderRef = CGDataProviderCreateWithData(NULL, screenBuffer, w * 4 * h, NULL); >+ dataProviderRef = CGDataProviderCreateWithData(NULL, ds->data, ds_get_linesize(ds) * ds_get_height(ds), NULL); > Again we don't have to set any property in ds anymore, the backend does that for us. data is already allocated and width, height, linesize already set. > int gArgc; > char **gArgv; >@@ -316,18 +317,13 @@ > // draw screen bitmap directly to Core Graphics context > if (dataProviderRef) { > CGImageRef imageRef = CGImageCreate( >- screen.width, //width >- screen.height, //height >- screen.bitsPerComponent, //bitsPerComponent >- screen.bitsPerPixel, //bitsPerPixel >- (screen.width * 4), //bytesPerRow >-#if __LITTLE_ENDIAN__ >- CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4 >- kCGImageAlphaNoneSkipLast, >-#else >+ ds_get_width(ds), //width >+ ds_get_height(ds), //height >+ 8, //bitsPerComponent >+ ds_get_bits_per_pixel(ds), //bitsPerPixel >+ ds_get_linesize(ds), //bytesPerRow > CGColorSpaceCreateDeviceRGB(), //colorspace for OS X < 10.4 (actually ppc) > kCGImageAlphaNoneSkipFirst, //bitmapInfo >-#endif > dataProviderRef, //provider > NULL, //decode > 0, //interpolate >@@ -397,20 +393,8 @@ > CGDataProviderRelease(dataProviderRef); > if (screenBuffer) > free(screenBuffer); This is the critical part where we ask cocoa to do any needed conversions between the source image format given by the backend and the real display format. In order to do this we call CGImageCreate with the image source properties from DisplayState. >>From a little googling it seems to me that CGImageCreate and CGContextDrawImage are able to handle the conversions. >@@ -983,12 +967,13 @@ > COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n"); > > // register vga outpu callbacks >- ds->dpy_update = cocoa_update; >- ds->dpy_resize = cocoa_resize; >- ds->dpy_refresh = cocoa_refresh; >- >- // give window a initial Size >- cocoa_resize(ds, 640, 400); >+ dcl = qemu_mallocz(sizeof(DisplayChangeListener)); >+ if (!dcl) >+ exit(1); >+ dcl->dpy_update = cocoa_update; >+ dcl->dpy_resize = cocoa_resize; >+ dcl->dpy_refresh = cocoa_refresh; >+ register_displaychangelistener(ds, dcl); > > // register cleanup function > atexit(cocoa_cleanup); > These changes are needed to support the new register_displaychangelistener interface, they shouldn't need any explanation. It could also be useful to look at the changes I made to vnc.c and sdl.c in the third patch. Please ask if you need anything else. Cheers, Stefano Stabellini --------------070307030706000509030409 Content-Type: text/x-diff; name="cocoa.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="cocoa.patch" diff -r 51cf78066aca cocoa.m --- a/cocoa.m Fri Dec 19 10:38:50 2008 +0000 +++ b/cocoa.m Fri Dec 19 11:22:53 2008 +0000 @@ -58,6 +58,7 @@ NSWindow *normalWindow; id cocoaView; static void *screenBuffer; +static DisplayChangeListener *dcl; int gArgc; char **gArgv; @@ -316,18 +317,13 @@ // draw screen bitmap directly to Core Graphics context if (dataProviderRef) { CGImageRef imageRef = CGImageCreate( - screen.width, //width - screen.height, //height - screen.bitsPerComponent, //bitsPerComponent - screen.bitsPerPixel, //bitsPerPixel - (screen.width * 4), //bytesPerRow -#if __LITTLE_ENDIAN__ - CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4 - kCGImageAlphaNoneSkipLast, -#else + ds_get_width(ds), //width + ds_get_height(ds), //height + 8, //bitsPerComponent + ds_get_bits_per_pixel(ds), //bitsPerPixel + ds_get_linesize(ds), //bytesPerRow CGColorSpaceCreateDeviceRGB(), //colorspace for OS X < 10.4 (actually ppc) kCGImageAlphaNoneSkipFirst, //bitmapInfo -#endif dataProviderRef, //provider NULL, //decode 0, //interpolate @@ -397,20 +393,8 @@ CGDataProviderRelease(dataProviderRef); if (screenBuffer) free(screenBuffer); - screenBuffer = malloc( w * 4 * h ); - ds->data = screenBuffer; - ds->linesize = (w * 4); - ds->depth = 32; - ds->width = w; - ds->height = h; -#ifdef __LITTLE_ENDIAN__ - ds->bgr = 1; -#else - ds->bgr = 0; -#endif - - dataProviderRef = CGDataProviderCreateWithData(NULL, screenBuffer, w * 4 * h, NULL); + dataProviderRef = CGDataProviderCreateWithData(NULL, ds->data, ds_get_linesize(ds) * ds_get_height(ds), NULL); // update windows if (isFullscreen) { @@ -939,11 +923,11 @@ [cocoaView displayRect:rect]; } -static void cocoa_resize(DisplayState *ds, int w, int h) +static void cocoa_resize(DisplayState *ds) { COCOA_DEBUG("qemu_cocoa: cocoa_resize\n"); - [cocoaView resizeContentToWidth:w height:h displayState:ds]; + [cocoaView resizeContentToWidth:ds_get_width(ds) height:ds_get_height(ds) displayState:ds]; } static void cocoa_refresh(DisplayState *ds) @@ -983,12 +967,13 @@ COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n"); // register vga outpu callbacks - ds->dpy_update = cocoa_update; - ds->dpy_resize = cocoa_resize; - ds->dpy_refresh = cocoa_refresh; - - // give window a initial Size - cocoa_resize(ds, 640, 400); + dcl = qemu_mallocz(sizeof(DisplayChangeListener)); + if (!dcl) + exit(1); + dcl->dpy_update = cocoa_update; + dcl->dpy_resize = cocoa_resize; + dcl->dpy_refresh = cocoa_refresh; + register_displaychangelistener(ds, dcl); // register cleanup function atexit(cocoa_cleanup); --------------070307030706000509030409--