[Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Samuel Benson]

Version 2 does as follows:

[1]: Corrects endianness on issues by using native BGR to RGB conversion
[2]: Uses DisplayState accessors for obtaining graphics context  
information, which
[3]: Removes now unused variables, and
[4]: Allows reading of varying color modes (32bit/24/16), and  
converting to native colorspace
[5]: Attempts to keep itself centered on screen (as opposed to bottom  
right, which immediately
      goes off screen after bios load) on context changes (window  
resizes)

Testing working on i386 (gentoo, Windows 2000) and PPC (debian) guests  
on PPC and x86 Macs.

In regards to [4], Windows 2000 displays fine on quick tests, but on  
the lowest setting
I could test, 16bit color depth at 4bpp, colors are slightly off. I  
used gentoo
install-x86-minimal-2008.0 in framebuffer mode to test above setting;  
the usual grey text is
now blue, and Tux appears to be BGR shifted. I do not know if previous  
code worked at such
a low color setting.

Signed-off-by: Samuel Benson <qemu_ml@digitalescape.info>

---

  cocoa.m |   53 ++++++++++++++++++++---------------------------------
  1 files changed, 20 insertions(+), 33 deletions(-)

diff --git a/cocoa.m b/cocoa.m
index fe13952..55ff2b4 100644
--- a/cocoa.m
+++ b/cocoa.m
@@ -57,7 +57,7 @@ typedef struct {
  int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
  NSWindow *normalWindow;
  id cocoaView;
-static void *screenBuffer;
+static DisplayChangeListener *dcl;

  int gArgc;
  char **gArgv;
@@ -292,9 +292,6 @@ int cocoa_keycode_to_qemu(int keycode)
  {
      COCOA_DEBUG("QemuCocoaView: dealloc\n");

-    if (screenBuffer)
-        free(screenBuffer);
-
      if (dataProviderRef)
          CGDataProviderRelease(dataProviderRef);

@@ -305,9 +302,6 @@ int cocoa_keycode_to_qemu(int keycode)
  {
      COCOA_DEBUG("QemuCocoaView: drawRect\n");

-    if ((int)screenBuffer == -1)
-        return;
-
      // get CoreGraphic context
      CGContextRef viewContextRef = [[NSGraphicsContext  
currentContext] graphicsPort];
      CGContextSetInterpolationQuality (viewContextRef,  
kCGInterpolationNone);
@@ -320,10 +314,10 @@ int cocoa_keycode_to_qemu(int keycode)
              screen.height, //height
              screen.bitsPerComponent, //bitsPerComponent
              screen.bitsPerPixel, //bitsPerPixel
-            (screen.width * 4), //bytesPerRow
+            (screen.width * (screen.bitsPerComponent/2)), //bytesPerRow
  #if __LITTLE_ENDIAN__
              CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), // 
colorspace for OS X >= 10.4
-            kCGImageAlphaNoneSkipLast,
+            kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
  #else
              CGColorSpaceCreateDeviceRGB(), //colorspace for OS X <  
10.4 (actually ppc)
              kCGImageAlphaNoneSkipFirst, //bitmapInfo
@@ -395,22 +389,12 @@ int cocoa_keycode_to_qemu(int keycode)
      // update screenBuffer
      if (dataProviderRef)
          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);
+    //sync host window color space with guests
+	screen.bitsPerPixel = ds_get_bits_per_pixel(ds);
+	screen.bitsPerComponent = ds_get_bytes_per_pixel(ds) * 2;
+
+    dataProviderRef = CGDataProviderCreateWithData(NULL,  
ds_get_data(ds), w * 4 * h, NULL);

      // update windows
      if (isFullscreen) {
@@ -423,6 +407,7 @@ int cocoa_keycode_to_qemu(int keycode)
      }
      screen.width = w;
      screen.height = h;
+	[normalWindow center];
      [self setContentDimensions];
      [self setFrame:NSMakeRect(cx, cy, cw, ch)];
  }
@@ -740,6 +725,7 @@ int cocoa_keycode_to_qemu(int keycode)
          [normalWindow setTitle:[NSString stringWithFormat:@"QEMU"]];
          [normalWindow setContentView:cocoaView];
          [normalWindow makeKeyAndOrderFront:self];
+		[normalWindow center];

      }
      return self;
@@ -939,11 +925,11 @@ static void cocoa_update(DisplayState *ds, int  
x, int y, int w, int h)
      [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:(int)(ds_get_width(ds)) height: 
(int)(ds_get_height(ds)) displayState:ds];
  }

  static void cocoa_refresh(DisplayState *ds)
@@ -975,20 +961,21 @@ static void cocoa_refresh(DisplayState *ds)
  static void cocoa_cleanup(void)
  {
      COCOA_DEBUG("qemu_cocoa: cocoa_cleanup\n");
-
+	qemu_free(dcl);
  }

  void cocoa_display_init(DisplayState *ds, int full_screen)
  {
      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;
+	dcl = qemu_mallocz(sizeof(DisplayChangeListener));
+	
+    // register vga output callbacks
+    dcl->dpy_update = cocoa_update;
+    dcl->dpy_resize = cocoa_resize;
+    dcl->dpy_refresh = cocoa_refresh;

-    // give window a initial Size
-    cocoa_resize(ds, 640, 400);
+	register_displaychangelistener(ds, dcl);

      // register cleanup function
      atexit(cocoa_cleanup);

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[C.W. Betts]

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

It would help if the patch file was included.  Either the mail browser  
that I use (Mac OS X Mail) or the mail transport messes up the  
formatting, making it hard to apply the patch using the patch command.
On Jan 25, 2009, at 1:23 AM, Samuel Benson wrote:

> diff --git a/cocoa.m b/cocoa.m
> index fe13952..55ff2b4 100644
> --- a/cocoa.m
> +++ b/cocoa.m
> @@ -57,7 +57,7 @@ typedef struct {
> int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
> NSWindow *normalWindow;
> id cocoaView;
> -static void *screenBuffer;
> +static DisplayChangeListener *dcl;
>
> int gArgc;
> char **gArgv;
> @@ -292,9 +292,6 @@ int cocoa_keycode_to_qemu(int keycode)
> {
>     COCOA_DEBUG("QemuCocoaView: dealloc\n");
>
> -    if (screenBuffer)
> -        free(screenBuffer);
> -
>     if (dataProviderRef)
>         CGDataProviderRelease(dataProviderRef);
>
> @@ -305,9 +302,6 @@ int cocoa_keycode_to_qemu(int keycode)
> {
>     COCOA_DEBUG("QemuCocoaView: drawRect\n");
>
> -    if ((int)screenBuffer == -1)
> -        return;
> -
>     // get CoreGraphic context
>     CGContextRef viewContextRef = [[NSGraphicsContext  
> currentContext] graphicsPort];
>     CGContextSetInterpolationQuality (viewContextRef,  
> kCGInterpolationNone);
> @@ -320,10 +314,10 @@ int cocoa_keycode_to_qemu(int keycode)
>             screen.height, //height
>             screen.bitsPerComponent, //bitsPerComponent
>             screen.bitsPerPixel, //bitsPerPixel
> -            (screen.width * 4), //bytesPerRow
> +            (screen.width * (screen.bitsPerComponent/2)), // 
> bytesPerRow
> #if __LITTLE_ENDIAN__
>             CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), // 
> colorspace for OS X >= 10.4
> -            kCGImageAlphaNoneSkipLast,
> +            kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
> #else
>             CGColorSpaceCreateDeviceRGB(), //colorspace for OS X <  
> 10.4 (actually ppc)
>             kCGImageAlphaNoneSkipFirst, //bitmapInfo
> @@ -395,22 +389,12 @@ int cocoa_keycode_to_qemu(int keycode)
>     // update screenBuffer
>     if (dataProviderRef)
>         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);
> +    //sync host window color space with guests
> +	screen.bitsPerPixel = ds_get_bits_per_pixel(ds);
> +	screen.bitsPerComponent = ds_get_bytes_per_pixel(ds) * 2;
> +
> +    dataProviderRef = CGDataProviderCreateWithData(NULL,  
> ds_get_data(ds), w * 4 * h, NULL);
>
>     // update windows
>     if (isFullscreen) {
> @@ -423,6 +407,7 @@ int cocoa_keycode_to_qemu(int keycode)
>     }
>     screen.width = w;
>     screen.height = h;
> +	[normalWindow center];
>     [self setContentDimensions];
>     [self setFrame:NSMakeRect(cx, cy, cw, ch)];
> }
> @@ -740,6 +725,7 @@ int cocoa_keycode_to_qemu(int keycode)
>         [normalWindow setTitle:[NSString stringWithFormat:@"QEMU"]];
>         [normalWindow setContentView:cocoaView];
>         [normalWindow makeKeyAndOrderFront:self];
> +		[normalWindow center];
>
>     }
>     return self;
> @@ -939,11 +925,11 @@ static void cocoa_update(DisplayState *ds, int  
> x, int y, int w, int h)
>     [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:(int)(ds_get_width(ds)) height: 
> (int)(ds_get_height(ds)) displayState:ds];
> }
>
> static void cocoa_refresh(DisplayState *ds)
> @@ -975,20 +961,21 @@ static void cocoa_refresh(DisplayState *ds)
> static void cocoa_cleanup(void)
> {
>     COCOA_DEBUG("qemu_cocoa: cocoa_cleanup\n");
> -
> +	qemu_free(dcl);
> }
>
> void cocoa_display_init(DisplayState *ds, int full_screen)
> {
>     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;
> +	dcl = qemu_mallocz(sizeof(DisplayChangeListener));
> +	
> +    // register vga output callbacks
> +    dcl->dpy_update = cocoa_update;
> +    dcl->dpy_resize = cocoa_resize;
> +    dcl->dpy_refresh = cocoa_refresh;
>
> -    // give window a initial Size
> -    cocoa_resize(ds, 640, 400);
> +	register_displaychangelistener(ds, dcl);
>
>     // register cleanup function
>     atexit(cocoa_cleanup);
>


[-- Attachment #2: Type: text/html, Size: 8062 bytes --]

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Samuel Benson]

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

Here it is non-inlined.

	—Sam

[-- Attachment #2: cocoa-display.patch --]
[-- Type: application/octet-stream, Size: 4330 bytes --]

diff --git a/cocoa.m b/cocoa.m
index fe13952..55ff2b4 100644
--- a/cocoa.m
+++ b/cocoa.m
@@ -57,7 +57,7 @@ typedef struct {
 int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
 NSWindow *normalWindow;
 id cocoaView;
-static void *screenBuffer;
+static DisplayChangeListener *dcl;
 
 int gArgc;
 char **gArgv;
@@ -292,9 +292,6 @@ int cocoa_keycode_to_qemu(int keycode)
 {
     COCOA_DEBUG("QemuCocoaView: dealloc\n");
 
-    if (screenBuffer)
-        free(screenBuffer);
-
     if (dataProviderRef)
         CGDataProviderRelease(dataProviderRef);
 
@@ -305,9 +302,6 @@ int cocoa_keycode_to_qemu(int keycode)
 {
     COCOA_DEBUG("QemuCocoaView: drawRect\n");
 
-    if ((int)screenBuffer == -1)
-        return;
-
     // get CoreGraphic context
     CGContextRef viewContextRef = [[NSGraphicsContext currentContext] graphicsPort];
     CGContextSetInterpolationQuality (viewContextRef, kCGInterpolationNone);
@@ -320,10 +314,10 @@ int cocoa_keycode_to_qemu(int keycode)
             screen.height, //height
             screen.bitsPerComponent, //bitsPerComponent
             screen.bitsPerPixel, //bitsPerPixel
-            (screen.width * 4), //bytesPerRow
+            (screen.width * (screen.bitsPerComponent/2)), //bytesPerRow
 #if __LITTLE_ENDIAN__
             CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4
-            kCGImageAlphaNoneSkipLast,
+            kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
 #else
             CGColorSpaceCreateDeviceRGB(), //colorspace for OS X < 10.4 (actually ppc)
             kCGImageAlphaNoneSkipFirst, //bitmapInfo
@@ -395,22 +389,12 @@ int cocoa_keycode_to_qemu(int keycode)
     // update screenBuffer
     if (dataProviderRef)
         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);
+    //sync host window color space with guests
+	screen.bitsPerPixel = ds_get_bits_per_pixel(ds);
+	screen.bitsPerComponent = ds_get_bytes_per_pixel(ds) * 2;
+
+    dataProviderRef = CGDataProviderCreateWithData(NULL, ds_get_data(ds), w * 4 * h, NULL);
 
     // update windows
     if (isFullscreen) {
@@ -423,6 +407,7 @@ int cocoa_keycode_to_qemu(int keycode)
     }
     screen.width = w;
     screen.height = h;
+	[normalWindow center];
     [self setContentDimensions];
     [self setFrame:NSMakeRect(cx, cy, cw, ch)];
 }
@@ -740,6 +725,7 @@ int cocoa_keycode_to_qemu(int keycode)
         [normalWindow setTitle:[NSString stringWithFormat:@"QEMU"]];
         [normalWindow setContentView:cocoaView];
         [normalWindow makeKeyAndOrderFront:self];
+		[normalWindow center];
 
     }
     return self;
@@ -939,11 +925,11 @@ static void cocoa_update(DisplayState *ds, int x, int y, int w, int h)
     [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:(int)(ds_get_width(ds)) height:(int)(ds_get_height(ds)) displayState:ds];
 }
 
 static void cocoa_refresh(DisplayState *ds)
@@ -975,20 +961,21 @@ static void cocoa_refresh(DisplayState *ds)
 static void cocoa_cleanup(void)
 {
     COCOA_DEBUG("qemu_cocoa: cocoa_cleanup\n");
-
+	qemu_free(dcl);
 }
 
 void cocoa_display_init(DisplayState *ds, int full_screen)
 {
     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;
+	dcl = qemu_mallocz(sizeof(DisplayChangeListener));
+	
+    // register vga output callbacks
+    dcl->dpy_update = cocoa_update;
+    dcl->dpy_resize = cocoa_resize;
+    dcl->dpy_refresh = cocoa_refresh;
 
-    // give window a initial Size
-    cocoa_resize(ds, 640, 400);
+	register_displaychangelistener(ds, dcl);
 
     // register cleanup function
     atexit(cocoa_cleanup);

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

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Anthony Liguori]

C.W. Betts wrote:
> It would help if the patch file was included.  Either the mail browser 
> that I use (Mac OS X Mail) or the mail transport messes up the 
> formatting, making it hard to apply the patch using the patch command.

Inlining is fine (and preferred to me) but Samuel's mail client munged 
the white space in the patch.

Regards,

Anthony Liguori

> On Jan 25, 2009, at 1:23 AM, Samuel Benson wrote:
>
>> diff --git a/cocoa.m b/cocoa.m
>> index fe13952..55ff2b4 100644
>> --- a/cocoa.m
>> +++ b/cocoa.m
>> @@ -57,7 +57,7 @@ typedef struct {
>> int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
>> NSWindow *normalWindow;
>> id cocoaView;
>> -static void *screenBuffer;
>> +static DisplayChangeListener *dcl;
>>
>> int gArgc;
>> char **gArgv;
>> @@ -292,9 +292,6 @@ int cocoa_keycode_to_qemu(int keycode)
>> {
>>     COCOA_DEBUG("QemuCocoaView: dealloc\n");
>>
>> -    if (screenBuffer)
>> -        free(screenBuffer);
>> -
>>     if (dataProviderRef)
>>         CGDataProviderRelease(dataProviderRef);
>>
>> @@ -305,9 +302,6 @@ int cocoa_keycode_to_qemu(int keycode)
>> {
>>     COCOA_DEBUG("QemuCocoaView: drawRect\n");
>>
>> -    if ((int)screenBuffer == -1)
>> -        return;
>> -
>>     // get CoreGraphic context
>>     CGContextRef viewContextRef = [[NSGraphicsContext currentContext] 
>> graphicsPort];
>>     CGContextSetInterpolationQuality (viewContextRef, 
>> kCGInterpolationNone);
>> @@ -320,10 +314,10 @@ int cocoa_keycode_to_qemu(int keycode)
>>             screen.height, //height
>>             screen.bitsPerComponent, //bitsPerComponent
>>             screen.bitsPerPixel, //bitsPerPixel
>> -            (screen.width * 4), //bytesPerRow
>> +            (screen.width * (screen.bitsPerComponent/2)), //bytesPerRow
>> #if __LITTLE_ENDIAN__
>>             CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), 
>> //colorspace for OS X >= 10.4
>> -            kCGImageAlphaNoneSkipLast,
>> +            kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
>> #else
>>             CGColorSpaceCreateDeviceRGB(), //colorspace for OS X < 
>> 10.4 (actually ppc)
>>             kCGImageAlphaNoneSkipFirst, //bitmapInfo
>> @@ -395,22 +389,12 @@ int cocoa_keycode_to_qemu(int keycode)
>>     // update screenBuffer
>>     if (dataProviderRef)
>>         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);
>> +    //sync host window color space with guests
>> + screen.bitsPerPixel = ds_get_bits_per_pixel(ds);
>> + screen.bitsPerComponent = ds_get_bytes_per_pixel(ds) * 2;
>> +
>> +    dataProviderRef = CGDataProviderCreateWithData(NULL, 
>> ds_get_data(ds), w * 4 * h, NULL);
>>
>>     // update windows
>>     if (isFullscreen) {
>> @@ -423,6 +407,7 @@ int cocoa_keycode_to_qemu(int keycode)
>>     }
>>     screen.width = w;
>>     screen.height = h;
>> + [normalWindow center];
>>     [self setContentDimensions];
>>     [self setFrame:NSMakeRect(cx, cy, cw, ch)];
>> }
>> @@ -740,6 +725,7 @@ int cocoa_keycode_to_qemu(int keycode)
>>         [normalWindow setTitle:[NSString stringWithFormat:@"QEMU"]];
>>         [normalWindow setContentView:cocoaView];
>>         [normalWindow makeKeyAndOrderFront:self];
>> + [normalWindow center];
>>
>>     }
>>     return self;
>> @@ -939,11 +925,11 @@ static void cocoa_update(DisplayState *ds, int 
>> x, int y, int w, int h)
>>     [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:(int)(ds_get_width(ds)) 
>> height:(int)(ds_get_height(ds)) displayState:ds];
>> }
>>
>> static void cocoa_refresh(DisplayState *ds)
>> @@ -975,20 +961,21 @@ static void cocoa_refresh(DisplayState *ds)
>> static void cocoa_cleanup(void)
>> {
>>     COCOA_DEBUG("qemu_cocoa: cocoa_cleanup\n");
>> -
>> + qemu_free(dcl);
>> }
>>
>> void cocoa_display_init(DisplayState *ds, int full_screen)
>> {
>>     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;
>> + dcl = qemu_mallocz(sizeof(DisplayChangeListener));
>> +
>> +    // register vga output callbacks
>> +    dcl->dpy_update = cocoa_update;
>> +    dcl->dpy_resize = cocoa_resize;
>> +    dcl->dpy_refresh = cocoa_refresh;
>>
>> -    // give window a initial Size
>> -    cocoa_resize(ds, 640, 400);
>> + register_displaychangelistener(ds, dcl);
>>
>>     // register cleanup function
>>     atexit(cocoa_cleanup);
>>
>

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Stefano Stabellini]

Samuel Benson wrote:

> Version 2 does as follows:
> 
> [1]: Corrects endianness on issues by using native BGR to RGB conversion
> [2]: Uses DisplayState accessors for obtaining graphics context
> information, which
> [3]: Removes now unused variables, and
> [4]: Allows reading of varying color modes (32bit/24/16), and converting
> to native colorspace
> [5]: Attempts to keep itself centered on screen (as opposed to bottom
> right, which immediately
>      goes off screen after bios load) on context changes (window resizes)
> 
> Testing working on i386 (gentoo, Windows 2000) and PPC (debian) guests
> on PPC and x86 Macs.
> 
> In regards to [4], Windows 2000 displays fine on quick tests, but on the
> lowest setting
> I could test, 16bit color depth at 4bpp, colors are slightly off. I used
> gentoo
> install-x86-minimal-2008.0 in framebuffer mode to test above setting;
> the usual grey text is
> now blue, and Tux appears to be BGR shifted. I do not know if previous
> code worked at such
> a low color setting.
> 
> Signed-off-by: Samuel Benson <qemu_ml@digitalescape.info>

This patch looks fine to me: it uses the new interface the way it is
supposed to be used.

However if you are seeing some color problems you may still have few
bugs in the cocoa color conversion code.
To help you track them down, I'll explain a little bit more the pixel
formats that the graphic card exposes to cocoa, no matter what the host
endianness is:

- 24 bits depth, 32 bits per pixel (8888)
- 16 bits depth, 16 bits per pixel (565)

usually the pixel format is RGB, look at
console.c:qemu_default_pixelformat to have the details.
There is just one exception: if the guest endianness differs from the
host endianness and the guest native color depth is 24 bits with 32 bits
per pixels, then these masks are used instead:
console.c:qemu_different_endianness_pixelformat.
You'll probably never see this latter case in your tests, so you can
just ignore it for the moment.

Did you do the blue shifted Tux test on a ppc Mac? If so my guess is
that you have to call kCGImageAlphaNoneSkipLast instead of
kCGImageAlphaNoneSkipFirst because the format exposed by the graphic
card is already converted into big endian.

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Stefano Stabellini]

Samuel Benson wrote:

> Version 2 does as follows:
> 
> [1]: Corrects endianness on issues by using native BGR to RGB conversion
> [2]: Uses DisplayState accessors for obtaining graphics context
> information, which
> [3]: Removes now unused variables, and
> [4]: Allows reading of varying color modes (32bit/24/16), and converting
> to native colorspace
> [5]: Attempts to keep itself centered on screen (as opposed to bottom
> right, which immediately
>      goes off screen after bios load) on context changes (window resizes)
> 
> Testing working on i386 (gentoo, Windows 2000) and PPC (debian) guests
> on PPC and x86 Macs.
> 
> In regards to [4], Windows 2000 displays fine on quick tests, but on the
> lowest setting
> I could test, 16bit color depth at 4bpp, colors are slightly off. I used
> gentoo
> install-x86-minimal-2008.0 in framebuffer mode to test above setting;
> the usual grey text is
> now blue, and Tux appears to be BGR shifted. I do not know if previous
> code worked at such
> a low color setting.
> 
> Signed-off-by: Samuel Benson <qemu_ml@digitalescape.info>



This patch looks fine to me: it uses the new interface the way it is
supposed to be used.

However if you are seeing some color problems you may still have few
bugs in the cocoa color conversion code.
To help you track them down, I'll explain a little bit more the pixel
formats that the graphic card exposes to cocoa, no matter what the host
endianness is:

- 24 bits depth, 32 bits per pixel (8888)
- 16 bits depth, 16 bits per pixel (565)

usually the pixel format is RGB, look at
console.c:qemu_default_pixelformat to have the details.
There is just one exception: if the guest endianness differs from the
host endianness and the guest native color depth is 24 bits with 32 bits
per pixels, then these masks are used instead:
console.c:qemu_different_endianness_pixelformat.
You'll probably never see this latter case in your tests, so you can
just ignore it for the moment.

Did you do the blue shifted Tux test on a ppc Mac? If so my guess is
that you have to call kCGImageAlphaNoneSkipLast instead of
kCGImageAlphaNoneSkipFirst because the format exposed by the graphic
card is already converted into big endian.

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Stefano Stabellini]

Samuel Benson wrote:

> Version 2 does as follows:
> 
> [1]: Corrects endianness on issues by using native BGR to RGB conversion
> [2]: Uses DisplayState accessors for obtaining graphics context
> information, which
> [3]: Removes now unused variables, and
> [4]: Allows reading of varying color modes (32bit/24/16), and converting
> to native colorspace
> [5]: Attempts to keep itself centered on screen (as opposed to bottom
> right, which immediately
>      goes off screen after bios load) on context changes (window resizes)
> 
> Testing working on i386 (gentoo, Windows 2000) and PPC (debian) guests
> on PPC and x86 Macs.
> 
> In regards to [4], Windows 2000 displays fine on quick tests, but on the
> lowest setting
> I could test, 16bit color depth at 4bpp, colors are slightly off. I used
> gentoo
> install-x86-minimal-2008.0 in framebuffer mode to test above setting;
> the usual grey text is
> now blue, and Tux appears to be BGR shifted. I do not know if previous
> code worked at such
> a low color setting.
> 
> Signed-off-by: Samuel Benson <qemu_ml@digitalescape.info>





This patch looks fine to me: it uses the new interface the way it is
supposed to be used.

However if you are seeing some color problems you may still have few
bugs in the cocoa color conversion code.
To help you track them down, I'll explain a little bit more the pixel
formats that the graphic card exposes to cocoa, no matter what the host
endianness is:

- 24 bits depth, 32 bits per pixel (8888)
- 16 bits depth, 16 bits per pixel (565)

usually the pixel format is RGB, look at
console.c:qemu_default_pixelformat to have the details.
There is just one exception: if the guest endianness differs from the
host endianness and the guest native color depth is 24 bits with 32 bits
per pixels, then these masks are used instead:
console.c:qemu_different_endianness_pixelformat.
You'll probably never see this latter case in your tests, so you can
just ignore it for the moment.

Did you do the blue shifted Tux test on a ppc Mac? If so my guess is
that you have to call kCGImageAlphaNoneSkipLast instead of
kCGImageAlphaNoneSkipFirst because the format exposed by the graphic
card is already converted into big endian.

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Samuel Benson]

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


On Jan 26, 2009, at 5:05 AM, Stefano Stabellini wrote:
>
> However if you are seeing some color problems you may still have few
> bugs in the cocoa color conversion code.
> To help you track them down, I'll explain a little bit more the pixel
> formats that the graphic card exposes to cocoa, no matter what the  
> host
> endianness is:
>
> - 24 bits depth, 32 bits per pixel (8888)
> - 16 bits depth, 16 bits per pixel (565)
>
> usually the pixel format is RGB, look at
> console.c:qemu_default_pixelformat to have the details.
> There is just one exception: if the guest endianness differs from the
> host endianness and the guest native color depth is 24 bits with 32  
> bits
> per pixels, then these masks are used instead:
> console.c:qemu_different_endianness_pixelformat.
> You'll probably never see this latter case in your tests, so you can
> just ignore it for the moment.

Yeah, I had to do some math to get it to correctly accept the change  
in pixel depth,
but what I have seems to work 80% of the time which I find very odd.

> Did you do the blue shifted Tux test on a ppc Mac? If so my guess is
> that you have to call kCGImageAlphaNoneSkipLast instead of
> kCGImageAlphaNoneSkipFirst because the format exposed by the graphic
> card is already converted into big endian.


Yes it was, but a `make clean` fixed that right up.
The problem I'm seeing now is an x86 guest on an x86 host at 16 bit  
color,
Windows XP at 16bit color on an x86 mac seems to incorrectly read the  
pixel data; 24 bit works
like a charm. Gentoo in framebuffer mode at 16 bit color renders  
correctly on PPC host, as does
Windows XP in 16 and 24 bit color. There is no difference in the match  
calculation for 16bit
color in the host code for PPC or x86, so very odd that only the x86  
version seems corrupt.

	—Sam

[-- Attachment #2: Type: text/html, Size: 2264 bytes --]

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Stefano Stabellini]

Samuel Benson wrote:

> Yes it was, but a `make clean` fixed that right up.
> The problem I'm seeing now is an x86 guest on an x86 host at 16 bit color,
> Windows XP at 16bit color on an x86 mac seems to incorrectly read the
> pixel data; 24 bit works
> like a charm. Gentoo in framebuffer mode at 16 bit color renders
> correctly on PPC host, as does
> Windows XP in 16 and 24 bit color. There is no difference in the match
> calculation for 16bit
> color in the host code for PPC or x86, so very odd that only the x86
> version seems corrupt.
> 





As I was telling you before, the guest in 16bpp is one of the "odd"
cases, because everything usually is converted into 32bpp.
It only happen when both host and guest are x86 because the 16bpp
optimization that the displaystate interface change introduced is only
enabled when guest and host have the same endianness.

I think the way you specify bitsPerComponent may be incorrect in the
16bpp case, because the format is actually 565:

red 5 bits
green 6 bits
blue 5 bits

isn't there a way to provide the full color masks like with sdl?

guest_screen = SDL_CreateRGBSurfaceFrom(ds_get_data(ds), ds_get_width(ds), ds_get_height(ds),
                                            ds_get_bits_per_pixel(ds), ds_get_linesize(ds),
                                            ds->surface->pf.rmask, ds->surface->pf.gmask,
                                            ds->surface->pf.bmask, ds->surface->pf.amask);

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Samuel Benson]

On Jan 27, 2009, at 4:47 AM, Stefano Stabellini wrote:

> As I was telling you before, the guest in 16bpp is one of the "odd"
> cases, because everything usually is converted into 32bpp.
> It only happen when both host and guest are x86 because the 16bpp
> optimization that the displaystate interface change introduced is only
> enabled when guest and host have the same endianness.

> I think the way you specify bitsPerComponent may be incorrect in the
> 16bpp case, because the format is actually 565:
>

It was; since OS X is an RGBA system, it also assumes everything to be  
RGBA, so when I did the
math for 16bpp, it comes out 4444. It turns out OSX not only assumes,  
but forces everything
it does into RGBA; valid bits-per-component are forced into powers of  
2. Also, according to
CoreImage mail-list, OSX does not support RGB 565;  however it does  
support XRGB 1555.

> isn't there a way to provide the full color masks like with sdl?
>
> guest_screen = SDL_CreateRGBSurfaceFrom(ds_get_data(ds),  
> ds_get_width(ds), ds_get_height(ds),
>                                             
> ds_get_bits_per_pixel(ds), ds_get_linesize(ds),
>                                            ds->surface->pf.rmask, ds- 
> >surface->pf.gmask,
>                                            ds->surface->pf.bmask, ds- 
> >surface->pf.amask);

I spent a couple hours pouring thru the documentation, and being  
unable to find a similar
function, I was forced to google. That's when I came across the above  
mentioned "answer".

Now, you stated that the 16bpp case only exists when both host and  
guest are x86; Is there a way
I can trick it into thinking its a x86 guest on a ppc host so it would  
return the upconverted
32bpp? I'm assuming it would be just a toggle of the  
QEMU_BIG_ENDIAN_FLAG on the DisplaySurface.

The only other option I can think of is passing it thru some  
CoreVector Imaging constructs to
try to convert it to XRGB and telling it to ignore the leading fake  
alpha bit.

—Sam

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Stefano Stabellini]

Samuel Benson wrote:

> 
> On Jan 27, 2009, at 4:47 AM, Stefano Stabellini wrote:
> 
>> As I was telling you before, the guest in 16bpp is one of the "odd"
>> cases, because everything usually is converted into 32bpp.
>> It only happen when both host and guest are x86 because the 16bpp
>> optimization that the displaystate interface change introduced is only
>> enabled when guest and host have the same endianness.
> 
>> I think the way you specify bitsPerComponent may be incorrect in the
>> 16bpp case, because the format is actually 565:
>>
> 
> It was; since OS X is an RGBA system, it also assumes everything to be
> RGBA, so when I did the
> math for 16bpp, it comes out 4444. It turns out OSX not only assumes,
> but forces everything
> it does into RGBA; valid bits-per-component are forced into powers of 2.
> Also, according to
> CoreImage mail-list, OSX does not support RGB 565;  however it does
> support XRGB 1555.
> 
>> isn't there a way to provide the full color masks like with sdl?
>>
>> guest_screen = SDL_CreateRGBSurfaceFrom(ds_get_data(ds),
>> ds_get_width(ds), ds_get_height(ds),
>>                                            ds_get_bits_per_pixel(ds),
>> ds_get_linesize(ds),
>>                                            ds->surface->pf.rmask,
>> ds->surface->pf.gmask,
>>                                            ds->surface->pf.bmask,
>> ds->surface->pf.amask);
> 
> I spent a couple hours pouring thru the documentation, and being unable
> to find a similar
> function, I was forced to google. That's when I came across the above
> mentioned "answer".
> 
> Now, you stated that the 16bpp case only exists when both host and guest
> are x86; Is there a way
> I can trick it into thinking its a x86 guest on a ppc host so it would
> return the upconverted
> 32bpp? I'm assuming it would be just a toggle of the
> QEMU_BIG_ENDIAN_FLAG on the DisplaySurface.
> 
> The only other option I can think of is passing it thru some CoreVector
> Imaging constructs to
> try to convert it to XRGB and telling it to ignore the leading fake
> alpha bit.
> 




Maybe cocoa does not support RGB565 rendering but it has to provide a
way to convert preexiting RGB565 pictures into a format that it can
actually render.  Isn't there an image manipulation library, or
something like that?

Sorry for insisting on this but I would like to expoit all the other
possible options before adding hacks to the interface or removing the
16bpp shared video buffer.

Anyway if it is really not possible I could add something like:

---

diff --git a/hw/vga.c b/hw/vga.c
index 2084ff4..b742c78 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1621,7 +1621,7 @@ static void vga_draw_graphic(VGAState *s, int full_update)
         disp_width != s->last_width ||
         height != s->last_height ||
         s->last_depth != depth) {
-#if defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
+#if defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN) && !defined(CONFIG_COCOA)
         if (depth == 16 || depth == 32) {
 #else
         if (depth == 32) {

---

this would prevent 16bpp to ever be expoed to cocoa, but you would still be
required of being able to render inverted ARGB8888, I mean the pixel
format specified by console.c:qemu_different_endianness_pixelformat in
32bpp case.
Do you think cocoa can do that?
You can try if that works using a Windows VM with stdvga in 32bpp (no 24bpp,
I really mean 32bpp) on a PPC host.

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Andreas Färber]

Am 25.01.2009 um 09:23 schrieb Samuel Benson:

> Version 2 does as follows:
>
> [1]: Corrects endianness on issues by using native BGR to RGB  
> conversion
> [2]: Uses DisplayState accessors for obtaining graphics context  
> information, which
> [3]: Removes now unused variables, and
> [4]: Allows reading of varying color modes (32bit/24/16), and  
> converting to native colorspace
> [5]: Attempts to keep itself centered on screen (as opposed to  
> bottom right, which immediately
>     goes off screen after bios load) on context changes (window  
> resizes)
>
> Testing working on i386 (gentoo, Windows 2000) and PPC (debian)  
> guests on PPC and x86 Macs.
>
> In regards to [4], Windows 2000 displays fine on quick tests, but on  
> the lowest setting
> I could test, 16bit color depth at 4bpp, colors are slightly off. I  
> used gentoo
> install-x86-minimal-2008.0 in framebuffer mode to test above  
> setting; the usual grey text is
> now blue, and Tux appears to be BGR shifted. I do not know if  
> previous code worked at such
> a low color setting.
>
> Signed-off-by: Samuel Benson <qemu_ml@digitalescape.info>

Acked-by: Andreas Faerber <andreas.faerber@web.de>

The patch fixes compilation, and I have tested the following guests on  
ppc host:
Debian/sparc32 (OpenBIOS and penguin feet are yellow)
Haiku/x86 @ 16bit, 15bit, 8bit (blue background, everything looks  
normal)

This patch implicitly adds a new feature, it centers the window on  
size change ("[normalWindow center];"). Previously the window would  
extend below the desktop bounds after the BIOS/OpenBIOS screen. Maybe  
that should be split out from the DisplayState changes?

This patch probably needs a Signed-off-by from Stefano since it is  
likely based upon his draft patch from Dec 19.

There was an additional patch from Stefano in this thread that I have  
not yet tested. It was supposed to fix some Windows guests as a last  
resort (disabling 16-bit optimizations due to lacking support in  
Cocoa), but nobody replied to it yet apparently.

I haven't checked in-depth in which way C. W. Bett's diff is related  
to these two. It does seem to contain an additional output typo fix  
not in here. Could you post that as a separate patch?

Andreas

> cocoa.m |   53 ++++++++++++++++++++---------------------------------
> 1 files changed, 20 insertions(+), 33 deletions(-)
>
> diff --git a/cocoa.m b/cocoa.m
> index fe13952..55ff2b4 100644
> --- a/cocoa.m
> +++ b/cocoa.m
> @@ -57,7 +57,7 @@ typedef struct {
> int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
> NSWindow *normalWindow;
> id cocoaView;
> -static void *screenBuffer;
> +static DisplayChangeListener *dcl;
>
> int gArgc;
> char **gArgv;
> @@ -292,9 +292,6 @@ int cocoa_keycode_to_qemu(int keycode)
> {
>     COCOA_DEBUG("QemuCocoaView: dealloc\n");
>
> -    if (screenBuffer)
> -        free(screenBuffer);
> -
>     if (dataProviderRef)
>         CGDataProviderRelease(dataProviderRef);
>
> @@ -305,9 +302,6 @@ int cocoa_keycode_to_qemu(int keycode)
> {
>     COCOA_DEBUG("QemuCocoaView: drawRect\n");
>
> -    if ((int)screenBuffer == -1)
> -        return;
> -
>     // get CoreGraphic context
>     CGContextRef viewContextRef = [[NSGraphicsContext  
> currentContext] graphicsPort];
>     CGContextSetInterpolationQuality (viewContextRef,  
> kCGInterpolationNone);
> @@ -320,10 +314,10 @@ int cocoa_keycode_to_qemu(int keycode)
>             screen.height, //height
>             screen.bitsPerComponent, //bitsPerComponent
>             screen.bitsPerPixel, //bitsPerPixel
> -            (screen.width * 4), //bytesPerRow
> +            (screen.width * (screen.bitsPerComponent/2)), // 
> bytesPerRow
> #if __LITTLE_ENDIAN__
>             CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), // 
> colorspace for OS X >= 10.4
> -            kCGImageAlphaNoneSkipLast,
> +            kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
> #else
>             CGColorSpaceCreateDeviceRGB(), //colorspace for OS X <  
> 10.4 (actually ppc)
>             kCGImageAlphaNoneSkipFirst, //bitmapInfo
> @@ -395,22 +389,12 @@ int cocoa_keycode_to_qemu(int keycode)
>     // update screenBuffer
>     if (dataProviderRef)
>         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);
> +    //sync host window color space with guests
> +	screen.bitsPerPixel = ds_get_bits_per_pixel(ds);
> +	screen.bitsPerComponent = ds_get_bytes_per_pixel(ds) * 2;
> +
> +    dataProviderRef = CGDataProviderCreateWithData(NULL,  
> ds_get_data(ds), w * 4 * h, NULL);
>
>     // update windows
>     if (isFullscreen) {
> @@ -423,6 +407,7 @@ int cocoa_keycode_to_qemu(int keycode)
>     }
>     screen.width = w;
>     screen.height = h;
> +	[normalWindow center];
>     [self setContentDimensions];
>     [self setFrame:NSMakeRect(cx, cy, cw, ch)];
> }
> @@ -740,6 +725,7 @@ int cocoa_keycode_to_qemu(int keycode)
>         [normalWindow setTitle:[NSString stringWithFormat:@"QEMU"]];
>         [normalWindow setContentView:cocoaView];
>         [normalWindow makeKeyAndOrderFront:self];
> +		[normalWindow center];
>
>     }
>     return self;
> @@ -939,11 +925,11 @@ static void cocoa_update(DisplayState *ds, int  
> x, int y, int w, int h)
>     [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:(int)(ds_get_width(ds)) height: 
> (int)(ds_get_height(ds)) displayState:ds];
> }
>
> static void cocoa_refresh(DisplayState *ds)
> @@ -975,20 +961,21 @@ static void cocoa_refresh(DisplayState *ds)
> static void cocoa_cleanup(void)
> {
>     COCOA_DEBUG("qemu_cocoa: cocoa_cleanup\n");
> -
> +	qemu_free(dcl);
> }
>
> void cocoa_display_init(DisplayState *ds, int full_screen)
> {
>     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;
> +	dcl = qemu_mallocz(sizeof(DisplayChangeListener));
> +	
> +    // register vga output callbacks
> +    dcl->dpy_update = cocoa_update;
> +    dcl->dpy_resize = cocoa_resize;
> +    dcl->dpy_refresh = cocoa_refresh;
>
> -    // give window a initial Size
> -    cocoa_resize(ds, 640, 400);
> +	register_displaychangelistener(ds, dcl);
>
>     // register cleanup function
>     atexit(cocoa_cleanup);
>
>

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Stefano Stabellini]

Andreas Färber wrote:

> Acked-by: Andreas Faerber <andreas.faerber@web.de>
> 
> The patch fixes compilation, and I have tested the following guests on  
> ppc host:
> Debian/sparc32 (OpenBIOS and penguin feet are yellow)
> Haiku/x86 @ 16bit, 15bit, 8bit (blue background, everything looks  
> normal)
> 
> This patch implicitly adds a new feature, it centers the window on  
> size change ("[normalWindow center];"). Previously the window would  
> extend below the desktop bounds after the BIOS/OpenBIOS screen. Maybe  
> that should be split out from the DisplayState changes?
> 
> This patch probably needs a Signed-off-by from Stefano since it is  
> likely based upon his draft patch from Dec 19.
> 
> There was an additional patch from Stefano in this thread that I have  
> not yet tested. It was supposed to fix some Windows guests as a last  
> resort (disabling 16-bit optimizations due to lacking support in  
> Cocoa), but nobody replied to it yet apparently.
> 
> I haven't checked in-depth in which way C. W. Bett's diff is related  
> to these two. It does seem to contain an additional output typo fix  
> not in here. Could you post that as a separate patch?
> 







The problem with this patch is that it won't handle correctly the
following case: host and guest x86, host is a MacOS host, guest is a
Windows guest with 16bpp resolution.  The cause of the problem is that
Windows uses 565 RGB for 16bpp while MacOS seems to be unable not only
to render but even to convert this resolution into another.

Due to the lack of alternatives I suggest to introduce the following
hack to workaround the problem:

---

diff --git a/hw/vga.c b/hw/vga.c
index 2084ff4..b742c78 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1621,7 +1621,7 @@ static void vga_draw_graphic(VGAState *s, int full_update)
         disp_width != s->last_width ||
         height != s->last_height ||
         s->last_depth != depth) {
-#if defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
+#if defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN) && !defined(CONFIG_COCOA)
         if (depth == 16 || depth == 32) {
 #else
         if (depth == 32) {


---

Apart from this issue the cocoa patch is fine, you can add my signed off
line to it.

Re: [Qemu-devel] [PATCH] [v2] Update cocoa.m to match new DisplayState code

[Anthony Liguori]

Samuel Benson wrote:
> Version 2 does as follows:

This patch doesn't apply.  Looks like your mailer munged the whitespace.

Regards,

Anthony Liguori