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

Support for font ligatures #36

Closed
jasonlong opened this issue May 26, 2015 · 38 comments
Closed

Support for font ligatures #36

jasonlong opened this issue May 26, 2015 · 38 comments

Comments

@jasonlong
Copy link
Contributor

This might very well be more trouble than it's worth to implement, but I was wondering if it would be possible to (re)implement font ligatures so that fonts like Fira Code would work.

According to the Hasklig project that inspired Fira Code, MacVim can't render ligatures because of this old commit.

It sounds like there are side effects to simply reverting this commit, but if they aren't too nasty, it would be awesome to have ligature support.

@HealsCodes
Copy link
Contributor

@jasonlong - the commit you're referencing disables ligatures in the old ATSU renderer.
The current CoreText renderer never hat ligatures in the first place..

But since I'm in the same boat and I'd love to use Fira Code I wrote a patch to make ligatures available
in the CoreText renderer and also provide an option to turn them of and on:

ligature-demo

I've opened PR #56 witch includes my changes.

@jpetrie
Copy link
Contributor

jpetrie commented Aug 26, 2015

@douglasdrumond This can probably get closed since #56 was merged.

@HealsCodes
Copy link
Contributor

👍 however I still have a little improvement (rounding of character positions) that didn't make it into the PR. The only difference being that the current version can have a slight "shimmer" when moving between lines which results from minor differences in the character positions if not rounded.

@jpetrie
Copy link
Contributor

jpetrie commented Oct 10, 2015

Closed in #56.

@jordwalke
Copy link
Contributor

Sometimes when I move my cursor over the rendering, it reverts back to the original characters? Should this issue be reopened until the issues are resolved?

(Great work implementing it though!)

@HealsCodes
Copy link
Contributor

That's a known limitation of the current implementation (also visible in the demo above).
If you like you can take a look at PR #56 for more details.

@jordwalke
Copy link
Contributor

Is there another open github issue tracking it?
I experimented with adding text editor ligatures, and I found that drawing all content in the editor one line at a time (and only ever one line at a time) was the easiest way to get everything working consistently - apologies if that suggestion is incredibly naive.

@HealsCodes
Copy link
Contributor

As it would imply some deep changes to the current renderer there is not other issue tracking it.
It was agreed so on so far that the splitting on moving the cursor over a ligature is an acceptable behavior for now.

MacVims CoreText rendering is using a lot of partial redraws to keep an acceptable speed, which in turn can result in liguatures splitting back up if the cursor moves over them (the ligature detection only receives the text to be drawn not the whole line). I'm afraid there is no simple way to fix that.
But feel free to give it a try 👍

@jordwalke
Copy link
Contributor

I think your gif demo shows something different than what I'm describing. In your example, I think it doesn't seem as bad because the moment you leave the line, it renders correctly. What I'm seeing is that even if you leave the line - it still is not repaired. I believe the reason is that you have line highlighting on (highlighting the current line) - whereas I do not. The behavior I'm seeing would warrant leaving this issue open - whereas if I had the experience you demonstrated, I would say that it is somewhat usable. Currently, with what I'm seeing it's not. Would you try disabling your current line highlighting and seeing if you agree?

@jordwalke
Copy link
Contributor

screen shot 2015-11-07 at 12 35 06 am

Also, as you can see here, there seems to be some other serious issue. That line only has one semicolon at the end, yet two are rendered because - for some reason - when the == glyph is broken back into the two separate characters, it makes the line shorter and draws the new semicolon, while leaving the old one.

@HealsCodes
Copy link
Contributor

Would you be willing to compile MacVim from source?
The issue you describe looks like part of what was discussed in the original PR after it was merged :(
I have a gist of MMCoreTextRenderer.mm that should at least get rid of the spacing issues (I've seen them before).

@jordwalke
Copy link
Contributor

Yes I would - do you have a branch that I should try?

@HealsCodes
Copy link
Contributor

You can use the current MacVim master or my branch for the original PR. But you need to replace MMCoreTextRenderer.mm with the file from my gist.

@splhack splhack reopened this Nov 7, 2015
@splhack
Copy link
Contributor

splhack commented Nov 7, 2015

@Shirk Could you update your MMCoreTextRenderer.mm with the latest master and send a pull request?

@HealsCodes
Copy link
Contributor

@splhack - I can but first I'd like some feedback if possible.

@splhack
Copy link
Contributor

splhack commented Nov 7, 2015

Pull request is better than gist :)

@HealsCodes
Copy link
Contributor

Agreed, and it would have ended up in the PR but that was unfortunately merged before..

HealsCodes added a commit to HealsCodes/macvim that referenced this issue Nov 7, 2015
Do a proper analysis of the ligatures (if any) and try to avoid double
character draws / moving spaces etc.
@HealsCodes
Copy link
Contributor

@splhack - I opened #122 as WIP.

The changes look and work OK from my view but I like to get feedback from you, @jordwalke and @jpetrie before removing the WIP-tag (see the checklist in the PR itself).

@jordwalke
Copy link
Contributor

I don't understand what you did, but I think that diff definitely improves things and fixes critical bugs. A couple of thoughts:

  1. The lengths don't see to jump around anymore when you move through lines (even when I have done defaults write org.vim.MacVim MMCellWidthMultiplier 0.9 - a custom setting) so that's good.
  2. The bugs with double rendering seem to be gone.
  3. When I move my cursor over a ligature, it breaks the ligature apart like I previously mentioned. If you have cursorline enabled, it is only for the duration of your cursor being on that line. If there's no better solution, is there some way to force cursorline to be on when ligature support is enabled - (even if it means forcing cursorline on with the same background color as normal). This would at least be nice to have as a warning on the preferences pane ("enable cursorline for better rerendering of ligatures").
  4. Splitting a ligature when you move the cursor over a ligature is actually a good thing IMHO. What's not good, is when it isn't restored after you move away from that ligature. Is this hard to fix?

This is strictly better than what was in master so I imagine there won't be trouble getting this merged?

@jordwalke
Copy link
Contributor

There's one remaining major issue with your patch (off by one error?)

If I type => everything looks fine:

screen shot 2015-11-07 at 11 10 07 pm

But then when I type one more character immediately after, it ruins the previous ligature.
screen shot 2015-11-07 at 11 10 12 pm

However, if I had left a space before entering another character, the ligature is not destroyed.

screen shot 2015-11-07 at 11 12 17 pm

Of course it's always destroyed if the cursor is exactly over the ligature, which we already discussed - but these pictures above show something different.

@HealsCodes
Copy link
Contributor

Looks like that one could be fixed by always drawing the whole line.
Anyone with enough knowledge about the renderer available to point me one the right direction?

Am 08.11.2015 um 08:13 schrieb Jordan W notifications@github.com:

There's one remaining major issue with your patch (off by one error?)

If I type => everything looks fine:

But then when I type one more character immediately after, it ruins the previous ligature.

However, if I had left a space before entering another character, the ligature is not destroyed.

Of course it's always destroyed if the cursor is exactly over the ligature, which we already discussed - but these pictures above show something different.


Reply to this email directly or view it on GitHub.

@jordwalke
Copy link
Contributor

@Shirk: Do you know what's up with this rendering issue for Pragmata Pro?
Other ligature fonts seem to render fine.
fabrizioschiavi/pragmatapro#13

@HealsCodes
Copy link
Contributor

Looks like something moot with the font - could you try a different editor with ligature support and compare?

@jordwalke
Copy link
Contributor

Here's a screenshot of TextEdit which supports ligatures:
Everything looks fine in this editor:
screen shot 2015-11-08 at 2 37 53 am

Another thing that this shows, is that the * character appears to be rendered incorrectly in MacVim. Here's what the stars look like for Pragmata Pro in MacVim.

screen shot 2015-11-08 at 2 40 22 am

Not only are they showing up incorrectly as literal "stars", but they are compressed to half their normal rendering width.

@jordwalke
Copy link
Contributor

What could cause the word "back" to be rendered as a glyph that looks like "backwards"?

@HealsCodes
Copy link
Contributor

Current ligature-mode enables 'full (including rare)' ligatures - I've changed this to only use basic ligatures in my PR - please checkout and try if it helps.

@jordwalke
Copy link
Contributor

That seemed to fix the issue with Pragmata Pro nicely! (The other issues remain as expected).

@HealsCodes
Copy link
Contributor

Ok.. then consider Pragmata Pro a sneaky font.
I'll keep looking into a way to force a draw of the whole line but I'm not optimistic.
The partial draws seem to come from deep inside the vim core and just get handled in the mac Gui like in any other..

@jordwalke
Copy link
Contributor

Can you go into detail about my thoughts on having MacVim maintain its own virtual grid of characters in plain character arrays? I thought since text drawing is very expensive, it might benefit from the same kind of "virtual DOM" concept we employ in ReactJS where we are at liberty to rerender whenever we want, but then do a more intelligent diffing strategy to see if anything actually changed before physically rerendering certain parts to pixels. You could apply your own heuristic to determine if a change is actually "worthy" of redrawing. (Apologies if this is already how MacVim's text works - I have no experience with its implementation).

@HealsCodes
Copy link
Contributor

#122 changed from WIP to ready-to-be-merged, it fixes the reported spacing issues and also add compatibility with the new emoji support. I'll keep looking into finding a way to trigger redraws for the current line if the cursor moves up / down but for now the worst issues are resolved.

splhack added a commit that referenced this issue Nov 13, 2015
Fix spacing issues on ligatures as seen on #36.
@splhack
Copy link
Contributor

splhack commented Nov 13, 2015

Added macligatures option in #129. It removes Enable support for ligatures from Preferences panel but it allows to enable/disable Ligatures at any time!

:set macligatures
:set nomacligatures

@splhack
Copy link
Contributor

splhack commented Nov 13, 2015

.gvimrc example

" highlight CursorLine guibg=NONE ← it doesn't work for redrawing Ligatures
set cursorline
set guifont=Fira\ Code:h11
set macligatures

@splhack splhack closed this as completed Nov 14, 2015
@jordwalke
Copy link
Contributor

How do you get the cursorline to be the exact same color as whatever the background color is without setting it to NONE which doesn't really work? How can you keep it in sync whenever the color scheme changes?

@HealsCodes
Copy link
Contributor

@jordwalke - not sure but you might wan't to check out #133

@rafaelrinaldi
Copy link

Just wanted to thank @Shirk and everyone involved on making font ligatures happen in Vim. The effort is truly appreciated!

@jordwalke
Copy link
Contributor

Yes, agreed - this is truly wonderful. I tell everyone I can about these ligatures. Thank you everyone.

@HealsCodes
Copy link
Contributor

I'm glad the feature is getting traction (even in the current not 100% complete state).
I promise to look into the few known open issues once I find the time.

The only downside is that I'm now going to have to save up for a Pragmata Pro license 😉

@rafaelrinaldi
Copy link

Maybe @fabrizioschiavi can offer a discount of some sort since you have put so much effort into supporting the font he created :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants