kovidgoyal / kitty

Cross-platform, fast, feature-rich, GPU based terminal
https://sw.kovidgoyal.net/kitty/
GNU General Public License v3.0
23.77k stars 959 forks source link

Disabling ligatures disables other OpenType features (which are enabled by default) too #2247

Closed ctrlcctrlv closed 4 years ago

ctrlcctrlv commented 4 years ago

I discovered this while thinking about ctrlcctrlv/TT2020#3

The problem is this:

https://github.com/kovidgoyal/kitty/blob/70071fe1f679f8fbd514fb061df02b55628e7c9f/kitty/fonts.c#L779

It turns off all OpenType shaping if disable_ligatures is true, not only ligatures. disable_shaping is a more accurate name for this option.

It would be better if I could have more fine tuning, e.g.—

disable_ligatures always # Internally appends to below option "liga" 0, "clig" 0, "dlig" 0 
font_shaping_tags "calt" 1, "ss02" 1

hb_feature_from_string() now accepts CSS font-feature-settings format.

I am working on this right now so would appreciate being given time to submit a PR. If I fail to figure it out I'll let you know, but the code seems well done, and the problem simple (for me, since I know my way around OpenType fonts and also HarfBuzz).

kovidgoyal commented 4 years ago

That line does not disable all ligatures, it disables only the CALT ligatures. See the init_font function at line 357. If you want to disable LIGA or DLIG, but not CALT, the place to do it is in init_font(). Currently the list of fonts to do this for is hardcoded, someday I might make it a user option, however, for now, juts send a PR adding your fonts postscript name there.

ctrlcctrlv commented 4 years ago

The OpenType standard allows any kind of substitution in any tag. HarfBuzz supports this fully, unlike some other old shapers.

That means, a GSUB type 4 substitution (ligature) can be anywhere in the tables.

Although some fonts put their ligatures in calt, it's non-standard to say the least. The correct tag is liga or perhaps clig.

So, basing this off of tags will never work fully for all fonts, and will have side effects.

Giving users control over tags is better, and that's what my PR will do

kovidgoyal commented 4 years ago

On Fri, Jan 03, 2020 at 02:34:56AM -0800, Fredrick Brennan wrote:

The OpenType standard allows any kind of substitution in any tag. HarfBuzz supports this fully, unlike some other old shapers.

That means, a GSUB type 4 substitution (ligature) can be anywhere in the tables.

Although some fonts put their ligatures in calt, it's non-standard to say the least. The correct tag is liga or perhaps clig.

All programming fonts I have ever encountered put their substitutions in CALT. Other kinds of substitutions aren't really appropriate for monospaced fonts/text display such as in a terminal.

ctrlcctrlv commented 4 years ago

Are you against this feature? Will you not accept a PR which allows users to enable/disable tags by name?

Here's my proposed description of the feature in kitty.conf:

#: Choose exactly which OpenType features to enable or disable. This
#: is useful as some fonts might have many features worthwhile in a
#: terminal—for example, Fira Code Retina includes a discretionary
#: feature, `zero`, which in that font changes the appearance of the
#: zero (0), to make it more easily distinguishable from Ø. Fira Code
#: Retina also includes other discretionary features known as
#: Stylistic Sets which have the tags `ss01` through `ss20`.

#: This option has the same format as the CSS option with the same
#: name.

#:     font_feature_settings "calt" on, "zero" on, "liga" off

I'm almost done writing it. I'm not asking you to do anything but review the code once I'm done.

kovidgoyal commented 4 years ago

No I am fine with it, though it should be keyed by font name.

On Fri, Jan 03, 2020 at 04:11:33AM -0800, Fredrick Brennan wrote:

Are you against this feature? Will you not accept a PR which allows users to enable/disable tags by name?

Here's my proposed description of the feature in kitty.conf:

#: Choose exactly which OpenType features to enable or disable. This
#: is useful as some fonts might have many features worthwhile in a
#: terminal—for example, Fira Code Retina includes a discretionary
#: feature, `zero`, which in that font changes the appearance of the
#: zero (0), to make it more easily distinguishable from Ø. Fira Code
#: Retina also includes other discretionary features known as
#: Stylistic Sets which have the tags `ss01` through `ss20`.

#: This option has the same format as the CSS option with the same
#: name.

#:     font_feature_settings "calt" on, "zero" on, "liga" off

I'm almost done writing it. I'm not asking you to do anything but review the code once I'm done.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/kovidgoyal/kitty/issues/2247#issuecomment-570556023

--


Dr. Kovid Goyal https://www.kovidgoyal.net https://calibre-ebook.com


ctrlcctrlv commented 4 years ago

Noted, thank you, will do.

I like this code base. It's well written, easy to find my way around. The mixture of Python and C is elegant. I enjoyed figuring out how the option parsing works. Certainly much easier to contribute to than FontForge, where I spend most of my time. :-)

ctrlcctrlv commented 4 years ago

I think once this is done, by the way, it will be the most powerful font management in a terminal emulator, ever. You can even enable stylistic alternates / stylistic sets and esoteric stuff like Character Variants.

Yes, maybe many users will never use this, I agree with you totally; but I don't use half of Kitty's features.

kovidgoyal commented 4 years ago

Noted, thank you, will do.

I like this code base. It's well written, easy to find my way around. The mixture of Python and C is elegant. I enjoyed figuring out how the option parsing works. Certainly much easier to contribute to than FontForge, where I spend most of my time. :-)

Thanks, good to hear. Although am sure fontforge is a much larger/older codebase not designed from the ground up for python.

kovidgoyal commented 4 years ago

On Fri, Jan 03, 2020 at 04:39:29AM -0800, Fredrick Brennan wrote:

I think once this is done, by the way, it will be the most powerful font management in a terminal emulator, ever. You can even enable stylistic alternates / stylistic sets and esoteric stuff like Character Variants.

Yes, maybe many users will never use this, I agree with you totally; but I don't use half of Kitty's features.

Oh I love uber-configurable software. So you wont get pushback from me unless something affects performance or some other key goal.

ctrlcctrlv commented 4 years ago

Very good to hear!

Indeed, the FontForge project received its first commit in the year 2000. Although at times frustrating, the work is rewarding. I am trying to convert it from greyscale bitmaps to color bitmaps so we can support COLR/CPAL fonts. I'm sure you can imagine how difficult that is in a pure C program with tons of structs made for greyscale bitmaps, and many functions expecting them. This change is a vacation compared to that. :beach_umbrella:

kovidgoyal commented 4 years ago

Ouch that sounds painful, all the best!! And feel free to come over to kitty land if you need a break :)

ctrlcctrlv commented 4 years ago

Update: I got it working! Fira Code Retina is perfect for testing this feature.

Here it is with +zero, compared to -zero!!!

In kitty.conf:

font_feature_settings Fira Code Retina: +zero

It's however midnight in my country and I'd like to do a little clean up before submitting my patch, so I need to go to bed.

kovidgoyal commented 4 years ago

Looks good :)