Stateful, fast renderer for MMCoreTextView #858

s4y · 2019-02-19T02:17:37Z

This change replaces the current rendering paths in MMCoreTextView with one which draws from an array of structs that represent the state of each character cell. It applies draw commands to the array
and marks any touched cells as needing display instead of drawing to a view or image buffer.

In my own tests, this renderer seems to be roughly twice as fast of the current one. Because it seems to work for the use cases of all of the current renderers and reused a lot of code, this PR replaces the other paths instead of adding a preference.

Fixes #796.

amadeus · 2019-02-19T03:08:33Z

One small thing I noticed with this latest version, fonts are a bit thinner:

I guess visually I prefer the thicker looking fonts, but I realize it's kinda been a re-occuring thing with Mojave that font rendering has thinned up a bit, and might just be what I need to get used too now?

ychin · 2019-02-19T03:42:33Z

I haven't had time to look into the PR yet, but thinner text is usually a sign that the font smoothing / subpixel rendering (which is deprecated in Mojave) isn't properly configured.

Also, I think we may want to make sure this runs faster than the 10.13 pre-Mojave renderer if we are going to remove that. I have checked out other similar types of renderer before and they don't usually perform faster than the native scrollRect: implementation if we are updating the texts in software. Best way to test is using a high-res Retina screen at fullscreen and just scroll a full page of text.

juliuszint · 2019-02-19T08:42:38Z

When started from the commandline a new line is output to standard out.

The issue mentioned here #796 (comment) was also present in MacVim before.

As for performance i am using it on a 4k external Monitor on a MacBook Pro 15-inch 2017 and it seems just as fast as the old renderer if not faster. Obviously this is just my perception if there are any profiling methods let me know.

juliuszint · 2019-02-19T08:56:58Z

NERDTree in combination with vim-devicons works almost perfect, except for the rendering issue on the cursor line, where the folder icon gets clipped.

chdiza · 2019-02-19T18:42:07Z

If it's going to make fonts look bad (thin, faint) on non-retina monitors, then there should be a preference to turn this new renderer on or off.

ychin · 2019-02-19T19:52:02Z

@chdiza I think the bar is this renderer wouldn’t degrade rendering. It’s been on my todo list to take a screenshot and do unit tests against known rendering results but it’s tricky to do it right.

s4y · 2019-02-19T22:41:53Z

@chdiza The mystery font rendering change is a bug; I noticed it but haven’t had a chance to track it down yet. Please keep hunting for bugs if possible!

@ychin If the performance isn’t good enough, note that my next move will be to use separate movable CALayers in the scroll region. Only new lines are rendered during scrolling, which should be blazing fast. This plays nicely with the macOS compositor and is how browsers scroll — I think it’s a pretty optimal choice. (It could even enable things like per-split smooth scrolling…)

If performance is an issue with this PR, I could add that now instead of holding off.

s4y · 2019-02-19T22:43:54Z

@juliuszint Thanks! I don’t use any plugins that insert images. Now I’ve got one to try.

amadeus · 2019-02-21T10:40:22Z

Ok - I did find some pretty serious bugs. I normally use Source Code Pro as my main font, but I was messing around with https://github.com/tonsky/FiraCode and set macligatures and the app would straight up crash or show some weird rendering artifacts while typing out the ligatures.

Some fairly easy repros:

:set guifont=FiraCode-Retina:h14
:set macligatures
:edit example.js

Then just type like an arrow function:

const example = arg => true;

And you should see some weird rendering bugs and probably a crash or two. If not, try other ligatures like !== or ===

Another thing I noticed, not sure if a bug or not (it appears to be in the previous version too so not a result of this branch, but potentially something that could be fixed here), in insert mode with the | style cursor, it will end up blinking underneath text.

amadeus · 2019-02-22T21:23:44Z

Found another funky issue with line heights and how they are rendered:

It happens on the other side of the powerline, although sometimes doing stuff in the window causes the line to get cropped properly.

EDIT: Also been noticing that non native full screen, when activated on my iMac, seems to get stuck in a weird way where the window isn't full screen, but it just renders a blank screen, and I can't interact with it any longer, I have to close macvim completely to recover. I wasn't noticing this on my Macbook Pro, for some reason :/

s4y · 2019-03-28T14:48:44Z

Okay, a bit closer! The new renderer should also be even faster now.

One small thing I noticed with this latest version, fonts are a bit thinner: (#858 (comment))

This should now match the current renderer.

NERDTree in combination with vim-devicons works almost perfect, except for the rendering issue on the cursor line, where the folder icon gets clipped. (#858 (comment))

This is still an issue in the current patch, but I can work on it soon. It's also partly broken in the old renderer: the icons render OK, but moving the cursor down the column to the right of the icons erases the parts that hang into that column.

(Details: The glyphs are wider than a cell but not double-width, so the first cell gets drawn in full (background, then foreground) and then the cell to the right gets drawn in full (background, then foreground). The righthand cell's background is drawn over the glyph. I think the answer is to draw the backgrounds in one pass, then the foregrounds in another pass.)

Every once in a while, a tiny artifact will appear, in roughly the same-ish location. It looks like a dead pixel but I can confirm its not since (a it comes through on screenshots) and be it follows the window when I move it around. If I scroll the document it goes away (#796 (comment))

This should be fixed, want to give it another shot?

Ok - I did find some pretty serious bugs. I normally use Source Code Pro as my main font, but I was messing around with https://github.com/tonsky/FiraCode and set macligatures and the app would straight up crash or show some weird rendering artifacts while typing out the ligatures. (#858 (comment))

The crashes should be fixed, and rendering should now be similar to shipping MacVim, which is also a little bit broken (ligatures may have some parts drawn as individual characters while you edit them). I think the behavior in this PR is close enough to the current state for now, but I have thoughts on how to fully fix it in the future.

Found another funky issue with line heights and how they are rendered: (#858 (comment))

Fixed!

EDIT: Also been noticing that non native full screen, when activated on my iMac, seems to get stuck in a weird way where the window isn't full screen, but it just renders a blank screen… (#858 (comment))

I believe this is fixed too, want to give it another try?

amadeus · 2019-03-28T19:57:26Z

Woots! Giving it a try!

amadeus · 2019-03-28T21:47:39Z

So non-native fullscreen works when you go into it via :set fu, but doing :set nofu completely hard crashes macvim it seems.

chdiza · 2019-03-29T01:53:15Z

I've just tried this on Mojave for about 1 minute, and I don't see the NNFS crash that @amadeus mentioned.

chdiza · 2019-03-29T01:55:30Z

Whoops, never mind. There's the crash!

I didn't see it at first because I have S-Cmd-F bound to a VimL function that does some preparatory work before it calls :set nofu. For some reason that doesn't crash, but if I manually type :set nofu it does.

s4y · 2019-05-01T00:49:51Z

I'm hacking on this right now. While exiting full screen, -setMaxRows:columns: may be called while Vim is still drawing at the old, bigger size, and the drawing commands can point out of bounds 😕. I currently have the following approaches in mind:

Validate/filter drawing commands.
Have grid_cell() (or a new function like grid_cell_safe()) return a pointer to a scratch cell when it's called with out-of-bounds coordinates.
Call setMaxRows:columns: when the draw commands switch to the new size, not sooner.

…but I'm still thinking through it.

s4y · 2019-05-01T03:51:08Z

Hey all, mind re-testing? The :set nofu crash should be fixed.

setMaxRows:columns: gets called a few different ways, and while I'm convinced it's possible to shuffle things so that it's aligned with the draw commands coming from Vim, I'm not going to try right now 🙂.

s4y · 2019-05-01T16:04:16Z

Just did a little bit of cleanup. I'll leave this PR as a draft until I get some feedback, but otherwise it may be ready for review!

pirminis · 2019-05-01T17:01:57Z

@s4y is it possible to install this version from pull-request using brew? or it is possible only by building macvim yourself?

edit: I've just built it myself (latest MacOS Mojave), renderer works fine!

chdiza · 2019-05-02T15:24:58Z

@s4y I've been playing a bit with this and I haven't seen the crash yet.

There are however some odd visual artifacts when moving in to non-native fullscreen. I think I might have mentioned this before in some other thread, but: I can see, very briefly, a rendering of the contents of the MacVim window intermediate in size between where one starts and the full-blown fullscreen. E.g., I can see very briefly Vim's menubar occupying half of the MacBook's screen width, even though when non in fullscreen it takes up about a quarter and when in fullscreen it takes the whole width.

(And curiously, that same menubar is much higher up on the screen during this "flash" than at the start point, enough so that I believe that &lines is somehow being temporarily reset to something much smaller. I.e., my MacVim window is 50 &lines tall, I hit the key chord to enter NNFS, I can briefly see a Vim menubar (&ls=2) that's so high up on the screen that I suspect that &lines got quickly set to somewhere between 15 and 20, and then I end up in NNFS.)

I should mention this is with MMFullScreenFadeTime set to 0. (I despise animations and fadeout/in.) When I get a chance I'll adjust that and see if it makes a difference.

s4y · 2020-10-17T15:01:42Z

And… squashed!

ychin · 2020-10-21T17:52:24Z

Hi @s4y sorry for the delay but I'm still finding one issue with it. I haven't been able to look closer into why that's the case yet, but I suspect is a bad interaction with the cache:

After you call set macligatures, and then paste in this line:
```
😀😨
```
You will notice that the 2nd emoji character keeps jumping around.

Other than mostly looks good, but still need to take a closer look at the NSCache implementation.

Also, thanks for squashing! I think I wasn't clear and I was hoping we only squash when the entire review is signed off as I wanted to diff the latest changes you did, so for future commits can you only squash in the end?

s4y · 2020-10-22T06:06:40Z

@ychin Oof, good catch. I see the problem and I'll try to land a fix shortly. For the moment — unsquashed!

s4y · 2020-10-22T15:41:57Z

@ychin re. #858 (comment): I believe this is fixed now!

eirnym · 2020-11-19T00:40:07Z

@ychin is something left to fix in this branch to launch this renderer at least as a beta?

ychin · 2020-11-30T09:05:05Z

@s4y The pull request looks good! Sorry for the delay again. Please squash and I'll merge.

One minor issue I found has to do with underline rendering. Right now, an empty space would not get the underline drawn. To see a quick repro, you can do set ft=Markdown, and then paste in the following line:

[this is a test](https://github.com/macvim-dev/macvim)

The whole "this is a test" line should have underline, but in your change, the spaces wouldn't. I think it's a small bug in setString: where you just need to make sure the if ([substring isEqualToString:@" "]) early out makes sure to set the sp, fg, and textFlags on the cell.

I'm ok with merging this without the fix, but it should also be a one line fix.

I think we really need better tests for this… Currently I have to go through all known renderer features and make sure they work but after this change merges we should look into better ways to automate this to prevent regressions in the renderer.

Thanks for the work and patience!

ychin · 2020-12-01T23:08:06Z

src/MacVim/MMCoreTextView.m

+        if (flags & DRAW_TRANSP)
+            insertionPoint = cell->insertionPoint;
+        if ([substring isEqualToString:@" "]) {
+            *cell = (GridCell){ .bg = bg, .insertionPoint = insertionPoint };


Need to set the other variables such as fg/flags/sp as well to make e.g. underline draw properly.

This change reworks `MMCoreTextView` to keep track of the state of the screen instead of drawing to the screen or to an image. This lets `drawRect:` draw any part of the view at any time, as needed. This change came about when the old strategy stopped working: The old strategy calls `drawRect:` for the entire view to handle any draw command from the backend, but drew only the changes on top of the old content of the view. This did not work in new versions of macOS that use layers, because `drawRect:` is now expected to fill the entire rect with new content. If it doesn't, the rest of the view will just contain garbage. bbad3ed worked around this issue by adding an intermediate CGImage which was preserved between draws. This fixed the problem but made rendering slower. With the change, the intermediate image is no longer needed and rendering is much faster overall, which resolves macvim-dev#796. As part of this change, font substitution is now handled by Core Text, which changes which fallback fonts are used in some cases but matches other macOS apps.

s4y · 2020-12-03T17:47:39Z

@ychin All set! I committed the underline fix as 2ef8f65 for posterity, then squashed.

s4y · 2020-12-03T17:49:21Z

@ychin Oh, and…

I think we really need better tests for this… Currently I have to go through all known renderer features and make sure they work but after this change merges we should look into better ways to automate this to prevent regressions in the renderer.

If you could start documenting the process you go through, I'm very open to helping automate it.

Thanks for the work and patience!

Same to you!

amadeus · 2020-12-04T07:35:50Z

🙏🙏🙏

ychin · 2020-12-04T07:39:17Z

Alright this is now merged!

If you could start documenting the process you go through, I'm very open to helping automate it.

Yeah currently MacVim only uses the default Vim unit tests, which aren't enough. I think we need to write up some MacVim-specific XCTest and I have a list of tests that I want to add (e.g. input handling, window resizing, etc) along with text rendering. I will ping you when I make that issue/PR if you want to help.

As for the process, it's unfortunately been a little ad-hoc, mostly playing with known edge cases like with/without macligatures, fullscreen (two types) modes, East Asian languages, emojis, different styles, composing characters, and so on. And I have a list of characters/lines consisting of an assortment of those that I just paste in and see if it looks correct. But this is why we need to get automated testing going to not have to rely on ad-hoc testing :)

@s4y

Updated to Vim 8.2.2127. *Note*: This release doesn't natively support Apple Silicon / M1 yet, but does work under Rosetta. See below. Features ==================== Big Sur / macOS 11 -------------------- - MacVim now has an updated app icon (#1054), and preference pane / toolbars have been updated to match Big Sur's interface guidelines. (#1128) - Fixed Touch Bar warnings when launching MacVim from the terminal. #1114 - SF Symbol characters will show up properly as double-width as most of these icons would take up more than one column. Note that these characters are specific to macOS and would not work in other platforms. #1129 Renderer / scrolling performance improvements -------------------- The Core Text renderer has been rewritten and is now much faster! Scrolling should not stutter and lag like before and generally it should feel a lot smoother now. Thanks to Sidney San Martín (@s4y) for the contribution. #858 With this change, the non-Core-Text renderer is now considered deprecated. The old renderer is accessible either through the Preference Pane (under Advanced) or by setting the defaults "MMRenderer" to 0. It works for now, but it will be removed in a future update as it has known bugs. Menu Localization -------------------- Menus are now localized, see `:h langmenu` for how Vim menu localization works. You can use `set langmenu=none` to turn it off if you would like. #1099 There still exists a few menu items that are not localized, and the general MacVim GUI is not localized as well. If you would like to help, please use #1102 to coordinate with MacVim dev team. Getting help / Help menu -------------------- - Help menu's search bar now searches Vim documentation as well! See #1095. - Vimtutor is now bundled with MacVim, and you can access vimtutor from the Help menu (#1096). There is also a link to the latest release notes as well (#1131). General ==================== - This release does not contain a native universal app for Apple Silicon / M1 Macs yet. The release binary will still work under Rosetta, which should provide enough performance, but if you use Python/etc plugins, you need to make sure you have x86 versions of Python/etc installed (which is still the default for Homebrew as of this release). MacVim is buildable under Apple Silicon, so if you need a native binary, you could build it yourself by downloading the source from the Github repository. See #1136 for progress on releasing a universal app for Apple Silicon. - MacVim has enabled the Github Discussions feature, which serves as a good spot for general discussions and questions. See #1130 and check it out! Fixes ==================== - Launching MacVim from the Dock with locales that use "," for decimal separators now works correctly. #11 (Vim 8.2.1738) - `WinBar` menus (which are used by plugins like vimspector) now work properly and don't create dummy menu items. #918 - Using `:browse tabnew` no longer crashes MacVim in terminal mode. #1107 (Vim 8.2.1842) Misc ==================== - Scripting languages versions: - Python is now built against 3.9, up from 3.8. - Lua is now built against 5.4, up from 5.3. Compatibility ==================== Requires macOS 10.9 or above. Script interfaces have compatibility with these versions: - Lua 5.4 - Perl 5.18 - Python2 2.7 - Python3 3.9 - Ruby 2.7

@s4y

Updated to Vim 8.2.2127. *Note*: This release doesn't natively support Apple Silicon / M1 yet, but does work under Rosetta. See below. Features ==================== Big Sur / macOS 11 -------------------- - MacVim now has an updated app icon (#1054), and preference pane / toolbars have been updated to match Big Sur's interface guidelines. (#1128) - Fixed Touch Bar warnings when launching MacVim from the terminal. #1114 - SF Symbol characters will show up properly as double-width as most of these icons would take up more than one column. Note that these characters are specific to macOS and would not work in other platforms. #1129 Renderer / scrolling performance improvements -------------------- The Core Text renderer has been rewritten and is now much faster! Scrolling should not stutter and lag like before and generally it should feel a lot smoother now. Thanks to Sidney San Martín (@s4y) for the contribution. #858 With this change, the non-Core-Text renderer is now considered deprecated. The old renderer is accessible either through the Preference Pane (under Advanced) or by setting the defaults "MMRenderer" to 0. It works for now, but it will be removed in a future update as it has known bugs. Menu Localization -------------------- Menus are now localized, see `:h langmenu` for how Vim menu localization works. You can use `set langmenu=none` to turn it off if you would like. #1099 There still exists a few menu items that are not localized, and the general MacVim GUI is not localized as well. If you would like to help, please use #1102 to coordinate with MacVim dev team. Getting help / Help menu -------------------- - Help menu's search bar now searches Vim documentation as well! See #1095. - Vimtutor is now bundled with MacVim, and you can access vimtutor from the Help menu (#1096). There is also a link to the latest release notes as well (#1131). General ==================== - This release does not contain a native universal app for Apple Silicon / M1 Macs yet. The release binary will still work under Rosetta, which should provide enough performance, but if you use Python/etc plugins, you need to make sure you have x86 versions of Python/etc installed (which is still the default for Homebrew as of this release). MacVim is buildable under Apple Silicon, so if you need a native binary, you could build it yourself by downloading the source from the Github repository. See #1136 for progress on releasing a universal app for Apple Silicon. - MacVim has enabled the Github Discussions feature, which serves as a good spot for general discussions and questions. See #1130 and check it out! Fixes ==================== - Launching MacVim from the Dock with locales that use "," for decimal separators now works correctly. #11 (Vim 8.2.1738) - `WinBar` menus (which are used by plugins like vimspector) now work properly and don't create dummy menu items. #918 - Using `:browse tabnew` no longer crashes MacVim in terminal mode. #1107 (Vim 8.2.1842) Misc ==================== - Scripting languages versions: - Python is now built against 3.9, up from 3.8. - Lua is now built against 5.4, up from 5.3. Compatibility ==================== Requires macOS 10.9 or above. Script interfaces have compatibility with these versions: - Lua 5.4 - Perl 5.18 - Python2 2.7 - Python3 3.9 - Ruby 2.7

eirnym · 2020-12-12T11:59:07Z

How to enable this renderer by default?

s4y · 2020-12-12T17:08:05Z

As for the process, it's unfortunately been a little ad-hoc, mostly playing with known edge cases like with/without macligatures, fullscreen (two types) modes, East Asian languages, emojis, different styles, composing characters, and so on. And I have a list of characters/lines consisting of an assortment of those that I just paste in and see if it looks correct. But this is why we need to get automated testing going to not have to rely on ad-hoc testing :)

I think even having a written list of these would help a ton in the future! (Running through tests manually isn't too bad, figuring out what to test has been the tricky part.)

ychin · 2020-12-12T21:23:23Z

@eirnym As I have mentioned before this is the only renderer now. It's always the default.

Recent change to stateful renderer (macvim-dev#858) has inadvertantly changed how MacVim handles line spacing. Previously, MacVim intentionally ignores the line spacing of a font and creates a new dummy font that essentially has line spacing of 1, but the new code uses the font as is. This means font with non-standard line spacing (e.g. Input Mono) will look different. This is technically the correct way to handle fonts but is different from how MacVim has worked for years. Also, see last time this regression (where MacVim didn't discard line spacing) happened in macvim-dev#928 / macvim-dev#949 which was fixed in macvim-dev#957. Also see macvim-dev#977 where the bug was filed the other way requesting for using the font's line spacing instead of discarding it. This commit re-introduces the behavior to discard line spacing, but only provides it as an option (can be set in the preference pane), while defaulting to using the line spacing as that seems more correct. Note that from a casual survey of other terminals and editors, this behavior is quite inconsistent. Xcode does use the font's line spacing, and was partially the motivation of switching to that as a default. Close macvim-dev#1152.

s4y changed the title ~~Stateful, fast renderer for MMCoreTextView~~ [WIP] Stateful, fast renderer for MMCoreTextView Feb 19, 2019

s4y force-pushed the stateful-render branch from 5558eaf to f1a8909 Compare February 19, 2019 02:20

s4y mentioned this pull request Feb 19, 2019

Scrolling performance regression using intermediate NSImage #796

Closed

s4y force-pushed the stateful-render branch from f1a8909 to 8c560ca Compare February 19, 2019 02:24

s4y force-pushed the stateful-render branch 2 times, most recently from cd315bb to 475a7b0 Compare March 28, 2019 14:40

s4y force-pushed the stateful-render branch from 475a7b0 to 153cbca Compare May 1, 2019 03:46

s4y force-pushed the stateful-render branch from 153cbca to a4f9674 Compare May 1, 2019 16:02

s4y changed the title ~~[WIP] Stateful, fast renderer for MMCoreTextView~~ Stateful, fast renderer for MMCoreTextView May 1, 2019

s4y force-pushed the stateful-render branch from a4f9674 to 6079c89 Compare May 3, 2019 14:24

ychin mentioned this pull request Oct 18, 2020

Unicode characters clipped #1110

Closed

s4y force-pushed the stateful-render branch from 3aee6b2 to 800da6f Compare October 22, 2020 06:05

ychin reviewed Dec 1, 2020

View reviewed changes

s4y force-pushed the stateful-render branch from 2ef8f65 to dba6293 Compare December 3, 2020 17:45

ychin merged commit 1961d05 into macvim-dev:master Dec 4, 2020

ychin added this to the snapshot-167 milestone Dec 4, 2020

ychin added the Renderer Text rendering issues, including CoreText renderer label Dec 10, 2020

s4y deleted the stateful-render branch December 12, 2020 16:24

This was referenced Dec 24, 2020

What happened to fonts? #1152

Closed

Command bar w/ increased size when in fullscreen and tabs #833

Closed

ychin mentioned this pull request Dec 26, 2020

Fix issue with font line spacing support and add option for it #1153

Merged

This was referenced Aug 4, 2022

Implement Metal renderer for MacVim #1262

Open

GUI keyboard repeat rate suffers with many long lines #1259

Open

Stateful, fast renderer for MMCoreTextView #858

Stateful, fast renderer for MMCoreTextView #858

Conversation

s4y commented Feb 19, 2019

amadeus commented Feb 19, 2019

ychin commented Feb 19, 2019 • edited Loading

juliuszint commented Feb 19, 2019

juliuszint commented Feb 19, 2019

chdiza commented Feb 19, 2019

ychin commented Feb 19, 2019

s4y commented Feb 19, 2019

s4y commented Feb 19, 2019

amadeus commented Feb 21, 2019

amadeus commented Feb 22, 2019 • edited Loading

s4y commented Mar 28, 2019

amadeus commented Mar 28, 2019

amadeus commented Mar 28, 2019

chdiza commented Mar 29, 2019

chdiza commented Mar 29, 2019

s4y commented May 1, 2019 • edited Loading

s4y commented May 1, 2019 • edited Loading

s4y commented May 1, 2019

pirminis commented May 1, 2019 • edited Loading

chdiza commented May 2, 2019

s4y commented Oct 17, 2020

ychin commented Oct 21, 2020

s4y commented Oct 22, 2020

s4y commented Oct 22, 2020

eirnym commented Nov 19, 2020

ychin commented Nov 30, 2020

ychin Dec 1, 2020

Choose a reason for hiding this comment

s4y Dec 3, 2020

Choose a reason for hiding this comment

s4y commented Dec 3, 2020

s4y commented Dec 3, 2020

amadeus commented Dec 4, 2020

ychin commented Dec 4, 2020

eirnym commented Dec 12, 2020

s4y commented Dec 12, 2020

ychin commented Dec 12, 2020

ychin commented Feb 19, 2019 •

edited

Loading

amadeus commented Feb 22, 2019 •

edited

Loading

s4y commented May 1, 2019 •

edited

Loading

s4y commented May 1, 2019 •

edited

Loading

pirminis commented May 1, 2019 •

edited

Loading