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

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

[Samuel Benson]

Hello all,

Attached patch is an attempt to update cocoa.m to be in sync with the  
new
displaystate changes committed a few weeks back.

Tested working on OS X 10.5.6 on G4 and G5 PPC machines.

Tests and comments welcome.

---

cocoa.m |   44 +++++++++++++++++++++++++-------------------
1 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/cocoa.m b/cocoa.m
index fe13952..d4b3df0 100644
--- a/cocoa.m
+++ b/cocoa.m
@@ -58,6 +58,7 @@ 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;
@@ -293,7 +294,7 @@ int cocoa_keycode_to_qemu(int keycode)
      COCOA_DEBUG("QemuCocoaView: dealloc\n");

      if (screenBuffer)
-        free(screenBuffer);
+        qemu_free(screenBuffer);

      if (dataProviderRef)
          CGDataProviderRelease(dataProviderRef);
@@ -396,18 +397,18 @@ int cocoa_keycode_to_qemu(int keycode)
      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;
+        qemu_free(screenBuffer);
+    screenBuffer = qemu_mallocz( w * 4 * h );
+
+    ds->surface->data = screenBuffer;
+    ds->surface->linesize =  (w * 4);
+    ds->surface->pf.depth = 32;
+    ds->surface->width = w;
+    ds->surface->height = h;
  #ifdef __LITTLE_ENDIAN__
-    ds->bgr = 1;
+    ds->surface->flags = 0x00;
  #else
-    ds->bgr = 0;
+    ds->surface->flags = QEMU_BIG_ENDIAN_FLAG;
  #endif

      dataProviderRef = CGDataProviderCreateWithData(NULL,  
screenBuffer, w * 4 * h, NULL);
@@ -939,11 +940,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:(ds_get_width(ds)) height: 
(ds_get_height(ds)) displayState:ds];
  }

  static void cocoa_refresh(DisplayState *ds)
@@ -969,26 +970,31 @@ static void cocoa_refresh(DisplayState *ds)
              [cocoaView handleEvent:event];
          }
      } while(event != nil);
+	COCOA_DEBUG("qemu_cocoa: vga_hw_update()\n");
      vga_hw_update();
  }

  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;

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

      // register cleanup function
      atexit(cocoa_cleanup);

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

[Stefano Stabellini]

Samuel Benson wrote:

> Hello all,
> 
> Attached patch is an attempt to update cocoa.m to be in sync with the new
> displaystate changes committed a few weeks back.
> 
> Tested working on OS X 10.5.6 on G4 and G5 PPC machines.
> 
> Tests and comments welcome.



Hi Samuel, thanks for your efforts.


> @@ -396,18 +397,18 @@ int cocoa_keycode_to_qemu(int keycode)
>      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;
> +        qemu_free(screenBuffer);
> +    screenBuffer = qemu_mallocz( w * 4 * h );
> +
> +    ds->surface->data = screenBuffer;
> +    ds->surface->linesize =  (w * 4);
> +    ds->surface->pf.depth = 32;
> +    ds->surface->width = w;
> +    ds->surface->height = h;
>  #ifdef __LITTLE_ENDIAN__
> -    ds->bgr = 1;
> +    ds->surface->flags = 0x00;
>  #else
> -    ds->bgr = 0;
> +    ds->surface->flags = QEMU_BIG_ENDIAN_FLAG;
>  #endif
> 
>      dataProviderRef = CGDataProviderCreateWithData(NULL, screenBuffer,
> w * 4 * h, NULL);



It would be better not to change the DisplayState surface from
cocoa\sdl\vnc but try to render the format exposed by the emulated
graphic card ourself.
In fact in most cases the emulated vga exposes a 32bpp surface with
linesize = width * 4, so you don't need to force it yourself.
The only exception is the 16bpp case, but you should be able to render
that as well without too many problems.


>  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;
> 
> +    register_displaychangelistener(ds, dcl);
> +   
>      // give window a initial Size
> -    cocoa_resize(ds, 640, 400);
> +    cocoa_resize(ds);
> 



The initialization seems correct, the call to cocoa_resize may be
unnecessary.

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

[Samuel Benson]

On Jan 24, 2009, at 9:36 AM, Stefano Stabellini wrote:

> Samuel Benson wrote:
>
>> @@ -396,18 +397,18 @@ int cocoa_keycode_to_qemu(int keycode)
>>     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;
>> +        qemu_free(screenBuffer);
>> +    screenBuffer = qemu_mallocz( w * 4 * h );
>> +
>> +    ds->surface->data = screenBuffer;
>> +    ds->surface->linesize =  (w * 4);
>> +    ds->surface->pf.depth = 32;
>> +    ds->surface->width = w;
>> +    ds->surface->height = h;
>> #ifdef __LITTLE_ENDIAN__
>> -    ds->bgr = 1;
>> +    ds->surface->flags = 0x00;
>> #else
>> -    ds->bgr = 0;
>> +    ds->surface->flags = QEMU_BIG_ENDIAN_FLAG;
>> #endif
>>
>>     dataProviderRef = CGDataProviderCreateWithData(NULL,  
>> screenBuffer,
>> w * 4 * h, NULL);
>
>
>
> It would be better not to change the DisplayState surface from
> cocoa\sdl\vnc but try to render the format exposed by the emulated
> graphic card ourself.
> In fact in most cases the emulated vga exposes a 32bpp surface with
> linesize = width * 4, so you don't need to force it yourself.
> The only exception is the 16bpp case, but you should be able to render
> that as well without too many problems.

The previous code forced this change, and as this code merely synced  
it to the new structures,
I did not think anything to be wrong with it. If the structures come  
pre initialized,
I'll remove these lines and see if it still works.

The one section I did have trouble understanding was the  
DisplaySurface flags; are those set
in regards to the host endianness or the guests?

>> 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;
>>
>> +    register_displaychangelistener(ds, dcl);
>> +
>>     // give window a initial Size
>> -    cocoa_resize(ds, 640, 400);
>> +    cocoa_resize(ds);
>>
>
>
>
> The initialization seems correct, the call to cocoa_resize may be
> unnecessary.

You are right, quick test showed the initial resize call to be  
unnecessary.

I've also managed get a hold of an x86 Mac, so I will post a v2 once I  
test it out there.

	—Sam

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

[Alexander Graf]

On 24.01.2009, at 20:16, Samuel Benson wrote:

>
> On Jan 24, 2009, at 9:36 AM, Stefano Stabellini wrote:
>
>> Samuel Benson wrote:
>>
>>> @@ -396,18 +397,18 @@ int cocoa_keycode_to_qemu(int keycode)
>>>    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;
>>> +        qemu_free(screenBuffer);
>>> +    screenBuffer = qemu_mallocz( w * 4 * h );
>>> +
>>> +    ds->surface->data = screenBuffer;
>>> +    ds->surface->linesize =  (w * 4);
>>> +    ds->surface->pf.depth = 32;
>>> +    ds->surface->width = w;
>>> +    ds->surface->height = h;
>>> #ifdef __LITTLE_ENDIAN__
>>> -    ds->bgr = 1;
>>> +    ds->surface->flags = 0x00;
>>> #else
>>> -    ds->bgr = 0;
>>> +    ds->surface->flags = QEMU_BIG_ENDIAN_FLAG;
>>> #endif
>>>
>>>    dataProviderRef = CGDataProviderCreateWithData(NULL,  
>>> screenBuffer,
>>> w * 4 * h, NULL);
>>
>>
>>
>> It would be better not to change the DisplayState surface from
>> cocoa\sdl\vnc but try to render the format exposed by the emulated
>> graphic card ourself.
>> In fact in most cases the emulated vga exposes a 32bpp surface with
>> linesize = width * 4, so you don't need to force it yourself.
>> The only exception is the 16bpp case, but you should be able to  
>> render
>> that as well without too many problems.
>
> The previous code forced this change, and as this code merely synced  
> it to the new structures,
> I did not think anything to be wrong with it. If the structures come  
> pre initialized,
> I'll remove these lines and see if it still works.
>
> The one section I did have trouble understanding was the  
> DisplaySurface flags; are those set
> in regards to the host endianness or the guests?
>
>>> 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;
>>>
>>> +    register_displaychangelistener(ds, dcl);
>>> +
>>>    // give window a initial Size
>>> -    cocoa_resize(ds, 640, 400);
>>> +    cocoa_resize(ds);
>>>
>>
>>
>>
>> The initialization seems correct, the call to cocoa_resize may be
>> unnecessary.
>
> You are right, quick test showed the initial resize call to be  
> unnecessary.
>
> I've also managed get a hold of an x86 Mac, so I will post a v2 once  
> I test it out there.

It seems to be partly broken on x86 Macs. An x86 guest showed BGR vs  
RGB issues and a PowerPC guest just stayed on black screen. Both  
worked fine with -vnc.

Alex

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

[Samuel Benson]

On Jan 24, 2009, at 3:22 PM, Alexander Graf wrote:

> It seems to be partly broken on x86 Macs. An x86 guest showed BGR vs  
> RGB issues and a PowerPC guest just stayed on black screen. Both  
> worked fine with -vnc.
>
> Alex

I've was able to get a hold of an x86 mac and see the same, and have  
corrected the the ppc
guest issues for both hosts, but it also shows colorspace issues.

The old DisplayState could directly set BGR or RBG via the bgr flag,  
but I cannot seem to find
a similar construct in the new DisplayState code. I've assumed it was  
merged into the flags for
DIsplaySurface, but a few hours testing proved that wrong.

CoreGraphics (the OSX native rendering system) purports there is a way  
to do such colorspace
conversions within itself, but a quick google says "no", so I'll  
continue to look.

	—Sam

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

[Stefano Stabellini]

Samuel Benson wrote:

>> It would be better not to change the DisplayState surface from
>> cocoa\sdl\vnc but try to render the format exposed by the emulated
>> graphic card ourself.
>> In fact in most cases the emulated vga exposes a 32bpp surface with
>> linesize = width * 4, so you don't need to force it yourself.
>> The only exception is the 16bpp case, but you should be able to render
>> that as well without too many problems.
> 
> The previous code forced this change, and as this code merely synced it
> to the new structures,
> I did not think anything to be wrong with it. If the structures come pre
> initialized,
> I'll remove these lines and see if it still works.


The old DisplayState interface was supposed to be initialized by cocoa,
then the emulated graphic card would do the conversion into the format
requested by cocoa.

The new DisplayState interface is initialized by the graphic card and
cocoa is supposed to be able to render whatever format is exposed by the
graphic card. The pixel formats actually exposed are very few, see my
other mail.

But from the second version of the patch I gather that you already
understand how the new interface works.

 
> The one section I did have trouble understanding was the DisplaySurface
> flags; are those set
> in regards to the host endianness or the guests?
> 

QEMU_BIG_ENDIAN_FLAG refers to the host endianness.
The guest video buffer is automatically converted into the host
endianness if necessary, so you don't have to worry about it.
You can always assume that the DisplayState surface has the same
endianness as the host, as long as you repect the pixelformat
ds->surface->pf (see my other mail and console.c:qemu_default_pixelformat).

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

[Anthony Liguori]

Samuel Benson wrote:
> Hello all,
>
> Attached patch is an attempt to update cocoa.m to be in sync with the new
> displaystate changes committed a few weeks back.
>
> Tested working on OS X 10.5.6 on G4 and G5 PPC machines.
>
> Tests and comments welcome.

Can you add a signed-off-by?

Regards,

Anthony Liguori

> ---
>
> cocoa.m |   44 +++++++++++++++++++++++++-------------------
> 1 files changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/cocoa.m b/cocoa.m
> index fe13952..d4b3df0 100644
> --- a/cocoa.m
> +++ b/cocoa.m
> @@ -58,6 +58,7 @@ 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;
> @@ -293,7 +294,7 @@ int cocoa_keycode_to_qemu(int keycode)
>      COCOA_DEBUG("QemuCocoaView: dealloc\n");
>
>      if (screenBuffer)
> -        free(screenBuffer);
> +        qemu_free(screenBuffer);
>
>      if (dataProviderRef)
>          CGDataProviderRelease(dataProviderRef);
> @@ -396,18 +397,18 @@ int cocoa_keycode_to_qemu(int keycode)
>      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;
> +        qemu_free(screenBuffer);
> +    screenBuffer = qemu_mallocz( w * 4 * h );
> +
> +    ds->surface->data = screenBuffer;
> +    ds->surface->linesize =  (w * 4);
> +    ds->surface->pf.depth = 32;
> +    ds->surface->width = w;
> +    ds->surface->height = h;
>  #ifdef __LITTLE_ENDIAN__
> -    ds->bgr = 1;
> +    ds->surface->flags = 0x00;
>  #else
> -    ds->bgr = 0;
> +    ds->surface->flags = QEMU_BIG_ENDIAN_FLAG;
>  #endif
>
>      dataProviderRef = CGDataProviderCreateWithData(NULL, 
> screenBuffer, w * 4 * h, NULL);
> @@ -939,11 +940,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:(ds_get_width(ds)) 
> height:(ds_get_height(ds)) displayState:ds];
>  }
>
>  static void cocoa_refresh(DisplayState *ds)
> @@ -969,26 +970,31 @@ static void cocoa_refresh(DisplayState *ds)
>              [cocoaView handleEvent:event];
>          }
>      } while(event != nil);
> +    COCOA_DEBUG("qemu_cocoa: vga_hw_update()\n");
>      vga_hw_update();
>  }
>
>  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;
>
> +    register_displaychangelistener(ds, dcl);
> +   
>      // give window a initial Size
> -    cocoa_resize(ds, 640, 400);
> +    cocoa_resize(ds);
>
>      // register cleanup function
>      atexit(cocoa_cleanup);
>
>
>

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

[C.W. Betts]

It compiles fine on all four Mac OS X targets (i386, x86_64, ppc,  
ppc64).  It does throw a warning when compiling on 64-bit hosts:
/Users/cwbetts/makestuff/qemu-allmac/src/cocoa.m: In function ‘- 
[QemuCocoaView drawRect:]’:
/Users/cwbetts/makestuff/qemu-allmac/src/cocoa.m:309: warning: cast  
from pointer to integer of different size
Also, as mentioned, it doesn't show anything on the screen unless it's  
console text (i.e. the BIOS on the i386 guest).

A feature request:  On VirtualBox, it is capable of intercepting most,  
if not all, keyboard calls.  This is useful when the key you want to  
press on the emulated side is a key reserved for the Mac OS X system,  
i.e. F12.  I would really appreciate it if someone could make this  
feature available for Qemu.
On Jan 23, 2009, at 3:41 PM, Samuel Benson wrote:

> Hello all,
>
> Attached patch is an attempt to update cocoa.m to be in sync with  
> the new
> displaystate changes committed a few weeks back.
>
> Tested working on OS X 10.5.6 on G4 and G5 PPC machines.
>
> Tests and comments welcome.
>
>
>
>

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

[Samuel Benson]

On Jan 25, 2009, at 3:14 PM, C.W. Betts wrote:

> It compiles fine on all four Mac OS X targets (i386, x86_64, ppc,  
> ppc64).  It does throw a warning when compiling on 64-bit hosts:
> /Users/cwbetts/makestuff/qemu-allmac/src/cocoa.m: In function ‘- 
> [QemuCocoaView drawRect:]’:
> /Users/cwbetts/makestuff/qemu-allmac/src/cocoa.m:309: warning: cast  
> from pointer to integer of different size
> Also, as mentioned, it doesn't show anything on the screen unless  
> it's console text (i.e. the BIOS on the i386 guest).

Version 2 of the patch was posted yesterday, which should correct most  
of those issues,
though I have not tried compiling a 64 bit version on a 64 bit host;  
will look into that.

> A feature request:  On VirtualBox, it is capable of intercepting  
> most, if not all, keyboard calls.  This is useful when the key you  
> want to press on the emulated side is a key reserved for the Mac OS  
> X system, i.e. F12.  I would really appreciate it if someone could  
> make this feature available for Qemu.

I'll look into this as well, since the function are also in cocoa.m

	—Sam

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

[Christian Brunschen]

</lurk>

> CoreGraphics (the OSX native rendering system) purports there is a  
> way to do such colorspace
> conversions within itself, but a quick google says "no", so I'll  
> continue to look.

CoreImage, however, does include the necessary facilities, including  
the existing 'CIColorMatrix' filter which will allow fairly arbitrary  
color transformations.

// Christian Brunschen