Skip to content
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

[Feature request] Setting font features and font variations. #503

Open
Martin-Eriksson opened this issue Feb 4, 2025 · 3 comments · May be fixed by #506
Open

[Feature request] Setting font features and font variations. #503

Martin-Eriksson opened this issue Feb 4, 2025 · 3 comments · May be fixed by #506

Comments

@Martin-Eriksson
Copy link

Martin-Eriksson commented Feb 4, 2025

The ability to enable or disable font features would be a useful addition. I'm making a text editor, and I want to support the user changing font features and variations to their preference.

Font features are things like ligatures, 'texture healing', and alternate characters (slashed vs dotted zero). Font variations are things like font width, font weight and font slant.

As an example, the MonaSpace family of fonts (https://monaspace.githubnext.com/), supports many font features and varations.

Here is a modified version of the showfont example, using the Monaspace Argon font, with different font features. The left is with all the stylistic sets (ligatures) , the middle is with the default font features, and the right is with "-calt" which disables font healing (see how the m's are no longer different widths depending on what characters come before and after it):

Image

API ideas

TTF_SetFontFeatures(font, "calt liga=1 -kern ss01 aalt[3:5]=2"); // space-separated features
TTF_SetFontVariations(font, "wght=500 slnt=-7.5"); // space-separated variations

(the format for each feature/variation are just what harfbuzz supports, see hb_feature_from_string and hb_variation_from_string in https://harfbuzz.github.io/harfbuzz-hb-common.html )

Should there be TTF_GetFontFeatures and TTF_GetFontVariations as well?

How should enabling or disabling kerning using TTF_SetFontFeatures interact with TTF_SetFontKerning?

There are essentially an infinite number of font features a user can specify. For example, there are 100 character variants, cv00-cv99 a font can chose to add so implementing this will require dynamic memory allocation, unless we set an arbitrary limit on how many features can be specified.

For font variations, there are only 5 registered opentype variations, at least currently. But MDN has an example of using "GRAD" as a custom font variation. If custom font variations are a thing, we probably shouldn't limit this one to the five registered variations.

Implementation

If this is something you want added to the library, I can attempt to implement this. I've implemented TTF_SetFontFeatures, which seems to work with basic testing. I can open a draft PR once I've implemented the other functions, if you want me to.

@slouken
Copy link
Collaborator

slouken commented Feb 4, 2025

API ideas

TTF_SetFontFeatures(font, "calt liga=1 -kern ss01 aalt[3:5]=2"); // space-separated features
TTF_SetFontVariations(font, "wght=500 slnt=-7.5"); // space-separated variations
(the format for each feature/variation are just what harfbuzz supports, see hb_feature_from_string and hb_variation_from_string in https://harfbuzz.github.io/harfbuzz-hb-common.html )

This seems good to me, and similar in style to the other optional HarfBuzz features we support.

Should there be TTF_GetFontFeatures and TTF_GetFontVariations as well?

Yes, we can just save a copy of the previously set string and return a pointer to that, or NULL if they haven't been set.

How should enabling or disabling kerning using TTF_SetFontFeatures interact with TTF_SetFontKerning?

Offhand I think they should be independent. When HarfBuzz is enabled it will be used for shaping and there's no other interaction with the SDL_ttf code, so it can be considered a special case that the application can use or not, in conjunction with other font features.

There are essentially an infinite number of font features a user can specify. For example, there are 100 character variants, cv00-cv99 a font can chose to add so implementing this will require dynamic memory allocation, unless we set an arbitrary limit on how many features can be specified.

Dynamic memory allocation is fine.

For font variations, there are only 5 registered opentype variations, at least currently. But MDN has an example of using "GRAD" as a custom font variation. If custom font variations are a thing, we probably shouldn't limit this one to the five registered variations.

Dynamic memory allocation for this is fine too. Most people won't use this feature so it'll be a NULL pointer in the font, and we shouldn't arbitrarily restrict someone who wants to use it.

Implementation

If this is something you want added to the library, I can attempt to implement this. I've implemented TTF_SetFontFeatures, which seems to work with basic testing. I can open a draft PR once I've implemented the other functions, if you want me to.

Yes please, that would be great.

Thank you!

@Martin-Eriksson
Copy link
Author

Martin-Eriksson commented Feb 4, 2025

How should enabling or disabling kerning using TTF_SetFontFeatures interact with TTF_SetFontKerning?

Offhand I think they should be independent. When HarfBuzz is enabled it will be used for shaping and there's no other interaction with the SDL_ttf code, so it can be considered a special case that the application can use or not, in conjunction with other font features.

I'm not clear on what you mean here. What do you mean by independent? That if we've called TTF_SetFontFeatures, TTF_SetFontKerning should be ignored? (if so, should calling TTF_SetFontFeatures(font, NULL) be used to get back to "normal" mode and use the value from TTF_SetFontKerning?)

Currently, if kerning is explicitly specified to be enabled or disabled in TTF_SetFontFeatures, it ignores the value of TTF_SetFontKerning. Which also means that you can't use TTF_GetFontKerning to know if the feature is actually on or not (should TTF_SetFontFeatures modify font->enable_kerning if 'kern' is specified?).

If kerning is not explicitly set in the TTF_SetFontFeatures string, then the value of TTF_SetFontKerning/font->enable_kerning is used.

Should there be TTF_GetFontFeatures and TTF_GetFontVariations as well?

Yes, we can just save a copy of the previously set string and return a pointer to that, or NULL if they haven't been set.

Currently I'm parsing the feature string into an array of hb_feature_t when calling TTF_SetFontFeature, and not saving the original string. You'd prefer I save the string instead and parse it into an array of hb_feature_t temporarily in CollectGlyphsFromFont (where hb_shape is called with the features array)? Saving the string itself would make the Get functions easier to implement.

For error handling, I'm thinking we set the string to NULL, and call SDL_SetError on the first invalid feature/variation?

EDIT: Wait, that's not possible if we don't parse the string right away. Should we store both the original string and the array of hb_feature_t? Or should we only store the array and make the Get functions parse them to strings again?

EDIT2: If we aren't storing the original string, should the Get functions allocate a string, or should they take a buffer and length to fill?

int TTF_GetFontFeatures(TTF_Font *font, char *text, size_t maxlen);
// or
char *TTF_GetFontFeatures(TTF_Font *font);

@slouken
Copy link
Collaborator

slouken commented Feb 5, 2025

I'm not clear on what you mean here. What do you mean by independent? That if we've called TTF_SetFontFeatures, TTF_SetFontKerning should be ignored? (if so, should calling TTF_SetFontFeatures(font, NULL) be used to get back to "normal" mode and use the value from TTF_SetFontKerning?)

I mean if the application uses one API to affect kerning, then it shouldn't use the other. They operate independently and we don't have to track or synchronize them (unless that's easy to do and makes sense?) It sounds like you're already handling it fine:

Currently, if kerning is explicitly specified to be enabled or disabled in TTF_SetFontFeatures, it ignores the value of TTF_SetFontKerning. Which also means that you can't use TTF_GetFontKerning to know if the feature is actually on or not (should TTF_SetFontFeatures modify font->enable_kerning if 'kern' is specified?).

If kerning is not explicitly set in the TTF_SetFontFeatures string, then the value of TTF_SetFontKerning/font->enable_kerning is used.

Yes, that seems reasonable.

Should there be TTF_GetFontFeatures and TTF_GetFontVariations as well?

Yes, we can just save a copy of the previously set string and return a pointer to that, or NULL if they haven't been set.

Currently I'm parsing the feature string into an array of hb_feature_t when calling TTF_SetFontFeature, and not saving the original string. You'd prefer I save the string instead and parse it into an array of hb_feature_t temporarily in CollectGlyphsFromFont (where hb_shape is called with the features array)? Saving the string itself would make the Get functions easier to implement.

Maybe do both? I'm not sure how expensive the parsing is, but CollectGlyphsFromFont() gets called quite a bit, so caching information used there is helpful.

For error handling, I'm thinking we set the string to NULL, and call SDL_SetError on the first invalid feature/variation?

Makes sense.

EDIT: Wait, that's not possible if we don't parse the string right away. Should we store both the original string and the array of hb_feature_t? Or should we only store the array and make the Get functions parse them to strings again?

EDIT2: If we aren't storing the original string, should the Get functions allocate a string, or should they take a buffer and length to fill?

In SDL3, the convention is to return allocated memory and have the caller free it, so the lifetime of the memory isn't in question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants