Skip to content

Conversation

@jonasschnelli
Copy link
Contributor

should fix #5897

@jonasschnelli
Copy link
Contributor Author

Font is no slightly bigger on my mac. But totally okay.

After:
bildschirmfoto 2015-03-14 um 12 39 03

Before:
bildschirmfoto 2015-03-14 um 12 41 55

@microcai
Copy link

works for me.

@microcai
Copy link

Linux/X11 KDE environment: DPI = 192

screenshot before this patch applied

screenshot_bitcoin

screenshot with this patch applied
screenshot_bitcoin2

@jonasschnelli
Copy link
Contributor Author

@microcai hmm... looks good but i don't have a idea why your font size of the time-cell has also changed.

@microcai
Copy link

@jonasschnelli I tried to use standard zoom method Ctrl+Key_PLUS

@luke-jr
Copy link
Member

luke-jr commented Mar 14, 2015

What does font-size:1em do differently than omitting the attribute?

@Diapolo
Copy link

Diapolo commented Mar 14, 2015

Can you check if we use hard-coded px sizes in other places? Perhaps it's time to replace them?

@jonasschnelli
Copy link
Contributor Author

@luke-jr: Good question. 1em as font-size should be the default if there is no style-changing elsewhere. Will check this soon.

@Diapolo: The question is how Qt deals with HiDPI. On OSX 12px font size will result in 24px on screen in HiDPI (as expected). On linux we just saw that it will stay on 12px. There (fonts) em metrics makes sense. For other hard-coded px values (widgets size, offsets) it could end up value-doubles on HiDPI depending of Qt's handling.
So yes for changing all font-sizes to em, no for px widget sizes and offsets.

@microcai
Copy link

to make things clear, zoom in pixel level is only supported in osx platform. both Linux and windows version of Qt just uses DPI. so 12px font stay in 12px font even under windows.

@luke-jr
Copy link
Member

luke-jr commented Mar 15, 2015

I would expect Qt to default to the user font size preference.

@jonasschnelli jonasschnelli force-pushed the 2015/03/qt_fix_font_size branch from dff6013 to 046bc52 Compare March 15, 2015 12:42
@jonasschnelli
Copy link
Contributor Author

as @luke-jr proposed i just removed the font-size: 1em setting. This should result in the same behavior maybe it also detects some way of using the standard os font-sizes.
@microcai could you test again?

@microcai
Copy link

@jonasschnelli I alread did.

@jonasschnelli
Copy link
Contributor Author

@microcai hows the result? without 1em same as with? thanks.

@microcai
Copy link

@jonasschnelli just see the screenshot. there is barely visible difference with "1em" or without "1em"

@microcai
Copy link

@jonasschnelli 1em seems to be the default font size. so I can't really tell the difference either 1em or omit it altogether.

@sipa sipa added the GUI label Mar 16, 2015
@laanwj
Copy link
Member

laanwj commented Mar 16, 2015

Looks good to me, although I wonder why we ever added a fixed font size (in pixels!), that must have been a workaround for something.

@fanquake
Copy link
Member

@laanwj Looks like it was introduced here c6aa86a, nearly 3 years ago.

@laanwj
Copy link
Member

laanwj commented Mar 18, 2015

OK, so I added it myself when I introduced this code. I don't remember why.

@laanwj
Copy link
Member

laanwj commented Mar 18, 2015

Trying it out jogged my memory. Without font-size, and with font-size:1em it looks like:
untitled

The default monospace font size on Ubuntu is huge.

@luke-jr
Copy link
Member

luke-jr commented Mar 18, 2015

Meh, that's not too huge. If the terminal looks the same, I'd say it's expected. Surely Ubuntu lets users configure their defaults?

@jonasschnelli
Copy link
Contributor Author

@laanwj It seems like that font-size: <n>em; does not have any effect. Or at least with values <1em. I tried 0.8 and 0.1em. Looks exact the same as with 1em. I also tried 10pt. Looks also good but generate font-size difference between ubuntu/linux and osx.

I created a HiDPI ubuntu VM and did take a closer look at bitcoin-qt HiDPI unter linux. There are many issues (icon sizes, splash-screen, overview screen, etc., etc,)

This PR goes into the right direction (removing fix px values) but will not solve the whole HiDPI issue.

@microcai
Copy link

@jonasschnelli what about using 12pt ?

@jonasschnelli
Copy link
Contributor Author

@microcai using 12pt would enlarge the font-size on linux as well as on osx. 10pt would make sense but even there the font-size is about ~10-20% larger.
IMO we should just remove the font-size as it is in this PR.

@laanwj
Copy link
Member

laanwj commented Mar 19, 2015

I really don't like the default font size on Linux. @luke-jr If you compare it with the normal, non-monospace font as used for the time and the tab titles it IS huge.

Thinking of that, maybe we should remove the monospace? Looks good here.

@luke-jr
Copy link
Member

luke-jr commented Mar 19, 2015

I want to compare it with the terminal program. Whenever possible, we should use an OS/user-defined look and feel, not try to second-guess it.

@laanwj
Copy link
Member

laanwj commented Mar 19, 2015

The terminal, at least on Ubuntu, uses a less ugly font and also more
reasonably sized than whatever Qt chooses with 'monospace'. It's not
realistic to expect it to pick the same font as the OS's terminal.

We have to make our own font decision here. There, picking using the
default (non-monospace) font seems safest.

@jonasschnelli
Copy link
Contributor Author

non-monospace would look like this (see below).
I don't have a strong position here. There is not much of a difference between monospace and standard font in terms of usability of the rpc console.

bildschirmfoto 2015-03-19 um 21 40 09

@laanwj
Copy link
Member

laanwj commented Mar 24, 2015

Looks good to me. We don't (as far as I see) use the fixed width property anywhere for formatting, so a proportional font does just as well and may even be more readable.

@jonasschnelli jonasschnelli force-pushed the 2015/03/qt_fix_font_size branch from 046bc52 to c816833 Compare March 27, 2015 08:50
@jonasschnelli
Copy link
Contributor Author

Removed the "monospace" font style. IMO safest solution.

@laanwj laanwj merged commit c816833 into bitcoin:master Mar 30, 2015
laanwj added a commit that referenced this pull request Mar 30, 2015
c816833 [Qt] fix rpc console font size to flexible metrics (Jonas Schnelli)
@dexX7 dexX7 mentioned this pull request Oct 22, 2015
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

font size too small in debug console.

7 participants