Skip to content

Commit 383be18 broke fullscreen mode on El_Capitan #127

Closed
Closed
Commit 383be18 broke fullscreen mode on El_Capitan#127

Description

@chdiza
chdiza
opened on Nov 10, 2015

383be18 attempted to fix Split Screen mode in ElCap. Unfortunately, it broke "regular" fullscreen (i.e., fullscreen but not split) on ElCap.

Here is the way in which fullscreen is now broken on ElCap. It holds no matter whether we're talking about the "Lion style" fullscreen or the older fullscreen mode. Both exhibit the same basic brokenness, though the Lion style mode has it slightly worse.

To reproduce: open a new MacVim window and :e somefile so that text appears in the window. Now go into fullscreen mode. Now shrink the font size using "Cmd-".

Expected behavior: the font size gets smaller, but MacVim stays in fullscreen mode and auto-resets &columns and &rows to expand. This was also the actual behavior prior to 383be18.

Actual Behavior:

  1. On "Lion style" fullscreen, the fonts size does get smaller, but MacVim doesn't auto-reset &rows and &columns; also, MacVim ignores the background key in &fuoptions.
  2. On "old style" fullscreen, the font size gets smaller, and MacVim still covers up all of the desktop, but it does not auto-reset &rows and &columns to fill the new extra space. Here at least MacVim obeys &fuoptions.

I'm shocked I didn't notice this almost immediately, since I frequently tweak the font size while in fullscreen. (Helps to fit more text on the screen when there are long lines.)

As far as I can tell, this only affects El Capitan.

Activity

  1. chdiza commented on Nov 10, 2015

    @chdiza
    chdiza
    on Nov 10, 2015
    ContributorAuthor

    Here are before & after screen shots.

    This is "before". I.e., just in fullscreen mode:
    screen shot 2015-11-10 at 12 03 26 a

    Here is what it looks like after shrinking the font size with Cmd-in "Lion-style" fullscreen mode:

    screen shot 2015-11-10 at 12 03 59 a

    I have fuoptions set so that the background should be the same color as the text background. In that photo you can see that it's not.

    Here is a second "after" shot, only this time I'm in the "old-style" fullscreen mode:
    screen shot 2015-11-10 at 12 03 33 a

    There you can see that MacVim is obeying fuoptions.

    But both of the after pictures exhibit the brokenness. The place where text goes should have auto-adjusted to fill the newly available space after the font size shrank.

  2. chdiza commented on Nov 12, 2015

    @chdiza
    chdiza
    on Nov 12, 2015
    ContributorAuthor

    The commit that caused the breakage added this:

    +    if (floor(NSAppKitVersionNumber) > NSAppKitVersionNumber10_10_Max) {
+        // NOTE: Prevent to resize the window in Split View on El Capitan or
+        // later.
+        return NO;
+    }

    I'm not a Cocoa programmer, but there must be a way to wrap this in a conditional that first checks whether we're in Split Screen mode.

  3. limintang commented on Jan 18, 2016

    @limintang
    limintang
    on Jan 18, 2016

    This commit also causes another issue: the native full screen on El Capitan sometimes does not work correctly:

    screen shot

  4. jordwalke commented on May 26, 2016

    @jordwalke
    jordwalke
    on May 26, 2016
    Contributor

    I definitely noticed this too. It's a pain to fix the window size when resizing font size because you have to exist full screen mode and then reenter fullscreen mode again (with the animations, it takes a while to do both).

  5. chdiza commented on May 26, 2016

    @chdiza
    chdiza
    on May 26, 2016
    ContributorAuthor

    Yeah, this is irritating. I apply a patch locally that reverts the bad commit. I never use "Split Screen", so I don't mind having a Vim that has poorly behaving Split Screen.

    Split Screen is new and niche; fullscreen is not. Until both can be non-broken, I would have thought that proper fullscreen is more important.

  6. jordwalke commented on May 26, 2016

    @jordwalke
    jordwalke
    on May 26, 2016
    Contributor

    Agreed about split screen. I haven't even tried it because I just assumed Apple would apply some gratuitous animation that makes me wait for 2 seconds before it takes effect.

  7. s4y commented on Apr 1, 2017

    @s4y
    s4y
    on Apr 1, 2017
    Contributor

    I put a branch up that I believe fixes the root cause of this issue:

    master...s4y:s4y/fullscreen_contentSize

    There is still a flicker caused by Vim updating the screen twice. It may have been there before, but I want to track down before I make a PR. Unfortunately, I'm coming down with a cold and I'm going to try very hard not to work on it until I feel better, so I'm putting the patch out there in its current state. (The maintainers are welcome to merge it if they want, since it makes changing the font size in full screen possible at least.)

  8. s4y commented on Apr 2, 2017

    @s4y
    s4y
    on Apr 2, 2017
    Contributor

    @chdiza Since you alerted me to this, I'd appreciate your help in giving the above change a workout.

  9. chdiza commented on Apr 2, 2017

    @chdiza
    chdiza
    on Apr 2, 2017
    ContributorAuthor

    I'd appreciate your help in giving the above change a workout.

    Will do!

  10. chdiza commented on Apr 2, 2017

    @chdiza
    chdiza
    on Apr 2, 2017
    ContributorAuthor

    I'm not sure I applied the patch properly, but I sure hope I did, because whatever I did appears at first glance to work like a charm. Not only does it remove the need for my workaround, and not only does it allow font size changing while in NNFS without keeping &colums fixed, but ... is it too good to be true? ... It seems like it might have eliminated the various ephemeral NNFS artifacts introduced by #405.

    I'll test more tomorrow when I am well-rested, to make sure I didn't do something silly.

  11. chdiza commented on Apr 2, 2017

    @chdiza
    chdiza
    on Apr 2, 2017
    ContributorAuthor

    It was too good to be true, unfortunately. With that patch applied, then even when NOT in fullscreen I can't change the &lines or &columns manually by doing :se lines=40 for instance. There is a quick flicker, which makes me think that &lines is being changed and then immediately set back to where it was.

    That said, I'm not seeing any problems with the patch with resepect to NNFS stuff.

  12. chdiza commented on Apr 2, 2017

    @chdiza
    chdiza
    on Apr 2, 2017
    ContributorAuthor

    For instance, my default MacVim window size is 80x50. If I do :se lines=25, then there is a very fast flicker, but the window doesn't change size and there are still 50 lines of text.

  13. chdiza commented on Apr 2, 2017

    @chdiza
    chdiza
    on Apr 2, 2017
    ContributorAuthor

    I note that I can adjust the size of the MacVim window with the mouse, and it will stick.

  14. chdiza
    added a commit that references this issue on Apr 2, 2017

    Fix the contentSize calculation for changing font size in full screen.

    9f6bb78

  15. s4y commented on Apr 3, 2017

    @s4y
    s4y
    on Apr 3, 2017
    Contributor

    @chdiza: Thanks, that's exactly the kind of issue I was worried about.

    I pushed an update to the same branch (master...s4y:s4y/fullscreen_contentSize). It only affects full screen now, so setting lines and columns should work again. Mind giving it another shot?

    It feels a little bit hacky, but it's the best approach I can find right now.

  16. chdiza commented on Apr 3, 2017

    @chdiza
    chdiza
    on Apr 3, 2017
    ContributorAuthor

    @s4y Thanks. I did some quick testing (more to follow later of course). It's true that &lines and &columns work again, and it's true that the patch fixes the NNFS worries mentioned in this and the other thread.

    I see one regression: If I open a MacVim window (not fullscreen, but regular, with dimensions e.g., 80x40), then go into NNFS, then increase the font size by an incremement, then exit NNFS, then MacVim forgets that it's supposed to be 80x40 and instead winds up at 80 x N where N is however many &lines fit on the monitor (69 in this particular instance).

    If I then go back into NNFS, then exit NNFS (without tweaking the font size any further), then MacVim forgets what the non-fullscreen values of &columns were supposed to be too. For instance, if I started the whole procedure with 80x40 and follow the process described, the MacVim window ends up having as many columns as fit on the monitor and as many rows will fit on the monitor (but of course without being in fullscreen).

    This behavior was not the case before.

    Again, I'm not a Cocoa programmer, but it "smells" like we're closing in on the answer 😄.

  17. chdiza commented on Apr 5, 2017

    @chdiza
    chdiza
    on Apr 5, 2017
    ContributorAuthor

    @s4y I've been continuing to test today, on two different machines. What I said in the previous comment continues to hold.

  18. chdiza commented on Apr 6, 2017

    @chdiza
    chdiza
    on Apr 6, 2017
    ContributorAuthor

    @s4y It seems that the "can't remember what the dimensions are supposed to be outside of NNFS" problem was not introduced by your patch. Snapshot 130 shows this behavior too, at least if the font size is bumped by at least two increments.

    MacVim used to not behave like that, which is why I called it a regression. But I was apparently wrong in claiming that the regression was introduced by your patch. This makes me think that the patch is good to merge, at least provisionally. :)

  19. chdiza
    added a commit that references this issue on Apr 9, 2017

    Choose how to resize the Vim view based on the full screen mode.

    6d2e8f8
  20. splhack
    closed this as completedin #495on Apr 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions