@cryza this is awesome 😍 🎉, thanks for what looks like a lot of work and the detailed explanation re how you went about implementing it.

Just gave it a try locally and I'm getting:

Which relates to this function:

I was trying it out using FiraCode-Retina and then switched over to Hasklig both of which threw the same error, lemme know if I can provide anymore info re. my setup

PS: sorry for almost immediately piling in there with an issue 😆

@akin909 Thanks for trying it out so quickly!
It seems like there are two things happening here:

• Chrome seems to be smarter in its algorithm for determining which font to use. While font-manager will not manage to resolve FiraCode-Retina to a font file, Chrome still manages to correctly render text to the atlas with that setting, even using the right font. To verify my theory: Yu should see some logs prefixed with [OpenTypeLigatureGrouper] in the verbose part of your console which tells you if it found a font file, which one it used and if it found ligatures there.

• Some piece of code seems to break when the font could not be found or doesn't contain the right data. I need to check on that quickly :)

Until then, you can try to use Fira Code instead as a setting, that worked for me.

@cryza using Fira Code works like a charm 😄 ✨ sooo nice 👍, I'm wondering whether the way we specify fonts might be at fault(although we cant really guarantee how users will specify it anyway), not sure how it works on linux and windows but on MacOS to find the font name that oni can use I use the postscript name there was a related issue a while ago.

Not sure how font-manager resolves the name, but it always seemed a bit odd we had to resort to that although not sure how it would work in vscode or atom might be the exact same thing 🤷‍♂️

Also wasn't getting many [OpenType] logs the error seemed to occur before much actually happened.

@akin909 I made the code for finding the right font file a bit smarter, so it should now also work with FiraCode-Retina!

Just had a go with the most recent changes, I now get

Seems to correctly find the fonts but run out of space, tried a couple of fonts hasklig seemed to work, various versions of FiraCode didn't wasn't sure how the renderer allots space but seems to relate to the prefill with ascii function (guesstimating from the stack trace)

Very cool work guys! One thing you might look at sometime would be a new project we've been building on top of fontkit and font-manager already: textkit. It's a full text layout engine in JS.

It does font loading and substitution, line wrapping, complex script itemization, and more. It handles run grouping similar to your groupCells for rich text support. The output is a container filled with lines of text, each containing runs of glyphs. There is a pluggable renderer architecture to allow rendering to any surface: currently there is a PDF renderer, but more could easily be added. The rest of it is also pluggable so you can swap out the implementation of any part.

It's already being used in react-pdf for their text layout needs. Aside from that, there isn't much docs yet so you can check out a small example of how the API works here: https://github.com/foliojs/textkit/blob/master/temp.js

@devongovett Cool, thanks for the hint about textkit! I would never have found that repo on my own :)

I just had a quick look at it: If I understood it correctly, the PDF renderer takes the glyph paths from the glyph run and renders them to the context just like they are described, right?
I'm asking so specifically because like stated in my other comment, we put a lot of focus on using all possible optimizations towards crispness and readability of our text. With Chrome's harfbuzz-based renderer, we get auto-hinting and subpixel antialiasing to optimize sharpness. That's the main reason we decided not to render the glyph paths directly to the canvas ourselves.
So my current assessment would be: If we somehow manage to do (auto)-hinting and subpixel-antialiased rendering ourselves with textkit, fontkit, or any other JS font library for that matter, this might be a really interesting option.

@akin909 Thanks for testing again :)
I also squeezed some logic for pre-warming the atlas with all ASCII glyphs on initialization; It seems like on your setup (I guess you have a rather high-dpi screen?) this blows up the allocated texture space 💥. Let me check if I can adjust some numbers or implement some scaling there.

@cryza the PDF renderer doesn't render the glyph paths using fontkit, it just places the glyphs in the PDF using native PDF text setting operators. This ensures that the text is selectable, copyable, etc. Unfortunately, as I mentioned above, canvas doesn't have a way to render text by glyph id instead of characters without some hacks, so that would be the main challenge.

@cryza I'm now noting an error ->

And no text is being rendered in the buffer ->

Btw hasklig and Fira Code still work though using FiraCode-Retina is what brings on the above

It works! 🎉 no console errors, no crashes ⭐️

I ran some more performance tests to find out if the ligatures add a notable amount of delay when typing. While there is of course a bit of added time per frame, my personal perception tells me that this is not noticeable. The profiler also shows that less than 5% of the computation time is spent on the ligatures. I think there are other places that we can and should optimize first before e.g. going asynchronous with the ligature resolution.

This means I would vouch for merging this PR, given that the CI passes and someone approves :)

@cryza been running this a little bit, and just noticed that all the text that was previously italicised is now bold, I know somewhere in this PR you changed how the renderer handles bolding text and I'm wondering if that might be a cause?

I don't think its a major blocker since there this PR adds a lot of other things but thought I should raise it

Codecov Report

Merging #2560 into master will increase coverage by 0.36%.
The diff coverage is 21.39%.

@@            Coverage Diff             @@
##           master    #2560      +/-   ##
==========================================
+ Coverage   44.93%   45.29%   +0.36%     
==========================================
  Files         352      361       +9     
  Lines       14336    14555     +219     
  Branches     1863     1903      +40     
==========================================
+ Hits         6442     6593     +151     
- Misses       7669     7738      +69     
+ Partials      225      224       -1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd94771...9d486e6. Read the comment docs.

Thanks for the thorough testing, @akin909 ! You were right, for normal font weights the renderer tried to write a broken style string to the canvas which was blocked and resulted in the glyph re-using the previous style instead - which was the bold style for all pre-rendered glyphs. I was able to fix the style string calculation now :)

Also big thanks to @CrossR for testing this on Windows! Apparently the font matching algorithm within font-manager works differently on Windows than it does for Linux and Mac OS, which is why it didn't find the correct font file for Fira Code. I replaced the lookup with a platform-independent scan through all available fonts now and it seems to be working fine. :)