Re: [Qemu-devel] [PATCH 0 of 7] DisplayState interface change

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

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

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

[-- Attachment #2: cocoa.patch --]
[-- Type: text/x-diff, Size: 2945 bytes --]


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