factor in font ligatures while rendering hints #162
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue is described in #158 #161.
The problem is the
match.x
position is essentially taken as the cursor position to displayhints (with some utf-8 adjustments). This works in all cases except when ligatures are
involved, since that's a choice of the font. Ligatures are becoming more and more common -
especially in people's shell prompts, as well as various modern TUIs and CLIs that are meant
to only be consumed by humans and not programs.
We can only know whether we'll have ligatures by asking a font to shape the text. This PR
uses
rustybuzz
for that.Alternatives
We could try to do something a bit "clever" (read: hacky): if the string at the cursor
position doesn't match, we can go backwards on the line to find the correct match. This
should be much faster than shaping the font, and potentially more reliable, since we'd be
able to handle any ligatures. Even though it isn't a "precise" solution and just a heuristic,
analyzing and shaping fonts is a pretty chaotic, messy world anyway.
We could change fundamentally how it searches a line by having it be based on cursor advances.
Unsure what kind of impacts this could have on bugs on performance, but I'm definitely not feeling
great about doing this.
Performance
rustybuzz
themselves note:It also looks like they may potentially not include the known optimizations since it makes
the code harder to sync with the
harfbuzz
, which this is a port ofThe end-user experience is still pretty solid - there's maybe a millisecond pause that's
otherwise not there. Considering we're only rendering one screen's worth of text, it doesn't
come into a play too much. I say this as a person that's already used to doing the motion
pretty quickly. I can run some benchmarking if we're serious about this change.
We could potentially use
harfbuzz
directly and use it as an executable. It's very activeand well-maintained, and much more performant. However, it adds external dependencies.
Notes
The user has to provide
font-file
as an config. If not provided, we don't bother withligatures. If they use different font families that have different ligature behaviors in
different parts of their terminal, there isn't really any way to handle that.
I just currently dumped in the font I'm using for the test; if we want to actually upstream
this PR, I'll create a separate test font with just the minimal components
This PR is kind of a proof-of-concept, but I've written it to be as upstreamable as possible.
There are a couple edge cases to handle; I'll go through those more rigorously if there's
chance of upstreaming.