Merge pull request #1547 from ychin/font-change-newtab-no-flicker

Reduce flicker when changing fonts/adding tabs in go+=k/fullscreen

Author: ychin

src/MacVim/MMBackend.m

@@ -2206,10 +2206,8 @@ - (void)handleInputEvent:(int)msgid data:(NSData *)data
         const void *bytes = [data bytes];
         int idx = *((int*)bytes) + 1;
         send_tabline_menu_event(idx, TABLINE_MENU_CLOSE);
-        [self redrawScreen];
     } else if (AddNewTabMsgID == msgid) {
         send_tabline_menu_event(0, TABLINE_MENU_NEW);
-        [self redrawScreen];
     } else if (DraggedTabMsgID == msgid) {
         if (!data) return;
         const void *bytes = [data bytes];
@@ -2259,8 +2257,6 @@ - (void)handleInputEvent:(int)msgid data:(NSData *)data
         [self queueMessage:msgid data:d];
 
         gui_resize_shell(cols, rows);
-    } else if (ResizeViewMsgID == msgid) {
-        [self queueMessage:msgid data:data];
     } else if (ExecuteMenuMsgID == msgid) {
         NSDictionary *attrs = [NSDictionary dictionaryWithData:data];
         if (attrs) {
@@ -2332,7 +2328,7 @@ - (void)handleInputEvent:(int)msgid data:(NSData *)data
         [self setImState:YES];
     } else if (DeactivatedImMsgID == msgid) {
         [self setImState:NO];
-    } else if (BackingPropertiesChangedMsgID == msgid) {
+    } else if (RedrawMsgID == msgid) {
         [self redrawScreen];
     } else if (LoopBackMsgID == msgid) {
         // This is a debug message used for confirming a message has been
@@ -2725,8 +2721,6 @@ - (void)handleSetFont:(NSData *)data
         CONVERT_FROM_UTF8_FREE(ws);
     }
     CONVERT_FROM_UTF8_FREE(s);
-
-    [self redrawScreen];
 }
 
 - (void)handleDropFiles:(NSData *)data

src/MacVim/MMCoreTextView.h

@@ -71,17 +71,15 @@ NS_ASSUME_NONNULL_BEGIN
 
     MMTextViewHelper            *helper;
 
-    NSMutableDictionary<NSNumber *, NSFont *> *fontVariants;
-    NSMutableSet<NSString *> *characterStrings;
-    NSMutableDictionary<NSNumber *,NSCache<NSString *,id> *> *characterLines;
-    
     // These are used in MMCoreTextView+ToolTip.m
     id trackingRectOwner_;              // (not retained)
     void *trackingRectUserData_;
     NSTrackingRectTag lastToolTipTag_;
     NSString* toolTip_;
 }
 
+@property (nonatomic) NSSize drawRectOffset; ///< A render offset to apply to the draw rects. This is currently only used in specific situations when rendering is blocked.
+
 - (instancetype)initWithFrame:(NSRect)frame;
 
 //

src/MacVim/MMCoreTextView.m

@@ -222,6 +222,10 @@ static void grid_free(Grid *grid) {
 
 @implementation MMCoreTextView {
     Grid grid;
+    NSMutableSet<NSString *> *characterStrings; ///< Storage for characters in the grid
+
+    NSMutableDictionary<NSNumber *, NSFont *> *fontVariants; ///< Cache for fonts used for each variant (e.g. italic)
+    NSMutableDictionary<NSNumber *,NSCache<NSString *,id> *> *characterLines; ///< Cache for built CTLine objects
 
     BOOL alignCmdLineToBottom; ///< Whether to pin the Vim command-line to the bottom of the window
     int cmdlineRow; ///< Row number (0-indexed) where the cmdline starts. Used for pinning it to the bottom if desired.
@@ -251,8 +255,9 @@ - (instancetype)initWithFrame:(NSRect)frame
     antialias = YES;
 
     [self setFont:[NSFont userFixedPitchFontOfSize:0]];
-    fontVariants = [[NSMutableDictionary alloc] init];
     characterStrings = [[NSMutableSet alloc] init];
+
+    fontVariants = [[NSMutableDictionary alloc] init];
     characterLines = [[NSMutableDictionary alloc] init];
 
     helper = [[MMTextViewHelper alloc] init];
@@ -276,8 +281,8 @@ - (void)dealloc
     [fontWide release];  fontWide = nil;
     [defaultBackgroundColor release];  defaultBackgroundColor = nil;
     [defaultForegroundColor release];  defaultForegroundColor = nil;
-    [fontVariants release];  fontVariants = nil;
     [characterStrings release];  characterStrings = nil;
+    [fontVariants release];  fontVariants = nil;
     [characterLines release];  characterLines = nil;
 
     [helper setTextView:nil];
@@ -478,9 +483,7 @@ - (void)setFont:(NSFont *)newFont
     cellSize.width = columnspace + ceil(em * cellWidthMultiplier);
     cellSize.height = linespace + defaultLineHeightForFont(font);
 
-    [self clearAll];
     [fontVariants removeAllObjects];
-    [characterStrings removeAllObjects];
     [characterLines removeAllObjects];
 }
 
@@ -498,9 +501,7 @@ - (void)setWideFont:(NSFont *)newFont
         fontWide = [newFont retain];
     }
 
-    [self clearAll];
     [fontVariants removeAllObjects];
-    [characterStrings removeAllObjects];
     [characterLines removeAllObjects];
 }
 
@@ -1485,6 +1486,9 @@ - (NSRect)rectForRow:(int)row column:(int)col numRows:(int)nr
     rect.size.width = nc*cellSize.width;
     rect.size.height = nr*cellSize.height;
 
+    rect.origin.x += _drawRectOffset.width;
+    rect.origin.y += _drawRectOffset.height;
+
     // Under smooth resizing, full screen, or guioption-k; we frequently have a frame size that's not
     // aligned with the exact grid size. If the user has 'cursorline' set, or the color scheme uses
     // the NonText highlight group, this will leave a small gap on the right filled with bg color looking
@@ -1505,13 +1509,15 @@ - (NSRect)rectForRow:(int)row column:(int)col numRows:(int)nr
         const CGFloat gapHeight = frame.size.height - grid.rows*cellSize.height - insetSize.height - insetBottom;
         if (row >= cmdlineRow) {
             rect.origin.y -= gapHeight;
+            rect.origin.y -= _drawRectOffset.height; // Pinning ignores draw rect offset for stability
         } else if (row + nr - 1 >= cmdlineRow) {
             // This is an odd case where the gap between cmdline and the top-aligned content is inside
             // the rect so we need to adjust the height as well. During rendering we draw line-by-line
             // so this shouldn't cause any issues as we only encounter this situation when calculating
             // the rect in setNeedsDisplayFromRow:.
             rect.size.height += gapHeight;
             rect.origin.y -= gapHeight;
+            rect.origin.y -= _drawRectOffset.height; // Pinning ignores draw rect offset for stability
         }
     }
     return rect;

src/MacVim/MMTextView.h

@@ -31,6 +31,8 @@
     MMTextViewHelper    *helper;
 }
 
+@property (nonatomic) NSSize drawRectOffset; // Unused. Only used by MMCoreTextView
+
 - (id)initWithFrame:(NSRect)frame;
 
 - (void)setPreEditRow:(int)row column:(int)col;

src/MacVim/MMVimController.m

@@ -697,6 +697,11 @@ - (void)handleMessage:(int)msgid data:(NSData *)data
         break;
         case BatchDrawMsgID:
         {
+            if ([windowController isRenderBlocked]) {
+                // Drop all batch draw commands while blocked. If we end up
+                // changing out mind later we will need to ask Vim to redraw.
+                break;
+            }
             [[[windowController vimView] textView] performBatchDrawWithData:data];
         }
         break;
@@ -717,13 +722,11 @@ - (void)handleMessage:(int)msgid data:(NSData *)data
         case ShowTabBarMsgID:
         {
             [windowController showTabline:YES];
-            [self sendMessage:BackingPropertiesChangedMsgID data:nil];
         }
         break;
         case HideTabBarMsgID:
         {
             [windowController showTabline:NO];
-            [self sendMessage:BackingPropertiesChangedMsgID data:nil];
         }
         break;
 
@@ -753,7 +756,13 @@ - (void)handleMessage:(int)msgid data:(NSData *)data
 
         case ResizeViewMsgID:
         {
-            [windowController resizeView];
+            // This is sent when Vim wants MacVim to resize Vim to fit
+            // everything within the GUI window, usually because go+=k is set.
+            // Other gVim usually blocks on this but for MacVim it is async
+            // to reduce synchronization points so we schedule a resize for
+            // later. We ask to block any render from happening until we are
+            // done resizing to avoid a momentary annoying flicker.
+            [windowController resizeVimViewBlockRender];
         }
         break;
         case SetWindowTitleMsgID:

src/MacVim/MMVimView.m

@@ -914,16 +914,7 @@ - (void)frameSizeMayHaveChanged:(BOOL)keepGUISize
     [textView setFrame:textViewRect];
 
     // Immediately place the scrollbars instead of deferring till later here.
-    // Deferral ended up causing some bugs, in particular when in <10.14
-    // CoreText renderer where [NSAnimationContext beginGrouping] is used to
-    // bundle state changes together and the deferred placeScrollbars would get
-    // the wrong data to use. An alternative would be to check for that and only
-    // call finishPlaceScrollbars once we call [NSAnimationContext endGrouping]
-    // but that makes the code mode complicated. Just do it here and the
-    // performance is fine as this gets called occasionally only
-    // (pendingPlaceScrollbars is mostly for the case if we are adding a lot of
-    // scrollbars at once we want to only call placeScrollbars once instead of
-    // doing it N times).
+    // Otherwise in situations like live resize we will see the scroll bars lag.
     self.pendingPlaceScrollbars = NO;
     [self placeScrollbars];
 

src/MacVim/MMWindowController.h

@@ -28,18 +28,22 @@
     MMVimView           *vimView;
     BOOL                setupDone;
     BOOL                windowPresented;
-    BOOL                shouldResizeVimView;
-    BOOL                shouldKeepGUISize;
+
+    BOOL                shouldResizeVimView; ///< Indicates there is a pending command to resize the Vim view
+    BOOL                shouldKeepGUISize; ///< If on, the Vim view resize will try to fit in the existing window. If off, the window resizes to fit Vim view.
+    BOOL                blockRenderUntilResize; ///< Indicates that there should be no text rendering until a Vim view resize is completed to avoid flicker.
+
     BOOL                shouldRestoreUserTopLeft;
-    BOOL                shouldMaximizeWindow;
     int                 updateToolbarFlag;
     BOOL                keepOnScreen;
     NSString            *windowAutosaveKey;
+
     BOOL                fullScreenEnabled; ///< Whether full screen is on (native or not)
     MMFullScreenWindow  *fullScreenWindow; ///< The window used for non-native full screen. Will only be non-nil when in non-native full screen.
     int                 fullScreenOptions;
     BOOL                delayEnterFullScreen;
     NSRect              preFullScreenFrame;
+
     MMWindow            *decoratedWindow;
     NSString            *lastSetTitle;
     NSString            *documentFilename; ///< File name of document being edited, used for the icon at the title bar.
@@ -68,7 +72,10 @@
 - (void)setTextDimensionsWithRows:(int)rows columns:(int)cols isLive:(BOOL)live
                       keepGUISize:(BOOL)keepGUISize
                      keepOnScreen:(BOOL)onScreen;
-- (void)resizeView;
+- (void)resizeVimViewAndWindow;
+- (void)resizeVimView;
+- (void)resizeVimViewBlockRender;
+- (BOOL)isRenderBlocked;
 - (void)zoomWithRows:(int)rows columns:(int)cols state:(int)state;
 - (void)setTitle:(NSString *)title;
 - (void)setDocumentFilename:(NSString *)filename;