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.

Sign up for GitHub

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

Spec for font configuration objects #10383

Closed
wants to merge 3 commits into from
+102 −0
Closed

Spec for font configuration objects #10383

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b465e8f
first draft
PankajBhojwani Jun 9, 2021
6aa0f56
spell
PankajBhojwani Jun 9, 2021
40518f4
visual hierarchy, null option, rewriter
PankajBhojwani Jun 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions doc/specs/#6049 - Font config objects.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
---
author: Pankaj Bhojwani, [email protected]
created on: 2021-6-8
last updated: 2021-6-8
issue id: #6049
---

# Font configuration objects

## Abstract

This spec outlines how we can support 'font configuration objects' in our settings, which will make editing all font-related settings easier and will also allow users to customize font behaviour based on certain text attributes. For example, bold text can have a different font configuration object as compared to italic text.

## Inspiration

Reference: [#6049](https://github.com/microsoft/terminal/issues/6049)

As we move towards allowing more font-related settings, it will be helpful to have them all in one object within the profile, rather than cluttered all over the profile itself. To elaborate, [#1790](https://github.com/microsoft/terminal/issues/1790) contains several work items regarding font configurability that we have yet to complete. As these get added in, it will be preferable to have a single object that contains all font-related settings.

## Solution Design

The implementation design for font configuration objects will be very similar to the way appearance configuration objects were implemented ([reference](https://github.com/microsoft/terminal/pull/8345)). There will be a default font object, that will contain settings for the rendering of font in the default case. Beyond that, a number of optional font objects can be created for various use cases such as the rendering of bold font or italic font. These optional font objects will have a smaller number of allowed parameters, and will inherit from the default font object (i.e. any parameters that they do not define will be taken from the default font object).

### Allowed parameters and allowed non-default font configurations

As of now, Windows Terminal supports 3 font-related settings: `fontFace`, `fontWeight` and `fontSize`. The default font object will be allowed to define all 3 of these settings, whereas the non-default font objects will only be allowed to define `fontFace` and `fontWeight`. We do not allow the non-default font objects to define a `fontSize` as we do not want there to be resizes as the text switches between different types of font like bold/italic.

In the initial stages of this feature, we will only allow two non-default font configurations: `italicFont` and `boldFont`.

### Inheritance

The font configuration class will be implemented such that it allows for inheritance between objects of the class. As described earlier, within the same profile, non-default font configuration objects will inherit from the default font configuration object.

*Between* profiles, each profile's default font configuration object will inherit from the default font configuration object defined in `profiles.defaults`. As for the non-default font configuration objects, these are treated as a single setting. I.e. a profile's `italicFont` font configuration object does *not* inherit from the `italicFont` object defined in `profiles.defaults`, it is simply its own setting. The `italicFont` configuration object defined in `profiles.defaults` only applies to profiles that do not define their own `italicFont`.

This design is in line with the way we handle `defaultAppearance` and `unfocusedAppearance`, and was deliberately chosen so we maintain consistency across these different classes.

## UI/UX Design

Users will be able to add font configurations to their profiles that will look something like this:

```
"font": // this is the default
{
"fontFace": "Cascadia Mono",
"fontWeight": "Normal",
"fontSize": 12
},
"boldFont":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense visually in the JSON to imply the hierarchy with something like:

"font":
{
   "face" : "Cascadia Mono",
   "weight" : "Normal",
   "size" : 12,
   "variants": [
      "bold" : {
          "face" : "Consolas",
          "weight" : "Extra-Bold"
      },
      "italic" : {
          "weight" : "Light"
      }
   ]
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmmm very interesting. Would any key in font be available to variants? So somebody could set font:face:Cascadia font:variants:bold:face:Consolas?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda like this

Copy link
Contributor Author

@PankajBhojwani PankajBhojwani Jun 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmmm very interesting. Would any key in font be available to variants? So somebody could set font:face:Cascadia font:variants:bold:face:Consolas?

Of the 3 keys available now, weight, face, and size, only size will be disallowed to variants (we don't want weird resizing from different parts of the text being bold or something)

{
"fontFace": "Consolas",
"fontWeight": "Extra-Bold"
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait... we can will be able to do this? Use multiple fonts within the same text buffer? (important for me to know for my a11y text attributes PR haha)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know what, let me test that out and get back to you

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it, and short answer: yes we can have multiple fonts in the same text buffer

Long answer: there's funky stuff that goes on because of the different fonts not necessarily having the same glyph size and so the spacing becomes weird - we should be able to do some creative scaling to avoid this problem though

"italicFont":
{
"fontWeight": "Light"
}
```

In the settings UI, the current way font settings are presented to the user can be preseved, and additional dropdowns can be added for `boldFont` and `italicFont`. Each dropdown will contain options to configure `fontFace` and `fontWeight`.

## Further implementation details

Currently, the `DxRenderer` is given a desired font and it creates a text layout for the default case and a separate text layout for the italic case. To support font configuration objects, the `DxRenderer` will need to be updated to be able to receive a collection of desired fonts, and it will create a separate text layout for each desired font. These text layouts will then be obtained by the renderer when needed, as it already does for the italic case.

## Capabilities

### Accessibility

Should not affect accessibility.

### Security

Should not affect security.

### Reliability

We have had several issues when parsing `fontFace` from settings in the past. This will add further potential for parsing errors simply because there will be more places where we will need to parse `fontFace`. Unfortunate as it is, it is an unavoidable aspect of this feature.

### Compatibility

This feature changes the way we expect to parse font settings. However, we have to make sure we are still able to parse font settings the way they are currently, so as to not break functionality for legacy users.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have precedent for a settings-file-rewriter that pre-passes legacy configs and just rewrites them into the current format? (Instead of leaving multiple parse-mechanisms in the current format loader?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't, but it is probably a good idea.


### Performance, Power, and Efficiency

Rendering several different font faces will probably have a performance impact. Again, this is an unavaoidable aspect of this feature.

## Potential Issues

Already covered in the sections above.

## Future considerations

Similar to the discussion we had regarding appearance configurations, we will need to figure out how we want to deal with overlapping configurations. I.e. if font configurations are defined for both bold and italic, how should font that is both bold and italic be handled?
Copy link
Contributor Author

@PankajBhojwani PankajBhojwani Jun 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am tempted to say that this issue is much more pressing when it comes to fonts than it was for appearance configurations, to the point where I wonder if we should decide not to proceed further with this until we figure this out

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, text can't be both bold and italic, right? Because terminals would only say if text should be "more bright" or "more faint", I think? But I'm also confused because the API for TextAttribute has IsBold and IsFaint as two separate variables, so theoretically somebody could call SetIsBold(true) and SetIsFaint(true) back to back, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dumb idea, if text is both "more bright" and "more faint", would that mean that it negates itself and you just see text at normal brightness? In other words, would IsBold() && IsFaint() mean that we should just use the default font config?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think text can be both bold and italic, try doing:

echo ^[[1;3m in command prompt, you get text that's both bold and italic

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof. We almost need people to be able to define a "BoldItalic" variant too.


## Resources