-
-
Notifications
You must be signed in to change notification settings - Fork 683
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
Support optional ligatures in CoreText #56
Conversation
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. |
I can't comment on the code itself, but I'd love to have this functionality. 👍 |
@douglasdrumond - any news for this PR? |
I'll try to yank this into my local build and run some tests on it tonight. |
I noticed a couple things:
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. |
@jpetrie - thanks for testing let me address your points one by one:
I'm also using Fira Code on my system so we should see similar results. |
@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 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 |
@jpetrie some of that could be related to the line length in the demo GIF. My current font settings are: set guifont=Fira\ Code\ Regular\ for\ PowerLine:h10 |
I think I've determined the "shimmering" is due to the way Post-processing the positions after 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 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. |
@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 Let me try to give an example that'll clarify the how / why:
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 Regarding the intra-line redraw I agree with you, it's probably fixable but will definitely be tricky. |
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;" 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.
|
@jpetrie - could you do me a favor and try the
If you can confirm it fixes the "shimmering" for you I'm going to add the changes to this PR. |
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 |
Support optional ligatures in CoreText
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).