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

qt: Fix font size and scaling issues #3734

Merged
merged 30 commits into from
Sep 28, 2020

Conversation

xdustinface
Copy link

This PR should fix all remaining places where the font scale wasn't applied yet (besides the splashscreen which probably also should follow anytime later). It also in general improves the font-scaling process and from now on it only updates fonts where really required. See commit messages.

@thephez This should fix all the issues you pointed out in slack
@PastaPastaPasta 64ce1b5 might fix the one you sent me. At least it makes it better here on mac where i had it similar like you with a low font scale though.
@strophy 82c1612 could be a fix for your "I can't get the same menu size like in other applications" thing

@xdustinface xdustinface added this to the 16 milestone Sep 25, 2020
@PastaPastaPasta
Copy link
Member

Stuff still getting cropped in Network traffic for me

cropped

@xdustinface
Copy link
Author

@PastaPastaPasta Please look at the network traffic stats again with 3c52ea3

@PastaPastaPasta @UdjinM6 I rebased this PR now on #3717 to include 0156d4d in here as it seems to make more sense in here. Should fix the min width issue. Thoughts?

@xdustinface xdustinface changed the title qt: Fix font scaling issues qt: Fix font size and scaling issues Sep 26, 2020
@xdustinface
Copy link
Author

Okaay one more.. see the last three commits i changed it to resize depending on the font size. Now if any of the appearance attributes gets changed or if the PS UI gets shown/hidden the OptionsDialog gets resized to the required size.

@UdjinM6
Copy link

UdjinM6 commented Sep 26, 2020

Seems to be working as expected for me (could use QTimer::singleShot(0, this, SLOT(updateWidth())); instead of calling it directly in ctor to fix issues with buttons visibility not being available at that point but 3170db0 is fine ok-ish too :) ).

EDIT: Technically speaking though, you could one day want a visible but disabled tab there and 3170db0 won't work.

@xdustinface
Copy link
Author

xdustinface commented Sep 26, 2020

Seems to be working as expected for me (could use QTimer::singleShot(0, this, SLOT(updateWidth())); instead of calling it directly in ctor to fix issues with buttons visibility not being available at that point but 3170db0 is fine ok-ish too :) ).

Does the single shot timer 100% guaranteed only fire after the widgets visibility state flipped? I have no idea just curious.

EDIT: Technically speaking though, you could one day want a visible but disabled tab there and 3170db0 won't work.

Dropped 3170db0 and added 75f4389 that should do it better

EDIT: Forgot to include the header file into the commit hence the second force push... 🙈

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

Does the single shot timer 100% guaranteed only fire after the widgets visibility state flipped?

Yes, this timer event should be the last one in the event queue afaik

Dropped 3170db0 and added 75f4389 that should do it better

Mostly works for me :) two suggestions, see below

src/qt/optionsdialog.cpp Show resolved Hide resolved
src/qt/optionsdialog.h Outdated Show resolved Hide resolved
@UdjinM6
Copy link

UdjinM6 commented Sep 26, 2020

Working as expected for me now (but I had no issues with Network traffic before, so can't say if it was also improved or not).

@xdustinface
Copy link
Author

Working as expected for me now (but I had no issues with Network traffic before, so can't say if it was also improved or not).

Yeah i also had no issues with it since it was not looking bad on macOS but now it should just scale as expected based on the really required size not on the "on macOS required" size :D

Added three more commits:

d94655e + 62a6cbc To give the main window the same resizing behaviour depending on the enabled tabs like in the options window. For me on my system the 980 min width is reasonable but not sure if other systems require more there? @PastaPastaPasta how is min size looking there if PS + Masternode are hidden?

35dc39b To allow manual resizing after the window has been resized dynamically after restart/reopen or appearance adjustments.

@PastaPastaPasta
Copy link
Member

Below are the only few remaining problems I could find on really edge case font settings. Don't think it's critical to get these fixed

Screenshot from 2020-09-26 15-31-18

###########################################

Screenshot from 2020-09-26 15-28-33

###########################################

Screenshot from 2020-09-26 15-27-59

###########################################

Screenshot from 2020-09-26 15-27-40

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

ACK

@xdustinface
Copy link
Author

Two more fixes:

c5390d5 To make sure all widgets receive the font weight updates if "font-weight-normal" gets updated.

95a1698 To make sure a widget which for example had a bold font initially gets updated to normal font if requested. Like all tab-like buttons we have, the currently clicked one is bold, the others normal. Without this commit they don't update properly if the font-weight gets adjusted in appearance tab.

Those behaviours were broken by the changes of this PR, e.g. 2bfe078 and 41524ea

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

ACK

@UdjinM6
Copy link

UdjinM6 commented Sep 28, 2020

Needs rebase

@xdustinface
Copy link
Author

Needs rebase

Done

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

See below + (maybe) d734bf1 + found 2 issues:

  1. overlay and status bar font issue: start the wallet (with --regtest to have the overlay popup shown), go to Options, change font size to lowest, close, start again (everything seems normal), go to Options and change font size to the largest. Everything else is huge but not overlay and status bar texts, it's like they have some medium size font now. Restart the wallet and now they are huge too. Same works the other way around: start with huge fonts, change to tiniest (medium size for overlay and status bar again), restart (now they are tiny too).

  2. by tweaking PS visibility and width on start we override main window geometry settings. Should either respect it somehow maybe or stop saving/loading it cause it doesn't make a lot of sense otherwise.

EDIT: as discussed in Slack, n1 is an edge case and window geometry in n2 is still useful for window positioning, both are ok to fix them later if it's not smth trivial.

src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved
src/qt/optionsdialog.cpp Outdated Show resolved Hide resolved
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

ACK

@xdustinface
Copy link
Author

xdustinface commented Sep 28, 2020

See below + (maybe) d734bf1 + found 2 issues:

In general im with you, having a smaller min size is preferable but i would rather revisit this sometimes later i tested it a bit and it breaks things with other font sizes and seems to mess a bit with traditional too.

The other commits i just added should fix the remaining scaling issues + they remove (as discussed in slack) the fixed pitch font. About the scaling: The previous default font implementation was not optimal because widgets without a object name set received an empty key from getKey(). The initial idea there was to only add the default font size for an instance of a class /UI element only once not for each instance. Now with d82c5d7 they are stored for the actual instance and removed again if the instance is not longer available in the UI. And cb23aae makes sure that there is no redundant/double scaling due to font size inheritance.

EDIT: Just noticed i didn't drop a964fe6 before i pushed. Actually i wanted to open another PR for that because it seems a bit unrelated here.. But the arg call is about font size at least so its kiiiind of related.. want me to remove it? Let me know.

@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Sep 28, 2020

Nah, I think a964fe6 is related enough. It's fine

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

ACK

xdustinface added a commit to xdustinface/dash that referenced this pull request Sep 28, 2020
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

👍

ACK

@UdjinM6 UdjinM6 merged commit 4cb2af8 into dashpay:develop Sep 28, 2020
xdustinface added a commit to xdustinface/dash that referenced this pull request Sep 28, 2020
xdustinface added a commit to xdustinface/dash that referenced this pull request Sep 29, 2020
* qt: Make sure font size in MasternodeList gets scaled as expected

* qt: Make sure font size in ShutdownWindow gets scaled as expected

* qt: Drop obsolete application font updates

* qt: Scale QMenu and QMessageBox globally

To make sure non-custom context menus + QMessageBox instances createy by
static calls like QMessageBox::critical are scaled also.

* qt: Avoid redundant scaling for tooltips and menus

* qt: Only update widget's font if required

* qt: Merge GUIUtil::mapFontSizeUpdates into GUIUtil::mapNormalFontUpdates

* qt: Remove obsolete setFixedPitchFont call

* qt: Use setFixedPitchFont in SendCoinsEntry

* qt: Scale font size in increments of 0.25

* qt: Properly scale network traffic stats depending on font metrics

* qt: Update min/max width of OptionsDialog depending on buttons width

* qt: Emit a signal whenever any attribute of AppearanceWidget changed

* qt: Update OptionsDialog width if the appearance changed

* qt: Calculate the initial wide right after the window showed up

Make sure the visibility state of the widgets is correct before width calculations.

* qt: Call parent class showEvent + override it explicit

Co-authored-by: UdjinM6 <[email protected]>

* qt: Let OptionsDialog emit a signal if appearance gets changed

* qt: Resize main toolbar depending on visible buttons / font attributes

* qt: Reset max width after it has been set to still allow window resizing

* qt: Properly update the weight of widgets with default font attributes

* qt: Handle updates to the font attributes

* qt: Use resize() instead of setMaximumWidth()

Co-authored-by: UdjinM6 <[email protected]>

* qt: Call GUIUtil::updateFonts in ModalOverlay constructor

* qt: Make sure default fonts are stored properly for the related widget

* qt: Ignore some low level classes in GUIUtil::updateFont

* rpc: Remove obsolete `.arg()` call

* qt: Drop fixedPitchFont

* qt: Avoid redundant font updates. Let GUIUtil::updateFont handle them

* qt: Scale recent transactions on OverviewPage

They were scaled by font inheritance before

* qt: Ignore QListView in GUIUtil::updateFonts
@strophy
Copy link

strophy commented Oct 1, 2020

No, the problem with different size font in the menu and title bar remains. It is impossible to get them to match, and I think the reason is they are using different layout engines. The kerning looks really bad, letters have too much or not enough spacing on each side, icons in the menu are missing padding and it generally just doesn't feel "native" for Windows. I can count 6 different font sizes and weights in this screenshot alone, not counting the Dash logo...
image

@bitterseed
Copy link

bitterseed commented Oct 1, 2020

dash

Also having some scaling issues on ubuntu with a 1920x1080 display
Zooming in until my toolbar matches yours will give you a good approximation of what it looks like on my display

This issue has been around since 0.15, at which point I believe some hi-res themes were removed. I believe having 'hi-res' in the config file also causes dashcore to fail when loading

Cheers

gades pushed a commit to cosanta/cosanta-core that referenced this pull request Mar 8, 2022
* qt: Make sure font size in MasternodeList gets scaled as expected

* qt: Make sure font size in ShutdownWindow gets scaled as expected

* qt: Drop obsolete application font updates

* qt: Scale QMenu and QMessageBox globally

To make sure non-custom context menus + QMessageBox instances createy by
static calls like QMessageBox::critical are scaled also.

* qt: Avoid redundant scaling for tooltips and menus

* qt: Only update widget's font if required

* qt: Merge GUIUtil::mapFontSizeUpdates into GUIUtil::mapNormalFontUpdates

* qt: Remove obsolete setFixedPitchFont call

* qt: Use setFixedPitchFont in SendCoinsEntry

* qt: Scale font size in increments of 0.25

* qt: Properly scale network traffic stats depending on font metrics

* qt: Update min/max width of OptionsDialog depending on buttons width

* qt: Emit a signal whenever any attribute of AppearanceWidget changed

* qt: Update OptionsDialog width if the appearance changed

* qt: Calculate the initial wide right after the window showed up

Make sure the visibility state of the widgets is correct before width calculations.

* qt: Call parent class showEvent + override it explicit

Co-authored-by: UdjinM6 <[email protected]>

* qt: Let OptionsDialog emit a signal if appearance gets changed

* qt: Resize main toolbar depending on visible buttons / font attributes

* qt: Reset max width after it has been set to still allow window resizing

* qt: Properly update the weight of widgets with default font attributes

* qt: Handle updates to the font attributes

* qt: Use resize() instead of setMaximumWidth()

Co-authored-by: UdjinM6 <[email protected]>

* qt: Call GUIUtil::updateFonts in ModalOverlay constructor

* qt: Make sure default fonts are stored properly for the related widget

* qt: Ignore some low level classes in GUIUtil::updateFont

* rpc: Remove obsolete `.arg()` call

* qt: Drop fixedPitchFont

* qt: Avoid redundant font updates. Let GUIUtil::updateFont handle them

* qt: Scale recent transactions on OverviewPage

They were scaled by font inheritance before

* qt: Ignore QListView in GUIUtil::updateFonts

Co-authored-by: UdjinM6 <[email protected]>
PastaPastaPasta added a commit that referenced this pull request Dec 10, 2024
e080686 fix: don't shrink window when setting minimum width (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Closes #5886
  * We've been setting restrictions on the width of the window since [dash#3734](#3734) (see d351fca and c0447b0), meaning, the behavior as described in [dash#5886](#5886) has been present since v0.16 (v0.15 and earlier used the old UI design).
    * We do not set any restrictions on height and in that respect, Dash Qt will behave similar to Bitcoin Qt in that it can be resized to be arbitrarily small.

  ## How Has This Been Tested?

  Crossed compiled for `x86_64-w64-mingw32` and tested resulting `dash-qt.exe` on Windows 11 22H2 (Build 22621.431). Followed reproduction steps as mentioned in [dash#5886](#5886), found width to persist even after closing Options modal using buttons and the close button on the top right.

  ## Breaking Changes

  None expected.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    generally utACK e080686
  UdjinM6:
    utACK e080686

Tree-SHA512: ed3874c830c0ddf1a9e1166b75e3088969b7dfae8bca25af47045727e14a270ca2bb740e2c30b657b26a4b8e542dff8d898e5d5d2c9482d67da15578e0054632
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants