Skip to content

Commit 7787583

Browse files
committed
Fix full screen window restore / multi-screen / misc issues
Native full-screen: Fix restoring window when we don't have smooth resize setting set. The NSWindow has a contentResizeIncrement set and macOS seems to have a bug/quirk where it would ignore it when entering full screen, but on exit, it will use it to determine the final window size. This means it would restore the window to a slightly different and wrong size. From testing, seems like Apple's own Terminal just works around that by always resizing the window after existing full screen and other apps just have this subtle bug. For MacVim, we just fix it by temporarily removing the contentResizeIncrement while exiting full screen, which is a small hack but it works. Non-native full screen: Fix restoring window when we have smooth resize setting set. Previously we remember the number of lines/columns and manually resize to that on exit, but it could result in the restored window size being wrong. Just do the more sensible thing and simply keep the old window size and rescale the Vim view inside it. It simplifies the code and make everything works more intuitively, and it makes sure the old window size is respected. Long term goal is to keep MMFullScreenWindow as simple as possible the window controller whould be the one doing the more complicated integration work. Also fix manually dragging a non-native full screen window across multiple screen (e.g in Mission Control) to work properly, and make sure the restored window is in the new screen. Other improvements: - Make sure the non-native full screen window has "movable" set to NO, and configure the window so it cannot go full screen. These make sure macOS would properly gray out the Window menu item for window tiling/full screen/etc. Previously the user could use those accidentally which leads to bad results. - Remove the previous code that tried to make sure we restore the window the current Space (i.e. virtual desktop). macOS has no explicit control for Mission Control Spaces and the old method does not work and relies on a hacky way of configuring the collection method. This is a niche situation anyway. See code comments for details. Tests - In order to get the tests for this to pass in CI, I had to add a somewhat hacky solution to manually inject a user click to pretend we have clicked on the window to resize it. There is an obscure macOS quirk/bug where it seems to restore a full screen window to an old location that the user has manually interacted with but this bug only seems to happen in a VM and not a real machine. The workaround allows our tests to pass consistently regardless of where it is run.
1 parent 954e990 commit 7787583

File tree

4 files changed

+346
-111
lines changed

4 files changed

+346
-111
lines changed

src/MacVim/MMFullScreenWindow.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,18 @@
1717
@interface MMFullScreenWindow : NSWindow {
1818
NSWindow *target;
1919
MMVimView *view;
20-
NSPoint oldPosition;
2120
NSString *oldTabBarStyle;
2221
int options;
2322
int state;
2423

25-
// These are only valid in full-screen mode and store pre-fu vim size
26-
int nonFuRows, nonFuColumns;
27-
2824
/// The non-full-screen size of the Vim view. Used for non-maxvert/maxhorz options.
2925
NSSize nonFuVimViewSize;
3026

3127
// This stores the contents of fuoptions_flags at fu start time
3228
int startFuFlags;
3329

3430
// Controls the speed of the fade in and out.
31+
// This feature is deprecated and off by default.
3532
double fadeTime;
3633
double fadeReservationTime;
3734
}

src/MacVim/MMFullScreenWindow.m

Lines changed: 85 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ - (MMFullScreenWindow *)initWithWindow:(NSWindow *)t view:(MMVimView *)v
5858
backgroundColor:(NSColor *)back
5959
{
6060
NSScreen* screen = [t screen];
61-
62-
// XXX: what if screen == nil?
61+
if (screen == nil) {
62+
screen = [NSScreen mainScreen];
63+
}
6364

6465
// you can't change the style of an existing window in cocoa. create a new
6566
// window and move the MMTextView into it.
@@ -81,10 +82,12 @@ - (MMFullScreenWindow *)initWithWindow:(NSWindow *)t view:(MMVimView *)v
8182
view = [v retain];
8283

8384
[self setHasShadow:NO];
84-
[self setShowsResizeIndicator:NO];
8585
[self setBackgroundColor:back];
8686
[self setReleasedWhenClosed:NO];
8787

88+
// this disables any menu items for window tiling and for moving to another screen.
89+
[self setMovable:NO];
90+
8891
NSNotificationCenter *nc = [NSNotificationCenter defaultCenter];
8992
[nc addObserver:self
9093
selector:@selector(windowDidBecomeMain:)
@@ -169,7 +172,11 @@ - (void)enterFullScreen
169172

170173
// NOTE: The window may have moved to another screen in between init.. and
171174
// this call so set the frame again just in case.
172-
[self setFrame:[[target screen] frame] display:NO];
175+
NSScreen* screen = [target screen];
176+
if (screen == nil) {
177+
screen = [NSScreen mainScreen];
178+
}
179+
[self setFrame:[screen frame] display:NO];
173180

174181
oldTabBarStyle = [[view tabBarControl] styleName];
175182

@@ -178,8 +185,6 @@ - (void)enterFullScreen
178185
[[view tabBarControl] setStyleNamed:style];
179186

180187
// add text view
181-
oldPosition = [view frame].origin;
182-
183188
[view removeFromSuperviewWithoutNeedingDisplay];
184189
[[self contentView] addSubview:view];
185190
[self setInitialFirstResponder:[view textView]];
@@ -195,9 +200,18 @@ - (void)enterFullScreen
195200
}
196201

197202
[self setAppearance:target.appearance];
198-
199203
[self setOpaque:[target isOpaque]];
200204

205+
// Copy the collection behavior so it retains the window behavior (e.g. in
206+
// Stage Manager). Make sure to set the native full screen flags to "none"
207+
// as we want to prevent macOS from being able to take this window full
208+
// screen (e.g. via the Window menu or dragging in Mission Control).
209+
NSWindowCollectionBehavior wcb = target.collectionBehavior;
210+
wcb &= ~(NSWindowCollectionBehaviorFullScreenPrimary);
211+
wcb &= ~(NSWindowCollectionBehaviorFullScreenAuxiliary);
212+
wcb |= NSWindowCollectionBehaviorFullScreenNone;
213+
[self setCollectionBehavior:wcb];
214+
201215
// reassign target's window controller to believe that it's now controlling us
202216
// don't set this sooner, so we don't get an additional
203217
// focus gained message
@@ -206,27 +220,16 @@ - (void)enterFullScreen
206220

207221
// Store view dimension used before entering full-screen, then resize the
208222
// view to match 'fuopt'.
209-
[[view textView] getMaxRows:&nonFuRows columns:&nonFuColumns];
210223
nonFuVimViewSize = view.frame.size;
211224

212225
// Store options used when entering full-screen so that we can restore
213226
// dimensions when exiting full-screen.
214227
startFuFlags = options;
215228

216-
// HACK! Put window on all Spaces to avoid Spaces (available on OS X 10.5
217-
// and later) from moving the full-screen window to a separate Space from
218-
// the one the decorated window is occupying. The collection behavior is
219-
// restored further down.
220-
NSWindowCollectionBehavior wcb = [self collectionBehavior];
221-
[self setCollectionBehavior:NSWindowCollectionBehaviorCanJoinAllSpaces];
222-
223229
// make us visible and target invisible
224230
[target orderOut:self];
225231
[self makeKeyAndOrderFront:self];
226232

227-
// Restore collection behavior (see hack above).
228-
[self setCollectionBehavior:wcb];
229-
230233
// fade back in
231234
if (didBlend) {
232235
[NSAnimationContext currentContext].completionHandler = ^{
@@ -252,22 +255,6 @@ - (void)leaveFullScreen
252255
}
253256
}
254257

255-
// restore old vim view size
256-
int currRows, currColumns;
257-
[[view textView] getMaxRows:&currRows columns:&currColumns];
258-
int newRows = nonFuRows, newColumns = nonFuColumns;
259-
260-
// resize vim if necessary
261-
if (currRows != newRows || currColumns != newColumns) {
262-
int newSize[2] = { newRows, newColumns };
263-
NSData *data = [NSData dataWithBytes:newSize length:2*sizeof(int)];
264-
MMVimController *vimController =
265-
[[self windowController] vimController];
266-
267-
[vimController sendMessage:SetTextDimensionsMsgID data:data];
268-
[[view textView] setMaxRows:newRows columns:newColumns];
269-
}
270-
271258
// fix up target controller
272259
[self retain]; // NSWindowController releases us once
273260
[[self windowController] setWindow:target];
@@ -277,7 +264,17 @@ - (void)leaveFullScreen
277264
// fix delegate
278265
id delegate = [self delegate];
279266
[self setDelegate:nil];
280-
267+
268+
// if this window ended up on a different screen, we want to move the
269+
// original window to this new screen.
270+
if (self.screen != target.screen && self.screen != nil && target.screen != nil) {
271+
NSPoint topLeftPos = NSMakePoint(NSMinX(target.frame) - NSMinX(target.screen.visibleFrame),
272+
NSMaxY(target.frame) - NSMaxY(target.screen.visibleFrame));
273+
NSPoint newTopLeftPos = NSMakePoint(NSMinX(self.screen.visibleFrame) + topLeftPos.x,
274+
NSMaxY(self.screen.visibleFrame) + topLeftPos.y);
275+
[target setFrameTopLeftPoint:newTopLeftPos];
276+
}
277+
281278
// move text view back to original window, hide fullScreen window,
282279
// show original window
283280
// do this _after_ resetting delegate and window controller, so the
@@ -286,42 +283,50 @@ - (void)leaveFullScreen
286283
[view removeFromSuperviewWithoutNeedingDisplay];
287284
[[target contentView] addSubview:view];
288285

289-
[view setFrameOrigin:oldPosition];
290286
[self close];
291287

292288
// Set the text view to initial first responder, otherwise the 'plus'
293289
// button on the tabline steals the first responder status.
294290
[target setInitialFirstResponder:[view textView]];
295291

296-
// HACK! Put decorated window on all Spaces (available on OS X 10.5 and
297-
// later) so that the decorated window stays on the same Space as the full
298-
// screen window (they may occupy different Spaces e.g. if the full-screen
299-
// window was dragged to another Space). The collection behavior is
300-
// restored further down.
301-
NSWindowCollectionBehavior wcb = [target collectionBehavior];
302-
[target setCollectionBehavior:NSWindowCollectionBehaviorCanJoinAllSpaces];
303-
304-
#if (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7)
305-
// HACK! On Mac OS X 10.7 windows animate when makeKeyAndOrderFront: is
306-
// called. This is distracting here, so disable the animation and restore
307-
// animation behavior after calling makeKeyAndOrderFront:.
308-
NSWindowAnimationBehavior a = NSWindowAnimationBehaviorNone;
309-
if ([target respondsToSelector:@selector(animationBehavior)]) {
310-
a = [target animationBehavior];
311-
[target setAnimationBehavior:NSWindowAnimationBehaviorNone];
312-
}
313-
#endif
292+
// On Mac OS X 10.7 windows animate when makeKeyAndOrderFront: is called.
293+
// This is distracting here, so disable the animation and restore animation
294+
// behavior after calling makeKeyAndOrderFront:.
295+
NSWindowAnimationBehavior winAnimBehavior = [target animationBehavior];
296+
[target setAnimationBehavior:NSWindowAnimationBehaviorNone];
297+
298+
// Note: Currently, there is a possibility that the full-screen window is
299+
// in a different Space from the original window. This could happen if the
300+
// full-screen was manually dragged to another Space in Mission Control.
301+
// If that's the case, the original window will be restored to the original
302+
// Space it was in, which may not be what the user intended.
303+
//
304+
// We don't address this for a few reasons:
305+
// 1. This is a niche case that wouldn't matter 99% of the time.
306+
// 2. macOS does not expose explicit control over Spaces in the public APIs.
307+
// We don't have a way to directly determine which space each window is
308+
// on, other than just detecting whether it's on the active space. We
309+
// also don't have a way to place the window on another Space
310+
// programmatically. We could move the window to the active Space by
311+
// changing collectionBehavior to CanJoinAllSpace or MoveToActiveSpace,
312+
// and after it's moved, unset the collectionBehavior. This is tricky to
313+
// do because the move doesn't happen immediately. The window manager
314+
// takes a few cycles before it moves the window over to the active
315+
// space and we would need to continually check onActiveSpace to know
316+
// when that happens. This leads to a fair bit of window management
317+
// complexity.
318+
// 3. Even if we implement the above, it could still lead to unintended
319+
// behaviors. If during the window restore process, the user navigated
320+
// to another Space (e.g. a popup dialog box), it's not necessarily the
321+
// correct behavior to put the restored window there. What we want is to
322+
// query the exact Space the full-screen window is on and place the
323+
// original window there, but there's no public APIs to do that.
314324

315325
[target makeKeyAndOrderFront:self];
316326

317-
#if (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7)
318-
// HACK! Restore animation behavior.
319-
if (NSWindowAnimationBehaviorNone != a)
320-
[target setAnimationBehavior:a];
321-
#endif
322-
323-
// Restore collection behavior (see hack above).
324-
[target setCollectionBehavior:wcb];
327+
// Restore animation behavior.
328+
if (NSWindowAnimationBehaviorNone != winAnimBehavior)
329+
[target setAnimationBehavior:winAnimBehavior];
325330

326331
// ...but we don't want a focus gained message either, so don't set this
327332
// sooner
@@ -365,16 +370,11 @@ - (void)applicationDidChangeScreenParameters:(NSNotification *)notification
365370
// hidden/displayed.
366371
ASLogDebug(@"Screen unplugged / resolution changed");
367372

368-
NSScreen *screen = [target screen];
369-
if (!screen) {
370-
// Paranoia: if window we originally used for full-screen is gone, try
371-
// screen window is on now, and failing that (not sure this can happen)
372-
// use main screen.
373-
screen = [self screen];
374-
if (!screen)
375-
screen = [NSScreen mainScreen];
373+
NSScreen *screen = [self screen];
374+
if (screen == nil) {
375+
// See windowDidMove for more explanations.
376+
screen = [NSScreen mainScreen];
376377
}
377-
378378
// Ensure the full-screen window is still covering the entire screen and
379379
// then resize view according to 'fuopt'.
380380
[self setFrame:[screen frame] display:NO];
@@ -549,12 +549,24 @@ - (void)windowDidMove:(NSNotification *)notification
549549
if (state != InFullScreen)
550550
return;
551551

552-
// Window may move as a result of being dragged between Spaces.
552+
// Window may move as a result of being dragged between screens.
553553
ASLogDebug(@"Full-screen window moved, ensuring it covers the screen...");
554554

555+
NSScreen *screen = [self screen];
556+
if (screen == nil) {
557+
// If for some reason this window got moved to an area not associated
558+
// with a screen just fall back to a main one. Otherwise this window
559+
// will be stuck on a no-man's land and the user will have no way to
560+
// use it. One known way this could happen is when the user has a
561+
// larger monitor on the left (where MacVim was started) and a smaller
562+
// on the right. The user then drag the full screen window to the right
563+
// screen in Mission Control. macOS will refuse to place the window
564+
// because it is too big so it gets placed out of bounds.
565+
screen = [NSScreen mainScreen];
566+
}
555567
// Ensure the full-screen window is still covering the entire screen and
556568
// then resize view according to 'fuopt'.
557-
[self setFrame:[[self screen] frame] display:NO];
569+
[self setFrame:[screen frame] display:NO];
558570
}
559571

560572
@end // MMFullScreenWindow (Private)

0 commit comments

Comments
 (0)