From 5502bbf9ff3c653c8e58cf2c94b6437c4b93dc5e Mon Sep 17 00:00:00 2001 From: Yee Cheng Chin Date: Mon, 20 Aug 2018 02:11:32 -0700 Subject: [PATCH 1/3] Fixed native fullscreen rendering corruption bug Fix the issue where MacVim would occasionally draws corrupted image in fullscreen (it would draw mostly black). The easiest way to reproduce this is as follows: 1. Make a new MacVim window, enter fullscreen 2. Open a new tab or hit Cmd-= a few times 3. Switch to another fullscreen app or desktop, click around, then switch back 4. Observe most of the screen is black. The reason this happens is that the MacVim resize code always tries to resize the window to fit the content size (calculated from how many lines / columns we have and whether other elements like tab bar are visible). This means the resize code (resizeWindowToFitContentSize:keepOnScreen:) would make the window smaller than the full size of screen. For some reason, when you switch away from the space, macOS decides to resize the window back to screen size again, causing a window resize event to happen. The resize event invalidates the NSView, causing it to draw black. This is also why fullscreen mode has black bars on top / bottom, which is especially jarring when font size is large of `linespace` is high. The fix is to treat guioptions 'k' to be on when in full screen mode, since the option means we will always try to fit the Vim content inside the window, rather than resize the window to fit the Vim content. This way the fullscreen Vim window will take up the whole screen and won't keep getting resized. This is also more similar to how native Gvim works when maximized. Close #496 (black bars) Close #557, close #674 (full screen rendering issues) A related issue is that MacVim (without CGLayer backing) doesn't actually know how to redraw itself properly when invalidated, which is the root cause of this bug. It receives Vim draw calls incrementally and doesn't actually cache the rendered content, so it relies on the fact that MacVim's NSWindow doesn't usually invalidates all the content which allows it to draw incrementally without needing to perform a full redraw. This is why non-native fullscreen requires CGLayer backing mode as macOS's behavior in this mode (basically a borderless window) is that it does clear the NSWindow's content when setWantsDisplay: is called. This is also why Vim live window resizing is limited to cell size instead of allowing smooth resize (to avoid having to trigger redraws). These are issues that should be fixed later. --- src/MacVim/MMWindowController.m | 36 +++++++++++---------------------- 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/MacVim/MMWindowController.m b/src/MacVim/MMWindowController.m index 1003d3b451..0f937f40b3 100644 --- a/src/MacVim/MMWindowController.m +++ b/src/MacVim/MMWindowController.m @@ -599,39 +599,27 @@ - (void)processInputQueueDidFinish if (!didMaximize) { NSSize originalSize = [vimView frame].size; int rows = 0, cols = 0; - NSSize contentSize = [vimView constrainRows:&rows columns:&cols - toSize: - fullScreenWindow ? [fullScreenWindow frame].size : - fullScreenEnabled ? desiredWindowSize : - [self constrainContentSizeToScreenSize:[vimView desiredSize]]]; // 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; + 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 { - [vimView setFrameSizeKeepGUISize:originalSize]; - } - - if (fullScreenWindow) { - // NOTE! Don't mark the full-screen content view as needing an - // update unless absolutely necessary since when it is updated - // the entire screen is cleared. This may cause some parts of - // the Vim view to be cleared but not redrawn since Vim does - // not realize that we've erased part of the view. - if (!NSEqualSizes(originalSize, contentSize)) { - [[fullScreenWindow contentView] setNeedsDisplay:YES]; - [fullScreenWindow centerView]; - } - } else { - if (!avoidWindowResize) { - [self resizeWindowToFitContentSize:contentSize - keepOnScreen:keepOnScreen]; - } + NSSize frameSize = fullScreenWindow ? [fullScreenWindow frame].size : (fullScreenEnabled ? desiredWindowSize : originalSize); + [vimView setFrameSizeKeepGUISize:frameSize]; } } From ad821e46d9f50d8ce6f0a7a647b77a538aaf0187 Mon Sep 17 00:00:00 2001 From: Yee Cheng Chin Date: Sun, 26 Aug 2018 00:08:12 -0700 Subject: [PATCH 2/3] Fix toggling native fullscreen on/off to restore window to correct size Previously when using native fullscreen mode, if you toggle fullscreen off, the window will end up taking up the whole screen rather than restoring back to the original size. Fix that. The real issue is because when you resize MacVim's window (which the fullscreen restore code does), then callback (windowDidResize) triggered a complicated set of callbacks by calling setFrameSize:, which in turn resizes Vim, which in turn calls windowDidResize again, which usually does the right thing, but not always. Fix the window resize handler code to always respect the new window size by calling setFrameSizeKeepGUISize: instead which doesn't resize the window. --- src/MacVim/MMWindowController.m | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/MacVim/MMWindowController.m b/src/MacVim/MMWindowController.m index 0f937f40b3..1f0b73daf4 100644 --- a/src/MacVim/MMWindowController.m +++ b/src/MacVim/MMWindowController.m @@ -1047,14 +1047,10 @@ - (void)windowDidResize:(id)sender // 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 // (rows,columns) changed. - if (shouldKeepGUISize) { - // This happens when code manually call setFrame: when we are performing - // an operation that wants to preserve GUI size (e.g. in updateToolbar:). - // Respect the wish, and pass that along. - [vimView setFrameSizeKeepGUISize:[self contentSize]]; - } else { - [vimView setFrameSize:[self contentSize]]; - } + // Calling setFrameSizeKeepGUISize: instead of setFrameSize: prevents a + // degenerate case where frameSizeMayHaveChanged: ends up resizing the window + // *again* causing windowDidResize: to be called. + [vimView setFrameSizeKeepGUISize:[self contentSize]]; } - (void)windowDidChangeBackingProperties:(NSNotification *)notification From fa0fad6d73240a5ced691ab1f62213d46a86d1b4 Mon Sep 17 00:00:00 2001 From: Yee Cheng Chin Date: Mon, 20 Aug 2018 23:54:39 -0700 Subject: [PATCH 3/3] Fixed double-clicking on border resulting in bad resize Fix issue where if the user double-clicks on the window border or corner to resize MacVim (the macOS behavior is to resize the window all the way to the screen's border), it results in an incomplete resize and also takes a long time. The code was spamming the Vim instance with live resize messages, leading to slowdown and dropped messages. Fix it by rate limiting the messages to one at a time, which speeds things up, and clean up when live resize finishes to make sure things look right. --- src/MacVim/MMVimView.h | 2 ++ src/MacVim/MMVimView.m | 14 +++++++++++++- src/MacVim/MMWindowController.m | 8 ++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/MacVim/MMVimView.h b/src/MacVim/MMVimView.h index 8ecaa24b2c..002ce9df70 100644 --- a/src/MacVim/MMVimView.h +++ b/src/MacVim/MMVimView.h @@ -27,6 +27,8 @@ NSMutableArray *scrollbars; } +@property BOOL pendingLiveResize; + - (MMVimView *)initWithFrame:(NSRect)frame vimController:(MMVimController *)c; - (MMTextView *)textView; diff --git a/src/MacVim/MMVimView.m b/src/MacVim/MMVimView.m index 6202a186f3..4bedcf9084 100644 --- a/src/MacVim/MMVimView.m +++ b/src/MacVim/MMVimView.m @@ -920,7 +920,19 @@ - (void)frameSizeMayHaveChanged:(BOOL)keepGUISize "%dx%d (%s)", cols, rows, constrained[1], constrained[0], MessageStrings[msgid]); - [vimController sendMessageNow:msgid data:data timeout:1]; + if (msgid != LiveResizeMsgID || !self.pendingLiveResize) { + // 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 + // limit sending live resize one at a time, or the IPC will get + // swamped which causes slowdowns and some messages will also be dropped. + // As a result we basically discard all live resize messages if one + // is already going on. liveResizeDidEnd: will perform a final clean + // up resizing. + self.pendingLiveResize = (msgid == LiveResizeMsgID); + + [vimController sendMessageNow:msgid data:data timeout:1]; + } // We only want to set the window title if this resize came from // a live-resize, not (for example) setting 'columns' or 'lines'. diff --git a/src/MacVim/MMWindowController.m b/src/MacVim/MMWindowController.m index 1f0b73daf4..d444f112c5 100644 --- a/src/MacVim/MMWindowController.m +++ b/src/MacVim/MMWindowController.m @@ -398,6 +398,7 @@ - (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 (setupDone && !live && !keepGUISize) { shouldResizeVimView = YES; @@ -712,6 +713,13 @@ - (void)liveResizeDidEnd [lastSetTitle release]; lastSetTitle = nil; } + + // 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]; } - (void)setBlurRadius:(int)radius