bug#32932: 27.0.50; render bugs on macOS Mojave

[Alan Third <alan@idiocy.org>]

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

On Mon, Feb 10, 2020 at 07:21:30AM -0800, Aaron Jensen wrote:
> On Sun, Feb 9, 2020 at 11:44 PM Alan Third <alan@idiocy.org> wrote:
> >
> > Anyway, one last thing to try. I was reading up on how MacVim handled
> > this same issue, and while their drawing techniques are quite
> > different, they did find performance problems with CGLayers so
> > switched to using CGImages. I’ve given that a go and I don’t think
> > this patch is quite as fast here as the CGLayer one, but it may be
> > faster for you.
> 
> 9.6s, which is around what 26 macport is, twice as slow as 27, but
> about 1/4 the time it took with previous versions of 28. It also
> doesn't seem to slow down too much (12.8) when I go full width. It's
> still usable, for sure.
> 
> No artifacts here as well, so I'd say this is a winner, at least for me.
> 
> Thank you for figuring something out.

I’m happy we’ve got to something usable at last!

New patch attached, it’s basically the same as the last one but
cleaned up and with a proper commit message. It’s not as fast as Emacs
26 and 27, but shouldn’t have any of the visual glitching we’ve seen
in that codebase and is hopefully Fast Enough. I don’t think anyone’s
likely to complain, but I’ll leave it a few days before I push to
master.

Thanks everyone for your help.
-- 
Alan Third

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

From 97fe485718df20a58a4d48937f78614fc432ced1 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 CGImage instead of NSBitmapImageRep (bug#32932)

* src/nsterm.m (ns_update_end):
(ns_clear_frame): Remove forced draws.
(ns_draw_fringe_bitmap):
(ns_dumpglyphs_image): No longer need to invert images as the context
is already flipped.
([EmacsView updateFrameSize:]):
([EmacsView initFrameFromEmacs:]): Use new function.
([EmacsView createDrawingBuffer]): Replaces createDrawingBufferWithRect:.
([EmacsView focusOnDrawingBuffer]): Set CGImage context.
([EmacsView windowDidChangeBackingProperties:]): Use new function.
([EmacsView copyRect:to:]): Copy using CGImages.
([EmacsView wantsUpdateLayer]):
([EmacsView updateLayer]): New Functions.
([EmacsView drawRect:]): We no longer do anything special here for
Cocoa.
([EmacsView windowDidChangeBackingProperties:]): Fix indentation and
add NSTRACE.
---
 src/nsterm.h |   4 +-
 src/nsterm.m | 152 +++++++++++++++++++++++++++++----------------------
 2 files changed, 90 insertions(+), 66 deletions(-)

diff --git a/src/nsterm.h b/src/nsterm.h
index 980ca534cf..7c6197f128 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;
+   CGContextRef 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..567f3b3794 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];
@@ -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,7 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   maximizing_resize = NO;
 #endif
 
-  [self createDrawingBufferWithRect:r];
+  [self createDrawingBuffer];
 
   win = [[EmacsWindow alloc]
             initWithContentRect: r
@@ -8229,52 +8213,65 @@ - (instancetype)toggleToolbar: (id)sender
 }
 
 
-- (void)createDrawingBufferWithRect:(NSRect)rect
-  /* Create and store a new NSBitmapImageRep for Emacs to draw
-     into.
+#ifdef NS_IMPL_COCOA
+- (void)createDrawingBuffer
+  /* Create and store a new CGGraphicsContext 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 equivalent, so under
+     GNUstep we retain the old method of drawing direct to the
+     EmacsView.  */
 {
-#ifdef NS_IMPL_COCOA
+  NSTRACE ("EmacsView createDrawingBuffer]");
+
+  NSGraphicsContext *screen;
+  CGColorSpaceRef colorSpace = [[[self window] colorSpace] CGColorSpace];
+  CGFloat scale = [[self window] backingScaleFactor];
+  NSRect frame = [self frame];
+
   if (drawingBuffer != nil)
-    [drawingBuffer release];
+    CGContextRelease (drawingBuffer);
 
-  drawingBuffer = [[self bitmapImageRepForCachingDisplayInRect:rect] retain];
-#endif
+  drawingBuffer = CGBitmapContextCreate (nil, NSWidth (frame) * scale, NSHeight (frame) * scale,
+                                         8, 0, colorSpace,
+                                         kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host);
+
+  /* This fixes the scale to match the backing scale factor, and flips the image.  */
+  CGContextTranslateCTM(drawingBuffer, 0, NSHeight (frame) * scale);
+  CGContextScaleCTM(drawingBuffer, scale, -scale);
 }
 
 
-#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]];
+  NSTRACE ("EmacsView focusOnDrawingBuffer]");
+
+  NSGraphicsContext *buf =
+    [NSGraphicsContext
+        graphicsContextWithCGContext:drawingBuffer flipped:YES];
+
+  [NSGraphicsContext setCurrentContext:buf];
 }
 
 
 - (void)windowDidChangeBackingProperties:(NSNotification *)notification
   /* Update the drawing buffer when the backing scale factor changes.  */
 {
-   CGFloat old = [[[notification userInfo]
+  NSTRACE ("EmacsView windowDidChangeBackingProperties:]");
+
+  CGFloat old = [[[notification userInfo]
                     objectForKey:@"NSBackingPropertyOldScaleFactorKey"]
-                   doubleValue];
-   CGFloat new = [[self window] backingScaleFactor];
+                  doubleValue];
+  CGFloat new = [[self window] backingScaleFactor];
 
-   if (old != new)
-     {
-       NSRect frame = [self frame];
-       [self createDrawingBufferWithRect:frame];
-       ns_clear_frame (emacsframe);
-       expose_frame (emacsframe, 0, 0, NSWidth (frame), NSHeight (frame));
-     }
+  if (old != new)
+    {
+      NSRect frame = [self frame];
+      [self createDrawingBuffer];
+      ns_clear_frame (emacsframe);
+      expose_frame (emacsframe, 0, 0, NSWidth (frame), NSHeight (frame));
+    }
 }
-#endif
+#endif /* NS_IMPL_COCOA */
 
 
 - (void)copyRect:(NSRect)srcRect to:(NSRect)dstRect
@@ -8284,13 +8281,31 @@ - (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];
+  CGImageRef copy;
+  NSRect frame = [self frame];
+  NSAffineTransform *setOrigin = [NSAffineTransform transform];
+
+  [[NSGraphicsContext currentContext] saveGraphicsState];
+
+  /* Set the clipping before messing with the buffer's
+     orientation.  */
+  NSRectClip (dstRect);
+
+  /* Unflip the buffer as the copied image will be unflipped, and
+     offset the top left so when we draw back into the buffer the
+     correct part of the image is drawn.  */
+  CGContextScaleCTM(drawingBuffer, 1, -1);
+  CGContextTranslateCTM(drawingBuffer, 0, -NSHeight (frame)
+                        - (NSMinY (dstRect) - NSMinY (srcRect)));
+
+  /* Take a copy of the buffer and then draw it back to the buffer,
+     limited by the clipping rectangle.  */
+  copy = CGBitmapContextCreateImage (drawingBuffer);
+  CGContextDrawImage (drawingBuffer, frame, copy);
+
+  CGImageRelease (copy);
 
+  [[NSGraphicsContext currentContext] restoreGraphicsState];
   [self setNeedsDisplayInRect:dstRect];
 #else
   hide_bell();              // Ensure the bell image isn't scrolled.
@@ -8304,6 +8319,24 @@ - (void)copyRect:(NSRect)srcRect to:(NSRect)dstRect
 }
 
 
+#ifdef NS_IMPL_COCOA
+- (BOOL)wantsUpdateLayer
+{
+    return YES;
+}
+
+
+- (void)updateLayer
+{
+  NSTRACE ("EmacsView updateLayer]");
+
+  CGImageRef contentsImage = CGBitmapContextCreateImage(drawingBuffer);
+  self.layer.contents = (id)contentsImage;
+  CGImageRelease(contentsImage);
+}
+#endif
+
+
 - (void)drawRect: (NSRect)rect
 {
   NSTRACE ("[EmacsView drawRect:" NSTRACE_FMT_RECT "]",
@@ -8312,14 +8345,6 @@ - (void)drawRect: (NSRect)rect
   if (!emacsframe || !emacsframe->output_data.ns)
     return;
 
-#ifdef NS_IMPL_COCOA
-  [drawingBuffer drawInRect:rect
-                   fromRect:rect
-                  operation:NSCompositingOperationSourceOver
-                   fraction:1
-             respectFlipped:NO
-                      hints:nil];
-#else
   int x = NSMinX (rect), y = NSMinY (rect);
   int width = NSWidth (rect), height = NSHeight (rect);
 
@@ -8327,7 +8352,6 @@ - (void)drawRect: (NSRect)rect
   block_input ();
   expose_frame (emacsframe, x, y, width, height);
   unblock_input ();
-#endif
 }
 
 
-- 
2.24.0