Re: Emacs NS 'scratch/ns/draw-to-bitmap' branch performance

[Alan Third <alan@idiocy.org>]

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

On Wed, Feb 05, 2020 at 11:00:05AM -0500, Daniel Pittman wrote:
> G'day Alan, et al.
> 
> After building Emacs with the ns/draw-to-bitmap branch merged, on macOS
> 10.15.2, I'm seeing some performance issues.  I'd like, Alan, to know how
> you would prefer me to proceed in a way that best helps you (and anyone
> else here) resolve them.
> 
> This is on a MacBook Pro (15-inch, 2019), Intel UHD Graphics 630 1536 MB +
> Radeon Pro 555X machine, driving the 2880 x 1800 internal LCD at the
> default 1680 x 1050 apparent resolution.  `frame-edges` are at 78, 45 and
> 1677, 1049, as far as Emacs is concerned.
> 
> The trivial reproduction, for me, is to hold any key (eg: "n") to get
> auto-repeat inserting it.  On this machine I see approximately 50
> characters every 10 seconds, or ~ 5 "frames" per second.  This is, of
> course, substantially lower than anywhere else in the OS, and also far
> below what I'd experience prior to that merge.
> 
> This is a pretty clear "graphics update" cost:
> (list (progn (goto-char 0) (benchmark-run 40 (next-line))) (progn
> (goto-char 0) (benchmark-run 40 (next-line) (redisplay))))
> => ((0.006762000000000001 0 0.0) (9.116147 0 0.0))
> 
> I can dig into this myself, and see if I can't figure out what is slowing
> everything down, but I figured I'd ask first.

To start with you should probably try the attached patch. I find it
much faster, but others still report it as unacceptably slow.

> PS: I didn't check yet, but one of my first points of call is going to be
> the "canDrawConcurrently" flag on the NSView, since the bitmap rep means it
> doesn't need to block anything while redrawing, yes?
> 
> https://developer.apple.com/documentation/appkit/nsview/1483425-candrawconcurrently?language=objc

That makes sense, but I’ve added that to the patch, but it doesn’t
seem to make much of a difference here.

You can try commenting it out at nsterm.m:7431, see if you can see any
difference. Perhaps we need to do something different to really let it
do its thing?

Honestly, though, I’d be happy for you to have a look as we’re rather
struggling. Using the offscreen bitmap means Emacs doesn’t have any
odd graphical glitches. The other method, described here

    https://debbugs.gnu.org/cgi/bugreport.cgi?bug=32932#412

does have serious glitches but has *much* better performance.
-- 
Alan Third

[-- Attachment #2: v3-0001-Use-CGLayer-instead-of-NSBitmapImageRep-bug-32932.patch --]
[-- Type: text/plain, Size: 8166 bytes --]

From 86017c66a8fef5abf091c957a6b9a40dfd276413 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 1 Feb 2020 21:17:29 +0000
Subject: [PATCH v3] Use CGLayer instead of NSBitmapImageRep (bug#32932)

---
 src/nsterm.h |  4 +--
 src/nsterm.m | 95 +++++++++++++++++++++++++---------------------------
 2 files changed, 48 insertions(+), 51 deletions(-)

diff --git a/src/nsterm.h b/src/nsterm.h
index 980ca534cf..eefa4d706d 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -418,7 +418,7 @@ #define NSTRACE_UNSILENCE()
    NSWindow *nonfs_window;
    BOOL fs_is_native;
 #ifdef NS_IMPL_COCOA
-   NSBitmapImageRep *drawingBuffer;
+   CGLayerRef drawingBuffer;
 #endif
 @public
    struct frame *emacsframe;
@@ -464,7 +464,7 @@ #define NSTRACE_UNSILENCE()
 - (void)focusOnDrawingBuffer;
 #endif
 - (void)copyRect:(NSRect)srcRect to:(NSRect)dstRect;
-- (void)createDrawingBufferWithRect:(NSRect)rect;
+- (void)createDrawingBuffer;
 
 /* Non-notification versions of NSView methods. Used for direct calls.  */
 - (void)windowWillEnterFullScreen;
diff --git a/src/nsterm.m b/src/nsterm.m
index 9d427b9b38..0f26db4d8a 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1141,7 +1141,6 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 
 #ifdef NS_IMPL_COCOA
   [NSGraphicsContext setCurrentContext:nil];
-  [view display];
 #else
   block_input ();
 
@@ -2853,7 +2852,9 @@ so some key presses (TAB) are swallowed by the system.  */
   ns_unfocus (f);
 
   /* as of 2006/11 or so this is now needed */
-  ns_redraw_scroll_bars (f);
+  /* FIXME: I don't see any reason for this and removing it makes no
+     difference here.  Do we need it for GNUstep?  */
+  //ns_redraw_scroll_bars (f);
   unblock_input ();
 }
 
@@ -3169,18 +3170,6 @@ so some key presses (TAB) are swallowed by the system.  */
 
       NSTRACE_RECT ("fromRect", fromRect);
 
-      /* Because we're drawing into an offscreen buffer which isn't
-         flipped, the images come out upside down.  To work around it
-         we need to do some fancy transforms.  */
-      {
-        NSAffineTransform *transform = [NSAffineTransform transform];
-        [transform translateXBy:0 yBy:NSMaxY(imageRect)];
-        [transform scaleXBy:1 yBy:-1];
-        [transform concat];
-
-        imageRect.origin.y = 0;
-      }
-
       [img drawInRect: imageRect
              fromRect: fromRect
             operation: NSCompositingOperationSourceOver
@@ -3938,11 +3927,6 @@ Function modeled after x_draw_glyph_string_box ().
 
       NSAffineTransform *doTransform = [NSAffineTransform transform];
 
-      /* We have to flip the image around the X axis as the offscreen
-         bitmap we're drawing to is flipped.  */
-      [doTransform scaleXBy:1 yBy:-1];
-      [doTransform translateXBy:0 yBy:-[img size].height];
-
       /* ImageMagick images don't have transforms.  */
       if (img->transform)
         [doTransform appendTransform:img->transform];
@@ -4838,7 +4822,7 @@ in certain situations (rapid incoming events).
   if (NILP (window->vertical_scroll_bar))
     {
       if (width > 0 && height > 0)
-	ns_clear_frame_area (f, left, top, width, height);
+        ns_clear_frame_area (f, left, top, width, height);
 
       bar = [[EmacsScroller alloc] initFrame: r window: win];
       wset_vertical_scroll_bar (window, make_mint_ptr (bar));
@@ -7104,7 +7088,7 @@ - (void) updateFrameSize: (BOOL) delay
          from non-native fullscreen, in other circumstances it appears
          to be a noop.  (bug#28872) */
       wr = NSMakeRect (0, 0, neww, newh);
-      [self createDrawingBufferWithRect:wr];
+      [self createDrawingBuffer];
       [view setFrame: wr];
 
       // To do: consider using [NSNotificationCenter postNotificationName:].
@@ -7444,7 +7428,8 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   maximizing_resize = NO;
 #endif
 
-  [self createDrawingBufferWithRect:r];
+  [self setCanDrawConcurrently:YES];
+  [self createDrawingBuffer];
 
   win = [[EmacsWindow alloc]
             initWithContentRect: r
@@ -8229,20 +8214,30 @@ - (instancetype)toggleToolbar: (id)sender
 }
 
 
-- (void)createDrawingBufferWithRect:(NSRect)rect
-  /* Create and store a new NSBitmapImageRep for Emacs to draw
-     into.
+- (void)createDrawingBuffer
+  /* Create and store a new CGLayer for Emacs to draw into.
 
-     Drawing to an offscreen bitmap doesn't work in GNUstep as there's
-     a bug in graphicsContextWithBitmapImageRep
-     (https://savannah.gnu.org/bugs/?38405).  So under GNUstep we
-     retain the old method of drawing direct to the EmacsView.  */
+     We can't do this in GNUstep as there's no CGLayer or equivalent.
+     So under GNUstep we retain the old method of drawing direct to
+     the EmacsView.  */
 {
 #ifdef NS_IMPL_COCOA
+  NSGraphicsContext *screen;
+  CGFloat scale = [[self window] backingScaleFactor];
+  NSRect frame = [self frame];
+
   if (drawingBuffer != nil)
-    [drawingBuffer release];
+    CGLayerRelease (drawingBuffer);
 
-  drawingBuffer = [[self bitmapImageRepForCachingDisplayInRect:rect] retain];
+  screen = [NSGraphicsContext graphicsContextWithBitmapImageRep:
+                    [self bitmapImageRepForCachingDisplayInRect:frame]];
+
+  drawingBuffer = CGLayerCreateWithContext ([screen CGContext],
+                                            CGSizeMake (NSWidth (frame) * scale,
+                                                        NSHeight (frame) * scale),
+                                            nil);
+
+  CGContextScaleCTM(CGLayerGetContext (drawingBuffer), scale, scale);
 #endif
 }
 
@@ -8250,11 +8245,12 @@ - (void)createDrawingBufferWithRect:(NSRect)rect
 #ifdef NS_IMPL_COCOA
 - (void)focusOnDrawingBuffer
 {
-  /* Creating the graphics context each time is very slow, but it
-     doesn't seem possible to cache and reuse it.  */
-  [NSGraphicsContext
-    setCurrentContext:
-      [NSGraphicsContext graphicsContextWithBitmapImageRep:drawingBuffer]];
+  CGContextRef cgctx = CGLayerGetContext (drawingBuffer);
+  NSGraphicsContext *buf =
+    [NSGraphicsContext
+        graphicsContextWithCGContext:cgctx flipped:YES];
+
+  [NSGraphicsContext setCurrentContext:buf];
 }
 
 
@@ -8269,7 +8265,7 @@ - (void)windowDidChangeBackingProperties:(NSNotification *)notification
    if (old != new)
      {
        NSRect frame = [self frame];
-       [self createDrawingBufferWithRect:frame];
+       [self createDrawingBuffer];
        ns_clear_frame (emacsframe);
        expose_frame (emacsframe, 0, 0, NSWidth (frame), NSHeight (frame));
      }
@@ -8284,13 +8280,18 @@ - (void)copyRect:(NSRect)srcRect to:(NSRect)dstRect
   NSTRACE_RECT ("Destination", dstRect);
 
 #ifdef NS_IMPL_COCOA
-  [drawingBuffer drawInRect:dstRect
-                   fromRect:srcRect
-                  operation:NSCompositingOperationCopy
-                   fraction:1.0
-             respectFlipped:NO
-                      hints:nil];
+  NSRect frame = [self frame];
+  CGRect offsetRect = CGRectMake (NSMinX (dstRect) - NSMinX (srcRect),
+                                  NSMinY (dstRect) - NSMinY (srcRect),
+                                  NSWidth (frame), NSHeight (frame));
+  [[NSGraphicsContext currentContext] saveGraphicsState];
 
+  NSRectClip (dstRect);
+
+  CGContextDrawLayerInRect ([[NSGraphicsContext currentContext] CGContext],
+                             offsetRect, drawingBuffer);
+
+  [[NSGraphicsContext currentContext] restoreGraphicsState];
   [self setNeedsDisplayInRect:dstRect];
 #else
   hide_bell();              // Ensure the bell image isn't scrolled.
@@ -8313,12 +8314,8 @@ - (void)drawRect: (NSRect)rect
     return;
 
 #ifdef NS_IMPL_COCOA
-  [drawingBuffer drawInRect:rect
-                   fromRect:rect
-                  operation:NSCompositingOperationSourceOver
-                   fraction:1
-             respectFlipped:NO
-                      hints:nil];
+  CGContextRef ctx = [[NSGraphicsContext currentContext] CGContext];
+  CGContextDrawLayerInRect (ctx, NSRectToCGRect ([self frame]), drawingBuffer);
 #else
   int x = NSMinX (rect), y = NSMinY (rect);
   int width = NSWidth (rect), height = NSHeight (rect);
-- 
2.24.0