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

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

[Daniel Pittman]

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

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.

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

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

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

[Alan Third]

[-- 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

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

[Daniel Pittman]

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

On Wed, Feb 5, 2020 at 6:10 PM Alan Third <alan@idiocy.org> wrote:

> On Wed, Feb 05, 2020 at 11:00:05AM -0500, Daniel Pittman wrote:
> > 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.
> 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.


OK.  I'll see about spending some time in the area, see if I can't improve
performance for me and hopefully everyone else.  Thanks for the pointers
and patches, which I'll play around with, and see what comes of testing
them, and additional profiling.

I can't promise a strict timeline, as this is a personal time project, but
I'm definitely motivated to improve performance here.  I'll keep in touch
via the list on progress.  Thank you for your quick response.

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

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

[Daniel Pittman]

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

On Wed, Feb 5, 2020 at 6:10 PM Alan Third <alan@idiocy.org> wrote:

> 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.
>

With a current build from HEAD I'm unable to reproduce the performance
issues I was experiencing.  I'll dig in if anything turns up, but it seems
like commit c4ca8219dd is performing just fine over here.

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