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 optional ligatures in CoreText #56

Merged

Conversation

HealsCodes
Copy link
Contributor

This PR adds optional support for ligatures to the CoreText renderer.
It's my first shot at this but it turned out to work pretty nice (not as messy as ligatures looked in ATSU).

ligature-demo

@douglasdrumond
Copy link
Member

Thanks. I'm inclined to merge this, since it's optional. I'm not with my Mac right now, I'll look into it as soon as I can.

@jasonlong
Copy link
Contributor

I can't comment on the code itself, but I'd love to have this functionality. 👍

@HealsCodes
Copy link
Contributor Author

@douglasdrumond - any news for this PR?

@jpetrie
Copy link
Contributor

jpetrie commented Aug 11, 2015

I'll try to yank this into my local build and run some tests on it tonight.

@jpetrie
Copy link
Contributor

jpetrie commented Aug 12, 2015

I noticed a couple things:

  • you need a font that actually supports ligatures (this is obvious in hindsight, but as I didn't have one installed I thought I'd compiled the wrong branch, initially); it may be worth noting this in the help text that shows up below the checkbox?
  • with the ligature option on, using a non-ligature-supporting font (say, Menlo), typing sometimes causes the text to "swim" a pixel or so horizontally. This is particularly noticeable typing a ! or something, as in "hello world! " (note trailing space). It isn't terribly distracting, but worth noting.
  • the ligature option seems to become bound to a window when that window is open; toggling it has no effect on open windows.
  • with the ligature option on, using a ligature font, typing a ligature != converted as expected, but afterwards I hit space to continue typing and this artifact appeared (only the first "half" of the ligature character is rendered?):

1

  • with ligatures, if the cursor passed over them, they would revert to normal characters. My cursor is set to a block, which might impact the behavior. redraw! restores them.

The last issue is, I feel, the most problematic. I'd say the artifacts are also a bit of an issue, and the swimming may be (it's not clear to me how noticeable it would be to something who wasn't actively looking for something to behave different with the ligatures on, like I was).

I installed and used Fira Code as a ligature font to test with. May or may not have something to do with my results.

@HealsCodes
Copy link
Contributor Author

@jpetrie - thanks for testing let me address your points one by one:

  1. I agree, while it should be obvious adding a note that may help.
  2. I'm using MacVim with enabled ligatures since I created this PR and so far I didn't notice any swimming (I'm on a Retina MBP). However this could be related to the modifications done to get the ligature spacing right.
    (But then technically there should be no difference in behavior if the font has ligatures or not.)
  3. Yes that's the case. Same as the whole Use CoreText Renderer option.
    It's part of how the rendering views in MacVim are designed - they get their OS X related options set at creation time and have them immutable while the window exists.
  4. I can confirm that behavior as it happens to me too from time to time.
    It might be a result of merging / splitting ligatures while the line is edited (see 5.).
    IMHO it would be best to show ligatures only on the non-active lines but I honestly don't know how to get that hooked up.
  5. Yes that's actually the case - ligatures get reverted to normal characters once you move the cursor to the line containing them. That's also visible in my demo GIF above.
    While I think it makes sense to have it that way I have to say: I'm not doing this on purpose!
    All my changes did was enable the display of ligatures in the recurseDraw method.
    The splitting seems to be part of the CoreText editing mode in itself - so no Idea how to change that if it is required.

I'm also using Fira Code on my system so we should see similar results.

@jpetrie
Copy link
Contributor

jpetrie commented Aug 12, 2015

@Shirk Your GIF shows the ligatures converting to their component glyphs if the cursor is on the line containing the ligature (and reverting back to ligatures when the cursor moves off), which seems reasonable to me... but that's not the behavior I'm seeing. I'm seeing the ligatures stay ligatures until the cursor is actually on top of one of the two characters that make up the ligature, at which point they switch to component glyphs and never switch back to ligatures until I execute :redraw!, which feels like a bug.

What's your GUI font size at? I wonder if that's related since it will impact the glyph sizing. I'll see if I can track down why this might be happening, I've done some work in recurseDraw before so I sortof know my way around it.

@HealsCodes
Copy link
Contributor Author

@jpetrie some of that could be related to the line length in the demo GIF.
In day-to-day editing the glyphs decompose the same way you're describing it (when moving over them) but for me they also get re-combined once I move to a different line.

My current font settings are:

set guifont=Fira\ Code\ Regular\ for\ PowerLine:h10

@jpetrie
Copy link
Contributor

jpetrie commented Aug 19, 2015

I think I've determined the "shimmering" is due to the way ligatureGlyphsForChars determines glyph positions; the CoreText API it calls produces accurate, floating-point positions which likely account for kerning (and/or other things, as using kCTKernAttributeName to disable kerning doesn't fix the issue). Basically the presence or absence of other glyphs around any given glyph slightly adjust the spacing.

Post-processing the positions after CTRunGetPositions (to round them up to the next integer value) stops the shimmering. So, however, does simply not calling CTRunGetPositions. I see you added this in a follow-up commit, can you describe the scenario that led to your needing to do this? I think I can fix the shimmering but I'm not sure if it will just recreate the bug you were trying to avoid.


The issue with the glyphs sometimes "reverting" to non-ligature form is because MacVim only redraws a portion of a line via a row/column location and length indication in many cases. When this happens, the surrounding glyphs that give a ligature context aren't always included in the character range given to recurseDraw and so no ligature is produced.

A naive approach would be to always redraw an entire line in ligature mode to ensure the full context is always there. This is problematic, however, because the information needed to adjust the draw to the full line isn't available at the point the draw begins (I don't think). It's possible we can adjust the call site that issues the messages, but this comes from the Mac-specific portion of vim itself, I believe. Even if we do that, the cursor rendering appears to always be done as a single character and that would need to change as well, which may be tricky to work in.

Hmm.

@HealsCodes
Copy link
Contributor Author

@jpetrie thank you so much - you're doing a great job with your reviews!

I had to think a little about why I added the position adjustment to ligatureGlyphsForChars but the reason was IMHO pretty straight forward - the adjustment is required to prevent characters following a ligature from being squashed into it (accidentally the exact problem the ATSU renderer has).

Let me try to give an example that'll clarify the how / why:

  • recurseDraw is called with the string "a !=b"
  • it creates a run of glyphs[5] and a matching positions[5]
  • ligatureGlyphsForChars is called and shortens that to glyphs[4] (merging != into a ligature)
  • recurseDraw continues with the processed glyphs[4] but now renders the "b" which belonged to positon[4] in the input at position[3] (ligatures counts as one, not two glyphs)

So before adjusting the positions the character directly following the ligature could end up being drawn inside the second half of the [double width] ligature glyph 😞

As you noticed my fix was to simply update positions to match the new set of glyphs. The downside being that I can't tell if a glyph is a ligature or not and thus having to recreate all positions. If it would be possible to tell a ligature glyph apart from a normal glyph positons could simply be adjusted to remove the unused positions of multi-part merged glyphs.


Regarding the intra-line redraw I agree with you, it's probably fixable but will definitely be tricky.
I'm not sure if it bothers me enough to look that deep into the CoreText specifics but since I'm obviously not the only one interested in getting this included I see no reason to stop here 😄

@jasonlong
Copy link
Contributor

@Shirk @jpetrie Thanks for the hard work looking into this. 👏

@jpetrie
Copy link
Contributor

jpetrie commented Aug 19, 2015

As far as I am aware the only way to determine if a glyph is a ligature involves inspecting the hinting tables for the font itself, and I don't think Cocoa has an API to do so. So I'm not inclined to suggest going down that path.

Not all ligatures are "double-wide;" fi and fl ligatures, for example, tend not to be, but I don't think there is a good way to make such ligatures work in a monospaced context, so the guidance should probably be to not use fonts that include glyphs for those ligatures.

The simplest solution to the positioning issue seems to be to post-process the positions and round them up to the next integer value or the next multiple of the font character width (used in the original position computation), whichever looks better.

MMBackend.m is responsible for issuing the draw messages (for example, one of the problematic messages is of type DrawStringDrawType). I believe a solution to the redraw problems would be to pass along extra information when ligatures are enabled which contains the entire text of the row in question. This would let ligatures be mapped correctly, and then drawn individually per the original data in the message.

@HealsCodes
Copy link
Contributor Author

@jpetrie - could you do me a favor and try the MMCoreTextView.m from this gist?

This version determines the actual character width from CoreText and the cellWidth from MacVim.
When post-processing character spacing is then always rounded up to multiples of cellWidth.

Scratch that. It had strange spacing issues in my listchars (nbsp:⬙,trail:⬘,tab:⋯\ ,eol:⇠)..
Now it's simply applying ceil() to round the x-position.

Looks fine for me:
ligature-spacing

If you can confirm it fixes the "shimmering" for you I'm going to add the changes to this PR.
Btw. this should also work for ligatures which are smaller than cellWidth 😄

@HealsCodes
Copy link
Contributor Author

Since I didn't want to do an umpteenth edit to my previous commit: MMCoreTextView.m

A version that can determine if a glyph is a ligature and will reuse the existing positions array.
It will skip positions that are covered by multi-character ligatures but keeping them for smaller ligatures / regular characters.

douglasdrumond added a commit that referenced this pull request Aug 23, 2015
Support optional ligatures in CoreText
@douglasdrumond douglasdrumond merged commit 9d6e28e into macvim-dev:master Aug 23, 2015
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

Successfully merging this pull request may close these issues.

None yet

4 participants