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

Add font weight options #6048

Merged
8 commits merged into from May 20, 2020
Merged

Add font weight options #6048

8 commits merged into from May 20, 2020

Conversation

miniksa
Copy link
Member

@miniksa miniksa commented May 20, 2020

Summary of the Pull Request

Adds the ability to specify the font weight in the profiles, right next to the size and the font face.

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

  • Cascadia Code normal
    image

  • Cascadia Code bold
    image

  • Segoe UI Semilight
    image

  • Segoe UI Black
    image

  • Segoe UI 900 (value for Black)
    image

@ghost ghost added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels May 20, 2020
@DHowett
Copy link
Member

DHowett commented May 20, 2020

Is it time for us to move from fontFace to font{ face, weight, ... } or font{ family, weight ... }?

@mdtauk
Copy link

mdtauk commented May 20, 2020

The problem I see with this option, is when they add support for Bold and Italic emphasis text. What happens if the bold weight of a font is already being used as the default weight?

Perhaps if there were three font settings, one for Normal, one for Bold, and one for Italic - then the user would be responsible for the choices?

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

It's beautiful.

doc/cascadia/profiles.schema.json Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/Profile.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/Profile.h Outdated Show resolved Hide resolved
@DHowett
Copy link
Member

DHowett commented May 20, 2020

@mdtauk

they

you mean we, right? 😁

@miniksa
Copy link
Member Author

miniksa commented May 20, 2020

The problem I see with this option, is when they add support for Bold and Italic emphasis text. What happens if the bold weight of a font is already being used as the default weight?

Perhaps if there were three font settings, one for Normal, one for Bold, and one for Italic - then the user would be responsible for the choices?

I'm not going to hold back progress because this isn't perfect. We can't support bolding yet (which is #109). I do think that perhaps we can, at some future date, offer folks the choice of which presentation they want for any of those states, like you say, by offering a full font specification for each state. But we'll cross that bridge when we get to it.

@mdtauk
Copy link

mdtauk commented May 20, 2020

@mdtauk

they

you mean we, right? 😁

Point taken, although I can't make heads nor tails of C and C++ code, so I await the talents of others.

doc/cascadia/profiles.schema.json Outdated Show resolved Hide resolved
doc/cascadia/SettingsSchema.md Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/Profile.cpp Outdated Show resolved Hide resolved
…ency back and forth with the fact that names or values are valid. Change DirectWrite to OpenType because it's more generic of a term.
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

Yea so this looks great. Maybe we do need to have a conversation about "font": { ... } vs "fontFace": "", "fontWeight":"", "lineSpacing":"", ..., but I think this is a fine start

src/renderer/dx/DxRenderer.cpp Show resolved Hide resolved
@miniksa
Copy link
Member Author

miniksa commented May 20, 2020

@DHowett asked me to try Fira Code variable weight font. So I did. And it works. But I made it so the numerical value can be specified anywhere from 100-990 so you can take advantage of the variableness.

@miniksa miniksa added the AutoMerge Marked for automatic merge by the bot when requirements are met label May 20, 2020
@ghost
Copy link

ghost commented May 20, 2020

Hello @miniksa!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 5 hours 54 minutes. No worries though, I will be back when the time is right! 😉

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@miniksa
Copy link
Member Author

miniksa commented May 20, 2020

@msftbot wait 10 minutes before merging

@ghost
Copy link

ghost commented May 20, 2020

Hello @miniksa!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Wed, 20 May 2020 20:17:08 GMT, which is in 10 minutes

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@DHowett
Copy link
Member

DHowett commented May 20, 2020

I can't sign off a second time, but pretend I did.

@ghost ghost merged commit 8265d94 into master May 20, 2020
@ghost ghost deleted the dev/miniksa/font-weight branch May 20, 2020 20:17
@mdtauk
Copy link

mdtauk commented May 20, 2020

@DHowett asked me to try Fira Code variable weight font. So I did. And it works. But I made it so the numerical value can be specified anywhere from 100-990 so you can take advantage of the variableness.

If the font chosen is a Variable font, then the V2 Settings UI should probably display a slider for the weight value.

@tormento
Copy link

@miniksa which of the various web release Fira Code did you try? Official repository on GitHub? I tried to use "fontWeight": "300", on Terminal Preview 1.0.1402.0 but it has no effect.

@miniksa
Copy link
Member Author

miniksa commented May 26, 2020

@tormento, I just checked this in 6 days ago. I don't believe we have released a build with this feature yet. Soon.

@tormento
Copy link

@miniksa in the mean time, could you address me to the Fira Code repository you used? I am using FiraCodeGX.ttf 4.0 from official repository on GitHub.

@miniksa
Copy link
Member Author

miniksa commented May 26, 2020

@tormento I used the 4.0 release off of the GitHub repository tonsky/FiraCode. I took the file FiraCode-VF.ttf out of the variable_ttf folder in the zip.

@tormento
Copy link

@miniksa the VF variant doesn't show the major families, i.e. light, regular, medium, etc.

The GX one yes, at least on Windows 10 x64 19631.

@miniksa
Copy link
Member Author

miniksa commented May 26, 2020

@tormento, OK? But it works. Either build master and try it or wait for a release please.

@tormento
Copy link

@miniksa yeah, yeah. It was just a hint about the two different versions 😇

jelster pushed a commit to jelster/terminal that referenced this pull request May 28, 2020
## Summary of the Pull Request
Adds the ability to specify the font weight in the profiles, right next to the size and the font face.

## PR Checklist
* [x] Closes microsoft#1751 
* [x] I work here.
* [x] Tested manually, see below.
* [x] Added documentation to the schema 
* [x] Requires docs.microsoft.com update, filed as MicrosoftDocs/terminal#26
* [x] I'm a core contributor.

## Detailed Description of the Pull Request / Additional comments
- Weights can be specified according to the OpenType specification values. We accept either the friendly name or the numerical value that applies to each weight.
- Weights are carried through per-profile and sent into the renderer.
- Weights are carried through to the TSF/IME overlay.
- The names are restricted to the set seen at https://docs.microsoft.com/en-us/uwp/api/windows.ui.text.fontweights.
- There are alternate names at https://docs.microsoft.com/en-us/windows/win32/api/dwrite/ne-dwrite-dwrite_font_weight for the same values (ultra-black is just an alias for extra-black at 950).

## Validation Steps Performed
- Cascadia Code normal
![image](https://user-images.githubusercontent.com/18221333/82480181-46117380-9a88-11ea-9436-a5fe4ccd4350.png)

- Cascadia Code bold
![image](https://user-images.githubusercontent.com/18221333/82480202-4f9adb80-9a88-11ea-9e27-a113b41387f5.png)

- Segoe UI Semilight
![image](https://user-images.githubusercontent.com/18221333/82480306-73f6b800-9a88-11ea-93f7-d773ab7ccce8.png)

- Segoe UI Black
![image](https://user-images.githubusercontent.com/18221333/82480401-9688d100-9a88-11ea-957c-0c8e03a8cc29.png)

- Segoe UI 900 (value for Black)
![image](https://user-images.githubusercontent.com/18221333/82480774-26c71600-9a89-11ea-8cf6-aaeab1fd0747.png)
@ghost
Copy link

ghost commented Jun 18, 2020

🎉Windows Terminal Preview v1.1.1671.0 has been released which incorporates this pull request.:tada:

Handy links:

@DHowett DHowett mentioned this pull request Jul 15, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Settings Issues related to settings and customizability, for console or terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for configuring Font Weight
6 participants