Skip to content

qt: Handle fonts of deleted widgets properly, streamline the flow in GUIUtil::updateFonts#3772

Merged
UdjinM6 merged 8 commits into
dashpay:developfrom
UdjinM6:fix3767
Oct 28, 2020
Merged

qt: Handle fonts of deleted widgets properly, streamline the flow in GUIUtil::updateFonts#3772
UdjinM6 merged 8 commits into
dashpay:developfrom
UdjinM6:fix3767

Conversation

@UdjinM6
Copy link
Copy Markdown

@UdjinM6 UdjinM6 commented Oct 13, 2020

Fixes #3767

Use QPointer-s to process deleted widgets properly, streamline the flow (less loops and map scans).
@UdjinM6 UdjinM6 added this to the 16.1 milestone Oct 13, 2020
@UdjinM6 UdjinM6 changed the title qt: Handle deleted widgets font properly, streamline the flow in GUIUtil::updateFonts qt: Handle fonts of deleted widgets properly, streamline the flow in GUIUtil::updateFonts Oct 13, 2020
@UdjinM6 UdjinM6 marked this pull request as ready for review October 13, 2020 14:57
@xdustinface
Copy link
Copy Markdown

@UdjinM6 I can confirm this solves the issues pointed out in #3767 (FYI, i was able to reproduce it on macOS also).

But this fix introduces a new scaling bug due to font inheritance (Just start the wallet with a very low/high font scale to reproduce it in most screens).

See suggestion-3772 which contains a fix (8c075b9) for the introduced issue and also some more refactoring in GUIUtil::updateFonts.

@UdjinM6
Copy link
Copy Markdown
Author

UdjinM6 commented Oct 22, 2020

@xdustinface Good catch! Cherry-picked them all 👍

Copy link
Copy Markdown

@xdustinface xdustinface left a comment

Choose a reason for hiding this comment

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

ACK

Comment thread src/qt/guiutil.cpp
++nUpdatable;

QFont font = w->font();
assert(font.pointSize() > 0);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I really don't love using asserts... If they do catch they provide basically no info to the user for us to debug with...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

>0 means that it's either a default font size (set via setApplicationFont) or some another valid value and font size was not set via setPixelSize. IMO it's safe to use it here, should only crash if we break smth in code (e.g. use setPixelSize).

Copy link
Copy Markdown
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.

utACK

@PastaPastaPasta PastaPastaPasta linked an issue Oct 27, 2020 that may be closed by this pull request
@UdjinM6 UdjinM6 merged commit e730732 into dashpay:develop Oct 28, 2020
UdjinM6 added a commit to UdjinM6/dash that referenced this pull request Oct 28, 2020
…`GUIUtil::updateFonts` (dashpay#3772)

* Refactor and fix `GUIUtil::updateFonts`

Use QPointer-s to process deleted widgets properly, streamline the flow (less loops and map scans).

* Add some debug output

* qt: Rename mapNormalFontUpdates -> mapFontUpdates

The "Normal" was added when we also had other maps containing font updates

* qt: Count removed items, adjust debug logs

* qt: Use the emplace result for the default font size

* qt: Perform all widget font updates later in a seperate step

* qt: Drop pointSize checks

* qt: Refactor app class font scaling

Co-authored-by: xdustinface <xdustinfacex@gmail.com>
@UdjinM6 UdjinM6 deleted the fix3767 branch November 26, 2020 13:26
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Mar 10, 2022
…`GUIUtil::updateFonts` (dashpay#3772)

* Refactor and fix `GUIUtil::updateFonts`

Use QPointer-s to process deleted widgets properly, streamline the flow (less loops and map scans).

* Add some debug output

* qt: Rename mapNormalFontUpdates -> mapFontUpdates

The "Normal" was added when we also had other maps containing font updates

* qt: Count removed items, adjust debug logs

* qt: Use the emplace result for the default font size

* qt: Perform all widget font updates later in a seperate step

* qt: Drop pointSize checks

* qt: Refactor app class font scaling

Co-authored-by: xdustinface <xdustinfacex@gmail.com>
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.

[qt] Abnormal non-deterministic scaling of Settings text

3 participants