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

MacVim would previously show a quick flicker when adjusting font (e.g.
Cmd =/-) or showing/hiding tabs/scroll bar when in fixed window size
mode (guioptions+=k or full screen). This was because after the state
change, Vim requests a resize asynchronously to the GUI to fit it in the
window.  MacVim does so after changing the font/showing the tab, leading
to a momentary incorrect result before Vim then redraws the resized
grid. In normal GVim this is not an issue because Vim requests the
resize synchronously in a single-process environment, and we would like
to avoid that as the message passing between Vim/MacVim and designed to
be mostly non-blocking.

To fix this, after receiving the Vim resize request, we block all
further text rendering commands, until Vim has resized / redrawn,
preventing the short period of time where text view is drawing the old
state using the new font. For tabs / scroll bars, the text view itself
has moved after the new layout, so we temporarily apply a render offset
to make the text view pretend it didn't move and looks mostly the same
to the user while we wait for Vim to redraw with the updated grid.

There are some potential ways to still see flicker, but they are mostly
edge cases:

- When changing fonts, if Vim is slow and the user gets MacVim to
  re-draw the text view (e.g. dragging the window to resize) while we
  wait for Vim to resize, it would still draw an incorrect result (since
  it has the new font, but old text grid). This should realistically
  only happen if Vim takes an abnormal amount of time to respond.
- For tabs / scrollbars we have a similar issue. We immediately
  place/remove them while we wait for Vim to resize, which could cause a
  small visual discontinuity (easiest way is to toggle `go+=e`). From
  testing, having the tab bar / etc immediately show up and hide feels
  better as the user feels like something has happened, so keeping the
  responsiveness is more important than delaying showing/hiding the tab
  bar for visual stability (not to mention the deferral is more
  complicated to implement).

If Vim takes a long time to resize/redraw, this change could make font
size change *feel* less responsive because nothing happens on the screen
until the fully redrawn screen is shown. This is ok, and if Vim takes so
long to resize then that's the actual issue to address.

This change also removes unnecessary code:

- Excessive and unnecessary redraws when showing/hiding tabs and setting
  fonts. They were written a long time ago as temporary hacks which
  survived till now. From testing this makes changing font size and
  showing/hiding tabs feel a fair bit more responsive because Vim isn't
  trying to redraw over and over again now.
- Stale "maximize" code that has long been unused. It was trying to solve
  a similar issue but long obsolete and disabled.

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;