[PATCH] ui/cocoa: Do not copy members of pixman image

[PATCH] ui/cocoa: Do not copy members of pixman image

[Akihiko Odaki]

The old CocoaView had an idea of synchronizing the host window
configuration and the guest screen configuration. Here, the guest screen
actually means pixman image given ui/cocoa display implementation.

However, [CocoaView -drawRect:] directly interacts with the pixman
image buffer in reality. There is no such distinction of "host" and
"guest." This change removes the "host" configuration and let drawRect
consistently have the direct reference to pixman image. It allows to
get rid of the error-prone "sync" and reduce code size a bit.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 ui/cocoa.m | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 13fba8103e1..8eff3a99e93 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -70,8 +70,6 @@
 typedef struct {
     int width;
     int height;
-    int bitsPerComponent;
-    int bitsPerPixel;
 } QEMUScreen;
 
 NSWindow *normalWindow, *about_window;
@@ -284,7 +282,6 @@ @interface QemuCocoaView : NSView
     QEMUScreen screen;
     NSWindow *fullScreenWindow;
     float cx,cy,cw,ch,cdx,cdy;
-    CGDataProviderRef dataProviderRef;
     pixman_image_t *pixman_image;
     BOOL modifiers_state[256];
     BOOL isMouseGrabbed;
@@ -331,8 +328,6 @@ - (id)initWithFrame:(NSRect)frameRect
     self = [super initWithFrame:frameRect];
     if (self) {
 
-        screen.bitsPerComponent = 8;
-        screen.bitsPerPixel = 32;
         screen.width = frameRect.size.width;
         screen.height = frameRect.size.height;
 
@@ -344,8 +339,7 @@ - (void) dealloc
 {
     COCOA_DEBUG("QemuCocoaView: dealloc\n");
 
-    if (dataProviderRef) {
-        CGDataProviderRelease(dataProviderRef);
+    if (pixman_image) {
         pixman_image_unref(pixman_image);
     }
 
@@ -424,18 +418,28 @@ - (void) drawRect:(NSRect) rect
     CGContextSetShouldAntialias (viewContextRef, NO);
 
     // draw screen bitmap directly to Core Graphics context
-    if (!dataProviderRef) {
+    if (!pixman_image) {
         // Draw request before any guest device has set up a framebuffer:
         // just draw an opaque black rectangle
         CGContextSetRGBFillColor(viewContextRef, 0, 0, 0, 1.0);
         CGContextFillRect(viewContextRef, NSRectToCGRect(rect));
     } else {
+        int w = pixman_image_get_width(pixman_image);
+        int h = pixman_image_get_height(pixman_image);
+        int bitsPerPixel = PIXMAN_FORMAT_BPP(pixman_image_get_format(pixman_image));
+        int bitsPerComponent = DIV_ROUND_UP(bitsPerPixel, 8) * 2;
+        CGDataProviderRef dataProviderRef = CGDataProviderCreateWithData(
+            NULL,
+            pixman_image_get_data(pixman_image),
+            w * 4 * h,
+            NULL
+        );
         CGImageRef imageRef = CGImageCreate(
-            screen.width, //width
-            screen.height, //height
-            screen.bitsPerComponent, //bitsPerComponent
-            screen.bitsPerPixel, //bitsPerPixel
-            (screen.width * (screen.bitsPerComponent/2)), //bytesPerRow
+            w, //width
+            h, //height
+            bitsPerComponent, //bitsPerComponent
+            bitsPerPixel, //bitsPerPixel
+            (w * (bitsPerComponent/2)), //bytesPerRow
 #ifdef __LITTLE_ENDIAN__
             CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4
             kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
@@ -458,7 +462,7 @@ - (void) drawRect:(NSRect) rect
         [self getRectsBeingDrawn:&rectList count:&rectCount];
         for (i = 0; i < rectCount; i++) {
             clipRect.origin.x = rectList[i].origin.x / cdx;
-            clipRect.origin.y = (float)screen.height - (rectList[i].origin.y + rectList[i].size.height) / cdy;
+            clipRect.origin.y = (float)h - (rectList[i].origin.y + rectList[i].size.height) / cdy;
             clipRect.size.width = rectList[i].size.width / cdx;
             clipRect.size.height = rectList[i].size.height / cdy;
             clipImageRef = CGImageCreateWithImageInRect(
@@ -469,6 +473,7 @@ - (void) drawRect:(NSRect) rect
             CGImageRelease (clipImageRef);
         }
         CGImageRelease (imageRef);
+        CGDataProviderRelease(dataProviderRef);
     }
 }
 
@@ -511,7 +516,6 @@ - (void) switchSurface:(pixman_image_t *)image
 
     int w = pixman_image_get_width(image);
     int h = pixman_image_get_height(image);
-    pixman_format_code_t image_format = pixman_image_get_format(image);
     /* cdx == 0 means this is our very first surface, in which case we need
      * to recalculate the content dimensions even if it happens to be the size
      * of the initial empty window.
@@ -529,17 +533,11 @@ - (void) switchSurface:(pixman_image_t *)image
     }
 
     // update screenBuffer
-    if (dataProviderRef) {
-        CGDataProviderRelease(dataProviderRef);
+    if (pixman_image) {
         pixman_image_unref(pixman_image);
     }
 
-    //sync host window color space with guests
-    screen.bitsPerPixel = PIXMAN_FORMAT_BPP(image_format);
-    screen.bitsPerComponent = DIV_ROUND_UP(screen.bitsPerPixel, 8) * 2;
-
     pixman_image = image;
-    dataProviderRef = CGDataProviderCreateWithData(NULL, pixman_image_get_data(image), w * 4 * h, NULL);
 
     // update windows
     if (isFullscreen) {
-- 
2.24.3 (Apple Git-128)

Re: [PATCH] ui/cocoa: Do not copy members of pixman image

[Gerd Hoffmann]

On Fri, Feb 12, 2021 at 09:06:29AM +0900, Akihiko Odaki wrote:
> The old CocoaView had an idea of synchronizing the host window
> configuration and the guest screen configuration. Here, the guest screen
> actually means pixman image given ui/cocoa display implementation.
> 
> However, [CocoaView -drawRect:] directly interacts with the pixman
> image buffer in reality. There is no such distinction of "host" and
> "guest." This change removes the "host" configuration and let drawRect
> consistently have the direct reference to pixman image. It allows to
> get rid of the error-prone "sync" and reduce code size a bit.

Added to UI patch queue.

thanks,
  Gerd