Merge pull request #757 from s4y/fix-the-flicker

Fix rendering issues when built against the 10.14 SDK.

runtime/doc/gui_mac.txt

@@ -715,6 +715,9 @@ to use in normal mode and type ":set imd" followed by ":set noimd".
 This list is by no means exhaustive, it only enumerates some of the more
 prominent bugs/missing features.
 
+- Under macOS Mojave (10.14), the default renderer (Core Text renderer) has
+  some performance issues and scrolling is not as smooth as previous macOS
+  versions (10.13 or below).
 - Localized menus are not supported.  Choosing anything but "English" in the
   "International" pane of "System Prefences" may break the menus (and
   toolbar).

src/MacVim/MMAppController.m

@@ -248,6 +248,7 @@ fsEventCallback(ConstFSEventStreamRef streamRef,
         [NSNumber numberWithBool:YES],    MMNativeFullScreenKey,
         [NSNumber numberWithDouble:0.25], MMFullScreenFadeTimeKey,
         [NSNumber numberWithBool:NO],     MMUseCGLayerAlwaysKey,
+        @(shouldUseBufferedDrawing()),    MMBufferedDrawingKey,
         [NSNumber numberWithBool:YES],    MMShareFindPboardKey,
         nil];
 

src/MacVim/MMCoreTextView.h

@@ -41,6 +41,27 @@
     CGPoint                     *positions;
     NSMutableArray              *fontCache;
 
+    // Issue draws onto an CGImage that caches the drawn results instead of
+    // directly in drawRect:. This is the default behavior in cases where simply
+    // drawing incrementally in drawRect: doesn't work. Those cases are:
+    // 1. Non-native fullscreen
+    // 2. 10.14+ (views are always layer-backed which means the view buffer will
+    //    be cleared and we can't incrementally draw in drawRect:)
+    //    
+    // This can be configured by setting MMBufferedDrawingKey in user defaults.
+    BOOL                        cgBufferDrawEnabled;
+    BOOL                        cgBufferDrawNeedsUpdateContext;
+    CGContextRef                cgContext;
+    
+    // *Deprecated*
+    // Draw onto a CGLayer instead of lazily updating the view's buffer in
+    // drawRect: which is error-prone and relying on undocumented behaviors
+    // (that the OS will preserve the old buffer).
+    //
+    // This is deprecated. Use cgBufferDrawEnabled instead which is more
+    // efficient.
+    //    
+    // This can be configured by setting MMUseCGLayerAlwaysKey in user defaults.
     BOOL                        cgLayerEnabled;
     CGLayerRef                  cgLayer;
     CGContextRef                cgLayerContext;

src/MacVim/MMCoreTextView.m

@@ -132,6 +132,10 @@ defaultAdvanceForFont(NSFont *font)
 {
     if (!(self = [super initWithFrame:frame]))
         return nil;
+    
+    cgBufferDrawEnabled = [[NSUserDefaults standardUserDefaults]
+            boolForKey:MMBufferedDrawingKey];
+    cgBufferDrawNeedsUpdateContext = NO;
 
     cgLayerEnabled = [[NSUserDefaults standardUserDefaults]
             boolForKey:MMUseCGLayerAlwaysKey];
@@ -168,6 +172,8 @@ defaultAdvanceForFont(NSFont *font)
     [drawData release];  drawData = nil;
     [fontCache release];  fontCache = nil;
 
+    CGContextRelease(cgContext);  cgContext = nil;
+
     [helper setTextView:nil];
     [helper release];  helper = nil;
 
@@ -215,6 +221,7 @@ defaultAdvanceForFont(NSFont *font)
         [defaultForegroundColor release];
         defaultForegroundColor = fgColor ? [fgColor retain] : nil;
     }
+    [self setNeedsDisplay:YES];
 }
 
 - (NSColor *)defaultBackgroundColor
@@ -444,13 +451,33 @@ defaultAdvanceForFont(NSFont *font)
 }
 
 - (void)setFrameSize:(NSSize)newSize {
-    if (!drawPending && !NSEqualSizes(newSize, self.frame.size) && drawData.count == 0) {
-        [NSAnimationContext beginGrouping];
-        drawPending = YES;
+    if (!NSEqualSizes(newSize, self.bounds.size)) {
+        if (!drawPending && !cgBufferDrawEnabled) {
+            // When resizing a window, it will invalidate the buffer and cause
+            // MacVim to draw black until we get the draw commands from Vim and
+            // we draw them out in drawRect. Use beginGrouping to stop the
+            // window resize from happening until we get the draw calls.
+            //
+            // The updateLayer/cgBufferDrawEnabled path handles this differently
+            // and don't need this.
+            [NSAnimationContext beginGrouping];
+            drawPending = YES;
+        }
+        if (cgBufferDrawEnabled) {
+            cgBufferDrawNeedsUpdateContext = YES;
+        }
     }
+    
     [super setFrameSize:newSize];
 }
 
+- (void)viewDidChangeBackingProperties {
+    if (cgBufferDrawEnabled) {
+        cgBufferDrawNeedsUpdateContext = YES;
+    }
+    [super viewDidChangeBackingProperties];
+}
+
 - (void)keyDown:(NSEvent *)event
 {
     [helper keyDown:event];
@@ -606,6 +633,87 @@ defaultAdvanceForFont(NSFont *font)
     return NO;
 }
 
+- (void)updateCGContext {
+    if (cgContext) {
+        CGContextRelease(cgContext);
+        cgContext = nil;
+    }
+
+    NSRect backingRect = [self convertRectToBacking:self.bounds];
+    cgContext = CGBitmapContextCreate(NULL, NSWidth(backingRect), NSHeight(backingRect), 8, 0, self.window.colorSpace.CGColorSpace, kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host);
+    CGContextScaleCTM(cgContext, self.window.backingScaleFactor, self.window.backingScaleFactor);
+    
+    cgBufferDrawNeedsUpdateContext = NO;
+}
+
+- (BOOL)wantsUpdateLayer {
+    return cgBufferDrawEnabled;
+}
+
+- (void)updateLayer {
+    if (!cgContext) {
+        [self updateCGContext];
+    } else if (cgBufferDrawNeedsUpdateContext) {
+        if ([drawData count] != 0) {
+            [self updateCGContext];
+        } else {
+            // In this case, we don't have a single draw command, meaning that
+            // Vim hasn't caught up yet and hasn't issued draw commands. We
+            // don't want to use [NSAnimationContext beginGrouping] as it's
+            // fragile (we may miss the endGrouping call due to order of
+            // operation), and also it makes the animation jerky.
+            // Instead, copy the image to the new context and align it to the
+            // top left and make sure it doesn't stretch. This makes the
+            // resizing smooth while Vim tries to catch up in issuing draws.
+            CGImageRef oldImage = CGBitmapContextCreateImage(cgContext);
+
+            [self updateCGContext]; // This will make a new cgContext
+            
+            CGContextSaveGState(cgContext);
+            CGContextSetBlendMode(cgContext, kCGBlendModeCopy);
+            
+            // Filling the background so the edge won't be black.
+            NSRect newRect = [self bounds];
+            float r = [defaultBackgroundColor redComponent];
+            float g = [defaultBackgroundColor greenComponent];
+            float b = [defaultBackgroundColor blueComponent];
+            float a = [defaultBackgroundColor alphaComponent];
+            CGContextSetRGBFillColor(cgContext, r, g, b, a);
+            CGContextFillRect(cgContext, *(CGRect*)&newRect);
+            CGContextSetBlendMode(cgContext, kCGBlendModeNormal);
+
+            // Copy the old image over to the new image, and make sure to
+            // respect scaling and remember that CGImage's Y origin is
+            // bottom-left.
+            CGFloat scale = self.window.backingScaleFactor;
+            size_t oldWidth = CGImageGetWidth(oldImage) / scale;
+            size_t oldHeight = CGImageGetHeight(oldImage) / scale;
+            CGFloat newHeight = newRect.size.height;
+            NSRect imageRect = NSMakeRect(0, newHeight - oldHeight, (CGFloat)oldWidth, (CGFloat)oldHeight);
+
+            CGContextDrawImage(cgContext, imageRect, oldImage);
+            CGImageRelease(oldImage);
+            CGContextRestoreGState(cgContext);
+        }
+    }
+    
+    // Now issue the batched draw commands
+    if ([drawData count] != 0) {
+        [NSGraphicsContext saveGraphicsState];
+        NSGraphicsContext.currentContext = [NSGraphicsContext graphicsContextWithCGContext:cgContext flipped:self.flipped];
+        id data;
+        NSEnumerator *e = [drawData objectEnumerator];
+        while ((data = [e nextObject]))
+            [self batchDrawData:data];
+        [drawData removeAllObjects];
+        [NSGraphicsContext restoreGraphicsState];
+    }
+
+    CGImageRef contentsImage = CGBitmapContextCreateImage(cgContext);
+    self.layer.contents = (id)contentsImage;
+    CGImageRelease(contentsImage);
+}
+
 - (void)drawRect:(NSRect)rect
 {
     NSGraphicsContext *context = [NSGraphicsContext currentContext];
@@ -653,7 +761,25 @@ defaultAdvanceForFont(NSFont *font)
 
 - (void)performBatchDrawWithData:(NSData *)data
 {
-    if (cgLayerEnabled && drawData.count == 0 && [self getCGContext]) {
+    if (cgBufferDrawEnabled) {
+        // We batch up all the commands and actually perform the draw at
+        // updateLayer. The reason is that right now MacVim has a lot of
+        // different paths that could change the view size (zoom, user resizing
+        // from either dragging border or another program, Cmd-+/- to change
+        // font size, fullscreen, etc). Those different paths don't currently
+        // have a consistent order of operation of (Vim or MacVim go first), so
+        // sometimes Vim gets updated and issue a batch draw first, but
+        // sometimes MacVim gets notified first (e.g. when window is resized).
+        // If frame size has changed we need to call updateCGContext but we
+        // can't do it here because of the order of operation issue. That's why
+        // we wait till updateLayer to do it where everything has already been
+        // done and settled.
+        //
+        // Note: Should probably refactor the different ways window size could
+        // be changed and unify them instead of the status quo of spaghetti.
+        [drawData addObject:data];
+        [self setNeedsDisplay:YES];
+    } else if (cgLayerEnabled && drawData.count == 0 && [self getCGContext]) {
         [cgLayerLock lock];
         [self batchDrawData:data];
         [cgLayerLock unlock];
@@ -669,6 +795,9 @@ defaultAdvanceForFont(NSFont *font)
 
 - (void)setCGLayerEnabled:(BOOL)enabled
 {
+    if (cgContext)
+        return;
+
     cgLayerEnabled = enabled;
 
     if (!cgLayerEnabled)
@@ -1491,7 +1620,18 @@ recurseDraw(const unichar *chars, CGGlyph *glyphs, CGPoint *positions,
 
 - (void)scrollRect:(NSRect)rect lineCount:(int)count
 {
-    if (cgLayerEnabled) {
+    if (cgContext) {
+        NSRect fromRect = NSOffsetRect(self.bounds, 0, -count * cellSize.height);
+        NSRect toRect = NSOffsetRect(rect, 0, -count * cellSize.height);
+        CGContextSaveGState(cgContext);
+        CGContextClipToRect(cgContext, toRect);
+        CGContextSetBlendMode(cgContext, kCGBlendModeCopy);
+        CGImageRef contentsImage = CGBitmapContextCreateImage(cgContext);
+        CGContextDrawImage(cgContext, fromRect, contentsImage);
+        CGImageRelease(contentsImage);
+        CGContextRestoreGState(cgContext);
+        [self setNeedsDisplayCGLayerInRect:toRect];
+    } else if (cgLayerEnabled) {
         CGContextRef context = [self getCGContext];
         int yOffset = count * cellSize.height;
         NSRect clipRect = rect;

src/MacVim/MMVimView.m

@@ -189,7 +189,7 @@ enum {
 
 - (BOOL)isOpaque
 {
-    return YES;
+    return textView.defaultBackgroundColor.alphaComponent == 1;
 }
 
 - (void)drawRect:(NSRect)rect
@@ -511,6 +511,7 @@ enum {
         MMScroller *sb = [scrollbars objectAtIndex:i];
         [sb setNeedsDisplay:YES];
     }
+    [self setNeedsDisplay:YES];
 }
 
 

src/MacVim/MMWindowController.m

@@ -543,6 +543,7 @@
     [decoratedWindow setOpaque:isOpaque];
     if (fullScreenWindow)
         [fullScreenWindow setOpaque:isOpaque];
+    [decoratedWindow setBackgroundColor:back];
 
     [vimView setDefaultColorsBackground:back foreground:fore];
 }
@@ -1543,11 +1544,17 @@
     BOOL windowTextured = ([decoratedWindow styleMask] &
                             NSWindowStyleMaskTexturedBackground) != 0;
     BOOL hideSeparator  = NO;
-
-    if (fullScreenEnabled || tabBarVisible)
+    
+    if (floor(NSAppKitVersionNumber) >= NSAppKitVersionNumber10_10) {
+        // The tabline separator is mostly an old feature and not necessary
+        // modern macOS versions.
         hideSeparator = YES;
-    else
-        hideSeparator = toolbarHidden && !windowTextured;
+    } else {
+        if (fullScreenEnabled || tabBarVisible)
+            hideSeparator = YES;
+        else
+            hideSeparator = toolbarHidden && !windowTextured;
+    }
 
     [self hideTablineSeparator:hideSeparator];
 }

src/MacVim/MacVim.h

@@ -45,6 +45,9 @@
 #ifndef NSAppKitVersionNumber10_12
 # define NSAppKitVersionNumber10_12 1504
 #endif
+#ifndef NSAppKitVersionNumber10_13
+# define NSAppKitVersionNumber10_13 1561
+#endif
 
 #if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_12
 // Deprecated constants in 10.12 SDK

src/MacVim/Miscellaneous.h

@@ -53,6 +53,7 @@ extern NSString *MMNativeFullScreenKey;
 extern NSString *MMUseMouseTimeKey;
 extern NSString *MMFullScreenFadeTimeKey;
 extern NSString *MMUseCGLayerAlwaysKey;
+extern NSString *MMBufferedDrawingKey;
 
 
 // Enum for MMUntitledWindowKey
@@ -156,3 +157,4 @@ NSArray *normalizeFilenames(NSArray *filenames);
 
 BOOL shouldUseYosemiteTabBarStyle();
 BOOL shouldUseMojaveTabBarStyle();
+BOOL shouldUseBufferedDrawing();

src/MacVim/Miscellaneous.m

@@ -49,6 +49,7 @@ NSString *MMNativeFullScreenKey         = @"MMNativeFullScreen";
 NSString *MMUseMouseTimeKey             = @"MMUseMouseTime";
 NSString *MMFullScreenFadeTimeKey       = @"MMFullScreenFadeTime";
 NSString *MMUseCGLayerAlwaysKey         = @"MMUseCGLayerAlways";
+NSString *MMBufferedDrawingKey          = @"MMBufferedDrawing";
 
 
 
@@ -316,3 +317,14 @@ shouldUseMojaveTabBarStyle()
 #endif
     return false;
 }
+
+BOOL
+shouldUseBufferedDrawing()
+{
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_14
+    if (@available(macos 10.14, *)) {
+        return YES;
+    }
+#endif
+    return NO;
+}