scratch/ns/performance 690bb68 3/6: Fix problems with reusing IOSurfaces

[Alan Third]

branch: scratch/ns/performance
commit 690bb687f6f2026e42b878be5fe243c4562e2427
Author: Alan Third <alan@idiocy.org>
Commit: Alan Third <alan@idiocy.org>

    Fix problems with reusing IOSurfaces
    
    * src/nsterm.h: New EmacsSurface class and update EmacsView
    definitions.
    * src/nsterm.m ([EmacsView dealloc]):
    ([EmacsView viewDidResize:]): Handle new EmacsSurface class.
    ([EmacsView initFrameFromEmacs:]): Remove reference to old method.
    ([EmacsView createDrawingBuffer]): Remove method.
    ([EmacsView focusOnDrawingBuffer]):
    ([EmacsView unfocusDrawingBuffer]):
    ([EmacsView windowDidChangeBackingProperties:]): Use new EmacsSurface
    class.
    ([EmacsView copyRect:to:]): Get information from the context instead
    of direct from the IOSurface.
    ([EmacsView updateLayer]): Use new EmacsSurface class.
    ([EmacsSurface initWithSize:ColorSpace:]):
    ([EmacsSurface dealloc]):
    ([EmacsSurface getSize]):
    ([EmacsSurface getContext]):
    ([EmacsSurface releaseContext]):
    ([EmacsSurface getSurface]):
    ([EmacsSurface copyContentsTo:]): New class and methods.
---
 src/nsterm.h |  34 +++---
 src/nsterm.m | 332 +++++++++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 266 insertions(+), 100 deletions(-)

diff --git a/src/nsterm.h b/src/nsterm.h
index b244208..50405ea 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -414,6 +414,7 @@ typedef id instancetype;
    ========================================================================== 
*/
 
 @class EmacsToolbar;
+@class EmacsSurface;
 
 #ifdef NS_IMPL_COCOA
 @interface EmacsView : NSView <NSTextInput, NSWindowDelegate>
@@ -435,8 +436,7 @@ typedef id instancetype;
    BOOL fs_is_native;
    BOOL in_fullscreen_transition;
 #ifdef NS_DRAW_TO_BUFFER
-   IOSurfaceRef surface;
-   CGContextRef drawingBuffer;
+   EmacsSurface *surface;
 #endif
 @public
    struct frame *emacsframe;
@@ -480,7 +480,6 @@ typedef id instancetype;
 #ifdef NS_DRAW_TO_BUFFER
 - (void)focusOnDrawingBuffer;
 - (void)unfocusDrawingBuffer;
-- (void)createDrawingBuffer;
 #endif
 - (void)copyRect:(NSRect)srcRect to:(NSRect)dstRect;
 
@@ -707,6 +706,24 @@ typedef id instancetype;
 @end
 
 
+@interface EmacsSurface : NSObject
+{
+  NSMutableArray *cache;
+  NSSize size;
+  CGColorSpaceRef colorSpace;
+  IOSurfaceRef currentSurface;
+  IOSurfaceRef lastSurface;
+  CGContextRef context;
+}
+- (EmacsSurface *) initWithSize: (NSSize)s ColorSpace: (CGColorSpaceRef)cs;
+- (void) dealloc;
+- (NSSize) getSize;
+- (CGContextRef) getContext;
+- (void) releaseContext;
+- (IOSurfaceRef) getSurface;
+@end
+
+
 /* ==========================================================================
 
    Rendering
@@ -724,17 +741,6 @@ extern EmacsMenu *svcsMenu;
 @end
 #endif
 
-/* This is a private API, but it seems we need it to force the CALayer
-   to recognise that the IOSurface has been updated.
-
-   I believe using it will prevent Emacs from ever making it into the
-   Apple App Store.  😎 */
-#ifdef NS_DRAW_TO_BUFFER
-@interface CALayer (Private)
-- (void)setContentsChanged;
-@end
-#endif
-
 #endif  /* __OBJC__ */
 
 
diff --git a/src/nsterm.m b/src/nsterm.m
index 9fea83d..e621a69 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -6273,8 +6273,7 @@ not_in_argv (NSString *arg)
             object:nil];
 
 #ifdef NS_DRAW_TO_BUFFER
-  CGContextRelease (drawingBuffer);
-  CFRelease (surface);
+  [surface release];
 #endif
 
   [toolbar release];
@@ -7297,8 +7296,9 @@ not_in_argv (NSString *arg)
   if ([self wantsUpdateLayer])
     {
       CGFloat scale = [[self window] backingScaleFactor];
-      int oldw = (CGFloat)CGBitmapContextGetWidth (drawingBuffer) / scale;
-      int oldh = (CGFloat)CGBitmapContextGetHeight (drawingBuffer) / scale;
+      NSSize size = [surface getSize];
+      int oldw = size.width / scale;
+      int oldh = size.height / scale;
 
       NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
 
@@ -7308,6 +7308,9 @@ not_in_argv (NSString *arg)
           NSTRACE_MSG ("No change");
           return;
         }
+
+      [surface release];
+      surface = nil;
     }
 #endif
 
@@ -7320,9 +7323,6 @@ not_in_argv (NSString *arg)
                      FRAME_PIXEL_TO_TEXT_HEIGHT (emacsframe, newh),
                      0, YES, 0, 1);
 
-#ifdef NS_DRAW_TO_BUFFER
-  [self createDrawingBuffer];
-#endif
   SET_FRAME_GARBAGED (emacsframe);
   cancel_mouse_face (emacsframe);
 }
@@ -7593,10 +7593,6 @@ not_in_argv (NSString *arg)
   [NSApp registerServicesMenuSendTypes: ns_send_types
                            returnTypes: [NSArray array]];
 
-#ifdef NS_DRAW_TO_BUFFER
-  [self createDrawingBuffer];
-#endif
-
   /* Set up view resize notifications.  */
   [self setPostsFrameChangedNotifications:YES];
   [[NSNotificationCenter defaultCenter]
@@ -8316,81 +8312,41 @@ not_in_argv (NSString *arg)
 
 
 #ifdef NS_DRAW_TO_BUFFER
-- (void)createDrawingBuffer
-  /* Create and store a new CGGraphicsContext for Emacs to draw into.
-
-     We can't do this in GNUstep as there's no equivalent, so under
-     GNUstep we retain the old method of drawing direct to the
-     EmacsView.  */
+- (void)focusOnDrawingBuffer
 {
-  NSTRACE ("EmacsView createDrawingBuffer]");
-
-  if (! [self wantsUpdateLayer])
-    return;
-
-  NSGraphicsContext *screen;
-  CGColorSpaceRef colorSpace = [[[self window] colorSpace] CGColorSpace];
   CGFloat scale = [[self window] backingScaleFactor];
-  NSRect frame = [self frame];
-  int width, height, bytesPerRow;
-
-  if (drawingBuffer != nil)
-    {
-      CGContextRelease (drawingBuffer);
-      CFRelease (surface);
-    }
-
-  width = NSWidth (frame) * scale;
-  height = NSHeight (frame) * scale;
-  bytesPerRow = IOSurfaceAlignProperty (kIOSurfaceBytesPerRow, width * 4);
-
-  surface = IOSurfaceCreate
-    ((CFDictionaryRef)@{(id)kIOSurfaceWidth:[NSNumber numberWithInt:width],
-        (id)kIOSurfaceHeight:[NSNumber numberWithInt:height],
-        (id)kIOSurfaceBytesPerRow:[NSNumber numberWithInt:bytesPerRow],
-        (id)kIOSurfaceBytesPerElement:[NSNumber numberWithInt:4],
-        (id)kIOSurfacePixelFormat:[NSNumber 
numberWithInt:kCVPixelFormatType_32RGBA]});
-
-  drawingBuffer = CGBitmapContextCreate (IOSurfaceGetBaseAddress (surface),
-                                         IOSurfaceGetWidth (surface),
-                                         IOSurfaceGetHeight (surface),
-                                         8,
-                                         IOSurfaceGetBytesPerRow (surface),
-                                         colorSpace,
-                                         IOSurfaceGetPixelFormat (surface));
-
-  /* This fixes the scale to match the backing scale factor, and flips the 
image.  */
-  CGContextTranslateCTM(drawingBuffer, 0, IOSurfaceGetHeight (surface));
-  CGContextScaleCTM(drawingBuffer, scale, -scale);
-}
 
+  NSTRACE ("[EmacsView focusOnDrawingBuffer]");
 
-- (void)focusOnDrawingBuffer
-{
-  IOReturn lockStatus;
+  if (! surface)
+    {
+      NSRect frame = [self frame];
+      NSSize s = NSMakeSize (NSWidth (frame) * scale, NSHeight (frame) * 
scale);
 
-  NSTRACE ("[EmacsView focusOnDrawingBuffer]");
+      surface = [[EmacsSurface alloc] initWithSize:s
+                                        ColorSpace:[[[self window] colorSpace]
+                                                     CGColorSpace]];
+    }
 
-  if ((lockStatus = IOSurfaceLock (surface, 0, nil)) != kIOReturnSuccess)
-    NSLog (@"Failed to lock surface: %x", lockStatus);
+  CGContextRef context = [surface getContext];
 
-  NSGraphicsContext *buf =
-    [NSGraphicsContext
-        graphicsContextWithCGContext:drawingBuffer flipped:YES];
+  CGContextTranslateCTM(context, 0, [surface getSize].height);
+  CGContextScaleCTM(context, scale, -scale);
 
-  [NSGraphicsContext setCurrentContext:buf];
+  [NSGraphicsContext
+    setCurrentContext:[NSGraphicsContext
+                        graphicsContextWithCGContext:context
+                                             flipped:YES]];
 }
 
 
 - (void)unfocusDrawingBuffer
 {
-  IOReturn lockStatus;
-
   NSTRACE ("[EmacsView unfocusDrawingBuffer]");
 
   [NSGraphicsContext setCurrentContext:nil];
-  if ((lockStatus = IOSurfaceUnlock (surface, 0, nil)) != kIOReturnSuccess)
-    NSLog (@"Failed to unlock surface: %x", lockStatus);
+  [surface releaseContext];
+  [self setNeedsDisplay:YES];
 }
 
 
@@ -8399,11 +8355,11 @@ not_in_argv (NSString *arg)
 {
   NSTRACE ("EmacsView windowDidChangeBackingProperties:]");
 
-  if (! [self wantsUpdateLayer])
-    return;
-
   NSRect frame = [self frame];
-  [self createDrawingBuffer];
+
+  [surface release];
+  surface = nil;
+
   ns_clear_frame (emacsframe);
   expose_frame (emacsframe, 0, 0, NSWidth (frame), NSHeight (frame));
 }
@@ -8422,12 +8378,15 @@ not_in_argv (NSString *arg)
     {
 #endif
       double scale = [[self window] backingScaleFactor];
-      int bpe = IOSurfaceGetBytesPerElement (surface);
-      void *pixels = IOSurfaceGetBaseAddress (surface);
-      int rowSize = IOSurfaceGetBytesPerRow (surface);
-      int srcRowSize = NSWidth (srcRect) * scale * bpe;
-      void *srcPixels = pixels + (int)(NSMinY (srcRect) * scale * rowSize + 
NSMinX (srcRect) * scale * bpe);
-      void *dstPixels = pixels + (int)(NSMinY (dstRect) * scale * rowSize + 
NSMinX (dstRect) * scale * bpe);
+      CGContextRef context = [[NSGraphicsContext currentContext] CGContext];
+      int bpp = CGBitmapContextGetBitsPerPixel (context) / 8;
+      void *pixels = CGBitmapContextGetData (context);
+      int rowSize = CGBitmapContextGetBytesPerRow (context);
+      int srcRowSize = NSWidth (srcRect) * scale * bpp;
+      void *srcPixels = pixels + (int)(NSMinY (srcRect) * scale * rowSize
+                                       + NSMinX (srcRect) * scale * bpp);
+      void *dstPixels = pixels + (int)(NSMinY (dstRect) * scale * rowSize
+                                       + NSMinX (dstRect) * scale * bpp);
 
       if (NSIntersectsRect (srcRect, dstRect)
           && NSMinY (srcRect) < NSMinY (dstRect))
@@ -8441,8 +8400,6 @@ not_in_argv (NSString *arg)
                    srcPixels + y * rowSize,
                    srcRowSize);
 
-      [self setNeedsDisplayInRect:dstRect];
-
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 101400
     }
   else
@@ -8480,12 +8437,14 @@ not_in_argv (NSString *arg)
 
 - (void)updateLayer
 {
-  CALayer *layer = [self layer];
-
   NSTRACE ("[EmacsView updateLayer]");
 
-  [layer setContents:(id)surface];
-  [layer setContentsChanged];
+  /* This can fail to update the screen if the same surface is
+     provided twice in a row, even if its contents have changed.
+     There's a private method, -[CALayer setContentsChanged], that we
+     could use to force it, but we shouldn't often get the same
+     surface twice in a row.  */
+  [[self layer] setContents:(id)[surface getSurface]];
 }
 #endif
 
@@ -9528,6 +9487,207 @@ not_in_argv (NSString *arg)
 @end  /* EmacsScroller */
 
 
+#ifdef NS_DRAW_TO_BUFFER
+
+/* ==========================================================================
+
+   A class to handle the screen buffer.
+
+   ========================================================================== 
*/
+
+@implementation EmacsSurface
+
+
+/* An IOSurface is a pixel buffer that is efficiently copied to VRAM
+   for display.  In order to use an IOSurface we must first lock it,
+   write to it, then unlock it.  At this point it is transferred to
+   VRAM and if we modify it during this transfer we may see corruption
+   of the output.  To avoid this problem we can check if the surface
+   is "in use", and if it is then avoid using it.  Unfortunately to
+   avoid writing to a surface that's in use, but still maintain the
+   ability to draw to the screen at any time, we need to keep a cache
+   of multiple surfaces that we can use at will.
+
+   The EmacsSurface class maintains this cache of surfaces, and
+   handles the conversion to a CGGraphicsContext that AppKit can use
+   to draw on.
+
+   The cache is simple: if a free surface is found it is removed from
+   the cache and set as the "current" surface.  Once Emacs is done
+   with drawing to the current surface, the previous surface that was
+   drawn to is added to the cache for reuse, and the current one is
+   set as the last surface.  If no free surfaces are found in the
+   cache then a new one is created.
+
+   When AppKit wants to update the screen, we provide it with the last
+   surface, as that has the most recent data.
+
+   FIXME: It is possible for the cache to grow if Emacs draws faster
+   than the surfaces can be drawn to the screen, so there should
+   probably be some sort of pruning job that removes excess
+   surfaces.  */
+
+
+- (EmacsSurface *) initWithSize: (NSSize)s
+                     ColorSpace: (CGColorSpaceRef)cs
+{
+  NSTRACE ("[EmacsSurface initWithSize:ColorSpace:]");
+
+  cache = [[NSMutableArray arrayWithCapacity:3] retain];
+  size = s;
+  colorSpace = cs;
+
+  return self;
+}
+
+
+- (void) dealloc
+{
+  if (context)
+    CGContextRelease (context);
+
+  if (currentSurface)
+    CFRelease (currentSurface);
+  if (lastSurface)
+    CFRelease (lastSurface);
+
+  for (id object in cache)
+    CFRelease ((IOSurfaceRef)object);
+
+  [cache removeAllObjects];
+
+  [super dealloc];
+}
+
+
+/* Return the size values our cached data is using.  */
+- (NSSize) getSize
+{
+  return size;
+}
+
+
+/* Return a CGContextRef that can be used for drawing to the screen.
+   This must ALWAYS be paired with a call to releaseContext, and the
+   calls cannot be nested.  */
+- (CGContextRef) getContext
+{
+  IOSurfaceRef surface = NULL;
+
+  NSTRACE ("[EmacsSurface getContextWithSize:]");
+  NSTRACE_MSG (@"IOSurface count: %lu", [cache count] + (lastSurface ? 1 : 0));
+
+  for (id object in cache)
+    {
+      if (!IOSurfaceIsInUse ((IOSurfaceRef)object))
+      {
+        surface = (IOSurfaceRef)object;
+        [cache removeObject:object];
+        break;
+      }
+    }
+
+  if (!surface)
+    {
+      int bytesPerRow = IOSurfaceAlignProperty (kIOSurfaceBytesPerRow,
+                                                size.width * 4);
+
+      surface = IOSurfaceCreate
+        ((CFDictionaryRef)@{(id)kIOSurfaceWidth:[NSNumber 
numberWithInt:size.width],
+            (id)kIOSurfaceHeight:[NSNumber numberWithInt:size.height],
+            (id)kIOSurfaceBytesPerRow:[NSNumber numberWithInt:bytesPerRow],
+            (id)kIOSurfaceBytesPerElement:[NSNumber numberWithInt:4],
+            (id)kIOSurfacePixelFormat:[NSNumber 
numberWithUnsignedInt:'RGBA']});
+    }
+
+  IOReturn lockStatus = IOSurfaceLock (surface, 0, nil);
+  if (lockStatus != kIOReturnSuccess)
+    NSLog (@"Failed to lock surface: %x", lockStatus);
+
+  [self copyContentsTo:surface];
+
+  currentSurface = surface;
+
+  context = CGBitmapContextCreate (IOSurfaceGetBaseAddress (currentSurface),
+                                   IOSurfaceGetWidth (currentSurface),
+                                   IOSurfaceGetHeight (currentSurface),
+                                   8,
+                                   IOSurfaceGetBytesPerRow (currentSurface),
+                                   colorSpace,
+                                   IOSurfaceGetPixelFormat (currentSurface));
+  return context;
+}
+
+
+/* Releases the CGGraphicsContext and unlocks the associated
+   IOSurface, so it will be sent to VRAM.  */
+- (void) releaseContext
+{
+  NSTRACE ("[EmacsSurface releaseContextAndGetSurface]");
+
+  CGContextRelease (context);
+  context = NULL;
+
+  IOReturn lockStatus = IOSurfaceUnlock (currentSurface, 0, nil);
+  if (lockStatus != kIOReturnSuccess)
+    NSLog (@"Failed to unlock surface: %x", lockStatus);
+
+  /* Put lastSurface back on the end of the cache.  It may not have
+     been displayed on the screen yet, but we probably want the new
+     data and not some stale data anyway.  */
+  if (lastSurface)
+    [cache addObject:(id)lastSurface];
+  lastSurface = currentSurface;
+  currentSurface = NULL;
+}
+
+
+/* Get the IOSurface that we want to draw to the screen.  */
+- (IOSurfaceRef) getSurface
+{
+  /* lastSurface always contains the most up-to-date and complete data.  */
+  return lastSurface;
+}
+
+
+/* Copy the contents of lastSurface to DESTINATION.  This is required
+   every time we want to use an IOSurface as its contents are probably
+   blanks (if it's new), or stale.  */
+- (void) copyContentsTo: (IOSurfaceRef) destination
+{
+  IOReturn lockStatus;
+  void *sourceData, *destinationData;
+  int numBytes = IOSurfaceGetAllocSize (destination);
+
+  NSTRACE ("[EmacsSurface copyContentsTo:]");
+
+  if (! lastSurface)
+    return;
+
+  lockStatus = IOSurfaceLock (lastSurface, kIOSurfaceLockReadOnly, nil);
+  if (lockStatus != kIOReturnSuccess)
+    NSLog (@"Failed to lock source surface: %x", lockStatus);
+
+  sourceData = IOSurfaceGetBaseAddress (lastSurface);
+  destinationData = IOSurfaceGetBaseAddress (destination);
+
+  /* Since every IOSurface should have the exact same settings, a
+     memcpy seems like the fastest way to copy the data from one to
+     the other.  */
+  memcpy (destinationData, sourceData, numBytes);
+
+  lockStatus = IOSurfaceUnlock (lastSurface, kIOSurfaceLockReadOnly, nil);
+  if (lockStatus != kIOReturnSuccess)
+    NSLog (@"Failed to unlock source surface: %x", lockStatus);
+}
+
+
+@end /* EmacsSurface */
+
+
+#endif
+
+
 #ifdef NS_IMPL_GNUSTEP
 /* Dummy class to get rid of startup warnings.  */
 @implementation EmacsDocument