-
-
Notifications
You must be signed in to change notification settings - Fork 23.5k
Use Inter as the default editor font, features enabled #111140
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
Conversation
9153ba9 to
63d55a7
Compare
|
I was the middle of adding the disambiguation on my PR as well. 😛 Surprised you also didn't use the Variable font here as you suggested there. Also, wouldn't be better to enable even if a custom font is picked? It's probably desirable in that case too, and the Inter font would still keep it when it appears as a fallback. |
It would be reasonable to do so if both Regular and Bold variants together weren't lighter than InterVariable.
|
|
Note that you could use Adwaita Sans, which is Inter preconfigured to have disambiguation enabled: https://gitlab.gnome.org/GNOME/adwaita-fonts There's also Adwaita Mono (a customized Iosevka) which could replace JetBrains Mono, as it's designed to pair well with Adwaita Sans. |
|
@Calinou I tried that on my PR initially, but it only has the Variable fonts, and they are in the ttf format. |
And while it's possible to convert them to woff2, its size would still be bigger than a Regular + Bold pair in the end. |
|
Updated so it only touches the lowercase L like Adwaita Sans does (cv05). The other ones look too glitchy with the default editor font size and hinting on. |
|
Some more features have been suggested here: #111118 (comment) |
I tested them before, and all of them showed the same buggy hinting as serifed uppercase i. If you look closely, the problem even happens with lowercase L, except it's barely noticeable until you compare with a lower or higher size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's for the best if this one is merged before the new theme one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me. I'm cool with either font so I don't have a strong opinion for or against the change, but since it's a design choice for the new theme we're implementing, let's go with it.
We planned to merge this close to merging the new theme (in the same dev snapshot), so we get all visual changes in the same release.
In case we want those other variations, #111461 is mandatory. EDIT: Apparently not, in case we use font feature freeze. On the other hand, doing so implies modifying the font, which requires copyright info modification by the terms of OFL (at least Adwaita Sans, which is a font feature freeze by GNOME that only enables a character variant, does that). With that said, I think it's better to wait for HarfBuzz auto-hinting. |
Offtopic but, if we truly want a customized Iosevka, the best variant would probably be Iosevka SS14 Extended, which is customized to look like JetBrains Mono. On the other hand, JetBrains Mono's WOFF2 file is 90.2KiB, while the average pre-compiled Iosevka WOFF2 won't be lower than 300 KiB. And comparing, I personally don't see any advantages. |
|
With the issue with variant hinting gone, I have re-enabled serif uppercase i and slashed 0. Added more screenshots to the main comment. The proposal suggests If anyone disagrees with slashed 0, we can just replace |
|
I would go for |
|
I personally don't like the slashed zero! I feel like there's very few places in the editor where the distinction between |
|
Stuff like slashed zero probably should be configurable, we have editor option to set OpenType feature tags for the code font, so no reason not to have one for main font as well (but UI should be improved, currently it's just a text field). |
|
I have updated so it only applies
|
This comment was marked as outdated.
This comment was marked as outdated.
b6cfff5 to
9e705d1
Compare
|
As a follow-up of #111773, I added to Main Font Custom OpenType Features's documentation that Inter supports it, and which ones are enabled/disabled by default specifically by Godot. Unless someone really wants changes, I'm considering this finished for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great now 🙂
For example, tabular numbers make the Frame Time panel look more stable over time:
frame_time_panel_tabular_numbers.mp4
Code looks good to me.
|
Thanks! |











Related to #111118 (part of it is most of this PR)
Closes godotengine/godot-proposals#9012.
Disclaimer
Some time ago I did a work to replace the default font with Inter. It is the font I have used in Godot since way before godot-minimal-theme was created.
I didn't make a pull request out of it as I thought a default theming change like this wouldn't be accepted before 5.0 or something, until #111118 appeared and included the same font change, which made me consider that the change is on the table now.
As I believe some things are better done separately, and also wanted to bring attention to certain OpenType features that improve readability, I opted to open this PR.
Description
Noto Sans' main point is that it covers a significant amount of glyphs and avoids displaying invalid characters, but that's only true if you're using all of its fallbacks.
Inter, on the other hand, is a typeface designed for most types of user interface, with many claims that its readability is much better than that of Noto Sans. FOSS projects like GNOME and Blender have adopted it.
This PR replaces the default Noto Sans font (but not its fallbacks) with the Inter font.
Additionally, I touched some OpenType features:
calt: Contextual alternates. Comes enabled by default in Inter, so it was disabled.ss04: Enables serifedIand tailedl, which makes them better distinct as the default is literally indistinguishable.tnum: Tabular figures. Enabling them makes numeric fields in the Inspector look better.These are only applied if Main Font is unset and shouldn't carry over to custom fonts. They may be enabled/disabled through Main Font Custom OpenType Features by setting them to false (it's documented).
Using font feature freeze would imply modification by the terms of OFL, so to avoid having to manipulate copyright stuff, features gets set through code.
I got some pangrams and other examples in the editor for reference:
Before (Noto Sans):
After (Inter with disambiguation):
Considerations
Droid Sans, Roboto, Noto Sans and Inter all seem to be built on the same foundation, so metrics should be pretty much compatible.
In fact, Inter began as a superset of Roboto, which itself uses Noto Sans as a fallback on Android.
There are concerns about some concerns about some glyphs from Noto Sans missing in Inter, but those are usually not available in a keyboard.