(updated, now with 30% less code-churn)

updated to support the whole :sign thingys

👍 on the surface, building with these commits seems to work! I'll continue to test and see if I can find anything.

This appears to get rid of the disappearing text problem, but it's really herky-jerky on about half of all switches between full and non-full screens, particularly if one does not have MMFullScreenFadeTime set to 0.0 in the pref plist.

Even with it set to 0.0 (which I like), there are occasional delayed reactions going into or coming out of fullscreen, or times when the window won't respond to a request to switch.

Nevertheless, this is a good start! I'm glad someone is finally working on fixing this problem. It's single-handedly preventing me from upgrading to Sierra.

@chdiza mind doing a quick screen cap so I can see what you're talking about more specifically?

👍

It looks good it looks similar with Chromium implementation https://chromium.googlesource.com/chromium/chromium/+/master/content/browser/renderer_host/backing_store_mac.mm

Could you update scrolling part as Chromium's? Apparently no need to clone CGLayer in 10.6 and later. https://chromium.googlesource.com/chromium/chromium/+/master/content/browser/renderer_host/backing_store_mac.mm#190
And it would be good to adjust code formatting, revert unrelated changes and squash commits.

@splhack ok, will see if drawing from layer -> layer's own context works. looks like it should.

re: revert unrelated, the only unrelated change here is the ASLogNotice lines, with that change it compiles with MM_DEBUG_DRAWING=1, otherwise I get:

/Users/ben/src/macvim/src/MacVim/MMCoreTextView.m:894:36: warning: format specifies type 'char *' but the argument has type 'SEL' [-Wformat]
    ASLogNotice(@"====> BEGIN %s", _cmd);
                              ~~   ^~~~

happy to put that in a separate PR if you like tho.

re: code formatting -- didn't find a style guide, just followed best I could. What do you need here?

@splhack yup, it worked to draw the existing layer on top of its own context. Also cleaned and squashed some.

@chdiza yeah ok, I see what you mean now; after the fade to black it sometimes re-presents the view at its old size for too long.

@chdiza is this branch any help to the jerking problems? osheroff#1 I honestly can't really tell.

I can't get the osheroff#1 patch to apply.

patch -p1 <~/1.patch
patching file src/MacVim/MMCoreTextView.m
Hunk #2 FAILED at 604.
Hunk #3 succeeded at 626 (offset -1 lines).
Hunk #4 succeeded at 916 (offset -1 lines).
1 out of 4 hunks FAILED -- saving rejects to file src/MacVim/MMCoreTextView.m.rej
patching file src/MacVim/MMFullScreenWindow.m

yeah, the patch is on top of the osheroff/master branch. you'd need both.

OK, I won't have time to test until tomorrow.

Was able to test just now. It doesn't help, and indeed even seems worse in the few minutes of playing. Also it spams a bunch of stuff to the terminal when started with mvim, but that's probably debugging code you turned on.

ok with fade-time of 0.0 it's pretty glitchy. I think it kinda makes it better with the default fade time.

The opposite. I forgot to mention that my "worse" result (in the osheroff repo) was with default fade time. I did not test it with my preferred 0.0 (which is mostly working fine in this PR).

huh. plz do do a screen-cap (or maybe just record yr screen with your phone, for me screen cap doesn't get the fade-out), I'm just not sure I'm seeing what you're seeing.

at any rate, I wouldn't think the full-screen jerks should block a merge (given 10.12 is busted in this config without the patch). What you say @splhack? Mergable?

@osheroff I thought it's mergable but I felt some rendering lag when I tested this diff on 10.11.6. Maybe need runtime version check (10.12 or later) to enable this backing CGLayer.

I don't know how to record my screen, and I don't have a mobile device. When I get a chance, I will try to describe in detail what I see.

The present PR, built and used on El Cap, and MMFullScreenFadeTime of 0.0, results in a small visual regression. Specifically, once the full screen is painted with the background color, but before any of the text in the Vim window gets displayed, there is the quick blink of a small rectangle that is exactly the size of the cursor. This blink happens in a strange place, namely about 1/3 of the way down from the top of the screen, which is of course not where the real cursor ends up once the Vim window's text gets drawn atop the fullscreen background. My colorscheme is dark background/white cursor, so what blinks is a little white rectangle, cursor shaped.

I don't recall this glitch being present when I built this PR on Sierra. I'll look for it when I get a chance.

I don't recall this glitch being present when I built this PR on Sierra. I'll look for it when I get a chance.

It is there on Sierra too.

It happens upon exiting fullscreen mode.

To reproduce (Sierra and ElCap), set the MMFullScreenFadeTime to 0.0 (I doubt this has anything to do with it, but you never know). Then mvim -u NONE -U NONE -N and make the MacVim window take up the left half of the screen and all of the vertical space. Then enter non-native fullscreen. Then exit it. I see a black, cursor-sized rectangle appear in the leftmost column, about an inch down from the Mac menu bar. Then the MacVim window is drawn and the cursor shows up in the right place (northwest corner).

The location of the blink is influenced in some way by the size of the MacVim window prior to entering fullscreen. If the MacVim window is really tall to start with (like full height of screen), the blink happens an inch or two from the top of the screen. If it's a bit shorter, the blink happens further down (two or three inches). If the MacVim window starts out around half the height of the screen, the blink occurs near the bottom of the window. If the MacVim window starts out short (e.g., 25 rows), the blink is invisible, probably because it's "happening" below the last line of the MacVim window.

@splhack pointers on how to do a portable runtime OS check? I'm really quite new to osx coding.

The other option regarding latency; it seemed at some point that painting on the layer inside performBatchDrawWithData improved latency somewhat -- the layer was then ready and painted from a background thread. the only issue was a need to introduce locking.

I've been using this for a couple days and I think it's at least worth merging in its current state since it fixes a major issue. Improvements to performance and improving the transitions could come in later PRs.

@splhack I tried to reproduce lag issues on a 10.9 box with no luck. I am upgrading it to 10.11 to see, but it seemed just as fast as the master branch.

@chdiza I did a survey of three different full-screen-entering code paths:

The master branch:

this branch:

https://github.com/osheroff/macvim/tree/smoothout_fullscreen_enter

As you can see, both the master branch and this branch have roughly the same problem with drawing intermediate results; it doesn't seem like this branch makes the situation all that much worse than it was in the master branch. The approach of "blank out the text view until we've resized" appears to show promise; I think I got the timing better than in the last patch, at it doesn't show much intermediate state.

I'll add runtime version check for the rendering lag.