Support ligature

[zenixls2]

Fork the topic from mainstream.
Any issue related to my fork should go here.

Notice that this fork might take a long time to develop and possibly not (able to) merge back to the mainstream.

[blaggacao]

@kchibisov How are the odds this work could inspire upstream?
If this could be achieved it would play nicely in a platform independent way: b92f094#r39874718

[kchibisov]

If it won't regress any performance we're all ears, which will likely require a lot of effort from us as well, however upstream is focused on a bit different things right now. So the patch must be complete and don't regress any rendering and throughput performance.

[zenixls2]

If possible, I want to know more details on how the core developers estimate the performance.

[kchibisov]

https://github.com/alacritty/alacritty/blob/master/CONTRIBUTING.md#performance and also render_timer values.

[zenixls2]

Ok, I've got some data in performance.md.
Seems no big influence on performance.

[kchibisov]

Please, look at render_timer while running vtebench the performance of this branch is ~15x slower than master branch. (2ms to render frame compared to 30ms on it). Even your performance.md is showing that due from increased throughput performance, meaning that VTE parser has more time to run and meaning, due to less amount of locks due to very slow rendering.

Unless you'll have performance in a margin of error on render_timer or let's say 50% difference, I won't even try to look into it.

[blaggacao]

I see you already put some effort in some tweaks in your latest commit 9427d56 Just wanted to highlight that. Maybe with the guidance he gave, this can be pushed further to convince @kchibisov into looking at it?

[zenixls2]

It seems like the performance hit from ligature is unavoidable. How do you think if performance is unchanged when ligature option is turned off?

In branch ligature I've introduced a text_run hashset to cache the rendered output. This works for most cases, but not for the tests of random writes.

In ligature_test branch I'm trying to replace all RenderableCells with TextRun. There is slight performance improvement but may introduce some issue on Colors in config.

[zenixls2]

@blaggacao do you have a windows machine or mac os machine to build? I want to know if my changes break the other systems. The final goal is to support harfbuzz for shaping in different systems, but hard to do so by myself since i currently only have one linux machine...

[blaggacao]

I'm afraid, I'm not even having one within reach, let alone using one. @kchibisov would you be able to suggest somebody or a some way?

[kchibisov]

harfbuzz only works on linux/bsd. On other platforms core text and direct write should be used.

[dashezup]

Could country flag emoji issue also be solved? Are those also ligatures?

alacritty#4025

[zenixls2]

@dashezup not sure, maybe you could give it a try on the latest commit in ligature branch. My libraries in centos are too old to support emojis, also I don't have a mac to test if my modifications really works.

Update: i guess it's already resolved in my branch.

[danielementary]

Your fork has been working pretty well by my side but I noticed recently that ligatures do not work anymore. Which information could be relevant to share in order for this to be solved besides the fact I am using the package from aur/alacritty-ligatures-git 0.7.0.1745.ga15eb3b-1 ?

[SiamionRalionak]

Hey, I ran into the same problem after the last commit.

System
OS: Linux 5.9.16-1 (Manjaro 20.2 Nibia);
Version: alacritty 0.7.0-dev (a15eb3b)
Linux: X11, i3
Package: aur/alacritty-ligatures-git
Font/Terminal Size: Iosevka SS09 / maximized borderless

Here are examples:

• before the commit
  

• and after
  

Thanks in advance, appreciate your work.

[zenixls2]

I tried to confirm from remote but gl rendering is not supported in vnc.
might be something wrong in my fork of crossfont.
currently I have no way for fixing it and the situation will be lasting until Jan 4th since I could not sit in front of my linux pc.

[zenixls2]

@SiamionRalionak @danielementary can u check again? Should be fine now

[Philipp-M]

Hey, I've tested the new build. unfortunately the issue isn't fixed yet.

Would it be possible to use merging master instead of rebasing one squashed commit onto master.
And probably use separate commits for new changes? It would be easier to monitor the changes and more feasible to help.
Moreover an issue I have currently, as I'm using fixed commits (for packaging reproducibility), is the garbage collection of github. All older rebased commits disappear after a week or so (that's the frequency github runs garbage collection to remove dangling commits) could be more or less, but after some time it's not possible anymore to download older (in this case working) commits.

Later when a PR to the upstream is opened, it should be no issue to rebase and squash everything into one commit onto master, as the merge commits are resolved while squashing, so no manual changes should be necessary if the current master is merged into this branch.

Thanks for the great work, keeping it up to date with master btw.

[zenixls2]

@Philipp-M did you tested the latest ligature branch, or just the aur package? Aur package is still not updated, and I'm not the maintainer of it either, so it might take time to react. I'm currently using the same version in my branch and it works fine with Iosevka Term Extended.

I've also tested the latest crossfont ligature branch that Fira Code works fine.

I tried to merge the upstream before, but in the end gave up because there were too many redundant conflicts to resolve. Using single rebase commit on top helps me rect faster to the changes in upstream. To address the garbage collection issue you mentioned, probably i should create a branch for backup?

[danielementary]

Hey @zenixls2, thanks for your reaction. I just cloned the repo and built the ligature branch but the issue do no seem to be solved. Is there anything I can help you with ?
My current font is Iosevka Term, I also tried Fira Code but none of them seem to work.

[zenixls2]

What about explicitly set ligatures = true in config file?
This is the only reason I could think of

[danielementary]

Okay, so this was apparently a config issue, it is working now. Thanks for your precious help and contribution, this is priceless.

[Philipp-M]

Can confirm, setting font.ligatures = true fixes the issue.

Some comments though after skimming the patch:

alacritty/alacritty.yml

Line 182 in 974e49f

#ligature: true

shows ligature instead of ligatures, this should probably be ligatures.

And I don't see anywhere, where font.ligatures gets its default value (true) AFAIK, the default value of a boolean is false isn't it?
So this might explain the issue we had?

[zenixls2]

Yes, correct. Recently there was a commit in upstream that changes the serialize/deserialize method of configuration. In order to resolve the conflict and fix the compilation, I remove the use of struct DefaultTrueBool(or DefaultBoolTrue, whatever) and modify it to bool. Probably at that time I forgot to define the Default trait implementation of font config.

Anyway will fix this soon.

Also there was a big change in the upstream recently and introduce lots of conflicts. That's why there's no update in my branch since then. Anyone who is waiting please be patient.

[jorgebef]

I just wanted to give my appreciation for your efforts and offer my Mac system if you need to do any testing on said platform.
I would love to contribute to this project!

[mehdisadeghi]

I just tried this fork on Arch Linux and it works perfectly for Arabic script. Thanks for the effort put into the fork. I just want to mention, for what it's worth it, HarfBuzz compiles to WASM, and there are WASM runtimes other than Browsers, just in case.

[luisdavim]

will this ever be pushed to upstream Alacritty?
Was this ever mentioned in alacritty#50 ?

[zenixls2]

will this ever be pushed to upstream Alacritty?
Was this ever mentioned in alacritty#50 ?

Only if all platforms are tested and supported.
Also, current implementation still have a lot of bugs.
It's not of performance but about stability now that blocks me from sending pr.

[azzarello]

I compiled the binary from source with cargo as the instructions describe, but when trying any of the formats of ligatures: true that are discussed in this thread do not seem to enable ligatures and simply give me config errors. I am using the JetBrains Mono Nerd patched font for reference.
Any help would be greatly appreciated, cloned the repo today to the build for reference as well.

[zenixls2]

I compiled the binary from source with cargo as the instructions describe, but when trying any of the formats of ligatures: true that are discussed in this thread do not seem to enable ligatures and simply give me config errors. I am using the JetBrains Mono Nerd patched font for reference.
Any help would be greatly appreciated, cloned the repo today to the build for reference as well.

Did you checkout the ligature branch?

[azzarello]

I did not, and now I am not sure why I didn't think of that before this.

[AtifChy]

update the fork please

[TheOPtimal]

Please update the fork, this is like five versions behind

[TheOPtimal]

also it doesn't seem to work, I'm using Fira Code and with ligatures: true

[joelnordell]

Just wanted to jump in and say thanks for the work on this! I have compiled this branch and am running successfully on MacOS Big Sur. Would be amazing to see it contributed & accepted upstream so that it will be able to track with new releases. Have you attempted to rebase this patch on the current upstream master yet? If I have some time maybe I'll look into that sometime.

To me, ligature support is more important than keeping up with the latest releases -- so even if this isn't updated or merged, I'll likely keep running this build for a long time.

Thanks again for making this fork!

[zenixls2]

No time for tracking the changes in the upstream. It will definitely take few days to rebase again and debug . Any pr is welcomed