diff --git a/src/MacVim/MMBackend.m b/src/MacVim/MMBackend.m index 0d35f841a3..66ea170252 100644 --- a/src/MacVim/MMBackend.m +++ b/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 diff --git a/src/MacVim/MMCoreTextView.h b/src/MacVim/MMCoreTextView.h index 6dd64125d4..15ec0b9c64 100644 --- a/src/MacVim/MMCoreTextView.h +++ b/src/MacVim/MMCoreTextView.h @@ -71,10 +71,6 @@ NS_ASSUME_NONNULL_BEGIN MMTextViewHelper *helper; - NSMutableDictionary *fontVariants; - NSMutableSet *characterStrings; - NSMutableDictionary *> *characterLines; - // These are used in MMCoreTextView+ToolTip.m id trackingRectOwner_; // (not retained) void *trackingRectUserData_; @@ -82,6 +78,8 @@ NS_ASSUME_NONNULL_BEGIN 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; // diff --git a/src/MacVim/MMCoreTextView.m b/src/MacVim/MMCoreTextView.m index 5b91b1ad51..57f67a42a5 100644 --- a/src/MacVim/MMCoreTextView.m +++ b/src/MacVim/MMCoreTextView.m @@ -222,6 +222,10 @@ static void grid_free(Grid *grid) { @implementation MMCoreTextView { Grid grid; + NSMutableSet *characterStrings; ///< Storage for characters in the grid + + NSMutableDictionary *fontVariants; ///< Cache for fonts used for each variant (e.g. italic) + NSMutableDictionary *> *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,6 +1509,7 @@ - (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 @@ -1512,6 +1517,7 @@ - (NSRect)rectForRow:(int)row column:(int)col numRows:(int)nr // 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; diff --git a/src/MacVim/MMTextView.h b/src/MacVim/MMTextView.h index 570dc1c3f6..b18ee08203 100644 --- a/src/MacVim/MMTextView.h +++ b/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; diff --git a/src/MacVim/MMVimController.m b/src/MacVim/MMVimController.m index 2137dedf43..bff551ac1d 100644 --- a/src/MacVim/MMVimController.m +++ b/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: diff --git a/src/MacVim/MMVimView.m b/src/MacVim/MMVimView.m index 9a7a5a4781..c9840471ed 100644 --- a/src/MacVim/MMVimView.m +++ b/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]; diff --git a/src/MacVim/MMWindowController.h b/src/MacVim/MMWindowController.h index 544385f8c2..4b8a0a2278 100644 --- a/src/MacVim/MMWindowController.h +++ b/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; diff --git a/src/MacVim/MMWindowController.m b/src/MacVim/MMWindowController.m index 8b195e28e5..05890bcedf 100644 --- a/src/MacVim/MMWindowController.m +++ b/src/MacVim/MMWindowController.m @@ -91,7 +91,6 @@ - (void)updateTablineSeparator; - (void)hideTablineSeparator:(BOOL)hide; - (void)doFindNext:(BOOL)next; - (void)updateToolbar; -- (BOOL)maximizeWindow:(int)options; - (void)applicationDidChangeScreenParameters:(NSNotification *)notification; - (void)enterNativeFullScreen; - (void)processAfterWindowPresentedQueue; @@ -414,6 +413,10 @@ - (void)setTextDimensionsWithRows:(int)rows columns:(int)cols isLive:(BOOL)live [vimView setDesiredRows:rows columns:cols]; vimView.pendingLiveResize = NO; + if (blockRenderUntilResize) { + blockRenderUntilResize = NO; + [vimView.textView setDrawRectOffset:NSZeroSize]; + } if (vimView.pendingLiveResizeQueued) { // There was already a new size queued while Vim was still processing // the last one. We need to immediately request another resize now that @@ -426,11 +429,11 @@ - (void)setTextDimensionsWithRows:(int)rows columns:(int)cols isLive:(BOOL)live // only one outstanding resize message at a time // inframeSizeMayHaveChanged:. vimView.pendingLiveResizeQueued = NO; - [self resizeView]; + [self resizeVimView]; } if (setupDone && !live && !keepGUISize) { - shouldResizeVimView = YES; + [self resizeVimViewAndWindow]; keepOnScreen = onScreen; } @@ -458,7 +461,20 @@ - (void)setTextDimensionsWithRows:(int)rows columns:(int)cols isLive:(BOOL)live } } -- (void)resizeView +/// Resize the Vim view to its desired size based on number of rows/cols. +/// Ignores the window size and force the window to resize along with it. +- (void)resizeVimViewAndWindow +{ + if (setupDone) + { + shouldResizeVimView = YES; + } +} + +/// Resize the Vim view to match the GUI window size. This makes sure the GUI +/// doesn't change its size and fit everything within it to match. If the text +/// view ends up changing size, will send a message to Vim to resize itself. +- (void)resizeVimView { if (setupDone) { @@ -467,6 +483,26 @@ - (void)resizeView } } +/// Resize the Vim view to match GUI window size, but also block any text +/// rendering from happening while we wait for Vim to be resized. This is used +/// to avoid any flickering as the current rendered texts are going to be +/// invalidated very soon as Vim will be resized and issue new draw commands. +/// +/// This happens when say we have guioptions+=k and the user changes the font +/// or shows the tab bar. +- (void)resizeVimViewBlockRender +{ + [self resizeVimView]; + if (shouldResizeVimView) { + blockRenderUntilResize = YES; + } +} + +- (BOOL)isRenderBlocked +{ + return blockRenderUntilResize; +} + - (void)zoomWithRows:(int)rows columns:(int)cols state:(int)state { [self setTextDimensionsWithRows:rows @@ -805,7 +841,6 @@ - (void)setFont:(NSFont *)font [[vimView textView] setFont:font]; [self updateResizeConstraints:NO]; - shouldMaximizeWindow = YES; } - (void)setWideFont:(NSFont *)font @@ -834,69 +869,89 @@ - (void)processInputQueueDidFinish if (updateToolbarFlag != 0) [self updateToolbar]; + const int oldTextViewRows = vimView.textView.pendingMaxRows; + const int oldTextViewCols = vimView.textView.pendingMaxColumns; + const NSRect oldTextViewFrame = vimView.textView.frame; + BOOL vimViewSizeChanged = NO; + // NOTE: If the window has not been presented then we must avoid resizing // the views since it will cause them to be constrained to the screen which // has not yet been set! if (windowPresented && shouldResizeVimView) { shouldResizeVimView = NO; - // Make sure full-screen window stays maximized (e.g. when scrollbar or - // tabline is hidden) according to 'fuopt'. - - BOOL didMaximize = NO; - if (shouldMaximizeWindow && fullScreenEnabled && - (fullScreenOptions & (FUOPT_MAXVERT|FUOPT_MAXHORZ)) != 0) - didMaximize = [self maximizeWindow:fullScreenOptions]; - - shouldMaximizeWindow = NO; - - // Resize Vim view and window, but don't do this now if the window was - // just reszied because this would make the window "jump" unpleasantly. - // Instead wait for Vim to respond to the resize message and do the - // resizing then. - // TODO: What if the resize message fails to make it back? - if (!didMaximize) { - NSSize originalSize = [vimView frame].size; - int rows = 0, cols = 0; - - // Setting 'guioptions+=k' will make shouldKeepGUISize true, which - // means avoid resizing the window. Instead, resize the view instead - // to keep the GUI window's size consistent. - bool avoidWindowResize = shouldKeepGUISize || fullScreenEnabled; - - if (!avoidWindowResize) { - NSSize contentSize = [vimView constrainRows:&rows columns:&cols - toSize: - fullScreenWindow ? [fullScreenWindow frame].size : - fullScreenEnabled ? desiredWindowSize : - [self constrainContentSizeToScreenSize:[vimView desiredSize]]]; - - [vimView setFrameSize:contentSize]; - - [self resizeWindowToFitContentSize:contentSize - keepOnScreen:keepOnScreen]; - } - else { - NSSize frameSize; - if (fullScreenWindow) { - // Non-native full screen mode. - NSRect desiredFrame = [fullScreenWindow getDesiredFrame]; - frameSize = desiredFrame.size; - [vimView setFrameOrigin:desiredFrame.origin]; // This will get set back to normal in MMFullScreenWindow::leaveFullScreen. - } else if (fullScreenEnabled) { - // Native full screen mode. - frameSize = desiredWindowSize; - } else { - frameSize = originalSize; - } - [vimView setFrameSizeKeepGUISize:frameSize]; + NSSize originalSize = [vimView frame].size; + int rows = 0, cols = 0; + + // Setting 'guioptions+=k' will make shouldKeepGUISize true, which + // means avoid resizing the window. Instead, resize the view instead + // to keep the GUI window's size consistent. + // Note: Vim should always have requested shouldKeepGUISize to be true + // when in full screen, but we check for it anyway for safety. + bool avoidWindowResize = shouldKeepGUISize || fullScreenEnabled; + + if (!avoidWindowResize) { + NSSize contentSize = [vimView constrainRows:&rows columns:&cols + toSize: + fullScreenWindow ? [fullScreenWindow frame].size : + fullScreenEnabled ? desiredWindowSize : + [self constrainContentSizeToScreenSize:[vimView desiredSize]]]; + + [vimView setFrameSize:contentSize]; + + [self resizeWindowToFitContentSize:contentSize + keepOnScreen:keepOnScreen]; + } + else { + NSSize frameSize; + if (fullScreenWindow) { + // Non-native full screen mode. + NSRect desiredFrame = [fullScreenWindow getDesiredFrame]; + frameSize = desiredFrame.size; + [vimView setFrameOrigin:desiredFrame.origin]; // This will get set back to normal in MMFullScreenWindow::leaveFullScreen. + } else if (fullScreenEnabled) { + // Native full screen mode. + frameSize = desiredWindowSize; + } else { + frameSize = originalSize; } + [vimView setFrameSizeKeepGUISize:frameSize]; } keepOnScreen = NO; shouldKeepGUISize = NO; + + vimViewSizeChanged = (vimView.textView.pendingMaxColumns != oldTextViewCols || vimView.textView.pendingMaxRows != oldTextViewRows); } - + + if (blockRenderUntilResize) { + if (vimViewSizeChanged) { + const NSRect newTextViewFrame = vimView.textView.frame; + + // We are currently blocking all rendering to prevent flicker. If + // the view frame moved (this happens if the tab or left scroll bar + // were shown/hidden) the user will see a temporary flicker as the + // text view was moved before Vim has udpated us with new draw calls + // to match the new size. To alleviate this, we temporarily apply + // a drawing offset in the text view to counter the offset. To the + // user it would appear that the text view hasn't moved at all. + [vimView.textView setDrawRectOffset:NSMakeSize(NSMinX(oldTextViewFrame) - NSMinX(newTextViewFrame), NSMaxY(oldTextViewFrame) - NSMaxY(newTextViewFrame))]; + } else { + // We were blocking all rendering until Vim has been resized. However + // in situations where we turned out to not need to resize Vim to + // begin with, we need to remedy the situation as we dropped some + // draw commands before. We simply ask Vim to redraw itself for + // simplicity instead of caching all the draw commands for this. + // This could happen if we changed guifont (which makes Vim think + // we need to resize) but turned out we set it to the same font so + // the grid size is the same and no need to resize. + blockRenderUntilResize = NO; + [vimView.textView setDrawRectOffset:NSZeroSize]; + + [vimController sendMessage:RedrawMsgID data:nil]; + } + } + // Tell Vim view to update its scrollbars which is done once per update. // Do it last so whatever resizing we have done above will take effect // immediate too instead of waiting till next frame. @@ -921,7 +976,6 @@ - (void)showTabline:(BOOL)on { [vimView showTabline:on]; [self updateTablineSeparator]; - shouldMaximizeWindow = YES; if([[NSUserDefaults standardUserDefaults] boolForKey:MMWindowUseTabBackgroundColorKey]) { if (on) { @@ -1010,7 +1064,7 @@ - (void)liveResizeDidEnd // with live resizing to make sure we don't havae any stale sizes due // to rate limiting of IPC messages during live resizing.. vimView.pendingLiveResizeQueued = NO; - [self resizeView]; + [self resizeVimView]; } } @@ -1378,12 +1432,6 @@ - (void)windowDidResize:(id)sender if (!setupDone) return; - // NOTE: We need to update the window frame size for Split View even though - // in full-screen on El Capitan or later. - if (floor(NSAppKitVersionNumber) <= NSAppKitVersionNumber10_10_Max - && fullScreenEnabled) - return; - // NOTE: Since we have no control over when the window may resize (Cocoa // may resize automatically) we simply set the view to fill the entire // window. The vim view takes care of notifying Vim if the number of @@ -1408,7 +1456,7 @@ - (void)windowDidResize:(id)sender - (void)windowDidChangeBackingProperties:(NSNotification *)notification { ASLogDebug(@""); - [vimController sendMessage:BackingPropertiesChangedMsgID data:nil]; + [vimController sendMessage:RedrawMsgID data:nil]; } // This is not an NSWindow delegate method, our custom MMWindow class calls it @@ -1638,12 +1686,6 @@ - (void)windowWillEnterFullScreen:(NSNotification *)notification - (void)windowDidEnterFullScreen:(NSNotification *)notification { - if (floor(NSAppKitVersionNumber) > NSAppKitVersionNumber10_10_Max) { - // NOTE: On El Capitan, we need to redraw the view when entering - // full-screen using :fullscreen option (including Ctrl-Cmd-f). - [vimController sendMessage:BackingPropertiesChangedMsgID data:nil]; - } - // Sometimes full screen will de-focus the text view. This seems to happen // when titlebar is configured as hidden. Simply re-assert it to make sure // text is still focused. @@ -1701,12 +1743,6 @@ - (void)windowWillExitFullScreen:(NSNotification *)notification - (void)windowDidExitFullScreen:(NSNotification *)notification { - if (floor(NSAppKitVersionNumber) > NSAppKitVersionNumber10_10_Max) { - // NOTE: On El Capitan, we need to redraw the view when leaving - // full-screen by moving the window out from Split View. - [vimController sendMessage:BackingPropertiesChangedMsgID data:nil]; - } - // We set the resize increment to 1,1 above just to sure window size was // restored properly. We want to set it back to the correct value, which // would not be 1,1 if we are not using smooth resize. @@ -1734,7 +1770,6 @@ - (void)windowDidFailToExitFullScreen:(NSWindow *)window ASLogNotice(@"Failed to EXIT full-screen, maximizing window..."); fullScreenEnabled = YES; - [self maximizeWindow:fullScreenOptions]; // Sometimes full screen will de-focus the text view. This seems to happen // when titlebar is configured as hidden. Simply re-assert it to make sure @@ -1793,7 +1828,7 @@ - (void)updateResizeConstraints:(BOOL)resizeWindow // We only want to resize the window down to match the Vim size if not using smooth resizing. // This resizing is going to re-snap the Window size to multiples of grid size. Otherwise // the resize constraint is always going to be at an offset to the desired size. - shouldResizeVimView = YES; + [self resizeVimViewAndWindow]; } } } @@ -2069,83 +2104,10 @@ - (void)updateToolbar updateToolbarFlag = 0; } -- (BOOL)maximizeWindow:(int)options -{ - // Note: - // This is deprecated code and will be removed later. 'fuopt' should be - // handled in processInputQueueDidFinish instead. - - if (floor(NSAppKitVersionNumber) > NSAppKitVersionNumber10_10_Max) { - // NOTE: Prevent to resize the window in Split View on El Capitan or - // later. - return NO; - } - - int currRows, currColumns; - [[vimView textView] getMaxRows:&currRows columns:&currColumns]; - - // NOTE: Do not use [NSScreen visibleFrame] when determining the screen - // size since it compensates for menu and dock. - int maxRows, maxColumns; - NSScreen *screen = [decoratedWindow screen]; - if (!screen) { - ASLogNotice(@"Window not on screen, using main screen"); - screen = [NSScreen mainScreen]; - } - NSSize size = [screen frame].size; - [vimView constrainRows:&maxRows columns:&maxColumns toSize:size]; - - ASLogDebug(@"Window dimensions max: %dx%d current: %dx%d", - maxRows, maxColumns, currRows, currColumns); - - // Compute current fu size - int fuRows = currRows, fuColumns = currColumns; - if (options & FUOPT_MAXVERT) - fuRows = maxRows; - if (options & FUOPT_MAXHORZ) - fuColumns = maxColumns; - - // If necessary, resize vim to target fu size - if (currRows != fuRows || currColumns != fuColumns) { - // The size sent here is queued and sent to vim when it's in - // event processing mode again. Make sure to only send the values we - // care about, as they override any changes that were made to 'lines' - // and 'columns' after 'fu' was set but before the event loop is run. - NSData *data = nil; - int msgid = 0; - if (currRows != fuRows && currColumns != fuColumns) { - int newSize[2] = { fuRows, fuColumns }; - data = [NSData dataWithBytes:newSize length:2*sizeof(int)]; - msgid = SetTextDimensionsMsgID; - } else if (currRows != fuRows) { - data = [NSData dataWithBytes:&fuRows length:sizeof(int)]; - msgid = SetTextRowsMsgID; - } else if (currColumns != fuColumns) { - data = [NSData dataWithBytes:&fuColumns length:sizeof(int)]; - msgid = SetTextColumnsMsgID; - } - NSParameterAssert(data != nil && msgid != 0); - - ASLogDebug(@"%s: %dx%d", MMVimMsgIDStrings[msgid], fuRows, fuColumns); - MMVimController *vc = [self vimController]; - [vc sendMessage:msgid data:data]; - [[vimView textView] setMaxRows:fuRows columns:fuColumns]; - - // Indicate that window was resized - return YES; - } - - // Indicate that window was not resized - return NO; -} - - (void)applicationDidChangeScreenParameters:(NSNotification *)notification { if (fullScreenWindow) { [fullScreenWindow applicationDidChangeScreenParameters:notification]; - } else if (fullScreenEnabled) { - ASLogDebug(@"Re-maximizing full-screen window..."); - [self maximizeWindow:fullScreenOptions]; } } diff --git a/src/MacVim/MacVim.h b/src/MacVim/MacVim.h index 6bcb61057d..b863bad620 100644 --- a/src/MacVim/MacVim.h +++ b/src/MacVim/MacVim.h @@ -348,7 +348,7 @@ extern const char * const MMVimMsgIDStrings[]; MSG(SetTooltipMsgID) \ MSG(GestureMsgID) \ MSG(AddToMRUMsgID) \ - MSG(BackingPropertiesChangedMsgID) \ + MSG(RedrawMsgID) \ MSG(SetBlurRadiusMsgID) \ MSG(SetBackgroundOptionMsgID) \ MSG(NotifyAppearanceChangeMsgID) \ diff --git a/src/MacVim/MacVimTests/MacVimTests.m b/src/MacVim/MacVimTests/MacVimTests.m index 4e632ad5e9..7fb3551f13 100644 --- a/src/MacVim/MacVimTests/MacVimTests.m +++ b/src/MacVim/MacVimTests/MacVimTests.m @@ -25,6 +25,7 @@ // Expose private methods for testing purposes @interface MMAppController (Private) + (NSDictionary*)parseOpenURL:(NSURL*)url; +- (void)processInputQueues:(id)sender; @end @interface MMVimController (Private) @@ -225,7 +226,90 @@ - (void)waitForEventHandling { method_setImplementation(method, origIMP); } -/// Wait for Vim to process all pending messages in its queue. +static BOOL vimProcessInputBlocked = NO; + +/// Block / unblock all Vim message handling from happening. This allows tests +/// to run with a strong guarantee of ordering of events without them being +/// subject to timing variations. Any outstanding block will be cleared at the +/// end of the tests during teardown automatically. +- (void)blockVimProcessInput:(BOOL)block { + SEL sel = @selector(processInputQueues:); + Method method = class_getInstanceMethod([MMAppController class], sel); + + __weak __typeof__(self) self_weak = self; + + static IMP origIMP = nil; + static BOOL blockedMethodCalled = NO; + static BOOL teardownAdded = NO; + if (block) { + if (origIMP == nil) + origIMP = method_getImplementation(method); + IMP newIMP = imp_implementationWithBlock(^(id self, id sender) { + blockedMethodCalled = YES; + }); + method_setImplementation(method, newIMP); + vimProcessInputBlocked = YES; + + if (!teardownAdded) { + [self addTeardownBlock:^{ + if (vimProcessInputBlocked) { + [self_weak blockVimProcessInput:NO]; + } + teardownAdded = NO; + }]; + teardownAdded = YES; + } + } else { + if (origIMP != nil) { + method_setImplementation(method, origIMP); + + if (blockedMethodCalled) { + MMAppController *app = MMAppController.sharedInstance; + [app performSelectorOnMainThread:@selector(processInputQueues:) withObject:nil waitUntilDone:NO]; + blockedMethodCalled = NO; + } + } + vimProcessInputBlocked = NO; + } +} + +/// Wait for a specific message from Vim. Optionally, after receiving the +/// mesasge, block all future Vim message handling until manually unblocked +/// or this method was called again. This is useful for tests that want to +/// test a sequence of events with tight ordering and not be subject to timing +/// issues. +- (void)waitForVimMessage:(int)messageID blockFutureMessages:(BOOL)blockMsgs { + XCTestExpectation *expectation = [self expectationWithDescription:@"VimMessage"]; + + SEL sel = @selector(handleMessage:data:); + Method method = class_getInstanceMethod([MMVimController class], sel); + + __weak __typeof__(self) self_weak = self; + + IMP origIMP = method_getImplementation(method); + IMP newIMP = imp_implementationWithBlock(^(id self, int msgid, NSData *data) { + typedef void (*fn)(id,SEL,int,NSData*); + ((fn)origIMP)(self, sel, msgid, data); + if (msgid == messageID) { + [expectation fulfill]; + if (blockMsgs) { + [self_weak blockVimProcessInput:YES]; + } + } + }); + + if (vimProcessInputBlocked) { + // Make sure unblock message handling first or we will deadlock. + [self blockVimProcessInput:NO]; + } + + method_setImplementation(method, newIMP); + [self waitForExpectations:@[expectation] timeout:10]; + method_setImplementation(method, origIMP); +} + +/// Wait for Vim to process all pending messages in its queue. In future we +/// should migrate to having tests directly call waitForVimMessage directly. - (void)waitForVimProcess { // Implement this by sending a loopback message (Vim will send the message // back to us) as a synchronization mechanism as Vim handles its messages @@ -776,6 +860,85 @@ - (void) testWindowResize { } } +/// Test resizing the Vim view to match window size (go+=k / full screen) works +/// and produces a stable image. +- (void) testResizeVimView { + [self createTestVimWindow]; + + MMAppController *app = MMAppController.sharedInstance; + MMWindowController *win = [[app keyVimController] windowController]; + MMTextView *textView = [[[[app keyVimController] windowController] vimView] textView]; + + [self sendStringToVim:@":set guioptions+=k guifont=Menlo:h10\n" withMods:0]; + [self waitForEventHandlingAndVimProcess]; + + // Set a default 30,80 base size for the entire test + [self sendStringToVim:@":set lines=30 columns=80\n" withMods:0]; + [self waitForEventHandlingAndVimProcess]; + XCTAssertEqual(textView.maxRows, 30); + XCTAssertEqual(textView.maxColumns, 80); + + // Test that setting a bigger font will trigger a resize of Vim view to + // smaller grid, but also block intermediary rendering to avoid flicker. + [self sendStringToVim:@":set guifont=Menlo:h13\n" withMods:0]; + [self waitForVimMessage:SetFontMsgID blockFutureMessages:YES]; + XCTAssertEqual(textView.maxRows, 30); + XCTAssertEqual(textView.maxColumns, 80); + XCTAssertLessThan(textView.pendingMaxRows, 30); // confirms that we have an outstanding resize request to make it smaller + XCTAssertLessThan(textView.pendingMaxColumns, 80); + XCTAssertTrue(win.isRenderBlocked); + // Vim has responded to the size change. We should now have unblocked rendering. + [self waitForVimMessage:SetTextDimensionsNoResizeWindowMsgID blockFutureMessages:YES]; + XCTAssertLessThan(textView.maxRows, 30); + XCTAssertLessThan(textView.maxColumns, 80); + XCTAssertFalse(win.isRenderBlocked); + + // Make sure if we set it again to the same font, we won't block since we + // didn't actually resize anything. + [self sendStringToVim:@":set guifont=Menlo:h13\n" withMods:0]; + [self waitForVimMessage:SetFontMsgID blockFutureMessages:YES]; + XCTAssertFalse(win.isRenderBlocked); + + // Set it back and make sure it went back to the original rows/cols + [self sendStringToVim:@":set guifont=Menlo:h10\n" withMods:0]; + [self waitForVimMessage:SetTextDimensionsNoResizeWindowMsgID blockFutureMessages:YES]; + XCTAssertEqual(textView.maxRows, 30); + XCTAssertEqual(textView.maxColumns, 80); + + // Test making a new tab would do the same + [self sendStringToVim:@":tabnew\n" withMods:0]; + [self waitForVimMessage:ShowTabBarMsgID blockFutureMessages:YES]; + XCTAssertEqual(textView.maxRows, 30); + XCTAssertLessThan(textView.pendingMaxRows, 30); + XCTAssertGreaterThan(textView.drawRectOffset.height, 0); + XCTAssertTrue(win.isRenderBlocked); + [self waitForVimMessage:SetTextDimensionsNoResizeWindowMsgID blockFutureMessages:YES]; + XCTAssertLessThan(textView.maxRows, 30); + XCTAssertEqual(textView.drawRectOffset.height, 0); + XCTAssertFalse(win.isRenderBlocked); + [self blockVimProcessInput:NO]; + + // Repeat the same font size change test in full screen to exercise that + // code path. In particular, it should act like go+=k even if the option + // was not explicitly set. + [self setDefault:MMNativeFullScreenKey toValue:@NO]; // non-native is faster so use that + [self sendStringToVim:@":set guioptions-=k fullscreen\n" withMods:0]; + [self waitForFullscreenTransitionIsEnter:YES isNative:NO]; + int fuRows = textView.maxRows; + int fuCols = textView.maxColumns; + [self sendStringToVim:@":set guifont=Menlo:h13\n" withMods:0]; + [self waitForVimMessage:SetFontMsgID blockFutureMessages:YES]; + XCTAssertEqual(textView.maxRows, fuRows); + XCTAssertEqual(textView.maxColumns, fuCols); + XCTAssertLessThan(textView.pendingMaxRows, fuRows); + XCTAssertLessThan(textView.pendingMaxColumns, fuCols); + XCTAssertTrue(win.isRenderBlocked); + [self waitForVimMessage:SetTextDimensionsNoResizeWindowMsgID blockFutureMessages:YES]; + XCTAssertLessThan(textView.maxRows, fuRows); + XCTAssertLessThan(textView.maxColumns, fuCols); + XCTAssertFalse(win.isRenderBlocked); +} + #pragma mark Full screen tests - (void)waitForNativeFullscreenEnter { diff --git a/src/MacVim/gui_macvim.m b/src/MacVim/gui_macvim.m index 0d030f04ca..7ea4212c11 100644 --- a/src/MacVim/gui_macvim.m +++ b/src/MacVim/gui_macvim.m @@ -1945,6 +1945,12 @@ * Re-calculates size of the Vim view to fit within the window without having * to resize the window. Usually happens after UI elements have changed (e.g. * adding / removing a toolbar) when guioptions 'k' is set. + * + * In other GVim implementations this is a synchronous operation (via + * gui_mch_newfont). In MacVim we need to request the GUI process to resize us, + * and we do this asynchronously to avoid introducing sync points. It does mean + * Vim will temporarily draw/behave using the old size until the it receives + * the resize message from the GUI. */ void gui_mch_resize_view(void) diff --git a/src/gui.c b/src/gui.c index 1bc329c764..6a10ef5e2a 100644 --- a/src/gui.c +++ b/src/gui.c @@ -1714,7 +1714,8 @@ gui_set_shellsize( #endif #ifdef FEAT_GUI_MACVIM - if (!mustset && (vim_strchr(p_go, GO_KEEPWINSIZE) != NULL)) + if (!mustset && (vim_strchr(p_go, GO_KEEPWINSIZE) != NULL + || p_fullscreen)) { /* We don't want to resize the window, so instruct the GUI to resize * the view to be within the constraints of the current window's size */