diff --git a/src/MacVim/MMCoreTextView.h b/src/MacVim/MMCoreTextView.h index 9bdf59f7e4..42a246f3e4 100644 --- a/src/MacVim/MMCoreTextView.h +++ b/src/MacVim/MMCoreTextView.h @@ -48,6 +48,7 @@ NS_ASSUME_NONNULL_BEGIN { // From MMTextStorage int maxRows, maxColumns; + int pendingMaxRows, pendingMaxColumns; NSColor *defaultBackgroundColor; NSColor *defaultForegroundColor; NSSize cellSize; @@ -112,6 +113,9 @@ NS_ASSUME_NONNULL_BEGIN - (int)maxColumns; - (void)getMaxRows:(int*)rows columns:(int*)cols; - (void)setMaxRows:(int)rows columns:(int)cols; +- (int)pendingMaxRows; +- (int)pendingMaxColumns; +- (void)setPendingMaxRows:(int)rows columns:(int)cols; - (void)setDefaultColorsBackground:(NSColor *)bgColor foreground:(NSColor *)fgColor; - (NSColor *)defaultBackgroundColor; diff --git a/src/MacVim/MMCoreTextView.m b/src/MacVim/MMCoreTextView.m index 6c146e7e3f..c687cb8480 100644 --- a/src/MacVim/MMCoreTextView.m +++ b/src/MacVim/MMCoreTextView.m @@ -309,6 +309,24 @@ - (void)setMaxRows:(int)rows columns:(int)cols grid_resize(&grid, rows, cols); maxRows = rows; maxColumns = cols; + pendingMaxRows = rows; + pendingMaxColumns = cols; +} + +- (int)pendingMaxRows +{ + return pendingMaxRows; +} + +- (int)pendingMaxColumns +{ + return pendingMaxColumns; +} + +- (void)setPendingMaxRows:(int)rows columns:(int)cols +{ + pendingMaxRows = rows; + pendingMaxColumns = cols; } - (void)setDefaultColorsBackground:(NSColor *)bgColor diff --git a/src/MacVim/MMTextView.h b/src/MacVim/MMTextView.h index 036192c1a8..570dc1c3f6 100644 --- a/src/MacVim/MMTextView.h +++ b/src/MacVim/MMTextView.h @@ -25,6 +25,9 @@ NSRect *invertRects; int numInvertRects; + int pendingMaxRows; + int pendingMaxColumns; + MMTextViewHelper *helper; } @@ -57,6 +60,9 @@ - (int)maxColumns; - (void)getMaxRows:(int*)rows columns:(int*)cols; - (void)setMaxRows:(int)rows columns:(int)cols; +- (int)pendingMaxRows; +- (int)pendingMaxColumns; +- (void)setPendingMaxRows:(int)rows columns:(int)cols; - (NSRect)rectForRowsInRange:(NSRange)range; - (NSRect)rectForColumnsInRange:(NSRange)range; - (void)setDefaultColorsBackground:(NSColor *)bgColor diff --git a/src/MacVim/MMTextView.m b/src/MacVim/MMTextView.m index d092cabd74..e7c4923cf2 100644 --- a/src/MacVim/MMTextView.m +++ b/src/MacVim/MMTextView.m @@ -401,9 +401,27 @@ - (void)getMaxRows:(int*)rows columns:(int*)cols - (void)setMaxRows:(int)rows columns:(int)cols { + pendingMaxRows = rows; + pendingMaxColumns = cols; return [(MMTextStorage*)[self textStorage] setMaxRows:rows columns:cols]; } +- (int)pendingMaxRows +{ + return pendingMaxRows; +} + +- (int)pendingMaxColumns +{ + return pendingMaxColumns; +} + +- (void)setPendingMaxRows:(int)rows columns:(int)cols +{ + pendingMaxRows = rows; + pendingMaxColumns = cols; +} + - (NSRect)rectForRowsInRange:(NSRange)range { return [(MMTextStorage*)[self textStorage] rectForRowsInRange:range]; diff --git a/src/MacVim/MMVimView.h b/src/MacVim/MMVimView.h index ba04f90553..ba65674645 100644 --- a/src/MacVim/MMVimView.h +++ b/src/MacVim/MMVimView.h @@ -28,7 +28,8 @@ } @property BOOL pendingPlaceScrollbars; -@property BOOL pendingLiveResize; +@property BOOL pendingLiveResize; ///< An ongoing live resizing message to Vim is active +@property BOOL pendingLiveResizeQueued; ///< A new size has been queued while an ongoing live resize is already active - (MMVimView *)initWithFrame:(NSRect)frame vimController:(MMVimController *)c; diff --git a/src/MacVim/MMVimView.m b/src/MacVim/MMVimView.m index c062aedec5..0ce0167c66 100644 --- a/src/MacVim/MMVimView.m +++ b/src/MacVim/MMVimView.m @@ -953,19 +953,23 @@ - (void)frameSizeMayHaveChanged:(BOOL)keepGUISize [textView constrainRows:&constrained[0] columns:&constrained[1] toSize:textViewSize]; - int rows, cols; - [textView getMaxRows:&rows columns:&cols]; - - if (constrained[0] != rows || constrained[1] != cols) { + if (constrained[0] != textView.pendingMaxRows || constrained[1] != textView.pendingMaxColumns) { NSData *data = [NSData dataWithBytes:constrained length:2*sizeof(int)]; int msgid = [self inLiveResize] ? LiveResizeMsgID : (keepGUISize ? SetTextDimensionsNoResizeWindowMsgID : SetTextDimensionsMsgID); ASLogDebug(@"Notify Vim that text dimensions changed from %dx%d to " - "%dx%d (%s)", cols, rows, constrained[1], constrained[0], + "%dx%d (%s)", textView.pendingMaxColumns, textView.pendingMaxRows, constrained[1], constrained[0], MMVimMsgIDStrings[msgid]); - if (msgid != LiveResizeMsgID || !self.pendingLiveResize) { + if (msgid == LiveResizeMsgID && self.pendingLiveResize) { + // We are currently live resizing and there's already an ongoing + // resize message that we haven't finished handling yet. Wait until + // we are done with that since we don't want to overload Vim with + // messages. + self.pendingLiveResizeQueued = YES; + } + else { // Live resize messages can be sent really rapidly, especailly if // it's from double clicking the window border (to indicate filling // all the way to that side to the window manager). We want to rate @@ -976,6 +980,13 @@ - (void)frameSizeMayHaveChanged:(BOOL)keepGUISize // up resizing. self.pendingLiveResize = (msgid == LiveResizeMsgID); + // Cache the new pending size so we can use it to prevent resizing Vim again + // if we haven't changed the row/col count later. We don't want to + // immediately resize the textView (hence it's "pending") as we only + // do that when Vim has acknoledged the message and draws. This leads + // to a stable drawing. + [textView setPendingMaxRows:constrained[0] columns:constrained[1]]; + [vimController sendMessageNow:msgid data:data timeout:1]; } diff --git a/src/MacVim/MMWindowController.m b/src/MacVim/MMWindowController.m index 454532be09..0e389bd6c5 100644 --- a/src/MacVim/MMWindowController.m +++ b/src/MacVim/MMWindowController.m @@ -418,7 +418,22 @@ - (void)setTextDimensionsWithRows:(int)rows columns:(int)cols isLive:(BOOL)live // user drags to resize the window. [vimView setDesiredRows:rows columns:cols]; + vimView.pendingLiveResize = NO; + 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 + // Vim was done with the last message. + // + // This could happen if we are in the middle of rapid resize (e.g. + // double-clicking on the border/corner of window), as we would fire + // off a lot of LiveResizeMsgID messages where some will be + // intentionally omitted to avoid swamping IPC as we rate limit it to + // only one outstanding resize message at a time + // inframeSizeMayHaveChanged:. + vimView.pendingLiveResizeQueued = NO; + [self resizeView]; + } if (setupDone && !live && !keepGUISize) { shouldResizeVimView = YES; @@ -931,12 +946,15 @@ - (void)liveResizeDidEnd [decoratedWindow setTitle:lastSetTitle]; } - // If we are in the middle of rapid resize (e.g. double-clicking on the border/corner - // of window), we would fire off a lot of LiveResizeMsgID messages where some will be - // intentionally omitted to avoid swamping IPC. If that happens this will perform a - // final clean up that makes sure the Vim view is sized correctly within the window. - // See frameSizeMayHaveChanged: for where the omission/rate limiting happens. - [self resizeView]; + if (vimView.pendingLiveResizeQueued) { + // Similar to setTextDimensionsWithRows:, if there's still outstanding + // resize message queued, we just immediately flush it here to make + // sure Vim will get the most up-to-date size here when we are done + // 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]; + } } - (void)setBlurRadius:(int)radius diff --git a/src/MacVim/MacVimTests/MacVimTests.m b/src/MacVim/MacVimTests/MacVimTests.m index cdfd33c639..b87978726f 100644 --- a/src/MacVim/MacVimTests/MacVimTests.m +++ b/src/MacVim/MacVimTests/MacVimTests.m @@ -40,6 +40,16 @@ - (NSMutableArray*)vimControllers { } @end +static BOOL forceInLiveResize = NO; +@implementation MMVimView (testWindowResize) +- (BOOL)inLiveResize { + // Mock NSView's inLiveResize functionality + if (forceInLiveResize) + return YES; + return [super inLiveResize]; +} +@end + @interface MacVimTests : XCTestCase @end @@ -583,4 +593,144 @@ - (void) testTitlebarDocumentIcon { [self waitForVimClose]; } +/// Test resizing the MacVim window properly resizes Vim +- (void) testWindowResize { + MMAppController *app = MMAppController.sharedInstance; + + [app openNewWindow:NewWindowClean activate:YES]; + [self waitForVimOpenAndMessages]; + + NSWindow *win = [[[app keyVimController] windowController] window]; + MMVimView *vimView = [[[app keyVimController] windowController] vimView]; + MMTextView *textView = [[[[app keyVimController] windowController] vimView] textView]; + + // Set a default 30,80 base size for the entire test + [self sendStringToVim:@":set lines=30 columns=80\n" withMods:0]; + [self waitForEventHandlingAndVimProcess]; + XCTAssertEqual(30, textView.maxRows); + XCTAssertEqual(80, textView.maxColumns); + + const NSRect winFrame = win.frame; + + { + // Test basic resizing functionality. Make sure text view is updated properly + NSRect newFrame = winFrame; + newFrame.size.width -= textView.cellSize.width; + newFrame.size.height -= textView.cellSize.height; + + [win setFrame:newFrame display:YES]; + XCTAssertEqual(30, textView.maxRows); + XCTAssertEqual(80, textView.maxColumns); + [self waitForVimProcess]; + XCTAssertEqual(29, textView.maxRows); + XCTAssertEqual(79, textView.maxColumns); + + [win setFrame:winFrame display:YES]; + [self waitForVimProcess]; + XCTAssertEqual(30, textView.maxRows); + XCTAssertEqual(80, textView.maxColumns); + } + + { + // Test rapid resizing where we resize faster than Vim can handle. We + // should be updating a pending size indicating what we expect Vim's + // size should be and use that as the cache. Previously we had a bug + // we we used the outdated size as cache instead leading to rapid + // resizing sometimes leading to stale sizes. + + // This kind of situation coudl occur if say Vim is stalled for a bit + // and we resized the window multiple times. We don't rate limit unlike + // live resizing since usually it's not needed. + NSRect newFrame = winFrame; + newFrame.size.width -= textView.cellSize.width; + newFrame.size.height -= textView.cellSize.height; + + [win setFrame:newFrame display:YES]; + XCTAssertEqual(30, textView.maxRows); + XCTAssertEqual(80, textView.maxColumns); + XCTAssertEqual(29, textView.pendingMaxRows); + XCTAssertEqual(79, textView.pendingMaxColumns); + + [win setFrame:winFrame display:YES]; + XCTAssertEqual(30, textView.maxRows); + XCTAssertEqual(80, textView.maxColumns); + XCTAssertEqual(30, textView.pendingMaxRows); + XCTAssertEqual(80, textView.pendingMaxColumns); + + [self waitForVimProcess]; + XCTAssertEqual(30, textView.maxRows); + XCTAssertEqual(80, textView.maxColumns); + } + + { + // Test rapid resizing again, but this time we don't resize back to the + // original size, but instead incremented multiple times. Just to make + // sure we actually get set to the final size. + NSRect newFrame = winFrame; + for (int i = 0; i < 5; i++) { + newFrame.size.width += textView.cellSize.width; + newFrame.size.height += textView.cellSize.height; + [win setFrame:newFrame display:YES]; + } + XCTAssertEqual(30, textView.maxRows); + XCTAssertEqual(80, textView.maxColumns); + XCTAssertEqual(35, textView.pendingMaxRows); + XCTAssertEqual(85, textView.pendingMaxColumns); + + [self waitForVimProcess]; + XCTAssertEqual(35, textView.maxRows); + XCTAssertEqual(85, textView.maxColumns); + + [win setFrame:winFrame display:YES]; // reset back to original size + [self waitForVimProcess]; + XCTAssertEqual(30, textView.maxRows); + XCTAssertEqual(80, textView.maxColumns); + } + + { + // Test live resizing (e.g. when user drags the window edge to resize). + // We rate limit the number of messages we send to Vim so if there are + // multiple resize events they will be sequenced to avoid overloading Vim. + forceInLiveResize = YES; // simulate live resizing which can only be initiated by a user + [vimView viewWillStartLiveResize]; + + NSRect newFrame = winFrame; + for (int i = 0; i < 5; i++) { + newFrame.size.width += textView.cellSize.width; + newFrame.size.height += textView.cellSize.height; + [win setFrame:newFrame display:YES]; + } + + // The first time Vim processes this it should have only received the first message + // due to rate limiting. + XCTAssertEqual(30, textView.maxRows); + XCTAssertEqual(80, textView.maxColumns); + XCTAssertEqual(31, textView.pendingMaxRows); + XCTAssertEqual(81, textView.pendingMaxColumns); + + // After Vim has processed the messages it should now have the final size + [self waitForVimProcess]; // first wait for Vim to respond it processed the first message, where we send off the second one + [self waitForVimProcess]; // Vim should now have processed the last message + XCTAssertEqual(35, textView.maxRows); + XCTAssertEqual(85, textView.maxColumns); + XCTAssertEqual(35, textView.pendingMaxRows); + XCTAssertEqual(85, textView.pendingMaxColumns); + + forceInLiveResize = NO; + [vimView viewDidEndLiveResize]; + [self waitForVimProcess]; + XCTAssertEqual(35, textView.maxRows); + XCTAssertEqual(85, textView.maxColumns); + + [win setFrame:winFrame display:YES]; // reset back to original size + [self waitForEventHandlingAndVimProcess]; + XCTAssertEqual(30, textView.maxRows); + XCTAssertEqual(80, textView.maxColumns); + } + + // Clean up + [[app keyVimController] sendMessage:VimShouldCloseMsgID data:nil]; + [self waitForVimClose]; +} + @end