Embed monospaced font

hebasto commented at 6:11 pm on August 28, 2020: member

Qt does not guarantee that the actual applied font matches to the requested one. It was noted (https://github.com/bitcoin/bitcoin/pull/16432#issuecomment-514486077):

the monospace font looks a bit weird no macOS

… because it is not monospaced. Also some discrepancies I’ve noted on Windows while testing Qt 5.15 (#19716).

Of course, we could check the actual font with QFontInfo, and try to choose another font. But this PR suggests to just embed a monospaced font, and get the GUI look (partially) independent from a platform.

Roboto Mono was chosen after discussion with Bitcoin Design community, and due to its Apache License, Version 2.0.

Changes are scoped to the Overview page only.

Screenshots on macOS 10.15.6 (images are simulated by code patching):

master (ca30d34cf94b7797272ef1920ca4b48716e7f999)
this PR (3fdd5b6bd17a679d6e3876682266092159c52d59)

More screenshots added after #79 (comment):

Linux Mint 20.1 + Cinnamon DE

DeepinScreenshot_select-area_20210221205410

Windows 10 (with depends)

DeepinScreenshot_select-area_20210221205056

macOS Big Sur (with depends)

DeepinScreenshot_select-area_20210221202917

hebasto commented at 6:11 pm on August 28, 2020: member

Many thanks to @GBKS !

hebasto renamed this:
~~gui: Add IBM Plex Mono font~~
Add IBM Plex Mono font
on Aug 28, 2020

Sjors commented at 12:05 pm on August 30, 2020: member

Before and after screenshots on various OS’s would be nice.

I’m generally not a fan of messing with the default font choices of operating systems / desktop environments. However, if upstream QT doesn’t use a proper monospaced font for macOS, then it makes sense for at least that platform.

hebasto commented at 5:26 pm on August 30, 2020: member

Just for record: another PR that embeds a font is https://github.com/bitcoin/bitcoin/pull/16883.

RandyMcMillan commented at 6:25 am on September 3, 2020: contributor

There have been concerns about OFL Licensing in the past…

https://github.com/bitcoin/bitcoin/pull/17311#issuecomment-550453875

https://github.com/bitcoin/bitcoin/pull/17311#issuecomment-550591089

But if OFL isn’t an issue - then fonts-liberation is a good candidate as well because we can stay in one family of fonts and remain consistent across different presentations.

https://en.wikipedia.org/wiki/Liberation_fonts

michaelfolkson commented at 7:15 am on September 3, 2020: member

Looking for Concept (N)ACKs (plus comments if you have them) from designers cc @GBKS @Bosch-0 @johnsBeharry

Bosch-0 commented at 8:09 am on September 3, 2020: none

Concept ACK

Though using Roboto Mono would be a better alternative:

IBM Plex Mono uses a OFL license which as pointed out in https://github.com/bitcoin/bitcoin/pull/17311#issuecomment-550591089 has some concerns about its ‘free’ status. Roboto mono uses Apache Licence, Version 2.0 which as far as I’m concerned isn’t an issue.
Roboto Mono is a variable font which allows it to have its properties changed (weight, width etc.) without needing a separate font file. This will keep the overall font file size down.
This may be a subjective take but I think Roboto Mono is also slightly more readable.

IBM Plex Mono

Roboto Mono

hebasto commented at 10:45 am on September 3, 2020: member

About font sizes:

0$ ls -go
1total 380
2-rw-rw-r-- 1 111532 Mar 11  2018 IBMPlexMono-Medium.ttf
3-rw-rw-r-- 1  86820 May 13  2015 RobotoMono-Medium.ttf
4-rw-rw-r-- 1 182172 May 13  2015 RobotoMono-VariableFont_wght.ttf

hebasto renamed this:
~~Add IBM Plex Mono font~~
Embed monospaced font
on Sep 3, 2020

hebasto commented at 11:48 am on September 3, 2020: member

It appears that Qt (at least 5.12.8) cannot work with variable fonts. At least QFont::setWeight() (<weight>..</weight> in *.ui file) is noop for RobotoMono-VariableFont_wght.ttf.

Bosch-0 commented at 11:59 am on September 3, 2020: none

Ahh that’s a shame, not a huge issue though, as the mono font is only going to be used in a few places the benefits of using a variable font is negligible. The static medium Roboto Mono font style should be all we need to use.

I think the licensing issue with OFL still makes Roboto Mono a more suitable option than IBM Plex Mono.

hebasto force-pushed on Sep 3, 2020

hebasto commented at 12:23 pm on September 3, 2020: member

Updated 15f37db53124f31d5499ce0604f907b9564aa506 -> 3fdd5b6bd17a679d6e3876682266092159c52d59 (pr79.01 -> pr79.02, diff):

used Roboto Mono instead of IBM Plex Mono as requested by @Bosch-0
changes are scoped only the Overview page as it is a clear visual bug fix

~Going to update OP soon.~ OP updated.

hebasto commented at 12:53 pm on September 3, 2020: member

@Sjors

Before and after screenshots on various OS’s would be nice.

Done.

I’m generally not a fan of messing with the default font choices of operating systems / desktop environments. However, if upstream QT doesn’t use a proper monospaced font for macOS, then it makes sense for at least that platform.

This is under discussion now in Bitcoin Design Community (https://github.com/BitcoinDesign, https://bitcoindesign.slack.com). Please join the discussion :)

luke-jr commented at 1:58 pm on September 3, 2020: member

Concept NACK to overriding the platform. If users don’t like their platform’s font choices, they should change it there.

Also concept NACK to moving discussion to a private proprietary platform.

moneyball commented at 4:38 pm on September 3, 2020: contributor

I agree with @luke-jr that the bitcoin design community should not require Core contributors to join Slack in order to debate changes to the Core GUI (although developers are more than welcome to join the conversation there!). I think it is fine if there is discussion on Slack or video calls or any medium, but for any kind of formal proposal or discussion or debate, it should occur on GitHub as that is the existing process for the project.

flack commented at 9:21 am on September 20, 2020: contributor

Just my two cents for the screenshot in the OP: Using monospace for the balance numbers seems like a weird workaround (weird because you have two different fonts for the same line of information, the label in the normal UI font and then the number & unit in the monospace one). It would be much nicer to use a non-monospace font throughout that supports tabular figures (see https://www.fonts.com/content/learning/fontology/level-3/numbers/proportional-vs-tabular-figures). Or doesn’t that work in Qt?

hebasto commented at 9:37 am on September 20, 2020: member

@flack

Just my two cents for the screenshot in the OP: Using monospace for the balance numbers seems like a weird workaround (weird because you have two different fonts for the same line of information, the label in the normal UI font and then the number & unit in the monospace one). It would be much nicer to use a non-monospace font throughout that supports tabular figures (see https://www.fonts.com/content/learning/fontology/level-3/numbers/proportional-vs-tabular-figures). Or doesn’t that work in Qt?

Thanks! TIL. I didn’t know about “tabular figures”.

RandyMcMillan commented at 5:18 am on December 8, 2020: contributor

@hebasto -

The Roboto Family is an excellent choice as an embedded font (imo). I believe we should be proactive and embed the entire family. It would eliminate this unknown factor in Gui design moving forward.

Keep in mind - fonts do pose a security issue. https://www.zdnet.com/article/google-open-sources-internal-tool-for-finding-font-related-security-bugs/ https://security.stackexchange.com/questions/91347/how-can-a-font-be-used-for-privilege-escalation

In essence - embedding fonts explicitly enhances determinism.

hebasto commented at 11:38 am on February 1, 2021: member

@luke-jr

Concept NACK to overriding the platform. If users don’t like their platform’s font choices, they should change it there.

Being a financial application the Bitcoin Core requires a monospaced/fixed-pitch font (or, at least, tabular figures font).

The Qt’s font matching algorithm does not guarantee that:

the monospaced font will be chosen at all (see https://github.com/bitcoin/bitcoin/pull/16432#issuecomment-514486077)
the chosen monospaced font has expected style and weight (see #87 (comment))
an upgrade of the system or Qt won’t change the Bitcoin Core look unexpectedly

A agree that user’s ability to customize fonts is good. The question is how users could customize the monospaced font for Qt application now?

I’m not aware of such possibilities on Windows 10 and macOS 11.1. On Linux I used the qt5ct package, but it fails to customize the monospaced font.

If there are no ways to customize the monospaced font for Qt application, are you going to insist on your point?

laanwj commented at 11:51 am on February 1, 2021: member

The Roboto Family is an excellent choice as an embedded font (imo). I believe we should be proactive and embed the entire family.

Agree. Despite really disliking it on a conceptual level, I think ‘ship your own fonts’ is the modern way. The less assumptions we have to make about the operating system, platform, and what is already installed, the better. This will also help in making fully static binaries. E.g. bitcoin/bitcoin#18110

(That is, for the distributed binaries. When building from source you can do whatever you like.)

michaelfolkson commented at 11:54 am on February 1, 2021: member

Concept NACK to overriding the platform. If users don’t like their platform’s font choices, they should change it there.

If this is purely a design choice (there are no negative side effects to this change) I’m happy to leave to the designers. If the designers are a Concept ACK I’m a Concept ACK.

laanwj commented at 6:17 pm on February 2, 2021: member

Code review and license review ACK 3fdd5b6bd17a679d6e3876682266092159c52d59 Please do update contrib/debian/copyright to explitily list the apache license for the embedded font files.

Changes are scoped to the Overview page only.

I think we have to replace all fixed-width fonts with this one, not just the overview page (not necessarily in thie PR ofcourse).

jonasschnelli commented at 7:37 am on February 4, 2021: contributor

Concept ACK 3fdd5b6bd17a679d6e3876682266092159c52d59 - please add the license attribution

luke-jr commented at 5:56 pm on February 4, 2021: member

The Qt’s font matching algorithm does not guarantee that:

the monospaced font will be chosen at all (see bitcoin/bitcoin#16432 (comment)) the chosen monospaced font has expected style and weight (see #87 (comment))

We already have functional workarounds for these.

an upgrade of the system or Qt won’t change the Bitcoin Core look unexpectedly

That’s desirable and expected behaviour.

I’m not aware of such possibilities on Windows 10 and macOS 11.1.

Crazy. It looks like Windows 10 removed the ability for users to configure their fonts. But that’s a Windows problem, not necessarily ours…

I’m not surprised Apple denies users choice.

Note that your suggestions here do NOT restore user choice, but instead make it even harder for them to configure it!

On Linux I used the qt5ct package, but it fails to customize the monospaced font.

It’s right there in System Settings for me, as a “Fixed width” font choice.

hebasto force-pushed on Feb 21, 2021

DrahtBot added the label Needs rebase on Feb 21, 2021

gui: Add Roboto Mono font 89e421918e

qt: Make GUIUtil::fixedPitchFont aware of embedded font 623de12d04

qt: Choose monospaced font in C++ code rather in `*.ui` file

Setting the "Monospace" font family in a `*.ui` file does not work on
macOS, at least on Big Sur with Qt 5.15 (neither via the "font" property
nor via the "styleSheet" property). Qt chooses the ".AppleSystemUIFont"
instead of ".AppleSystemUIFontMonospaced".

This change makes macOS choose the correct monospaced font.

22e0114d05

gui: Add monospaced font settings 67f26319a0

hebasto force-pushed on Feb 21, 2021

hebasto commented at 7:10 pm on February 21, 2021: member

Updated 3fdd5b6bd17a679d6e3876682266092159c52d59 -> 67f26319a0ca7e34e3db17d9330133622a40de09 (pr79.02 -> pr79.04):

rebased due to the conflict with #115
added the license attribution as requested by @laanwj and @jonasschnelli
added an option to opt-out embedded monospaced font in Menu -> Options-> Display as requested by @luke-jr (as I understand his idea)

luke-jr commented at 7:15 pm on February 21, 2021: member

added an option to opt-out embedded monospaced font in Menu -> Options-> Display as requested by @luke-jr (as I understand his idea)

That seems like a reasonable compromise.

Can you also not add the font to the repo, make it optional at build time (perhaps configurable?), and bundle it via depends?

hebasto commented at 7:33 pm on February 21, 2021: member

Can you also not add the font to the repo, make it optional at build time (perhaps configurable?), and bundle it via depends?

What reasons do you see for that?

luke-jr commented at 7:40 pm on February 21, 2021: member

So that people who don’t want this can build without it…

DrahtBot removed the label Needs rebase on Feb 21, 2021

laanwj commented at 12:00 pm on February 22, 2021: member

Tested ACK 67f26319a0ca7e34e3db17d9330133622a40de09

The option in the configuration dialog is really nicely done.

A configure option to build without internal fonts can be added later. IMO adding the font to the repository itself is fine. It’s only 87kB, not worth the overhead of an external depends download.

laanwj merged this on Feb 22, 2021

laanwj closed this on Feb 22, 2021

hebasto deleted the branch on Feb 22, 2021

rebroad commented at 10:55 pm on May 1, 2021: contributor

I’m a little confused, what version is the “before” picture from? I’m running 0.21 and it’s already a fixed font.

hebasto commented at 7:35 am on May 2, 2021: member

@rebroad

I’m a little confused, what version is the “before” picture from? I’m running 0.21 and it’s already a fixed font.

I suppose your system is not macOS, right?

hebasto referenced this in commit 3ad1b8899b on May 25, 2021

bitcoin-core locked this on Aug 16, 2022

Embed monospaced font #79