Subject: Re: bug#42406: Mouse-wheel scrolling can be flickering

From: Alan Third <[email-masked]>
Date: Wed, 16 Dec 2020 21:31:53 +0000

On Tue, Dec 15, 2020 at 03:06:03PM +0100, Konrad Podczeck wrote:
> Just to clarify, despite of the title of this bug, it is not a
> flickering, but the mouse-wheel scrolling preformance decreases with
> the number of open frames, in the sense that it becomes more and
> more sticky.

I don't think the attached patch will fix this, but it may improve
things a little. Can you please try it?

-- 
Alan Third
>From d6951394d803c36c9dafc8c941b7b40033851ec4 Mon Sep 17 00:00:00 2001
From: Alan Third <[email-masked]>
Date: Wed, 16 Dec 2020 21:12:04 +0000
Subject: [PATCH] Improve drawing performance on macOS

* configure.ac: Require IOSurface framework.
* src/nsterm.h: Add new definitions.
* src/nsterm.m (ns_update_end):
(ns_unfocus): Use new unfocusDrawingBuffer method.
(ns_draw_window_cursor): Move ns_focus to before we set colors.
([EmacsView dealloc]): Release the new IOSurface.
([EmacsView createDrawingBuffer]): Use a new IOSurface to draw to
and point a CGBitmapContext to it.
([EmacsView focusOnDrawingBuffer]): Lock the IOSurface for drawing.
([EmacsView unfocusDrawingBuffer]): New function.
([EmacsView updateLayer]): Removed as no longer needed.
---
 configure.ac |  2 +-
 src/nsterm.h | 13 +++++++++
 src/nsterm.m | 75 +++++++++++++++++++++++++++++++++++++++-------------
 3 files changed, 70 insertions(+), 20 deletions(-)

diff --git a/configure.ac b/configure.ac
index 888b415148..88afd7348e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -5491,7 +5491,7 @@ AC_DEFUN
    if test "$HAVE_NS" = "yes"; then
      libs_nsgui="-framework AppKit"
      if test "$NS_IMPL_COCOA" = "yes"; then
-        libs_nsgui="$libs_nsgui -framework IOKit -framework Carbon"
+        libs_nsgui="$libs_nsgui -framework IOKit -framework Carbon -framework IOSurface"
      fi
    else
      libs_nsgui=
diff --git a/src/nsterm.h b/src/nsterm.h
index f292993d8f..6c456d3724 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -435,6 +435,7 @@ #define NS_DRAW_TO_BUFFER 1
    BOOL fs_is_native;
    BOOL in_fullscreen_transition;
 #ifdef NS_DRAW_TO_BUFFER
+   IOSurfaceRef surface;
    CGContextRef drawingBuffer;
 #endif
 @public
@@ -478,6 +479,7 @@ #define NS_DRAW_TO_BUFFER 1
 
 #ifdef NS_DRAW_TO_BUFFER
 - (void)focusOnDrawingBuffer;
+- (void)unfocusDrawingBuffer;
 - (void)createDrawingBuffer;
 #endif
 - (void)copyRect:(NSRect)srcRect to:(NSRect)dstRect;
@@ -729,6 +731,17 @@ #define NS_DRAW_TO_BUFFER 1
 @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 7972fa4dab..8241630a50 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -72,6 +72,10 @@ Updated by Christian Limpach ([email-masked])
 #include <Carbon/Carbon.h>
 #endif
 
+#ifdef NS_DRAW_TO_BUFFER
+#include <IOSurface/IOSurface.h>
+#endif
+
 static EmacsMenu *dockMenu;
 #ifdef NS_IMPL_COCOA
 static EmacsMenu *mainMenu;
@@ -1165,7 +1169,7 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
   if ([FRAME_NS_VIEW (f) wantsUpdateLayer])
     {
 #endif
-      [NSGraphicsContext setCurrentContext:nil];
+      [FRAME_NS_VIEW (f) unfocusDrawingBuffer];
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 101400
     }
   else
@@ -1273,6 +1277,8 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
   if ([FRAME_NS_VIEW (f) wantsUpdateLayer])
     {
 #endif
+      if (! ns_updating_frame)
+        [FRAME_NS_VIEW (f) unfocusDrawingBuffer];
       [FRAME_NS_VIEW (f) setNeedsDisplay:YES];
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 101400
     }
@@ -3404,6 +3410,8 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
   /* Prevent the cursor from being drawn outside the text area.  */
   r = NSIntersectionRect (r, ns_row_rect (w, glyph_row, TEXT_AREA));
 
+  ns_focus (f, &r, 1);
+
   face = FACE_FROM_ID_OR_NULL (f, phys_cursor_glyph->face_id);
   if (face && NS_FACE_BACKGROUND (face)
       == ns_index_color (FRAME_CURSOR_COLOR (f), f))
@@ -3414,8 +3422,6 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
   else
     [FRAME_CURSOR_COLOR (f) set];
 
-  ns_focus (f, &r, 1);
-
   switch (cursor_type)
     {
     case DEFAULT_CURSOR:
@@ -6369,6 +6375,7 @@ - (void)dealloc
 
 #ifdef NS_DRAW_TO_BUFFER
   CGContextRelease (drawingBuffer);
+  CFRelease (surface);
 #endif
 
   [toolbar release];
@@ -8427,23 +8434,49 @@ - (void)createDrawingBuffer
   CGColorSpaceRef colorSpace = [[[self window] colorSpace] CGColorSpace];
   CGFloat scale = [[self window] backingScaleFactor];
   NSRect frame = [self frame];
+  int width, height, bytesPerRow;
 
   if (drawingBuffer != nil)
-    CGContextRelease (drawingBuffer);
+    {
+      CGContextRelease (drawingBuffer);
+      CFRelease (surface);
+    }
+
+  width = NSWidth (frame) * scale;
+  height = NSHeight (frame) * scale;
+  bytesPerRow = IOSurfaceAlignProperty (kIOSurfaceBytesPerRow, width * 4);
 
-  drawingBuffer = CGBitmapContextCreate (nil, NSWidth (frame) * scale, NSHeight (frame) * scale,
-                                         8, 0, colorSpace,
-                                         kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host);
+  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, NSHeight (frame) * scale);
+  CGContextTranslateCTM(drawingBuffer, 0, IOSurfaceGetHeight (surface));
   CGContextScaleCTM(drawingBuffer, scale, -scale);
+
+  [[self layer] setContents:(id)surface];
 }

 - (void)focusOnDrawingBuffer
 {
-  NSTRACE ("EmacsView focusOnDrawingBuffer]");
+  IOReturn lockStatus;
+
+  NSTRACE ("[EmacsView focusOnDrawingBuffer]");
+
+  if ((lockStatus = IOSurfaceLock (surface, 0, nil)) != kIOReturnSuccess)
+    NSLog (@"Failed to lock surface: %x", lockStatus);
 
   NSGraphicsContext *buf =
     [NSGraphicsContext
@@ -8453,6 +8486,20 @@ - (void)focusOnDrawingBuffer
 }

+- (void)unfocusDrawingBuffer
+{
+  IOReturn lockStatus;
+
+  NSTRACE ("[EmacsView unfocusDrawingBuffer]");
+
+  [NSGraphicsContext setCurrentContext:nil];
+  if ((lockStatus = IOSurfaceUnlock (surface, 0, nil)) != kIOReturnSuccess)
+    NSLog (@"Failed to unlock surface: %x", lockStatus);
+
+  [[self layer] setContentsChanged];
+}
+
+
 - (void)windowDidChangeBackingProperties:(NSNotification *)notification
   /* Update the drawing buffer when the backing properties change.  */
 {
@@ -8541,16 +8588,6 @@ - (BOOL)wantsUpdateLayer
   /* Running on macOS 10.14 or above.  */
   return YES;
 }
-
-
-- (void)updateLayer
-{
-  NSTRACE ("[EmacsView updateLayer]");
-
-  CGImageRef contentsImage = CGBitmapContextCreateImage(drawingBuffer);
-  [[self layer] setContents:(id)contentsImage];
-  CGImageRelease(contentsImage);
-}
 #endif

-- 
2.29.2