Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core Text rendering issues with macOS 10.14 Mojave #751

Closed
jaingaurav opened this Issue Sep 23, 2018 · 48 comments

Comments

Projects
None yet
@jaingaurav
Copy link

jaingaurav commented Sep 23, 2018

I just did a fresh install of macOS 10.14 Mojave and found that MacVim has a number of rendering issues while scrolling. The test gets garbled and the window can sometimes go completely black.

Seems like the workaround was to disable Core Text rending in the advanced preferences.

@mario-grgic

This comment has been minimized.

Copy link

mario-grgic commented Sep 24, 2018

This happens only when you compile MacVim with 10.14 SDK (previous SDK works fine). For me the MacVim GUI flashes dark/light grey with desert color scheme in rhythm with the cursor blinking, if "Core Text Renderer" is turned on.

@ychin

This comment has been minimized.

Copy link
Member

ychin commented Sep 25, 2018

This is because of a Mojave change that made all NSView automatically become layer-backed, which will break MacVim's rendering since it relies on behaviors from the legacy renderer. Fortunately it only happens when you link against the new 10.14 SDK so old builds would still work, but we would need a long term solution, presumably a Metal-based renderer similar to other fancy high-performance terminal emulator like Alacritty or Kitty.

Currently the CoreText renderer also supports a CGLayer mode but it has miscellaneous issues including slow scrolling so I don't think that's the way forward.

More info:

@amadeus

This comment has been minimized.

Copy link

amadeus commented Sep 25, 2018

Unfortunately it appears that MacVim is completely unusable on Mojave. Is there any temporary fix?

@ychin

This comment has been minimized.

Copy link
Member

ychin commented Sep 25, 2018

@amadeus Are you building MacVim yourself using Xcode 10 on Majave? If so, you can build using the old SDK which will fix this issue. Easiest way is to download Xcode 9.4.1 from Apple Developer and build using that. If you are downloading builds from official releases I don't think it should be broken.

Otherwise you can do this in Terminal to turn on CGLayer mode: defaults write org.vim.MacVim MMUseCGLayerAlways -bool YES. I don't recommend this route though as it makes scrolling slow.

@J-Liu

This comment has been minimized.

Copy link

J-Liu commented Sep 25, 2018

@joelfrederico

This comment has been minimized.

Copy link

joelfrederico commented Sep 26, 2018

@ychin Could you give me a bit more detail on the problem? I'd like to hack a bit on it, although I predict I won't be able to patch it before the regular MacVim committers have fixed it. It seems to me like the issue is someplace in NSWindow not having a background color set? And you want to fix it by dropping Core Text entirely and switching to Metal/Metal2?

@ychin

This comment has been minimized.

Copy link
Member

ychin commented Sep 27, 2018

@joelfrederico I was actually planning to implement a first-pass for a fix for this this weekend since I have been looking at the renderer code when fixing some fullscreen rendering bugs, but if you do want to tackle the issue feel free to do that too.

The way Vim works is that it treats the GUI as a dumb terminal and just issue draw calls to the GUI like it does for a terminal. The way MacVim draws is that it batches up these draw commands and calls setNeedsDisplay to tell Cocoa that it needs to redraw, and then in the callback drawRect it plays back those draw calls Vim sent it. MacVim itself doesn't cache the full state of the texts, only Vim does.

The issue is that these draw commands Vim sent it are only incremental draw commands assuming the previous draws are still valid. When you call setNeedsDisplay technically you are telling the OS to invalidate the whole window even though we are only interested in invalidating part of the window to draw. Previously it somehow worked because for some reason Cocoa didn't clear the whole window for us even when setNeedsDisplay is called when MacVim is in windowed mode. The new APIs always clear the screen when you call that function so you need to know how to redraw the whole screen instead of just playing back the incremental draw commands. Note that this was already a problem in non-native fullscreen which is why we automatically turn on CGLayer mode in that. I think we may kind of fix this by using setNeedsDisplayInRect instead and make sure to set the proper dirty rect, but it has some other issues and doesn't work with scrollRect. Or we could just cache the text states in the renderer so MacVim always knows how to redraw itself.

Another issue is we currently use NSView's scrollRect function to scroll the window (which is deprecated) which I think is GPU accelerated given how smooth MacVim's scrolling is. I think this function no longer work under the new change to layered back view, so we would need to implement scrolling using other ways. If we cache the text states as described above we could just re-draw the texts using CoreText when we scroll but it will be slower than the previous scrollRect implementation since text rendering is slower than a simple Blit operation on the GPU.

The CGLayer implementation (you can manually turn that on by doing defaults write org.vim.MacVim MMUseCGLayerAlways -bool YES) from what I understand was implemented to address these issues when a previous macOS update broke non-native fullscreen mode, by caching the drawn content onto a CGLayer backing. This way you could incrementally draw to the CGLayer, and then just present the layer when you need to draw. Sadly CGLayer is not GPU accelerated and that's why scrolling in CGLayer mode is a lot slower than the normal one on a high resolution display.

All of this is kind of messy which is why I think we should just implement a hardware accelerated renderer (using Metal) to replace CGLayer. We will still use CoreText for the text rendering. We could either just use a GPU texture as backing layer and draw texts on it which ties into the current renderer easier, or we could cache the font glyphs which would allow us to draw texts much quicker but may be tricky to implement (since MacVim supports ligature etc). Another issue with cached glyphs is they won't work (easily) with subpixel font smoothing, which isn't an issue in Mojave since that feature got removed anyway :(, but in older OS we may still want to have subpixel font rendering to be consistent with other parts of the OS.

This is the gist of what I understand of the rendering bug, maybe we should make another issue dedicated to writing a new renderer.

@chdiza

This comment has been minimized.

Copy link
Contributor

chdiza commented Sep 27, 2018

but in older OS we may still want to have subpixel font rendering to be consistent with other parts of the OS.

I think supporting older macOS is mandatory, but I'm not sure how compatible that is with using Metal.

@joelfrederico

This comment has been minimized.

Copy link

joelfrederico commented Sep 27, 2018

@ychin Thanks for the detailed description!

@joelfrederico I was actually planning to implement a first-pass for a fix for this this weekend since I have been looking at the renderer code when fixing some fullscreen rendering bugs, but if you do want to tackle the issue feel free to do that too.

Please don't change your plans thinking I'm working on it! I'm just getting my feet wet, I don't expect to actually be able to do much of anything.

@eirnym

This comment has been minimized.

Copy link
Contributor

eirnym commented Sep 28, 2018

but in older OS we may still want to have subpixel font rendering to be consistent with other parts of the OS.

I think supporting older macOS is mandatory, but I'm not sure how compatible that is with using Metal.

Metal first appeared in macOS El Capitan, but not every graphics card supports it. So we could have current implementation as a fallback if Metal is failed to load or it set in settings before Mojave and Metal backend only on macOS Mojave+.

@chdiza

This comment has been minimized.

Copy link
Contributor

chdiza commented Sep 30, 2018

Download the binary form https://github.com/macvim-dev/macvim/releases

This didn't solve anything on my Mojave machine. MacVim (the whole window) blinks along with the cursor.

@J-Liu

This comment has been minimized.

Copy link

J-Liu commented Sep 30, 2018

Download the binary form https://github.com/macvim-dev/macvim/releases

This didn't solve anything on my Mojave machine. MacVim (the whole window) blinks along with the cursor.

It works, for me. 151 on Mojave.

@xiaohk

This comment has been minimized.

Copy link

xiaohk commented Oct 2, 2018

Any updates on this?

I have tried defaults write org.vim.MacVim MMUseCGLayerAlways -bool YES. It prevents the window from blinking, but scrolling makes the app crash...

I also downloaded 151 binary from https://github.com/macvim-dev/macvim/releases, it doesn't work for me.

@ychin

This comment has been minimized.

Copy link
Member

ychin commented Oct 3, 2018

@xiaohk How does the release not work for you? Does it crash or just blink a lot when you type? Do you have an external monitor? You can try to remove the CGLayer setting first by doing defaults delete org.vim.MacVim MMUseCGLayerAlways, and also maybe try a fresh vimrc. I'm mostly asking because I'm running Mojave and the release does work for me.

@felixbuenemann

This comment has been minimized.

Copy link
Contributor

felixbuenemann commented Oct 3, 2018

@ychin I think iTerm's Metal renderer uses pre-generated glyph images, it could serve as an example implementation.

@felixbuenemann

This comment has been minimized.

Copy link
Contributor

felixbuenemann commented Oct 3, 2018

Oh and it also has subpixel antialiasing, see this google doc for the technical details:

Subpixel Antialiasing in iTerm2's Metal Renderer

@xiaohk

This comment has been minimized.

Copy link

xiaohk commented Oct 3, 2018

@xiaohk How does the release not work for you? Does it crash or just blink a lot when you type? Do you have an external monitor? You can try to remove the CGLayer setting first by doing defaults delete org.vim.MacVim MMUseCGLayerAlways, and also maybe try a fresh vimrc. I'm mostly asking because I'm running Mojave and the release does work for me.

The release blinks with my cursor. I don't have an external monitor. I did try to set defaults delete org.vim.MacVim MMUseCGLayerAlways, but then scrolling froze MacVim.

@chdiza

This comment has been minimized.

Copy link
Contributor

chdiza commented Oct 3, 2018

The release blinks with my cursor.

This is what I keep getting, even when I wipe the preferences and even when I start up with --clean and all that.

If I turn off CoreText rendering in the prefs dialog the blinking stops.

I have a non-retina display in my laptop.

@fabiomcosta

This comment has been minimized.

Copy link

fabiomcosta commented Oct 4, 2018

Thx you all for working on this.
For now I'm just using normal vim as a fallback.

@s4y

This comment has been minimized.

Copy link
Contributor

s4y commented Oct 5, 2018

I hacked on this a while back but haven't touched it recently. I'm not convinced that a Metal rewrite is necessary! The renderer just needs the ability to draw arbitrary rects instead of relying on being able to draw on top of existing content. The tricky part is that, currently, it doesn't keep any kind of state; it just draws as it receives commands. That would be the biggest change, but not that big.

If redrawing everything is too expensive, there are strategies like using tiles that could be moved around without redrawing the whole window, or just using a layer per line, for example, and moving those to scroll without redrawing.

@agrieser agrieser referenced this issue Oct 5, 2018

Closed

Macvim install fails on MacOS 10.14 #32464

6 of 6 tasks complete
@y00rb

This comment has been minimized.

Copy link

y00rb commented Oct 6, 2018

Does it same issue? My window always flashing, can't display normal like before.

  1. Upgrade to Mojave
  2. upgrade macvim to latest. See flashing problem, so reinstall macvim
  3. brew uninstall macvim then brew update + brew install macvim

But problem is exists too
macvim

@balajirama

This comment has been minimized.

Copy link

balajirama commented Oct 6, 2018

I agree with the above observation. I see the same issue blinking window. It doesn't let me work.

@balajirama

This comment has been minimized.

Copy link

balajirama commented Oct 6, 2018

I see the blinking mccvim too. Exact same steps as y00rb above. Attached is a zipped movie.

macvim-blink.mov.gz

@balajirama

This comment has been minimized.

Copy link

balajirama commented Oct 6, 2018

My wife found out that mario-grgic pointed out that I need to turn off "Core Text renderer". I did that and it still blinked. But I found out that if we turn off "Prefer Native full-screen support". That will help.

Attached is a screenshot of what you need to do:

image

@mario-grgic

This comment has been minimized.

Copy link

mario-grgic commented Oct 6, 2018

Turning off "Use Core Text renderer" and restarting (i.e. completely quit application, not just close window) should do it alone.

@balajirama

This comment has been minimized.

Copy link

balajirama commented Oct 7, 2018

Yes. Thanks a lot!

@y00rb

This comment has been minimized.

Copy link

y00rb commented Oct 7, 2018

@balajirama Thank you very much!!! I did disable Use Core Text renderer, then It works right now
@mario-grgic Thank you very much too! I did read it, but not very understand how turn off until see the screen-shot

@s4y

This comment has been minimized.

Copy link
Contributor

s4y commented Oct 25, 2018

Cross-posting this from my PR: my job’s okay with me spending some work time on this, but only if there’s a top-level LICENSE file (and there currently isn’t). If one can be added first, I’ll pursue a fix.

@J-Liu

This comment has been minimized.

Copy link

J-Liu commented Nov 11, 2018

ping

@s4y

This comment has been minimized.

Copy link
Contributor

s4y commented Nov 14, 2018

@J-Liu

This comment has been minimized.

Copy link

J-Liu commented Nov 14, 2018

@s4y
thx, but I'm waiting for a snapshot 152 dmg file...

@holycheater

This comment has been minimized.

Copy link

holycheater commented Nov 22, 2018

Snapshot 151 works for me on MBP15 with Mojave.
Doesn't work from homebrew, probably still older version

@alswl

This comment has been minimized.

Copy link

alswl commented Nov 23, 2018

@oseiberts11

This comment has been minimized.

Copy link

oseiberts11 commented Nov 23, 2018

My homebrew version (claiming to be 151) on MacOS 10.14.1 was still doing the flashing, until I disabled "Use Core Text Renderer" in the Preferences.

s4y added a commit to s4y/macvim that referenced this issue Nov 24, 2018

Fix rendering issues when built against the 10.14 SDK.
Before this commit, the Core Text renderer relied on a legacy behavior
of NSView to keep its content across draws. setNeedsDisplayInRect: could
be used to draw over parts of the existing content as drawing commands
were received without needing to redraw old content.

Layer-backed views lose this behavior and may be asked to redraw any or
all of their content at any time, and layer backing becomes the default
for apps built against the macOS 10.14 SDK.

This change adds a way to draw to an intermediate NSImageRep and then
draw that to the view. It's similar to the CGLayer path, but I wasn't
able to get the CGLayer path to work without hanging or crashing when I
scrolled. My best guess from looking at stack traces is that using
CGContextDrawLayerInRect to draw a layer into itself doesn't actually
copy pixels, but adds the self-draw as an action to be performed when
the CGLayer is drawn into a bitmap context. Scrolling stacks these
actions, which either hang or overflow the stack when drawn.

The new code is controlled by the MMDrawToImage user default, which is
on by default in this change. Fixes macvim-dev#751.
@alswl

This comment has been minimized.

Copy link

alswl commented Nov 27, 2018

@oseiberts11 disable Use Core Text Renderer makes MacVim horrible for CJK IME user.

@9mm

This comment has been minimized.

Copy link

9mm commented Nov 27, 2018

@alswl yes my vim looks like burnt toast, it's horrible to code in. I'm just hanging in there until it's fixed, hopefully within a week or something :/

@alswl

This comment has been minimized.

Copy link

alswl commented Nov 27, 2018

My alternative solution for CJK IME user is using vim in Terminal.
😂

@loadaverage

This comment has been minimized.

Copy link

loadaverage commented Nov 27, 2018

@amadeus Are you building MacVim yourself using Xcode 10 on Majave? If so, you can build using the old SDK which will fix this issue. Easiest way is to download Xcode 9.4.1 from Apple Developer and build using that. If you are downloading builds from official releases I don't think it should be broken.

Otherwise you can do this in Terminal to turn on CGLayer mode: defaults write org.vim.MacVim MMUseCGLayerAlways -bool YES. I don't recommend this route though as it makes scrolling slow.

Can't use this version due homebrew error:

Error: Your Xcode (9.4.1) is too outdated.
Please update to Xcode 10.1 (or delete it).
Xcode can be updated from the App Store.

s4y added a commit to s4y/macvim that referenced this issue Dec 2, 2018

Fix rendering issues with the 10.14 SDK.
Before this commit, the Core Text renderer relied on a legacy behavior
of NSView to keep its content across draws. setNeedsDisplayInRect: drew
over parts of the existing content as drawing commands were received,
without ever redrawing old content.

Layer backed views don't preserve content between draws and may be asked
to redraw at any time, and layer backing is default on in the 10.14 SDK.

This change adds a way to draw to a CGBitmapContext and then display
that in the view's layer. It's similar to the CGLayer path, but I wasn't
able to get the CGLayer path to work without hanging or crashing when I
scrolled. My best guess from looking at stack traces is that using
CGContextDrawLayerInRect to draw a layer into itself doesn't actually
copy pixels, but adds the self-draw as an action to be performed when
the CGLayer is drawn into a bitmap context. Scrolling stacks these
actions, which either hangs or overflows the stack when drawn.

The new code is controlled by the MMBufferedDrawing user default, which
is on by default on macOS >= 10.14 with this change. Fixes macvim-dev#751.

s4y added a commit to s4y/macvim that referenced this issue Dec 2, 2018

Fix rendering issues with the 10.14 SDK.
Before this commit, the Core Text renderer relied on a legacy behavior
of NSView to keep its content across draws. setNeedsDisplayInRect: drew
over parts of the existing content as drawing commands were received,
without ever redrawing old content.

Layer backed views don't preserve content between draws and may be asked
to redraw at any time, and layer backing is default on in the 10.14 SDK.

This change adds a way to draw to a CGBitmapContext and then display
that in the view's layer. It's similar to the CGLayer path, but I wasn't
able to get the CGLayer path to work without hanging or crashing when I
scrolled. My best guess from looking at stack traces is that using
CGContextDrawLayerInRect to draw a layer into itself doesn't actually
copy pixels, but adds the self-draw as an action to be performed when
the CGLayer is drawn into a bitmap context. Scrolling stacks these
actions, which either hangs or overflows the stack when drawn.

The new code is controlled by the MMBufferedDrawing user default, which
is on by default on macOS >= 10.14 with this change. Fixes macvim-dev#751.
@a7hybnj2

This comment has been minimized.

Copy link

a7hybnj2 commented Dec 3, 2018

Just upgraded via "brew upgrade macvim" and mvim started to this issue. Luckily disabling CORE worked for me. I look forward to the update.

@ychin ychin closed this in #757 Dec 4, 2018

@ychin

This comment has been minimized.

Copy link
Member

ychin commented Dec 4, 2018

PR #757 should have fixed this issue. Sorry for the wait. Feel free to re-open if it didn't fix the issue. Note that there's a slight performance impact to scrolling but it should be acceptable, and it's a known issue.

@a7hybnj2

This comment has been minimized.

Copy link

a7hybnj2 commented Dec 4, 2018

Great! I will be watching brew for when the update is available.

@amcgregor

This comment has been minimized.

Copy link

amcgregor commented Dec 4, 2018

@a7hybnj2 I can confirm, if you install the Homebrew version with --HEAD, the fix is included.

How I updated:

brew uninstall macvim
brew install macvim --HEAD --with-override-system-vim --with-luajit
open /usr/local/Cellar/macvim

After that, I drag the MacVim app bundle/icon from the subdirectory of the now open Finder window into my Dock. Done and done.

@ApolloTang

This comment has been minimized.

Copy link

ApolloTang commented Dec 5, 2018

if you try to install macvim via brew install macvim --HEAD but fail to compile please see #790 (comment)

@ychin ychin added the Renderer label Dec 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.